Merged
Conversation
warengonzaga
added
library
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWorkspace-wide patch bumps to 1.0.1, marks many packages private, adds an update-checker (with cache and prompt context) integrated into CLI and agent loop, adds Husky commit hooks and commit-message validator, updates CI workflows, and introduces a new Svelte landing site. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Startup
participant UpdateChecker as Update Checker
participant FS as File System (cache)
participant NPM as NPM Registry
participant Agent as Agent Loop
CLI->>UpdateChecker: checkForUpdate(currentVersion, dataDir)
activate UpdateChecker
UpdateChecker->>FS: readCache(dataDir)
FS-->>UpdateChecker: cached UpdateInfo or null
alt cache fresh
UpdateChecker-->>CLI: cached UpdateInfo
else cache miss/stale
UpdateChecker->>UpdateChecker: detectRuntime()
UpdateChecker->>NPM: fetchLatestVersion() (5s timeout)
NPM-->>UpdateChecker: latest version or null
UpdateChecker->>UpdateChecker: isNewerVersion(current, latest)
UpdateChecker->>FS: writeCache(dataDir, UpdateInfo)
UpdateChecker-->>CLI: UpdateInfo (or null)
end
deactivate UpdateChecker
CLI->>UpdateChecker: buildUpdateContext(UpdateInfo)
UpdateChecker-->>CLI: updateContext string
CLI->>Agent: attach updateContext
Agent->>Agent: inject updateContext into system prompt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compactor/package.json (1)
1-38:⚠️ Potential issue | 🟡 MinorAdd
"type": "module"for consistency with the monorepo — compactor uses ESM syntax.All 18 sibling packages declare
"type": "module", but compactor is missing this field. The source code uses ESM syntax (import/export statements,.jsfile extensions in relative paths), making this field necessary for correct module resolution in the monorepo context.🔧 Proposed fix
"name": "@tinyclaw/compactor", "private": true, "version": "1.0.1", "description": "Layered conversation compaction and token optimization for Tiny Claw", "license": "GPL-3.0", "author": "Waren Gonzaga", + "type": "module", "main": "dist/index.js",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/compactor/package.json` around lines 1 - 38, Add the missing top-level "type": "module" field to this package's package.json so the ESM import/export syntax and .js relative imports resolve consistently with the other monorepo packages; update the JSON object in packages/compactor/package.json (near the existing fields like "name", "private", "version", "main", "types") to include "type": "module".
🧹 Nitpick comments (1)
packages/core/tests/update-checker.test.ts (1)
216-222: Network-dependent tests may be flaky in isolated CI environments.Tests at lines 216–222 and 224–230 rely on
checkForUpdatemaking a real HTTP call to the npm registry. In offline or firewalled CI runners, these tests won't fail (thanks to theresult === null || typeof result === 'object'guard), but they also won't exercise the fetch path. Consider mockingfetch(e.g., viaglobalThis.fetch = ...in abeforeEach) to deterministically test both success and failure paths.Not critical since the current assertions are defensive, but it would improve test confidence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/update-checker.test.ts` around lines 216 - 222, The tests calling checkForUpdate in update-checker.test.ts are relying on real network calls; replace that by mocking globalThis.fetch in a beforeEach and restoring it in an afterEach so you can deterministically simulate both success and failure paths for the tests at lines around the checks (the tests invoking checkForUpdate with tempDir); implement one mock that returns a resolved Response-like object with the expected registry JSON for the success case and another that throws or returns an error response for the network-failure case, ensuring the tests assert the proper null or object behavior without using the network.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/update-checker.ts`:
- Around line 94-102: The parse logic in isNewerVersion currently maps
pre-release strings like "1.0.0-beta.1" to NaN for the patch, corrupting
comparisons; update the parse function inside isNewerVersion to first strip any
pre-release or build metadata (remove anything from the first '-' or '+' onward
and also trim a leading "v"), then split the remainder on '.' and convert parts
to numbers, coercing NaN to 0 (e.g., via Number(...) and fallback to 0 or using
isNaN checks), and then slice(0,3) to get MAJOR, MINOR, PATCH. This ensures
isNewerVersion compares stable numeric parts correctly when inputs include
prerelease/build suffixes.
---
Outside diff comments:
In `@packages/compactor/package.json`:
- Around line 1-38: Add the missing top-level "type": "module" field to this
package's package.json so the ESM import/export syntax and .js relative imports
resolve consistently with the other monorepo packages; update the JSON object in
packages/compactor/package.json (near the existing fields like "name",
"private", "version", "main", "types") to include "type": "module".
---
Nitpick comments:
In `@packages/core/tests/update-checker.test.ts`:
- Around line 216-222: The tests calling checkForUpdate in
update-checker.test.ts are relying on real network calls; replace that by
mocking globalThis.fetch in a beforeEach and restoring it in an afterEach so you
can deterministically simulate both success and failure paths for the tests at
lines around the checks (the tests invoking checkForUpdate with tempDir);
implement one mock that returns a resolved Response-like object with the
expected registry JSON for the success case and another that throws or returns
an error response for the network-failure case, ensuring the tests assert the
proper null or object behavior without using the network.
🛠️ Container Build Complete - Dev BuildBuild Status: ✅ Success 📦 Pull ImageDocker Hub: docker pull warengonzaga/tinyclaw:dev-7c8f470GHCR: docker pull ghcr.io/warengonzaga/tinyclaw:dev-7c8f470📋 Build Details
🏷️ Image Tags• 🔍 Testing Your Changes
🚀 Quick Start# Pull and run the container
Docker Hub: docker pull warengonzaga/tinyclaw:dev-7c8f470
docker run <your-options> <image>🔒 Security Scan Results📋 Pre-Build Security Checks✅ Source Code Scan: 0 vulnerabilities found 🐳 Container Image Vulnerabilities
📊 Detailed Security ReportsView detailed vulnerability reports in the GitHub Security tab. 🤖 Powered by Container Build Flow Action v1.2.0 |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.0.1-dev.ecde7d1 @tinyclaw/plugins@1.0.1-dev.ecde7d1 @tinyclaw/plugin-channel-discord@1.0.1-dev.ecde7d1 @tinyclaw/plugin-channel-friends@1.0.1-dev.ecde7d1 @tinyclaw/plugin-provider-openai@1.0.1-dev.ecde7d1 @warengonzaga/tinyclaw@1.0.1-dev.ecde7d1This package was built automatically by the Package Build Flow action. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/release.yml (2)
26-43:⚠️ Potential issue | 🟠 Major
BEFORE_SHAis the push HEAD, not the pre-push base — multi-commit pushes can silently skip releases.
github.event.workflow_run.head_commit.idis the SHA of the last commit in the push (i.e., HEAD), soBASE="${BEFORE_SHA}~1"is always equivalent toHEAD~1— identical to the fallback. Both if-branches produce the same result, meaning the diff always spans only the final commit. Ifpackage.jsonwas changed in an earlier commit of a multi-commit push but not the last one,changed=falseis emitted and no release fires.The intended pre-push base SHA is not directly available in
workflow_runcontext. A common workaround is to grep all commits in the push range rather than a single-commit diff, or persist the pre-push SHA as a workflow artifact from the CI run.💡 Suggested approach using the push commit range
- env: - BEFORE_SHA: ${{ github.event.workflow_run.head_commit.id }} - run: | - # Prefer GitHub-provided refs when available, fall back to HEAD~1 - if [ -n "$BEFORE_SHA" ] && git cat-file -e "${BEFORE_SHA}^" 2>/dev/null; then - BASE="${BEFORE_SHA}~1" - elif git cat-file -e HEAD~1 2>/dev/null; then - BASE="HEAD~1" - else - # Initial commit or unreachable parent — treat as changed - echo "changed=true" >> $GITHUB_OUTPUT - exit 0 - fi - if git diff "$BASE" HEAD --name-only | grep -qE 'package\.json$'; then - echo "changed=true" >> $GITHUB_OUTPUT - else - echo "changed=false" >> $GITHUB_OUTPUT - fi + env: + HEAD_SHA: ${{ github.event.workflow_run.head_commit.id }} + BASE_SHA: ${{ github.event.workflow_run.head_branch }} + run: | + if git cat-file -e HEAD~1 2>/dev/null; then + # Walk all commits reachable from HEAD but not already released + # by comparing against the merge-base with the previous tag + PREV_TAG=$(git describe --tags --abbrev=0 HEAD~1 2>/dev/null || echo "") + if [ -n "$PREV_TAG" ]; then + BASE="$PREV_TAG" + else + BASE="HEAD~1" + fi + else + echo "changed=true" >> $GITHUB_OUTPUT + exit 0 + fi + if git diff "$BASE" HEAD --name-only | grep -qE 'package\.json$'; then + echo "changed=true" >> $GITHUB_OUTPUT + else + echo "changed=false" >> $GITHUB_OUTPUT + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 26 - 43, The current logic treats BEFORE_SHA (github.event.workflow_run.head_commit.id) as a single-parent base and sets BASE="${BEFORE_SHA}~1", which only diffs the final commit and misses earlier commits in a multi-commit push; change the logic to build and diff a commit range instead: when BEFORE_SHA is set use a range like "${BEFORE_SHA}..HEAD" (and fall back to "HEAD~1..HEAD" or initial-commit handling if no BEFORE_SHA), then run git diff "$RANGE" --name-only (instead of git diff "$BASE" HEAD) to detect changes to package.json; keep the existing fallbacks that mark changed=true for initial commits and write results to GITHUB_OUTPUT.
9-13:⚠️ Potential issue | 🟠 MajorRemove orphaned
packages: writeandsecurity-events: writepermissions.With the container and package jobs removed, no step in this workflow publishes to GitHub Packages or uploads SARIF/security events. The
wgtechlabs/release-build-flow-actiondocumentation explicitly requires onlycontents: writeto create releases and changelogs. Retaining these unused elevated permissions violates the principle of least privilege.Proposed fix
permissions: contents: write - packages: write - security-events: write pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 9 - 13, Remove the unused elevated permissions from the workflow by deleting the "packages: write" and "security-events: write" entries under the permissions block, keeping only the required "contents: write" (and existing "pull-requests: write") as the action only needs contents write to create releases/changelogs; update the permissions block so it no longer lists packages or security-events.
🧹 Nitpick comments (7)
.github/workflows/commit-lint.yml (2)
23-25: The commit-message regex is duplicated between this workflow and.husky/validate-commit-msg.mjs.The pattern on Line 25 mirrors the one in the Husky validation script. If either is updated independently, they'll drift. Consider extracting the pattern to a shared source (e.g., a JSON config or a small shared script) that both the CI workflow and the Husky hook consume.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/commit-lint.yml around lines 23 - 25, The PATTERN regex is duplicated between .github/workflows/commit-lint.yml (variable PATTERN) and .husky/validate-commit-msg.mjs; extract the shared commit-message regex into a single source (e.g., a JSON/YAML config or small JS module) and update both commit-lint.yml and .husky/validate-commit-msg.mjs to read the pattern from that shared file instead of hardcoding PATTERN, ensuring both the workflow and the Husky hook reference the same unique symbol (the shared config key or exported constant) so updates remain in one place.
27-28: Unquoted SHA interpolation in shell — safe here but fragile.
${{ github.event.pull_request.base.sha }}is injected directly into the shell command. GitHub SHAs are hex-only, so this isn't exploitable, but it's a good practice to wrap expression interpolations in quotes to guard against future refactors that might introduce user-controlled values:- COMMITS=$(git log --format="%s" ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}) + COMMITS=$(git log --format="%s" "${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/commit-lint.yml around lines 27 - 28, Wrap the SHA interpolations in quotes in the git log invocation to avoid brittle shell parsing: when building COMMITS from git log, quote the expressions github.event.pull_request.base.sha and github.event.pull_request.head.sha so the git log command uses "${{ github.event.pull_request.base.sha }}" and "${{ github.event.pull_request.head.sha }}"; this keeps the git log call and the COMMITS assignment robust against future changes or whitespace-containing values..husky/pre-commit (1)
1-1: Running the full test suite on every commit may slow down developer workflow.Consider using
bun test --bailto fail fast on first error, or scoping to only changed files. If the test suite grows, this hook could become a bottleneck. Developers can alwaysgit commit --no-verifyto bypass, but that undermines the gate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.husky/pre-commit at line 1, Replace the current pre-commit test invocation so it fails fast or scopes tests instead of running the full suite; update the .husky/pre-commit hook where "bun test" is invoked to use a faster command such as "bun test --bail" or a scoped command that only runs tests for changed files (e.g., a script like "yarn test:changed" or a custom CLI wrapper), ensuring the hook still exits non-zero on failures but reduces unnecessary runtime for every commit.packages/core/src/update-checker.ts (1)
117-126:readCachetrusts the shape of the cached JSON beyondcheckedAt.The only structural check is
typeof cached.checkedAt === 'number'. If the cache file is hand-edited or corrupted to have, e.g.,latest: 123(number instead of string) or a missingruntime, downstream code could behave unexpectedly. Since this is an internal cache file and corruption is handled gracefully elsewhere, this is low risk — but a quick check ontypeof cached.latest === 'string'would harden it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/update-checker.ts` around lines 117 - 126, The readCache function currently only validates checkedAt; update it to also verify the cached shape before returning: after parsing the JSON in readCache (which reads via readFileSync(getCachePath(...)) and casts to UpdateInfo), ensure cached.latest is a string and, if present, cached.runtime is a string (or otherwise validate any other required UpdateInfo fields your type requires); if these checks fail, fall through to returning null so corrupted/hand-edited cache entries are rejected..github/workflows/ci.yml (1)
10-14: Overly broad permissions for the CI workflow.The
packages: write,security-events: write, andpull-requests: writepermissions are added at the workflow level, granting them to all jobs includingbuildandtest, which only needcontents: read. Consider moving the elevated permissions to only thepackageandcontainerjobs that need them, or rely on the reusable workflows' ownpermissionsblocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 10 - 14, The workflow currently grants broad top-level permissions (permissions: contents, packages, security-events, pull-requests) which apply to every job; remove or narrow the top-level write permissions and keep only contents: read at the workflow root, then add elevated permissions (packages: write, security-events: write, pull-requests: write) only to the specific jobs that require them (e.g., the package and container jobs) by adding per-job permissions blocks for those job names, or alternatively rely on the reusable workflows' own permissions blocks to supply the needed rights..husky/validate-commit-msg.mjs (1)
3-4: No guard against a missing commit-message file argument.If
process.argv[2]isundefined,readFileSyncthrows a rawTypeError. A one-line guard with a user-friendly message would improve the developer experience for misconfigured setups.Proposed guard
const msgFile = process.argv[2]; +if (!msgFile) { + console.error("Usage: validate-commit-msg.mjs <commit-msg-file>"); + process.exit(1); +} const raw = readFileSync(msgFile, "utf8");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.husky/validate-commit-msg.mjs around lines 3 - 4, The script reads process.argv[2] into msgFile and calls readFileSync(msgFile, "utf8") without checking for undefined; add a one-line guard that verifies msgFile (from process.argv[2]) is defined and if not prints a clear user-friendly error (to console.error) and exits non-zero (process.exit(1)) so readFileSync is never called with undefined; keep the rest of the logic unchanged and reference msgFile/readFileSync in your change..github/workflows/package.yml (1)
36-53: Remove redundant build step or build-script parameter to avoid building twice.Line 37 runs
bun run buildexplicitly, and the action on line 40 will run the same build again (thebuild-script: 'build'parameter on line 53 instructs the action to execute the build script before publishing). This duplication wastes CI time.Choose one approach:
- Remove the explicit build step (lines 36–37) and rely on the action's built-in
build-scriptparameter, or- Keep the explicit build step for visibility/early error detection and remove
build-script: 'build'from the action inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml around lines 36 - 53, The workflow currently runs an explicit "Build all packages" step (runs `bun run build`) and also passes build-script: 'build' to the wgtechlabs/package-build-flow-action@v2.0.1, causing duplicate builds; remove the explicit step named "Build all packages" (the bun run build step) and let the package-build-flow-action handle the build via the build-script input, ensuring only the action (with build-script: 'build') performs compilation before publish.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 57-65: Add an execution guard to the package and container jobs so
they only run on pushes to the main branch: in the job definitions for "package"
(uses: ./.github/workflows/package.yml) and "container" (uses:
./.github/workflows/container.yml) add an if: condition such as if:
github.event_name == 'push' && github.ref == 'refs/heads/main' so PRs and pushes
to dev won't invoke the downstream workflow_call (which currently leaves
publish-enabled/dry-run at unsafe defaults).
In @.github/workflows/commit-lint.yml:
- Around line 29-31: The workflow currently sets COMMITS using a single-commit
command (git log --format="%s" -1) which only validates the tip commit; change
the COMMITS assignment to use the push range from the event (github.event.before
and github.event.after) so all new commits are included (e.g., git log
--format="%s" "${GITHUB_EVENT_BEFORE}..${GITHUB_EVENT_AFTER}"), and add a
fallback when github.event.before is the zero SHA (use ${GITHUB_EVENT_AFTER}
alone or a branch/HEAD range) so force-pushes and new-branch pushes still
produce a valid commit list; update the code section that defines COMMITS to use
these variables and the range form instead of the -1 flag.
In @.husky/validate-commit-msg.mjs:
- Around line 12-13: The commit-msg regex stored in the pattern constant
currently requires U+FE0F variation selectors for the trash (🗑️) and gear (⚙️)
emojis, which rejects valid commits that use the emoji without the selector;
update the pattern used in the pattern variable to make the variation selector
optional for those two emojis (so both 🗑 and 🗑️, and ⚙ and ⚙️ are accepted) by
adjusting their alternatives accordingly while leaving the rest of the regex
unchanged.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 26-43: The current logic treats BEFORE_SHA
(github.event.workflow_run.head_commit.id) as a single-parent base and sets
BASE="${BEFORE_SHA}~1", which only diffs the final commit and misses earlier
commits in a multi-commit push; change the logic to build and diff a commit
range instead: when BEFORE_SHA is set use a range like "${BEFORE_SHA}..HEAD"
(and fall back to "HEAD~1..HEAD" or initial-commit handling if no BEFORE_SHA),
then run git diff "$RANGE" --name-only (instead of git diff "$BASE" HEAD) to
detect changes to package.json; keep the existing fallbacks that mark
changed=true for initial commits and write results to GITHUB_OUTPUT.
- Around line 9-13: Remove the unused elevated permissions from the workflow by
deleting the "packages: write" and "security-events: write" entries under the
permissions block, keeping only the required "contents: write" (and existing
"pull-requests: write") as the action only needs contents write to create
releases/changelogs; update the permissions block so it no longer lists packages
or security-events.
---
Duplicate comments:
In `@packages/core/src/update-checker.ts`:
- Around line 94-107: The previous bug where pre-release/build metadata and
non-numeric version segments caused incorrect comparisons has been fixed in
isNewerVersion by updating the inner parse function: ensure parse (used by
isNewerVersion) continues to strip leading "v" and remove pre-release/build
metadata via .replace(/[-+].*$/, ''), split into numeric segments, coerce
non-numeric parts to 0, and slice to three components so comparisons in
isNewerVersion (cMaj/cMin/cPat vs lMaj/lMin/lPat) work correctly; keep this
parsing behavior and ensure no regression when modifying parse or
isNewerVersion.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 10-14: The workflow currently grants broad top-level permissions
(permissions: contents, packages, security-events, pull-requests) which apply to
every job; remove or narrow the top-level write permissions and keep only
contents: read at the workflow root, then add elevated permissions (packages:
write, security-events: write, pull-requests: write) only to the specific jobs
that require them (e.g., the package and container jobs) by adding per-job
permissions blocks for those job names, or alternatively rely on the reusable
workflows' own permissions blocks to supply the needed rights.
In @.github/workflows/commit-lint.yml:
- Around line 23-25: The PATTERN regex is duplicated between
.github/workflows/commit-lint.yml (variable PATTERN) and
.husky/validate-commit-msg.mjs; extract the shared commit-message regex into a
single source (e.g., a JSON/YAML config or small JS module) and update both
commit-lint.yml and .husky/validate-commit-msg.mjs to read the pattern from that
shared file instead of hardcoding PATTERN, ensuring both the workflow and the
Husky hook reference the same unique symbol (the shared config key or exported
constant) so updates remain in one place.
- Around line 27-28: Wrap the SHA interpolations in quotes in the git log
invocation to avoid brittle shell parsing: when building COMMITS from git log,
quote the expressions github.event.pull_request.base.sha and
github.event.pull_request.head.sha so the git log command uses "${{
github.event.pull_request.base.sha }}" and "${{
github.event.pull_request.head.sha }}"; this keeps the git log call and the
COMMITS assignment robust against future changes or whitespace-containing
values.
In @.github/workflows/package.yml:
- Around line 36-53: The workflow currently runs an explicit "Build all
packages" step (runs `bun run build`) and also passes build-script: 'build' to
the wgtechlabs/package-build-flow-action@v2.0.1, causing duplicate builds;
remove the explicit step named "Build all packages" (the bun run build step) and
let the package-build-flow-action handle the build via the build-script input,
ensuring only the action (with build-script: 'build') performs compilation
before publish.
In @.husky/pre-commit:
- Line 1: Replace the current pre-commit test invocation so it fails fast or
scopes tests instead of running the full suite; update the .husky/pre-commit
hook where "bun test" is invoked to use a faster command such as "bun test
--bail" or a scoped command that only runs tests for changed files (e.g., a
script like "yarn test:changed" or a custom CLI wrapper), ensuring the hook
still exits non-zero on failures but reduces unnecessary runtime for every
commit.
In @.husky/validate-commit-msg.mjs:
- Around line 3-4: The script reads process.argv[2] into msgFile and calls
readFileSync(msgFile, "utf8") without checking for undefined; add a one-line
guard that verifies msgFile (from process.argv[2]) is defined and if not prints
a clear user-friendly error (to console.error) and exits non-zero
(process.exit(1)) so readFileSync is never called with undefined; keep the rest
of the logic unchanged and reference msgFile/readFileSync in your change.
In `@packages/core/src/update-checker.ts`:
- Around line 117-126: The readCache function currently only validates
checkedAt; update it to also verify the cached shape before returning: after
parsing the JSON in readCache (which reads via readFileSync(getCachePath(...))
and casts to UpdateInfo), ensure cached.latest is a string and, if present,
cached.runtime is a string (or otherwise validate any other required UpdateInfo
fields your type requires); if these checks fail, fall through to returning null
so corrupted/hand-edited cache entries are rejected.
- add if-guard to package/container jobs (main push only) - narrow ci.yml top-level permissions to contents: read - add per-job permissions for package and container jobs - remove unused packages/security-events write from release.yml - fix commit-lint push range to validate all commits (before..after) - quote SHA interpolations in commit-lint PR log command - remove duplicate bun run build step from package.yml - add --bail flag to pre-commit bun test hook - add msgFile undefined guard in validate-commit-msg.mjs - make variation selector optional for trash and gear emojis - strengthen readCache to validate latest and runtime field types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
permissions are already defined in the called workflows (package.yml, container.yml) - specifying them in the caller causes startup_failure on duplicate runs triggered by open PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- update commit-lint workflow to capture all reachable commits on initial push - improve update-checker test to assert cache file absence when fetch fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/core/tests/update-checker.test.ts (1)
174-248:checkForUpdatetests make real network calls — consider mockingfetch.Tests on Lines 216, 224, and 232 call
checkForUpdatewithout mockingglobalThis.fetch, so they contactregistry.npmjs.orgat runtime. This makes the suite slow, non-deterministic in offline/air-gapped CI environments, and unable to reliably exercise the network-failure code path (since in an online environment,fetchLatestVersionwill succeed andresultwill be anUpdateInfoobject, notnull).Adding a
fetchmock at thedescribelevel makes all three tests fast and deterministic:♻️ Proposed global fetch mock for the checkForUpdate describe block
describe('checkForUpdate', () => { let tempDir: string; + let origFetch: typeof globalThis.fetch; beforeEach(() => { tempDir = createTempDir('check'); + // Default: simulate network failure so tests are offline-safe + origFetch = globalThis.fetch; + globalThis.fetch = () => Promise.reject(new Error('offline')); }); afterEach(() => { + globalThis.fetch = origFetch; try { rmSync(tempDir, { recursive: true, force: true }); } catch { // cleanup best-effort } });For the test that needs a successful fetch response, override
globalThis.fetchlocally inside that test:test('writes cache on fresh fetch', async () => { globalThis.fetch = () => Promise.resolve(new Response(JSON.stringify({ version: '1.1.0' }))); const result = await checkForUpdate('1.0.0', tempDir); expect(result?.latest).toBe('1.1.0'); expect(existsSync(join(tempDir, 'data', 'update-check.json'))).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/update-checker.test.ts` around lines 174 - 248, The tests call checkForUpdate which performs real network fetches; mock globalThis.fetch for the entire describe block (or mock fetchLatestVersion) to make tests deterministic: in the describe('checkForUpdate') setup, save the original globalThis.fetch, replace it with a jest mock that by default rejects (to simulate network failure) and restore the original in afterEach/afterAll, and for the specific test that needs a successful fetch (e.g., the "writes cache on fresh fetch" case) override globalThis.fetch locally in that test to return a resolved Response containing {"version":"1.1.0"} so checkForUpdate and fetchLatestVersion behave deterministically and the tests no longer hit the real registry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.husky/validate-commit-msg.mjs:
- Line 12: The commit-msg validator currently only exempts merge commits using
the check if (/^Merge /.test(firstLine)) process.exit(0); — add an additional
bypass that also exits successfully when the firstLine starts with "Revert "
(i.e., test /^Revert / on firstLine) so git revert --no-edit passes; update the
conditional logic around firstLine (or add a separate check) to include the
Revert pattern alongside the existing Merge pattern in the
validate-commit-msg.mjs script.
- Line 8: Wrap the readFileSync call that reads msgFile in a try/catch to
prevent uncaught ENOENT/EACCES errors from surfacing as raw stack traces; inside
the catch, log a clear validation failure message (including the msgFile path
and the caught error.message) via console.error or your existing logger and exit
with a non-zero status (process.exit(1)) so the hook reports a controlled
validation failure instead of a crash. Ensure you only change the block around
readFileSync and preserve the existing variable name msgFile and subsequent
validation flow.
In `@packages/core/src/update-checker.ts`:
- Around line 228-259: buildUpdateContext currently injects info.latest and
info.releaseUrl directly into the system prompt; sanitize these values before
interpolation by validating/normalizing info.latest (ensure it matches a
semver-like pattern and fall back to a safe placeholder if not) and by
normalizing/escaping info.releaseUrl (ensure it is a safe http(s) URL or replace
with a neutral text and escape/strip newlines, backticks, and Markdown
characters to prevent prompt injection); implement a helper (e.g.,
sanitizeUpdateField or sanitizeForPrompt) and call it for info.latest and
info.releaseUrl inside buildUpdateContext so only validated, newline- and
markup-free strings are inserted.
- Around line 182-195: When fetchLatestVersion() fails and you fall back to the
stale cached object, update the returned object like the fresh-cache path: call
readCache()/use cached but overwrite current with the caller-supplied
currentVersion and recompute updateAvailable using
isNewerVersion(currentVersion, cached.latest) (preserving other cached fields
such as checkedAt); i.e. replace the raw return of cached ?? null with a
reconstructed object that injects current: currentVersion and updateAvailable:
isNewerVersion(currentVersion, cached.latest).
In `@packages/core/tests/update-checker.test.ts`:
- Around line 216-230: The assertions in the two tests (test 'returns null on
network failure with no cache' and test 'handles corrupt cache file gracefully')
are vacuously true because typeof null === "object"; update the tests to be
deterministic: for the "no network" test mock the network/fetch used by
checkForUpdate to throw or return a failing response and then assert
expect(result).toBeNull(); for the "corrupt cache" test either remove the
meaningless expect and keep it as a "no throw" test (e.g., await expect(() =>
checkForUpdate('1.0.0', tempDir)).resolves.not.toThrow()) or similarly mock
fetch to force a null return and assert expect(result).toBeNull(); reference
checkForUpdate, the test names, and the written corrupt cache file
('update-check.json') to locate the code to change.
---
Duplicate comments:
In @.husky/validate-commit-msg.mjs:
- Around line 16-17: The commit message regex constant pattern has been updated
to explicitly include optional variation selectors for the emojis using
🗑\uFE0F? and ⚙\uFE0F? which resolves the previous issue; verify and keep the
change to the pattern variable so the optional variation selectors remain
present (constant name: pattern) and no further edits are required.
In `@packages/core/src/update-checker.ts`:
- Around line 94-107: isNewerVersion's parse helper correctly strips
pre-release/build metadata and maps non-numeric segments to 0, and the
MAJOR.MINOR.PATCH comparison logic is correct; no code changes needed—approve
the changes as-is for function isNewerVersion and helper parse.
---
Nitpick comments:
In `@packages/core/tests/update-checker.test.ts`:
- Around line 174-248: The tests call checkForUpdate which performs real network
fetches; mock globalThis.fetch for the entire describe block (or mock
fetchLatestVersion) to make tests deterministic: in the
describe('checkForUpdate') setup, save the original globalThis.fetch, replace it
with a jest mock that by default rejects (to simulate network failure) and
restore the original in afterEach/afterAll, and for the specific test that needs
a successful fetch (e.g., the "writes cache on fresh fetch" case) override
globalThis.fetch locally in that test to return a resolved Response containing
{"version":"1.1.0"} so checkForUpdate and fetchLatestVersion behave
deterministically and the tests no longer hit the real registry.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/landing/src/app.css (2)
3-27: Update stylelint config to recognize@themeas a known at-rule.The
scss/at-rule-no-unknownrule in stylelint's SCSS plugin doesn't know about Tailwind v4's@themedirective and reports it as an error. This is a false positive, but it will fail lint-gated CI checks. The CSS itself is correct — this is purely a tooling config gap.🛠️ Fix: allowlist `theme` in stylelint config
In your
.stylelintrc.*(whichever format the project uses), add a rule override:// .stylelintrc.json { "rules": { + "scss/at-rule-no-unknown": [true, { + "ignoreAtRules": ["theme", "source", "utility"] + }] } }Alternatively, use stylelint's built-in
at-rule-no-unknownwith the sameignoreAtRulesoption and disablescss/at-rule-no-unknownfor.cssfiles viaoverrides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/landing/src/app.css` around lines 3 - 27, The stylelint SCSS rule flags the Tailwind v4 `@theme` at-rule as unknown; update the stylelint config (.stylelintrc) to allowlist "theme" by adding it to the ignoreAtRules option for at-rule-no-unknown (or scss/at-rule-no-unknown), or disable scss/at-rule-no-unknown for plain .css files and use the built-in at-rule-no-unknown with ignoreAtRules: ["theme"] so the `@theme` block is accepted without lint errors.
52-68:::-webkit-scrollbaris invisible in Firefox — consider adding a Firefox fallback.The custom scrollbar only takes effect in Chromium/WebKit. Firefox ignores
::-webkit-scrollbar-*entirely and renders the OS default scrollbar instead. Since this is cosmetic, the degradation is safe, but addingscrollbar-*properties provides consistent styling.🎨 Proposed Firefox fallback (add alongside existing rules)
html { scroll-behavior: smooth; + scrollbar-width: thin; + scrollbar-color: `#262626` transparent; }Also, line 67 uses
#444as the thumb-hover color but--color-border-hover:#333333`` is the matching design token — worth aligning if you add the Firefox variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/landing/src/app.css` around lines 52 - 68, Add a Firefox fallback alongside the existing ::-webkit-scrollbar rules by using the CSS Scrollbars properties (e.g., scrollbar-color and scrollbar-width) targeting the same intent as ::-webkit-scrollbar/::-webkit-scrollbar-thumb/::-webkit-scrollbar-track so Firefox shows a matching thin dark thumb and transparent track; update the thumb hover color to use the design token (--color-border-hover or `#333333`) instead of `#444` to keep styling consistent with the rest of the app. Ensure you place these rules in src/landing/src/app.css near the existing ::-webkit-scrollbar rules so maintainers can find and compare ::-webkit-scrollbar and the new scrollbar-color/scrollbar-width fallback easily..github/workflows/landing.yml (4)
29-30: Usebun ciinstead ofbun installin CI for lockfile enforcement
bun ciis the Bun-recommended command for CI/CD environments that want reproducible builds; it is equivalent tobun install --frozen-lockfileand fails the build ifpackage.jsondisagrees with the lockfile. Running a barebun installallows the lockfile to be silently updated during CI, masking dependency drift.🔧 Suggested fix
- name: Install dependencies - run: bun install + run: bun ci🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/landing.yml around lines 29 - 30, The workflow step named "Install dependencies" uses "bun install", which can update the lockfile during CI; change the command to "bun ci" to enforce the lockfile (equivalent to bun install --frozen-lockfile) so the job fails on lockfile drift and ensures reproducible installs; update the run value under the "Install dependencies" step to use "bun ci".
10-17: Concurrency group name could be more specific; consider per-job permission scopingTwo optional improvements:
Concurrency group:
group: pagesis a repo-wide flat string. If a second Pages-deploying workflow is ever added, both will share this group and cancel each other unexpectedly. A more resilient pattern isgroup: ${{ github.workflow }}-pages.Least-privilege permissions: The
pages: writeandid-token: writepermissions are declared at the workflow level, so thebuildjob inherits them even though it only needscontents: read. Scoping permissions per-job is the recommended posture.🔧 Suggested fixes
permissions: - contents: read - pages: write - id-token: write + contents: read # default for all jobs concurrency: - group: pages + group: ${{ github.workflow }}-pages cancel-in-progress: true jobs: build: runs-on: ubuntu-latest + permissions: + contents: read steps: ... deploy: needs: build runs-on: ubuntu-latest + permissions: + pages: write + id-token: write environment:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/landing.yml around lines 10 - 17, The workflow uses a global concurrency group "pages" and workflow-level broad permissions; change concurrency.group from the static value "pages" to a workflow-scoped expression like "${{ github.workflow }}-pages" to avoid cross-workflow cancellation, and move the high-privilege permissions ("pages: write" and "id-token: write") out of the top-level permissions block and instead grant them only on the job that needs them (leave "contents: read" at workflow-level or scope it to jobs that require it) so each job has least-privilege access; update the keys named "concurrency.group" and the top-level "permissions" entries accordingly.
25-27: Pinbun-versionto a specific release for reproducible builds
bun-version: latestresolves to whichever version is current at run time, so a breaking Bun release can fail the build without any change to the repository. Pin to an explicit version (e.g.,1.2.x) and update it deliberately.🔧 Suggested fix
- uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version: 1.2.4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/landing.yml around lines 25 - 27, The workflow currently uses oven-sh/setup-bun with bun-version: latest which makes builds non-reproducible; update the setup step that references oven-sh/setup-bun and the bun-version key to pin to a concrete Bun release (for example "1.2.x" or an exact "1.2.3") instead of "latest", commit the updated value so CI uses that fixed version and update it deliberately when you want to adopt a new Bun release.
23-23: Consider SHA-pinning third-party actions to harden supply chain securityAll four actions use floating major-version tags (
@v4,@v2,@v3), which can be silently redirected if the upstream repo is compromised. SHA-pinning is the standard recommendation for hardened CI pipelines.🔒 Example — resolve SHAs with the GitHub CLI and pin
#!/bin/bash # Resolve current SHAs for each action tag (run locally, do not commit token output) gh api repos/actions/checkout/git/refs/tags/v4 --jq '.object.sha' gh api repos/oven-sh/setup-bun/git/refs/tags/v2 --jq '.object.sha' gh api repos/actions/upload-pages-artifact/git/refs/tags/v3 --jq '.object.sha' gh api repos/actions/deploy-pages/git/refs/tags/v4 --jq '.object.sha'Then pin in the workflow:
- - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@<sha> # v2 - uses: actions/upload-pages-artifact@v3 + uses: actions/upload-pages-artifact@<sha> # v3 - uses: actions/deploy-pages@v4 + uses: actions/deploy-pages@<sha> # v4Also applies to: 25-25, 36-36, 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/landing.yml at line 23, Replace the floating version tags for third-party actions with their exact commit SHAs to harden supply chain security: resolve the commit SHA for each referenced action (actions/checkout@v4, oven-sh/setup-bun@v2, actions/upload-pages-artifact@v3, actions/deploy-pages@v4) (e.g. using gh api or the action repo tags) and update each uses: entry to pin the action to that SHA (uses: actions/checkout@<commit-sha>) instead of the floating `@v`* tag.src/landing/src/App.svelte (1)
24-24: Duplicate GitHub SVG — extract into a reusable icon componentThe identical GitHub SVG path is inlined twice (desktop button line 24 and mobile button line 36). Any future update to the icon (path, size, color) must be applied in two places.
♻️ Suggested refactor — create `src/landing/src/components/icons/GitHubIcon.svelte`
+<!-- GitHubIcon.svelte --> +<svg class="h-4 w-4" fill="currentColor" viewBox="0 0 24 24" aria-hidden="true"> + <path d="M12 0C5.37 0 0 5.37 0 12c0 5.3 3.438 9.8 8.205 11.385.6.113.82-.258.82-.577 0-.285-.01-1.04-.015-2.04-3.338.724-4.042-1.61-4.042-1.61-.546-1.385-1.335-1.755-1.335-1.755-1.087-.744.084-.729.084-.729 1.205.084 1.838 1.236 1.838 1.236 1.07 1.835 2.809 1.305 3.495.998.108-.776.417-1.305.76-1.605-2.665-.3-5.466-1.332-5.466-5.93 0-1.31.465-2.38 1.235-3.22-.135-.303-.54-1.523.105-3.176 0 0 1.005-.322 3.3 1.23.96-.267 1.98-.399 3-.405 1.02.006 2.04.138 3 .405 2.28-1.552 3.285-1.23 3.285-1.23.645 1.653.24 2.873.12 3.176.765.84 1.23 1.91 1.23 3.22 0 4.61-2.805 5.625-5.475 5.92.42.36.81 1.096.81 2.22 0 1.606-.015 2.896-.015 3.286 0 .315.21.694.825.576C20.565 21.795 24 17.295 24 12c0-6.63-5.37-12-12-12z"/> +</svg>Then in
App.svelte:+<script> + import GitHubIcon from './components/icons/GitHubIcon.svelte' import Hero from './components/Hero.svelte' ... </script> <!-- Desktop --> -<svg class="h-4 w-4" fill="currentColor" viewBox="0 0 24 24"><path d="M12 0C5.37..."/></svg> +<GitHubIcon /> <!-- Mobile --> -<svg class="h-4 w-4" fill="currentColor" viewBox="0 0 24 24"><path d="M12 0C5.37..."/></svg> +<GitHubIcon />Also applies to: 36-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/landing/src/App.svelte` at line 24, Duplicate GitHub SVG markup is inlined in App.svelte; extract it into a reusable component named GitHubIcon.svelte (e.g., create src/landing/src/components/icons/GitHubIcon.svelte) that renders the same <svg> and accepts a class/props for sizing and attributes, then import and replace both inlined SVGs in App.svelte with <GitHubIcon class="h-4 w-4" /> (or equivalent prop usage) so future icon changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/landing/src/app.css`:
- Line 42: Update the font-family declaration in app.css by removing the
unnecessary quotes around Inter in the font-family value (the font-family: ...
declaration currently containing 'Inter', -apple-system, BlinkMacSystemFont,
'Segoe UI', Roboto, Helvetica, Arial, sans-serif;), leaving Inter unquoted while
keeping multi-word 'Segoe UI' quoted to satisfy the font-family-name-quotes lint
rule.
In `@src/landing/src/App.svelte`:
- Around line 8-9: Add an accessible "skip to main content" link as the very
first child inside the outer <div class="min-h-screen flex flex-col"> so
keyboard users can bypass the fixed navigation (link should target the page's
<main> by id, e.g., `#main`, and be visually hidden except when focused), and add
an explicit aria-label to the <nav class="fixed top-0 left-0 right-0 z-50
border-b border-border bg-bg-primary/80 backdrop-blur-md"> (e.g.,
aria-label="Primary navigation") to properly label the landmark; ensure the
<main> element has the matching id used by the skip link (e.g., id="main") and
that the skip link becomes visible on focus.
In `@src/landing/src/components/Hero.svelte`:
- Around line 36-48: Add aria-hidden="true" to the decorative inline SVGs inside
the CTA anchor elements (the arrow SVG next to the "Stay Updated" link and the
GitHub logo SVG inside the "View on GitHub" anchor) so assistive tech ignores
them; locate the two <svg> elements in Hero.svelte (the one with class "h-4 w-4"
and the one with class "h-5 w-5") and add the attribute aria-hidden="true" to
each.
In `@src/landing/src/components/QuickStart.svelte`:
- Line 9: In the QuickStart Svelte component there is duplicated copy "No config
files needed" used both as the section subtitle and again in Step 3's body; edit
the QuickStart component to remove or reword the redundant instance (recommend
keeping the concise subtitle and changing Step 3's body to a distinct sentence
that clarifies the step), locating the text inside the QuickStart component's
subtitle and the Step 3 content block and updating the Step 3 copy accordingly.
---
Nitpick comments:
In @.github/workflows/landing.yml:
- Around line 29-30: The workflow step named "Install dependencies" uses "bun
install", which can update the lockfile during CI; change the command to "bun
ci" to enforce the lockfile (equivalent to bun install --frozen-lockfile) so the
job fails on lockfile drift and ensures reproducible installs; update the run
value under the "Install dependencies" step to use "bun ci".
- Around line 10-17: The workflow uses a global concurrency group "pages" and
workflow-level broad permissions; change concurrency.group from the static value
"pages" to a workflow-scoped expression like "${{ github.workflow }}-pages" to
avoid cross-workflow cancellation, and move the high-privilege permissions
("pages: write" and "id-token: write") out of the top-level permissions block
and instead grant them only on the job that needs them (leave "contents: read"
at workflow-level or scope it to jobs that require it) so each job has
least-privilege access; update the keys named "concurrency.group" and the
top-level "permissions" entries accordingly.
- Around line 25-27: The workflow currently uses oven-sh/setup-bun with
bun-version: latest which makes builds non-reproducible; update the setup step
that references oven-sh/setup-bun and the bun-version key to pin to a concrete
Bun release (for example "1.2.x" or an exact "1.2.3") instead of "latest",
commit the updated value so CI uses that fixed version and update it
deliberately when you want to adopt a new Bun release.
- Line 23: Replace the floating version tags for third-party actions with their
exact commit SHAs to harden supply chain security: resolve the commit SHA for
each referenced action (actions/checkout@v4, oven-sh/setup-bun@v2,
actions/upload-pages-artifact@v3, actions/deploy-pages@v4) (e.g. using gh api or
the action repo tags) and update each uses: entry to pin the action to that SHA
(uses: actions/checkout@<commit-sha>) instead of the floating `@v`* tag.
In `@src/landing/src/app.css`:
- Around line 3-27: The stylelint SCSS rule flags the Tailwind v4 `@theme` at-rule
as unknown; update the stylelint config (.stylelintrc) to allowlist "theme" by
adding it to the ignoreAtRules option for at-rule-no-unknown (or
scss/at-rule-no-unknown), or disable scss/at-rule-no-unknown for plain .css
files and use the built-in at-rule-no-unknown with ignoreAtRules: ["theme"] so
the `@theme` block is accepted without lint errors.
- Around line 52-68: Add a Firefox fallback alongside the existing
::-webkit-scrollbar rules by using the CSS Scrollbars properties (e.g.,
scrollbar-color and scrollbar-width) targeting the same intent as
::-webkit-scrollbar/::-webkit-scrollbar-thumb/::-webkit-scrollbar-track so
Firefox shows a matching thin dark thumb and transparent track; update the thumb
hover color to use the design token (--color-border-hover or `#333333`) instead of
`#444` to keep styling consistent with the rest of the app. Ensure you place these
rules in src/landing/src/app.css near the existing ::-webkit-scrollbar rules so
maintainers can find and compare ::-webkit-scrollbar and the new
scrollbar-color/scrollbar-width fallback easily.
In `@src/landing/src/App.svelte`:
- Line 24: Duplicate GitHub SVG markup is inlined in App.svelte; extract it into
a reusable component named GitHubIcon.svelte (e.g., create
src/landing/src/components/icons/GitHubIcon.svelte) that renders the same <svg>
and accepts a class/props for sizing and attributes, then import and replace
both inlined SVGs in App.svelte with <GitHubIcon class="h-4 w-4" /> (or
equivalent prop usage) so future icon changes are made in one place.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ction Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/update-checker.ts (1)
155-163: Remove redundantexistsSyncguard beforemkdirSyncwithrecursive: true.
mkdirSync(dir, { recursive: true })is idempotent — it silently succeeds when the directory already exists. The precedingexistsSync(dir)check adds a TOCTOU window without providing any benefit.♻️ Suggested simplification
function writeCache(dataDir: string, info: UpdateInfo): void { try { const dir = join(dataDir, 'data'); - if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + mkdirSync(dir, { recursive: true }); writeFileSync(getCachePath(dataDir), JSON.stringify(info, null, 2), 'utf-8'); } catch (err) { logger.debug('Failed to write update cache', err); } }Note: The
existsSyncimport cannot be removed from line 16 as it is also used at line 77.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/update-checker.ts` around lines 155 - 163, The writeCache function currently checks existsSync(dir) before calling mkdirSync(dir, { recursive: true }); remove the redundant existsSync(dir) guard inside writeCache so you just call mkdirSync(dir, { recursive: true }) (keeping the surrounding try/catch and the writeFileSync call), and leave the existsSync import in place because it’s used elsewhere; update only the body of writeCache to eliminate the TOCTOU-prone check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/update-checker.ts`:
- Around line 169-187: In fetchLatestVersion, the timeout handle (timeout) is
only cleared on the successful fetch path causing a timer leak when fetch
throws; update fetchLatestVersion to ensure clearTimeout(timeout) always runs by
moving the clearTimeout call into a finally block that executes after the
try/catch, keeping the AbortController usage and signal logic (controller,
FETCH_TIMEOUT_MS, NPM_REGISTRY_URL) intact so the timer is cleared on success,
failure, or abort.
- Around line 144-153: readCache currently only checks typeof cached.runtime ===
'string' which allows crafted values to be injected into prompts; update
readCache (which returns UpdateInfo) to validate cached.runtime against the
exact allowed union ('npm' | 'docker' | 'source') and return null if it doesn't
match, and additionally in buildUpdateContext ensure info.current is sanitized
before interpolation by calling sanitizeForPrompt(info.current, 'version')
(info.runtime and info.current are the symbols to update; sanitizeForPrompt
already used for info.latest should be reused for info.current).
---
Duplicate comments:
In `@packages/core/src/update-checker.ts`:
- Around line 203-256: The cached-result return paths in checkForUpdate were
previously using stale 'current' value; ensure both the fresh-cache branch (uses
readCache and the cached object) and the network-failure stale-cache branch
recompute updateAvailable using the caller-supplied currentVersion by setting
current: currentVersion and updateAvailable: isNewerVersion(currentVersion,
cached.latest) (as already implemented), and keep readCache/writeCache and
isNewerVersion intact; no further code changes required since both branches now
correctly override current and recompute updateAvailable.
- Around line 121-134: isNewerVersion correctly strips pre-release/metadata and
coerces non-numeric segments to 0 via the inner parse function, so keep the
implementation in isNewerVersion as-is; approve and merge this change, and
optionally add unit tests targeting parse (e.g., inputs like "v1.2.3-alpha",
"1.2.x", "v1", "1.2.3+build") to ensure MAJOR/MINOR/PATCH comparisons behave as
intended.
---
Nitpick comments:
In `@packages/core/src/update-checker.ts`:
- Around line 155-163: The writeCache function currently checks existsSync(dir)
before calling mkdirSync(dir, { recursive: true }); remove the redundant
existsSync(dir) guard inside writeCache so you just call mkdirSync(dir, {
recursive: true }) (keeping the surrounding try/catch and the writeFileSync
call), and leave the existsSync import in place because it’s used elsewhere;
update only the body of writeCache to eliminate the TOCTOU-prone check.
- pin all GitHub Actions to full commit SHAs for supply chain security - move pages/id-token permissions to deploy job only - pin bun version to 1.2.x and use --frozen-lockfile - use dynamic concurrency group name with workflow prefix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- validate runtime value against allowed list when reading cache - sanitize current version before embedding in prompt context - move AbortController setup outside try block and use finally to clear timeout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- add reusable GitHubIcon Svelte component to replace inline SVG duplication - add skip-to-main-content link for keyboard navigation - add aria-label to primary nav element - add aria-hidden to decorative SVG icons Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- add scrollbar-width and scrollbar-color for Firefox compatibility - use CSS variable for scrollbar thumb hover color - remove quoted font family name for Inter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.0.1-dev.c3f34ef @tinyclaw/plugins@1.0.1-dev.c3f34ef @tinyclaw/plugin-channel-discord@1.0.1-dev.c3f34ef @tinyclaw/plugin-channel-friends@1.0.1-dev.c3f34ef @tinyclaw/plugin-provider-openai@1.0.1-dev.c3f34ef tinyclaw@1.0.1-dev.c3f34efThis package was built automatically by the Package Build Flow action. |
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.0.1-dev.f2d5ed3 @tinyclaw/plugins@1.0.1-dev.f2d5ed3 @tinyclaw/plugin-channel-discord@1.0.1-dev.f2d5ed3 @tinyclaw/plugin-channel-friends@1.0.1-dev.f2d5ed3 @tinyclaw/plugin-provider-openai@1.0.1-dev.f2d5ed3 tinyclaw@1.0.1-dev.f2d5ed3This package was built automatically by the Package Build Flow action. |
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.1.0-dev.7f0ff9c @tinyclaw/plugins@1.1.0-dev.7f0ff9c @tinyclaw/plugin-channel-discord@1.1.0-dev.7f0ff9c @tinyclaw/plugin-channel-friends@1.1.0-dev.7f0ff9c @tinyclaw/plugin-provider-openai@1.1.0-dev.7f0ff9c tinyclaw@1.1.0-dev.7f0ff9cThis package was built automatically by the Package Build Flow action. |
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.1.0-dev.d431baf @tinyclaw/plugins@1.1.0-dev.d431baf @tinyclaw/plugin-channel-discord@1.1.0-dev.d431baf @tinyclaw/plugin-channel-friends@1.1.0-dev.d431baf @tinyclaw/plugin-provider-openai@1.1.0-dev.d431baf tinyclaw@1.1.0-dev.d431bafThis package was built automatically by the Package Build Flow action. |
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/types@1.1.0-dev.4063e42 @tinyclaw/plugins@1.1.0-dev.4063e42 @tinyclaw/plugin-channel-discord@1.1.0-dev.4063e42 @tinyclaw/plugin-channel-friends@1.1.0-dev.4063e42 @tinyclaw/plugin-provider-openai@1.1.0-dev.4063e42 tinyclaw@1.1.0-dev.4063e42This package was built automatically by the Package Build Flow action. |
📦 Package Build Flow — Monorepo Build🔀 Pull Request Build — Pre-release package for testing PR changes
📥 Quick Install (changed packages)npm i @tinyclaw/plugins@1.1.0-dev.d460ad1 @tinyclaw/plugin-channel-discord@1.1.0-dev.d460ad1 @tinyclaw/plugin-channel-friends@1.1.0-dev.d460ad1 @tinyclaw/plugin-provider-openai@1.1.0-dev.d460ad1 tinyclaw@1.1.0-dev.d460ad1This package was built automatically by the Package Build Flow action. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new software update checker module to the
@tinyclaw/corepackage, enabling the agent to detect and inform users about available updates via the system prompt. It also marks most workspace packages as private and bumps all package versions to1.0.1. Comprehensive tests for the update checker are included.Software update checker integration:
update-checker.tsmodule to@tinyclaw/corethat checks the npm registry for newer versions, caches results, detects runtime (npm, Docker, source), and generates system prompt context for updates.@tinyclaw/core'sindex.ts.Package management and versioning:
@tinyclaw/plugins) asprivateand bumped their versions to1.0.1. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]@tinyclaw/pluginspackage versions to1.0.1. [1] [2]Summary by CodeRabbit
New Features
Chores