Skip to content

feat(agents): a failed resume opens the agent's own session picker (#311) - #319

Merged
simion merged 2 commits into
simion:mainfrom
GabrielDumbrava:fix/resume-failure-picker
Sep 21, 2026
Merged

simion merged 2 commits into
simion:mainfrom
GabrielDumbrava:fix/resume-failure-picker

Conversation

@GabrielDumbrava

Copy link
Copy Markdown
Contributor

Closes #311

Problem

A stored session id that no longer resolves exits within RESUME_FAILURE_MS, and termic answered it with a fresh session and a toast.
The conversation was usually still there: only termic's pointer was stale.
The agent's own reason for refusing ("No conversation found", or claude's "running as a background session ... claude attach") was repainted over by the respawn.

What this does

A new per-agent capability, resume_picker_args, names the args that open the agent's own session picker, as discussed on the issue: no termic-side reading of any agent's session files.

  • When a stored id fails to resume, the next spawn opens that picker instead of minting a fresh session.
  • The session picked there comes back over the hooks (session <id>), exactly as any /resume does since claude tab resumes the session from before /clear after a relaunch #306, so the next relaunch resumes it.
  • Leaving the picker (Esc exits 1) starts a fresh session, once; there is no loop back into the picker.
  • The toast offers the picker and carries the agent's own line, read from the raw output of the resume's first RESUME_FAILURE_MS (src/lib/resumeTail.ts). xterm's buffer can lag the exit event; Rust drains every byte before emitting it.
  • Without agent hooks the pick cannot be reported, so the toast says to install them. The picker still gets the user their conversation now.
  • An agent with no picker keeps today's fresh fallback, with the agent's line in the toast.

The field is editable in Settings > Agents ("Session picker args") and a clone inherits its parent's.
There used to be a "Resume it" banner for this; it outlived the failure it described, which is why this is the agent's own picker plus a transient toast, and nothing termic keeps on screen.

Defaults: only what is measured

Only claude is installed here, so only claude gets a default: --resume with no id.
Measured on claude 2.1.278:

Action Result
claude --resume --name <task> opens claude's "Resume session" picker (search, this project's sessions)
pick a session SessionStart with source: resume, the chosen id, entrypoint cli, which the READY hook reports
Esc exits 1, no SessionStart

Every other agent stays empty until measured, per docs/adding-an-agent.md, which now lists what to measure (the picker opens with termic's name_args after it, a pick is REPORTED, leaving it exits). The rest are for the follow-up PR.

Existing installs get claude's default through the same load-time backfill as the other capability lists, which is safe since #316.

Review fixes (second commit)

A read-only review by a second agent found four things, each checked in the code:

  • The pty-exit listener is now registered before pty_attached. A process that has already exited has its buffered output released by the ack, and the waiter fires pty-exit right behind it; acking first left the exit to race onPtyExit's own round trip. Pre-existing for every fast exit, but a lost exit here is a failed resume that neither opens the picker nor starts fresh. No test forces this race; it is a timing window.
  • The resume-tail capture checks one boolean on the data path, closed by the existing survive timer, instead of a time comparison per chunk for the life of every resumed session.
  • The hookless case and its toast, above.
  • The Settings hint no longer promises that an emptied picker field stays empty on a built-in agent: the backfill restores it, as for the other lists.

Tests

  • npm test: 2276 passed. New: spawnArgsForCli with a picker (replaces every resume block, keeps --name), resumePickerArgsForCli (built-in default, none for an unmeasured agent, clone inheritance and override), and resumeTail (escape and OSC stripping, line joining, trimming).
  • cargo test: 1076 passed. New: an existing claude entry without the field gains --resume on load; codex stays empty.
  • agent.e2e.ts, new describe (3 cases), with fakeclaude reproducing claude's three shapes (a dead id prints "No conversation found" and exits 1; --resume alone is the picker; a lone Esc leaves it): a failed resume spawns --resume with no id instead of --session-id, the toast offers the picker and carries the agent's line, the picked id is stored and resumed on the next relaunch, and leaving the picker starts fresh exactly once. Controls: without the picker all three fail; without the fallback the last one does. Passed 3 runs in a row.
  • Full e2e suite: the same 11 failures in the same 8 spec files as a clean main build (85b4d12); none only on this branch.

Manual verification

Not manually verified in the app: testing it here would have cost me a live session, so I opened this without a hand test.
The claude behaviour it relies on (picker opens, a pick fires SessionStart source=resume, Esc exits 1) was measured directly against claude 2.1.278 in a scratch directory.

🤖 Generated with Claude Code

GabrielDumbrava and others added 2 commits September 21, 2026 16:54
…imion#311)

A stored session id that no longer resolves exits within
RESUME_FAILURE_MS, and termic answered it with a fresh session and a
toast. The conversation was usually still there; only termic's pointer
was stale, and the agent's own reason for refusing ("No conversation
found", or "running as a background session ... claude attach") was
repainted over by the respawn.

A new per-agent capability, `resume_picker_args`, names the args that
open the agent's own picker. When a stored id fails, the next spawn opens
that picker instead of minting; the session picked there comes back over
the hooks (`session <id>`), as any /resume does since simion#306, so termic
reads no agent's session files. Leaving the picker (Esc exits 1) starts a
fresh session once. The toast offers the picker and carries the agent's
own line, read from the raw output of the resume's first
RESUME_FAILURE_MS: xterm's buffer can lag the exit event, while Rust
drains every byte before emitting it.

Default only where measured: claude, `--resume` (2.1.278: the picker opens
with --name after it; picking fires SessionStart source=resume with the
chosen id; Esc exits 1). Other agents stay empty until measured; the
field is editable in Settings > Agents and a clone inherits its parent's.
Existing installs get claude's default through the same load-time
backfill as the other capability lists (safe since simion#316).

Tests: unit (arg composition, inheritance, tail parsing), Rust (backfill),
e2e with `fakeclaude` reproducing claude's three shapes. Controls: without
the picker all three e2e cases fail; without the fallback the last one does.

Closes simion#311

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
From a read-only review by a second agent, each checked in the code:

- Register the pty-exit listener BEFORE acking the PTY. A process that
  has already exited has its buffered output released by `pty_attached`,
  and the waiter fires pty-exit right behind it, so acking first left the
  exit to race `onPtyExit`'s own round trip. A lost exit is a failed
  resume that neither opens the picker nor starts fresh, and the tab sits
  on a dead PTY. Pre-existing for every fast exit; this made it matter.
- The resume-tail capture checks one boolean on the data path, closed by
  the existing survive timer, instead of a time comparison per chunk for
  the whole life of every resumed session.
- Without agent hooks the picked session cannot be reported, so the next
  relaunch cannot find it again: the toast now says to install hooks.
  Not a regression (it started fresh before); the picker still gets the
  user their conversation now.
- The Settings hint no longer promises an empty picker field stays empty
  on a built-in agent: the load-time backfill restores it, as it does for
  the other capability lists.

Full e2e suite: the same 11 failures in the same 8 spec files as a clean
main build (85b4d12), none only on this branch.

Refs simion#311

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

@simion simion left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged, thanks @GabrielDumbrava. Race reorder is correct and the picker loop avoidance holds up, only a minor cosmetic nit noted, nothing blocking.

@simion
simion merged commit c4e17e5 into simion:main Sep 21, 2026
3 checks passed
simion added a commit that referenced this pull request Sep 21, 2026
A saved `procmon` window-state entry of 880x620 came back as 440x310
LOGICAL on a 2x display, under this window's own 560x320 minimum. The
plugin puts prior bounds back verbatim and does not honour
min_inner_size, which the main window already clamps for (lib.rs, "it
does NOT enforce minWidth / minHeight"); this window had no clamp.

At 440 the row grid's first column collapses and the process name
renders at ZERO width. The title is in the DOM and invisible, so every
row reads as bare numbers with no name at all.

Measured both ways rather than reasoned about: with the saved entry the
webview is 440 CSS px and the name span is 0px; with the entry deleted,
880 and 85px. `activity.e2e.ts` "names the agent row after the tab" is
what catches it, and it failed the same way on 85b4d12, so this predates
#319.

`procmon` joins "main" in skip_initial_state and the open restores
POSITION alone, so the monitor opens at its built 880x620 every time. A
monitor panel is a fixed-shape table; opening at its designed size costs
less than remembering one that can come back unusable. Main and profile
windows share the plugin's behaviour but survive it, because their
halved size still clears their 900x600 minimum.

Two clamps I tried first and dropped: one after `build()` measures the
builder's size, because the plugin's restore lands later (logged 880x620
at build, 440x310 twenty seconds on), and an explicit
`restore_state(SIZE|POSITION)` there did not take either.

No new test. The existing case is the regression test and now passes
deterministically instead of depending on what the developer's own
window-state file happens to hold.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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.

[feature request] When a session resume fails, say so and offer the recent sessions instead of starting fresh in silence

2 participants