fix(ci): regenerate pnpm lockfile and repair the CI failures it uncovered - #2
Conversation
The committed pnpm-lock.yaml carried an "overrides" block that no package.json in the tree declares, so every workflow running "pnpm install --frozen-lockfile" failed with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Regenerated the lockfile against the current manifests with pnpm 9.15.0. Also removes the root package-lock.json. The root is a pnpm workspace root and no workflow referenced this file; the npm-based jobs all point at frontend/package-lock.json, which is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
CI status — the lockfile fix worked; the remaining failures are pre-existingThis PR did what it set out to do. On the base commit ( What that exposed is that main was already red underneath, on eight separate pre-existing problems that install failure had been masking. None of them are caused by this diff, which touches only
I'm not fixing these here. Each is a real bug outside the scope of a lockfile repair, and several are decisions rather than mechanical fixes — resolving ~30 files' worth of conflict markers, renumbering a shipped migration, deciding whether those gitleaks hits are false positives, and enabling a repo-level security setting. Bundling them into this PR would bury the one-line-of-intent change that actually fixes the install. The likely root cause of several of theseThe stale The current root manifest declares exactly one: If that truncation was accidental, restoring those devDependencies plus the missing Generated by Claude Code |
Regenerating the lockfile got `pnpm install --frozen-lockfile` past the gate it had been failing at, which let every later CI step run for the first time in a while and surfaced eight independent breakages. None were caused by the lockfile change; all of them predate it. Root manifest had lost its devDependencies and scripts. The stale lockfile still recorded ten root devDependencies that package.json no longer declared, which is why `.github/scripts/coverage-report.mjs` could not resolve istanbul-lib-coverage and why the root eslint.config.js imports were unresolvable. Restored those, plus the `test`, `test:integration`, `build` and `changeset` scripts that ci.yml, integration-tests.yml, deploy-vps.yml and release.yml invoke. Duplicate goose migration version. 0020_pr_reviews.sql and 0020_review_run_unique_pr_sha.sql both claimed version 20, so goose panicked on every migrate and took three Go packages down with it. 0020_review_run_unique_pr_sha.sql landed first (Untrivial-ai#2193, ancestor of Untrivial-ai#2200) and review_store.go documents its index as "migration 0020", so the later collider becomes 0021. CRLF line endings broke two toolchains. ao-doctor.sh and ao-update.sh were unrunnable on the Unix runners, and Vite could not parse packages/ao/bin/postinstall.js because its hashbang line ended in \r. Converted the seven shell scripts and that module to LF, and added a .gitattributes pinning the formats where CRLF is actively harmful. Committed merge-conflict markers. Forty files under frontend/src/landing and skills/bug-triage carried unresolved `<<<<<<< Updated upstream` markers from a stash pop, which made Prettier exit 2. Kept the upstream side throughout: 38 of the conflicts were pure formatting, ci-recovery differed only in emphasis markers, and for SKILL.md the upstream side is the one matching this repo (aoagents org, port 3001, hookPATH in session_manager/manager.go). Gitleaks false positives. Allowlisted the two agent session-ID fixtures in the Go adapter tests and the PostHog *project* key, which is bundled into the renderer and public by design. Listed by value rather than by path so any other secret in those files still trips the scan. Dependency Review is non-blocking. The action hard-fails unless the Dependency graph is enabled in repository settings, which is off by default on forks, so it was failing on a repo setting rather than on the diff. Enabling that setting re-arms it with no further change here. Also gitignores packages/web/.next, which the documented web build creates and nothing was excluding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
The .gitattributes added in the previous commit pins *.js/*.mjs to LF, so these twelve files were left permanently dirty in every working tree: CRLF in the index, LF on checkout. Renormalizing them settles that. Each is byte-identical once carriage returns are stripped. Also excludes pnpm-lock.yaml from Prettier, alongside the two npm lockfiles already listed. The format job commits its own output back, so leaving a pnpm-generated file in scope means every push rewrites ~2900 lines of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
All eight failures fixedPushed
The root manifest was the common causeFour of the eight traced back to one thing: Two findings worth flaggingThe duplicate migration was a genuine data-integrity bug, not just a broken test. CRLF line endings were breaking two toolchains. On the two judgement callsFor
Verified locallyEvery job reproduced against the pushed commit, on Node 22 / pnpm 9.15.0 / Go 1.24: Two caveats on that run, both environmental rather than code: One thing I deliberately did not do
The Generated by Claude Code |
Test Coverage Report
Per-file breakdown
Uncovered lines
|
Three path-filtered workflows ran for the first time on this branch because it touches backend/, tests/integration/ and packages/. api-drift ran `npm ci` at the repo root, which needed the stray package-lock.json this PR removed. The root is a pnpm workspace and this job only needs the root openapi-typescript devDependency, so it now uses pnpm like every other job; keeping a second npm-managed lockfile in sync by hand is what let it drift in the first place. The root `api` script switches to `pnpm run` for the same reason. Verified end to end: the job regenerates schema.ts with no diff. The format job reformatted .github/workflows/*.yml and then had its auto-commit push rejected — GitHub refuses a GITHUB_TOKEN-authenticated push that touches workflow files, so this was fatal to the job the moment Prettier reached one. There is no permission that grants it, so workflows are now excluded from Prettier. Test Fresh Onboarding failed because docker-compose runs tests/integration/onboarding-test.sh as its `command:` and the file was mode 644. The node:20 image's entrypoint prepends `node` to anything it cannot execute, so the shell script was parsed as JavaScript. Set the executable bit on the five scripts that are invoked by path; the two bundled CLI assets stay 644 because runRepoScript spawns them via `bash` explicitly. Test (windows-latest) failed on six FTS tests because better-sqlite3 has no compiled binding there. That file already intends to skip when better-sqlite3 is unavailable, but its guard only caught a require() failure — the JS wrapper loads fine and only opening a database surfaces the missing binding. Probe by opening one, so the platform skips as documented. Linux still runs all six for real. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
Round two — four more fixed, one is not minePushed
|
| Run | Commit | Branch | Result |
|---|---|---|---|
| 1 | 0d9fc4e |
main | ❌ failure |
| 2 | 28a49c3 |
main | ❌ failure |
| 3 | 073c332 |
this PR | ❌ failure |
Every run this workflow has ever had, on main and here alike, has failed. It has never been green in this fork. The same suite passes on Linux with my changes applied (go test -tags e2e ./internal/cli/... → ok, 22.8s), so it's Windows-specific — a real bug, but one that predates this PR by two commits on main and needs a Windows box to diagnose. Debugging it from here would be guesswork, and it's a long way from "regenerate the lockfile."
I'd suggest it gets its own issue.
Generated by Claude Code
The action diffs against main so it reports only what a PR introduces. The shallow default checkout leaves it without a merge base, so it falls back to listing every finding in the changed files — including pre-existing ones — and warns about it on each run. Matches the fetch-depth: 0 already used by the coverage and gitleaks jobs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…rule The gitleaks `scan` job went red on six findings, every one of them in the secret-redaction code, its tests, or the audit note describing it: activity-events.ts names the token prefixes it strips, and the test asserts the stripping works using AWS's own published example key (AKIAIOSFODNN7EXAMPLE). These were already known false positives — the config allowlists them, but by commit SHA, with a note claiming the files were "already deleted from the tree". They are not deleted, so that list only held while nothing touched them again. The prettier auto-format job touched them, producing a new commit the list does not cover. Allowlisting by path instead makes it durable; the commit list stays for the history scan. Also drops the trailing `?` from the Slack rule. The token body was optional, so a bare `xoxb-` matched anywhere — including prose and comments that merely name the prefix. Requiring the body still matches every real token (verified against the fixtures) and stops matching mentions of the prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…indows hashing Onboarding test: dropped steps 6-10. They ran `ao start --no-orchestrator` and polled a dashboard on :9000 plus a terminal WebSocket on :14801. That flag no longer exists — `ao start` resolves the installed desktop app, opens it, and exits, because the app now owns the daemon and state. There is no headless dashboard to poll and no desktop app in the container, so those steps asserted against an architecture the CLI has shed. Steps 1-5 still cover what the test is named for: a clean machine can clone, run setup.sh, get a working `ao`, and produce a config the CLI accepts. Gitleaks: the previous allowlist entries went under `files`, which is matched against the bare filename, so full-path patterns silently never matched and the job stayed red. Moved them to `paths` and added the two literal fixture values under `regexes` so the redaction tests stay covered even if they move file. Windows: the kimicode test helper hashed the raw workspace path while resolveWorkspacePath hashes the realpath'd one. Those agree on Linux only because fakeHome is already resolved; Windows canonicalises further, so every fixture wrote to a different bucket than discovery scanned and 17 assertions saw undefined. The helper now mirrors production — stat, then realpath, falling back to the raw string. No production code changed, and all 103 kimicode tests still pass on Linux. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…ormat commits The Windows log gave the kimicode failures away: the fixture bucket was under "C:\Users\RUNNER~1\...", an 8.3 short name straight from tmpdir(). The plugin hashes realpath(workspacePath), and realpath expands that to "C:\Users\runneradmin\...", so discovery looked in a different md5 bucket than the one the test populated and every lookup returned undefined. The setup already canonicalised fakeHome for the same reason on macOS (/var vs /private/var), but plain realpathSync resolves symlinks without touching short names — it fixes macOS and silently misses Windows. realpathSync.native does both. Two more failures were ENOTEMPTY from rmSync in afterEach: Windows holds a brief handle on files the plugin just read, so whichever test ran last before the handle dropped failed on cleanup rather than on an assertion. Retries handle it. Gitleaks: added the two prettier auto-format commits to the allowlist. That job reflowed the entire tree, so the redaction fixtures and security-doc examples reappear as additions under new SHAs — same offenders and same rationale as the rewrite-graft commits already listed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
The previous round took the scan from six findings to one, so the allowlist is being applied — the log confirms gitleaks v7.4.0 running with --config-path=.github/.gitleaks.toml. The survivor is unrelated to the earlier batch: SECURITY.md documents keeping credentials in env vars and shows "GITHUB_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx". `x` is alphanumeric, so the Github-token rule matches 36 characters of filler. It has to be matched by pattern rather than added to the commit list: the finding is attributed to the PR's merge commit, which gets a new SHA on every run. A 20-character run of literal x cannot occur in a real credential, so this suppresses placeholders without masking a leak — verified against a realistic token, and the only files in the tree it affects are SECURITY.md and this config's own comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…aking it realpathSync.native fixed the bucket mismatch — the Windows log now shows the resolved C:\Users\runneradmin path and the assertions pass. What was left was 15 failures that were all ENOTEMPTY from afterEach, never an assertion, with the retry budget stretching the run from 6s to 50s without ever succeeding. That pointed at a handle nothing was releasing. extractKimiSummary built a readline interface over a ReadStream and called rl.close() when done. That ends the interface but leaves the descriptor open, and every exit from the loop is a `break` — on the byte cap, or on the first TurnBegin, which is the common path — so the stream was abandoned mid-read. POSIX lets you unlink a file that still has an open handle, which is why this never showed up on Linux or macOS; Windows keeps the session directory alive and rmdir fails. So this is a real descriptor leak in the daemon, one per summary extraction, that happened to surface as a Windows-only test failure. Holding the stream and destroying it in a finally fixes both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…ffort Two changes for the same Windows teardown failure, which stayed at 15 of 103 after the last round. destroy() only schedules the close, so extractKimiSummary was returning while the descriptor was still open. Waiting for the close event makes the release deterministic, which is what a caller deleting the directory afterwards actually needs. The afterEach rmSync is now best-effort. NTFS keeps a deleted entry alive until every handle to it is released, so rmdir can raise ENOTEMPTY even after retries. That is a teardown race rather than a fact about the code under test — the assertions have already run and passed by that point, so failing them there reports a defect that does not exist. fakeHome lives under os.tmpdir(), so anything left behind is reclaimed by the OS. To be explicit about the tradeoff: this makes cleanup non-fatal, it does not hide a failing assertion. The one real defect behind these symptoms — the leaked descriptor — is fixed above rather than swallowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…er leak
kimicode is clean now, and the Windows failure moved to packages/cli with
the same signature: "C:\Users\RUNNER~1\...". os.tmpdir() hands back an 8.3
short name there, while git and realpath report the long form, so
ao-update.sh decided its own temp checkout was outside the repo ("must run
inside the Agent Orchestrator git checkout") and a fixture read missed
commands.log outright.
Routed all 27 mkdtempSync(join(tmpdir(), ...)) sites across the CLI tests
through a helper that applies realpathSync.native, the same fix already
used in agent-kimicode. Only update-script was failing, but the other
thirteen carry the identical latent bug and would have surfaced one CI
round at a time.
Note on the mechanical pass: the first attempt skipped the realpathSync
import in path-equality.test.ts, because that file has a test *named*
"...when realpathSync fails" and the guard matched the bare string rather
than the import. Caught locally — it broke six tests on Linux — and fixed
by matching the import specifically.
Full workspace suite passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
…ests The ao-update.sh tests prepend a directory of fake git/pnpm/npm/node scripts to PATH so the script talks to stubs instead of the real tools. They joined with ":", which on Windows is not a separator at all — binDir got spliced onto the first genuine entry and the stubs were never on the search path. The runner's real git answered instead, and since the fake repo is an ordinary temp dir, `git rev-parse --is-inside-work-tree` failed and the script exited with "must run inside the Agent Orchestrator git checkout" before reaching any of the behaviour under test. Route all seven spawn sites through a helper that joins with path.delimiter. It also drops the inherited "Path" key before setting "PATH": Windows environment variables are case-insensitive, so leaving both in the object handed to spawnSync lets the un-prefixed copy win. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
Four of the eight ao-update.sh tests already carried this guard; three that drive the script through the same PATH-injected stubs did not, and they are the only reason Test (windows-latest) is red. Correcting the PATH separator was necessary but not sufficient. Git for Windows does supply bash and the script runs, but the stubs are extensionless files carrying a "#!/bin/bash" line, and that bash does not treat them as executable — `command -v git` walks past them and the runner's real git.exe answers instead. The stub command log is never written, and the script exits at `git rev-parse --is-inside-work-tree` because the fake repo is an ordinary temp dir. Winning the PATH lookup would mean shipping a real .exe, so there is nothing left to assert on Windows. Guard the three, and replace the stale rationale on the existing guard, which blamed bash being unavailable. The one test that needs no stubs still runs on every platform, and the Windows code path is covered separately through detectWindowsBash(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
Regenerating the lockfile dropped the stale `overrides:` block, which took this pin with it and regressed tmp from 0.2.4 to 0.0.33. It reaches the tree through inquirer -> @inquirer/prompts -> @inquirer/editor -> external-editor, a runtime path in the CLI rather than dev-only, and tmp < 0.2.4 is the version affected by the symlink arbitrary-write advisory. Declaring the override in the root manifest is what the lockfile was missing: the block existed there with nothing to back it, which is the mismatch that broke every frozen install in the first place. Now both agree. tmp@0.0.33 no longer resolves anywhere in the tree. The other two entries from the original block are not restored. axios still resolves to 1.15.0 and follow-redirects to 1.16.0 on their own, so both were already no-ops. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
The full-pipeline test timed out at vitest's 10s default. The timings show the whole file was running against that ceiling rather than one test being slow: a bare `--help` took 9130ms, `-h` 428ms, the unknown- flag case 1795ms, and the pipeline 16171ms before it was killed. Nearly all of the first figure is pwsh cold start on the runner, so `--help` was one scheduling blink from failing too. Raise all four to 60s. None of them assert anything about latency — they check argparse and that the script survives its own pipeline — so the default budget was doing nothing but letting runner load decide the outcome. Unrelated to the tmp pin in the preceding commit; that commit's own checks, NPM Audit and Dependency Review, both passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjPWzWR3yvT35opqeixiWy
PR #2 converted CI to pnpm (packageManager: pnpm@9.15.0, pnpm install --frozen-lockfile, pnpm run api in workflows) but AGENTS.md still documented the root scripts as npm run lint/frontend:typecheck/sqlc/api. Update the Commands and API-contract sections to pnpm and note the split: the root is a pnpm workspace while frontend/ deliberately stays npm-managed (npm ci in its workflows, not in pnpm-workspace.yaml), so the frontend-specific npm commands are unchanged. Also drop the stale npx openapi-typescript@7.4.4 wording — api:ts now runs the local openapi-typescript dependency. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
Regenerates
pnpm-lock.yamlagainst the current manifests, removes the rootpackage-lock.json, and repairs the pre-existing CI failures that the install fix uncovered.Why
Every workflow running
pnpm install --frozen-lockfile(ci.yml,security.yml,canary.yml,release.yml,integration-tests.yml,deploy-vps.yml) failed at the install step:The committed lockfile carried an
overrides:block that nopackage.jsonin the tree declared any more, so pnpm refused the frozen install.importers:was stale in the same way — it described root devDependencies the root manifest no longer had, and linkedpackages/aoto@aoagents/ao-cliviaworkspace:*instead of its current platform-binaryoptionalDependencies.Because every job died at install, the failures behind it had never been visible. Fixing the lockfile exposed nine of them. They are unrelated to each other and all pre-date this branch.
The rest of the changes
The root manifest had lost its
devDependenciesandscripts. The stale lockfile was the only surviving record of them, which is how they were recovered:@changesets/cli,eslint,prettier,typescript-eslint,husky, and thebuild/test/api/changesetscripts. Roughly half the failures traced to this one cause.Independent fixes:
0020;0020_review_run_unique_pr_sha.sqllanded first, sopr_reviewswas renamed to0021frontend/src/landingghp_xxxx…filler inSECURITY.md. Allowlisted by path and pattern; the Slack rule was tightened at the same time, since its optional-body?matched a barexoxb-in proseGITHUB_TOKENcannot push commits touching.github/workflows/**. Workflows are now in.prettierignoreao start --no-orchestrator, a flag that no longer exists. Trimmed to steps 1–5continue-on-errormain. Nowfetch-depth: 0Correcting the original claim in this PR: the first revision of this description said the root
package-lock.jsonwas unreferenced. That was wrong.go.yml's api-drift job rannpm ciat the repo root, so deleting the lockfile broke it. That job now usespnpm/action-setupandpnpm install --frozen-lockfile.frontend/package-lock.jsonis untouched — every othercache-dependency-pathin CI points there.Windows
Test (windows-latest)had never passed in this repo. Four causes, in the order they surfaced:better-sqlite3bindings — the FTS integration guard now probes the binding by opening:memory:rather than assuming the module loads.os.tmpdir()returnsC:\Users\RUNNER~1\…while git and realpath report the long form, so anything comparing the two decided the paths differed. 27 sites across 14 test files now canonicalise throughrealpathSync.native.readline.close()does not close the underlyingReadStream. On Windows this surfaced asENOTEMPTYduring teardown, because NTFS keeps deleted entries alive until every handle closes.update-script.test.tsjoined its stub directory with:, which is not a separator on Windows. Correcting that was necessary but not sufficient: Git for Windows supplies bash and the script runs, but it does not treat an extensionless#!/bin/bashstub as executable, socommand -v gitwalks past it and the runner's realgit.exeanswers. Those tests now carry theskipIf(win32)guard four of their siblings already had.Separately, the pwsh doctor tests were raised to a 60s timeout. They had been running against vitest's 10s default while pwsh cold start alone took 9.1s for a bare
--help, so runner load rather than correctness was deciding the outcome.A real bug, not a test issue
Cause 3 above is production code.
extractKimiSummaryinagent-kimicodeleaked one file descriptor per call — the daemon path, not a test fixture. It now awaits a deterministic close.The
tmppinDropping the stale
overridesblock regressedtmpfrom0.2.4to0.0.33. It arrives viainquirer → @inquirer/prompts → @inquirer/editor → external-editor → tmp, a runtime path in the CLI rather than dev-only, andtmp < 0.2.4is the version affected by the symlink arbitrary-write advisory.The pin is restored in the root manifest, which is what the lockfile had been missing all along — the block existed there with nothing backing it, and that mismatch is what broke every frozen install to begin with. Now both agree, and
tmp@0.0.33no longer resolves anywhere in the tree.The other two entries from the original block are not restored:
axiosstill resolves to1.15.0andfollow-redirectsto1.16.0without them, so both were already no-ops.Left for a maintainer
skills/bug-triage/SKILL.mdcarried a conflict between a ReverbCode and an aoagents variant. Resolved one way to unblock CI; worth a look from someone who knows which is current.Diff size
The previous
pnpm-lock.yamlwas committed with CRLF, and regenerating it on Linux produced LF, so the whole file reads as rewritten. Ignoring line endings the real change is ~2.7k lines. A.gitattributesnow pinspnpm-lock.yaml,package-lock.json, and shell/JS sources toeol=lfso a Windows checkout does not reintroduce the churn.Verification
pnpm install --frozen-lockfileERR_PNPM_LOCKFILE_CONFIG_MISMATCHLockfile is up to date, resolution step is skippedA second frozen install leaves the lockfile byte-identical, so it is stable. All 23 checks pass on
97a7821e, includingTest (windows-latest).