Skip to content

Reconcile the design plan with the code - #52

Merged
mchwang merged 11 commits into
mainfrom
docs/reconcile-plan-with-code
Sep 26, 2026
Merged

mchwang merged 11 commits into
mainfrom
docs/reconcile-plan-with-code

Conversation

@mchwang

@mchwang mchwang commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Docs only: docs/designs/codeboost-plan-indexed-review.md and docs/implementation/guarded-merge.md. It brings the approved plan in line with the code on main (checked 2026-09-26 against 5e97aa3).

Decisions recorded

  • Ask is an interim exception to R1. runner/question-agent.ts runs the claude or codex CLI on the host, in a temporary folder, with tools, web and MCP off. It is limited to Ask. Lane D5 has merged (D5: Add the combined real-Docker isolation gate #50), so the container exists; the exception ends when lane F moves Ask onto D's invocation contract.
  • D20 amended: no "Merge anyway". codeboost never merges while a blocker remains; to override one, merge on GitHub. This matches docs/implementation/guarded-merge.md and the rule that no override bypasses server-side base protection. The screen spec, step 9, the decision table and DT7 are updated.

Status brought up to date

guarded-merge.md no longer says merge-queue branches stay blocked: it points to merge-queue.md (#46) and notes that adapters without queue inspection still fail closed.

Historical review records (engineering findings, decision ledger, reviewer concerns) are unchanged.

Validation

Evidence on head a756f5b:

  • npm run typecheck: passed.
  • npm test using CI's excludes: 435 passed, 9 failed in history, review and store (real-git tests). The machine had a load average of about 116 with 168 Docker containers running. This PR changes only one Markdown file, so GitHub CI on this head is the result to use.
  • Self-review of the full diff done under AGENTS.md; every status claim was checked against code, tests or gh pr view.

Review-lesson audit

No review findings yet.

Follow-ups (not in this PR)

  • Lane F: move Ask into the container.

🤖 Generated with Claude Code

Readiness report

  • Head: cbffd94. It marks the install preflight and npx entry as planned (the CLI checks only Node today), per Copilot's review of 49d5fcd. e5e4f05 added F1e F1e: planning API for lane G and feedback from review actions #60 and the D follow-ups required by the F1 runner lifecycle contract #51 merge condition to the F lane row. 49d5fcd corrects the README's merge-queue removal sentence, per Copilot's review of e5e4f05: a plain removal can be retried, and only a changed head needs a fresh review.
  • CI: test passed on cbffd94.
  • Mergeability: mergeable, no conflicts
  • Unresolved review threads: 0 (all 3 resolved)
  • Latest Copilot review (cbffd94): approval recommended, no findings.
  • Files: docs/designs/codeboost-plan-indexed-review.md and README.md; docs only.
  • Deferred follow-ups:
    • docs/designs/codeboost-plan-indexed-review.md:533 (L1: "rejected" closes a task) still conflicts with step 8 (Reject with feedback requeues the task). The F1 contract follows step 8 until this line is reconciled.
    • The README's Ask limit and the plan's R1 exception should be removed together when Run Ask in the lane D agent container #54 (Ask in the agent container) merges.

Record where the code differs from the approved plan and update stale status:

- Record Ask as an interim exception to R1: it runs the vendor CLI on the
  host with tools off until lane F moves it into the lane D container.
- Amend D20: there is no "Merge anyway"; to override a blocker, merge on
  GitHub. Matches docs/implementation/guarded-merge.md.
- Tick T1, T2, T4, T5, T10, T13, T14 with test evidence; point Files lines
  at core/linking.ts and core/approvals.ts instead of never-created modules.
- Mark increment 1 merged; add a lane status table (C, D, K done; E, F, H
  progress); record decided open questions (issue ranking, AgentDiff).
- Add a verified status note for design tasks DT2-DT15; none newly ticked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 26, 2026 07:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Documentation contains unresolved contradictions about merge queues, isolation limits, README status, and dependency checks.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

This docs-only PR reconciles the design plan with the current implementation, lane progress, safety decisions, and remaining work.

Changes:

  • Documents Ask’s interim host-CLI exception.
  • Updates merge, isolation, milestone, and task statuses.
  • Records validation evidence and implementation gaps.
File Summary
docs/​designs/​codeboost-plan-indexed-review.md Updated design decisions, implementation status, and roadmap progress.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/designs/codeboost-plan-indexed-review.md Outdated
The guarded merge gate doc still said merge-queue branches stay blocked.
#46 (closing #24) added queue lifecycle support. Point to merge-queue.md,
and state that adapters without queue inspection still fail closed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved documentation contradictions and inaccurate implementation guidance remain.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Document or implement missing startup tool and provider checks

docs/​designs/​codeboost-plan-indexed-review.md:648

This says codeboost checks for gh, Docker/Podman, and at least one provider, but the current CLI only enforces the Node version; startServer does not probe those tools or require a provider, and the runner has not wired in the container boundary yet. Since this document is explicitly reconciled against main, please qualify these as feature-specific/future requirements or add the missing startup checks.

mchwang and others added 2 commits September 26, 2026 02:37
Merge main. Record the F1 lifecycle contract (#49) and open F1a-F1c
(#53, #56, #57); record the ranked Issues screen (H4a, #55) with H4b's
trust action remaining; Issues is now a menu link, not a placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Documentation contains unresolved contradictions and claims that do not match the current implementation.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)

Comment thread docs/designs/codeboost-plan-indexed-review.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 10:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Two moderate documentation inconsistencies and several wording corrections remain unresolved.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)

README no longer says merge queues block merging; it describes the
enqueue-then-confirm behaviour. The plan's wave-3 note records the old
block as history instead of a live instruction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Documentation scope, isolation claims, queue guidance, and task sequencing contain unresolved inconsistencies.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (2)

Comment thread docs/designs/codeboost-plan-indexed-review.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved documentation mismatches remain across README scope, security disclosures, prerequisites, and queue validation.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity README omits the host-side Ask security limitation

docs/​designs/​codeboost-plan-indexed-review.md:227

This sentence says the README states the host-side Ask exception, but README.md only says that Ask provides read-only answers; it does not disclose that the CLI runs on the host with the user's environment/sign-in. Please update the README to carry this security-relevant limitation, or remove this claim from the plan so the two documents do not disagree.

The plan (R1 exception) says README states this limit; it did not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings document startup checks that the implementation does not currently perform.

Review effort: Lite
Findings: None

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Address the documented scope and behavior mismatches before approval.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Low severity Clarify ordinary removal retry versus changed-head fresh review

README.md:34

The queue lifecycle does not always send a removal back to review: for an ordinary removal with the same revision, snapshot, review version, and head, MergeCoordinator marks the attempt retryable after the required refresh and the UI offers retry; only a changed head or an attempt requiring fresh review blocks retry. Please distinguish ordinary removal/retry from the changed-head fresh-review path so this README does not promise a review reset that the implementation does not perform.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The design plan must clarify or implement the dependency checks it currently describes as startup preflight.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Document actual dependency preflight behavior

docs/​designs/​codeboost-plan-indexed-review.md:648

This new installation requirement says the CLI checks for git, gh, Docker/Podman, and provider credentials, but web/cli.ts only calls requireSupportedNode() before starting the server; the other dependencies are not preflighted. That makes the plan inaccurate for the current code. Either add those startup checks or describe them as feature-time requirements rather than checks performed by codeboost.

The CLI checks only the Node version today; say so instead of describing
the git/gh/container/sign-in preflight as current.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 26, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

Documentation-only changes with no blocking issues; remaining comments are minor wording clarifications.

Review effort: Lite
Findings: None

@mchwang
mchwang merged commit d496b9c into main Sep 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants