Add code-linked feedback and configurable review question agents

[mchwang]

Changes

Reviewers can select changed lines and attach exact code references to questions or change requests. Ask invokes Claude Code or Codex, chosen in Settings, and displays a persisted answer with pending/error/retry states. Previously feedback was item-level text without an agent response.

Selection actions appear beside the highlighted code; Conversation has a draggable divider, immediate submission feedback, and follows incoming answers while preserving earlier reading positions. References retain the reviewed commits and show outdated evidence rather than silently pointing to different code.

The question adapters run read-only in temporary directories with bounded context/output and timeouts. Settings and answer attempts persist in SQLite. Request change remains saved feedback; this PR does not implement source edits or merge automation. The approved scope and behavior are documented in docs/implementation/read-only-review.md.

Validation

• Regression coverage for snippet validation, stale references and assignment context, persistence, provider selection, timeout/cancellation/retry, reloads during cancellation, single-click submission, answer scrolling, resizing, selection toolbar placement, and native selection clearing.
• Both installed providers passed live connection checks; a copy of the PR #597 review database passed a real Settings → Ask → Claude answer flow.
• Current head f2e5538 passes 188 unit/integration tests, all 32 browser tests, typecheck, and diff checks locally. Both current-head CI runs pass; the head change only adds an AGENTS.md workflow rule and passes git diff --check locally.
• Automated review converged after 16 rounds. Valid findings fixed stale poll overwrites, outdated retries and historical-answer labeling, active-job replacement, shutdown admission and draining, abort-reason loss, premature cancellation cleanup, in-flight draft loss, reload handling during provider settlement, and assignment-context drift. Every fix has a failing-before/passing-after regression. Round 16 returned no findings; all five inline threads are resolved. No concrete findings were declined. Polling efficiency is tracked in Avoid rebuilding the full review when polling agent answers #12; the final summary-only refresh-draft concern is tracked in Preserve composer edits during in-flight refresh #13 for reproduction.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved issues affect request cleanup, stale retries, polling consistency, and polling efficiency.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Prevent stale question poll responses from overwriting newer state · New

What changed in this PR

Adds code-linked review feedback and configurable, persisted read-only Claude Code/Codex answers.

Changes:

• Adds snippet selection, references, and stale-reference handling.
• Adds provider settings, asynchronous answers, retries, and persistence.
• Improves conversation UI, polling, documentation, and test coverage.

File	Description
web/​server.ts	Adds settings, question, and retry endpoints.
web/​public/​style.css	Styles selection tools and conversation UI.
web/​public/​index.html	Adds settings, selection, and conversation controls.
web/​public/​app.js	Implements references, answers, polling, and resizing.
web/​cli.ts	Updates CLI guidance.
tsconfig.json	Enables erasable syntax validation.
test/​review.test.ts	Tests reference validation and staleness.
test/​questions.test.ts	Tests answer persistence and lifecycle.
test/​browser/​review.spec.ts	Tests end-to-end workflows.
runner/​store.ts	Persists references, answers, and settings.
runner/​review.ts	Validates and snapshots references.
runner/​questions.ts	Manages asynchronous question attempts.
runner/​question-agent.ts	Provides bounded read-only CLI adapters.
README.md	Documents question-agent support.
docs/​implementation/​read-only-review.md	Documents implementation and validation.

---

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

[Copilot]

Copilot review overview

🔵 Needs a closer look

Two moderate issues remain around timeout reporting and retrying outdated questions.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Prevent stale question poll responses from overwriting newer state

Previously missed (1)

In code that hasn't changed since last review

Hide retry action for outdated review questions

web/​public/​app.js:563

When a pending/failed question becomes outdated after the review snapshot or plan revision changes, answerOutdated is true but this branch still renders a Retry answer button. Clicking it sends the old note to the retry endpoint, which Questions.start rejects for the older snapshot; act then calls showFailure and wipes the entire review view instead of letting the user continue. Hide/replace the retry action for outdated notes (with an instruction to ask a new question against the current review).

[mchwang]

Round 2 summary follow-up: reproduced the outdated Retry button with a failing browser regression. Unanswered questions from older snapshots now instruct the reviewer to ask again against current code; completed historical answers retain their outdated label. All 27 browser tests and typecheck pass. The timeout-reporting summary supplied no concrete failing case; deadline/lease tests already cover timeout errors, retry, and late-result rejection. Please identify any remaining timeout interleaving in the next review.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved agent lifecycle, timeout reporting, polling consistency, and review-load issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Prevent retry from overwriting an active job entry · New

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved moderate and critical findings remain, including an agent shutdown race.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Prevent new question jobs during shutdown · New

Resolved since last review (1)

• Prevent retry from overwriting an active job entry

[Copilot]

Copilot review overview

🔵 Needs a closer look

Preserve the actionable timeout abort reason instead of replacing it with generic cancellation text.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Prevent new question jobs during shutdown

[mchwang]

Round 5 summary reproduced directly against the CLI adapter: both timeout and shutdown abort reasons became generic Agent cancelled text. The adapter now preserves Error-valued abort reasons. Two failing-before/passing-after regressions cover this. All 185 unit/integration tests pass; the adapter tests, typecheck, and diff checks pass after the final test typing adjustment. No concrete findings declined.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Five unresolved moderate findings remain and should be addressed before approval.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Retry enabled while timed-out attempt is still running

web/​public/​app.js:571

When the persisted lease expires, this renders a Retry answer button even though Questions deliberately keeps the timed-out invocation in its running map until the provider settles. Clicking Retry during that interval is rejected by start() as “Agent is already answering,” so the UI offers an action that cannot succeed; expose the active-attempt state (or keep the retry disabled/hidden until it settles) and only enable retry once a new attempt can be admitted.

[mchwang]

The documentation review returned no inline findings but identified one concrete timeout-state race. Polling now exposes active local invocations; the UI shows Finishing cancellation and withholds Retry until settlement. The expired-crash path performs one status poll and then becomes retryable. Final validation: 187 unit/integration tests, 32 browser tests, typecheck, and diff checks pass. Requesting a final current-head review.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Preserve the active-job marker in the initial review response so finishing cancellations remain retryable after reload.

Review effort: Lite
Findings: None

[mchwang]

Fixed the current-head review concern in 39ab862. Initial review and action responses now include the active invocation marker, so reloading while a timed-out provider is still settling keeps Retry hidden and continues polling. The browser regression now reloads during settlement. Final validation: 187 unit/integration tests, 32 browser tests, typecheck, and diff checks pass. @copilot review

[Copilot]

Copilot review overview

🔵 Needs a closer look

Four unresolved moderate findings affect assignment staleness, shutdown cleanup, and retry failure reporting.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Mark item answers outdated when review assignments change

runner/​review.ts:55

The stale marker does not account for review assignments: saveReview increments review_version and can move a segment into this item without changing the snapshot or plan revision. An item-level question answered before that assignment can therefore be rendered as current even though its prompt omitted the newly assigned code (the reference-specific outdated check cannot catch notes without references). Capture an assignment/context fingerprint for the answer or compare the current item linkage with the context used to start it, and mark the historical answer outdated when it changes.

[mchwang]

Fixed the concrete current-head review concern in 8b5b48b. Answers now persist a hash of the item segments supplied to the agent; later assignment changes preserve the answer but mark it as earlier review context. The regression assigns code into an item without changing the plan revision or snapshot and now passes. Final validation: 188 unit/integration tests, 32 browser tests, typecheck, and diff checks pass. @copilot review

[Copilot]

Copilot review overview

🟡 Changes recommended

Address the stale retry guard and draft/selection race, and reconcile the documented test counts.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Reject retries when the answer context is outdated · New

[mchwang]

Resolved the current-head stale-retry finding in a89dd11 with a failing-before/passing-after regression. @copilot review

[Copilot]

Copilot review overview

🔵 Needs a closer look

The broad UI, persistence, and external-agent lifecycle changes, plus a validation-count documentation discrepancy, warrant human review.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Reject retries when the answer context is outdated

[mchwang]

Added the requested durable workflow rule in f2e5538: before closing or merging a PR, extract its highest-value broadly reusable review lessons into AGENTS.md while omitting duplicates and one-off implementation details. Documentation-only change; git diff --check passes. @copilot review

[Copilot]

Copilot review overview

🔵 Needs a closer look

A moderate unresolved issue may discard edits made during an in-flight refresh.

Review effort: Lite
Findings: None