feat(*): add end-to-end trajectory debugging and regression workflows - #362
Conversation
Give every span a stable trajectory address: attempt.id defaults to the turn's trace id (a single-turn attempt), and an explicit begin_attempt / end_attempt scope groups multi-turn work under one id, so a task spanning several turns is addressable as one trajectory. Add the raven/trajectory package on top of tracing: - verdict.py: append-only verdicts.jsonl labeling attempts as pass, fail, or infra (environment crash, excluded from diagnosis), written outside tracing because status.code says whether code crashed, not whether the task succeeded. - store.py: pins.json registry marking attempts as corpus that purge tooling must never delete, plus iter_spans, a rotation-transparent reader over archived and active span logs with attempt/trace/session filters. Define Attempt, Trajectory Verdict, and Trajectory Pin in CONTEXT.md. This is stage 1 of the trajectory system (later stages: bundle save, scrubbed bug report, deterministic replay, regression tests). Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Package one attempt into a self-contained offline bundle directory: manifest, spans (artifact references rewritten to bundle-relative paths), copied artifacts, the session conversation record, and verdicts. Bundling declares the trajectory corpus, so the id is auto-pinned with reason "bundled". - raven/trajectory/bundle.py: collect_bundle resolves a turn's trace id to its canonical attempt id so the whole multi-turn attempt is packed, refuses ids that would escape the output root as directory names, builds in a staging dir and swaps it in whole so re-packs never keep stale artifacts, and resolves the session file from the configured agent workspace (falling back to the stock default). - raven/cli/trajectory_commands.py: new "raven trajectory" group with save / verdict / pin / unpin / list; list folds long attempt ids instead of truncating so they stay copy-pastable. - CONTEXT.md: define the Trajectory Bundle domain term. - tests: bundle collector unit tests plus an end-to-end run with the real tracer, CLI command tests, and the new top-level command added to the pinned command set in the smoke test. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The save command already maps a turn's trace id to its canonical attempt id, but verdict/pin/unpin wrote the literal id: a verdict recorded by trace id never showed up in the bundle (the readers are attempt-keyed), a pin protected only that one turn, and unpin could not release a pin saved under the canonical id. - raven/trajectory/store.py: new resolve_attempt_id shared resolver; collect_bundle now uses it too. - raven/cli/trajectory_commands.py: verdict and pin resolve first and refuse ids with no matching spans; unpin resolves best-effort and also drops the literal id so pins written before resolution still clear; save gains --workspace/-w for sessions created under a runtime-overridden workspace. - tests: resolver unit tests, CLI coverage for the resolution paths and the --workspace option. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Phase 3 of the trajectory system: redact a copy of a bundle and pack it into a shareable tarball, so a bug trajectory can be handed over without leaking credentials. - raven/trajectory/redact.py: three-layer redaction producing a sanitized copy; the original bundle is never modified. Layer 1 replaces known secret values collected from the config (validated model walk plus raw JSON walk so loader-stripped extension blocks are covered; both the named and the default config file) and from credential-shaped environment variables, including JSON-escaped spellings, with stable source-named placeholders. Layer 2 is a regex fallback for common credential shapes (sk-, AKIA, ghp_, xoxb-, Bearer, JWT, PEM blocks). Layer 3 scans the redacted copy for suspicious leftovers (charset-aware entropy thresholds; hex and letter-only tokens included) and reports them for human review without rewriting. Replacement is a single regex pass so a short value can never corrupt another secret's placeholder. Config secrets have no length floor; the EMPTY dummy is exempt only on api-key fields. Non-UTF-8 files are excluded from the copy and recorded. Per-layer statistics ship as redaction.json inside the copy. - raven/trajectory/report.py: Uploader protocol with a v1 local tarball backend (prints the path, uploads nothing) plus pack_report; an HTTP backend can slot in later without touching callers. - CLI: raven trajectory report <id> [--out FILE] [--yes] [--workspace DIR] [--config FILE]. Re-packs the bundle, redacts a staging copy, previews residual suspects, asks for confirmation (skipped by --yes; declining produces no tarball), then packs. --config seeds redaction with the traced agent's config on top of the default one and names the session-lookup workspace when --workspace is not given. - CONTEXT.md: define Trajectory Redaction and Trajectory Report. Tests: tests/test_trajectory_redact.py covers the three layers, the untouched original, escaped spellings, placeholder stability, binary policy, secret collection (extension blocks, both configs, unreadable config, api-key-scoped EMPTY exemption), residual scanning, packing, and an end-to-end run with the real tracer verifying fake keys never reach the tarball. Report CLI cases are merged into tests/test_cli_trajectory_commands.py. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Replay a saved trajectory bundle through the live harness: a ReplayProvider feeds recorded llm.output payloads back in recording order (chat and chat_stream, re-chunked deltas), and a ReplayToolRegistry answers execute() from recorded tool results so no real tool code ever runs. Divergence between the live request and the recorded llm.input (model, stream mode, messages, tool-call names and arguments, offered tool names) is detected per call under narrow normalization (runtime clock line, untrusted-fence nonces, prompt-cache breakpoints via prompt_cache.strip) with strict/warn policies; running out of recording or leaving it unconsumed is a divergence too, and strict keeps its first-divergence contract. run_replay drives an AgentLoop per recorded turn in a temporary workspace, reseeds pre-attempt session history from the bundle's session.jsonl (cut located by the manifest time window and confirmed by the first turn's input; ambiguity seeds nothing rather than preloading attempt messages), takes the streaming path for turns recorded with llm.stream, and suppresses tracing task-locally (new tracing suppress() context variable) so a replay emits no spans while concurrent real turns keep tracing. New command: raven trajectory replay <bundle-or-id> [--strict|--warn]. Exit codes: 0 complete replay, 1 bad target, 2 halted. Groundwork fix: the streaming aggregator no longer fabricates "stop" or "tool_calls" when the upstream never sent a finish_reason; it reports "unknown", so recorded trajectories can tell a cut-off stream from a completed one. No production consumer branches on the synthesized values (all compare against "error" only; truncation flagging reads the raw upstream value before the fallback). Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Turn fixed harness bugs into permanent CI regressions, golden-data style: replay a recorded trajectory through the live harness and assert that the divergence direction is the fixed behavior. - cassette minimizer (raven/trajectory/cassette.py): shrink a bundle to the exact surface load_recording consumes (consumed spans, artifacts, and payload fields only; system prompt content replaced by a placeholder; session sliced to the pre-attempt history with a verified reseed round trip), then redact the result. Payloads are never truncated: a field is kept whole or dropped whole. New CLI: raven trajectory minimize <bundle-or-id> [--out] [--config]. - boundary hardening: the destination swap only ever replaces an empty directory or a previous cassette (never an arbitrary directory), the destination and the source bundle may not contain each other, artifact references are traversal-checked before any read (dot-dot segments, paths outside artifacts/, and symlinks escaping the bundle are refused), and a crafted manifest attempt id cannot name a default output outside the cassettes directory. - assertion DSL (raven/trajectory/regression.py): expect.yaml declares where the replay's first divergence must land and what the live side must do there (message contains/not_contains/equals, tool name and params checks). The replay layer now records structured expected and actual values on each Divergence and captures every live request on the report (llm_requests/tool_requests) for programmatic assertions. - pytest integration: tests/trajectories/<case>/ = cassette/ plus expect.yaml, auto-discovered by tests/test_trajectory_regressions.py; two sample cases committed (faithful reproduction and divergence direction), recorded with a scripted provider and minimized. - CONTEXT.md: Trajectory Cassette and Trajectory Regression Case terms. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this revision can read files outside a replay bundle and has three trajectory fidelity/retention failures; see the inline notes.
I covered the repository rules and canonical trajectory terms, the full diff, callers and tracing/session/provider contracts, commit history and backward compatibility, and whether tests were weakened. In particular, the traversal test verifies that the outside file is unchanged, but not that it was never read.
Verification: uv run pytest tests/test_agent_loop_stream.py tests/test_cli_smoke.py tests/test_cli_trajectory_commands.py tests/test_tracing_api.py tests/test_trajectory_bundle.py tests/test_trajectory_cassette.py tests/test_trajectory_redact.py tests/test_trajectory_regressions.py tests/test_trajectory_replay.py tests/test_trajectory_store.py passed (223 tests); make check-large-files passed; git diff --check github/main...HEAD passed.
| path = Path(ref) | ||
| if path.is_absolute(): | ||
| return None | ||
| resolved = bundle_dir / path |
There was a problem hiding this comment.
_load_artifact rejects absolute paths but accepts traversal and symlink escapes. A bundle can set llm.output.artifact_path to ../../secret.json; load_recording() reads that file before the cassette validator runs, while trajectory replay never calls the validator at all. I reproduced this with a temporary bundle and got {'content': 'LOCAL-ONLY-SECRET', 'finish_reason': 'stop'} from a JSON file outside the bundle. This makes replaying a shared bundle an arbitrary local JSON read. Please resolve the reference and require it to remain under the bundle's artifacts/ directory before is_file() or read_text().
There was a problem hiding this comment.
Fixed in 0ac8336. Artifact references now go through one resolver shared by replay and cassette. It rejects traversal, paths outside artifacts/, and symlink escapes before is_file() or read_text() can access the target; absolute dangling source references retain the existing missing-artifact behavior. The new load_recording regression tests replace Path.read_text with a guard that fails if the outside file is ever read, covering both traversal and symlink cases.
There was a problem hiding this comment.
Verified on 0ac8336. The shared resolver rejects traversal and symlink escapes before artifact loading reaches is_file or read_text. My original ../../secret.json reproduction now raises ValueError, and the guarded-read tests cover both escape forms. This resolves my finding.
| # inside placeholders inserted for earlier, longer secrets. | ||
| variants: dict[str, str] = {} | ||
| for secret in secrets: | ||
| for variant in _variants(secret.value): |
There was a problem hiding this comment.
Exact credential names intentionally have no length floor, and this loop adds their raw value to a global substring replacement. With DEEPSEEK_API_KEY=k (a value the new end-to-end test itself installs), redacting the checked-in reproduction cassette made 108 replacements: session.key became session.[REDACTED:env.DEEPSEEK_API_KEY]ey and marker became mar[REDACTED:env.DEEPSEEK_API_KEY]er. Strict replay then diverged at call 2 and the regression's name_equals: marker check failed. Thus a local short credential silently changes the semantics of generated cassettes instead of merely sanitizing them. Please prevent low-specificity exact values from being applied as raw substrings, or otherwise preserve the parsed payload semantics.
There was a problem hiding this comment.
Fixed in 0ac8336. Exact values shorter than six characters now use token-boundary matching instead of global substring replacement. A standalone JSON value such as k is still redacted, while identifiers and values such as session.key, marker, and ok remain unchanged. The regression test asserts both the preserved payload semantics and the standalone secret replacement.
There was a problem hiding this comment.
Verified on 0ac8336. Re-running the original one-character-secret reproduction leaves session.key and marker intact; strict replay completes and the checked-in regression has no failures. The new test also confirms that a standalone JSON value of k is still redacted. This resolves my finding.
| ) | ||
| for j, tc in enumerate(output.get("tool_calls") or []) | ||
| ] | ||
| return LLMResponse( |
There was a problem hiding this comment.
The replayed LLMResponse drops thinking_blocks (and the existing trace output payload does not record it), even though AgentLoop uses that field to choose its empty-response recovery branch and persists it into subsequent assistant messages. A direct check with an empty response carrying Anthropic-style thinking_blocks chose RecoveryAction.PREFILL live, while the response reconstructed here chose RecoveryAction.RETRY. Real extended-thinking trajectories therefore take a different harness path during replay and can produce a false divergence before reaching the bug under test. Please carry this field through trace output, cassette minimization, reconstruction, and request comparison.
There was a problem hiding this comment.
Fixed in 0ac8336. thinking_blocks is now recorded in the trace output artifact, retained by cassette minimization, reconstructed on ReplayProvider responses, and included alongside reasoning_content in canonical assistant-message comparison. Regression coverage verifies the trace artifact, cassette round trip, reconstructed response, and request mismatch behavior.
There was a problem hiding this comment.
Verified on 0ac8336. thinking_blocks is present in the trace payload, retained by minimization, reconstructed on the response, and compared in assistant messages. In the original direct reproduction, both the live and reconstructed thinking-only response now choose RecoveryAction.PREFILL. This resolves my finding.
| if not id_: | ||
| raise ValueError("id is required") | ||
| current = pins(state_dir) | ||
| current[id_] = {"reason": reason, "ts": datetime.now(timezone.utc).isoformat()} |
There was a problem hiding this comment.
The registry read happens before atomic_replace acquires its lock, so the lock serializes only the final replacements, not this read-modify-write transaction. I started 24 processes on a barrier, each pinning a distinct id; all exited successfully but only 2 ids remained in pins.json. unpin has the same lost-update shape. Since a pin is the promised never-purge retention state, concurrent CLI/report activity can silently discard it. Please hold one cross-process lock across the read, mutation, and replace.
There was a problem hiding this comment.
Fixed in 0ac8336. The new atomic_update primitive holds the existing cross-process sidecar lock across the complete read, mutation, and atomic replacement. Both pin and unpin now use that transaction instead of reading before atomic_replace acquires the lock. Spawned multiprocessing barrier tests verify that concurrent distinct pin and unpin operations do not lose updates.
There was a problem hiding this comment.
Verified on 0ac8336. The new update callback runs under the same sidecar lock for the read and replacement. Re-running my 24-process barrier reproduction retained all 24 distinct pins, and the concurrent unpin test also passes. This resolves my finding.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
I verified all four fixes against their original reproductions and found no new issue in the 34c47f4..0ac8336 delta. This pass covered the repository rules and canonical trajectory contracts, the fix diff, affected callers and shared atomic-I/O consumers, commit history and backward compatibility, and whether the new tests weakened the original failure conditions.
Verification: the expanded affected suite passed (318 tests); make check-large-files passed; both diff checks passed; the 24-process pin reproduction retained all 24 ids; current GitHub checks pass or skip.
#354) ## Summary `ask_user` advertised things it did not do, and a batch cost the user more than one wait. This closes both, and gives the prompt the fields it needs to render a batch honestly. - **Two schema fields nothing read.** `multiple` and `custom` were declared per question and never passed to the broker, so a model asking for a multi-select got a single-select and never learned why. Both are removed. Multi-select is deliberately not implemented here: it would need a new answer syntax on the chat-channel surface, which has no dialog to hold a multi-select, and that is a larger decision than removing a field that never worked. - **One deadline per call, not per question.** A batch was N round-trips with N independent timeouts, so a three-question call could hold a lane for three times the surface's wait. The call now shares one budget, and a spent budget stops the batch instead of opening a fresh wait on every question left. - **The budget is configuration.** `tools.ask_user.timeout` (600s, must be positive) reaches the tool through `AgentLoop`, which is where the config already is. It deliberately does not read config at the transport: the TUI builds its broker before any config is loaded, and reading it there put a schema-validation failure in the path of the RPC server coming up. - **Calls that would waste the user's time are rejected before rendering**, with a message that steers the retry: more than four questions, a duplicate question text, or a question with exactly one option, which is not a decision. A duplicate option label is a typo with one obvious reading, so it is deduped rather than rejected. Zero options remains a free-form question. - **`header` and `recommended`.** A short chip label, and a 0-based index into `options` naming the option the agent would pick. When nobody answers, the text the model reads now names that option; before, the only signal was that no answer arrived. - **A recommendation is resolved against the options as submitted.** The index counts what the caller sent, but the list was deduplicated before the index was resolved against it. With `["a","a","b","c"]` and `recommended: 2` the caller means `"b"` while the broker was handed `"c"`, so the surface marked the wrong row and a timeout told the model the wrong intended fallback; an index past the deduped length (`3` on that list) failed the range check and the recommendation was dropped without a word. The label is now resolved before dedup narrows the list. Dedup keeps every distinct label, so the resolved label is always still one of the choices the surface can mark. - **An undeliverable question is reported, not dropped.** A question for a conversation with no live source was dropped with a log line the broker could not see, so the round-trip waited out its whole budget on a question nobody would ever see. The channel adapter now raises `QuestionUndeliverableError` and the broker fails safe at once. - **The prompt renders the batch.** Position in the call, the questions still to come, the recommended option marked, and the remaining budget counting down. Tab attaches a note to a selection, so an answer no longer has to be either a choice or free text. On a chat channel the position rides in the message text. - **The countdown renders whole seconds.** `timeout_s` is a remaining-time subtraction, so a 90-second budget arrives as `89.99999912502244` and the prompt decrements that same fractional value once a second. The formatter passed it straight through, so the line read `1m 29.99999912502244s` on every tick and widened the prompt that the padding exists to keep steady. The value is now ceiled before the branch is chosen, since ceiling inside the branches renders a ceiled `59.7` as `60s` rather than `1m 00s`. - **The `Other` row answers to its own number.** This one predates the rest of the branch. The options list draws `Other` as one more numbered row, but the quick-pick handler bounded itself at the number of real choices, so the last row was numbered and unreachable -- and worse than inert: the keystrokes that followed were swallowed by the options handler, so the next Enter submitted whichever option was still highlighted. Someone who thought they were typing a free-form answer sent a selection they never made. Its number now opens the text input, which is what Enter on that row already did, and the hint names the range it can take. The wire payload gains `header`, `recommended`, `timeout_s`, `index`, `total` and `batch`. `clarify.request` is a notification and is not part of `rpc-schema/openrpc.json`, so no generated artifact changes; `npm run lint:rpc` confirms `generated.ts` is still in sync. `deep_research` shares the same broker and passes none of the new fields, so its prompt is unchanged except that it now shows the countdown, which was always the real deadline. `timeout_s` stays fractional on the wire deliberately: it is also what the model is told about the budget, and rounding it at the source would change that value to serve a display concern. The formatter is the layer with a width to protect. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other Mostly fixes; the prompt affordances and the config field are additive. ## Verification Every new test was watched failing before the code that makes it pass. Rebased onto `22b71765`, which collapsed the earlier `Merge branch 'main'` commit into a linear six-commit history. The base gained #361 and #362; the only file this branch and those commits both touch is `raven/agent/loop/main.py`, and the replay was clean. `git merge-tree --write-tree` needs git 2.38 and this box has 2.34.1, so the pre-push check was that file-overlap table plus a full re-run at the new head rather than a dry-run merge. Everything below is from the new head, not carried forward. ``` python -m pytest tests/ -q 82 failed, 6576 passed, 47 skipped, 13 deselected # same suite on the new base 22b7176, in a separate worktree: 82 failed, 6545 passed, 47 skipped, 13 deselected # full failing-ID lists from both runs, sorted and diffed: diff branch_fail.txt base_fail.txt -> identical (0 introduced, 0 fixed) ``` The 82 failures are inherited: the base fails the same 82 IDs on this box, and the sets are identical rather than merely the same size. They are `test_tui_rpc_session.py` (58), `test_cli_cron_commands.py` (7), `test_cli_import_commands.py` (5), `test_tui_commands_error_codes.py` (3), `test_cli_onboard_commands.py` (3), and 6 more spread across 5 files. The +31 passing tests are the ones added here. Suites that exercise the changed code directly: ``` python -m pytest tests/test_ask_user_tool.py tests/test_question_broker.py \ tests/test_cli_gateway_commands.py tests/test_config_schema.py -q 81 passed npm run test -- --run src/__tests__/clarifyPrompt.test.tsx 11 passed npm run type-check -> clean npm run lint:rpc -> OK: generated.ts in sync python -m ruff check raven tests -> All checks passed! python -m ruff format --check raven tests -> 849 files already formatted python -m scripts.check_commit_messages github/main..HEAD -> exit 0 scripts/check_large_files.py -> exit 0 ``` Beyond the suites, the tool was exercised over the real path -- a real `ToolRegistry` (so `cast_params` and the schema validator run), a real `QuestionBroker`, and answers delivered through the real `clarify.respond` handler on a real `Dispatcher`: - the three rejections emit **zero** prompts, so a malformed call costs the user nothing, and the steer text reaches the model intact through the registry; - a batch answered by a responder taking 0.25s per question is handed `10.0000s`, `9.7495s`, `9.4991s` -- each question inherits what the last one left; - a batch whose budget is spent emits **one** prompt, not one per question; - an undeliverable question returns in `0.001s` against a 30s budget. `test_registry_dispatch_and_the_real_clarify_respond_route` was added from that exercise, because no committed test drove the registry before. It was proved non-vacuous by renaming the `recommended` keyword the tool passes the broker and watching it go red. The two review fixes were each proved the same way: - The recommendation cases were run against the deduped resolution and failed with `assert 'c' == 'b'` (wrong label) and `assert '' == 'c'` (recommendation dropped), then passed once the resolution moved ahead of dedup. `test_out_of_range_recommended_index_is_ignored` passes unchanged -- it has no duplicates, so it never encoded this. - The countdown cases were run against the pass-through formatter and failed on `expected ... to match /1m 30s/` and `/1m 00s/`, covering both the fractional payload and the `59.7` boundary that ceiling creates. `prompts.tsx` and `clarifyPrompt.test.tsx` are reported by Prettier, and were before this branch touched them: the hunks it wants are at lines this branch does not change (a JSX ternary and a `renderSync` call), so they are left alone rather than reformatted into this diff. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed `docs/Proactivity-Implementation.md` documented the two removed fields and the old narrow payload, so that paragraph is rewritten. `ui-tui/dist/entry.js` is gitignored, so testing the prompt by hand needs `npm run build --prefix ui-tui` first. ## Risk User-visible changes: - A model sending five questions, a duplicate question, or a one-option question now gets an error instead of prompting the user. The error text tells it what to do instead. - A batch that used to get a fresh timeout per question now shares one. A slow user answering question 3 of 4 past the budget gets the rest recorded as unanswered rather than being asked. - The prompt shows a countdown where it showed none, including for `deep_research`, and that countdown now reads in whole seconds. - A recommendation sent as an index past the deduped option count used to be dropped silently and now resolves. That is what the schema promises, but it is a real change for a caller that had been sending such an index. - Typing the `Other` row's number now opens the text input instead of doing nothing. Rollback is the six commits; nothing is persisted and no format changes on disk. The config field defaults to the previous 600s, so an untouched config behaves as before. Checked and deliberately not fixed, to keep this diff to one subject: - `QuestionBroker.await_question` still returns its default on `asyncio.CancelledError`, which swallows an outer cancellation. It sits beside the handler added here, but changing it means auditing how the agent loop reacts to a cancelled turn. - The gateway's inbound gate calls `pending_req(cid)` and then `reply(...)`. If the question times out between the two, the message answers nothing and starts no turn. `reply` already returns a bool, so using it as the condition would close this. - The registry's schema validator does not implement `maxItems`, so the cap in the schema is advisory to the model and `_prepare` is what enforces it. That is deliberate -- the model gets the steer text rather than a generic validation error -- and a test pins the two statements of the cap together so they cannot drift. - `ApprovalPrompt` in the same file hardcodes `1-2 quick pick`, which is correct for its two fixed options and has no `Other` row, so it is not the same bug. - `approvalRemainingSeconds()` feeds the approval countdown in the same file and is already `Math.max(0, Math.ceil(...))`, so it needs nothing here. It is also where the `ceil` convention comes from: the clarify countdown now rounds the same direction as the approval one rather than inventing a second rule. The two are still not made to share a formatter -- one takes an absolute deadline and the other a remaining duration. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes No new secret, no new surface, and no asset moved onto an unauthenticated one. The new config field is a timeout. ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…lipboard path lying (#360) ## Summary A TUI that enables mouse tracking owns the drag, so the terminal never builds a native selection and its own copy shortcut has nothing to copy. Copy-on-select is what makes a transcript selection copyable at all -- but the subscription bailed out on `!isMac`, so on Linux and Windows a drag highlighted text and copied nothing. This lifts that gate, and fixes what was found while making the result observable. **Copy-on-select on every platform.** The subscription moves out of `useMainApp` into `subscribeCopyOnSelect()`. Inlined in a hook that needs a live gateway to start, none of its four guards could be tested; each now has a case, and each was checked by removing only that guard and watching the suite go red. The read of the bus state stops being an unchecked cast: the ambient `useSelection()` declaration types it as `unknown`, so the module narrows it instead. **The copy is reported to the transcript.** Nothing on screen changes when a drag ends, so a line is written once a clipboard path has actually taken the text. The callback fires on a non-empty result only, since `copySelectionNoClear()` resolves to an empty text when nothing reached the clipboard. The first report of a session carries the path caveat, later ones stay terse: OSC 52 is the one path a terminal can still refuse, and the first copy is when a user is looking for the reason a paste came up empty. **The reported path comes from the write, not from the environment.** A path predicted from env state cannot be right in general, because the environment does not record what happened: inside tmux a `load-buffer` that fails falls through to raw OSC 52 and leaves `TMUX` set exactly as the case that worked. With a stale tmux socket the bytes went out as OSC 52 while both callers said "copied to the tmux buffer", pointing the user at a `set-clipboard` setting that was never involved. `setClipboard()` already computed the three facts that decide this -- whether native was attempted, whether the buffer loaded, whether a sequence was emitted -- so it now reports which one took the text, and null when none did. The value travels with the copied text out of `copySelectionNoClear()` and `copySelection()` to the two call sites that display it. The predictor is deleted rather than left exported from the package: after the rewiring it had no callers, and a helper that can name a path the data did not take is a defect waiting for the next caller. Its env-matrix cases were assertions about the predictor itself, so what replaces them drives the real `setClipboard()` with `tmux` stubbed, including the failed-load fallback. Reporting an observed path also has to wait for the observation. On Linux the native tool is discovered by a probe, and `copyNative()` answered the first call before that probe settled -- a display server means a tool could exist, not that one does. Naming the path from that optimism claimed a native write on a machine with `DISPLAY` and no `wl-copy`, `xclip` or `xsel`, and the same call reported failure once the probe finished. The first call now answers with the probe's own result. It is still started before the tmux await, so the probe runs alongside `load-buffer` rather than ahead of the report, and later calls still answer synchronously from the cache. The copy never ran ahead of that probe either -- the tool is spawned inside it -- so only the report did. **The path caveat is scoped to a session, not to the process.** The first copy of a session carries the caveat and later ones stay terse, but the flag holding that lived in a hook that outlives the session: `newSession()` and `resumeById()` replace `ui.sid` without remounting `useMainApp`, so every session after the first opened with the terse line and the user never learned an OSC 52 paste can come up empty. The tally moves into a reporter keyed on a session identifier, so the state and the thing it is scoped to live together and the boundary is unit-testable. The hook reads the sid through `getUiState()`, which keeps a session change from tearing down the bus subscription. **The report must not overclaim.** OSC 52 hands bytes to the terminal and the terminal decides whether to keep them, and nothing in the write path caps or chunks the payload. Measured on the real code: one 200x50 viewport of CJK is a 40 KB escape sequence, and a selection dragged through 2000 rows is 536 KB in a single sequence. Terminals drop an oversized sequence without a word, and `setClipboard()` reports success for all of it because bytes were written to stdout. So that path reports what it sent; native and tmux, which really did write a clipboard, still say copied. The count was UTF-16 code units, which reads three emoji as six characters and a combining accent as two, so it is counted by grapheme now. CJK was already right, being one code unit per character. **The documented env knobs were dead.** The OSC 52 override and the clipboard debug switch were readable only under the upstream `HERMES_TUI_` names, while every knob this project documents uses `RAVEN_TUI_` and `/copy`'s own failure hint names `RAVEN_TUI_FORCE_OSC52` and `RAVEN_TUI_DEBUG_CLIPBOARD`. Following that hint changed nothing. Both spellings are now read, `RAVEN_TUI_` first, with the `HERMES_TUI_` names kept as aliases so an environment that worked before still works. The debug switch moves behind `clipboardDebugEnabled()` so its four call sites cannot drift apart. ## Type - [x] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other Two boxes: the platform gate is the feature, the clipboard-path and env-name defects are fixes found while making it observable. ## Verification Run from `ui-tui/`, on Linux, node 22, at branch head after the rebase. ``` npx vitest run --no-file-parallelism Test Files 91 passed (91) Tests 1066 passed | 13 skipped (1079) npx tsc --noEmit -p tsconfig.json clean, exit 0 npm run lint 22 problems (0 errors, 22 warnings) python3 -m scripts.check_commit_messages github/main..HEAD exit 0 ``` The suite is run with `--no-file-parallelism` deliberately: the ink render tests fail under default worker parallelism at this suite size, and no CI job runs them. The 22 lint warnings are the baseline, not new: 22 before the branch and 22 after, none of them in a file this branch touches. Every file this branch touches passes `prettier --check`. Two of them did not at first, and that was this branch's doing rather than the base's: dropping a name from the `@hermes/ink` import and adding a wide function signature left both inside the print width while still wrapped. They are formatted in their own commit. The files already unformatted on `github/main`, `chatStream.ts` among them, are left alone rather than reformatted into this diff. Suites that exercise the changed code directly: `src/__tests__/copyOnSelect.test.ts` (12 cases), `src/__tests__/clipboard.test.ts` (30 cases), `packages/hermes-ink/src/ink/termio/osc.test.ts` (39 cases). An environment gap that made the first pass of this verification worthless, since it would silently affect anyone reviewing from a worktree: `ui-tui/node_modules` here was a symlink to another checkout's, so `node_modules/@hermes/ink -> ../../packages/hermes-ink` resolved against the symlink target and landed on that other checkout's copy of the package. `tsc` and the dist build therefore read an unmodified `hermes-ink` while the app code under test was the edited one. Fixed by giving the worktree a real `node_modules` whose `@hermes/ink` points at its own package, confirmed with `tsc --listFiles` naming the edited `ink.tsx`, then rebuilt and re-ran everything. Every number above is from after that. New tests were proved load-bearing rather than assumed: - Each of the four guards in `subscribeCopyOnSelect()` was removed on its own and the suite watched go red (1, 1, 2 and 2 failures), then restored. - The three cases that assert a copy did *not* happen are each paired with a positive assertion on the same subscription, because a bus that was never wired up satisfies the negative half by itself. - The path-reporting cases were checked by reverting the derivation to the deleted predictor's rule and watching the failed-load case report `tmux-buffer` again. - The Linux first-probe case was checked by reverting `copyNative()` to its optimistic `return true` and watching it fail on `success: true, path: 'native'` where it now asserts `success: false, path: null`. Establishing that this one was introduced here rather than inherited took running the same scenario against `github/main`, which reported `path: 'osc52'` for it: `success: true` with nothing written predates the branch, the native claim does not. - Of the three session-scoping cases, one is load-bearing: reverting the reporter to a process-wide flag fails "spends the caveat once per session". The other two -- a session returned to, and two reporters not sharing a tally -- pass with the flag as well, and are there to pin the property against a later refactor. Said plainly rather than implying all three prove the fix. One pre-existing assertion is flipped rather than kept. It pinned the terse copy-on-select line to "copied" on the OSC 52 path, which is the overclaim this branch removes, so the assertion encoded the defect instead of the behaviour worth holding. Rebased onto `22b71765`. The base gained #361, #362 and #355; the only file this branch and those commits both touch is `ui-tui/src/app/useMainApp.ts`, where #361 reworked the cover and prompt block while this branch replaced the copy-on-select effect in the same hook. The replay was clean and the whole verification above was re-run at the new head rather than carried forward. `git merge-tree --write-tree` needs git 2.38 and this box has 2.34.1, so the check was the file-overlap table plus the re-run, not a dry-run merge. Manual check of the rendered artifact, since `raven tui` runs the prebuilt `ui-tui/dist/entry.js` and nothing rebuilds it automatically: `npm run build`, then confirmed the new strings are in `dist/entry.js`. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed No doc change. `CONTEXT.md` has no clipboard or selection entry, and copy-on-select was already the term in use in the vendored fork, so nothing new is coined. The `/help` hotkey table is deliberately untouched: select-then-copy is ordinary terminal behaviour and does not need advertising. ## Risk Behaviour changes for users who are not on macOS: a drag now writes the clipboard and adds a transcript line. On macOS nothing changes except the wording of that line. `/copy` keeps its shape and gains the path the write actually took. One API change inside the vendored fork: `Ink.copySelection()` and `copySelectionNoClear()` return `{ text, path }` instead of the copied string, and `getClipboardPath()` is gone from the package's exports. Internally `copyNative()` may now answer with a promise on Linux's first copy, which `setClipboard()` awaits after starting the tmux write; no caller outside that function sees it. Both are consumed only within this repo, and the ambient declaration, the app interface and the two call sites move with them. Rollback is per concern. Reverting the feature commit restores the platform gate and leaves the fixes, which stand on their own. Reverting all of them returns the files to their base state; no data, config or on-disk format is involved, and no migration exists to undo. Checked and deliberately not fixed: - The OSC 52 write has no size cap and no chunking, so a large selection is handed to the terminal whole and silently dropped past whatever that terminal tolerates. This is why the OSC 52 path reports what it sent rather than claiming a copy. Capping or chunking it means changing the vendored write path and picking a threshold per terminal, neither of which belongs in this branch. - `Ink.copySelectionNoClear()` calls `getSelectedText()` outside its own `try`, so a throw there rejects the promise. The base already left that rejection unhandled at a bare `void` call, and adding a `.then()` does not change it. Fixing it means restructuring a vendored method this branch has no other reason to touch. - The effect's dependency array gains `sys`. It is provably stable (`useCallback` over `appendMessage`, itself `useCallback` with `[]`), so the subscription is not re-created in practice. If it ever were, the version de-dupe would reset and one extra line could be written; the path caveat would not repeat, because that tally now lives in a ref that survives the effect. - Running `/copy` before the first drag of a session shows the path caveat twice, once from each lane. Sharing that state between an explicit command and an automatic one to save a duplicated sentence is not worth the coupling. - Two narrative comments in `copyOnSelect.test.ts` were reduced rather than removed: each kept the constraint it was wrapped around (that the platform case can only mean something on a non-macOS runner, and that an empty result is the whole signal that nothing reached the clipboard) and lost the "defect this closes" framing. Reading section 1.1 as rejecting the task-context wrapper rather than the constraint underneath is a judgement call, flagged as one. The selection bus is faked in the new tests. The real one returns no-ops outside a fullscreen Ink instance bound to a TTY, so there is no way to drive it from vitest; the fake mirrors the ambient `useSelection()` contract, including `getState(): unknown`, which is why the narrowing is exercised rather than assumed. Flagging it because a hand-rolled stand-in for a real type is exactly the blind spot that hides a defect. Security: no new surface. Selected transcript text already reached the terminal through this path on macOS and through `/copy` everywhere; the same bytes now travel on the other platforms, on a user-initiated drag, which is what a native terminal selection would have done anyway. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
attempts.json is now the sole source of attempt grouping; the span-level mechanism (introduced by #362, zero production callers, never released) is removed: - build_span drops the attempt_id parameter and no longer writes the attempt.id attribute; attempt.id is a reserved key stripped from caller-supplied attributes so it cannot be injected. - TraceCtx loses its attempt field; the begin_attempt / end_attempt / current_attempt trio and the zero-caller turn_scope helper are deleted. - Read paths keep resolving the legacy span-level attempt.id attribute in old logs. - Docs and CONTEXT.md describe the final model: at read time an attempt id equals the trace id unless an attempts.json definition groups several traces under one minted id. - Test helpers default to the new span format (explicit attempt_id= still builds legacy fixtures); real-tracer E2E tests address attempts by trace id or via merge_attempts. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
attempts.json is now the sole source of attempt grouping; the span-level mechanism (introduced by #362, zero production callers, never released) is removed: - build_span drops the attempt_id parameter and no longer writes the attempt.id attribute; attempt.id is a reserved key stripped from caller-supplied attributes so it cannot be injected. - TraceCtx loses its attempt field; the begin_attempt / end_attempt / current_attempt trio and the zero-caller turn_scope helper are deleted. - Read paths keep resolving the legacy span-level attempt.id attribute in old logs. - Docs and CONTEXT.md describe the final model: at read time an attempt id equals the trace id unless an attempts.json definition groups several traces under one minted id. - Test helpers default to the new span format (explicit attempt_id= still builds legacy fixtures); real-tracer E2E tests address attempts by trace id or via merge_attempts. Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
## Summary `raven trajectory` had eight id-oriented subcommands; using them meant copying ids out of `list` by hand. This PR changes the attempt data model and adds the human face, in four phases: 1. **Attempt definitions** (`attempts.json` sidecar): an attempt id equals the trace id unless a definition groups several traces under one minted id. Merge creates a definition (absorbing prior definitions and legacy groups as aliases, so verdicts/pins recorded under old ids stay visible); split deletes it, migrating pins down to members - naturally undoable, span logs stay append-only. Pin migration is linearized under a fixed lock order (attempts > pins); every failure and concurrency interleaving over-protects, never under-protects. Legacy logs (span-level `attempt.id`) stay addressable and mergeable, but cannot be split (their grouping lives in append-only spans). 2. **Write-side removal**: the span-level attempt mechanism (the begin/end/current_attempt trio and the `attempt.id` span attribute, shipped in v0.1.13) is deleted; `attempt.id` is now a reserved attribute key stripped from caller input, so `attempts.json` is the sole grouping source by mechanism. Readers keep resolving legacy logs; zero data migration. This is a breaking API change, declared in the BREAKING CHANGE footer below. 3. **CLI adaptation and reader defect tolerance**: `list` folds definition members into one row and surfaces verdicts through the alias set; new `merge`/`split` subcommands wrap the data layer thinly; `unpin` clears definition, aliases, members, and the literal id in one transaction. Two whole crash surfaces are closed along the way: Rich markup injection (a legal id may contain `[/red]`; every dynamic value is escaped), and JSON-legal but type-broken records (per-line UTF-8 decoding, required-field string validation, non-object attribute containers) now degrade per record/field instead of killing commands. 4. **Interactive browser**: a bare `raven trajectory` on a TTY opens a three-screen questionary flow (sessions -> attempts -> actions: save / report / minimize / verdict / pin|unpin / split, plus multi-select merge). Menus and action messages never show a session key, trace id, or attempt id - only artifact paths carry ids. Every action triggers a full rescan (report bundles and auto-pins before confirming, so even an aborted action changed state); data-layer errors surface as one fixed id-free message; aggregation reads one snapshot per refresh and deduplicates records into logical spans keyed by (traceId, spanId), so a root turn's checkpoint+final pair counts once. CONTEXT.md gains the Attempt Definition term and the final-state Attempt wording. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_cli_trajectory_browse.py tests/test_cli_trajectory_commands.py tests/test_trajectory_store.py tests/test_trajectory_bundle.py tests/test_tracing_api.py -q` -> 241 passed - `uv run pytest tests/ -q --ignore=tests/integration` -> 6838 passed; the 11 failures + 20 errors match main's pre-existing environment-specific set node-id for node-id (cron timezone cases, everos/config root-permission cases, viewer probe, theme), compared as sorted FAILED/ERROR sets against a baseline recorded before this branch - `uv run ruff check .` and `uv run ruff format --check .` -> clean - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed (CONTEXT.md terms) ## Risk - Bare `raven trajectory` changes behavior: it opens the browser on a TTY (previously the help page) and exits 2 with a hint when non-interactive. Scripts are unaffected: the subcommand check runs before the TTY gate. - New spans no longer carry `attempt.id`, and `trace.begin_attempt` / `trace.end_attempt` / `trace.current_attempt` are removed. These shipped in v0.1.13, so an integration calling them raises AttributeError right after upgrading - an explicit, immediately visible failure rather than a silent one. Migration: group attempts after recording with `merge_attempts()` (library) or `raven trajectory merge` (CLI); reader fallback keeps existing logs addressable with zero data migration. - `attempts.json` is the only new mutable state; deleting it reverts every attempt to single-turn semantics. Span logs remain append-only. - Rollback: revert the squash commit; no data migration either way. The one known crash window (process death between merge's two file writes) leaves a harmless over-protective pin, documented in the store module. - [x] Security impact considered (markup-injection and path-escape surfaces closed; ids treated as untrusted in every renderer) - [x] Backward compatibility considered (legacy span-attribute logs stay addressable, mergeable, and listable) - [x] Rollback path is clear for risky changes ## Related Issues #362 (reference only: this PR removes the write-side attempt mechanism introduced there and shipped in v0.1.13). No issue is closed. BREAKING CHANGE: trace.begin_attempt, trace.end_attempt, and trace.current_attempt (shipped in v0.1.13) and the span-level attempt.id attribute are removed. Group attempts after recording instead: merge_attempts() in raven.trajectory, or `raven trajectory merge` in the CLI. Existing logs carrying a span-level attempt.id stay addressable through the reader fallback; no data migration is needed. --------- Co-authored-by: 江国庆 <guoqingjiang@deepglint.com> Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Summary
Add an end-to-end trajectory workflow for capturing, packaging, reporting, replaying, and permanently guarding Raven agent runs.
raven trajectorycommands to list, label, pin, save, report, replay, and minimize attempts. Saved bundles are self-contained and include rewritten artifact references, session history when available, verdicts, and a manifest.unknowninstead of fabricatingstoportool_calls, keeping recorded trajectories faithful to the provider response.Type
Verification
uv run --frozen pytest tests/ -q --ignore=tests/integration: 6862 passed, 33 skipped, 13 deselected, and 1 unrelated existing theme-rendering test failed. The branch does not modify that test, the theme implementation, or dependency manifests.uv run --frozen pytest tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare -q: the same unrelated theme-rendering failure reproduced in isolation.uv run --frozen ruff check .: passed.uv run --frozen ruff format --check .: 937 files already formatted.make check-large-files: passed.Risk
attempt.idattribute, and readers retain compatibility with older spans by falling back to the trace id.unknownfallback for a missing streamed finish reason. Production consumers were checked for dependence on the previous synthesized values.Related Issues
N/A