Repository navigation
feat(coding-agent): make a feedback report investigable - #297
Conversation
A feedback report used to carry the user's sentence, a session id pointing at a file on their machine, and little else; the comment was the only thing a maintainer could read. Everything needed to reproduce a problem was already on disk but never reached the report. One submission now carries the evidence, with no per-item selection: picking what to attach produced reports that were just the user's own sentence. - `step.usage_provenance` records the dispatch facts a later config change would hide: API dialect, catalog context window and output limit, and, at settlement, the request duration, time to first token, wire outcome, HTTP status, who answered, and how far the model got. A request the user cancelled because it looked stuck is therefore reportable (`durationSeconds`, `ttftMs: null`, `reached: "thinking"`). - `session-summary.ts` derives the content-free summary from the session branch: model, effort, counts and failure classes, tokens, cost, cache hit rate, thinking time, per-compaction before/after sizes, plan/goal/task state, the last turn's tool names, and the newest request id. It walks `parentId` from the file's last entry, so abandoned turns and forks no longer distort the numbers; a pass no request followed yet is marked `unresolved` and left out of the size totals. - `evidence.ts` gathers the summary, the log excerpt and the archive from one session so they cannot describe different conversations, and keeps the ten-minute recency rule for a guessed session — which now also stays inside the current project's folder instead of picking the newest file anywhere. - The archive keeps the head and tail of an oversized transcript (with a marker between) and attaches the full tool output files the transcript referenced, resolved inside the session's own `tool-output` directory. - Redaction is one policy: credentials everywhere (scanned over the whole file, so an early secret also redacts its later echoes), the home directory collapsed to `~` in the transcript, tool output, dev log, `cwd` and provider error text, and absolute paths, e-mails and URLs removed from diagnostics. - Each archived transcript row keeps its entry and gains a `feedback` object with that row's own tokens, cache hit rate, cost, timing and request id. This is written to the bundle copy only; the session file is never rewritten, so the growth is +2.2% uncompressed and +1.2-1.8% gzipped, once, on submission. - The report header names the build it came from: `versionSource` (embedded, override, or the `0.1.0` fallback a source build reports), `buildTime`, `runtime` (node/electron/bun), arch, OS release, locale and install source. - `/feedback <text>` no longer bypasses the evidence path, and both hosts show the same consent page; with no terminal the submission prints what it attached. The archive is only built when a bundle endpoint can receive it. - `pending-store` re-validates a saved body strictly before retry, including the new summary, request and build fields and the nested records, and accepts a stored archive that carries `tool-output/` entries or a negative `reclaimed` — a field written but not allowlisted would have silently lost every offline report. Covered by new summary, row-metric and archive round-trip tests, plus updated CLI/TUI interaction tests. `docs/feedback-report.md` states what a report carries, its bounds, and the host integration contract.
Running the built CLI against a scratch session showed the home directory
surviving into the report two ways.
`$HOME` is often a symlinked prefix of the resolved path it appears in: with
`HOME=/var/...` and a resolved cwd of `/private/var/...`, replacing the first
occurrence produced `/private~/project` — a corrupted path that still names the
machine's directory layout. Matching now requires a boundary before the match
as well as after, and the resolved spelling of the home directory is collapsed
too, which is the spelling a tool prints after `realpath`.
A host that supplies no live facts also produced an empty `runtime: {}` in the
summary, which reads as "nothing to say" rather than "nothing to add"; the
field is omitted when empty.
Covered by the boundary cases in the summary tests and by a local end-to-end run
of the built CLI: body, archive, per-row metrics, offline retry and the
no-bundle-endpoint path.
Local end-to-end run (after the review rounds)Ran the built CLI ( Verified on the actual wire:
Two real bugs surfaced only at runtime and are fixed in
Re-ran afterwards: feedback + slash-command suites 249 pass, Not covered here: the desktop/web hosts — they live on |
TUI + real-model acceptance runBeyond the CLI end-to-end run, I drove the real terminal host (node-pty, 110×40) and the RPC host against a real model ( TUI
|
The idle-expiry case advanced 1ms of virtual time at a time, up to 100 times, while waiting for a lane's final notification. That notification is sent only after the child's details are read from disk, so the wait was really a race against file I/O: it passes on an idle laptop and fails on a loaded CI runner, where the assertion then failed on `inputs[1]` with a bare TypeError instead of saying what happened. Advance in 10ms steps for a bounded virtual budget, fail the wait explicitly when it runs out, and let the assertion name the reply text and the notifications that did arrive. Behaviour under test is unchanged.
`-C/--cwd` runs a session in one project while the process keeps its launch directory, and `prepareFeedbackEvidence` falls back to `process.cwd()` when a host names no project. A host that has the session (the terminal) therefore now passes the session's own `cwd`, so a report can only ever describe the conversation of the project it was filed from. Covered by the resolution cases in `feedback-cwd-project.test.ts` and by the terminal handler test: a session whose file does not exist yet contributes no summary rather than a neighbouring project's conversation.
The context-projection case spawns the built CLI and SIGKILLs it after 20 s. That guard exists to end a hang, but 20 s is also roughly a cold start on a loaded CI runner, so the child was killed while still booting and the failure read as a session with no requests rather than as a timeout. Raise the guard to 90 s; the assertions are unchanged.
Merged latest
|
| Scope | Result |
|---|---|
packages/coding-agent (full) |
4971 pass; 6 fail — all in completion-check-cli, which also fails on clean main (environmental git fixture) |
apps/cli (full, built) |
748 pass |
packages/agent-core / providers |
539 / 461 pass |
tsgo --noEmit, biome --error-on-warnings, 15 repo guards |
clean |
Two CI flakes fixed separately (neither is feedback code)
step-subagent-events: waited 100 × 1 ms of virtual time for a notification that is only sent after reading the child's details from disk — a race against I/O that a loaded runner loses (it failed asinputs[1]being undefined). Now advances in 10 ms steps for a bounded budget and fails with the notifications that did arrive.runtime-context-overrides: SIGKILLed the spawned CLI after 20 s, which is close to a cold start on CI, so the child was killed mid-boot. The guard now bounds a hang at 60 s and the case gets an explicit 120 s budget, so a genuine hang is reported with the child's output instead of a bare vitest timeout.
Neither change weakens an assertion.
Still open (not this PR)
Web/Desktop host (feat/stepcode-desktop-mvp) still assembles its own six-field payload and needs to move onto this pipeline after it merges main; server-side ingestion (requestId index, bundle unpacking, retention) needs backend scheduling.
Brings the seven commits main gained since dff413f: - feat(coding-agent): make a feedback report investigable (#297) - fix(coding-agent): settle extension command turns before returning (#298) - fix(coding-agent): keep the tool block stable across plan mode (#296) - fix(cli): save model selection as default on Enter (#285) - feat(cli): add -C/--cwd flag (#289) - fix(coding-agent): preserve task-state prompt cache prefixes (#293) Two conflicts, both in the plan extension, where this branch had added behaviour on top of files main rewrote: - `plan-mode-tools.ts`: main's comment for `exitPlanMode` ("restore any tools plan mode added") with this branch's optional `submittedPlanHash` parameter, which records the proposal handed to an external reviewer. - `step-plan.ts`: main's `version: 2` entry and `carriesLegacyTools` migration path, plus this branch's `submittedPlanHash` / `planHashAtEnter` on the same entry, and both sides of the restore step (legacy tool baseline and the persisted hashes). Main's `resolveToCwd` guard and `planModeAddedTools` bookkeeping are kept as-is: plan mode now appends tools instead of replacing the list, which is what keeps the request's tool block stable. The host plan-mode test asserted the old refusal wording; the guard now says "Only the plan file may be written", so it matches on that.
Why
A user feedback report used to carry their sentence, a session id pointing at a file on their machine, and a few build fields. Nothing in it let a maintainer reproduce the problem: no conversation, no tool calls, no error text, no model or context state. The agreed direction was to align with DeepSeek Harness — one submission carries the evidence, with no per-item selection, because asking users to assemble a report produced reports that were just their own sentence.
Base:
main(3de15c16). All changes are in the product layer (packages/coding-agent/src/step/feedback), with the CLI and TUI as thin hosts.What a report carries now
versionSource(embedded / override / the0.1.0fallback a source build reports),buildTime,commit,runtime(node/electron/bun), arch, OS release, locale, install source.step.usage_provenance): API dialect, catalogcontextWindow/maxTokensat dispatch, and at settlementdurationMs,ttftMs, wireoutcome, HTTP status, who answered, and how far the model got.cwdcollapsed to~.feedbackobject with that row's own tokens, cache hit rate, cost, timing and request id.Consent and bounds
One review page, one decision.
/feedback <text>now goes through the same path behind the same page; with no terminal the submission prints what it attached. The archive is only built when a bundle endpoint can receive it. A guessed session stays inside the current project's folder and respects the ten-minute recency window, and contributes neither transcript nor id when stale.Redaction is one policy: credentials everywhere (scanned over the whole file so an early secret also redacts its later echoes),
$HOME→~in the transcript, tool output, dev log,cwdand provider error text, plus absolute paths, e-mails and URLs in diagnostics.Correctness notes worth reviewing
parentIdfrom the last entry: session files are append-only trees, so counting every line reported rewound turns and a request id from an abandoned branch.pending-storere-validates a saved body strictly before retry. A field written but not allowlisted silently loses every offline report, so the new summary/request/build fields and the nested records are validated, and a stored archive carryingtool-output/entries or a negativereclaimedis accepted.Validation
packages/coding-agent: feedback suites + slash-command suites — 249 tests pass; full package 4935 pass with the 6 knowncompletion-check-clifailures that also fail on cleanmain(environmental git fixture).apps/cli: 746 tests pass (after building the workspace, which the suite requires).packages/providers: 461 pass.tsgo --noEmit,biome check --error-on-warnings, and the repository guards (ts-imports,layer-direction,no-secret-leak,public-boundary,no-observability,coding-agent-entry-freeze,pinned-deps) pass.Docs
New
docs/feedback-report.md(what a report carries, bounds, redaction, consent, retry, host contract), referenced fromdocs/AGENTS.md.