G2: add planning authoring and suggestion cards

[mchwang]

Summary

• add Plans controls for whole-plan drafting and independent edit suggestions
• poll and dismiss durable planning requests with bounded backoff and guarded lifecycle ownership
• preserve newer guidance, completed historical results, focus, and stale-context visibility
• validate exact v1 result structures, endpoint mode, immutable plan binding, and lifecycle/result combinations before rendering
• render agent output as escaped literal text; applying a draft or card remains G3

Tracks #145. #146 owns G3 Apply and the transport-size follow-up. This PR does not change web/server.ts, Store, runner, or Docker/agent-isolation contracts.

Done when

npx playwright test test/browser/plans.spec.ts test/browser/plans-authoring.spec.ts && npm run typecheck passes and proves authoring/suggestion results cannot erase newer input, masquerade as current after a binding or lifecycle change, or render stale/malformed polling responses as current.

Validation

Validated head: b35e67bfac745dd3a4cafd44b0e8df568241bc09
Base: 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e

• focused G2 browser: 17/17 passed locally and independently
• planning/backend: 169/169 passed independently
• typecheck: passed locally, independently, and in both CI runs
• exact-head CI push run 37832029312: passed; full non-Docker Vitest suite and full Playwright suite passed
• exact-head CI pull-request run 37832038020: passed; full non-Docker Vitest suite and full Playwright suite passed
• visual check: passed against DESIGN.md
• full local npm test: infrastructure-blocked by shared Docker saturation; stopped after 15 minutes and 539 containers without a product assertion
• real-Docker agent-isolation suite: not run for G2 because no trigger-path file changed; PR F5: prepare rebases and run head-bound checks #149 owns the repository's active heavyweight Docker validation

Review record

1. Author self-review fixed Dismiss focus replacement and added the focus regression.
2. Independent rounds found and drove fixes for old-poll/cancellation ordering, malformed-status ownership, completed history, binding/mode validation, status rollback, hostile output, complete nested v1 payloads, and JSON-Schema-compatible Unicode code-point bounds.
3. Copilot rounds 1-5 found cancelled output retention, missing stale-dismissal coverage, invalidated actions/labels, disabled styling, stale architecture text, operation mode, complete payload validation, consumed-state presentation, and impossible failed output. Each valid finding received a targeted regression and a fresh exact-head independent review.
4. Latest independent full-diff review on 26d03e6...b35e67b is CLEAN. Independent validation: typecheck, planning/backend 169/169, focused G2 browser 17/17, and diff check.
5. Copilot round 6 on b35e67b reported zero open code findings and a validation-only summary concern. The claim that CI was unconfirmed is declined as factually contradicted by the two successful exact-head runs above. The separate Docker-backed suite remains explicitly infrastructure-blocked/not applicable to this UI-only diff; it is not represented as passed.

Review-lesson audit

• Lifecycle ordering, stale/consumed state, cancellation ownership, focus preservation, and controlled interleavings are covered by the existing Async jobs and polling, Async review UI, and Required race regressions rules.
• Exact response shapes, mode/binding checks, fail-closed malformed output, and exact-head readback are covered by External status confirmation and Review readiness.
• Complete typed payloads, Unicode bounds, and escaped hostile content are covered by Agent-controlled content plus the existing schema/claim-verification review rules.
• Disabled styling and architecture-status corrections are one-off implementation/documentation consistency fixes; no new operating rule is needed.
• No review finding requires a new AGENTS.md/CLAUDE.md rule.

Scope boundary

G2 starts, observes, displays, and dismisses planning results. G3 owns revision-bound Apply and request-time draft/attachment preservation. G4 owns real-provider/store integration and remaining T18 browser/adapter acceptance.

[mchwang]

Copilot review prerequisite record:

• Exact base/head: 26d03e6 / 83e7f2b
• CI: 2/2 test checks green on this head
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 remains F-owned and is outside G2 scope
• Latest independent review: clean full-diff pass on the exact base/head pair above; all earlier findings resolved
• Exact-head validation: typecheck passed; Plans browser 43/43; G2 focused browser 12/12; planning backend 63/63; diff check clean
• Local full-suite Docker gate: attempted but infrastructure-blocked by shared Docker saturation; remote CI full non-Docker and browser suites are green

[Copilot]

🟡 Changes recommended

Dismissed output remains visible, and the required stale-poll cancellation regression is missing.

2 open findings

• Cancelled requests still render retained drafts and suggestions · New
• Add regression test for stale poll response after dismissal · New

What changed in this PR

Adds agent-assisted plan drafting and edit suggestions to the Plans screen.

Changes:

• Adds drafting, suggestions, polling, dismissal, and history UI.
• Validates and safely renders lifecycle responses.
• Adds browser coverage and implementation documentation.

File	Description
web/​public/​style.css	Styles authoring controls and results.
web/​public/​index.html	Adds plan authoring UI.
web/​public/​app.js	Implements request lifecycle and rendering.
test/​browser/​plans-authoring.spec.ts	Tests authoring and lifecycle behavior.
docs/​implementation/​planning-screen.md	Documents G2 behavior and scope.

🧠 Review effort: Balanced

---

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

[mchwang]

Copilot re-review prerequisite record:

• Exact base/head: 26d03e6 / 4c1d1b0
• CI: 2/2 test checks green on this head
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 remains F-owned and is outside G2 scope
• Latest independent review: clean full-diff pass on this exact base/head pair; all prior independent and Copilot findings resolved
• Exact-head validation: typecheck passed; G2 focused browser 13/13; Plans browser 44/44; prior planning backend 63/63 remains unaffected; diff check clean
• Copilot round 1: two valid findings fixed in 4c1d1b0, replied to, resolved, and covered by failing-before/passing-after browser regressions
• Local full-suite Docker gate remains infrastructure-blocked by shared Docker saturation; both remote full non-Docker and browser CI suites are green

[Copilot]

🔵 Needs a closer look

Invalidated results expose a nonfunctional dismissal action, and disabled styling and architecture documentation remain inconsistent.

0 open findings

2 resolved since last review

• Cancelled requests still render retained drafts and suggestions
• Add regression test for stale poll response after dismissal

Previously missed (3)

In code that hasn't changed since last review

Dismiss action remains visible for invalidated results

web/​public/​app.js:1004

Only pending and ready requests can be cancelled by the Store, but this renders Dismiss for retained invalidated (and any failed) result as well. After an import invalidates a ready request, clicking this button reconciles back to invalidated, so the button and result never disappear. Limit the action to a current ready request while retaining it during cancelling to preserve focus.

Disabled Suggest buttons still appear enabled

web/​public/​style.css:844

The new primary Suggest button remains blue when aria-disabled: the later button.primary rule overrides the generic disabled colors with equal specificity. That makes an unavailable/in-flight action still look enabled, contrary to the disabled-state design contract. Add a more-specific override for these controls.

Architecture document has outdated feature-status entries

docs/​implementation/​planning-screen.md:7

This documents G2 as delivered, but the repository-wide architecture document still says “plan authoring and suggestion controls” are not built and describes the Plans screen as display/import only (docs/architecture.md:47,167). Update those build-state entries so developers do not get contradictory feature-status guidance.

🧠 Review effort: Balanced

[mchwang]

Copilot round 2 summary-only concerns addressed in 1fa7cb6:

• Stale or terminal non-ready results no longer expose Dismiss; the control remains only for a current ready result or the already-running cancelling state needed to preserve focus.
• Added failing-before/passing-after coverage that imports a newer revision while an older ready response is held, then proves the stale result has no Dismiss action.
• Primary aria-disabled controls now use the same surface, muted text and line border treatment as other disabled buttons. The computed-style regression is transition-safe and passed three repeated runs plus the full focused file.
• docs/architecture.md now consistently records G2 authoring and suggestion controls as built while G3 Apply remains future work.

Exact-head validation before push: typecheck passed; G2 browser 13/13. Fresh independent full-diff review is CLEAN on base/head 26d03e6 / 1fa7cb6. No concerns were declined.

[mchwang]

Copilot round 3 prerequisite record:

• Exact base/head: 26d03e6 / 1fa7cb6
• CI: 2/2 test checks green on this head
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 remains F-owned and is outside G2 scope
• Latest independent review: clean full-diff pass across all six files on this exact base/head pair
• Exact-head validation: typecheck passed; G2 focused browser 13/13; three repeated disabled-style runs 3/3; diff check clean
• Copilot rounds: round 1 two inline findings fixed; round 2 confirmed both resolved and surfaced three summary-only concerns, all reproduced or corrected and fixed in 1fa7cb6
• No findings were declined
• Local full-suite Docker gate remains infrastructure-blocked by shared Docker saturation; both remote full non-Docker and browser CI suites are green

[Copilot]

🔵 Needs a closer look

Poll validation misses operation mode, and invalidated historical results can be mislabeled as non-stale.

0 open findings

Previously missed (2)

In code that hasn't changed since last review

Mark invalidated historical results as stale

web/​public/​app.js:938

An invalidated request is stale even when its revision and snapshot IDs still match—for example, the Store can invalidate a ready result when its checkpoint continuation changes. If another request starts, this result moves to history, but this predicate returns false and the only remaining label becomes “Earlier result” rather than “Stale.” Include the lifecycle state in the stale predicate so invalidated historical output is never presented as current-context output.

Reject responses with a mode mismatched to the operation

web/​public/​app.js:974

The status validator ignores the operation discriminator returned by suggestion reads. A malformed response with mode: "draft" is accepted when it contains a suggestion-shaped reply (and draft responses accept an injected suggestion mode), so a cross-operation polling payload can render instead of failing closed. Validate the endpoint-specific mode shape along with the lifecycle fields, and cover the wrong-mode response in the malformed-status regression.

🧠 Review effort: Balanced

[mchwang]

Copilot round 3 summary-only concerns addressed in bd5380b:

• Invalidated requests are now stale by lifecycle state even when revision and snapshot still match. A failing-before/passing-after regression proves the current invalidated status and its later historical label both remain stale.
• Poll validation now enforces the endpoint-specific operation shape: suggestion reads require mode=suggest, while draft reads reject any mode discriminator because the draft API contract omits it.
• The wrong-operation regression covers both directions, proves no foreign payload renders, preserves request ownership, and then recovers to each real terminal response.
• Status validation first proves the payload is an object, so primitive JSON also fails through the standard guarded path.

Exact-head validation before push: typecheck passed; G2 browser 15/15. Fresh independent full-diff review is CLEAN on base/head 26d03e6 / bd5380b; the reviewer independently reran 15/15. No findings were declined.

[mchwang]

Copilot round 4 prerequisite record:

• Exact base/head: 26d03e6 / bd5380b
• CI: 2/2 test checks green on this head
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 remains F-owned and is outside G2 scope
• Latest independent review: clean full-diff pass across all six files on this exact pair; reviewer independently ran G2 browser 15/15
• Exact-head validation: typecheck passed; G2 focused browser 15/15; diff check clean
• Copilot rounds: round 1 fixed two inline findings; round 2 fixed three summary-only concerns; round 3 fixed two summary-only fail-closed concerns
• No findings were declined
• Local full-suite Docker gate remains infrastructure-blocked by shared Docker saturation; both remote full non-Docker and browser CI suites are green

[Copilot]

🔵 Needs a closer look

Partial payload validators allow malformed drafts and edit cards to be presented as valid results.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Validate complete edit payloads before accepting ready results

web/​public/​app.js:960

These branches accept schema-invalid edit cards as ready results. For example, add_item passes with new_item: null, and add_file passes with only a path, even though the v1 edit contract requires the complete typed payload (and null unused payloads). Such malformed polling data is then rendered as a valid suggestion. Validate the full edit shape and operation-specific payload invariants before accepting the status.

This issue also appears on line 980 of the same file.

🧠 Review effort: Balanced

[mchwang]

Copilot round 4 follow-up on the complete-result validation finding:

• Replaced partial ready-result checks with exact v1 plan/edit-reply validation, including complete nested item/file/check objects and operation-specific used/null payloads.
• Added failing-before coverage for incomplete add_item, partial add_file, and incomplete nested draft items, plus positive coverage for all nine edit operations.
• Independent review then found a Unicode boundary mismatch: JavaScript code-unit length could reject JSON-Schema-valid astral text and retain polling ownership. Fixed by bounded Unicode code-point counting and added a 4,000-emoji reply regression that verifies rendering and control release.

Exact reviewed pair: 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e...4c44e89c47a4e409a45bc7e95ee155eb19fc10c0

Independent review: CLEAN, no unresolved findings. Independent validation: typecheck; planning/backend 169/169; Plans browser 47/47; focused authoring browser 16/16; diff check. Local exact-head validation: typecheck and focused authoring browser 16/16.

Waiting for exact-head CI before requesting the next Copilot round.

[mchwang]

Ready for Copilot round 5 on exact pair 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e...4c44e89c47a4e409a45bc7e95ee155eb19fc10c0.

• CI: both exact-head test runs green (6m46s and 5m56s)
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 (G3 Apply, outside G2)
• Latest mandatory independent full-diff review: CLEAN on this exact pair
• Independent validation: typecheck; planning/backend 169/169; Plans browser 47/47; focused G2 browser 16/16; diff check
• Local exact-head validation: typecheck and focused G2 browser 16/16

Requesting Copilot only after the independent loop and CI completed cleanly.

[Copilot]

🔵 Needs a closer look

Consumed and malformed failed lifecycle states can be presented incorrectly.

0 open findings

Previously missed (2)

In code that hasn't changed since last review

Treat consumed results as stale

web/​public/​app.js:938

consumed is accepted as a valid lifecycle state, but it is neither stale here nor handled by renderPlanAuthor. If another tab or API client applies the result between polls, this tab still holds the old review data, so the consumed result renders as current with an empty status. Treat consumed output as stale (and prompt the user to refresh) because applying it necessarily advanced the plan revision.

Reject non-null results for failed requests

web/​public/​app.js:1041

failed currently falls through to accepting a non-null valid plan/reply. The Store can only attach output when publishing ready; failed requests settle from pending with no output. Accepting this impossible combination lets a malformed poll render and later archive agent output under a failed request. Require a null result for both pending and failed, while retaining the optional-result behavior only for cancelled/invalidated.

🧠 Review effort: Balanced

[mchwang]

Copilot round 5 follow-up on exact head 4c44e89:

• consumed results are now always stale because applying them necessarily advanced the durable plan outside this screen's current review response. They render with stale status, expose no Dismiss action, and release author controls.
• failed status now requires a null result. Impossible failed+output observations fail closed without rendering and retain pending polling ownership.
• Added a controlled failing-before/passing-after browser regression: the malformed failed response is observed while the second poll is held, then explicit release proves the consumed response renders stale and releases controls.

New exact reviewed pair: 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e...b35e67bfac745dd3a4cafd44b0e8df568241bc09

Independent full-diff review: CLEAN. Independent validation: typecheck; planning/backend 169/169; focused authoring browser 17/17; diff check. Local exact-head validation: typecheck and focused authoring browser 17/17.

Waiting for exact-head CI before requesting Copilot round 6.

[mchwang]

Ready for Copilot round 6 on exact pair 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e...b35e67bfac745dd3a4cafd44b0e8df568241bc09.

• CI: both exact-head test runs green (6m58s and 7m04s)
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Deferred follow-up: Align plan import request limit with valid schema payloads #146 (G3 Apply, outside G2)
• Latest mandatory independent full-diff review: CLEAN on this exact pair
• Independent validation: typecheck; planning/backend 169/169; focused G2 browser 17/17; diff check
• Local exact-head validation: typecheck and focused G2 browser 17/17

Requesting Copilot only after the changed-head independent loop and CI completed cleanly.

[Copilot]

🔵 Needs a closer look

The asynchronous lifecycle changes are substantial, while the full suite and CI remain unconfirmed.

0 open findings

🧠 Review effort: Balanced

[mchwang]

Copilot round 6 follow-up:

The review reported 0 open findings and one validation-only summary concern: “the full suite and CI remain unconfirmed.” No code failure or counterexample was identified.

Declined in part, with current exact-head evidence:

• CI is confirmed for b35e67bfac745dd3a4cafd44b0e8df568241bc09: runs 37832029312 and 37832038020 both succeeded.
• Each run executed typecheck, the repository's full non-Docker Vitest command, and the full Playwright command.
• The Docker-backed agent-isolation suite is reported separately and is not claimed as passed. It was locally infrastructure-blocked by shared-daemon saturation, this UI-only diff changes none of its trigger paths, and PR F5: prepare rebases and run head-bound checks #149 owns the active heavyweight Docker validation.

The PR body now records the exact commands/gates, all review rounds, and the review-lesson audit. No source change was made, so the clean independent review remains valid for exact pair 26d03e6...b35e67b.

[Copilot]

🟢 Approval recommended

The implementation is consistent with its lifecycle and security requirements and has comprehensive exact-head validation.

0 open findings

🧠 Review effort: Balanced

[mchwang]

Final G2 readiness record for exact pair 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e...b35e67bfac745dd3a4cafd44b0e8df568241bc09:

• Copilot round 7: Approval recommended, 0 open findings
• Mandatory independent full-diff review: CLEAN
• CI: both exact-head runs green
• Mergeability: MERGEABLE
• Unresolved review threads: 0
• Focused G2 browser: 17/17 locally and independently
• Planning/backend: 169/169 independently
• Typecheck: passed locally, independently, and in CI
• Review-lesson audit: complete in the PR body
• Deferred: Align plan import request limit with valid schema payloads #146 owns G3 Apply/transport follow-up
• Docker-backed full suite: explicitly not claimed; local attempt was infrastructure-blocked and this UI-only diff does not trigger that workflow

No merge performed; awaiting explicit authorization.