Skip to content

Fix stale worker alerts and Windows paste submission - #5

Open
dextah wants to merge 1 commit into
mainfrom
fix/interaction-release-scroll
Open

dextah wants to merge 1 commit into
mainfrom
fix/interaction-release-scroll

Conversation

@dextah

@dextah dextah commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Submitting a reply in a Claude or Codex worker terminal used to acknowledge the alert while leaving the worker in needs_input until another lifecycle hook arrived. An approved long-running command could therefore keep its old alert for the entire command. Submitted Enter/Ctrl+C now resumes that wait through the provider adapter. Editing, navigation, terminal reports, bracketed paste (including split markers), and plain multiline paste do not dismiss it. New permission/question hooks can raise another alert; boot trust/login waits remain provider-controlled.

Windows terminal input now queues an explicit Enter for a 500ms quiet period after a paste, preserving subsequent key order. PR/queue agent prompts use the same queue, with the existing 120ms separation retained on POSIX. Delayed input is tied to the original PTY and cannot reach a replacement process. This targets the Windows paste-burst path where Enter is interpreted as another pasted newline; Codex documents its 120ms Enter suppression window in paste_burst.rs.

The status transition is a submitted-reply fallback, not proof that a multi-step provider dialog is complete. Subsequent provider hooks remain authoritative. This does not change agent approval policies or submit anything without a requested Enter.

Validation:

  • npm run typecheck and npm run build pass.
  • 25 focused hook/input/size tests pass on Windows, including a real ConPTY paste/Enter test. Added input regressions to the existing Windows CI workflow.
  • Headless Edge exercised the actual terminal clipboard and Enter handlers against the Windows server input queue and checked the alert/status update; screenshot captured locally at .agent-office/worker-input.png. This used a simulated worker transport rather than a signed-in Claude/Codex process.
  • npm test was attempted but has existing Windows failures and hanging fixtures. Reproduced the registry path-separator failure and Unix-shell clone fixture failure on unchanged origin/main (105f150) in a separate worktree. The full suite was stopped after it stalled. Linux release CI will run the full suite.
  • Updated README and terminal controls documentation. Extracted input handling from the manager and lowered its size ceiling.

@dextah

dextah commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

PR #5 Windows baseline (2026-10-23)

Tested PR head 98b38e1 against fetched origin/main 105f150 in separate worktrees.

  • PR: npm run typecheck passed; npm run build passed; requested focused suite passed (25/25).
  • Full npm test: both PR and main were still running at the ~10 minute cutoff and were stopped. Both emitted the same eight failures: tests/client-registry.test.ts (the two InteractKind registry checks) and tests/clone.test.ts (progress/completion, stalled clone, cancel, restart, completed while offline, and cutoff cases). No PR-only failure appeared before cutoff; later results are unknown.

Risk A — Codex Ctrl+C

Not an issue for Codex versions that support the registered Interrupt hook. The office registers Interrupt in src/server/codex.ts:12; src/server/workers/lifecycle.ts:146-149 maps it to done. The generated hook test runs the real helper/transport path with an Interrupt payload and asserts done in tests/codex-hook-shell.test.ts:81. The generated-hook test does not exercise a live interactive Codex permission screen, so that exact key interaction was not independently reproduced. The Codex hook docs specify Interrupt for an interrupted active main-thread turn.

Risk B — text and Enter in one frame

No such sender found in the current UI paths; the parser edge is real. ReplyInput classifies any multi-character frame without ESC as paste, so a synthetic single frame yes\r does not clear the alert (src/server/workers/input.ts:15-25). The 3D terminal and phone keypad go through xterm onData (src/client/ui/terminal.ts:407, 488); prompt, Ask agent, and /lite prompts use worker.prompt; server prompts and file drops use pastePrompt/term.paste, which keep paste and Enter separate. A browser or IME that coalesces a committed reply and Enter into one unescaped frame could leave the alert up; that composition path was not reproduced.

Other diff risks

TerminalInput holds Enter and later keys in FIFO order during the Windows delay (src/server/workers/terminal-input.ts:20-41). The per-PTY WeakMaps are safe across replacement: delayed writes and callbacks check that w.pty is still the captured PTY (src/server/workers/input.ts:46-49, 67-69); worker startup assigns the new PTY (src/server/workers/manager.ts:758). DSH/ACP takes its own dsh.writeInput path and does not enter the terminal queue (src/server/workers/input.ts:60-62). No additional issue found.

Recommendation: merge. Typecheck, build, and focused tests pass; full-suite failures observed on PR are also present on 105f150, and the two identified input risks have no demonstrated regression in the current UI paths.

@dextah

dextah commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Follow-up correction on Risk B: I understated this edge in the report above. xterm documents that onData fires for pasted input (API); without bracketed paste, the PR parser explicitly expects clipboard text, including CR, as one unescaped chunk (src/server/workers/input.ts:15-16). For a pasted answer ending in CR, ReplyInput.submitted() therefore returns false even if the terminal consumes that CR as Enter, leaving the alert up. The parser test confirms the combined-chunk behavior, but we did not reproduce it in a live Codex/Claude permission prompt.

Verdict: Risk B is a confirmed conditional edge for unbracketed paste with a trailing CR, although ordinary key presses, keypad Enter, Ask agent, and prompt forms remain separate. I’d hold merge until this path is tested against the supported CLI(s), or the client marks an actual Enter separately from paste data. The full-suite comparison and other findings above are unchanged.

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.

1 participant