Skip to content

fix(workspace): New Agent overlay owns no input while hidden; retry after a failed create (#1269, #1270) - #1286

Merged
Juliusolsson05 merged 10 commits into
mainfrom
fix/new-agent-overlay-traps
Sep 25, 2026
Merged

Juliusolsson05 merged 10 commits into
mainfrom
fix/new-agent-overlay-traps

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1269. Fixes #1270.

Refs #1267: this fixes only the renderer side of steering q22. Main still relays the raw exception, so #1267 stays open.

Found by the Stage 3 C4 hunt (temp/quality-loop/hunt-c4.md). Plan: docs/plans/2026-09-25-new-agent-overlay-traps.md (first commit).

#1269: a hidden overlay owned all input

The overlay lives inside the retained workspace, which stays mounted under display:none while Reader, Spotlight, Settings or a fullscreen editor own the screen. The Command Palette is not focus-filtered, so "New Agent…" could open the overlay invisibly. Once open:

  • its capture keydown listener swallowed Escape and the arrow keys;
  • it turned the composer's Enter into "create an agent";
  • it froze every shortcut, twice over. Its owner marker blocked them, and useKeybinds' placement gate, which read only the store flag, preventDefaulted every ⌘/⌥ chord and ate the first Escape.

Now:

  • While hidden, the overlay renders nothing, registers no listener and stamps no marker.
  • GlobalEditorWorkspaceSlot also provides WorkspaceSurfaceHiddenContext, OR'ed with the outer value. That covers a fullscreen editor, and a takeover over a split editor.
  • The useKeybinds gate treats New Agent as open only when the flag is set and [data-new-agent-overlay] is mounted. Escape leaves Reader or Spotlight, and the palette chord works.
  • open is kept, so the picker appears once its surface is visible again (the flag is not persisted).

#1270: a dead overlay after a failed create

A create that returned no session left committingRef set: Enter did nothing while the overlay still blocked shortcuts. Now:

  • The latch reopens after a failed or rejected create, but only for the latest one; a generation counter stops a stale create from releasing a newer one.
  • The case where placement is refused ("its project was closed while it was starting") now shows a toast, saying "terminal" or "agent" as appropriate.

Spawn-failure text (q22)

sessionSpawnErrorMessage is the single place a session:spawn rejection becomes user-facing text. It feeds every spawn toast (create, split, linked, newTab, reload, rewind, provider switch, capability reload) and the orchestration tool error. It returns one of four things, and the text is always built from our own constants, never copied from the error:

  • the Claude proxy rewrite;
  • Workspace folder is missing: <cwd>, where <cwd> is the folder the renderer asked to spawn in, from the shared MISSING_WORKSPACE_FOLDER_PREFIX (fix(undo-close): a failed restore says why (#1242) #1264);
  • <kind> CLI not found. Open Setup (File › Setup…) …, only for a known provider id, from the new shared PROVIDER_CLI_NOT_FOUND_SUFFIX, which main's ProviderCliNotFoundError now also uses;
  • otherwise SESSION_START_FAILED_MESSAGE.

The create, split and linked-create toasts re-read the thrown message with the same recognizer, so curated text survives and anything else is the generic sentence.

Tests

  • NewAgentPlacementOverlay.renderer.test.tsx:
    • hidden surface;
    • fullscreen editor;
    • takeover over a split editor;
    • split editor on screen;
    • retry after a failed create, and after a rejected one;
    • the latch is kept after a successful create;
    • a stale create cannot reopen the latch.
  • focusModeKeyboardOwnership.renderer.test.tsx: Escape and the palette chord work under Reader while New Agent is open but not on screen, and Escape still goes to the visible overlay.
  • controlPlacement.renderer.test.tsx:
    • no raw posix_spawnp text in the create, split or linked toasts (recorded fixture);
    • the curated proxy, missing-folder and CLI sentences reach the create toast.
  • errorMessage.test.ts:
    • a raw rejection is flattened;
    • the missing-folder, proxy and CLI sentences are kept without the IPC wrapper;
    • nothing after the missing-folder prefix passes through;
    • the CLI sentence appears only for a known provider.

npx tsc -b is clean.

Residual

  • No test covers:
    • the real overlay's data-new-agent-overlay attribute;
    • the unplaced toast;
    • the "terminal" wording;
    • the mitmdump / Unable to locate mitm needles.
      If any of these break, the safe sentence shows, never raw text.
  • An abandoned create that later succeeds can close a newly reopened overlay. That close predates this PR.
  • Main still relays the raw provider exception over IPC (session:spawn relays the raw provider exception over IPC (q22 at the source) #1267).

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 25, 2026 06:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… after a failed create

- #1269: the overlay is retained under display:none by takeovers and the
  fullscreen editor, and the palette can open it there. Its capture
  listener ate Escape/arrows/Enter and its owner marker (a querySelector,
  blind to display:none) blocked every shortcut. Hidden, it now renders
  nothing and listens to nothing; `open` is kept so it appears when its
  surface returns. GlobalEditorWorkspaceSlot now provides the retained-
  surface hidden context too.
- #1270: a create that produced no session left the one-shot latch set, so
  Enter was dead in an overlay that still blocked input. It reopens.
- q22: the three spawn-failure toasts in pane.ts showed the raw IPC
  rejection; they now show SESSION_START_FAILED_MESSAGE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05 added a commit that referenced this pull request Sep 25, 2026
/#1286 opened

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05 and others added 5 commits September 25, 2026 06:49
…ounted overlay; spawn errors flattened at the source

- A: useKeybinds' placement gate keyed on the store flag alone, so under
  Reader/Spotlight/a fullscreen editor the first Escape closed an overlay the
  user never saw instead of leaving the takeover, and every chord (the
  palette's included) was dropped. It now requires the overlay to be mounted
  ([data-new-agent-overlay]), like the TLDR/goal latches.
- A2: a stale create settling late reopened the latch for a newer in-flight
  one (a duplicate agent). The release is now per create generation.
- A3: an unplaced spawn (project closed while starting) now says so.
- B3 (q22): sessionSpawnErrorMessage returned the raw provider exception,
  and newTab, reload, provider switch, rewind, capability reload and the
  orchestration tool error all showed it. It now returns the curated proxy
  or missing-folder text, or the safe sentence.
- B1/B2/B4: tests pin the slot composition both ways, all three pane.ts
  spawn catches, and the latch's success/rejection/stale outcomes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05 and others added 3 commits September 25, 2026 09:44
…res; the missing-CLI sentence survives

spawn already maps a rejection to a safe message, but the create toasts
dropped it for the generic sentence, so a deleted worktree looked
retryable and the proxy hint was lost. One recognizer (curatedSpawnMessage)
keeps exactly the three curated failures: Claude proxy startup, a missing
folder, and main's ProviderCliNotFoundError, whose sentence now lives in
shared. Everything else is still the generic sentence (q22). The unplaced
toast names terminals as terminals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rom our constants, never copied from the error

The missing-folder message was every byte after the prefix, so a provider
exception that contained the prefix and then a URL or token passed it to a
toast and the orchestration error. The folder named is now the cwd the
renderer asked to spawn in. The CLI sentence only names a known provider id.
Tests use the reviewer's probes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Final disposition (pre-merge), head 0c5aa6d. Three independent reviewers:

  • A (input ownership): two rounds.
  • B (tests, the q22 rule against showing raw errors): two rounds.
  • C (third reviewer): round 1 at ad6cb3a, round 2 at 7116732.

C's round-2 findings are fixed in 0c5aa6d. Nobody has reviewed that commit since; I verified it against the code.

Reviewer Round 1 Round 2
A (input ownership) FIX-BEFORE-MERGE MERGE-READY (at 2051233)
B (tests, q22) FIX-BEFORE-MERGE MERGE-READY (at 2051233)
C (third reviewer) FIX-BEFORE-MERGE (at ad6cb3a) FIX-BEFORE-MERGE (at 7116732)

Findings and dispositions:

Round 1, A and B (all fixed in 2051233; A and B confirmed in round 2)

  • A1, major: the useKeybinds gate followed the store flag, so a hidden overlay still swallowed the first Escape and every ⌘/⌥ chord under Reader, Spotlight or a fullscreen editor. Fixed: the gate also requires a mounted [data-new-agent-overlay]. A focus-mode ownership test covers it.
  • A2, minor: a stale create could release the latch for a newer one, which allowed a double create. Fixed with a generation counter, and a test replays that interleave.
  • A3, minor: the !placed path failed silently. Fixed with a toast.
  • A, cosmetic: stale comments in pane.ts and GlobalEditorWorkspaceSlot.tsx. Fixed.
  • B1, major: the slot's hidden formula was pinned only for fullscreen. Fixed with a takeover-over-split test and a split-without-takeover test.
  • B2, major: the splitFocused and createLinkedAgent toasts were untested. Fixed; the tests use the recorded posix_spawnp fixture.
  • B3, major: newTab, reload, rewind, provider switch, capability reload and the orchestration tool error still showed the raw spawn rejection. Fixed at the source: sessionSpawnErrorMessage now returns only curated text or SESSION_START_FAILED_MESSAGE.
  • B4, minor: the latch tests did not pin success, rejection, guard deletion or onClose while hidden. Fixed.

Round 2, A and B (test gaps only; none fixed; not blocking)

  • Nothing renders the real overlay and queries data-new-agent-overlay. When the overlay is visible, its owner marker already holds chords.
  • The "project was closed while it was starting" toast is untested. C's M8 mutant, which deletes it, still survives.
  • The placementOverlayOpen && guards on the gate's Escape close are untested. They are defence in depth.
  • Nothing asserts the "terminal" wording in splitFocused, or the mitmdump / Unable to locate mitm needles. If any of these regress, the user sees the safe generic sentence, never raw text.

Round 1, C (at ad6cb3a)

  • C1, major: create, split and linked-create toasts always showed the generic sentence, which discarded the curated proxy and missing-folder messages. Fixed in bd49018: spawnFailureToast re-reads the thrown message with curatedSpawnMessage. C confirmed this in round 2.
  • C2, major: the curated ProviderCliNotFoundError sentence ("… CLI not found. Open Setup (File › Setup…) …") was flattened. Fixed in bd49018: a shared PROVIDER_CLI_NOT_FOUND_SUFFIX builds main's message, and the renderer recognises it. C confirmed this in round 2.
  • C3, minor: the unplaced toast always said "agent". Fixed in bd49018: it now says "terminal" for terminals. It is still untested.
  • C suspicion, not demonstrated: an abandoned create that later succeeds can close a newly reopened overlay. That close predates this PR. Not fixed.

Round 2, C (at 7116732; both fixed in 0c5aa6d; I verified the code)

  • C1, major: the missing-folder branch returned every byte after the prefix, so a provider exception containing the prefix followed by a URL or token reached a toast. Fixed. curatedSpawnMessage(raw, cwd) now returns MISSING_WORKSPACE_FOLDER_PREFIX + cwd, where cwd is the folder the renderer asked to spawn in. Nothing from the error text is copied. All three pane.ts catches and session.ts pass that cwd. The test "never passes anything after the missing-folder prefix through" uses C's two probes.
  • C2, minor: the CLI kind was any lowercase token. Fixed: it must be in AGENT_PROVIDER_KINDS (claude, codex, opencode, grok, pi). The test "shows the CLI sentence only for a known provider" uses C's probe.
  • C test gap: making only the linked-agent catch use the generic sentence survives. Not fixed; the linked catch calls the same helper today.

Residuals:

Merges through batch PR #1318 once every member passes its gate.

🤖 Generated with Claude Code

Juliusolsson05 added a commit that referenced this pull request Sep 25, 2026
Conflicts resolved (steering q41 overlaps):
- RestoreBanner: main's swap/focus + button styling, with the batch's
  save-failure label; the two test files combined on one mutable context.
- BulkProviderSwitchModal: main's DialogActions footer, with #1312's stop
  (Cancel becomes "Stop after this agent" while locked).
- NewAgentPlacementOverlay: main's list keys under #1286's `active` gate;
  main's new listbox autofocus is gated on `active` too (a hidden overlay
  must not take focus).
- pane.ts: #1286's spawnFailureToast over main's raw err.message toasts
  (q22).
- ConversationsPicker: main's useListNavigation, with #1297's stale guard
  before nav.onKeyDown for every list key (PageUp/Down, Home/End, ⌃N/⌃P),
  hover stripped while stale, and `stale` in the reset key.
Also fixes main's red CI on 13ae608: nine tests still queried #1221's old
accessible names ("everywhere", the recorder's "Add"/"Press keys…").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05
Juliusolsson05 merged commit 1da672f into main Sep 25, 2026
1 of 2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/new-agent-overlay-traps branch September 25, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant