Skip to content

Commit 891e6c3

Browse files
mchwangclaude
andcommitted
Mark build step 4 done: #174, #176 and the step list are closed (#194 review round 13)
Merges main (#195, #196, #197) and updates the status lines this PR rewrites; tests that a held cancel refusal also survives Refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 17db340 commit 891e6c3

4 files changed

Lines changed: 15 additions & 7 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Review agent-made Git changes one plan item at a time. The approved plan lists each item's files and acceptance checks; the review engine shows which item produced each change and flags foreign or overlapping work.
44

5-
**Status:** the plan/linking library, SQLite store, local review screen, Plans screen (plan import, plus agent drafts and suggestions with revision-bound Apply; drafts and suggestions need a configured `github` review, not the demo), opt-in code-writing runner, durable local rebase, exact-head plan command checks, and the one-click merge job are implemented. Run `npm run demo` and open its private local URL. Ask, planning and the runner use Claude Code inside locked-down containers; Codex is refused because it reads code only by running commands ([why](docs/implementation/agent-isolation.md#codex-is-refused-in-every-phase)). They need Docker, plus `CLAUDE_CODE_OAUTH_TOKEN` (from `claude setup-token`). Startup builds the agent image, which can take a few minutes. A configured GitHub review can merge only after the guarded exact-head gate passes. The agent container, vendor-only network, credential-free command-check profile, and Claude/Codex adapters are described in [agent isolation](docs/implementation/agent-isolation.md). With a runner, the **Merge PR** button pushes the rebased head, waits for its required checks, and hands that exact pair to the guarded merge ([merge handoff](docs/implementation/merge-handoff.md)). The paired human review experiment was cancelled before results were recorded and no longer blocks roadmap work; optional future validation is tracked in [#19](https://github.com/codeabovelab/codeboost/issues/19).
5+
**Status:** the plan/linking library, SQLite store, local review screen, Plans screen (plan import, plus agent drafts and suggestions with revision-bound Apply; drafts and suggestions need a configured `github` review, not the demo), opt-in code-writing runner, durable local rebase, exact-head plan command checks, and the one-click merge job (with its step list and a **Cancel merge** button) are implemented. Run `npm run demo` and open its private local URL. Ask, planning and the runner use Claude Code inside locked-down containers; Codex is refused because it reads code only by running commands ([why](docs/implementation/agent-isolation.md#codex-is-refused-in-every-phase)). They need Docker, plus `CLAUDE_CODE_OAUTH_TOKEN` (from `claude setup-token`). Startup builds the agent image, which can take a few minutes. A configured GitHub review can merge only after the guarded exact-head gate passes. The agent container, vendor-only network, credential-free command-check profile, and Claude/Codex adapters are described in [agent isolation](docs/implementation/agent-isolation.md). With a runner, the **Merge PR** button pushes the rebased head, waits for its required checks, and hands that exact pair to the guarded merge ([merge handoff](docs/implementation/merge-handoff.md)). The paired human review experiment was cancelled before results were recorded and no longer blocks roadmap work; optional future validation is tracked in [#19](https://github.com/codeabovelab/codeboost/issues/19).
66

77
## Development
88

‎docs/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ These are the decisions that shape the structure. The full list, with the reason
410410
| C, K | Guarded merge, merge queue | Done |
411411
| D | Agent isolation boundary | Done. Since the isolation gate: bounded diff export (#76), task-change inspection and the runner's commit as a bounded `git bundle` (#66, PRs #85 and #90), owner-scoped recovery for Ask (#95), the plan schema for Claude (#97), gitlink mounts and the pre-launch tree check (#81, #99, PR #100), and Codex refused in every phase (#93, PR #106). The runner-contract follow-ups (#51) are closed: the last one tests export and removal through a recovery handle after a real restart (PR #109). |
412412
| E | Planning library and suggestions | Done. E4 (PR #45) added the acceptance fixtures and recorded Claude output that replays through validation and the Store. |
413-
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Required checks are now also read for a token without admin rights (observed with read-only access to public repositories): classic checks then come from the branch read, which has no `strict`, so classic protection gives such a token no atomic base guard. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180), and its recovery removes what an interrupted probe left (#181); follow-up #179. |
413+
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups #176 (PR #195) and #174 (PR #197) are closed, and the merge job's step list and Cancel button (D16) are built (PR #194). T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Required checks are now also read for a token without admin rights (observed with read-only access to public repositories): classic checks then come from the branch read, which has no `strict`, so classic protection gives such a token no atomic base guard. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180), and its recovery removes what an interrupted probe left (#181); follow-up #179. |
414414
| Hardening | Allowlisted subprocess environments | Done: every Git command (#82, #83) and every `gh` runner (#84, #86). |
415415
| Tests | Test suite reliability | Done. The full suite runs ordinary tests first and Docker-backed files serially; CI and the real-Docker gate passed on PR #132 (#129). |
416416
| G | Planning screen | Done (#145). The screen displays/imports the current plan, requests and renders a next-revision draft or suggestion cards, and applies results with revision-bound exact ambiguous replay. G4 composes the browser with the production planning setup, D-backed provider boundary, F endpoints and SQLite persistence; focused adapter, recorded-output and exact-head real-Docker planning suites cover the lower boundary. |

‎docs/designs/codeboost-plan-indexed-review.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ The numbers below identify delivery milestones, not a requirement to implement t
568568
1. **Plan format and linking engine.** A code library, tested with sample git histories. *Done.*
569569
2. **Read-only review screen.** It works on any branch whose commits are in the commit ledger, with a plan loaded into the database. It shows rows, segments, the four checks, approvals, and the per-item conversation. It does not merge (engineering review, O8). *Done; the screen has since gained the merge gate from build step 4.*
570570
3. **Optional validation.** A future real-PR comparison may test the assumptions in "How we will know it works," but it is non-blocking under the superseding decision above.
571-
4. **Merge gate and merging** (product workflow step 9): the merge rules, the pre-merge sequence, and merging through `gh`. Increment 1 (PR #23), merge-queue support (#38, #46, closing #24) and the pre-merge automation (lane F, F3–F6, #22) have merged. *Mostly done:* #174 and #176 are open. The merge job's step list and Cancel button are built. See the scope and completion criteria below.
571+
4. **Merge gate and merging** (product workflow step 9): the merge rules, the pre-merge sequence, and merging through `gh`. Increment 1 (PR #23), merge-queue support (#38, #46, closing #24) and the pre-merge automation (lane F, F3–F6, #22) have merged. *Done:* the last items are closed: #176 (PR #195), #174 (PR #197), and the merge job's step list and Cancel button (D16, PR #194). See the scope and completion criteria below.
572572
5. **Running agents.** Per-task clones, containers, agent adapters, permissions, one invocation per plan item, review rounds, the "already fixed" check, and opening PRs (step 6). Isolation boundary merged (lane D, PRs #31, #40, #44, #47, #50), and the runner merged (lane F; see the lane table in `docs/architecture.md`). Not built: review rounds and screen buttons for the runner actions.
573573
6. **Planning screen.** Writing plans with an agent, and approving plan changes (steps 2 and 3). *Done for T18:* planning backend (lane E, E1–E4) and the Plans screen (lane G, G1–G4, #145; PRs #148, #150, #151, #152). Not built: paste import, in-screen editing, the amendment-proposal view, check warnings, answering questions and a whole-plan approval step (#155). The suggestion cards do not yet conform to the v1 contract (#159).
574574
7. **Queue, schedule, and recovery** (steps 4 and 5).
@@ -588,9 +588,9 @@ Increment 1 alone did not rebase or run acceptance commands: a moved base or a m
588588
- [x] Deliver and validate increment 1 against its final head, including unit/integration and browser regressions, typecheck, and the required review loop.
589589
- [x] Add the automated pre-merge rebase path and preserve ledger mappings and attribution. Resolve foreign-commit conflicts under the approved agent policy (T3, T11); return conflicts requiring human action to review. *Done in F3–F5 (PRs #136, #138, #140, #144, #149).*
590590
- [x] Integrate runner-controlled, containerized execution of approved `cmd:` argv and bind results to the resulting head (T6; depends on the agent isolation and phase enforcement work in build step 5, including T1, T2, and T9). *Done in F5 (PR #149).*
591-
- [ ] Validate the complete pre-merge sequence, including the already-fixed check, approval freshness, refreshed required checks, guarded merge, and races involving either base or head changes (T6, T7, T12). *Built in F6a–F6c (PRs #153, #161, #177), and T7 is verified.* T6 and T12 are verified (PR #183). The merge job's step list and Cancel button are built (D16). Open: #174 and #176.
591+
- [x] Validate the complete pre-merge sequence, including the already-fixed check, approval freshness, refreshed required checks, guarded merge, and races involving either base or head changes (T6, T7, T12). *Built in F6a–F6c (PRs #153, #161, #177), and T7 is verified.* T6 and T12 are verified (PR #183). The merge job's step list and Cancel button are built (D16, PR #194). #176 was closed by PR #195 and #174 by PR #197.
592592

593-
**Sequencing decision.** The guarded gate can land before the agent runner. Build step 5 supplied the execution prerequisites, and the pre-merge automation shipped only after they were integrated (F3–F6, #22). Build step 4 stays open until the open items above are done: #174 and #176. The safe refusal in increment 1 was an intentional interim behavior, not evidence that the automation had shipped. Optional human validation in #19 remains non-blocking.
593+
**Sequencing decision.** The guarded gate can land before the agent runner. Build step 5 supplied the execution prerequisites, and the pre-merge automation shipped only after they were integrated (F3–F6, #22). Build step 4 stayed open until those items were done: #174, #176, and the step list and Cancel button. All three are now merged. The safe refusal in increment 1 was an intentional interim behavior, not evidence that the automation had shipped. Optional human validation in #19 remains non-blocking.
594594

595595
## Open questions
596596

‎test/browser/review.spec.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,12 +1091,15 @@ test('shows the refusal in the banner when an enabled Cancel is refused because
10911091
await page.route('**/api/action',async route=>{const body=route.request().postDataJSON();
10921092
if(body.action==='cancel-merge'){await route.fulfill({status:409,contentType:'application/json',body:JSON.stringify({error:'GitHub has been asked to merge; wait for its outcome.',job:merging})});return;}
10931093
if(body.action!=='merge'){await route.continue();return;}
1094-
const review=await clickReview(route);
1094+
started=true;const review=await clickReview(route);
10951095
await route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({...review,merge:runnerMerge(review.merge,{ready:false,action:null,job:merging}),job:merging})});});
10961096
let failNext=false;
10971097
await page.route('**/api/merge',route=>{if(failNext){failNext=false;return route.fulfill({status:502,contentType:'application/json',body:JSON.stringify({error:'GitHub returned HTTP 502.'})});}
10981098
return route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({queue:null,job:merging})});});
1099-
await page.route('**/api/review',async route=>{const response=await route.fetch(),body=await response.json();body.merge=runnerMerge(body.merge,{ready:true,action:'merge',job:null});await route.fulfill({response,json:body});});
1099+
// As a runner server answers a full review: the running job rides along once the click started it.
1100+
let started=false;
1101+
await page.route('**/api/review',async route=>{const response=await route.fetch(),body=await response.json();
1102+
body.merge=runnerMerge(body.merge,started?{ready:false,action:null,job:merging}:{ready:true,action:'merge',job:null});await route.fulfill({response,json:body});});
11001103
await page.goto(app.url);page.on('dialog',dialog=>dialog.accept());
11011104
await page.getByRole('button',{name:'Merge PR',exact:true}).click();
11021105
const cancel=page.getByRole('button',{name:'Cancel merge',exact:true});
@@ -1112,6 +1115,11 @@ test('shows the refusal in the banner when an enabled Cancel is refused because
11121115
failNext=true;
11131116
await expect(page.locator('#banner')).toHaveText(/^Could not refresh merge status\./,{timeout:15_000});
11141117
await expect(page.locator('#banner')).toHaveText('Could not cancel the merge. GitHub has been asked to merge; wait for its outcome.',{timeout:40_000});
1118+
// Refresh rewrites the banner too; the next poll, answering the same record, shows the refusal again.
1119+
const reloaded=page.waitForResponse(response=>response.url().endsWith('/api/review'));
1120+
await page.locator('#reload').click();await reloaded;
1121+
await expect(page.locator('#banner')).not.toHaveText(/^Could not cancel the merge\./);
1122+
await expect(page.locator('#banner')).toHaveText('Could not cancel the merge. GitHub has been asked to merge; wait for its outcome.',{timeout:40_000});
11151123
});
11161124
test('disables Cancel once the merge job asked GitHub to merge (D16)',async({page})=>{
11171125
const config={...app.service.config,demo:false};await app.close();removeDemoOutOfScope(config);

0 commit comments

Comments
 (0)