Skip to content

feat(agent): COD-529 add bounded inspection and guarded submission - #694

Open
aakhter wants to merge 8 commits into
Ark0N:masterfrom
aakhter:feat/COD-529-session-inspection
Open

aakhter wants to merge 8 commits into
Ark0N:masterfrom
aakhter:feat/COD-529-session-inspection

Conversation

@aakhter

@aakhter aakhter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Agents coordinating Codeman sessions currently choose between reading broad, repeated terminal history and sending a prompt with Enter immediately. This adds experimental, opt-in bounded evidence reads and a separate, verifiable stage/submit flow to the agent CLI and the authenticated API.

  • agent inspect: bounded UTF-8 evidence with opaque, expiring cursors tied to the caller, session, pane lifecycle generation and attributed transcript. A local Codex session reads strictly attributed rollout records; other targets return an explicitly labelled pane snapshot. Attribution remembers the attributed file per session and re-verifies only its head on later calls, so a large ~/.codex/sessions store is walked once. Cursors are capped per owner and session (newest 8) under the global 1,024.
  • agent stage / agent submit: type one printable line without Enter, then submit it once against a hash-confirmed receipt, after checking the exact composer, caret, lifecycle, input revision and geometry. Failed or ambiguous submits consume the receipt and require inspection before retrying.

Opt-in and experimental (per Discussion #695): the three routes stay under /api/v1 (and /api) with the experimental label in docs/versioning-policy.md, behind the synced setting agentCoordinationEnabled, default OFF. While it is off, each route answers 403 FORBIDDEN naming the setting, before session lookup or body validation; turning it on needs no restart. App Settings carries the toggle (with its zh-CN string).

Live-pane behaviour (measured on a real Codex 0.162.0 pane and tmux 3.4):

  • Stage polls after typing (up to 1 s, 50 ms apart) and issues a receipt only for a frame of the same generation, revision + 1, same width, and a composer hash equal to the message. Every refusal after typing says the message may already be in the composer and points at the Ctrl+U recovery.
  • capturePaneBuffer() reads the frame and the caret in one tmux call; it falls back to two calls only when the combined output cannot be split, and a thrown error is not retried.
  • A message that would open a Codex popup is refused before typing: a leading / or !, or a last word starting with @ or $. A popup row above the composer also fails the frame check. Both are registry data on the codex entry (workDetect.composerPopupLine, composerCommandPrefixes, composerCompletionSigils; the footer is workDetect.composerFooterLine). Slash commands go through send.
  • Submit waits until 250 ms after the staged keys, so Codex's paste-burst window cannot turn the Enter into a newline.
  • During a guarded submit, other input is held and written after it in arrival order instead of refused: WebSocket frames are applied and ACKed only when written, and send-key and approval answers wait their turn. The hold is capped (256 inputs or 1 MiB); past it the socket closes with 1013 so the browser resends in order. POST .../input answers 409 CONFLICT (nothing written) during the lease, which the browser's durable queue retries.

Errors: an unexpected inspection read error is 422 OPERATION_FAILED, logged with the session id and error code, never the path. A consumed receipt stays 409.

Initial guarded submission supports a recognised local Codex composer only. Wrapped, modal, working, remote/container or unverifiable composers are refused. Codex launch metadata cannot authoritatively track every in-CLI /resume; callers check the returned conversation identity. Direct tmux edits stay outside Codeman's input serialisation. Known trade-off: a cron job in paste mode on the same Codex pane writes text and Enter as two writes, so a guarded submit landing between them makes that run report as failed with the text left on the composer.

Docs: docs/api-reference.md, docs/versioning-policy.md, docs/cli-registry.md, docs/architecture-invariants.md, CLAUDE.md, the README, the wiki pages Driving-Codeman-From-An-Agent (absolute links) and Settings-Reference, CLI help, the skills/codeman reference and its synced plugin mirror, and the changeset.

Verification on 2aad9391:

  • Rebased on master; the duplicate send-keys -l -- hunk from fix(tmux): send prompts that start with a hyphen literally #677 is dropped.
  • npm run test:ci: 12,728 passed; the only failures are the 6 real-tmux cases in test/tmux-prefix-table.test.ts, which fail the same way on plain origin/master on this machine.
  • New and changed tests (including test/guarded-lease-held-input.test.ts, the stage settle window, popup frames, the 403 gate and the combined capture) each fail with their fix reverted.
  • Typecheck, lint, format, frontend syntax, public assets, browser excludes, the CLI catalogue check and the plugin mirror check pass.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you @aakhter for this. It adds bounded, cursor-paged inspection of another session's output and a guarded stage/submit flow that types a prompt into a local Codex composer without Enter and submits it once against a hash-confirmed receipt. The guard design is careful, the tests are thorough, and typecheck, lint, format and the rest of the static checks are green here.

Before going further, one design point: this adds three endpoints under /api/v1, which our versioning policy treats as stable public contract, and CLAUDE.md currently describes codeman agent as a client of existing routes. For a change this size CONTRIBUTING asks for a Discussion first. I want to settle the API shape there before we lock it in.

Three things make the main paths fail on ordinary setups, and each is a small fix:

  1. Attribution budget (src/agent-codex-identity.ts:99). Every .jsonl costs up to 4 KiB against a 1 MiB budget with 128 KiB reserved, so findExactCodexConversation() returns undefined past about 224 rollouts. With one owned rollout plus 300 unrelated ones it returns nothing and inspect falls back to pane-snapshot; a real store measured in src/codex-transcript.ts:16-19 holds 519. Please reuse or extend codex-transcript.ts (it already caches originator and thread id per file), or charge the budget only for files whose prefix matches, and add a test with a few hundred unrelated rollouts.
  2. Effort words (src/agent-staging.ts:48). The footer regex lists default|low|medium|high|xhigh|max, so a session at none, minimal or ultra can never stage. Please build the alternation from CODEX_REASONING_EFFORTS plus default, the way modelDetect.screenLine in stock.ts does, with a test per word.
  3. Width check before typing (src/agent-staging.ts:139). stage() writes the message and only then finds the composer wrapped, so it throws with no receipt while the text is already in the composer. Nothing in the guarded API can clear it, and the next plain codeman agent send submits it glued to the new prompt. Please refuse before typing when visibleWidth('› ' + message) would reach the pane width, and add a narrow-width test.

Smaller items:

  • src/agent-codex-identity.ts:29: realpathSync.native on the workspace and on meta.cwd runs on the request path. Please probe with probePathKind(..., { pastCap: true }) first and treat unknown as unattributed (the fix(cases): bound path probes for linked workspaces and session creation, so an unreachable mount cannot freeze the server #516 rule in CLAUDE.md).
  • src/agent-inspection.ts:136: a page that is simply full reports blockedReason: 'record-exceeds-output-budget'. Please set it only when the first pending record alone exceeds the budget.
  • src/session.ts:5212: every PTY resize bumps the generation that inspection cursors are scoped to, so opening the session on another device invalidates them. Please keep geometry out of the cursor scope.
  • CLAUDE.md:132 and :134 still say the agent CLI adds no route. Please update them and add a short section to docs/architecture-invariants.md for the guard rules.
  • skills/codeman/reference/inspection-and-staging.md: please drop the session-bridge references (lines 4-6) and document how an agent recovers a composer after a refused submit.
  • src/web/routes/session-routes.ts:1117: please register the new module in server.ts with the others rather than inside registerSessionRoutes.
  • src/session.ts:5057: the SubmitVerifier re-press now goes through writeViaMux('\r'), so it moves lastSubmitAt on each re-press for every CLI. Please note that in the changeset and cover it with a test.
  • Nits: src/tmux-manager.ts:4173 and :4185 (sendInputToPane) still lack the new --; the new members in src/session.ts:4738 and src/mux-interface.ts:319 sit between an existing JSDoc block and the method it documents.

Once the API question is settled and the three numbered items are in, I will review it again.

@aakhter

aakhter commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

The concrete review findings are addressed in e0c8a57a2; the full suite passed 12,372 tests with 12 skipped, and independent review found no blocking issues.

For the design question, I propose an experimental first phase and opened Discussion #695. Commit d381beebf explicitly scopes the exception to the three new CLI verbs and matching HTTP routes, including /api/v1 and /api aliases. Their flags, paths, payloads and behavior may change in minor or patch releases; existing authentication, ownership, shared envelope and other stable commands/routes keep their guarantees. The policy, CLI help, API docs, README and packaged skill now agree on that boundary.

This labeling/help follow-up passed 94 focused tests plus typecheck, lint, format and skill/plugin validation. Route organization remains available for revision in the Discussion; I am not treating the experimental proposal as maintainer approval.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you @aakhter for the follow-up. To restate what this does: it adds bounded, cursor-paged inspection of another session's output and a guarded stage/submit flow that types one line into a verified local Codex composer and later sends exactly one Enter against a hash-confirmed receipt.

Every item from my first review is in, and I checked each one:

  • Attribution budget: the separate 8 MiB prefix budget works. A scratch run against findExactCodexConversation() with one owned rollout attributes it next to 300 and 2,000 unrelated 19 KiB rollouts.
  • The footer is built from CODEX_REASONING_EFFORTS plus default, the width check runs before typing, paths are probed with probePathKind(..., { pastCap: true }), blockedReason is only set for a first record that alone exceeds the budget, geometry is out of the cursor scope, the module is registered in server.ts, -- is on every send-keys -l, and the SubmitVerifier lastSubmitAt change is in the changeset with a test.
  • Typecheck, lint, format, frontend syntax, browser excludes and the new tests are green here.

A few small things to fold in while the design question is open:

  1. src/agent-codex-identity.ts:109: every rollout is still charged up to 4 KiB before the originator filter, so attribution stops at about 2,048 files (the same run with 2,100 unrelated rollouts falls back to pane-snapshot), and each inspect call re-walks and re-reads the whole store. Codex never prunes ~/.codex/sessions, so a daily user reaches that within months. Please remember the attributed {path, dev:ino} per session and re-verify only that file's head on later calls (full walk only when that fails or to catch a /new conflict), or reuse the per-file head cache in codex-transcript.ts.
  2. src/web/routes/agent-session-routes.ts:103: any non-InspectionConflict error becomes 409 CONFLICT with nothing logged. Please return OPERATION_FAILED for those and log the session id and error code (not the path), so an unreadable rollout does not read as "retry".
  3. src/agent-staging.ts:10, :45, :94: the › glyph and the footer pattern duplicate codex's workDetect.promptGlyph and modelDetect.screenLine in stock.ts. Please take the glyph from the registry and either derive the footer test from it or move it into a named capability beside the other Codex screen patterns, so the next Codex footer change is a one-place edit.
  4. docs/wiki/Driving-Codeman-From-An-Agent.md:260: the wiki is mirrored to GitHub's wiki, where ../api-reference.md does not resolve, and the anchor should be #experimental-bounded-agent-inspection-and-guarded-input. Please use the full https://github.com/Ark0N/Codeman/blob/master/docs/api-reference.md#... URL like the other wiki pages.
  5. Nit, CLAUDE.md:132: the Agent CLI paragraph now has a line break after "ownership-checked"; please join it back into one line.

What happens next: the API shape (routes under /api/v1 with an experimental label, and codeman agent owning server routes) is my call to make on Discussion #695, and I will answer there before the next review round. Nothing else is needed from you until then beyond the small items above.

@Ark0N

Ark0N commented Oct 24, 2026

Copy link
Copy Markdown
Owner

@aakhter, the design question is answered on #695: #695 (comment)

In short: the three routes stay where they are under /api/v1 with the experimental label, behind one synced setting, default OFF, that answers 403 FORBIDDEN naming the setting while it is off (the cliManagementEnabled / mcpSyncEnabled pattern). Please add that together with the five small items from my last review here. #677 makes the same -- change on send-keys -l and is going into the next release, so drop that hunk from this PR when you rebase after it lands.

Gate inspect, stage and submit behind the synced agentCoordinationEnabled
setting (default OFF, App Settings > Cross-session features, zh-CN
strings). While off, all three routes answer 403 FORBIDDEN naming the
setting, checked before session lookup; the CLI verbs print it verbatim.

Remember the attributed Codex rollout per session and re-verify only its
head on later calls, rescanning only date directories not older than it
for a /new conflict. The first walk reads 1 KiB prefixes first, so a
store of 2,100+ unrelated rollouts still attributes.

Answer unexpected inspection errors with 422 OPERATION_FAILED and log
the session id and error code only.

Read the guarded composer glyph and footer from the CLI registry: codex
workDetect.promptGlyph and a new workDetect.composerFooterLine
capability, compiled through compileVersionRegex. Codex footer effort
words are one shared list with modelDetect.screenLine.

Fix the wiki API reference link, frame the versioning policy entry as a
case of the experimental rule, join the CLAUDE.md Agent CLI paragraph,
and update the changeset, skill reference and architecture notes.
@aakhter
aakhter force-pushed the feat/COD-529-session-inspection branch from d381bee to c78635a Compare October 25, 2026 03:25
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Ark0N. Rebased onto master; #677 already carries the -- on every send-keys -l, so this PR's duplicate -- hunk (and its duplicate leading-dash test) is dropped. The guard-specific beforeDispatch change in sendInput stays.

Design (#695):

  • The three routes stay under /api/v1 with the experimental label, behind one synced setting, agentCoordinationEnabled (default OFF; App Settings > Cross-session features > "Experimental agent coordination", with zh-CN strings). While it is off, all three answer 403 FORBIDDEN with a message naming the setting, checked before session lookup and body validation, read fresh like readMcpSyncEnabled. codeman agent inspect|stage|submit print the refusal verbatim (exit 1). Tests: each route off (key absent and false) -> 403 with the message, nothing written; on -> answers without a restart (test/routes/agent-session-routes.test.ts); the /api/v1 alias end to end in test/webserver-bound-port.test.ts; schema, load/save and zh-CN in test/agent-coordination-setting.test.ts; CLI passthrough in test/cli-agent-inspection.test.ts.
  • docs/versioning-policy.md now frames the subsection as a case of item 3 ("labeled experimental"), not a new exception, and says the session behavior underneath (input revision tracking, SubmitVerifier re-press) is covered as stable. The wiki page marks the three verbs experimental and opt-in.

Your 2026-10-23 items:

  1. agent-codex-identity.ts: the attributed {path, dev:ino} and its strict metadata are remembered per session (bounded LRU). A later call re-verifies only that file's head, then scans only the sessions/YYYY/MM/DD directories not older than it (numeric compare; non-numeric names are never pruned), which still catches a /new rollout. Any failed check drops the memo and does one full walk. The first walk now reads a 1 KiB prefix and widens to 4 KiB only when no complete originator field is in it, and skips non-UUID names unread. Tests: one owned rollout beside 2,100 unrelated 19 KiB rollouts attributes on the first call; a second call attributes under a 20-entry budget that a fresh walk cannot meet; a newer owned rollout after the memo is a conflict; a replaced file falls back to a full walk.
  2. agent-session-routes.ts: non-InspectionConflict errors are now 422 OPERATION_FAILED, logged with the session id and error code only (never the path). Stage and submit keep their documented 409 for a consumed receipt but now log the same way. Test: a rejected read with an EACCES carrying the path -> 422, path absent from body and log.
  3. agent-staging.ts: the glyph and footer come from the registry. Codex declares a new workDetect.composerFooterLine beside its other screen patterns in stock.ts, validated in schema.ts through compileVersionRegex() (and compiled through it again at use). Its effort words and modelDetect.screenLine's now share one CODEX_EFFORT_WORDS list, so a footer change is one edit. Guarded input is offered only to a CLI that declares composerFooterLine (codex today), not keyed on the transcript type. Documented in docs/cli-registry.md; the catalogue check is clean (the generated files carry no workDetect).
  4. Wiki link is now https://github.com/Ark0N/Codeman/blob/master/docs/api-reference.md#experimental-bounded-agent-inspection-and-guarded-input.
  5. CLAUDE.md: the Agent CLI paragraph is one line again, and names the setting.

The changeset, api-reference, architecture-invariants, Settings-Reference wiki page, packaged skill (and plugin mirror) and the cli-agent.ts @fileoverview are updated to match. Typecheck, lint, format, frontend syntax, public assets, browser excludes, plugin sync and CLI catalogue checks are green, and npm run test:ci shows no failures beyond those master has on this machine.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you @aakhter for this round. It adds the experimental, opt-in codeman agent inspect|stage|submit verbs and routes: bounded, cursor-paged evidence from a session's attributed Codex rollout (or a labeled pane snapshot), and a guarded flow that types one line into a verified local Codex composer and later sends exactly one Enter against a hash-confirmed receipt.

Everything from the earlier rounds is in and I checked it: the agentCoordinationEnabled gate (403 naming the setting, before session lookup, no restart needed), the attribution memo, 422 OPERATION_FAILED with id-and-code-only logging, the registry-driven composerFooterLine and shared CODEX_EFFORT_WORDS, the wiki link and the CLAUDE.md line. Typecheck, lint, format, frontend syntax, the skill/plugin mirror, the new tests and the full npm test gate are green here.

I also ran it against a real Codex 0.162.0 pane on an isolated instance. Inspect attributed the rollout (codex-jsonl with a conversation id and working cursors), stage typed without Enter, submit delivered one Enter and Codex answered, a replayed receipt was refused, an intervening keystroke invalidated the receipt, the width check refused before typing, and the documented Ctrl+U recovery cleared a stranded draft. Two things only show up on the live pane:

  1. Stage fails after typing on about 40% of calls (src/agent-staging.ts:173). The post-write snapshot is taken immediately after the keys land, and capturePaneBuffer() reads the screen and the cursor with two separate tmux calls (src/tmux-manager.ts:4324 and :4333). Codex repaints the composer 30 to 60 ms after the keys arrive, so the capture often shows the old composer while the cursor query already reports the new caret; readGuardedSnapshotDetails() rightly refuses that frame and stage answers 409 Input or pane changed while staging; no receipt was issued with the message already in the composer. Measured: 3 of 8 consecutive live stages, and 4 of 10 when replaying the server's exact write-then-capture sequence with your formatPaneSnapshot and readGuardedSnapshotDetails. The unit tests miss it because their capture mock is atomic. This is the round-1 case again (text typed, no receipt). Please poll the snapshot for a short bounded window after the write (for example up to 1 s, 50 ms apart) and take the first frame with the same generation and revision === before.revision + 1; requiring composerSha === promptSha256(message) before issuing the receipt would also prove the text landed. If the window runs out, say in the error that the message may be in the composer and point at the recovery steps. Please add a test where the first post-write snapshot is null and a later one is valid.

  2. A slash-command popup changes what Enter submits (src/agent-staging.ts:70). Staging /stat opens Codex's command popup above the composer row; readGuardedComposer() only checks the rows below the composer, so the frame verifies, the receipt hashes /stat, submit returns submitted: true, and Codex runs /status (the highlighted completion). The docs say modal composers are refused. Please refuse a frame with popup rows directly above the composer, or refuse at stage time a message starting with / (and @/$ mention tokens) and document that slash commands go through send, with a test for the popup frame.

Small things to fold in:

  • src/agent-staging.ts:180: independently of the retry, word this refusal like the submit one, since stage cannot know whether the text landed.
  • CLAUDE.md:192 (Auto-named sessions): the rule lists the WS path and POST .../input as the ONLY fromUser sources; stageGuardedInput() (src/session.ts:4808) is a third, so please add it.
  • src/agent-codex-identity.ts:138: the comment says an exhausted budget gets one full walk now, but a verified memo whose incremental scan returns undefined returns undefined without walking. Please align the comment (or the code).
  • While a guarded submit is in flight, a concurrent fire-and-forget POST .../input falls back to write(), which also refuses, and the route still answers 200; a sentence in the API docs would cover it.

With the stage retry and the popup refusal in, the rest of this is in good shape.

Stage now re-reads the pane after typing (every 50 ms, up to 1 s) and
issues a receipt only for a frame with the same generation, the input
revision its own write moved, and a composer hash equal to the
message's. Codex repaints 30 to 60 ms after the keys, so the first
capture was often stale and stage refused with the text already typed.
Every refusal after typing now says the message may be in the composer
and points at the Ctrl+U recovery. A visible-frame capture reads the
screen and the caret in one tmux invocation, with the old two calls as
the fallback.

Codex draws its suggestion popups above the composer and keeps the
footer, so a staged /stat verified and submit ran /status. Stage now
refuses a leading / or ! and a last word starting with @ or $ before
typing, and stage and submit refuse a frame with popup rows just above
the composer. The pattern and characters are registry data
(composerPopupLine, composerCommandPrefixes, composerCompletionSigils),
read from the codex 0.162.0 bottom_pane sources.

Submit waits until 250 ms after the staged keys: codex turns an Enter
within 120 ms of a typed burst into a newline. POST .../input answers
409 with its seq returned while a guarded submit holds the input lease,
instead of 200 for bytes it never wrote.

Also align the attribution memo comment with the code (a verified file
whose incremental scan fails is not re-walked, since the full walk
reads a superset), and list stage as a fromUser source in CLAUDE.md.
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for running it on a real 0.162.0 pane; both findings were real. Pushed as one commit on top (no rebase; still merges cleanly with master).

1. Stage refusing after typing. Fixed at both ends:

  • Stage now polls after the write (STAGE_SETTLE_TIMEOUT_MS 1000, STAGE_SETTLE_INTERVAL_MS 50, both documented in agent-staging.ts) and issues a receipt only for the first frame with the same generation, revision === before.revision + 1 and composerSha === promptSha256(message). A frame showing another writer's input, a new lifecycle or new geometry ends the wait at once. If the window runs out, the 409 says no receipt was issued, the message may already be in the composer, and to follow the stranded-draft recovery (Ctrl+U) before staging again.
  • capturePaneBuffer() now reads the visible frame and the caret in one tmux invocation (capture-pane ... \; display-message ...; tmux runs the list back to back with no pane output parsed in between), split by splitFrameAndCursor(), which refuses output whose last line is not the cursor query. Any doubt falls back to the old two calls. The poll stays as defence.
  • Tests: null first frame then valid gives a receipt; stale frame (old empty composer) then valid gives a receipt; never valid within the window is refused with that wording; a frame whose composer differs gives no receipt; a real Session whose first post-write captures are the torn frame you saw stages and submits; split/command-shape tests for the single tmux call. The store takes an injectable sleep and clock, so none of this uses real timers.

2. Popups. Both done, from codex-rs/tui/src/bottom_pane at rust-v0.162.0 (popup_state.rs says every suggestion popup renders above the composer and keeps the footer, which is exactly why /stat verified):

  • Stage refuses, before typing, a message starting with / (slash command) or ! (codex's shell command mode, same problem: Enter runs it), after leading whitespace, and a message whose last word starts with @ (file search / unified mentions) or $ (skills; every $ word, since whether codex opens it depends on its skill catalog). Popups follow the caret, which stage leaves at the end, so look at @src/app.ts please is accepted. The error says commands go through send.
  • Stage and submit refuse a frame with popup rows in the block directly above the composer: command rows (› /status show ...), the skill/mention key-hint row (enter insert · esc close, enter/tab insert · ...), and file search loading... / no matches. A file-search popup with results shows bare path rows no pattern can tell from prose, which is why the trigger refusal is there too.
  • Both are registry data, no CLI id in the code: workDetect.composerPopupLine (through compileVersionRegex()), composerCommandPrefixes and composerCompletionSigils (1 to 8 ASCII punctuation characters), with schema tests.
  • Tests: frame fixtures built from codex's own popup snapshots (command list for /stat, a wrapped command row, skill, mention, file loading, command no-matches), an idle frame with a transcript above that still reads, and each refused prefix plus accepted look-alikes (mail a@b.example, pay $5 today). Documented in the API reference, the skill reference, the wiki page and cli-registry.md.

Small things:

  • The post-write refusals (and the write failure) now say stage cannot tell whether the text landed, like the submit one.
  • CLAUDE.md Auto-named sessions (and the matching line in architecture-invariants.md) list stageGuardedInput() as the third fromUser source.
  • agent-codex-identity.ts: I kept the code and fixed the comment (and the API/invariants docs that said the same). The incremental scan reads a subset of what a full walk reads under the same budget, so a second candidate, an exhausted budget or an unreadable entry it hit is one the full walk would hit too; walking the whole never-pruned store there could only end the same way. A file that no longer verifies still gets the full walk. Added a test for the exhausted case (unattributed, memo dropped, the next call with room re-attributes).
  • Concurrent input: the route now reports it honestly. While a guarded submit holds the lease, POST .../input answers 409 CONFLICT ("nothing was written") and gives a tagged seq back, so the browser's durable queue (which keeps a frame on 409) delivers it on retry. The check sits right before the write with no await in between. Tested for raw, useMux, auto-mux and tagged input. Documented.

Live-pane differences I went through (from source, not a live codex; I did not run one, to avoid network and billing):

  • Paste burst: stage types plain keys (send-keys -l), which codex's paste_burst.rs reads as a paste, and for 120 ms after one codex turns Enter into a newline. A caller submitting the moment stage answers would have been told one Enter was delivered while codex only broke the line. Submit now waits until 250 ms after the staged keys (SUBMIT_QUIET_MS); a keystroke during the wait still invalidates the receipt. Tested.
  • Shell mode (!): refused before typing (above). It also swaps the composer glyph to !, so it never verified anyway.
  • Composer growing to several rows: refused before typing by the width check, and a wrapped composer fails the footer check.
  • Resize between stage and submit: bumps the geometry revision, refused (existing test).
  • Approval/question/trust overlays: replace the composer and match blockedLine, refused.
  • esc to interrupt working state: the working line nulls the snapshot, refused before typing; if it appears mid-settle, the poll times out with the may-be-typed wording.
  • Queued-message preview: only exists while a turn runs (working, refused); codex sends it when the turn ends.
  • A one-character message is typed as a key press; ? on an empty composer opens codex's shortcut overlay instead of inserting. That changes the footer, so no frame verifies and stage refuses after the window. Safe, if slower; nothing is submitted.
  • Vim normal mode, disabled input: the text or caret would not match exactly, so no receipt.

Gates: typecheck, lint, format, frontend syntax, public assets, browser excludes, plugin mirror and CLI catalogue checks pass, and the new tests fail when each fix is reverted. npm run test:ci: 12711 passed, 6 failed, all six in test/tmux-prefix-table.test.ts (real-tmux prefix-table tests that fail the same way on plain master on this machine; master additionally fails frontend-public-tooling there for a missing local asset).

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you @aakhter for this round. It adds the experimental, opt-in codeman agent inspect|stage|submit verbs and routes: bounded, cursor-paged evidence from another session, and a guarded flow that types one line into a verified local Codex composer and later sends exactly one Enter against a hash-confirmed receipt.

Both live-pane findings from the last round are fixed the way I asked: stage now polls for a frame whose composer hash equals the message, and the trigger refusal and popup-frame check are registry data. The small items (the after-typing wording, the fromUser line in CLAUDE.md, the attribution comment, the input-route answer during a submit) are all in. Typecheck, lint, format, frontend syntax, browser excludes, public assets and the focused tests (23 files, 573 tests) are green here, and the combined capture-pane \; display-message call works on a real tmux 3.4.

One thing needs a fix before merge:

  1. A browser keystroke that lands during a guarded submit is lost (src/session.ts:4875, src/web/routes/ws-routes.ts:206). The lease is held across the awaited tmux send-keys Enter, so the event loop processes WebSocket frames while it is held. A tagged frame with seq N is refused, forgetInputSeq rolls the watermark back and leaves it unACKed, but the browser's WebSocket path does not wait for ACKs, so seq N+1 lands right after the lease and moves the watermark to N+1. When the sweep redelivers N, the server rejects it as a duplicate and the browser drops it (tries > 1). I reproduced it with a real Session and a mux that runs beforeDispatch and then awaits the Enter like TmuxManager: a during the lease, b after it, a redelivered, and the PTY received only b. The POST path is fine because its drainer is serialized. Please pick one of these: hold writes while the lease is held and flush them in arrival order when it is released (ACKing a WebSocket frame only after its write; the message handler is synchronous, so this needs a small ordered queue), or, after refusing seq N for a client, refuse every later seq from that client until N is applied, so the sweep redelivers them in order. Please add a test shaped like the repro (N during the lease, N+1 after it, N redelivered: both reach the PTY, in order) and correct docs/api-reference.md:716, which says those keystrokes are redelivered.

Two small ones for the same push:

  1. src/web/routes/session-routes.ts:2463: a Shift+Enter or Ctrl+Enter during the lease makes performAuxiliaryInput throw, which the route logs as send-key failed and answers as INTERNAL_ERROR 'tmux send-keys failed'. The browser's fetch is fire-and-forget, so the newline vanishes. Please answer 409 CONFLICT for the lease refusal like the input route, without the error log (or route it through the same queue if that is your fix for item 1).
  2. src/tmux-manager.ts:4427: captureFrameWithCursor swallows a thrown error and the caller then runs the same capture again, so a hung tmux now blocks the event loop for two 5 s timeouts per visible-frame capture instead of one. That path serves every GET .../terminal load, not only guarded input. Please fall back to the two calls only when the combined call returned output that splitFrameAndCursor could not split, and let a thrown error reach the existing outer catch.

Optional nit, src/agent-inspection.ts:164: cursors are never retired on use and share one global FIFO of 1,024, so one agent polling every second evicts every other caller's cursors after about 17 minutes. A per-scope cap (for example the newest 8 per owner and session) would prevent that while keeping a retry of the last cursor.

With item 1 and its test in, this is ready to merge; items 2 and 3 can come in the same push.

The combined capture-pane and cursor query swallowed a thrown error and the
caller then ran the same capture again, so a hung tmux blocked the event loop
for two 5 s timeouts on every visible-frame capture, which serves every
GET .../terminal load. A thrown error now reaches the capture's own catch;
only output that splitFrameAndCursor cannot split falls back to the two
separate calls. The capture runs through a captureRunner() seam, like
inputRunner(), so the tests count the tmux calls each path makes.
Cursors shared one global FIFO of 1,024 and were never retired on use, so one
agent polling every second evicted every other caller's cursors after about
17 minutes. Each owner and session now keeps its newest 8 cursors, older ones
retired first, under the same global backstop; the cursor just used stays
among the newest, so a retry of the last call still works.
The input lease is held across the awaited tmux Enter, so WebSocket frames
keep arriving while it is held. A tagged frame (seq N) was refused and its
seq rolled back, but the browser's WebSocket path does not wait for ACKs:
seq N+1 landed after the lease and moved the watermark past N, and the
redelivered N was then dropped as a duplicate. The keystroke was lost.

Input that arrives during the lease is now held and written in arrival order
when the lease is released, whatever the submit's outcome. A WebSocket frame
is applied and ACKed only once written; managed writes (writeViaMux) and
named keys (send-key) wait for their turn the same way, and a held write that
is still being sent holds everything behind it. A keystroke held before the
guard runs still moves the input revision, so it still invalidates the Enter.
The hold is capped per lease (256 inputs, 1 MiB): past it the WebSocket is
closed with 1013 and nothing later from it is applied, so the browser resends
every unACKed frame in order, and send-key answers 409 CONFLICT instead of
logging a tmux failure as INTERNAL_ERROR. POST .../input keeps its 409 for the
lease plus the held input's drain; its browser drainer is serialized.

Docs: correct the api-reference paragraph that said these keystrokes are
redelivered, and describe the capture fallback and the per-scope cursor cap
from the previous two commits in api-reference, architecture-invariants and
the changeset.
@aakhter

aakhter commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Ark0N. All four items are in, three commits on top of 2832a03 (no rebase; still merges cleanly with master).

  1. Keystroke lost during a guarded submit (2aad939). I took your first design: input that arrives while the lease is held is held, not refused, and written in arrival order when the lease is released, whatever the submit's outcome. Why this one and not "refuse every later seq until N lands": the second needs a new per-client recovery state on the server that waits for a seq the client may never resend (a reload that drops its payloads, a tab that closes), so it either sticks the terminal or needs a TTL, and every keystroke in that window waits for the 2 s sweep plus the 4 s ACK timeout. Holding is a few milliseconds and needs no client change.

    • Session.holdForGuardedLease(run, bytes, data) queues the frame (the route calls it defensively, like lastInputSeq, so a port implementation without it still delivers input); the WebSocket route applies and ACKs it only when the session runs it, so a frame is ACKed only once written. The message handler stays synchronous.
    • writeViaMux and performAuxiliaryInput (send-key) wait for their turn in the same queue (_inGuardedTurn), and a held write that is still being sent holds everything behind it (guardedInputHeld), so a held Shift+Enter is not overtaken by the keystroke typed after it. Without a lease both still start synchronously, so the pending-write count rises before any await, as before.
    • A keystroke held before the guard runs still moves the input revision on arrival, so it still invalidates the Enter, and then lands.
    • Cap per lease: 256 inputs or 1 MiB. Past it the socket is closed with 1013 and nothing later from it is applied, so the browser reconnects and resends every unACKed frame in order.
    • POST path: confirmed fine. The browser's POST drainer sends one frame and awaits it, and a 409 keeps the frame queued. It now answers 409 for the lease plus the drain of held input, so it cannot overtake held frames either.
    • Test: test/guarded-lease-held-input.test.ts uses a real Session, the real WebSocket route through a fake socket, and a mux that runs beforeDispatch and then awaits the Enter like TmuxManager. Your repro (a seq N during the lease, b N+1 after it, a redelivered) now gives ['\r', 'a', 'b'] with ACKs [1, 2] and no dup; on 2832a03 it gives ['\r', 'b']. Variants: several frames during the lease, a second clientId interleaved by arrival, the lease ending in a refused Enter and in a thrown one, untagged frames, a frame held for a socket that closed (applied once, its redelivery is a duplicate), the cap and reconnect, a key still being sent holding later frames, and a keystroke held before the guard.
    • docs/api-reference.md ~716 now says what happens: held and written in order, ACKed when written, POST answers 409 for that span, and the cap behaviour.
  2. send-key during the lease (2aad939). It goes through the same queue, so Shift+Enter and Ctrl+Enter during a submit are delivered after it rather than refused. The only refusal left is a full hold queue, which throws GuardedInputBusyError; the route answers 409 CONFLICT for it with no send-key failed log. Test: answers 409 CONFLICT, without an error log, when a guarded submit cannot hold the key in test/routes/session-routes.test.ts, plus holds managed writes and named keys behind the lease... in the new file.

  3. Capture retry on a hung tmux (a29192a). A thrown combined call now reaches the capture's own catch; only output splitFrameAndCursor cannot split falls back to the two separate calls. The capture runs through a captureRunner() seam like inputRunner(), and the tests count tmux calls: one call when it splits, one call (and one error log) when it throws, three when it cannot split, two on the full-history path.

  4. Cursors (ecaddb4). Each owner and session keeps its newest 8 cursors (INSPECT_CURSORS_PER_SCOPE), under the same global 1,024 backstop. The cursor just used stays among the newest, so a retry of the last call works. Test: caps cursors per owner and session, so one polling scope cannot evict another: Alice pages 1,100 times, Bob's cursor and Alice's last one both still resolve, the oldest of nine is retired, and a new pane generation of Alice's session shares her budget while Bob's does not. Docs updated in api-reference, architecture-invariants and the skill reference (plugin mirror synced).

Same class, also fixed:

  • Approval answers during a lease used to come back as OPERATION_FAILED "Session is not accepting input"; writeViaMux now waits, so the answer goes out after the Enter.
  • Untagged WebSocket frames (wheel reports, legacy tools) were dropped during the lease with no redelivery behind them; they are held too.
  • The SubmitVerifier's retry Enter is the one write deliberately not held: once the submit's Enter is out, a re-press would submit whatever is on the composer next. It still cancels and moves the revision as before.

Swept and left as is: resize frames were never refused (they move the geometry revision); paste, the mobile accessory keys and voice dictation reach the session as WebSocket input frames, so they are covered by item 1; the legacy sendInput/runPrompt entrypoints still throw during the lease, but they spawn a one-shot PTY that a live codex session already refuses. For tmux, the PR's other capture paths (stage's settle poll, submit's snapshots, inspection's pane snapshot) return on the first failed capture and do not rerun it; the stage poll's 1 s deadline is checked after each capture.

Trade-off, stated plainly: cron's paste mode writes its text and its Enter as two raw PTY writes; if a guarded submit lands between them on the same codex session, the Enter is refused and the run is reported as failed with the text on the composer. It is reported, not silent, and it needs a cron job and an agent submit on one codex pane within the same 50 ms.

Mutation checks (fix reverted, tests that fail): WebSocket route not holding 9; the route calling the hold without its defensive check 11 (test/routes/ws-routes.test.ts); held async write not awaited before the next 1; writeViaMux refusing again 1; performAuxiliaryInput refusing again 3; no revision bump on hold 1; no hold cap 2; send-key without the 409 mapping 1; POST checking the lease only 1; combined capture swallowing its error again 1; no per-scope cursor cap 1.

Gates: typecheck, lint, format:check, check:frontend-syntax, check:public-assets (58 files), check:browser-excludes and the plugin sync check are green; prettier is clean on the changed tests. Focused tests: 18 files, 587 tests passed. npm run test:ci: 12,728 passed, 6 failed, all 6 in test/tmux-prefix-table.test.ts; plain origin/master on the same machine fails the same 6 there (it needs a real tmux) plus one test/git-clone.test.ts case that passes on the branch.

@Ark0N

Ark0N commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Thank you @aakhter for this round. The PR adds the experimental, opt-in codeman agent inspect|stage|submit verbs and routes: bounded, cursor-paged evidence from another session, and a guarded flow that types one line into a verified local Codex composer and later sends exactly one Enter against a hash-confirmed receipt.

All four items from my last review are in, and I checked each one:

  • Input that arrives during a guarded submit is held and written in arrival order. The repro (seq N during the lease, N+1 after it, N redelivered) now delivers both, in order, in test/guarded-lease-held-input.test.ts.
  • send-key waits its turn, and a full hold queue answers 409 CONFLICT without the error log.
  • A combined capture that throws is no longer rerun.
  • Cursors are capped per owner and session.

Typecheck, lint, format, frontend syntax, browser excludes, public assets, the plugin mirror, the CLI catalogue, the 22 focused test files and the full npm test gate are green here.

Two small things are left, both only reachable with the setting on and input landing within the few milliseconds of a submit:

  1. Untagged POSTs are dropped during a submit (src/web/routes/session-routes.ts:2089). The new check answers 409 for any input during the lease and its drain, tagged or not. Only the durable queue's POST drainer retries a 409. Two browser paths send untagged, fire-and-forget POSTs and never read the response:

    • the phone accessory bar's Esc, arrows, Opt+Enter, Tab, Shift+Tab and Ctrl+O (KeyboardAccessory.sendKey, src/web/public/keyboard-accessory.js:1169)
    • the Stop button's Ctrl+C (src/web/public/session-ui.js:2228)

    A tap in that window is now lost silently, where master always delivers it. Please keep the 409 for tagged requests (clientId + seq) and hold untagged ones behind the lease the way the WebSocket path does (for example session.holdForGuardedLease(() => session.write(inputStr, { fromUser: true }), inputStr.length, inputStr), answering 200 once held). Please add a route test for an untagged raw POST during the lease.

  2. Cron paste mode during the drain (src/session.ts:4785 and :4986). The guardedInputHeld JSDoc says a write that is not routed through the hold must refuse in that window. write() only refuses during the lease itself, which it has to, since the drain calls it. Cron's paste mode (src/cron/cron-service.ts:131 and :133) writes through write() directly, so it can land ahead of input still draining. A sentence saying so, in the JSDoc and in the trade-off note in docs/architecture-invariants.md, is enough here.

Both are small. I will apply them on top at merge time unless you push them first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants