Repository navigation
feat: keep rejected approaches visible across backend handoffs - #214
Conversation
QualityMax ReviewVerdict: COMMENT · Confidence: model judgment; findings have not been reproduced Re-review: Open the QualityMax check and select Re-review after it completes. Files eligible: 18 · Files reviewed: 18 · Files with findings: 1 · Findings: 2 · Inline cards: 0 Priority findings
Review gates
Important files
Advisory MODEL-ONLY candidates (2)These model judgments do not block this PR and are not posted inline. Check reachable callers and existing protections, and reproduce a concrete failure before treating a candidate as a defect. A second model's confirmation or a model-reviewed patch does not reproduce a failure.
Change diagram — Flowflowchart TD
subgraph "handoff-eval"
A[main] --> B{Command}
B -->|selfcheck| C[selfCheck]
B -->|run| D[runCommand]
B -->|summarize| E[summarizeCommand]
C --> F[newRepo]
C --> G[goTest]
C --> H[setTimeout]
C --> I[scoreRepo]
D --> J[plan]
D --> K[runOne]
K --> L[fakeBackend]
L --> M[Run]
end
Review lifecycleUse the inline cards to inspect evidence and suggested remediation. Re-run the QualityMax review after pushing a fix; unchanged cards are identified by their stable finding marker. Dismiss with a reason through the existing QualityMax/GitHub review feedback flow. 0 prior card(s) are stale/resolved on this head. Proof legend: GROUNDED deterministic scanner evidence · MODEL-ONLY advisory model judgment · MODEL-REVIEWED patch model judgment, not reproduction. QualityMax project results are available in the configured project. Receipt · commit |
|
| Gate | Result |
|---|---|
| 🔍 AI diff review | eval/handoff/run.go performs manual resource management for global state (stdout redirection and working directory) without using defer. If an error or panic occurs during execution, the process may be left with a corrupted stdout stream and an incorrect working directory, leading to resource leaks and potential failures in subsequent operations. The PR introduces a robust handoff mechanism for tracking rejected approaches, including proper state management, input validation, and redaction of sensitive data. The implementation correctly handles persistence, history compaction, and backend switching, with comprehensive test coverage for both the agent logic and the REPL command interface. The PR introduces a new /handoff command and associated logic to track backend switches and manage rejected approaches. The implementation correctly integrates with the existing REPL loop, command handling, and input redaction mechanisms, with appropriate updates to help text and test coverage. |
| 🔍 SAST | completed · 30 eligible / 30 reviewed · claude-haiku-4-5-20251001 |
| 🔍 Canonical PR review delivery | completed · 0 eligible / 0 reviewed · exact-head review #5557394607 and overview #6002650534 confirmed |
| 🧪 Repo Tests | ✅ 1506/1506 passed (go) |
| 🤖 AI Tests |
Powered by QualityMax — AI-Powered Test Automation
- Record the checkpoint digest each native CLI session received; resend only when the session is new, the checkpoint changed, or entries are being transferred, so it no longer accumulates in CLI history per turn. - Print a /handoff reject reminder after a backend switch that follows unrecorded work, noting interrupted turns and failed tool calls where the backend exposes them (CC is_error, Codex exit codes, OpenCode/agy errors, built-in JSON errors). It never records a rejection itself. - Accept any whitespace after /handoff and its subcommand; reject look-alikes like /handoffs instead of sending them to the model. - Support \| for a literal pipe in the approach and echo the parsed approach, warning when the evidence contains a pipe.
Dropping the oldest entries re-encoded the whole history once per dropped entry. Search for the fewest drops that fit instead: ~800 untransferred entries took ~13s before the backend started and now take under 2s. The test checks the result matches the previous linear search.
eval/handoff runs the live experiment from docs/HANDOFF.md: a fixed Go fixture where raising ReconnectTimeout is the tempting but wrong fix, a scripted phase A with real captured test output, checkpoint vs. control arms that differ only by the /handoff note, optional padding that pushes phase A out of the inline handoff, and one backend-B turn through RunCLI. Scoring samples the timeout constant during the turn (catching reverted attempts), runs hidden tests with the original test restored, and writes per-run artifacts plus summary.md. 'run' prints the plan and only calls backends with --yes; 'selfcheck' verifies the fixture without backends.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
|
|
Long or compacted conversations can bury a rejected approach even when the shared transcript retains its evidence. Add
/handoff reject <approach> | <evidence>to keep an explicit user decision visible to the next backend. For example, a note that increasing the reconnect timeout failed at both 5s and 60s accompanies the next turn after switching providers.The checkpoint is included in every CLI handoff, including native resumes with no missing transcript entries, and in built-in provider system prompts. It survives compaction and session save/resume.
/handofflists entries;/handoff forget <number>removes a rejection, and/clearresets the checkpoint. Removed notes remain superseded by the current list.Security: notes use existing retained-transcript redaction, are capped at 12 KiB, and are identified as user-supplied data. Input recall keeps only
/handoff, avoiding retention of raw pasted evidence. Notes make no additional model request and do not inspect credentials or provider-private state.This first version requires explicit user decisions. Failures are not automatically classified, and model compliance is not enforced.
docs/HANDOFF.mddescribes a controlled live backend comparison; that evaluation has not yet been run.Validation:
go test -coverprofile=<local-file> -covermode=atomic ./...go run ./eval/handoff selfcheck(baseline and timeout-only workaround fail; reference fix passes visible and hidden checks).go vet ./...go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 run-ldflags='-s -w -X main.Version=dev'(artifact under.context).Refreshed against main and fixed module imports. The controlled live backend comparison remains pending; this PR stays draft.
QualityMax follow-up: close the evaluation output reader when mirroring fails, so a closed stdout pipe cannot block a long backend turn. Defer restoration of stdout and the working directory and cleanup of the timeout watcher/timer if a backend panics. Regression tests cover both failures and pass with the race detector. The harness executes runs serially; its stdout capture is limited to the standalone evaluation process. Diff artifacts contain the generated fixture repository, not a user's working repository.
Remaining advisory candidates were checked: future parallel execution is outside this serial harness, and
--outdeliberately selects the user's artifact directory. Run paths beneath it use validated backend/arm enums and nonnegative integer padding/repeat values; the live plan requires--yes. No untrusted path segment was found in the reported deletion path.