feat(vscode-ext): add VS Code extension that launches the Raven TUI - #373
Closed
LivXue wants to merge 655 commits into
Closed
feat(vscode-ext): add VS Code extension that launches the Raven TUI#373LivXue wants to merge 655 commits into
LivXue wants to merge 655 commits into
Conversation
…ng local patches ## Change description > Swap the vendored `subagents/raven-research/Raven-X` checkout from `ce225550` to upstream `main` at `71abb5a6` (7 commits, 55 files, +12495/-324), and take the host-side consequences with it. > > The checkout is patch-free again, which is the point of taking this commit now. It had been carrying six locally patched files so the ask_user fixes could run here before upstream had them. Upstream took five of the six back, and declined the sixth for the reason already recorded against it. The tree is now byte-identical to an upstream commit, verified by archive-and-diff. The two patches that carried behaviour were already live here and are unchanged in substance: the turn answering a clarify is classified as research from the commit marker rather than by consulting the conversation gate, and `observers["research_trail"]` carries the rendered trail where the launcher can reach it. > > `pyproject.toml` and `uv.lock` are byte-identical across the swap, so the editable venv carries over with no `uv sync`. > > Host side, carried with it: > > - `config.json` enables `drFlow.askUser` in `first_turn` mode, drops `ask_user` from `disabledTools`, turns `finalShape.requireMarker` off, and rewrites `identityOverride`, which had told the model there was nobody to consult. The everos and memory blocks are deliberately untouched. > - `run.py` learns the clarify handoff as a third shape of committed reply, read from `turn_end.awaiting_user` and tested before `finish_reason`, because the prose route carries an ordinary `stop` and would otherwise ship a page of questions as a finished report. It appends the research trail to the reply: the report template tells the model not to close with its sources, promising a record this launcher never received. A blank line before the trail keeps its `---` a real rule rather than a setext heading. > - `run.py` restates two contracts the new build changed. `--wait-skill-extract` is inert upstream now; `--flush-skill-buffer` is live again and blocks on one request bounded by `flush_timeout_s`, still passed on every turn because a lone `-m` turn trips no boundary and the alternative is not "promote later" but "never". > - `subagent.json` says the first reply is normally questions rather than the report. That string is what the dispatching model reads, and both ways a caller gets it wrong are silent. > - `README.md` records the swap, the retired patch set, the verification that proves it, the ask_user calling contract, and corrects two stale claims the swap surfaced. > > Upstream changes worth naming for a reader: the everos write path is rebuilt (turn writes detach after 5s, deferred capture, dedup, 422 fixes); plugin config slices are validated and a memory identity mismatch is reported, both non-fatal; the test suite stops resolving the reply-language directive from the developer's own config, which had moved eight prompt-byte shas on a Chinese host. ## Type of change - [ ] Bug fix - [x] New feature - [ ] Document - [ ] Others ## Related issues (if there is) > Fix #1 ## Checklists ### Development - [x] Lint rules pass locally - [x] Application changes have been tested thoroughly - [x] Automated tests covering modified code pass ## Risk User-visible behaviour changes, and how to roll back: - The research agent now asks clarifying questions on its first turn (`askUser.mode: "first_turn"`), and its first reply to a caller is questions rather than the report. Callers must relay the questions and send the answers back to the same instance. Roll back by disabling `drFlow.askUser` in `config.json`. - The research trail (queries, pages read) is appended to the reply the host shows, where it previously was not. - `--flush-skill-buffer` now blocks on one promotion request per turn; a slow everos costs the turn the adapter's `flush_timeout_s` (bounded by the launcher's `--timeout`). - `drFlow.version` moved down to `dr@3.4-filetools-askuser`; older folded labels are rejected by the loader, so reverting `config.json` alone would not load on the new tree (and vice versa). The full rollback is the previous vendored tree plus its `config.json`, both recoverable from this repo's history. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ### Security - [ ] Security impact of change has been considered - [ ] Code follows company security practices and guidelines ### Code review - [x] Merge request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached as necessary - [ ] Merge request linked to JIRA ticket when applicable
## Summary Move fixture and live session rows behind the sessions data source. Route session opening and counting through that source, keep the live boot skeleton hold inside the rail store, and remove the final shared session writes plus three shell verbs. The shared-coupling ratchet now reports zero live writes to unowned bindings, two held legacy containers, and 53 shell verbs. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (47 files, 613 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - `git diff --check` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed because DOM and class output remain frozen. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is session switching or live boot rendering against stale fixture rows. Tests cover source-owned opening, skeleton hold and release, settings session counts, and parked turns. Reverting this merge restores the previous shared binding path. ## Related Issues N/A
## Summary Move the stable workspace record into the modern workspace store. Route live turn numbering, workspace reads, and parked-session snapshots through typed store methods, remove the stale `cmds` snapshot field, and retire the `wsState` shell verb. The shared-coupling ratchet now reports zero live writes to unowned bindings, one held legacy container, and 52 shell verbs. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (47 files, 617 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - `git diff --check` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed because DOM and class output remain frozen. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is losing workspace rows or turn numbering across a session switch. Tests cover the stable state identity, exact parked snapshot fields, restore behavior, queue and turn-phase round trips, and the workspace renderer. Reverting this merge restores the previous shared global path. ## Related Issues N/A
## Summary `raven acp` becomes an agent an editor can launch: it negotiates, mints and reloads sessions, runs a turn, streams the reply and its tool calls, asks before acting, and answers a cancel while a turn is in flight. Six commits, each one concern and each buildable on its own (verified: every commit was checked out into its own worktree, imported, and its own tests run). **The process model is a self-contained engine**, not a client of a resident gateway. That was the earlier preference and it does not survive the topology: an ACP agent IS a stdio child of the editor, so there is nothing to mount onto unless a gateway happens to be running, and an agent that only works when another service is up is not an agent an editor can launch. Notes on the parts that are not obvious: - Every inbound frame is handled in its own task, because `session/prompt` is suspended for as long as the turn takes and `session/cancel` has to be read during it. Shutdown answers instead of cancelling: on EOF the pending prompts are settled as cancelled and the handlers return through their own code, so a finite input (a scripted client, a smoke test) still gets its replies. - A prompt is never answered with a JSON-RPC error. A failed turn is explained as message content and then ends with a stop reason. Measured from the other side on another implementation: an error in reply to a turn-shaped request makes clients tear the whole turn down. - The schema is vendored and pinned, and its blindness is measured rather than assumed: `additionalProperties` appears 118 times and is true in every one, `method` is an unconstrained string, and `params` accepts null. Validating a frame against the union therefore accepts an invented method name, so the tests validate payloads against their own definition and check method names against the metaschema separately. - Questions take one of two paths by what the client declared. With elicitation forms, a question with no preset answers can come back as text. Without them it rides a permission request and a synthesised tool call, which the payload marks as synthesised rather than dressing up as a real one. The case that has no answer at all, a free-text question to a client with no elicitation, is recorded in the compatibility matrix instead of being papered over. - Model selection goes through `session/set_config_option`. `session/set_model` and `models.availableModels` appear in older material and do not exist in the schema. - Secrets are redacted before anything reaches the client, including the value under a key whose name says it is a secret. A credential file's path is deliberately NOT redacted: a path is not a secret, and hiding it would hide that the agent read credentials at all. - Six command families ask before running on this surface. What that does not reach is written down: a sub-agent builds its own tool with its own policy and no responder, so a delegated command still runs unprompted. Registering the matchers there would refuse those commands rather than ask about them, which is a different product decision and is left open rather than half-closed. Two fields were already on the wire with no declaration on either side, and are declared here: `file_change` on `tool.complete` and `blocking` on `tool.start`. Neither is cosmetic, because both declarations forbid unknown fields, so a validating consumer drops the whole event rather than the unknown key. Two new gates close the hole the existing union test left: one compares each variant's payload field names across the two declarations, the other validates every frame the outlet actually emits against the declared union. The second is what found `blocking`. Known fidelity losses, all in `docs/specs/2026-08-21-acp-agent-compatibility.md`: `tool_call_update` always reports completed because the event carries no success flag; `rawInput` is not sent because it carries a command line verbatim; there is no plan because its priority field is required and has no source here. Rebased onto current main (which moved nineteen commits during this work; the rebase is clean and the whole suite was re-run on top of it). The two commits main gained while this was in review (the squashes of !164 and !166) made four of the original commits redundant; they were dropped, and the only thing main lacked from them, the process-group integration smoke test, is back as its own commit. Verified by diffing the rebuilt branch against the pre-rebase state: the only difference is 38 lines that main's reviewed version of !164 has and the local copy did not, so nothing was lost. Two gaps that earlier rounds recorded rather than fixed are closed here, and both were the same mistake from different angles: a check that looks present and is not reached. A delegated command ran unannounced, because the families a surface asks about were registered on one tool while a sub-agent builds its own. They are declared per surface now and every tool built under it inherits them. What a delegated command gets is a refusal with a reason, not a prompt, because that tool has no approval responder and a tool that cannot ask fails closed. A ContextVar carries the declaration rather than a module global: measured, not preferred -- the global version passed every test run alone and failed the file, because any test that started a server changed the process for every test after it. A sandbox made the safer configuration ask LESS. The gate skipped the whole classification when the executor was sandboxed. A microVM does contain what a command does to files, so the deny list and the contained families are still skipped; it does not contain a push, an install, or a connection to another machine, and those are exactly the families this surface exists to ask about. One overlap worth naming rather than leaving to be found: `feat(ui): a turn ends with the files it produced` on main renders a turn's files in the web transcript. That reads a tool call's own metadata; the `media` event here is the turn-level `MediaOut` the outlet used to eat. Different producers, and both were concepts before this branch -- only the second had no wire event. Reviewers: @chandler.zhang, @Blockchain-Key. ## Verification - `uv run --frozen --all-extras pytest -q`: see the run recorded below. - `uv run --frozen python scripts/coverage_gate.py diff --base-ref origin/main`: see below. - `uv run --frozen python scripts/coverage_gate.py ratchet`: see below. - `uv run --frozen ruff check raven/ tests/` and `ruff format --check`: clean. - ACP integration suites (stdio smoke, adversarial, self-verification, tool cancellation) run with `-m ""`. - `raven/acp/` unit coverage: 100 percent line and 100 percent branch. - Front-end gates for the wire-contract commit, run against a pinned toolchain: `npm run lint:rpc` reports generated.ts in sync, `npm run type-check` exits 0, `npm test` passes 102 files and 1353 tests. - Every new test was checked to fail without its fix, including the two new contract gates. ## Risk The user-visible additions are a new CLI surface (`raven acp`) and three optional wire fields plus one new event variant, all defaulted, so a client that ignores them behaves as before. The change with the widest reach is the stderr log sink no longer printing local variables. It is one line, and the reason it is in this branch is that an editor speaking a protocol over stdout surfaces the agent's stderr to the user, so the leak has an audience here. Shell approval gains six command families, but only for a surface that registers them. The terminal registers none and behaves exactly as before. Rollback is a plain revert of the branch: no data migration, no persisted format change.
## Summary Defer the shared page boot until the assembled script has installed every live data source. Offline fixture mode uses the same deferred boot, while live mode claims it synchronously and queues the first data-driven paint from the final live manifest part. Remove the last live mutation of demo-owned storage. The shared-coupling ratchet now reports zero live writes to unowned bindings, zero held legacy containers, and 52 shell verbs. The rebase onto the latest main exposed an incomplete generated web RPC client in that new base. Regenerate it from the merged OpenRPC schema so the branch and main contract gate include the new blocking tool call, file change, and media event types. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (48 files, 620 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py refs/fdceb/main..HEAD` - `git diff --check refs/fdceb/main..HEAD` - Changed-text control-byte sweep The design inventory and turn-machine notes now record the corrected boot order. The frozen page DOM and stylesheet are unchanged. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is leaving the live page unbooted or opening an absent fixture session. Dedicated boot-order tests cover offline fixture boot, live ownership of the first paint, an empty initial live session source, and placement after every synchronous source installer. The generated RPC diff is deterministic output from the schema already on main. Reverting this merge restores immediate demo boot and the live cleanup path. ## Related Issues N/A
## Summary Move the shared duration formatter into the modern shell bundle and import it directly from composer, transcript, subagents, and dag. Reuse the existing title formatter directly from subagents instead of routing both pure helpers through RavenShell. Move the gateway host platform onto DS.workspace, where the host-side reveal and open-in-app actions already live. Retire the three corresponding shell verbs and reduce the shared-coupling ratchet from 52 to 49 verbs while the other two axes remain zero. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (49 files, 621 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py refs/fdceb/main_shell..HEAD` - `git diff --check refs/fdceb/main_shell..HEAD` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed. Duration wording, title stripping, page DOM, class names, and the frozen stylesheet remain unchanged. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is changing compact duration boundaries or using the browser platform for gateway-side actions. Unit tests pin the duration boundaries, island tests now exercise the production formatter instead of test-only shell fakes, and both fixture and live workspace sources expose the same gateway platform closure used before. Reverting this merge restores the three shell verbs. ## Related Issues N/A
## Summary Move composer draft persistence, debounce, and session ownership into the composer island. Preserve the new-draft claim when the first message creates a real session, and let the rail delete drafts through their owner instead of a legacy shell verb. Move uploaded image preview bytes into a narrow modern cache shared by composer and transcript. Retire six shell verbs and reduce the shared-coupling ratchet from 49 to 43 while the other two axes remain zero. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (49 files, 626 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py refs/fdceb/main_before_composer_push..HEAD` - `git diff --check refs/fdceb/main_before_composer_push..HEAD` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed. The page DOM, class names, draft storage key and limit, debounce timing, attachment note format, and frozen stylesheet remain unchanged. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is filing unsent text under the wrong conversation or losing the in-memory image preview after upload. Focused tests cover debounce timing, session switching, new-session claims, explicit deletion, send cleanup, upload caching, and transcript rendering. Reverting this merge restores the six legacy shell verbs. ## Related Issues N/A
## Summary Move the main transcript tail-follow state into the transcript feature. Let transcript rendering and composer scrolling share that owner directly, while the legacy layers keep only a narrow RavenIslands adapter for their remaining calls. Route composer notes directly to the transcript and pane drag measurements directly to the composer. Retire five shell verbs and reduce the shared-coupling ratchet from 43 to 38 while the other two axes remain zero. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (50 files, 627 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py refs/fdceb/main_before_tail_push..HEAD` - `git diff --check refs/fdceb/main_before_tail_push..HEAD` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed. The page DOM, class names, instant tail scroll, note rendering, pane bounds, and frozen stylesheet remain unchanged. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is forcing the transcript back to the tail while the reader is reviewing earlier output, or failing to update the composer offset during a pane drag. Focused tests cover the shared stuck state, instant scroll behavior, pill visibility, direct transcript notes, and per-frame dock measurements. Reverting this merge restores the five legacy shell verbs. ## Related Issues N/A
## Summary A direct chat with Claude Code over ACP now renders its tool calls under Claude Code's own tool names, so the transcript reads as a Claude Code conversation rather than a generic one. This matches the choice already shipped for codex, and for the same reason: knowing which agent ran a call is worth more than one vocabulary shared across transports. **The read boundary stops renaming.** `tool_vocabulary.RAVEN_NAME` mapped twelve claude-agent-acp names into Raven's own (`Bash` to `exec`, `Glob` to `find`) on the way to a client. Those twelve entries are gone. The ACP spec-kind entries above them stay, because the base dialect still reports `kind` and other adapters are read through it. No migration is needed: the stored record always held `Bash`, the rename happened on the way out, and a direct chat folds its live and its settled turns through one function, so the two change together and cannot disagree. `ARGUMENT_KEY` gains eleven entries in exchange, one per name except `Task`, each mapped to the key that name's old target already resolved to. Because every value is the old resolution, the promoted arguments come out identical and only the displayed name moves. Without it, `Glob` and `Grep` rows would report the directory searched and lose the pattern. **The TUI gains a verb table keyed by those names**, beside the codex one, so a run of six reads still collapses to one line instead of falling to the generic rule. Row shaping follows, because the shell-label branch, the path-clipping set and the quoted-needle set were all keyed by Raven's names: `Bash` had to join the first or a row would have regressed from two program names to a 40-character clip of the raw command. **A Bash row now shows the model's own description of what it was for.** The adapter sends Claude's `description` beside the command, and the row was showing a label derived from the pipeline instead. It arrives as a new optional `EpisodeTool.intent`, preferred inside the one function that both the folded and the expanded subject reach, so the two cannot diverge. The expanded detail block still shows the whole command. **The web UI needed a fallback.** Its tool-renderer registry is keyed by `Bash`, `Read`, `Write`, `Edit`, `Glob` and `Grep`, and a direct chat's messages do reach it. Once the names stopped being renamed, `Read`, `Write` and `Edit` began hitting renderers whose shared path helper read only `file_path`, while the read boundary promotes that subject onto `path` -- so the path vanished from the row, and the default renderer that used to print the arguments no longer ran. The helper now accepts both, `file_path` first, since the web UI's own agent sends that spelling. Reworking those renderers to speak Claude Code's vocabulary the way the TUI's tables do is left out of scope. `TodoWrite` never emits a tool call at all: the adapter routes its state to a plan frame, so the plan row is now named for the tool behind it, symmetric with codex's. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification All run after rebasing onto the current `main`: - `uv run pytest -q` -- 10766 passed, 0 failed, 50 skipped, exit 0 - `npm test --prefix ui-tui -- --no-file-parallelism` -- 106 files, 1505 passed, 0 failed, 13 skipped - `npm run type-check --prefix ui-tui` -- clean - `npm run lint --prefix ui-tui` -- 0 errors - `scripts/check_commit_messages.py origin/main..HEAD` -- exit 0 - `make check-large-files` -- clean - `make lint-python` -- 3 errors, in `raven/agent/tools/deliver.py`, `raven/cli/tui_commands.py` and `tests/test_channels_outlet.py`. All three are byte-identical to `main` on this branch, so the failures are inherited rather than introduced. `--no-file-parallelism` is not optional for the TUI suite: it flakes under default worker parallelism above roughly a hundred files, and a flake there cannot be told apart from a real break. Six existing assertions encoded the vocabulary this change removes, and were migrated. Only their name assertions changed; every argument assertion in them still passes exactly as written, which is what shows the re-keyed `ARGUMENT_KEY` preserved behaviour rather than merely appearing to. Evidence for the frame shapes is 224 frames captured from real sessions plus the adapter's own published source. Only `Bash` and `Read` were exercised on a live wire; every other name's input shape is read from the adapter's `toolInfoFromToolUse` switch, which ships unminified, and the argument tests are written to those declared shapes. The design doc separates what was measured from what was derived, and says which is which per name. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk Every tool row in a claude_code direct chat changes its verb. `read 6 files` becomes `Read 6 files`; a shell row becomes `Bash` plus the model's own description where one was sent, and the derived program label where it was not. A nested Claude Code subagent call no longer borrows the `delegated` verb and reads as `Task`. Already-stored sessions render the new way on resume, because the rename was never in the record -- there is no migration and no mixed state. Deliberately not covered: the `interrupted` flag the adapter sends alongside a command's output is left unread. It appeared zero times across the 224 captured frames, so its shape on a real interruption is unknown, and handling it would mean untested code written against a guessed shape. Rollback is a plain revert. Restoring the twelve `RAVEN_NAME` entries returns every row to Raven's vocabulary; the TUI table then simply goes unconsulted rather than breaking, and the web UI fallback is independent and safe to keep either way. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A
## Summary Move edit, write, and unified diff row construction into the workspace feature. Let the legacy workspace adapters and the transcript call renderer share the same typed implementation. Route delegated run stage rendering through DS.agents directly instead of sending it through RavenShell and back to the same source. Retire four pass-through shell verbs and reduce the shared-coupling ratchet from 38 to 34 while the other two axes remain zero. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (51 files, 630 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 tests) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py origin/main...HEAD` - `git diff --check origin/main...HEAD` - Changed-text control-byte sweep No user-facing documentation or screenshot update is needed. The hunk row shape, context folding, line counts, delegated transcript DOM, page classes, and frozen stylesheet remain unchanged. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The main risk is changing diff line numbers or failing to paint a delegated record. Focused tests pin edit context folding, write truncation, unified diff headers and numbering, and all three delegated record paths through the agents source. Reverting this merge restores the four shell pass-through verbs. ## Related Issues N/A
## Summary Move clipboard behavior into the modern shell bundle, read browser platform and language facts directly, and expose the running Raven version through `DS.settings`. This retires `copyToClip`, `lang`, `appVersion`, `modKey`, and `isMac` from `RavenShell`, reducing the guarded shell surface from 34 verbs to 29 without changing the served DOM, class names, styles, or single-file build contract. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (52 files, 633 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 29) - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 passed) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - Changed-text control-byte sweep - `git diff --check glhttps/main...HEAD` ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Clipboard refusal remains silent, the offline page still no-ops when the Clipboard API is unavailable, and the running version remains unknown until `system.version` answers. Rollback is the single squash commit. ## Related Issues N/A Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
## Summary Route every modern toast and context-menu caller directly to the existing `shell/toast` and `shell/menu` writers. Move `ToastAction` and `MenuItem` beside the writers that own those contracts, and remove both calls from `RavenShell`. Tests that assert writer output now mock the owning writer modules. Other shell fakes drop the two required placeholders they no longer need. The served DOM, class names, styles, legacy bare-name callers, and single-file build contract remain unchanged. The guarded shell surface falls from 29 verbs to 27. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (52 files, 633 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 27) - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 passed) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - Changed-text control-byte sweep - `git diff --check glhttps/main...HEAD` ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The direct imports are the same functions already published for the concatenated layers. Existing writer unit tests continue to cover DOM behavior, and feature tests preserve assertions for notices, actions, and menu entries. Rollback is the single squash commit. ## Related Issues N/A Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
## Summary Move slash-command names and help text onto `DS.composer`, beside the command catalogue they describe. Route execution-reach labels through a small modern catalogue helper, then remove the four obsolete `RavenShell` verbs and the dead demo reach formatter. The served DOM, class names, styles, live source overrides, and single-file build contract remain unchanged. The guarded shell surface falls from 27 verbs to 23. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (53 files, 635 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 23) - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 passed) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - Changed-text control-byte sweep - `git diff --check glhttps/main...HEAD` ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Both modes already share the same slash catalogue and translation function. Focused tests cover every reach key, the unknown-key fallback, slash rendering, and the affected settings and skills views. Rollback is the single squash commit. ## Related Issues N/A Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
## Summary Replace the legacy workspace sidebar with a persistent floating desk and a responsive multi-pane workspace. - Group registered agents with their dynamically sorted instances and preserve real backend registration types. - Open diffs, files, and subagent conversations in up to four responsive panes without breaking legacy workspace entry points. - Reuse the transcript and composer behavior for resumable subagent conversations, including optimistic messages and queued turns. - Add draggable and resizable desk geometry, launcher anchoring and snap behavior, pane resizing, fullscreen controls, and responsive layout limits. - Route transcript web links to the user's external browser and keep file navigation inside the active file pane. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `node ui/scripts/check-css.mjs` - passed. - `npm run type-check --prefix ui` - passed. - `npm test --prefix ui` - passed, 50 files and 627 tests. - `npm run build --prefix ui` - passed. - `make check-large-files` - passed. - `git diff origin/main...HEAD --check` - passed. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk The change replaces workspace presentation and routing while retaining compatibility adapters for existing sidebar actions. Rollback is the squash commit revert. No dependency or persistence schema changes are included. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A
…hat shadow a folder
## Summary
A pull that updates `subagents/` needs `install.sh` run again, and nothing said so.
Three things go stale; the script now addresses all three, and says which one applied.
Two of them re-running already fixed. A folder an upgrade **added** has no venv, and a
checkout whose dependencies moved needs a sync. Both are now visible: each folder's line
carries the delta `uv sync` produced (`4 added, 1 updated`, or `no change`), and the
summary names the folders this run built for the first time. Neither needs an argument.
What was missing was any signal that re-running was necessary, which is why nobody did.
The delta is read from a captured stream because `uv` writes it to stderr, which the
existing `> /dev/null` never suppressed - a four-folder run already printed that chatter,
interleaved and attributed to nothing.
The third could not be fixed by re-running at all. A config row written by an older
`install.py` **outranks the folder's own manifest**, because the table takes the whole
stored row: a manifest the pull updated never reaches the roster, and a command baked
against a tree that has since moved keeps naming the old path. Every stored row naming a
folder in the tree is now listed with the fields that disagree, and `--prune-stale`
deletes them so discovery supplies the row instead.
Key decisions:
- **Reporting is unconditional; deleting is not.** A row here is the user's config. The
ordering that moved registration out of this script - a first install runs it before a
configured raven exists - is the same one that makes a config write unsafe to assume,
so the default path still writes nothing outside the tree and the flag is the whole
consent. With no host raven to read the config through, the step is skipped rather than
failed, and it never creates a config file that was not already there.
- **The match is by name, and a name match cannot tell an outdated snapshot from a row
someone tuned by hand.** Rather than guess, both consequences are reported per row: a
row edited in the web UI is deleted along with the rest, and a row carrying
`enabled: false` is the only place that switch lives - a purely discovered row cannot be
switched off, so deleting one re-enables that agent. Neither warning is suppressed by
the flag.
- **Both sides of the comparison go through the schema.** `recommendedLlm` annotates the
installer and is not a config field, so the schema drops it and a stored row can never
carry one; comparing raw would have reported it as differing on every row, on every run,
forever.
- **The backup is the rollback, so it is taken from the bytes on disk.** The write path
expands a stored row to every defaulted field, so a copy taken through it would not
restore what was there. It is written before any deletion, and opened at `0600` rather
than chmodded afterwards, because an `openai` row among these carries its own api key.
- **A requested prune that could not run exits non-zero.** The flag names an action, and
reporting exit 0 after declining to perform it tells a scripted caller the opposite of
the truth. Plain reporting stays advisory and still exits 0.
## Type
- [ ] Fix
- [x] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other
## Verification
At the current head:
```
uv run pytest -q -p no:randomly
-> 10779 passed, 51 skipped, 13 deselected, 0 failed
```
**The first pipeline was red, and it found a real hole in this suite.** One test failed
with `install.sh: line 70: python3: command not found`. The narrowed `PATH` that makes
`command -v raven` find nothing also removed the `python3` the script's own helpers run,
and the suite borrowed one back from `/usr/bin`; the CI image ships none, because `uv`
brings its own interpreter. Every local run was green for exactly that reason, which is
the isolation failure this repo keeps filing - a suite reading the machine rather than
building what it needs.
Two tests were wrong in the same way and had been passing anyway: they name an
interpreter that runs and cannot import raven, pointed at `/usr/bin/python3`, so on a
machine without that file they exercised "the interpreter does not exist" while asserting
against the message for a different condition. Both now use a shim.
Reproduced rather than reasoned about. With `/usr/bin` and `/bin` mirrored into a
directory holding every tool except `python*`, the pre-fix suite fails exactly as CI did
and passes with the fix restored:
```
run A (mirrored PATH, fix reverted) -> 1 failed, 16 passed, 1 skipped
the same test, the same message as CI
run B (mirrored PATH, fix restored) -> 17 passed, 1 skipped
```
`main` was green on all six of its most recent pipelines, including this branch's base, so
the red was this branch's and was not read as inherited.
```
uv run ruff check tests/test_subagents_install_script.py subagents/ -> All checks passed!
uv run ruff format --check tests/test_subagents_install_script.py -> 1 file already formatted
bash -n subagents/install.sh -> clean
uv run python scripts/check_commit_messages.py origin/main..HEAD -> exit 0
make check-large-files -> exit 0
```
The script had no test at all before this. The new suite drives it under `bash` against a
tree, a host config and a `uv` built for the test, and pins `RAVEN_HOME` on every run
including the runs meant to find no host raven: the step deletes rows from the host's
config, so a test that fell through to the developer's own would delete their agents.
**Proof the tests are load-bearing, not theatre.** Every test was watched failing before
the code existed (9 red, 5 vacuously green). The 5 that could only pass vacuously were
then checked by mutation - each guard removed on its own, and the matching test observed
going red:
| mutation | test that went red |
|---|---|
| `mode` forced to `prune` | a colliding row survives a run without the flag |
| collision filter removed | a row no folder claims is not reported |
| `--dry-run` ignored | dry run refuses to prune |
| backup taken with no collisions | pruning nothing writes no backup |
End to end against the real four folders and a copy of a real host config: 4 vendored rows
deleted, 4 unrelated rows kept, the backup byte-identical to the pre-state, and the real
config untouched.
- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed
`subagents/README.md` gains an `After a pull` section covering all three cases and both
costs of the name match, and its exit-status sentence is corrected for the new condition.
## Risk
- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes
The user-visible change on a plain run is additional output: a per-folder sync delta, a
`newly built` summary line, and a report of the config rows that shadow a folder. No
config is written without `--prune-stale`.
With the flag, rows are deleted from `~/.raven/config.json`. That is the point, and it is
also the irreversible part: the deletion takes any web-UI edit on those rows with it, and
re-enables an agent whose stored row was the only thing switching it off. Both are reported
per row before anything happens, and the previous list is backed up beside the script
first. Restoring that file is the rollback.
Security: the backup can hold an `openai` entry with its own api key. It is created at
`0600` rather than chmodded after the write, and lands in a directory the repo already
ignores. Nothing else new reaches disk, and no route, RPC or other network surface is
added or widened.
Two callers run this script from inside raven (`subagents.build` over RPC, and onboarding).
Neither passes `--prune-stale`, so a build triggered from the page reads the config and
does not write it. Both read the last line of the script's output as an error detail when a
build fails; that line was the summary before this change and still is, so it has not moved.
Known gaps, disclosed rather than left to be found:
- The test that proves an unwritable backup aborts the prune skips as root, which is how
this suite already handles permission cases. That path was instead verified by hand
against an unwritable destination: the specific message appeared, the config kept all 8
rows, and the generic one was correctly not printed over it.
- The `uv` delta parser is exercised only against a stand-in whose output format was
sampled from real `uv`. A future change to that format would go unnoticed.
- The report lists `command` and `resumeCommand` as differing whenever the stored row was
written against a tree at another path. That is correct - it is the case
`_launcher_is_gone` exists for - but it means running the script from a copy of the tree
reports every row.
- Three files carry a `ruff` I001 that predates this branch and is not fixed here, because
the fix would widen the diff into unrelated files: `raven/agent/tools/deliver.py`,
`raven/cli/tui_commands.py` and `tests/test_channels_outlet.py`. Each was confirmed
failing on `origin/main` itself. The pipeline runs only the unit suite, so nothing
catches them before the monthly GitHub PR.
## Related Issues
N/A
## Summary Two changes, one MR because they were found together and touch the same tests: a production bug where `ps` output is truncated by `COLUMNS`, and a test-suite speedup that cuts the per-test timeout budgets. ### Bug: `ps` truncates the command line it is asked to read `ps -o command=` truncates to `$COLUMNS`, which defaults to 80 when the environment variable is unset - and it is unset in every context that calls this, because none of the three readers has a terminal. All three look for a marker that sits at the *end* of a command line, behind an absolute interpreter path, so past column 80 the marker is gone and each one answers the opposite of the truth. Measured on the machine this was found on, against the live everos server running under a stock `uv tool install` layout: the command line is 129 characters and its marker starts at column 83. With `-ww`: `_is_everos_server` returns True. Without: False, and `ps` returns exactly 80 characters. - `_is_everos_server` guards the signal path. When it wrongly returns False, `find_recorded_server` also fails its second check (the resolved root is past column 80 too), so raven reports "no server" while one is running. The dangerous direction: the caller concludes nothing is there and starts a second one. - `_cmdline_of` promises "the full command line"; truncation makes that false rather than merely imprecise. - the tracing viewer checks for `server.js` at the end of a node argv, so `_stop_viewer` cannot recognise the process it is meant to stop. `-ww` applied to all three. The existing regression test had been passing for a reason that had nothing to do with the code: the marker's column depends on where the checkout is, so it was green in a shallow clone and red in a git worktree, whose extra path segments pushed the marker past column 80. It now pins `COLUMNS` and pads the command line so it exercises the truncation wherever it runs, and it was confirmed to go red with `-ww` removed and green with it restored. ### Tests: stop paying a full second for budgets that exist to be exceeded These cases assert that a budget ran out. The work they race against outlasts it by design (a 5s sleep versus a 1s timeout, a stub that never answers), so the budget's size buys nothing, and shrinking one makes the case both cheaper and more certain: the outcome under test is the timeout winning, and a smaller budget gives the thing that must not happen less room, not more. The largest single cost was not a budget. `cli_dispatch` runs the command through `asyncio.to_thread`, and a thread cannot be cancelled - so when the dispatch times out the command runs on, and the loop's shutdown joins it. The 10-second sleep in the fake `slow` command was therefore paid in full by the teardown, 9.7s of it after the assertion had already passed. The sleep is now 1s against a 0.05s budget. This is a rare F1 near-miss where the giant teardown was the actual defect. Measured on the eight files where a cut actually lands: 64.3s -> 31.5s. A ninth file was in the first revision and is not any more. Two ACP cases had their `ready_timeout_ms` lowered from 1000 to 50, and `verify_agent` computes `budget = max(1.0, ready_timeout_ms / 1000)` - so both values produce the same one-second budget and the cases still cost 1.01s and 1.03s. Reverted rather than made to work: reaching under that floor means extracting an inline `max(1.0, ...)` into something a test can patch, which is a production change for two seconds and unrelated to this branch. Leaving them in would have been worse than not having them, because they read as if that config value governed the cost of those tests. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification At the rebased head: ``` uv run pytest -q -p no:randomly -> full suite: 10766 passed, 50 skipped, 13 deselected, 0 failed uv run ruff check raven/ tests/ (my changed files) -> clean uv run ruff format --check (my changed files) -> clean make check-large-files -> exit 0 uv run python scripts/check_commit_messages.py -> exit 0 ``` **Reproduced before claiming fixed.** Two lockstep checks: 1. The regression test is load-bearing: with `-ww` removed from `_is_everos_server`, `test_ps_recognises_a_process_whose_command_line_says_everos` goes red; restored, it goes green. 2. The bug was provable on main's behaviour: against the live everos PID, `ps -p <pid> -o command=` returned 80 chars and the marker was absent; the same process with `-ww` returned 129 chars and the marker was present. **Flake-proofing is measured, not asserted.** Attempting to shrink these budgets could buy a flaky suite if the timeout stops winning the race against the work under starvation, so the shrunk cases were run pinned to one core with a competitor pinned to the same core - a harsher model of the contended single-core `tests` runner CI uses: ``` 30 runs of the shrunk cases -> 0 failures (38 selected per run) 3 runs of all nine files -> 0 failures ``` Those runs were taken before the two ACP edits were reverted, so they cover a superset of what ships: the reverted cases were among the 38 and passed in every one of the 30 runs at their original one-second budget, which is the value they are back to. The 08-18 baseline for this suite (same files, old budgets, same machine) was 64.26s; the new one is 31.47s. The historical-version timing used `git archive` of `a3e34fac` with the current venv, run on the same box. After reverting the two no-op ACP edits the same nine files measured 33.4s against 32.4s with them in place, which is inside the run-to-run noise and is the evidence that those edits saved nothing. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes **Bug fix:** no user-visible behaviour change except that raven now recognises its own everos server and the tracing viewer's stop command, when the command line is longer than 80 characters. That is the correct behaviour; the previous behaviour was wrong only in that it kept a second server from being started when it should have. Rollback is the revert - the three diffs touch only the `ps` argv. **Test speedup:** the only production change (beyond the bug fix) is in the test file `tests/test_rpc_cli_dispatch.py`'s `slow` command sleep (10s -> 1s). A 1s sleep still outlasts the 0.05s budget by 20x, and the 30-run starvation test confirms the race still resolves correctly. `sleep 1.0` in a test is already an acknowledgement that a test may outlast its own timeout, so the shrink is not a race-risk. **Deliberately not shrunk**, because for these the direction is reversed and a smaller budget buys flakiness: - `test_live_child_still_gets_the_full_budget` asserts a probe count derived from the budget and the poll interval, so the budget is the subject rather than overhead. Its docstring says not to cut it short. - the reparented-child case needs its launcher to reach the line that writes the pidfile before the timeout kills the process group. - `wait_for(read_recv.receive(), ...)` asserts that a message ARRIVES inside the budget, which is the one shape where a tighter bound is a worse test. - two `ensure_everos_server` calls raise before the poll loop starts, so their budget is never spent and changing it would be diff noise. (Three calls were in fact left at 1.0: those two, plus the one on `test_live_child_still_gets_the_full_budget`, which is the budget-is-the-subject case above.) ## Related Issues N/A Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…essions
## Summary
Serve the built-in generic agent (the `Raven` row) over ACP and complete the
ACP server's session surface so raven-as-agent is usable end to end.
- A config row of a seed's name with `kind: "acp"` redeclares the built-in
generic agent over this raven's own `raven acp`: command and RAVEN_HOME are
resolved at materialization (config stays machine-portable), the write guard
and web RPC list/toggle follow the same rule (cli/openai rows still cannot
claim a seed name), and the acp row is editable like any config row.
- The ACP server implements `session/resume` and declares
`sessionCapabilities.resume`, which is what a raven acting as its client
reads to report the row resumable - an instance handle then continues the
session instead of starting fresh.
- `session/close` and `session/delete` drop a session from the connection and,
for delete, from the engine's store; a turn still running is cancelled, and
unknown ids are refused with -32002 (delete also handles lazy sessions that
were minted but never prompted).
- Tool rows carry the emit site's verdict: ToolResult/ToolOutput gain an `ok`
field (exec reports exit_code, policy refusals report false), ToolEvent /
RpcOutlet / the TUI and web wire contracts pass it through, and
`tool_call_update` reports `failed` instead of always `completed`.
- `session_info_update` (auto title, announced once per change) and
`available_commands_update` (a deliberate white-list of six user-facing
actions, announced per session) notifications are served.
- ACP capability snapshots are backfilled at startup: a background,
sequential verification of enabled acp rows with missing or stale snapshots,
so a fresh install reports stateful instead of hiding the agent from the
instance picker until someone runs a Test. Skipped on the acp channel itself
to avoid a verify cascade.
- The ACP compatibility matrix documents all of it.
Review round one landed five fixes in five commits: the snapshot backfill now
refreshes the materialized table after recording (and defers on mounted stacks
that have no registry to refresh), `session/delete` cancels a running turn
before touching the store, MCP maps `isError` into the result verdict, and the
native TUI consumes the verdict on its live path.
Scope is Python runtime + generated client types only; the two refactor
commits already on main (ui/ and ui-tui/ changes) are not touched.
## Type
- [ ] Fix
- [x] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other
## Verification
- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed
- `uv run pytest tests/test_acp_*.py tests/test_subagent_*.py tests/test_update_subagents.py tests/test_rpc_subagents.py tests/test_rpc_spine.py tests/test_rpc_bootstrap.py tests/test_cli_gateway_page.py tests/test_mcp_client.py` -> 1697 passed, 1 skipped at the review-round head; the two files the reviewer's run pointed at pass
- `uv run pytest tests/test_acp_methods.py tests/test_rpc_spine.py tests/test_rpc_contract_shapes.py tests/test_acp_updates.py` -> 388 passed (post-rebase sweep)
- `uv run ruff check` on the changed files -> clean
- `npm run gen:check --prefix ui` -> generated client matches the contract (gen:rpc run for ui-tui)
- `make lint` reports three pre-existing I001 import-order findings in files
this MR does not touch (`raven/agent/tools/deliver.py`, `raven/cli/tui_commands.py`,
`tests/test_channels_outlet.py`); all changed files pass ruff.
- E2E against the installed `raven acp` (real provider, temp RAVEN_HOME): 29/29 protocol checks pass, including handshake capabilities, command menu, tool failed flag, permission ask-and-reject, cancel, load/resume with context continuation, close/delete lifecycle, and multi-session coexistence
- The pre-submit sweep ran against `origin/main...HEAD` and produced two
follow-up fixes, landed in one commit: the startup snapshot backfill task is
now held by reference (asyncio keeps only a weak reference, so an unrefed
task can be collected mid-run), and the web transcript prefers the emit
site's `ok` verdict over the text heuristic, kept as the backstop for an old
server that does not send the field. Verification below was re-run at that
head.
Checked and deliberately not fixed:
- The web and tui transcripts do not render a distinct failed row yet other
than the text the error preview already carries; this MR makes the verdict
available on the wire and generated client types, and leaves the visual
treatment to the frontend work that owns it.
- `session/set_mode`, `logout`, audio, `plan`, fs/terminal client features and
per-session MCP servers remain unsupported per the compatibility matrix;
this MR closes session lifecycle, resume and failure visibility, not the
whole protocol surface.
- A failed verify in the startup backfill leaves the row stateless (listed,
not hidden) until a later Test or probe=true listing; making the backfill
retry later is a deliberate deferral.
## Risk
- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes
The generic agent's default dispatch changes only when config redeclares it
as acp; without that row everything behaves as before (the seed row and its
in-process transport are unchanged, and old code ignores such a row with a
warning). Instance handles for the built-in row are now ACP-backed, so their
statefulness comes from a capability snapshot that the startup backfill
produces - a missing snapshot leaves the row listed but stateless, never
unusable. RAVEN_HOME is inherited by the child engine by design, so an acp
redeclaration runs under the same config and provider as the host.
## Related Issues
N/A
Co-authored-by: Claude (deepseek-v4-flash-vision-exp) <noreply@anthropic.com>
## Summary Move the served front end's appearance preference and desktop-notification preference into modern shell owners. Preserve the existing localStorage keys, default-light first frame, document dataset contract, native desktop theme message, background-only notification rule, and forced settings test notice. The concat boot and live turn layers keep their established `lookLoad` and `ntfPush` names as thin shims into the modern bundle. Remove the obsolete appearance fields from demo state and the `look`, `ntf`, and `themeSet` entries from `RavenShell`. The served DOM, class names, styles, and single-file build contract remain unchanged. The guarded shell surface falls from 24 verbs to 21. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed - `npm run --prefix ui type-check` - `npm test --prefix ui` (55 files, 644 tests) - `npm run gen:check --prefix ui` - `npm run lint:i18n --prefix ui-tui` - `npm run --prefix ui build` - `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 21) - `node ui/scripts/check-css.mjs` - `uv run pytest tests/test_ui_language_repaint.py` (9 passed) - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - Changed-text control-byte sweep - `git diff --check glhttps/main...HEAD` ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Focused tests cover stored and malformed appearance preferences, every document dataset field, the effective system theme sent to the native host, direct theme toggles, notification persistence, permission and focus gates, the forced test notice, and both Settings page connections. Rollback is the single squash commit. ## Related Issues N/A Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…ts trace ## Summary A running sub-agent DAG node now shows what it is doing, in the TUI transcript, without a click. Under each running node's row sits one line carrying the tail of its sub-agent's message stream, refreshed while it works, so a reader watches the run rather than inferring it from a spinner. Clicking the row (or the node's box in the graph) opens a fixed-height box holding the node's conversation trace, drawn by the transcript's own message renderer. Clicking again returns to the line. `/dag <node>` gained the same trace, printed in full. The whole feature is client-side. `dag.node` already returned a running node's transcript from the live activity the runner publishes, and the ACP collector republishes the whole transcript on every frame, so a read is a complete snapshot rather than a delta. No backend change, no schema change, no client regeneration. Three decisions worth stating, because each one is a constraint the rest hangs off: - **The box is a constant height, always.** Its footer is drawn whether or not anything was cut, and a short trace is blank-padded. A box that grew as a run produced messages would shove every row beneath it several times a second, and the transcript's height model could then not state a panel's height without folding the trace first, while the fold would need the height. Fixing the height cuts that knot. - **The stream line is a character tail, not a step ticker.** It shows the last line's worth of whatever the agent is producing, so it visibly moves; a structured one-step-per-line view would sit still through a long tool call and cost a row per node. - **What a click opens widened.** A node used to expand to its prompt template, so a node whose call carried no template was not clickable. There is a trace now, so a node is expandable unless it is still pending with no template - the one case where a box would open onto nothing. The row and the node's box in the graph share one rule, so they cannot disagree. A live run against real ACP agents drove out four defects that a green test suite did not, and review a fifth. They are worth recording because they are one shape: a bound that was correct for the state the code used to be in, kept after the state changed underneath it. - The poll read its runs from the live turn state, which is cleared when the turn ends. A `run_subagent_dag` call returns as soon as it launches the run in the background, so a node routinely outlives its turn and the poll tore down after a single read. It now reads the runs pinned onto their tool rows as well, the live copy winning where both name a run. - Polling a pinned run then exposed that a pinned run's status is a frozen copy, never revised. The poll therefore cannot stop on the displayed status; it stops on the status the server reports in the response, and gives up on a node after a bounded number of consecutive failed reads so a pruned run directory cannot be polled forever. - That stop condition was itself wrong in a way worth naming: it asked whether the status was anything other than pending or running. `dag.node` reports a null status until the run writes its manifest, and a poll's first read of a node lands inside that window - so every node was marked finished on its first read. A negative exclusion list fails open; it is now a positive test over the four statuses that mean done. - The line echoed the node's own prompt back until the agent spoke, because the server brackets a trace with the prompt. It now skips that row, so a node that has produced nothing says so. - Review caught the last one: an expanded node was skipped once the store held any trace for it, which predated the status-based stop and was that stop's stand-in. A node expanded while it streams holds a trace by the time it goes terminal, so the read still owed at that moment - the one carrying the final answer - was suppressed and the box kept the last live snapshot. Leaving `running` rearms the poll, so one read per generation now bounds it and the response's terminal status settles it for good. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Run at the branch head, after rebasing onto main: - `cd ui-tui && npm test -- --no-file-parallelism` -> 110 files, 1584 passed, 13 skipped, 0 failed. The flag is required: these ink render tests flake under default worker parallelism at this suite size. - `uv run pytest -q` -> 10783 passed, 51 skipped, 13 deselected, 0 failed. No Python changed; this is evidence of that rather than a claim. - `cd ui-tui && npm run type-check` -> 0 errors. `cd ui && npm run type-check` -> 0 errors. The second is cheap insurance: it is the client that fails silently when a shared type widens. - `cd ui-tui && npm run lint` -> 0 errors, 23 warnings. The 23 match the pre-change baseline; one is a react-compiler advisory on the poll's dependency-array suppression, which follows the same pattern an existing hook in this tree already uses. - `cd ui-tui && npm run lint:rpc` -> generated.ts in sync. - `scripts/check_commit_messages.py origin/main..HEAD` -> exit 0. - `make check-large-files` -> exit 0. - Driven live in the TUI against real ACP sub-agents: the tail moves through the node's own tool calls and their results, the line reads `working...` before the agent has produced anything, the poll stops when the node finishes, clicking opens the box on the tail of the trace and clicking again restores the line, and the node's box in the graph toggles the same slot. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed Every new test was checked against its own absence: the fix was reverted, the test was watched to fail, and the fix restored. That mattered here - the suite was green while the feature was silently doing nothing, so a passing test is weaker evidence than usual on this change. ## Risk Every DAG panel gains rows: one per running node, twelve for an expanded one. That is the feature, but a live panel is taller than it was. The poll issues one `dag.node` read per running-or-expanded node every 500ms, bounded by a DAG's parallelism, over a local socket. Each response carries that node's transcript, capped server-side. If it proves heavy the narrower request is a tail-only parameter, deliberately not built up front. Three gaps are known and deliberately not addressed here: - A finished node keeps showing its stream line, because the pinned run's status is never revised once its turn closes. The poll stops correctly; only the row's appearance is stale. Fixing it means changing how a completed run reaches the pinned copy, which is outside this change. - `npm run lint:i18n` reports the generated catalogue is stale. This reproduces on main at the merge base, so it is inherited rather than introduced, and fixing it would widen this diff into an unrelated area. - The generated TypeScript for a DAG node's status omits `| null` while the Python model declares it optional. That mismatch is what made the third defect above easy to write: the type said null was impossible. Correcting it means regenerating the schema and both clients, which belongs in its own change. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes No new surface, no secret, no persisted artifact: the trace store is in memory and is cleared when the session resets. Rollback is per commit - the stream line, the trace box, the height terms and the command change are independent. ## Related Issues N/A
## Summary Removes the two packaged builtin playbooks (`deep-dive`, `topic-briefing`) from `raven/playbook/builtin/`. The two-layer library stays exactly as it is: a writable user root over a packaged read-only builtin root. What changes is that the builtin layer now ships empty, so an install carries only what the user wrote on that machine. The packaged layer remains the mechanism a release can use, without demo content landing in every install and in the monthly public release; `pyproject.toml` keeps the `raven/playbook/builtin/**/*.md` package glob for that reason. No production code changes. `load_playbook` was already gated on the library offering something (`loop/main.py`: `if not self._playbooks.empty`), so a fresh install simply does not register the loader until the first user playbook exists; `create_playbook` still registers whenever the feature is on, because an empty library is exactly when capturing the first workflow matters. Tests were updated to pin that gate both ways: the old assertion that `load_playbook` is always registered encoded the shipped-library premise and flipped, and a companion test proves a user-layer playbook opens the loader. Review round (chandler.zhang, blocking): the `PlaybookConfig` description embedded in `model_json_schema()` claimed the builtin layer ships two playbooks and that the loader is always offered. Fixed in `fix(config)` commit: the contract now states the conditional loader and keeps the measured 848-token figure as the populated-library shape. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [x] Other ## Verification - `uv run pytest tests/ -q -p no:randomly` at the pre-rebase head: 10821 passed, 51 skipped, 13 deselected, exit 0 (0 failures). - Post-rebase (onto 7d239509) relevant suites: `tests/test_playbook_store.py tests/test_playbook_compose_prompt.py tests/test_agent_loop_playbook_entry.py tests/test_playbook_tool.py tests/test_playbook_executor.py tests/test_cli_playbook_commands.py` -> 97 passed, exit 0. - `ruff check` and `ruff format --check` on the three changed test files: passed. - Load-bearing probe: with the `if not self._playbooks.empty` gate removed, the empty-library test fails and the populated-library test stays green (restored afterwards; `raven/agent/loop/main.py` is not part of this diff). - `scripts/check_commit_messages.py origin/main..HEAD`: exit 0. - Toolchain provenance: `raven.playbook.store` inside the test run resolved from the worktree checkout, not a shared install. - After the review fix: `pytest tests/test_rpc_schema_match.py tests/test_playbook_store.py tests/test_playbook_compose_prompt.py tests/test_agent_loop_playbook_entry.py tests/test_config_raven_sections.py tests/test_cli_config_precedence.py tests/test_config_loader.py -q -p no:randomly` -> 374 passed, 1 skipped, exit 0. - Contract check: `PlaybookConfig.model_json_schema()` embeds the new passage and no longer contains "ships two playbooks" or "always offered". - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Backward compatibility: the behavior change is that a fresh install no longer offers `load_playbook` until the user layer has a playbook, and `raven playbook run / list` simply report no builtins. An existing user layer is unaffected; an existing playbook that was copied from a builtin under the same name keeps its contents (the store has always resolved user-first). Rollback: restoring the two deleted files returns the previous behavior; no migration, no state, no schema. ## Related Issues N/A Co-authored-by: Claude (deepseek-v4-flash-vision-exp) <noreply@anthropic.com>
## Summary Prevent a failed MCP transport from cancelling the main agent turn during startup. Keep each MCP transport and handshake in one lifecycle so SDK task-group failures unwind into ordinary per-server connection errors. Raven can then continue with the remaining MCP servers and enter the model loop. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed - `uv run pytest tests/test_sandbox_unit.py -x -q` - 70 passed. - `uv run ruff check raven/agent/tools/mcp.py tests/test_sandbox_unit.py` - passed. - A real `raven agent` turn returned `RAVEN_FIXED_OK` after two configured MCP servers failed authentication. - Docs and screenshots are not needed because no user-facing contract changed. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes The change only alters failure isolation during MCP initialization. Roll back the single fix commit if transport lifecycle behavior regresses. ## Related Issues N/A --------- Co-authored-by: zhanghui <23442919+gloryfromca@users.noreply.github.com>
## Summary Eleven defects across the session rail, the floating desk, the sub-agent panes and the turn's product card, all reported from one round of use. Rail and desk placement: - A failed session reported from a leading dot while running and finished reported from the tail slot, so the row's state jumped ends on the one transition a reader watches for. Failed now uses the same slot. - The desk's default geometry hung a 300px palette to the LEFT of its launcher, over the centred transcript. It now hangs under the launcher, flush with the same edge, at 250x260. The geometry key is bumped to v5 so a stored desk does not carry the old corner forward. Panes opening on their own: - `openInstance` handed the row to the workspace and returned without recording what it opened, so the promotion in `refreshInstances` matched again on every heartbeat: the pane reopened every couple of seconds, clearing the reader's fullscreen and stealing the active pane. - A graph node promoted from its record view opened a second pane beside the first. The promoted view now takes the pane it was promoted from, and keeps fullscreen when that pane held it. Sub-agent transcript and chrome: - The agent stage withheld an answer until the record called itself settled, so a node whose list row had aged out never drew its answer at all. It is drawn as it arrives and redrawn in place on each poll. - The pane's reading measure named three segment classes, leaving folds, notices and the products bar against the left edge; a record with no composer still reserved the composer's 150px; and a `:has()` specificity inversion floated the palette a panel-width in from the edge while a pane was fullscreen. Fullscreen now takes the chat's own frame. - A retried model call left one thought row per attempt, which a reload then showed as one. A run of thought-only episodes merges into one row holding every thought and the summed clock. - All desk dividers inherited `pointer-events: none` from the grid, so no seam could be grabbed, and the two-pane seam was half the width it was drawn at. The turn's product card: - The dag card stretched a node graph to 1.7x the size the same graph has in the composer sheet. It draws at its own size and may still shrink to fit. - The single-delivery card carried empty caption to keep a square picture; the column is narrower and the caption tighter, taking the card from 182px to 155px. - A delivered file the workspace saw no write for (a playbook or a sub-agent wrote it on another lane) fell back to a grey icon. The tile now reads its own opening bytes from the URL it already probes. - The miniature was called `.mini`, which is also the page's small button, so it wore that button's border, radius, type, hover background and `white-space: nowrap` -- and the nowrap was inherited by every paragraph in the rendered document, which therefore could not wrap at any width. Renamed to `.amini`, and the miniature is now inset as a page with a faded tail rather than bleeding into the tile's corner. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `npm test --prefix ui` - passed, 57 files and 665 tests (15 added). - `npm run type-check --prefix ui` - passed. - `npm run gen:check --prefix ui` - passed, 132 methods. - `node ui/scripts/check-css.mjs` - passed. - `node ui/scripts/check-page.mjs` - passed. - `make lint-tui` - passed (generated.ts and the i18n catalogue in sync). - `make check-large-files` - passed. - Each new assertion was mutation-checked: reverting the fix it covers turns it red. - The product-card and dag-card items were additionally checked against real session data in a `raven serve` run over a copy of the author's workspace, not only against fixtures. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk User-visible presentation changes only; no RPC, schema or persistence changes. The one stored value that moves is the desk geometry key (v4 to v5), which resets a customised desk position and size to the new default once. Rollback is the squash commit revert. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A
…ities visible (#312) ## Summary A WeChat user asked for a recommendation and the bot answered "the web search tool is not configured here". That sentence was not the model's own phrasing -- it was the tool's error text, relayed outward. `web_search` needs a Serper key and that deployment has never had one, but the tool was registered unconditionally, so on every search-shaped question the model saw it, reached for it, and passed the setup error on to whoever was in the chat. Withholding the tool fixes that and creates a second problem: an unregistered tool is invisible. Nothing in a running Raven then says the capability exists at all -- the model is never offered it, no document lists it, and `raven doctor` reports on providers and memory but has never mentioned tools. The only way to learn that web search is one account and one edit away was to read the source. This does both halves, because either alone leaves a real gap: the deployer is told what this install can and cannot do, and the model is still not offered a tool it cannot run. ### Withholding the unusable tool `web_search` is gated on a resolved key, in the main loop and in the sub-agent surface (`subagent/manager.py`) -- a sub-agent that reaches for a search it cannot run reports the failure to its caller, and that text lands in the parent turn, the same leak one level down. The gate asks the tool, not the config. `WebSearchTool.api_key` resolves at call time from the constructor value *or* `SERPER_API_KEY`, so reading `tools.web.search.apiKey` alone would withdraw a working tool from any deploy that exports the variable and configures nothing. The tool's error message also hard-coded `~/.raven/config.json` while the gateway runs with `--config` elsewhere, so following it meant editing a file the process never reads. It names the path actually in force now. That text is reachable only if the key disappears after registration, which is exactly why it should be right: it is the message for the case the gate cannot cover. ### Three rules, and nowhere to read them Registration is decided per family, each a different shape: | family | rule | |---|---| | `web_search` | a resolved key, asked of the built tool | | `web_fetch` | nothing -- always registered, a key only improves extraction | | media x3 | an `api_key` *or* a `model`, either counting as configured | For media those are two questions, not one. A section naming only a model is registered, because a model alone counts as asking for the tool, and then every call returns a missing-key error. Whether a capability is *offered* and whether it *works* come apart there, and a report treating them as one fact ticks a capability that cannot run. Each rule is defensible where it sits. What is missing is anywhere to read them. Providers had the same sprawl once and answered it with `providers.auth`: a declarative table plus `credential_status` as the single authority, with an AST invariant enforcing that authority, because six surfaces had answered the same question six ways and each looked reasonable alone. Tools never got the equivalent. ### What this adds **Each tool answers for itself,** twice where the questions differ. `WebSearchTool.is_configured` reads the config value *or* `SERPER_API_KEY`, because those are two sources and only the tool consults both. The media base answers `is_configured` on a model *or* a key -- which is what stops an OpenRouter credential set for chat from silently switching on three tools that bill per call -- and answers `has_key` separately on the chain it actually resolves at call time: its own section, the borrowed provider key, then `OPENROUTER_API_KEY`. Both rules live with the credential they read. **`capabilities.py` describes the five** for a human deciding what to set up: what each does in one line, how much work it is (nothing / reuse a credential you already have / obtain an account), where the key goes, where to get one, and what it costs. It rules on nothing -- `is_configured` and `has_credential` both ask the tools -- so it cannot become a second opinion. **`raven doctor` grows a section**, listing every capability configured or not, ordered by how much the deployer has to do: ``` (markers render as a green check and a yellow bang; spelled [ok] and [!] here to keep this description ASCII, since it becomes the squash commit body) Tool capabilities web_fetch: [ok] Read a web page the agent already has the URL for image_generate: [ok] Generate an image (borrowed: providers.openrouter.apiKey) text_to_speech: [!] Generate speech from text no key resolves; calls will fail set: tools.media.speech.apiKey or env: OPENROUTER_API_KEY video_generate: - Generate a video switch on: tools.media.video.model key: reusing providers.openrouter.apiKey Billed per call; needs prepaid OpenRouter credit. web_search: - Search the web set: tools.web.search.apiKey or env: SERPER_API_KEY key from: https://serper.dev 2 capability(s) available but not set up; the agent is not offered them. ``` Three deliberate details. Naming the credential is load-bearing in both directions. A row that cannot distinguish a reused credential from a missing one sends someone to create an account they already have -- and a row that claims a reuse with nothing to reuse is worse, because acting on it means setting a model, getting a registered tool, and watching every call fail on a credential they were told they had. So the reuse line prints only when a key is genuinely there to pick up, and a capability already in that broken state says so outright instead of showing a satisfied tick. The credential is named at the path that holds it. For the media family `config_path` names the *model*, so reusing it as the key source pointed the deployer at a line with no credential in it. And each fact is on its own line rather than in a sentence, because the terminal wraps a long line mid-path and a config key broken across two rows cannot be copied, which is the only thing that row is for. Nothing here moves the exit code, including the warned row. An install without image generation is a choice, not a fault, and a doctor that fails on it teaches people to ignore doctor. The half-finished one is arguable -- the memory section does exit non-zero for a role the user configured that the server could not build -- but that failure is silent where it happens, recall just returning nothing, whereas this one returns an error string to the model on every call. Say the word and it becomes an exit code instead. ### Not in this change Registration still lives in `AgentLoop`. Having it read the table is the point of this shape and removes the last duplicate reader, but it edits `agent/loop/main.py`, which is under active change, and it is worth doing on its own once that settles. Until then the table is a description, and the tests below are what keep it honest. `deep_research` is deliberately absent: it is moving to the sub-agent surface and its tool is going away. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other Mixed on purpose: the first commit is a fix and the rest are the feature it made necessary. Splitting them would land a change that hides a capability without landing the one that makes it discoverable. ## Verification ``` make lint exit 0 make build exit 0 npm run test --prefix ui-tui 969 passed, 13 skipped uv run --all-extras pytest -q 6186 passed, 35 skipped, 1 failed uv run pytest tests/test_tool_capabilities.py \ tests/test_cli_doctor_commands.py \ tests/test_agent_loop_web_tools.py 64 passed uv run --extra dev ruff check raven tests scripts All checks passed uv run --extra dev ruff format --check 828 files already formatted npx commitlint --from origin/main --to HEAD exit 0 scripts/check_commit_messages.py origin/main..HEAD exit 0 ``` The one failure is `test_read_file_image.py::test_an_attachment_that_cannot_be_ read_costs_a_note_not_the_turn`, and it is not from this branch: it fails the same way on `main` at `1cb604a` with these commits absent. The case makes a file unreadable with `chmod 000`, which does not block a root user, so it fails for anyone running the suite as root and passes in CI. This branch does not touch that file or the code under it. The capability tests drive a real `AgentLoop` and compare what it registered against what the table predicts, rather than asserting the table against itself. The gated set is derived -- tools present once credentials are supplied, absent without -- so a sixth gated tool whose author forgets the table fails here rather than going unnoticed. That last claim was false when first written, and the last commit is what makes it true. The fixture listing the media tools by hand never switched a fourth one on, so a new one never joined the gated set and the assertion held over an already-incomplete table. #305, which adds a MiniMax voice-clone tool, is that case: merged against this branch the assertion passed. Read from `MediaGenConfig` instead, it fails and names the tool -- `gated but undeclared: ['voice_clone']`. Which means whichever of the two lands second turns this red, deliberately. The fix is one table entry, and it needs the rule that PR settles: `voice_clone` counts as configured on `api_key or api_base or model` in `effective_media_config` while registration still gates on `api_key or model`, so an `apiBase`-only install with no MiniMax key is configured by one rule and withheld by the other. That is the divergence this table exists to make visible, and it is worth resolving there rather than papering over here. Fifteen mutations, each caught: ``` the web_search gate removed from the main loop 3 failed the same gate removed from the sub-agent surface 1 failed media rule reads only the key 7 failed web_search rule reads only the config 1 failed a tool removed from the table 6 failed the doctor section not rendered 6 failed doctor lists only configured tools 4 failed unconfigured treated as a failure 17 failed the reuse line is printed unconditionally 1 failed the key source falls back to the model path 3 failed a keyless registered capability is not flagged 1 failed has_key always answers yes 1 failed has_credential collapses into is_configured 1 failed the credential chain drops OPENROUTER_API_KEY 3 failed the reuse check ignores the environment 3 failed ``` Four of those tests exist only because a mutation pass found the earlier versions insufficient, and the last rounds are why the shape changed. A media case that sets both a model and an OpenRouter key proves nothing about the "or model" half: the borrow fills the key in, so a rule reading only the key still answers correctly. The case that pins it has nothing to borrow. The sub-agent case is the same argument applied to the second call site: with the gate present only in the main loop, every test above still passes, because nothing was watching what the sub-agent surface registers. More usefully, mutating `has_key` to always answer yes changed no test at all in the first version, which said the ruling was not load-bearing: the doctor was inferring "no credential" from an *empty source string* rather than from the tool's answer, so a fourth credential source would have been reported as a missing one. `has_credential` is now its own fact, asked of the tools, and that mutation fails. Separately, autouse fixtures clear `SERPER_API_KEY` and `OPENROUTER_API_KEY`, without which several of these pass for the wrong reason on any machine where a developer exported one. Rendering was checked against a real config in six states -- nothing set, a provider key present, a model with nothing to borrow, a tool with its own key, a model plus a borrowable key, and an exported variable as the only source -- rather than only asserted on. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk One behaviour change: `web_search` is no longer offered to the model when no key resolves. That is the fix. A deployment that has a key, in config or in the environment, is unaffected; one that has none was getting an error string in place of an answer. The rest is additive. No other registration logic changed, so which tools an agent is offered is otherwise exactly what it was; `is_configured` moved the existing predicates onto the tools without altering them, and the mutation results above are what pins that. `has_key` and `_resolve_key` are new names for the chain `api_key` already resolved -- the property calls the extracted one, so callers see the same answers. `raven doctor` gains a section and no new exit code. Its zero-network guarantee holds: the table reads config and environment only. Two allowlist entries were added to `test_only_the_auth_module_decides_configuredness_from_a_key`, argued in place: `subagent/manager.py`, which asks the built tool whether a key resolved so an unusable search is withheld, and `capabilities.py`, which reads keys to *report* -- which source supplied one, and whether one is there to reuse. Both ruling halves are delegated to the tools, whose files were already listed. Rollback is a revert. Nothing is written and no configuration is read differently. - [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>
## Summary Audit every shell verb that survives, then retire the three whose indirection bought nothing. The remaining-legacy document's Axis 2 asked for the shell verb count to be visible and stopped there. `docs/specs/2026-08-25-shell-verb-audit.md` gives each of the 21 verbs on `3b68ff72` a verdict: 3 retire mechanically, 4 stay by argument, 9 need real work, 4 are gated on a page-chrome migration nobody has scoped. Driving the count to zero is explicitly not what the audit recommends. The code change takes the mechanical three. `openCron`, `openXa` and `openConn` each had a legacy half that was a single line calling straight back into another island, and no target was ever reassigned. The nav flyout rows, the rail's cron group and the settings page's connection links import the owning island instead; the three verbs, their `interface Shell` members and their legacy one-liners are gone. The ratchet falls from 21 to 18. Two findings worth the reviewer's attention, both of which changed a verdict: - `markNew` looks identical to the three and is deliberately kept. `features/rail/store.ts` already imports `shell/navfly`, so retiring the verb would turn a dependency the shell keeps one-way into a literal import cycle. - `closeDetail` reads as a one-line `dataset` write, which made it look like the cheapest row on the page. It is reassigned twice more (`demo/152-skills.js:88`, `demo/153-plugins.js:120`), each wrapping the previous -- a decorator chain that grows a link per page sharing the drawer. It is filed under real work, not mechanical. The nav flyout also gains a per-row test. The existing one picked a single row, so a row wired to the wrong opener, or to none, passed: a mutation making the xa row a no-op failed zero tests before this change and fails one after it. No wire-protocol change, no server change. The served DOM, class names and styles are untouched. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [x] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed Run on the rebased head, after `glhttps/main` moved to `6fdf1d7b`: - `npm run --prefix ui type-check` - `npm test --prefix ui` (57 files, 656 tests) - `npm run gen:check --prefix ui` (132 methods) - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 18) - `npm run --prefix ui build` plus `python3 ui/build.py` - `node ui/scripts/check-page.mjs` - `node ui/scripts/check-css.mjs` - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py glhttps/main..HEAD` - `git diff --check glhttps/main...HEAD`, plus a non-ASCII sweep of the added lines Mutation checks on the new test, because it replaces shell fakes with module mocks and a rewritten test that cannot fail is worse than the one it replaced: - xa row's opener replaced by a no-op: 0 tests failed before the new test, 1 after - cron and conn openers swapped: 1 test failed Both page modes were driven against the built page, since the change deletes legacy globals the offline layer used to define: - live (`raven serve`, real RPC): the three rows open `xaPage`, `connPage`, `cronPage` respectively; no console errors from real interaction - stub (`?stub=1` over the built `dist`): same three mappings, with `window.openCron` and `window.openXa` confirmed `undefined` ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes User-visible surface is three navigation entries plus two settings links, all verified in both page modes. The deleted legacy globals had no remaining callers anywhere in `ui/src` or `page.html`. Rollback is reverting the two commits; the audit document stands on its own and carries no behaviour. ## Related Issues N/A
…sage
## Summary
A session is named today by truncating its first user message to 40 characters, so the picker shows a raw opening line cut mid-word, often a pasted instruction. This asks a model to name it instead.
Key decisions:
- **The call runs beside the opening turn, never on its path.** Fired from `turn.send` after the submit, it reads only the first user message (waiting for the reply would name the answer rather than the request, and would leave the front end's placeholder spinning for a whole turn). Every refusal is silent, because the mechanical title `save()` derives is already in place: disabled by config, an opening message too short to name anything, a timeout, an answer that ignored the instruction, or a rename typed while the call was in flight.
- **No new metadata state.** A generated title carries the same `title_auto` marker the mechanical one does, so fork inheritance is unchanged. An earlier draft added a third marker distinguishing model-made from machine-derived; it was dropped because the human/not-human distinction is the only one any consumer reads.
- **Storage limits and display limits are separated.** The 24-codepoint generation budget is a runaway guard, not a layout rule (measured titles run 2-21 codepoints). `set_title` collapses a title to one line and refuses one past 200 characters rather than truncating it: a person typed that, and keeping the first 200 characters hands back a fragment they never wrote. Display truncation stays with each front end, which already ellipsises in both places.
- **The client-side 30-character cap is deleted, not aligned.** The GUI wrote a truncated first line on send so the rail would not sit on "New task"; the placeholder does that job now, so the constant that disagreed with the backend's 40 is gone rather than synchronised.
- **The GUI placeholder always ends, on a name and not on the default.** It gives up after 12s onto the opening line it captured when the wait began, preferring the stored title when the turn has already ended and written one. A placeholder outliving its call is the one state a reader cannot leave by waiting, and the stored title alone cannot be relied on here: `SessionManager.save` derives it at turn end, so a turn still working at 12s has none, and `refreshList` never re-reads the session the reader is looking at.
- **The TUI shows a resumed session's name** on the init bundle it already receives, and ignores the live event: the panel for that session has scrolled off above the turn that generated the name.
Two things a reviewer should know beyond the diff:
1. A refused rename in the GUI was silently swallowed (`.catch(() => {})`). Harmless while nothing could fail, but `set_title` now can, so the failure is reported and the row rolls back, mirroring `DS.sessions.pin`.
2. This project's pipeline runs two jobs, `tests` and `page`. `ruff`, `tsc`, `vitest` and the three codegen drift checks are not gated by CI at all, so the Verification list below is the only place they were run. Separately, `main` currently fails `make lint-python` on three files this branch does not touch (`raven/agent/tools/deliver.py`, `tests/test_channels_outlet.py`, `tests/test_rpc_files.py`) - real, but CI never sees it.
## Type
- [ ] Fix
- [x] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other
## Verification
Run on this branch after the review round, rebased onto `origin/main` at `01d39e58`:
- `uv run pytest -q --ignore=tests/test_start_webapp.py` - 10859 passed, 43 skipped, 0 failed
- `npm test --prefix ui` - 674 passed (58 files)
- `npm test --prefix ui-tui` - 1600 passed (110 files)
- `npm run type-check --prefix ui` and `--prefix ui-tui` - clean
- `npm run gen:check --prefix ui`, `npm run lint:rpc --prefix ui-tui`, `npm run lint:i18n --prefix ui-tui` - all three checked-in generated artifacts in sync
- `npm run lint --prefix ui-tui` - 23 warnings, none of them in a file this branch touches
- `uv run ruff check` and `ruff format --check` on every Python file in this diff - clean
- `make check-large-files` - clean
`tests/test_start_webapp.py` is excluded above because two of its cases fail on `main` in this environment: the test pins `PATH=/usr/bin:/bin`, so `bash` resolves to macOS 3.2 and `start_webapp.sh` fails to parse. Neither the script nor the test differs from `origin/main` on this branch.
Every new guard was mutation-checked - 17 mutations across both rounds, each confirmed to fail at least one test: the human-title race, the in-flight dedup, conversation-scoped emission, the discard-over-clamp rule, the minimum-input gate, the storage ceiling, both `_needs_naming` gates, resume carrying the title, the GUI timeout fallback, the settle path, the rail placeholder branch, and the TUI panel line. Writing them turned up a real defect: a model answering with an empty quoted string produced a title made of two quote characters.
- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed
## Risk
User-visible change: a new session gets a model-written name instead of its truncated first message, and the GUI shows a shimmer where the title goes until that name lands. Existing sessions are untouched; nothing rewrites a title already on disk.
Cost: one short model call per session, not per turn, against a first message already in context. `session_title.enabled` turns it off and `session_title.model` points it at a cheaper tier.
Rollback: set `session_title.enabled = false`. Sessions then keep the mechanical title exactly as they do today, and the GUI placeholder resolves through its 12s fallback. Reverting the commit is also clean - the only persisted change is the title string itself, written to a field that already existed.
- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes
## Related Issues
N/A
## Summary
The lexical half of RAVEN-20260823-001. Three read-only commands were refused
in one session because their comments said `there's`, `what's` and `raven's`.
`shell_policy.py` turns `#` off in its lexer, so English prose keeps taking
part in quote parsing: the apostrophe opened a quote that never closed, `shlex`
raised `No closing quotation`, and the fail-closed branch answered `hard_deny`.
Reproduced verbatim from the ledger's artifacts before touching anything --
all three `hard_deny`, `_command_segments` raising, and the same commands
without their comments allowed every time.
The user saw only `policy evaluation failed`, asked four times why, and the
model, given nothing to go on, guessed wrong twice.
### Both gates, not one
The same blindness runs the other way and reaches a second gate.
`ExecTool._guard_command` searches the deny list against raw text, so it never
hit the parse failure -- but a denied pattern written inside a comment blocked
the command there just the same:
```
ls -la # unlike dd if=/dev/zero, this one only lists
```
Fixing `ShellCommandPolicy` alone would only have changed which message the
user got. A test pins each gate, and a mutation that reverts one of them is
caught.
### One lexical view
Comments are removed once, by a walk that shares the quote and escape rules
`_split_on_operators` already uses -- those rules are what decide whether a `#`
is a comment at all. Every safety check reads that view: the deny patterns, the
delete and power matchers, the approval families a surface registered, and the
second gate's own list. The raw command still goes to the executor, the audit
trail and the approval prompt, which are about what the user asked for rather
than about what it does.
A `#` opens a comment only outside quoting and at the start of a word.
`foo#bar`, `${#PATH}`, `$#`, `file#1` and `'#!/bin/sh'` are arguments. Cutting
at every `#` would quietly shorten commands, which is how a classifier stops
seeing the half that matters -- pinned with `echo a#b; rm -rf /tmp/tree`, which
must still deny.
### What does not change
Fail-closed. An unterminated quote in the executable region is still refused,
including when a well-formed comment precedes it. `rm -rf` or `shutdown` beside
a comment still denies, on either side of it. A `bash -c` script is still read
through to the command it carries, comments and all.
### Scope
This is deliberately half the ledger entry. The other half -- separating
"refuse this tool call" from "end the whole turn", which today share one
`abort_action` flag across the main loop, the sub-agent loop, tracing and the
UI -- is a change to the `ToolResult` control contract and belongs in its own
MR. This one stops the false refusals without touching that contract.
## Type
- [x] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other
## Verification
```
pytest -q 11056 passed, 83 skipped
pytest tests/test_shell_comments.py -q 26 passed
ruff check / ruff format --check clean
```
Four mutations, each caught:
```
comments are never stripped 5 failed
a `#` anywhere starts a comment 1 failed
a `#` inside quotes starts one too 1 failed
only the policy gate learns the view 1 failed
```
The last is the one worth reading: it is what proves the second gate is
covered rather than assumed.
- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed
## Risk
The change is narrowing what the classifier reads, so the direction that
matters is whether anything dangerous can now hide in a comment. It cannot:
the shell would not run it either, and the executable half is classified
exactly as before. Tests cover a denied pattern on both sides of a comment, a
recursive delete and a power command before and after one, and a wrapper
carrying its own script.
The opposite risk -- over-eager stripping cutting a command short -- is the
reason a `#` must start a word. Six shapes that are not comments are pinned.
No interface changes and no migration. `executable_text` is a new export;
`_guard_command` and `evaluate` keep their signatures. Rollback is a revert.
- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes
## Related Issues
RAVEN-20260823-001, lexical half.
One note for the ledger while this is open: the entry's header carries P1 while
its own impact section concludes P2, on the grounds that the trigger is
bounded by comment style and recoverable by removing the comment. The two
readings imply different things about whether there is a workaround, and the
priority field is what the queue is ordered by.
## Summary Put tests under seven pieces of island-to-legacy wiring that nothing defended, and correct the count that found them. `!268` audited the shell verbs by reading them. This asks a different question by running them: for each verb, `shell()` returns a proxy that no-ops one named member, and the suite runs. A verb whose removal leaves the suite green is one a migration could cut silently. The same trick one layer down, on `ds()`, asks it of the data sources. Three verbs and four source members answered green. Each now has a test, and each test was checked by cutting the call it defends: | wiring | what a silent cut looks like | pinned by | |---|---|---| | `closeSet` | the settings dialog stays up behind the connections page | both cards, separately -- cutting either one fails | | `plugRedraw` | the caps page keeps the tab title and count from before an install | 4 of its 5 sites; `sync()` is not pinned | | `workspaceSetOpen` | the desk's panel never opens, or stays open empty | the guard as well as the calls: closing a pane that is not the last must not shut it | | `settings.model` | the card keeps showing the model that was there before the pick | the new model held outside the snapshot, so the assertion cannot pass without the re-read | | `plugins.reload` | the shelf behind the progress sheet holds the pre-install answer | waits, since the re-read lands a microtask late | | `transcript.okOf` | every tool row shows the wrong verdict | one clean result beside one error: forcing true fails, forcing false also fails | | `transcript.branch` | the branch action never appears | the button and the text it hands over | The last commit corrects the one before it rather than restating it. `agents.watch` was reported undefended and is not: with the tests exactly as they were, cutting the registration fails three of them. The sweep kills a member by proxying what `ds()` returns, and `hook()` reads `window.DS` directly, so the kill never reached the call site and green was read as undefended. Four call sites bypass `ds()` that way and are invisible to that measurement, named in the commit. So "5 of 61 undefended" should be read as **4 real holes out of 60 reachable calls, with four paths unmeasured**. **One gap this branch does not close, recorded here because the squash drops commit bodies.** `branchOf` guards on `lane.main`, and that guard is not pinned: removing `if (!lane.main) return null` leaves all transcript tests green. Building a non-main lane means driving the sub-agent surface, which is a different fixture than any of these tests set up. The hole the sweep named -- the source's `branch` reaching the button -- is closed; the guard beside it is not. The `plugRedraw` gap is the same shape: `sync()` is the fifth call site and is not pinned, only the four the table names. No production behaviour changes. One assertion is added to three existing sub-agent tests; everything else is new tests and recording fakes that were empty functions. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [x] Other Tests, plus one correction to a claim in this branch's own history. ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed On the rebased head, against `origin/main` at `9f9864b2`: - `npm run --prefix ui type-check` - `npm test --prefix ui` (58 files, 682 tests) - `npm run gen:check --prefix ui` (132 methods) - `node ui/scripts/count-shared-globals.mjs` (0 / 0 / 18) - `npm run --prefix ui build` plus `python3 ui/build.py` - `node ui/scripts/check-page.mjs`, `node ui/scripts/check-css.mjs` - `uv run --frozen --python 3.12 --extra dev python scripts/check_large_files.py origin/main..HEAD` - `git diff --check origin/main...HEAD`, plus a non-ASCII sweep of the added lines Every added test was re-checked after the rebase by cutting the call it defends. Seven of eight mutations fail a test; the eighth is `plugRedraw` inside `sync()`, which this branch does not claim to pin. Mutating the four sites it does claim -- `toggleView`, `backToMarket`, `search`, and the settings pair -- each fails. Re-running the verb sweep on this branch reports 0 of 18 verbs cuttable, against 3 before it. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Test-only. The three sub-agent tests changed `poll?.()` to an assertion plus `poll!()`, which makes a missing callback fail where it happens rather than three assertions later. Rollback is dropping the branch. ## Related Issues N/A
Absorbs the GitHub release into the downstream trunk. A merge rather than a rebase so v0.1.13 becomes a real ancestor: the next sync starts from v0.1.13 instead of v0.1.12, and the eventual contribution back to GitHub can still tell our commits from the upstream ones it already has. Cost this round: 51 conflict hunks over 27 files, against 54 over 31 for v0.1.12. The hunks were the cheap half. Upstream's #284 moves the model and provider from process-wide state to a per-session binding, and the trunk had 230 commits standing on the old shape. What git reported as a clean merge and was not --------------------------------------------- Fourteen upstream-changed paths do not exist here, because we renamed or moved them since v0.1.12. Rename detection found none of them, so their changes were merged into paths nothing reads, silently: * raven/tui_rpc/* -> raven/rpc/* (renamed in ce526ad). Nine files, including question_broker.py, whose whole #354 rewrite was lost, and methods/config.py, methods/model.py, methods/session.py, models.py and errors.py. Each was replayed by three-way merge against dd82f52. The three test modules were rebased function by function onto upstream's version instead, after a three-way merge spliced assertions into the wrong tests. * raven/agent/tools/mcp.py -> raven/mcp/client.py. Carried the release-blocking fix, and is handled in the commit that follows this one. * ui-tui/rpc-schema/openrpc.json -> rpc-schema/openrpc.json. Decisions --------- Follow upstream wherever the two disagree about provider and model: * providers/pin.py is gone, so a bare model id is refused instead of derived. A model id does not name whose credential serves it, and the derivation walked a list -- with two vendors configured, which one pays came down to their order in it. rpc/methods/config.py now requires the provider. * ModelSwitchInTurnError (-32009) is gone with it. A session switch lands on that session's next turn, so there is nothing to refuse mid-turn. * AgentLoop.refresh_context_window and the LazyProvider on_built wiring are gone: the window rides on the binding now. * The skill gate no longer forwards an unpaired pin. Keep ours where the trunk is ahead, and graft upstream's behaviour into it: * raven/agent/subagent/backends/raven_loop.py is the live sub-agent path, so manager.py's inline loop stays retired and the web_search gate from #312 applies there. * skill_hub/client.py keeps the public zip limits the skillhub RPC path imports, and takes upstream's query-string cap. * ask_user keeps our JSON-string normalizers and feeds them into upstream's batch contract, so a batch validated by _prepare is one _normalize_questions has already coerced. * The chat view composes both filters: hideIntroAfterFirstTurn drops the cover, visibleRows still hands the view to a direct chat. Tests removed, not repaired --------------------------- Eight tests covered mechanisms this merge deletes, and one of them hung rather than failed: * seven in test_agent_loop_model_switch.py over AgentLoop._pending_provider, the switch-parking mechanism #284 replaced with per-session bindings. Its replacement arrives with upstream as test_agent_loop_session_model.py. * test_a_refusal_during_a_turn_keeps_its_own_code in test_acp_methods.py, whose error class no longer exists. Baseline: the pipeline for 01d39e58, this merge's base, is green, so nothing here can be charged to a pre-existing failure. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…turn Ports the fix from upstream v0.1.13 (#365), which the merge could not deliver: it changed raven/agent/tools/mcp.py, and this tree moved that module to raven/mcp/client.py, so rename detection missed it and the change landed on a path nothing reads. The trunk carried the release-blocking bug while claiming the version that fixed it. The SDK opens streamableHttp inside an anyio task group. Entered into the caller's AsyncExitStack, a transport that dies takes the caller down when that outer stack unwinds -- past every per-server except -- so one MCP server with an expired credential cost the user the whole answer, with no reply at all. _mcp_server_connection owns the transport, the session and the handshake as one lifecycle over its own stack. The failure now unwinds inside that generator and reaches the caller as an ordinary exception, which the per-server handler already catches; the remaining servers still register and the turn reaches the model loop. manager.py, the live path, goes through the same connect_mcp_server and inherits it. Verified in both directions: the new test in test_mcp_client.py fails on the merge commit with the ExceptionGroup escaping, and passes here with the second server registering its tool. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
## Summary Opening the tracing dashboard was unusable on a state dir with a few months of history: the viewer rebuilt its entire snapshot on every request, and the page re-asks every 5 seconds, so it was never idle. Measured on a 3.2 GB tree (324k files, 102k spans, raven actively writing), a poll cost 7.6 s and the response was 325 MB. Four costs on the read path, each fixed here: 1. **The log reader recursed all of `logs/`** to find a few dozen `.log` files, walking the 324k-entry `audit-artifacts/` payload store on the way. Logs only ever land in the active file or `archive/<date>/` -- the only place either writer renames one to -- so the search stays inside those (600 ms -> 4 ms). Candidates are also stat'ed once each rather than inside a sort comparator. 2. **The deposit index walked the everos root per request**, which contains the engine's own virtualenv, and re-read every deposit. Dotted directories that hold no deposits are skipped and the index is reused while the tree is unchanged (433 ms -> 13 ms). The 54 `.md` files no longer visited are all pip-installed package readmes, which the entry parser already rejected. 3. **Nothing cached the snapshot.** The serialized body is now held, keyed on the inputs it was built from, and refreshed *behind* the response instead of making the reader wait -- a running raven appends spans continuously, so a poll that waits for the rebuild waits every single time. Only the body is retained, not the object graph it came from; retaining the graph measured at 2.4 GB resident and grows with history, while the body is bounded by the payload. The refresh is deferred to after the response leaves the socket, because the rebuild is synchronous and would otherwise stall the write it was meant to skip. 4. **The tree nested a copy of every span** beside the spans array, so the payload carried the whole history twice. It now carries ids, depth and nesting, resolved against `trace.spans` in the page. Search shared the disk cost and added its own: the result cap used an unlabelled `break`, which caps nothing past the first trace. A broad term therefore matched 13k spans and read an artifact off disk for each -- 138k reads returning 802 MB of text for one query. The cap now ends the scan, and artifact text is cached per path under a byte ceiling. Results, same tree and the UI's own polling cadence: | | before | after | |---|---|---| | `/api/data`, first request | 7.6 s | 3.3 s | | `/api/data`, subsequent polls | 4.8-7.6 s | 0.27 s | | payload size | 325 MB | 171 MB | | `/api/search` (`tool` / `memory` / `store`) | 19 s / 22 s / 72 s | 3.0 s / 2.6 s / 2.9 s | Not addressed, and deliberately left out: the archives have no retention policy at all, so the reader still reads the whole retained history on a rebuild. A time window would cut this further but removes older sessions from the panel, which is a product decision rather than a performance one. The remaining 2.6 s of a search is the rebuild it triggers; removing that needs an index that survives rebuilds, and two in-memory attempts were rejected here on measurement -- retaining the searchable text of every span exhausted memory, and re-parsing the held body per query was slower than rebuilding. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [x] Other (performance) ## Verification Commands run, and what they reported: - `uv run pytest -q` -- 11040 passed, 33 skipped, 2 failed. Both failures are in `tests/test_start_webapp.py` and reproduce unchanged on the pristine base commit, so they are not from this branch. - `uv run pytest -q tests/integration/test_tracing_viewer_e2e.py -m integration` -- 6 passed. New file; spawns the real Node viewer against a synthetic state dir and drives it over HTTP. - `uv run pytest -q tests/test_cli_tracing_commands.py tests/test_tracing_api.py tests/test_tracing_compact.py tests/test_no_otel_tracing.py` -- 86 passed. - `uv run ruff check` and `ruff format --check` on the new test -- both clean. - `make check-large-files` -- clean. - Each of the six new assertions was mutation-checked: undoing the log scoping, dropping the archive subtree, removing the snapshot cache, pinning the fingerprint constant, nesting whole spans in the tree, flattening the tree, reversing the search sort, and removing the result cap each fail exactly the intended test and nothing else. - The page itself was driven in a browser against the real tree: 181 session cards render, switching sessions redraws 267 span nodes nested to depth 3 with no missing fields, the content search returns 50 highlighted hits, and the console is clean. This matters because the tree change moves work into the page, which no automated gate covers -- `raven/tracing/viewer/*.js` sits outside every lint and type-check target in the Makefile. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk Behaviour changes a reader can see: - Data can be one refresh interval behind while raven is writing, by design. The page already polls, so it converges on the next tick; the e2e test pins that an appended span reaches a later poll. - A log file placed under `logs/` outside the active file and `archive/` is no longer read. Neither writer produces one. - A search for a broad term now returns the newest 50 of the first 200 matches scanned rather than of every match. The scan runs newest-first, so the result set is nearly identical, and the 200 cap was the existing intent. - The viewer holds the serialized payload for as long as it runs -- roughly 650 MB resident when idle on the tree measured here, against 60 MB before. `TRACE_ARTIFACT_CACHE_MAX_BYTES` bounds the artifact cache separately. Rollback is a revert of this commit; the changes are confined to the viewer's read path and add no state, schema or file format. ## Related Issues N/A
Member
Author
|
Round 1 findings (three P1) are fixed in ad9bacc: argv-preserving launch so a TUI exit reopens correctly and no shell expansion occurs, plus file-level docs on the five new files. PR description updated with the round 1 note; all gates pass (23 tests). |
Show the raven.openTui entry as an editor title button (menus.editor/title, navigation group) next to the active file tab, like the OpenCode and Claude Code extensions, and drop the status bar item, its facade surface, and the onStartupFinished activation.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: the code delta is sound, but the PR description must be corrected before merge because it contradicts this revision and becomes the squash commit body.
The description still says the extension uses whitelist-based POSIX/Windows shell quoting, reports 27 tests, records a real-shell quoting smoke test, and says the launch line is shell-quoted. This head removed buildSendText and its quoting tests in favor of direct shellPath/shellArgs; the current suite has 23 tests. The appended round-1 note states the new design, but it leaves the Summary, Verification, and Risk sections internally contradictory. AGENTS.md section 3.7 requires those sections to describe the current overall implementation and exact verification that will land in the squash commit.
Please replace the obsolete shell-quoting and 27-test claims with the direct-argv design and current 23-test/package results. No code change is required for this finding.
Coverage: I reviewed the delta from ad9bacccd803, the current full github/main...HEAD diff, repository rules and domain context, the command contribution and activation path, API facade callers, history, backward compatibility, and test changes. Moving the shortcut from the status bar to menus.editor/title is consistent across the manifest, implementation, facade, tests, and README. The removed test scaffolding only represented the deleted status-bar API; terminal behavior coverage remains, so tests were not weakened to obtain a green result.
Verification:
npm ci --prefix vscode-ext: passed with localEBADENGINEwarnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.- Extension lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 23 tests passed.
npm audit --prefix vscode-ext --audit-level=critical: passed with 0 vulnerabilities.- VSIX packaging: passed; 11 files, 10.93 KB.
- Large-file check, commitlint, repository commit-message checker, and
git diff --check: passed.
make is unavailable in this environment, so I ran the underlying targets directly.
Create the Raven terminal with location: editor so the TUI opens next to the active file tab like the OpenCode extension, instead of the terminal panel. The adapter maps the facade's location flag to viewColumn Beside.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: the previously reported PR-description blocker remains unresolved; I found no new code finding in this revision.
The c5ca365 delta correctly maps the facade's editor location to ViewColumn.Beside, passes that placement from the terminal manager, and updates its tests and README consistently. I rechecked the affected callers, current full diff, repository rules and domain context, history, backward compatibility, and test changes. Existing lifecycle and argv coverage remains intact, so the tests were not weakened to obtain a green result.
Verification:
npm ci --prefix vscode-ext: passed with the same localEBADENGINEwarnings from Node v23.10.0; CI uses Node 22.- Extension lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 23 tests passed.
npm audit --prefix vscode-ext --audit-level=critical: passed with 0 vulnerabilities.- VSIX packaging: passed; 11 files, 11.11 KB.
- Large-file check, commitlint, repository commit-message checker, and
git diff --check: passed.
make is unavailable in this environment, so I ran the underlying targets directly.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: the corrected PR description resolves the prior blocker, but the 0.1.1 bump leaves the committed lockfile on 0.1.0; see the inline note.
Coverage: I reviewed the delta from c5ca36550a4d, the live PR description, the current full github/main...HEAD diff, repository rules and domain context, package metadata and packaging output, affected callers, history, backward compatibility, and test changes. The PR description now accurately reflects the direct-argv implementation and 23-test suite. No tests were weakened.
Verification:
npm ci --prefix vscode-ext: passed with localEBADENGINEwarnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.- Extension lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 23 tests passed.
npm audit --prefix vscode-ext --audit-level=critical: passed with 0 vulnerabilities.- VSIX packaging: passed and produced
raven-vscode-0.1.1.vsix; 11 files, 11.12 KB. - Large-file check, commitlint, repository commit-message checker, and
git diff --check: passed.
make is unavailable in this environment, so I ran the underlying targets directly.
| "name": "raven-vscode", | ||
| "displayName": "Raven Agent", | ||
| "description": "Open the Raven TUI inside the VS Code integrated terminal.", | ||
| "version": "0.1.1", |
Member
There was a problem hiding this comment.
[P1] Keep the lockfile version in sync
This changes the package to 0.1.1, but both the top-level version and packages[""].version in package-lock.json remain 0.1.0. The packaged VSIX is therefore identified as 0.1.1 while the committed lock metadata still identifies the root package as 0.1.0. npm ci does not catch this root-version drift (it passed here), so please regenerate the lockfile through npm so all committed release metadata agrees on 0.1.1.
Member
Author
There was a problem hiding this comment.
Fixed in 48fd061: the lockfile was regenerated through npm, so the top-level version and packages[""].version now match 0.1.1 in package-lock.json.
Regenerate package-lock.json through npm so the committed root version matches the package.json bump. Co-authored-by: Claude <claude-sonnet-4-6> <noreply@anthropic.com>
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior lockfile finding is resolved: package.json, package-lock.json's top-level version, and packages[""].version all read 0.1.1. The new commit changes only those two lockfile metadata fields, with no dependency or integrity churn. The live PR description also accurately records the fix and remains ASCII-only.
Coverage: I reviewed the delta from 176c5a6, rechecked the full current github/main...HEAD diff and live PR description, repository rules (AGENTS.md and CLAUDE.md) and domain context, package metadata, affected callers, branch history, backward compatibility, and test changes. The extension remains additive, and this revision changes no tests, so no tests were weakened to obtain a green result.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 23 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.1.vsix contained 11 files and was 11.12 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, repository files, commit messages, PR title, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
Probe the login shell environment (bash/zsh on posix, powershell.exe/pwsh on Windows) and merge it into the Raven terminal so proxy settings and API keys from the user's shell rc apply inside VS Code, fixing OpenRouter regional 403s that only appeared when launching raven through the extension. The probe is cached after the first open.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: the Windows environment probe skips the user's PowerShell profile, and the new commits have made the PR description stale again; see the inline note and summary below.
The live description still says version 0.1.1, 23 tests, and a 0.1.1 VSIX, while this revision is 0.1.2 with 31 tests. More importantly, its Risk section says the extension handles no credentials, but this head deliberately copies proxy settings and API keys into the Raven terminal. Because the description becomes the squash commit body, AGENTS.md section 3.7 requires the Summary, Verification, and Risk sections to describe this current behavior and exact results. Please update those claims before merge.
Coverage: I reviewed the delta from 48fd061, rechecked the full current github/main...HEAD diff and live PR description, AGENTS.md, CLAUDE.md, CONTEXT-MAP.md and the TUI glossary, the extension entry point, path probing, VS Code API adapter, terminal manager and tests, branch history, backward compatibility, package metadata, and whether tests were weakened. The eight new tests are additive and existing coverage remains, although the Windows test currently asserts the defective -NoProfile invocation rather than validating a profile-defined variable.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 31 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.2.vsix contained 11 files and was 11.99 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, repository files, commit messages, PR title, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
| const script = | ||
| '[Console]::OutputEncoding=[System.Text.Encoding]::UTF8; Get-ChildItem Env: | ForEach-Object { "$($_.Name)=$($_.Value)" }' | ||
| for (const shell of ['powershell.exe', 'pwsh']) { | ||
| const result = runner(shell, ['-NoProfile', '-NonInteractive', '-Command', script], { |
Member
There was a problem hiding this comment.
[P1] Load the PowerShell profile before reading its environment
-NoProfile explicitly prevents PowerShell from loading $PROFILE. For a GUI-launched VS Code where an API key or proxy variable is assigned in that profile, this subprocess only repeats the environment VS Code already had, so Raven still misses the value and the advertised Windows behavior is ineffective. The test currently locks in this flag instead of exercising a profile-defined variable. Please probe the user's actual profile-loading shell environment and cover that case.
Member
Author
There was a problem hiding this comment.
Fixed in 1e73eef: -NoProfile removed so the probe subprocess loads the user PowerShell profile, keeping -NonInteractive to avoid prompts; timeout raised to 15000 for profile startup cost. The test now asserts the profile-loading invocation.
Drop -NoProfile so the env probe subprocess loads the user's PowerShell profile, where proxy and API key variables live; keep -NonInteractive to avoid prompts and raise the timeout for profile startup cost.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: removing -NoProfile fixes the prior finding for Windows PowerShell, but the probe still bypasses PowerShell 7 profiles, and the previously reported PR-description blocker remains unresolved; see the inline note.
The live Summary, Verification, and Risk sections still contain the already-reported 0.1.1, 23-test, and no-credentials claims. Appending a round note does not correct the current-state sections that become the squash commit body, so that blocker remains.
Coverage: I reviewed the delta from f2a30d8, rechecked the full current github/main...HEAD diff and live PR description, AGENTS.md, CLAUDE.md, CONTEXT-MAP.md and the TUI glossary, path probing and its callers, the VS Code API adapter, terminal lifecycle, branch history, backward compatibility, package metadata, and test changes. The modified test accurately verifies removal of -NoProfile, and no existing test was weakened, but it does not cover selecting the user's actual PowerShell edition/profile.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 31 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.2.vsix contained 11 files and was 11.97 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, repository files, commit messages, PR title, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
| function probeWindowsEnv(runner: ProbeRunner): Record<string, string> | null { | ||
| const script = | ||
| '[Console]::OutputEncoding=[System.Text.Encoding]::UTF8; Get-ChildItem Env: | ForEach-Object { "$($_.Name)=$($_.Value)" }' | ||
| for (const shell of ['powershell.exe', 'pwsh']) { |
Member
There was a problem hiding this comment.
[P1] Select the user's actual PowerShell profile
On a normal Windows host, powershell.exe is present and this command returns a nonempty environment, so the loop exits before ever trying pwsh. A user who has PowerShell 7 configured in VS Code and defines the proxy or API key in that edition's distinct $PROFILE still gets the Windows PowerShell profile instead, leaving Raven without the value. Please derive the configured/default shell (or otherwise select the correct edition) and cover the PowerShell 7 profile case.
Member
Author
There was a problem hiding this comment.
Fixed in 466e9aa: the Windows env probe now derives the VS Code-configured shell from terminal.integrated.defaultProfile.windows / profiles.windows, probes that edition first, and merges powershell.exe and pwsh environments with the preferred edition winning conflicts. Tests cover the PowerShell 7 profile case.
…ofiles Derive the user's Windows shell from terminal.integrated.defaultProfile.windows and probe that edition first, then merge the powershell.exe and pwsh environments with the preferred edition winning conflicts, so profile-defined variables from PowerShell 7 reach raven even when Windows PowerShell also succeeds.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: explicit path-based PowerShell 7 profiles now win correctly, but source-based and auto-detected VS Code profiles still lose their edition hint, and the previously reported PR-description blocker remains unresolved; see the inline note.
The live Summary, Verification, and Risk sections still contain the already-reported 0.1.1, 23-test, and no-credentials claims. Appending another review-round note does not correct those current-state sections, which become the squash commit body.
Coverage: I reviewed the delta from 1e73eef, rechecked the full current github/main...HEAD diff and live PR description, AGENTS.md, CLAUDE.md, CONTEXT-MAP.md and the TUI glossary, VS Code's terminal-profile configuration contract, path probing and configuration callers, the API adapter, terminal lifecycle, branch history, backward compatibility, package metadata, and test changes. The merge-precedence tests are additive and existing tests were not weakened, but they inject a literal path directly and do not exercise shellHint's configuration boundary.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 32 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.2.vsix contained 11 files and was 12.25 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, repository files, commit messages, PR title, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
| const config = api.workspace.getConfiguration('terminal.integrated') | ||
| const defaultProfile = config.get<string>('defaultProfile.windows') | ||
| const profiles = config.get<Record<string, { path?: string }>>('profiles.windows') | ||
| return (defaultProfile && profiles?.[defaultProfile]?.path) || undefined |
Member
There was a problem hiding this comment.
[P1] Resolve source-based and auto-detected profiles
VS Code supports Windows profiles declared as { source: "PowerShell" } and auto-detects default profiles without requiring a literal path (official profile documentation). For those common PowerShell 7 configurations, this expression returns undefined; probeWindowsEnv then uses its generic order and lets powershell.exe win conflicts, so the PowerShell 7 $PROFILE value is still lost. The new test bypasses this boundary by passing a path directly. Please resolve the actual configured/default executable for source-based and auto-detected profiles and cover that configuration path.
Member
Author
There was a problem hiding this comment.
Fixed in 8463ac1: the shell hint now comes from vscode.env.shell, which VS Code itself resolves from terminal.integrated.defaultProfile including source-based and auto-detected profiles; the manual profiles.windows path lookup remains only as a fallback. resolveWindowsShellHint is a pure function with tests covering env.shell preference, path fallback, and source-based profiles without a path.
Use vscode.env.shell, which VS Code resolves from defaultProfile including source-based and auto-detected profiles, as the preferred Windows shell hint; the manual profiles.windows path lookup stays as a fallback. Covers PowerShell 7 setups declared without a literal path.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
Blocking: the source-based/auto-detected profile defect is fixed and I found no new code issue, but this commit has made the PR description's exact verification stale again.
vscode.env.shell is the detected extension-host shell and is overridden by terminal.integrated.defaultProfile, so it supplies the resolved executable that the prior implementation lacked. The fallback remains available when that value is empty, and the new pure-function tests cover its precedence.
The live Verification and Summary still report 32 tests and a 12.25 KB VSIX, while this revision actually has 36 passing tests and packages 11 files at 12.33 KB. Because these current-state sections become the squash commit body and AGENTS.md section 3.7 requires exact commands and results, please update those two figures before merge.
Coverage: I reviewed the delta from 466e9aa, rechecked the full current github/main...HEAD diff and live PR description, AGENTS.md, CLAUDE.md, CONTEXT-MAP.md and the TUI glossary, the VS Code env.shell and terminal-profile contracts, API adapter and configuration callers, path probing, terminal lifecycle, branch history, backward compatibility, package metadata, and test changes. The four added tests are additive and no existing tests were weakened.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 36 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.2.vsix contained 11 files and was 12.33 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
Member
Author
|
Verified against the live description: the Verification and Summary sections now report the current figures (3 files / 36 tests, VSIX 11 files / 12.33 KB). The remaining mentions of 32 tests are inside the historical Round 7 note, which describes the state at that revision. No code change was needed for this round. |
Member
|
Confirmed. The live current-state Summary and Verification now report 3 files / 36 tests and an 11-file / 12.33 KB VSIX. The remaining 32-test text is clearly historical Round 7 context, so it does not contradict this revision. I reran |
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The live PR description now resolves the only remaining blocker: its current-state Summary and Verification accurately report version 0.1.2, 3 files / 36 tests, and an 11-file / 12.33 KB VSIX, while the 32-test mention is correctly scoped to historical Round 7 context. No code changed since my review of this head, where the vscode.env.shell fix resolved source-based and auto-detected PowerShell profiles without introducing a new code issue.
Verification on this answer turn: npm test --prefix vscode-ext passed all 36 tests, and git diff --check github/main...HEAD passed.
Sanitize the extraArgs setting at the config boundary: non-array values become an empty list and only string entries survive, so a misconfigured string no longer spreads into single-character argv entries. Add sanitizeExtraArgs with unit tests.
gloryfromca
reviewed
Aug 29, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new config-boundary sanitizer fixes the reported runtime type hazard: a scalar raven.extraArgs value now becomes an empty list, mixed arrays cannot pass non-string values into terminal argv, and valid string arrays preserve their order and contents. The live PR description accurately reflects this revision's 40-test and 12.41 KB package results.
Coverage: I reviewed the delta from 8463ac1, rechecked the full current github/main...HEAD diff and live PR description, AGENTS.md, CLAUDE.md, CONTEXT-MAP.md and the TUI glossary, the extension configuration boundary, Raven command construction and callers, package schema, branch history, backward compatibility, package metadata, and test changes. The four sanitizer tests are additive and no existing test was weakened.
Verification:
- npm ci, lint, type-check, test, build, and formatting checks: passed; Vitest reported 3 files and 40 tests passed. npm ci emitted local EBADENGINE warnings because Node v23.10.0 is outside two dependencies' supported even-major ranges; CI uses Node 22.
- npm audit --audit-level=critical: passed with 0 vulnerabilities.
- VSIX packaging: passed; raven-vscode-0.1.2.vsix contained 11 files and was 12.41 KB.
- Large-file check, commitlint, repository commit-message checker, and git diff --check: passed.
- Current GitHub checks are green for the extension, TUI, bridge, Windows self-upgrade, lint, repository files, commit messages, PR title, and pre-commit diff; the unrelated Python unit job was still pending when reviewed.
…ify headroom (#372) ## Summary Onboard model-default refreshes plus an EverOS startup fix: 1. Rerank default: Qwen/Qwen3-Reranker-4B has been retired from OpenRouter, so the onboard wizard's rerank example and EN/ZH recommendations now point to qwen/qwen3-reranker-8b, which OpenRouter still serves. 2. Stale defaults: the memory-LLM capability floor (gpt-4.1-mini), the multimodal recommendation (google/gemini-3-flash-preview), and the Azure OpenAI provider default (gpt-5.2-chat) all point at generations behind what OpenRouter/Azure serve today. They now read qwen/qwen3.8-flash (the EverOS client always sends temperature, which the GPT-5.x reasoning family rejects), google/gemini-3.7-flash, and gpt-5.6-sol. GPT-5.x Azure deployments force reasoning_effort="none" on tool calls, matching Microsoft's tool-calling requirement; configured effort still applies to non-tool calls. 3. Startup failure: an exhausted fs.inotify.max_user_instances makes the spawned EverOS server's watcher die at boot with "OSError: [Errno 24] inotify instance limit reached", which surfaces only as a cryptic exit buried in the dead-child log. ensure_everos_server now measures per-user inotify headroom (counting same-uid inotify instances via /proc/<pid>/fd links that read anon_inode:inotify against the kernel cap) before spawning: it raises the cap when the process is privileged, otherwise it fails fast with the exact sudo sysctl commands. A dead child whose log blames inotify gets the same hint appended to its error. ## Type - [x] Fix ## Verification - make coverage (full default suite, uv run --frozen --python 3.12 --all-extras pytest -q): 7119 passed, 2 failed, 37 skipped, 13 deselected. The 2 failures are pre-existing on main and environment-dependent (reproduced on main in a throwaway worktree): both expect a chmod-based write to be refused, which cannot happen when the suite runs as root (CAP_DAC_OVERRIDE): - tests/test_config_loader.py::test_migration_is_correct_even_when_the_file_cannot_be_written - tests/test_everos_server.py::TestAnUnwritableRootFailsAsAStartFailure::test_it_surfaces_as_runtime_error - make coverage-diff COVERAGE_BASE_REF=origin/main: 100.00% (57/57 executable changed lines), passed the 90.00% threshold - make coverage-ratchet: passed (line +2.11pp, branch +3.24pp over baseline) - uv run pytest tests/test_cli_onboard_commands.py: 251 passed - uv run pytest tests/test_azure_openai_provider.py: 9 passed - uv run ruff check (changed files): clean - uv run ruff format --check (changed files): clean - pre-commit commitlint hook (conventional commit message): passed on all commits - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally ## Risk - Security impact considered: the sysctl write runs only when the process already holds the privilege to write /proc/sys; otherwise the gate only returns the instructions. - Backward compatibility considered: the gate is a no-op when headroom exists or when procfs/sysctl cannot be read (non-Linux); the model-default changes are wizard text and one provider default constant, no config migration needed. - Rollback path is clear: revert the merge; the check is isolated to ensure_everos_server and three module-level helpers. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A
## Summary The Feishu adapter's reconnect loop retried on a fixed 5s sleep with no backoff and no attempt cap. lark-oapi's ws Client.start() blocks while the connection lives (the SDK reconnects transient drops internally with its own 120s interval) and only returns by raising, almost always a ClientException such as bad credentials or connection-limit exceeded. The old loop therefore hammered Feishu's auth endpoint every 5s on permanent failures. Changes: - Retries now use exponential backoff capped at 300s (5s initial, factor 2): 5s, 10s, 20s, 40s, 80s, 160s, 300s. - ClientException is logged as a permanent error instead of a generic warning. - The backoff sleep runs in 1s slices so stop() can interrupt a long backoff promptly (no lingering thread). Three tests added covering ladder growth/cap, ClientException handling, and stop() interrupting the sleep. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed Commands and results: - `uv run pytest tests/test_channels_feishu.py -q` -> 38 passed (1 warning is pre-existing, from test_stop_blocks_zombie_inbound). - `uv run ruff check raven/channels/adapters/feishu/channel.py tests/test_channels_feishu.py` -> All checks passed. - `uv run ruff format raven/channels/adapters/feishu/channel.py tests/test_channels_feishu.py` -> 2 files reformatted (applied). - pre-commit hooks all passed, including the conventional commit message check. No user-facing docs affected: this is an internal retry-cadence change with no config or interface change. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes No security surface change: only retry timing is affected. No config, CLI, or API change; existing deployments behave the same apart from slower retries after repeated connection failures. Rollback: revert the single commit. ## Related Issues N/A --------- Co-authored-by: Claude (deepseek-v4-pro) <noreply@anthropic.com>
gloryfromca
reviewed
Aug 31, 2026
gloryfromca
left a comment
gloryfromca
left a comment
Member
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This head only merges the latest main into the already-reviewed feature branch. I compared the old and new branch ranges: all 15 feature commits are patch-identical, and the merge has no conflict-resolution delta. I also reread the current github/main...HEAD diff and the affected extension entry point, command resolution, environment probing, terminal lifecycle, repository wiring, and tests.
Covered: AGENTS.md / CLAUDE.md / CONTEXT-MAP.md and the TUI context rules, the full PR diff, affected callers and history, backward compatibility, and whether tests were weakened. I found no new issue.
Verification on 3b90f78: npm ci; lint; type-check; 40/40 tests; build; formatting; npm audit (0 vulnerabilities); VSIX packaging (11 files, 12.41 KB); large-file check; commitlint; repository commit-message check; and git diff --check all passed. Live CI has all reported checks green except the unrelated Python unit job, which is still pending; the VS Code extension job passed.
gloryfromca
force-pushed
the
feat/vscode_tui_extension
branch
from
September 11, 2026 17:43
3b90f78 to
810ff21
Compare
Member
This PR was auto-closed by a history rewrite, not by a maintainerOn 2026-09-12 the Your work is safe. The source branch of this PR was rewritten onto the new To continue: open a new PR from the same branch. Please reference this PR number so the discussion stays traceable. The pre-rewrite history is preserved at the tag Sorry for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds vscode-ext, a VS Code extension that opens the Raven TUI in an editor-area tab next to the active file, mirroring the OpenCode extension model.
Post-review UI updates: the launch button lives in the editor title bar (menus.editor/title) instead of the status bar, the TUI opens as an editor-area tab (viewColumn Beside), and the version is 0.1.2 for marketplace publication.
Type
Verification
npm run lint (pass), npm run type-check (pass), npm run build (pass), npm run fmt (pass), npm test (3 files, 40 tests pass)
npm audit --audit-level=critical (pass, 0 vulnerabilities)
npm run package builds raven-vscode-0.1.2.vsix (11 files, 12.41 KB)
commitlint and scripts/check_commit_messages.py over the branch (pass)
CI on the branch: VS Code extension checks, TUI checks, bridge checks, commit messages, pull request title, repository files, python lint, Windows self-upgrade, and pre-commit diff all pass; the unrelated Python unit job was still pending at review time
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
No existing behavior changes: the extension is a new top-level package and the repo wiring only adds targets, hooks, and one CI job. Arguments reach raven through argv, so the extension performs no shell quoting. The extension launches raven with the user's login shell environment merged in, which can include proxy settings and API keys defined in that shell's profile; this mirrors what a direct terminal run would have and is the intended behavior. Rollback is deleting vscode-ext/ and reverting the wiring commit.
Related Issues
N/A
Review rounds (gloryfromca):
Round 1 (three P1 findings): fixed in ad9bacc. Raven now launches as the terminal process itself (shellPath/shellArgs), so a normal TUI exit closes the terminal and the next Raven: Open TUI recreates it, and arguments reach raven through argv without host-shell expansion on any platform; buildSendText was dropped with its tests. The five new files missing file-level purpose docs (ravenBin.test.ts, terminal.test.ts, pathProbe.test.ts, eslint.config.mjs, vitest.config.mts) now carry them.
Round 2: no blockers, can merge.
Rounds 3-4 (stale PR description): corrected here; Summary, Verification, and Risk now describe the direct-argv design and the current 23-test suite. LivXue confirmed all three round-1 findings fixed in ad9bacc. No code change was required for this round.
Round 5 (P1 lockfile drift): fixed in 48fd061. package-lock.json still identified the root package as 0.1.0 while package.json read 0.1.1, so the packaged VSIX was 0.1.1 but the committed lock metadata said 0.1.0. The lockfile was regenerated through npm (npm install) so both the top-level version and packages[""].version in package-lock.json now agree on 0.1.1. npm run lint / type-check / test / build / fmt pass.
Review round 6 (P1 PowerShell profile): fixed in 1e73eef. The Windows env probe no longer passes -NoProfile, so powershell.exe loads the user profile before the environment is read; -NonInteractive avoids prompts and the timeout covers profile startup cost.
Review round 7 (P1 PowerShell edition): fixed in 466e9aa. The probe reads terminal.integrated.defaultProfile.windows to select the user's actual PowerShell edition, probes it first, and merges both editions with the preferred one winning, so PowerShell 7 profile variables reach raven.
Round 7 follow-up (stale description): the Summary, Verification, and Risk sections above were corrected to the current 0.1.2 / 32-test / merged-login-env state so the squash commit body matches this revision; gates re-ran on 466e9aa (lint, type-check, test: 3 files / 32 tests, build, fmt, audit, and raven-vscode-0.1.2.vsix packaging).
Review round 8 (P1 source-based / auto-detected profiles): fixed in 8463ac1. The Windows shell hint now comes from vscode.env.shell, which VS Code itself resolves from terminal.integrated.defaultProfile including source-based and auto-detected profiles (no literal path needed); the manual profiles.windows lookup remains only as a fallback inside the pure resolveWindowsShellHint helper, which has direct unit coverage. Gates re-ran on the new head: lint, type-check, test: 3 files / 36 tests, build, fmt, audit, and raven-vscode-0.1.2.vsix packaging; CI extension, TUI, bridge, Windows self-upgrade, pre-commit diff, and python lint checks pass.
LivXue review (two findings): both fixed, verified on the current head.
Gates re-ran after both fixes: npm run lint, type-check, test (3 files, 36 tests), build, and fmt all pass.
Review round 9 (docs bug sweep #7): fixed in bfb1b7d. raven.extraArgs is now sanitized at the config boundary (sanitizeExtraArgs), so a misconfigured string cannot spread into single-character argv entries; only string entries of an array survive. Live figures updated to 40 tests and a 12.41 KB VSIX.