π§βπ» Establish code quality tooling and CI - #5
Conversation
Leverframe reviewπ’ Incremental review completed in 244s
Changes |
There was a problem hiding this comment.
Leverframe review
No actionable defects were found in the exact base-to-head diff. Verification used the repository-declared pnpm 11.23.0 with Node v24.20.0, matching CI's Node 24 line. The frozen install, full quality gate, test suite, coverage run, and production-artifact imports passed. The pinned GitHub Action revisions also resolve to their documented tags.
Findings
No actionable defects found.
Checks
4 passed Β· 0 failed Β· 1 not run
Show 5 checks
| Status | Check | Evidence |
|---|---|---|
| π’ passed | node --version; pnpm --version; corepack --version |
Node v24.20.0, pnpm 11.23.0, Corepack 0.35.0. |
| π’ passed | pnpm install --frozen-lockfile && pnpm check |
Lockfile verified; formatting, oxlint, ESLint, TypeScript checking, 20 unit/integration tests across 7 files, and the production build all passed. |
| π’ passed | pnpm test:coverage |
All 20 tests across 7 files passed under Vitest 4.1.11 with the V8 coverage provider. |
| π’ passed | node --input-type=module -e "import production build modules" |
The emitted CodexPro manifest loaded successfully with all 18 tools, and the emitted configuration module loaded successfully. |
| βͺ not run | pnpm test:e2e |
The required Docker Sandboxes sbx executable and local CodexPro sandbox template are unavailable in this environment. |
Limitations
- Real Docker Sandbox/CodexPro E2E behavior could not be executed because
sbxand its required local template are unavailable; coverage completeness is therefore marked false.
There was a problem hiding this comment.
Leverframe review
One actionable defect found. The full local quality gate passed, but real Docker Sandbox E2E verification was unavailable.
Findings
π‘ [MEDIUM] Initial snapshot failure orphans the launched command
src/codexpro/bash-sessions.ts:209 Β· confidence: high
The try block covers both launching the detached runner and reading its initial snapshot. If launch succeeds but the subsequent status/output call fails transiently, this line deletes the session metadata. The caller receives an error and no session ID, while the commandβunbounded by defaultβcontinues inside the sandbox and can no longer be polled or stopped through the public API.
Evidence: A focused executor harness allowed launch to succeed and injected an error on the following snapshot call. The delayed command still created its marker, while polling the discovered session ID returned Unknown Bash session.
Suggested action: Once launch succeeds, retain the session so the caller can retry polling, or reliably terminate the process group before deleting the session entry.
Checks
5 passed Β· 1 failed Β· 0 not run
Show 6 checks
| Status | Check | Evidence |
|---|---|---|
| π’ passed | node --version; pnpm --version |
Used Node v24.20.0 and the exact package-manager pin pnpm 11.23.0. |
| π’ passed | pnpm install --frozen-lockfile |
Frozen lockfile installation and supply-chain policy verification passed. |
| π’ passed | pnpm check |
Formatting, oxlint/eslint, TypeScript typecheck, 31 unit/integration tests, and production build passed. |
| π’ passed | node --input-type=module <<'NODE' ... failure-injection harness ... NODE |
Reproduced a continuing command whose session entry was deleted after the injected initial-snapshot failure. |
| π΄ failed | pnpm test:e2e |
Stopped at the documented prerequisite check because sbx is unavailable (spawnSync sbx ENOENT); this is not a code defect. |
| π’ passed | git diff --check 3f43290cc966d76b142ccdb1480805415e3dc255...e99eb001b9abb37fd23e064215870d1e39d3fd66 |
No whitespace errors. |
Limitations
- Real Docker Sandbox/CodexPro E2E verification was unavailable because the required
sbxexecutable and template are absent; coverage completeness is therefore false.
There was a problem hiding this comment.
Leverframe review
Reviewed the complete incremental diff using Node v24.20.0 and pnpm v11.23.0, matching the workflow's Node 24 environment and the exact packageManager/workflow pnpm pin. Found one low-severity concurrent lifecycle defect.
Findings
1 finding was published inline.
Checks
6 passed Β· 1 failed Β· 0 not run
Show 7 checks
| Status | Check | Evidence |
|---|---|---|
| π’ passed | node --version; pnpm --version |
Node v24.20.0 and pnpm 11.23.0. |
| π’ passed | pnpm install --frozen-lockfile |
Installed all dependencies from the unchanged frozen lockfile using pnpm 11.23.0. |
| π’ passed | pnpm check |
Formatting, oxlint, ESLint, TypeScript checking, 33 unit/integration tests, and the production build all passed. |
| π’ passed | pnpm exec vitest run --project integration test/integration/bash-sessions.test.ts |
All 12 Bash-session integration tests passed, including recovery after the initial snapshot failure. |
| π’ passed | pnpm exec vitest run --project integration test/integration/review-concurrent-destroy.test.ts |
A temporary focused regression harness reproduced the stale-handle path: start returned running after destruction, and polling rejected as unknown. |
| π’ passed | git diff --check e99eb001b9abb37fd23e064215870d1e39d3fd66...484ec8a3c398c95540c856950c1ba2bf8e827032 |
No whitespace errors were reported. |
| π΄ failed | command -v sbx && sbx --version |
The repository-required sbx executable is not installed in this environment. |
Limitations
- Real Docker Sandbox E2E verification was unavailable because
sbxand the required CodexPro template were absent. Docker Engine 29.7.2 was available, but it does not replace that repository-declared prerequisite.
There was a problem hiding this comment.
Leverframe review
Reviewed both incremental files using Node.js 24.20.0, pnpm 11.23.0, TypeScript 5.9.3, and Vitest 4.1.11. The production race fix is correct, but the added regression test leaks its detached host process and session directory on every run.
Findings
1 finding was published inline.
Checks
3 passed Β· 1 failed Β· 1 not run
Show 5 checks
| Status | Check | Evidence |
|---|---|---|
| π’ passed | corepack pnpm install --frozen-lockfile |
Installed the exact lockfile using pnpm 11.23.0; 307 packages were installed and supply-chain policy verification passed. |
| π’ passed | corepack pnpm exec vitest run --project integration test/integration/bash-sessions.test.ts --reporter=verbose |
All 13 Bash-session integration tests passed, including snapshot recovery and concurrent sandbox destruction. |
| π’ passed | pnpm check |
Formatting, oxlint/ESLint, TypeScript checking, all 34 unit/integration tests, and the production build passed using the repository-declared pnpm 11.23.0. |
| π΄ failed | corepack pnpm check |
A local Corepack nested-script PATH mismatch resolved pnpm 11.20.0 inside the script and triggered the repository's exact-version guard. Running the supported command directly with pnpm 11.23.0 passed. |
| βͺ not run | pnpm test:e2e |
The repository requires the sbx executable and a preinstalled CodexPro sandbox template; sbx was unavailable. Docker client/server 29.7.2 was present but is insufficient for this suite. |
Limitations
- Real Docker Sandbox/CodexPro E2E verification was unavailable because the required
sbxruntime was not installed; the changed race was verified with the focused integration harness and inspected against SandboxService locking and destruction-listener behavior.
Summary
Verification
pnpm checkβ passed formatting, linting, typechecking, 20 unit/integration tests, and the production build.pnpm test:coverageβ passed with a V8 coverage baseline and no enforced percentage threshold.docker run --rm -v "$PWD:/repo:ro" rhysd/actionlint:latest -color /repo/.github/workflows/ci.ymlβ passed with no workflow findings.pnpm test:e2eβ verified to fail fast when the Docker Sandboxessbxexecutable is unavailable instead of silently skipping; the real microVM E2E suite was not run in this environment.