Repository navigation
feat(*): type the approval request and let no approval wait forever - #617
Conversation
31e4b1a to
37fdce8
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: show when approval evidence was truncated before allowing the request.
I found one blocking correctness and safety issue inline.
Coverage: I reviewed the full diff, the approval gate/broker lifecycle and callers, both frontend request/response/recovery paths, all six branch commits, compatibility fallbacks for older frames/responders, whether deadline-related tests were weakened, and the Runtime/Web UI/TUI architecture rules in CONTEXT-MAP.md and the relevant context docs.
Verification:
uv run pytest tests/test_approval_broker.py tests/test_rpc_approval.py tests/test_permissions_gate.py tests/test_filesystem_tools.py tests/test_shell_approval.py tests/test_core_runtime_swap.py tests/test_rpc_spine.py tests/test_mcp_client.py tests/test_config_update.py tests/test_acp_permissions.py tests/test_contracts_two_tier_ledger.py -x: 719 passed.- Full Web UI suite: 2482 passed; type-check and generated RPC checks passed.
- Approval-focused TUI suite: 50 passed; type-check and generated i18n/RPC checks passed.
- Full TUI suite: 2053 passed, 13 skipped, 2 failed in untouched picker tests (
newInstancePicker.test.tsxandsubagentsHub.test.tsx); both failures were quick-key selection expectations, outside this diff.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: surface truncated approval evidence before it can be authorized.
The existing inline finding remains unresolved on this revision. The only delta from the previously reviewed head is formatting cleanup; GateSheet.tsx still renders the clipped diff/input branches without reading evidence.truncated. No new findings arose from the delta.
Coverage this round: the intervening commit and history, the prior finding against the current file, affected callers and compatibility behavior, test changes, and the applicable repository/context rules.
Verification:
- Approval/runtime Python suites: 719 passed.
- Affected Web UI suites: 65 passed; type-check passed.
- Approval-focused TUI suites: 50 passed; type-check passed.
The approval prompt reached the page as one English sentence plus a truncated argument line, and the request behind it died on a 35-second deadline the web never drew. Both halves are replaced. The request is typed. approval.request carries `kind` (shell.exec, file.write, mcp.call, unknown), `family` (the shell command family that words the prompt), `origin` (the main agent or a named sub-agent) and `evidence`, the tool's own account of the call: a command and where it would run, a path and the diff the write would make, an MCP tool and its input. Each tool describes itself through the new `Tool.approval_kind` and `Tool.approval_evidence`; the gate only reads them, and a hook that fails falls back to the arguments under the unknown layout. `created_at`, `expires_at` and `action_digest` leave the frame: nothing read them. No approval can wait forever, and none is lost in silence. The broker serializes requests per conversation, so a background sub-agent asking while the main agent's prompt is up waits its turn instead of replacing it on screen with nobody having answered. It keeps the requests it sent and answers approval.pending, so a reloaded or reconnected page draws them again, and one belonging to another conversation lights a line above the composer. A day-long floor replaces the 35-second ceiling. The TUI countdown and its expiry state go with it. The web sheet is redrawn from those facts: title and reason by family, evidence block by kind, three answers (deny by default, allow always with the rule it would save, allow once), and a landed line after the answer that can take a saved rule back (approval.revoke), carry a sentence on as the next message, or say so when the engine never took the answer. A digit typed into a field is text rather than an answer, and only the topmost sheet reads the keyboard. A rule that reached the config file gets no session grant beside it, so taking it back really does mean being asked again, unless a stricter rule of the user's own outranks it: there the grant stays and the same call is not asked twice. Contract tier 28 -> 29: `Tool` grows the two hooks and `ApprovalResponder.await_approval` the five keyword arguments. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Found by re-reading the change without the reviewers, which the org quota cut short. Two comments still told a reader an approval disappears 35 seconds after it was raised; a third place hardcodes the default agent's name with no reason beside it, and a reviewer would rightly ask. The rail row's and the phase notice's reason for existing survives the deadline going away, but it is a different reason: a row nobody notices used to mean a lapsed request, and now means a conversation that never finishes. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…wrote Two defects a reviewer found in the recovery and the undo this change added, both of them authority crossing a boundary it was not meant to. `approval.pending` answered every socket with every conversation's open requests, opaque ids included. A second surface could take one of those ids and call `approval.respond` with it, authorizing a command it was never asked about: the boundary `connection.conversation_scoped` draws for the push was not drawn for the pull. It is now. A connection is handed the requests for conversations it owns, plus the ones nobody owns, which were broadcast to every attached surface anyway and which hiding would strand. The undo matched on the rule's text, which is the wrong identity twice over. A rule the reader had written themselves, which `_persist` merely kept, was removed by an undo that never created it. And the undo could run before the write existed: measured, `approval.respond` answers the page first and the gate persists after, so a quick undo found nothing, reported a failure, and the rule then landed and stayed. The gate now reports what its write actually did, the undo names the answer rather than the rule and waits for that word, and a grant's receipt is read once so a second undo cannot reach a rule some later prompt wrote under the same text. Contract tier 30: `ApprovalOutcome` carries the id of the request it answered, which is what lets the gate's report be about one grant. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The lanes map carried a debt note and the grants map beside it did not, though it has the same shape: an entry per saved rule nobody undid, kept for the life of the process. One note now covers both, with the reason the ceiling is acceptable (a person's clicks in one process, a few dozen bytes each) and what to do if either shows up in a heap. Also says why the receipt is dropped on the timeout path as well: a gate that reports late must find no slot, so a rule cannot be removed long after its reader gave up. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…till owns Two ways the change could still strand a turn or reach past a boundary, both found by review of the finished branch. A confirm request arriving on the same conversation swept the gate's pending ask off the screen with nobody having answered it. Clarify's sweep already spared it; this one did not, and it is the same wait with the same cause: a clarify ends on its own after ten minutes and a confirm on its own hard limit, while the gate's ask has only a day-long floor under it. The sheet that no clock retires now says so on itself, because the class and the asks flag do not -- the confirm sheet wears both -- and the two sweeps that can reach one read that mark instead of each carrying its own copy of the rule. approval.revoke took an id and nothing else. approval.pending had just been scoped to the conversations a connection owns; the undo beside it, which writes to the config file, had no such test, so a socket holding an id could take back a rule written on a conversation it was never shown. The receipt now remembers whose conversation it was raised on and the method asks the same question pending asks, on the way out instead of the way in. An unclaimed conversation stays readable to both, which is the broadcast fallback they already share. Both tests were run against the code with the fix removed: the sweep one fails at one sheet instead of two, the undo one at ok=True with the rule gone. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Three more things review found on the finished branch, all of them about what happens when the easy path does not hold. Evidence had a ceiling on one path only. The filesystem tools cap what they read off disk, but a brand-new file's whole text went through the diff whichever size it was, an MCP call's arguments went whole, and so did the fallback's raw params. Measured, a sixteen-megabyte new file spent two thirds of a second in difflib on the event loop, and the approval behind it now waits a day rather than half a minute. The cap moves to where every tool's account of itself passes on its way to a prompt, so a tool cannot forget to have one, and what was cut is marked -- a prompt showing half a change has to say so. The undo spent its receipt by reading it, so a removal that failed on the way to disk could not be tried again: the rule was still there and the only record of whose it was had gone. Reading it now says what to remove and a separate word says the removal happened. A second undo still cannot reach a rule some later prompt wrote under the same text, because success is what spends it. The line that reports the failure names the rule, which is what the reader needs to delete it by hand. The sheet left after a refusal carries a field, and its twelve seconds ran from when it appeared. A reader typing a longer sentence watched it vanish with the words in it. Every keystroke now puts that clock back. Also: both surfaces that declare the command families now call one function instead of each writing out which matchers those are; two comments still described the deadline this change removed; and the shared context file said the saved rule is not editable, which is true of the page and not of the terminal. Each test was run against the code with its fix removed. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Formatting only, no behaviour. Two blank lines where ruff wants one after the import block, one call the removed countdown left too long for prettier, a blank line left behind by the deleted expiry test, and a trailing space. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Review found the half of the cap that was missing. The gate clips an oversized account of a call down to what a person reads and sets `truncated`, but the sheet rendered the clipped diff, input and arguments without ever reading that flag. The reader saw a change that looked whole, and answered about the part they could see. Every layout now carries the line, below the evidence, where the reader has just run out of it -- and in amber rather than the faint grey of "there was nothing to show", because this says the opposite: there is more than this. Run against the code with the line removed: the new case fails on the first layout it checks. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
37fdce8 to
30c571b
Compare
|
Fixed in 30c571b. You were right, and the gap was exactly where you put it:
Verification:
Separately, this revision is rebased onto |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior truncation finding is fixed: every evidence layout now discloses when the engine shortened its payload, and the tests cover all four layouts plus the untruncated case. No new findings.
Reviewed the current diff and the delta from the preceding revision, relevant callers and history, backward compatibility, test integrity, and the repository's AGENTS.md, CONTEXT, and architecture constraints. The rebase-only test-file delta comes from the updated main branch rather than this feature.
Verification:
uv run pytest tests/test_approval_broker.py tests/test_rpc_approval.py tests/test_permissions_gate.py tests/test_filesystem_tools.py tests/test_shell_approval.py tests/test_core_runtime_swap.py tests/test_rpc_spine.py tests/test_mcp_client.py tests/test_config_update.py tests/test_acp_permissions.py tests/test_contracts_two_tier_ledger.py -x: 719 passed- TUI approval tests: 50 passed; type-check, i18n generation, and RPC generation checks passed
- Web UI full suite: 2,483 passed; type-check, generated RPC contract, and class-namespace checks passed
|
Not a blocker -- three things I measured on ( 1. A
|
## Summary A conversation reached disk only when its turn finished, and the session store knows a session only by its file. So a conversation whose first turn was still running was in no listing: the rail kept it as an unsaved row only while it stayed the open one, a refresh of the list (an approval or a question arriving does that) dropped it once the reader had switched away, and a reload lost it outright while the turn kept running with nobody watching. Six commits (plus a merge of `main`, whose #617 moved the generated RPC clients): 1. `fix(agent)`: the question is written as soon as the prompt is assembled and before the first model call, through the same `_save_turn` that wrote it at turn end, so it is filed exactly once with its arrival timestamp. The three closing writes (healthy, cancelled, failed) start after it. `prev_len` is read before the opening write so the memory-backend slice and the output-limit marker still count from this turn's start, and the history handed to hooks is sliced there so it still ends with the previous turn, as documented. The session namer now runs before the `message.start` emit, otherwise the filed question made it decline to name a new session. 2. `feat(rpc)`: `running` on `SessionListItem` and `SessionInitInfo`, read from the active-turn table, so a page that has just loaded can tell. Both generated clients regenerated. 3. `feat(ui-web)`: the rail badges a running row from the list answer and lets that answer clear a stale badge; opening a conversation whose resume bundle says it is running puts the composer back into the streaming, cancellable state, so the stop button works, a send queues instead of being refused, and the deltas still to come land in a fresh step. What already streamed before the reload is not recoverable and is not pretended at. 4. `fix(*)`: the reload race the review found. `turn.subscribe` now answers `running`, read on the far side of the emitter's replay publish, so a completion that would make it stale is one the new subscription itself receives; the page keeps the resume hint (it pre-arms the machine so the replayed `message.start` does not draw the question twice) and, once subscribed, re-opens the conversation from disk when the hint said running but the subscription says otherwise while the machine is still busy. The offline fixtures answer the new `running` fields, which the fixture-shape gate had been failing on. 5. `fix(ui-web)`: found in the owner's acceptance pass -- after a reload the live row's clock restarted from zero. The resume path now anchors the live clock and the runtime's start stamp to the last user message's timestamp (the question is on disk from turn start, so it carries the turn's arrival), through the transcript's own stamp parser exposed via its seam. 6. `fix(*)`: from the reviewer's timezone finding and an adversarial pass over the served page. The resumed clock had been anchored to the question's stored stamp parsed in the browser; the loop writes that stamp off the gateway's wall clock with no offset, so a browser in another timezone read the offset back as the turn's age. `session.resume` now answers `running_ms`, measured on the server against its own clock (naive against `datetime.now()`, aware against `datetime.now(tz)`), and the page anchors at `Date.now()` minus that. The re-open of the fourth commit had landed on the kept-turn branch: `park()` keeps a busy machine and leaves `rt.events` set, so the second pass restored the streaming phase instead of reading the conversation off disk, and the stop button stood over a finished answer for the life of the tab -- reachable on every Stop that overlaps a reload, because `turn.cancel` emits its error frame before the active slot clears. The re-open now disarms the machine and the anchor first and tells the second pass the turn is over. A `run` badge that came from `session.list` never came off: the on-screen turn end now clears the current row's badge, and while any listed row says somebody else is answering, the page asks for the list again every five seconds, stopping when none is left. `running` was false for every turn `turn.send` did not start, so a cron or channel conversation answering for a minute listed as idle; `session.list` and `session.resume` now ask the scheduler as well (`is_session_answering`). Known limits after this PR, to follow up separately: a cron or channel turn is listed and badged while it runs, but opening it shows no stop button or live clock, because the spine emits no opening frame for a scheduler-origin turn, so `turn.subscribe` cannot say it is running (making it read the scheduler would re-open the Stop-overlaps-reload window). A message queued while a turn runs is still page-local and lost by a reload (rarely reached once #622 injects instead of queueing). A reload within about two seconds of a send boots a rail without the conversation until the next list. A gateway that dies mid-turn leaves the filed question with no answer and nothing on screen saying so: the wire has no `interrupted` flag on a transcript row and the page no renderer for one. Behaviour visible elsewhere: every consumer of the session list (TUI picker, `raven sessions list`, `session.most_recent`, the heartbeat's delivery target) now sees a conversation from the moment its turn starts rather than when it ends, and a row moves to the top of the list on send rather than on answer. Every turn costs one more locked append. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_agent_loop_turn_open_persist.py tests/test_agent_loop_broken_turn.py tests/test_agent_loop_iteration_hooks.py tests/test_session_manager.py tests/test_rpc_session.py tests/test_rpc_session_init_bundle.py tests/test_rpc_turn_session_naming_wiring.py tests/test_rpc_turn_send.py tests/test_rpc_session_naming.py tests/test_rpc_schema_match.py -q` -- 299 passed, and the same again with an empty `HOME`. New cases: the session is on disk and listed from inside the first provider call; the question is filed once and the answer follows it; a cancelled and a failed turn each leave the question once with what had streamed and the end marker; the slice handed to the memory backend still opens on the question; a hook's `session_history` stops before this turn's question; a session holding only its open question lists as a complete row with an auto title; naming still starts when the question is already filed; `session.list`, `session.resume` and `session.create` report `running`. The on-disk and naming cases fail against the previous `turn_path.py` / `turn.py`; the once-only and history cases fail under the single mutation each guards. - `uv run pytest tests/ -q` -- 24334 passed, 119 skipped. - `npm test --prefix ui-web -- src/features/rail src/state/session` -- 234 passed (new: a listed `running` row carries the badge; the server's answer clears a stale run badge while the reader's own marks stay; a resumed running conversation puts the turn machine into streaming/cancellable and repaints; a resumed idle one stays idle). - `uv run pytest tests/test_rpc_turn_subscribe.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py -q` -- 413 passed (new: `turn.subscribe` says running while a turn is buffered, not running on a quiet session, not running once the turn completed; red against the previous `turn.py`). `npm test --prefix ui-web -- src/state/session/registry.test.ts` -- 24 passed (new: resume running + subscribe running stays streaming; resume running + subscribe not running re-opens and ends idle, red against the previous `registry.ts`; resume not running + subscribe running is left to the replayed `message.start`). - `uv run pytest tests/test_rpc_session.py tests/test_rpc_turn_send.py tests/test_rpc_turn_cancel.py tests/test_rpc_turn_subscribe.py tests/test_rpc_schema_match.py -q` -- passes (new: resume of a busy session carries `running_ms` measured against a monkeypatched clock, the same for a naive and an offset-bearing stamp; idle answers no elapsed and an unparsable stamp null; `session.list` and `session.resume` say running for a lane the scheduler holds that `turn.send` never saw, and not for a different lane). Red against the previous `session.py`. - `npm test --prefix ui-web -- src/state/session` -- passes (new: the re-open reads the conversation back off disk with the REAL residency module in the harness -- second `session.resume` issued, transcript re-read, machine idle; the badge poll asks again after 5 s, clears when the answer says not running and stops; the visible turn end takes a listed `run` badge off the row; a resumed turn anchors to `Date.now() - running_ms`, and a bundle without it leaves the clock alone). Each red against the previous `registry.ts` / `pipeline.ts`. - `npm test --prefix ui-web` -- whole suite green, the contract gates included; `npm run type-check --prefix ui-web`, `npm run lint --prefix ui-web`, `npm run gen:check --prefix ui-web`, `npm run lint:rpc --prefix ui-tui`, `uv run ruff check raven tests` -- clean. - `npm run type-check --prefix ui-web`, `npm run lint --prefix ui-web`, `npm run gen:check --prefix ui-web`, `npm run lint:rpc --prefix ui-tui`, `uv run ruff check raven tests` -- clean. - Served page, second pass: a Stop fired from another connection while the page reloads, detected by the page's own second `session.resume` -- 2 of 14 attempts landed in the band and both ended idle with the question, the partial answer and the stop note (before: stuck with the stop button up); a background conversation's badge clears within the same sample the server turns idle (before: still on after 140 s); the badge on the conversation being watched clears at the turn's end; browsers in America/New_York and Asia/Tokyo read 11 s and 16 s of elapsed at 12 s and 17 s of wall time (before: 0 s and 60m17s); a cron turn lists and badges as running while it runs and clears at its end. - Served page, isolated agent home, a turn that sleeps in a shell tool: before the fix nothing is on disk or listed until the turn ends, and a reload lands on the new-task screen with no row; after the fix the file exists within a second of the send (metadata plus the question), `session.list` carries the key with one message and `running: true`, a reload lists the conversation with its badge, opening it shows the question and the stop button, the answer lands when the turn ends, and the JSONL holds the question exactly once; a cancelled turn also leaves the question once, followed by its end marker. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] 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 The turn path gains one write per turn on every surface, channels and cron included; a gateway that dies mid-turn now leaves a one-message conversation instead of nothing. The `running` field has a default, so older clients ignore it. `running_ms` is optional and defaults to null, so older clients ignore it. The listed-badge poll costs one `session.list` every five seconds only while a row shows another client answering. Rollback: revert the commits in reverse order. ## Related Issues N/A --------- Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…rity (#650) ## Summary A direct chat is answered by a named sub-agent instance, and that instance's tools are the ones that reach the permission gate. The approval frame has carried who is asking since it was typed -- `origin`, with a kind and a name, written at `rpc/spine.py` whenever the turn has a direct target. The web sheet reads it and titles itself "Allow auditor to delete files?". The terminal dropped it: `ApprovalReq` had no field for it at all, so the box a person answers named the command and never the asker. In a direct chat with a built-in sub-agent called `auditor`, the name appeared on the status line outside the box and nowhere inside it -- the one place a person decides whether to hand over authority. The frame's origin now reaches the box: ``` Approval needed - asked by auditor - Delete files using a shell command rm target.txt ``` Shown only for a sub-agent. The main agent asking is the ordinary case, and printing its name on every prompt would say nothing, so that header is unchanged. Found by running the acceptance item rather than reading it. The behaviour had a contract test that passed throughout, because it asserts on the responder's request dict -- one layer below either surface. The frame was right the whole time; the terminal never spent it. ## Type - [x] Fix ## Verification Driven in a terminal (tmux) against an isolated RAVEN_HOME, on the built-in sub-agent roster: ``` /new-instance auditor -> direct chat auditor/auditor-2e1366 "delete target.txt with rm" -> Approval needed - asked by auditor - Delete files... same request from the main agent -> Approval needed - Delete files... (unchanged) ``` Suites and gates: ``` ui-tui vitest run -> 143 files, 2071 passed ui-tui type-check / lint / lint:i18n / lint:rpc -> all clean ui-web vitest run -> 192 files, 2578 passed (includes the 38 gates) ui-web gen:check / type-check / lint -> all clean pytest tests/test_install_script.py -> 32 passed (it watches this catalogue) scripts/check_source_language.py github/main..HEAD -> rc=0 ``` Both new behaviours were reverse-verified by retracting their own half: ``` retraction red green the frame's origin is not carried 1 (carries a direct chat's name) 15 the prompt does not draw the segment 1 (names the sub-agent on the box) 15 ``` - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally ## Risk One extra segment on one header, and only when the frame says a sub-agent asked. A frame without `origin` (an older runtime, or any path that does not set it) takes the same branch as the main agent and renders exactly what it rendered before. Rollback is the one conditional in the header. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues #617 Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
…617) ## Summary The approval prompt used to reach the page as one English sentence plus a truncated argument line, with a 30 s countdown the web never drew and a 35 s deadline that silently retired the sheet. This makes the request typed and lets the sheet be drawn from facts instead of prose: - `approval.request` now carries `kind` (shell.exec / file.write / mcp.call / unknown), `family` (the shell command family that words the prompt), `origin` (the main agent or a named sub-agent) and `evidence` (the tool's own account of the call: the command and its directory, a path and the diff the write would make, an MCP tool and its input). Each tool describes itself through the new `Tool.approval_kind` / `Tool.approval_evidence`; the gate only reads them, and caps what they return, so no tool can send more than a person reads. `created_at`, `expires_at` and `action_digest` leave the frame: nothing read them. - No pending approval can block forever, and none is lost silently. The broker serializes requests per conversation (a background sub-agent asking while the main agent's prompt is up waits its turn instead of replacing it), keeps a day-long floor instead of the 35 s ceiling, and answers `approval.pending` so a reloaded or reconnected page draws the open requests again; a request in another conversation lights the line above the composer. No sweep takes a pending approval down unanswered: a clarify ends on its own after ten minutes and a confirm on its own hard limit, so both may be replaced, while the gate's ask has only that floor under it. The TUI countdown and its expiry state are removed. - The runtime and the ACP editor declare the same seven shell command families through one call, so the terminal, the page and the channels word a delete as a delete. - Web sheet redesign: title and why-line by family, evidence block by kind, three answers (deny, always with the suggested rule shown and where it applies, allow once), a landed line after the answer, undo for a saved rule (`approval.revoke`), a sentence field after a refusal that goes on as the next message, and a line above the composer when another conversation is waiting. - A rule that reached the config file gets no session grant beside it, so taking the rule back really does mean being asked again -- unless the user's own stricter rule outranks it (the strictest matching rule wins), in which case the session grant stays and the identical call is not asked twice. - Recovery and undo are scoped to the surface that asked. `approval.pending` and `approval.revoke` each answer only for the conversations a connection owns, plus the unclaimed ones whose frames are broadcast anyway, so an id alone cannot authorize a command or delete a rule on a conversation that socket was never shown. The undo names the answer rather than the rule's text and waits for the gate to report what its write did, because `approval.respond` answers the page before the gate persists. A removal that fails on the way to disk leaves the receipt unspent, so it can be pressed again; the line that reports the failure names the rule. - The landed line says so when the engine did not take the answer (`approval.respond` returned `ok: false` or the socket dropped); a digit typed into a field is text, not an answer; and the sentence field stays up while it is still being typed into. This replaces PR #608, which targeted `refactor/ui_web_architecture`. That work is now on `main`, so the five commits were replayed onto it and #608 was closed. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - Rebased from `refactor/ui_web_architecture` onto `main` (`git rebase --onto`), then onto the newer `main` tip again. One conflict, in `raven/core/runtime.py`: `main` had rewritten the hooks block this change appends after, resolved by keeping both. The generated web client was regenerated rather than hand-merged. - `uv run --frozen --python 3.12 --all-extras pytest -q` -- full suite. 3 failed, 24615 passed, 111 skipped in 4:40. The three are pre-existing: a `git worktree` at the `main` tip with none of this change runs the same three files and fails the same three (`3 failed, 95 passed`). - `cd ui-web && npx vitest run` -- 189 files / 2482 tests pass; `npx tsc --noEmit -p tsconfig.json` clean; `npm run gen:check` in sync (194 methods); `node scripts/check-class-namespace.mjs` clean; `npm run build && python3 ui-web/build.py` green (both booted-DOM goldens match at 236 nodes). - `cd ui-tui && npx vitest run` -- 143 files / 2068 tests pass; `npm run type-check`, `npm run lint:rpc`, `npm run lint:i18n` clean. - `scripts/check_source_language.py` and `scripts/check_large_files.py` clean over this PR's range; `npx commitlint` over the same range exits 0. - Five independent review passes over the finished branch (completeness, correctness, appropriateness, maintainability, evidence). Every behavioural finding is fixed and listed under Risk; each fix was re-run against the code with the fix removed, and fails there on the symptom it names. - Real host: `raven serve` on an isolated `RAVEN_HOME` in ask mode with deepseek-v4-pro, driven with playwright-cli; the TUI from source in tmux against the same home. Delete: titled as a delete, allowed once, files gone. Edit: diff shown, refused, note typed after the refusal arrived as the next message, file untouched. Always-allow wrote `permissions.tools.exec["glab mr list *"]`, undo removed it, and a rule the reader had written themselves was left alone. A pending approval in another conversation shows the waiting line; clicking it switches back; a reload draws the sheet again. A main-agent request and a background sub-agent request two seconds apart arrived one after the other, never side by side. - The real-host runs found two things the unit tests could not: the dock passes pointer events through, so the waiting line needed `pointer-events: auto` before a real click reached it; and the waiting line filtered out a conversation whose very first turn was blocked, which is not on disk yet and so not in the list. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk - A pending approval no longer expires in 35 s; it waits for the person, up to a day. Reloading the page redraws it; another conversation's request is announced above the composer. Two requests in one conversation are answered one at a time, in order. - Wire: `approval.request` gains four fields and drops three; the TUI reads none of the dropped ones any more. `approval.revoke` and `approval.pending` are new methods, both scoped to the calling connection. ACP's `session/request_permission` is unchanged. - A saved rule that a stricter user rule outranks no longer re-asks within the session (the session grant stays); before this change the same click would have re-asked on the next identical call. - One tool's evidence is cut at 16 KiB per value and marked when cut, so a very large write or MCP payload now shows a prompt that says it was shortened rather than sending the whole thing. - Contract tier version 28 -> 30 (`Tool` and `ApprovalResponder` shape, then `ApprovalOutcome` carrying the id of the request it answered). - Rollback: revert the commits; no data migrates. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: KT <74288668+0xKT@users.noreply.github.com>
## Summary A conversation reached disk only when its turn finished, and the session store knows a session only by its file. So a conversation whose first turn was still running was in no listing: the rail kept it as an unsaved row only while it stayed the open one, a refresh of the list (an approval or a question arriving does that) dropped it once the reader had switched away, and a reload lost it outright while the turn kept running with nobody watching. Six commits (plus a merge of `main`, whose #617 moved the generated RPC clients): 1. `fix(agent)`: the question is written as soon as the prompt is assembled and before the first model call, through the same `_save_turn` that wrote it at turn end, so it is filed exactly once with its arrival timestamp. The three closing writes (healthy, cancelled, failed) start after it. `prev_len` is read before the opening write so the memory-backend slice and the output-limit marker still count from this turn's start, and the history handed to hooks is sliced there so it still ends with the previous turn, as documented. The session namer now runs before the `message.start` emit, otherwise the filed question made it decline to name a new session. 2. `feat(rpc)`: `running` on `SessionListItem` and `SessionInitInfo`, read from the active-turn table, so a page that has just loaded can tell. Both generated clients regenerated. 3. `feat(ui-web)`: the rail badges a running row from the list answer and lets that answer clear a stale badge; opening a conversation whose resume bundle says it is running puts the composer back into the streaming, cancellable state, so the stop button works, a send queues instead of being refused, and the deltas still to come land in a fresh step. What already streamed before the reload is not recoverable and is not pretended at. 4. `fix(*)`: the reload race the review found. `turn.subscribe` now answers `running`, read on the far side of the emitter's replay publish, so a completion that would make it stale is one the new subscription itself receives; the page keeps the resume hint (it pre-arms the machine so the replayed `message.start` does not draw the question twice) and, once subscribed, re-opens the conversation from disk when the hint said running but the subscription says otherwise while the machine is still busy. The offline fixtures answer the new `running` fields, which the fixture-shape gate had been failing on. 5. `fix(ui-web)`: found in the owner's acceptance pass -- after a reload the live row's clock restarted from zero. The resume path now anchors the live clock and the runtime's start stamp to the last user message's timestamp (the question is on disk from turn start, so it carries the turn's arrival), through the transcript's own stamp parser exposed via its seam. 6. `fix(*)`: from the reviewer's timezone finding and an adversarial pass over the served page. The resumed clock had been anchored to the question's stored stamp parsed in the browser; the loop writes that stamp off the gateway's wall clock with no offset, so a browser in another timezone read the offset back as the turn's age. `session.resume` now answers `running_ms`, measured on the server against its own clock (naive against `datetime.now()`, aware against `datetime.now(tz)`), and the page anchors at `Date.now()` minus that. The re-open of the fourth commit had landed on the kept-turn branch: `park()` keeps a busy machine and leaves `rt.events` set, so the second pass restored the streaming phase instead of reading the conversation off disk, and the stop button stood over a finished answer for the life of the tab -- reachable on every Stop that overlaps a reload, because `turn.cancel` emits its error frame before the active slot clears. The re-open now disarms the machine and the anchor first and tells the second pass the turn is over. A `run` badge that came from `session.list` never came off: the on-screen turn end now clears the current row's badge, and while any listed row says somebody else is answering, the page asks for the list again every five seconds, stopping when none is left. `running` was false for every turn `turn.send` did not start, so a cron or channel conversation answering for a minute listed as idle; `session.list` and `session.resume` now ask the scheduler as well (`is_session_answering`). Known limits after this PR, to follow up separately: a cron or channel turn is listed and badged while it runs, but opening it shows no stop button or live clock, because the spine emits no opening frame for a scheduler-origin turn, so `turn.subscribe` cannot say it is running (making it read the scheduler would re-open the Stop-overlaps-reload window). A message queued while a turn runs is still page-local and lost by a reload (rarely reached once #622 injects instead of queueing). A reload within about two seconds of a send boots a rail without the conversation until the next list. A gateway that dies mid-turn leaves the filed question with no answer and nothing on screen saying so: the wire has no `interrupted` flag on a transcript row and the page no renderer for one. Behaviour visible elsewhere: every consumer of the session list (TUI picker, `raven sessions list`, `session.most_recent`, the heartbeat's delivery target) now sees a conversation from the moment its turn starts rather than when it ends, and a row moves to the top of the list on send rather than on answer. Every turn costs one more locked append. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_agent_loop_turn_open_persist.py tests/test_agent_loop_broken_turn.py tests/test_agent_loop_iteration_hooks.py tests/test_session_manager.py tests/test_rpc_session.py tests/test_rpc_session_init_bundle.py tests/test_rpc_turn_session_naming_wiring.py tests/test_rpc_turn_send.py tests/test_rpc_session_naming.py tests/test_rpc_schema_match.py -q` -- 299 passed, and the same again with an empty `HOME`. New cases: the session is on disk and listed from inside the first provider call; the question is filed once and the answer follows it; a cancelled and a failed turn each leave the question once with what had streamed and the end marker; the slice handed to the memory backend still opens on the question; a hook's `session_history` stops before this turn's question; a session holding only its open question lists as a complete row with an auto title; naming still starts when the question is already filed; `session.list`, `session.resume` and `session.create` report `running`. The on-disk and naming cases fail against the previous `turn_path.py` / `turn.py`; the once-only and history cases fail under the single mutation each guards. - `uv run pytest tests/ -q` -- 24334 passed, 119 skipped. - `npm test --prefix ui-web -- src/features/rail src/state/session` -- 234 passed (new: a listed `running` row carries the badge; the server's answer clears a stale run badge while the reader's own marks stay; a resumed running conversation puts the turn machine into streaming/cancellable and repaints; a resumed idle one stays idle). - `uv run pytest tests/test_rpc_turn_subscribe.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py -q` -- 413 passed (new: `turn.subscribe` says running while a turn is buffered, not running on a quiet session, not running once the turn completed; red against the previous `turn.py`). `npm test --prefix ui-web -- src/state/session/registry.test.ts` -- 24 passed (new: resume running + subscribe running stays streaming; resume running + subscribe not running re-opens and ends idle, red against the previous `registry.ts`; resume not running + subscribe running is left to the replayed `message.start`). - `uv run pytest tests/test_rpc_session.py tests/test_rpc_turn_send.py tests/test_rpc_turn_cancel.py tests/test_rpc_turn_subscribe.py tests/test_rpc_schema_match.py -q` -- passes (new: resume of a busy session carries `running_ms` measured against a monkeypatched clock, the same for a naive and an offset-bearing stamp; idle answers no elapsed and an unparsable stamp null; `session.list` and `session.resume` say running for a lane the scheduler holds that `turn.send` never saw, and not for a different lane). Red against the previous `session.py`. - `npm test --prefix ui-web -- src/state/session` -- passes (new: the re-open reads the conversation back off disk with the REAL residency module in the harness -- second `session.resume` issued, transcript re-read, machine idle; the badge poll asks again after 5 s, clears when the answer says not running and stops; the visible turn end takes a listed `run` badge off the row; a resumed turn anchors to `Date.now() - running_ms`, and a bundle without it leaves the clock alone). Each red against the previous `registry.ts` / `pipeline.ts`. - `npm test --prefix ui-web` -- whole suite green, the contract gates included; `npm run type-check --prefix ui-web`, `npm run lint --prefix ui-web`, `npm run gen:check --prefix ui-web`, `npm run lint:rpc --prefix ui-tui`, `uv run ruff check raven tests` -- clean. - `npm run type-check --prefix ui-web`, `npm run lint --prefix ui-web`, `npm run gen:check --prefix ui-web`, `npm run lint:rpc --prefix ui-tui`, `uv run ruff check raven tests` -- clean. - Served page, second pass: a Stop fired from another connection while the page reloads, detected by the page's own second `session.resume` -- 2 of 14 attempts landed in the band and both ended idle with the question, the partial answer and the stop note (before: stuck with the stop button up); a background conversation's badge clears within the same sample the server turns idle (before: still on after 140 s); the badge on the conversation being watched clears at the turn's end; browsers in America/New_York and Asia/Tokyo read 11 s and 16 s of elapsed at 12 s and 17 s of wall time (before: 0 s and 60m17s); a cron turn lists and badges as running while it runs and clears at its end. - Served page, isolated agent home, a turn that sleeps in a shell tool: before the fix nothing is on disk or listed until the turn ends, and a reload lands on the new-task screen with no row; after the fix the file exists within a second of the send (metadata plus the question), `session.list` carries the key with one message and `running: true`, a reload lists the conversation with its badge, opening it shows the question and the stop button, the answer lands when the turn ends, and the JSONL holds the question exactly once; a cancelled turn also leaves the question once, followed by its end marker. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] 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 The turn path gains one write per turn on every surface, channels and cron included; a gateway that dies mid-turn now leaves a one-message conversation instead of nothing. The `running` field has a default, so older clients ignore it. `running_ms` is optional and defaults to null, so older clients ignore it. The listed-badge poll costs one `session.list` every five seconds only while a row shows another client answering. Rollback: revert the commits in reverse order. ## Related Issues N/A --------- Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…rity (#650) ## Summary A direct chat is answered by a named sub-agent instance, and that instance's tools are the ones that reach the permission gate. The approval frame has carried who is asking since it was typed -- `origin`, with a kind and a name, written at `rpc/spine.py` whenever the turn has a direct target. The web sheet reads it and titles itself "Allow auditor to delete files?". The terminal dropped it: `ApprovalReq` had no field for it at all, so the box a person answers named the command and never the asker. In a direct chat with a built-in sub-agent called `auditor`, the name appeared on the status line outside the box and nowhere inside it -- the one place a person decides whether to hand over authority. The frame's origin now reaches the box: ``` Approval needed - asked by auditor - Delete files using a shell command rm target.txt ``` Shown only for a sub-agent. The main agent asking is the ordinary case, and printing its name on every prompt would say nothing, so that header is unchanged. Found by running the acceptance item rather than reading it. The behaviour had a contract test that passed throughout, because it asserts on the responder's request dict -- one layer below either surface. The frame was right the whole time; the terminal never spent it. ## Type - [x] Fix ## Verification Driven in a terminal (tmux) against an isolated RAVEN_HOME, on the built-in sub-agent roster: ``` /new-instance auditor -> direct chat auditor/auditor-2e1366 "delete target.txt with rm" -> Approval needed - asked by auditor - Delete files... same request from the main agent -> Approval needed - Delete files... (unchanged) ``` Suites and gates: ``` ui-tui vitest run -> 143 files, 2071 passed ui-tui type-check / lint / lint:i18n / lint:rpc -> all clean ui-web vitest run -> 192 files, 2578 passed (includes the 38 gates) ui-web gen:check / type-check / lint -> all clean pytest tests/test_install_script.py -> 32 passed (it watches this catalogue) scripts/check_source_language.py github/main..HEAD -> rc=0 ``` Both new behaviours were reverse-verified by retracting their own half: ``` retraction red green the frame's origin is not carried 1 (carries a direct chat's name) 15 the prompt does not draw the segment 1 (names the sub-agent on the box) 15 ``` - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally ## Risk One extra segment on one header, and only when the frame says a sub-agent asked. A frame without `origin` (an older runtime, or any path that does not set it) takes the same branch as the main agent and renders exactly what it rendered before. Rollback is the one conditional in the header. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues #617 Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
Summary
The approval prompt used to reach the page as one English sentence plus a truncated argument line, with a 30 s countdown the web never drew and a 35 s deadline that silently retired the sheet. This makes the request typed and lets the sheet be drawn from facts instead of prose:
approval.requestnow carrieskind(shell.exec / file.write / mcp.call / unknown),family(the shell command family that words the prompt),origin(the main agent or a named sub-agent) andevidence(the tool's own account of the call: the command and its directory, a path and the diff the write would make, an MCP tool and its input). Each tool describes itself through the newTool.approval_kind/Tool.approval_evidence; the gate only reads them, and caps what they return, so no tool can send more than a person reads.created_at,expires_atandaction_digestleave the frame: nothing read them.approval.pendingso a reloaded or reconnected page draws the open requests again; a request in another conversation lights the line above the composer. No sweep takes a pending approval down unanswered: a clarify ends on its own after ten minutes and a confirm on its own hard limit, so both may be replaced, while the gate's ask has only that floor under it. The TUI countdown and its expiry state are removed.approval.revoke), a sentence field after a refusal that goes on as the next message, and a line above the composer when another conversation is waiting.approval.pendingandapproval.revokeeach answer only for the conversations a connection owns, plus the unclaimed ones whose frames are broadcast anyway, so an id alone cannot authorize a command or delete a rule on a conversation that socket was never shown. The undo names the answer rather than the rule's text and waits for the gate to report what its write did, becauseapproval.respondanswers the page before the gate persists. A removal that fails on the way to disk leaves the receipt unspent, so it can be pressed again; the line that reports the failure names the rule.approval.respondreturnedok: falseor the socket dropped); a digit typed into a field is text, not an answer; and the sentence field stays up while it is still being typed into.This replaces PR #608, which targeted
refactor/ui_web_architecture. That work is now onmain, so the five commits were replayed onto it and #608 was closed.Type
Verification
Rebased from
refactor/ui_web_architectureontomain(git rebase --onto), then onto the newermaintip again. One conflict, inraven/core/runtime.py:mainhad rewritten the hooks block this change appends after, resolved by keeping both. The generated web client was regenerated rather than hand-merged.uv run --frozen --python 3.12 --all-extras pytest -q-- full suite. 3 failed, 24615 passed, 111 skipped in 4:40. The three are pre-existing: agit worktreeat themaintip with none of this change runs the same three files and fails the same three (3 failed, 95 passed).cd ui-web && npx vitest run-- 189 files / 2482 tests pass;npx tsc --noEmit -p tsconfig.jsonclean;npm run gen:checkin sync (194 methods);node scripts/check-class-namespace.mjsclean;npm run build && python3 ui-web/build.pygreen (both booted-DOM goldens match at 236 nodes).cd ui-tui && npx vitest run-- 143 files / 2068 tests pass;npm run type-check,npm run lint:rpc,npm run lint:i18nclean.scripts/check_source_language.pyandscripts/check_large_files.pyclean over this PR's range;npx commitlintover the same range exits 0.Five independent review passes over the finished branch (completeness, correctness, appropriateness, maintainability, evidence). Every behavioural finding is fixed and listed under Risk; each fix was re-run against the code with the fix removed, and fails there on the symptom it names.
Real host:
raven serveon an isolatedRAVEN_HOMEin ask mode with deepseek-v4-pro, driven with playwright-cli; the TUI from source in tmux against the same home. Delete: titled as a delete, allowed once, files gone. Edit: diff shown, refused, note typed after the refusal arrived as the next message, file untouched. Always-allow wrotepermissions.tools.exec["glab mr list *"], undo removed it, and a rule the reader had written themselves was left alone. A pending approval in another conversation shows the waiting line; clicking it switches back; a reload draws the sheet again. A main-agent request and a background sub-agent request two seconds apart arrived one after the other, never side by side.The real-host runs found two things the unit tests could not: the dock passes pointer events through, so the waiting line needed
pointer-events: autobefore a real click reached it; and the waiting line filtered out a conversation whose very first turn was blocked, which is not on disk yet and so not in the list.Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
A pending approval no longer expires in 35 s; it waits for the person, up to a day. Reloading the page redraws it; another conversation's request is announced above the composer. Two requests in one conversation are answered one at a time, in order.
Wire:
approval.requestgains four fields and drops three; the TUI reads none of the dropped ones any more.approval.revokeandapproval.pendingare new methods, both scoped to the calling connection. ACP'ssession/request_permissionis unchanged.A saved rule that a stricter user rule outranks no longer re-asks within the session (the session grant stays); before this change the same click would have re-asked on the next identical call.
One tool's evidence is cut at 16 KiB per value and marked when cut, so a very large write or MCP payload now shows a prompt that says it was shortened rather than sending the whole thing.
Contract tier version 28 -> 30 (
ToolandApprovalRespondershape, thenApprovalOutcomecarrying the id of the request it answered).Rollback: revert the commits; no data migrates.
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
N/A