Skip to content

fix(native-chat): offer a resume for every chat that was working, and say what it was doing - #22560

Merged
brennanb2025 merged 22 commits into
mainfrom
brennanb2025/resume-all-working-chats
Sep 25, 2026
Merged

brennanb2025 merged 22 commits into
mainfrom
brennanb2025/resume-all-working-chats

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 15 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​911 $\color{#cf222e}{\Huge{\mathbf{−}}}$​438 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​473
Prod 37 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​870 $\color{#cf222e}{\Huge{\mathbf{−}}}$​421 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​449

Offer a resume for every chat that was working, and say what it was doing

ELI5

When Orca quits or installs an update, it offers to resume chats that were busy. Until now it only offered a chat if its main agent was mid-reply. A chat whose main agent had finished but whose subagents, background commands, or monitors were still running got no offer; neither did a chat waiting on the user to approve something or answer a question. Those chats quietly stopped, and the user had to find each one and type "continue".

This change offers a resume for every chat the sidebar showed as working, and each row in the dialog says what that chat was doing — "2 subagents running", "Waiting for your answer: Can I attach…" — using a note Orca writes down at the exact moment it stops the chat.

Builds on #22448 (merged), which reworked the same dialog and its recovery records.

Before and after, as the user experiences it

  • Before: after an update, chats with running subagents or CI watches got no resume offer and died silently. A chat that was offered could be refused as "could not be resumed" because pressing Resume made Claude post its own "didn't finish before the previous session ended" notice first, and Orca mistook that notice for the user moving on. Offers silently vanished after 24 hours even if the user never saw them.
  • After: every chat the sidebar showed as working is offered, each row names what was stopped, Claude's own notice cannot withdraw an offer, and an offer stays until the user ends it — by resuming, dismissing, sending their own message in that chat, or closing the chat. Each of those endings deletes the record outright.

The mechanism

  1. One question decides "was this chat working?" The same check the sidebar renders (structuredAgentSessionShowsWork: the main agent's state folded with the provider's live background-task roster) is asked once per chat, right before shutdown stops that chat's provider process. That one answer is the offer.

  2. The offer is one stop-time snapshot, and the snapshot is also the description. At that same moment — one read of the chat's journal and one read of the live task roster — the offer records what the chat was doing: the main agent's own state (working / blocked / done, the exact vocabulary the status store already uses), the prompts it was waiting on (at most 4, labels capped at 200 characters), and the live background tasks (at most 16, same task-kind vocabulary the sidebar uses). No IDs are stored. The row's description of what the chat was doing is read only from this snapshot; the chat's name and last prompt are still read live. The description follows the sidebar's wording: "Monitoring" is only used when the main agent had finished, because while a reply is running the task list also contains that reply's own foreground command.

    Why a snapshot and not a later read: "what the sidebar showed at quit" exists only in memory at that moment. Once the chat is reattached after the restart, the provider rewrites its journal in its own words — restated subagent rows, a notice turn of its own — so any post-restart reading is reading a different document than the one the user saw. The previous iteration of this branch stored a journal position and re-derived the description from rows written after it; that needed revision plumbing, deduplication, epoch handling, and a mid-reply judgment over settlement rows. All of that is deleted here. The snapshot is bounded, display-only, and cannot be contradicted by anything that happens later.

  3. An offer ends only by the user's own actions, and every ending deletes it.

    • The user sends a message in that chat: the next listing notices the chat's newest user message no longer matches the one the offer recorded, and deletes the record — the same derive-then-prune rule the failure records already used, now shared by both.
    • Resume succeeds, or the user dismisses: already deleted; unchanged.
    • The user closes the chat: closing routes every "this chat is gone" path (tab close, close call, worker settlement, worktree removal) through one funnel, and that funnel now also deletes the chat's offer and any failure record. The deletion is advisory — it can never block the close itself.
    • The conversation was forked after the quit (a rewind): the offer can never be shown again, so the next listing deletes it the same way it deletes one ended by a newer user message. Deletion happens only on proof — an unreadable journal or a missing record keeps the offer.
    • There is no expiry. The 24-hour TTL is removed for offers and failure records. An expiry answered no failure these rules don't already answer better: an offer is not an obligation Orca acts on by itself — it is a question waiting for the user — and silently deleting a question because the user was away for a day is exactly the wrong behavior for the person who most needs the resume. Clock skew needs no special case either: a skewed timestamp cannot strand anything, because every ending is a user action, not a timer.
  4. The pre-send wait is gone. Before sending the "carry on" message, the previous code drained the provider's event stream for up to a bounded window so a racing user message might become visible first. That wait existed to feed a check that no longer reads anything the provider can change: the only thing that withdraws an offer is the user's own newer message, checked against the journal immediately before dispatch (stillResumable). The drain narrowed a race without closing it, added a timeout, a pending-events probe, and a failure mode of its own — deleted, along with the plumbing that carried it through four files. A pinned test now asserts the send path never touches the stream barrier at all.

  5. The continuation message is a function of the offer alone. The durable ledger fingerprints the message body, so every retry of one offer must produce byte-identical text. The wording is chosen by whether the offer carries a snapshot; nothing from the journal enters it.

Compatibility

  • The snapshot is a new optional field on the stored offer and on the wire row. An older build strips it and still reads the offer (unknown keys pass); a snapshot this build cannot read is dropped without costing the offer. The stored work identity gains no new kinds, because an unknown kind would make an older build reject the whole recovery file.
  • Older clients ignore the optional wire field; a marker written by an older build simply renders with no activity line and the original continuation wording.

Why this approach over the alternatives

  • Snapshot vs. journal re-reading: the journal is rewritten on reconnect, so a re-read can disagree with what the user saw; the snapshot cannot. It also deletes roughly 250 lines of re-derivation machinery.
  • Deletion vs. filtering: filtering a dead record on every read keeps it forever and multiplies the states a reader must handle. Every ending now removes the record, so "listed" means exactly "still owed".
  • No expiry vs. TTL: a TTL is a second withdrawal rule beside the user's own actions, and its only observable effect was destroying offers for users who were away. Nothing re-derives an offer after expiry, so the TTL could only lose work, never recover it.
  • No pre-send drain: the decision it served no longer depends on provider events; keeping it would be a guard around a race it cannot close.

Linked Issue

No issue. Reported directly: after an update, one chat was refused and six others were missing from the dialog.

Visual Proof

Same test profile and dev build. Before is the dialog as #22448 left it; after is this build with two seeded offers: a chat whose subagents and CI watch were stopped, and a chat stopped while waiting on a question.

Before After
before after

Testing

Latest review round (head 780b320dbd):

  • Live run of this build on macOS, hidden window, fresh test profile, real Claude chats and a real app restart — no hand-written recovery files. Five chats: one whose main agent had finished while a background shell kept running, one whose background subagent left shells running, one waiting on a question, one mid-reply running a long command, and one idle. After the restart the dialog listed the four working chats and not the idle one, each with its description. Resume sent the continuation message and the agent carried on; sending my own message in another offered chat deleted its offer; Dismiss all emptied the recovery file. Dialog after restart · Sidebar before restart · Resumed chat
  • That run found the mid-reply row reading "Was mid-reply · Monitoring: <the reply's own command>"; fixed and pinned by test (deleting the fix fails the test).
  • An earlier commit on this branch had also changed when an unwatched chat's agent process is stopped for being idle, which dropped the rule that keeps a message sent while Claude is still starting. That change is reverted: idle stopping is exactly as on main, and the starting-message tests pass again.
  • CI green; pnpm tc, oxlint, the changed-code gate and all four localization gates pass.

Earlier rounds:

All on this branch, pnpm tc clean and full pnpm lint (all four localization gates) passing.

  • Suites run green: the native-chat wire restart/resume suites (offer capture, endings, reattach, ownership, failure filing, teardown, holds, sends, settlements — 300+ tests across the touched files), the recovery capsule suites (37 tests), the resume dialog, activity label, and status-bar segment suites (78 tests), the RPC method suites (81 tests), and cross-version-agent-session-wire.unit.test.ts alone: 14 of 14.

  • Behaviors pinned by test and verified by deletion ablation — each behavior was deleted in source, its test confirmed to fail for that reason, and the source restored (9 ablations):

    • the snapshot is captured before the chat's process stops (moving it after the stop reddens the "work running at the stop" trio);
    • the snapshot records the live task roster (a subagent the close settles moments later still appears);
    • a blocked chat's state and its prompt come from one read;
    • the offer's description is the snapshot, untouched by post-restart journal rows;
    • a user send withdraws and deletes the offer;
    • closing the chat deletes its offer and its failure record;
    • offers and failure records have no expiry (re-adding one reddens the months-old-offer tests);
    • the continuation body is a function of the offer alone (retries are byte-identical);
    • the continuation's own message does not count as the user moving on (deleting that guard refuses all four reattach continuations).
  • Replayed a real failure through the new rule: a chat refused by the current release on 2026-09-24 (a background subagent was running at the update; on Resume, Claude opened and closed its own notice turn 2 s before the continue message). With that exact journal, session record and stored offer, the new rule keeps the offer (1 offered, 0 withdrawn).

  • Ran this build on macOS against a test profile with two seeded offers in the new format: the host listed both with their snapshots and the dialog rendered them (screenshot above). Resume was not pressed in that run.

  • Not tested here: Linux, Windows, SSH hosts. The change is host-side logic plus a renderer label, with no platform branches.

  • I manually tested these changes locally

  • Automated tests added/updated

Known limits

  • Only a clean quit, restart or update records offers. A crash or force-kill records none, as before.

  • A chat whose main agent had finished and which nobody was viewing can still be stopped for being idle (30 minutes) before the quit, as on main; if so its background work ended then, and it is not offered at the quit.

  • The status-bar count of chats to resume refreshes when the dialog is opened, not the moment the user sends a message in an offered chat. This was already the case before this change.

  • A chat whose reply never appears in the transcript can read as working in the shared status projection indefinitely; such a chat would also be offered a resume it may not need. That projection bug is a separate issue and is out of scope here.

  • The user-send race the old pre-send drain narrowed still exists in its irreducible form: a message accepted by the provider but not yet journaled at the instant of dispatch cannot refuse the continuation. Both providers queue a message sent mid-turn, so the cost is one extra "carry on" message, not lost work.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one design question in the body and one wording nit inline.

Reviewed changes

  • One "is this working?" predicate — structuredAgentSessionShowsWork folds the lead projection with the provider's live background roster exactly as the sidebar does, and teardown capture, the release clock (isWorking, replacing isTurnActive), and the restart candidate reader all now compose it.
  • Journal position instead of a work copy — markers carry an optional journalCursor; structuredAgentSessionRestartCutOff reads back what the shutdown settled (cancelled prompts, failed root tool calls, unverifiable subagents/background tasks) through journal.readSince, so the offer stores no duplicate of the work.
  • Predicate rewrite — journalTurn(turnId)/newestJournalTurn/liveWork/owedWork, a childWorkAtStop capture, and an admitted activity read before reattach and carried through the dispatch-time stillResumable check.
  • Marker-dependent continuation body — restartContinuationMessage(marker) keeps the original wording for old markers and uses a broader "work" wording once a journalCursor exists, so one offer's retries fingerprint the same body.
  • Renderer — resumeActivityLabel and the row's activity line, modal/settings copy switched from "mid-turn" to "working", and new i18n keys.

ℹ️ An unheld session can now wait out provider child work indefinitely

The release clock re-arms every grace window while isWorking is true, and isWorking counts any child task whose state is absent or non-terminal (isLiveChildWork). Codex publishes every task with no state at all, so any task it still lists pins the session, and the only thing that clears that roster is the eviction the clock is deferring. That is the intended trade — this PR exists so running subagents, commands and monitors are not killed with the tab — but there is no ceiling, so a provider child that never sends a terminal event keeps its app-server alive for the life of the process. Open question for the human: should there be a max hold on the deferral, or is terminal-event delivery reliable enough that the sidebar and the clock can both stay unbounded?

Technical details
# Unbounded release-clock deferral on child work

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-release-clock.ts` (new line 67) — a true `isWorking` re-arms instead of evicting; `arm()` only schedules another `graceMs`. No counter, deadline, or max hold exists.
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts` (new lines 206–214) — `isWorking` now folds background tasks; the removed predicate (`activeStructuredAgentSessionTurnId`) was non-null only while the lead's own turn ran.
- `src/shared/agent-status-child-work-liveness.ts:30-32` — `state !== 'done' && state !== 'idle'`, so absent state and `waiting`/`blocked`/`unverifiable` are live.
- `src/main/codex/codex-background-task-tracker.ts:61-74` — every emitted task is `{ id, kind, description }` with no `state`.
- `src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts` (new lines 436–453) — codifies the deferral: with `tasks` non-empty, `closeSession` is not called and the session survives several grace windows; it evicts only after `tasks = []`.

## Required outcome
Either a bound (max hold / max attempts / an absolute deadline) on how long child work may defer eviction, or an explicit accepted-risk note that the release clock may hold an app-server for the process lifetime when a provider never settles a child.

## Open questions for the human
- Is provider terminal-event delivery assumed reliable enough that no ceiling is needed?
- Should the clock read only `state === 'working'` child work rather than every non-terminal state?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 force-pushed the brennanb2025/resume-all-working-chats branch from 019889c to 50eb7fd Compare September 24, 2026 03:52
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/fix-chat-resume-toast to main September 24, 2026 03:52
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Native chat restart offers now capture lead state, prompts, and live child tasks before the provider child stops. Offers remain eligible across elapsed time and journal turn changes, but a newer user message can supersede them. Continuation messages use the captured activity, and dispatch no longer waits for streamed-event flushing. The resume interface displays available activity details. Session working-state checks also include background tasks.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 9d829

Restart offers are meant to persist until the user acts on them. However, if a chat's history cannot be read after restart, its offer is silently and permanently deleted, which can lose the resume prompt this change exists to preserve. A minor localization-convention issue also remains. Resolve the offer-deletion path before merging.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 50 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description thoroughly explains the behavior, implementation, user impact, visual changes, testing, compatibility, and known limits. However, it does not satisfy the required Linked Issue section … Add the issue reference in the required “Linked Issue” section, using the required “Fixes #” format. Complete any remaining required template sections or explicitly mark them as not applicable.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: offering resume options for all working chats and showing what each chat was doing.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 50 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description thoroughly explains the behavior, implementation, user impact, visual changes, testing, compatibility, and known limits. However, it does not satisfy the required Linked Issue section because it states “No issue” instead of providing an issue reference.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: d36a37b0-bcf0-4079-afe2-765b42f502cf

📥 Commits

Reviewing files that changed from the base of the PR and between 3ea15dd and 50eb7fd.

📒 Files selected for processing (33)
  • src/main/native-chat/agent-session-wire/structured-agent-session-hold-resume-race.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-holds.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-holds.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-release-clock.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-candidates.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-child-work.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-continuation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-cut-off.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-cut-off.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-interruption-test-harness.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-ownership.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-test-harness.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-teardown.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-shown-work.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
  • src/renderer/src/components/NativeChatResumeOnRestartAgentRow.tsx
  • src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx
  • src/renderer/src/components/NativeChatResumeOnRestartModal.tsx
  • src/renderer/src/components/native-chat-resume-activity-label.test.ts
  • src/renderer/src/components/native-chat-resume-activity-label.ts
  • src/renderer/src/components/native-chat-resume-on-restart-grouping.ts
  • src/renderer/src/components/settings/NativeChatExperimentalSetting.tsx
  • src/renderer/src/i18n/locales/en.json
  • src/shared/agent-session-restart-activity.ts
  • src/shared/agent-session-restart-continuation.test.ts
  • src/shared/agent-session-restart-continuation.ts
  • src/shared/agent-session-resume-marker.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment on lines +30 to +83
? translate(
'auto.components.NativeChatResumeOnRestartModal.activityApproval',
'Waiting for your approval: {{value0}}',
{ value0: named(prompt.label) }
)
: translate(
'auto.components.NativeChatResumeOnRestartModal.activityQuestion',
'Waiting for your answer: {{value0}}',
{ value0: named(prompt.label) }
)
)
} else if (activity.midReply) {
parts.push(
translate('auto.components.NativeChatResumeOnRestartModal.activityMidReply', 'Was mid-reply')
)
}
const agents = activity.tasks.filter((task) => task.kind === 'agent')
const watches = activity.tasks.filter((task) => task.kind !== 'agent')
const [onlyAgent] = agents
const [onlyWatch] = watches
if (agents.length === 1 && onlyAgent) {
parts.push(
translate(
'auto.components.NativeChatResumeOnRestartModal.activitySubagentOne',
'Subagent running: {{value0}}',
{ value0: named(onlyAgent.label) }
)
)
} else if (agents.length > 1) {
parts.push(
translate(
'auto.components.NativeChatResumeOnRestartModal.activitySubagentMany',
'{{value0}} subagents running',
{ value0: agents.length }
)
)
}
// Shells, monitors and workflows alone are what the sidebar labels monitoring.
if (watches.length === 1 && onlyWatch) {
parts.push(
translate(
'auto.components.NativeChatResumeOnRestartModal.activityMonitorOne',
'Monitoring: {{value0}}',
{ value0: named(onlyWatch.label) }
)
)
} else if (watches.length > 1) {
parts.push(
translate(
'auto.components.NativeChatResumeOnRestartModal.activityMonitorMany',
'Monitoring {{value0}} background tasks',
{ value0: watches.length }
)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rename the positional {{value0}} placeholders in the new activity strings.

The new strings use intent-named IDs. Their interpolation placeholders are positional {{value0}}, not named. Rename each placeholder to match its meaning. Examples: {{prompt}} for approval and question, {{name}} for one subagent or one monitor, {{count}} for the "many" forms. Update the matching entries in src/renderer/src/i18n/locales/en.json in the same change.

Example for one string
         : translate(
             'auto.components.NativeChatResumeOnRestartModal.activityQuestion',
-            'Waiting for your answer: {{value0}}',
-            { value0: named(prompt.label) }
+            'Waiting for your answer: {{prompt}}',
+            { prompt: named(prompt.label) }
           )

This follows the retrieved learning for stablyai/orca: "use intent-named localization IDs with named interpolation placeholders."

Source: Learnings

…opened

The offer's cut-off work was read from the items' current state, so once the
chat was opened (reattaching the provider, which rewrites the rows it lost),
a chat offered for its stopped subagents or background tasks dropped out of
the dialog and its retry. Judge the revision rows written after the marker's
cursor instead: they only accumulate, so the reading is the same before and
after reattach, and the per-run pre-reattach copy is no longer needed.

Also pass the lease fence into the shared "shows work" check, as the status
feed does, so a send stranded under an older fence cannot hold an unheld
session's provider alive while the sidebar shows it idle.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one stale comment inline; the redesign reads correctly.

Reviewed changes

  • The cut-off is read from journal rows, not the folded items — journalItemRevisions returns each item row's { itemId, body } (canonicalized) and structuredAgentSessionRestartCutOff judges those bodies, so a provider restatement after reattach no longer erases the settlement's unverifiable / cancelled / failed bodies. The admitted pre-reattach activity capture is removed from the resume set, the candidate reader, and the continuation host.
  • Repeated revisions are named once — a first() guard dedups prompts by item id and children by itemId + agent.id / taskId.
  • "Shows work" now takes the lease fence — structuredAgentSessionShowsWork forwards the session fence to projectStructuredAgentSessionStatus, so a send left pending under an older fence no longer keeps an unheld session's provider alive.
  • Tests — new structured-agent-session-shown-work.test.ts and the "user opened the chat before resuming" host test; the cut-off suite's history fixtures model overwritten revisions; teardown/ownership tests updated for the attention-offer behaviour.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

})

// On resume the provider restates what it lost in its own words, rewriting the row before the
// continuation is dispatched. What the offer was admitted for still stands.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment still refers to admitted, the pre-reattach activity capture this commit removes. Reword it to describe the current mechanism — the settlement's journal rows staying put after the provider restates — so it does not point at an abstraction that no longer exists.

brennanb2025 and others added 7 commits September 23, 2026 23:31
…each child stops

Every review loop found the same bug class in the "is this offer still owed?" re-derivation that
ran when a child was stopped and again after restart. It re-read a journal the provider had already
rewritten on reattach (a notice turn of its own, restated subagent rows) and kept misreading it:
a send made after an earlier completed turn was dropped at shutdown, and Claude's notice turn
refused the resume after restart.

- Teardown now snapshots each session with the sidebar's working check in a new eviction step
  right before its provider child is stopped (after draining events Orca already accepted), and
  keeps it once the stop is proven. The capture-first/confirm-on-stop re-judgment is gone.
- After restart an offer is withdrawn only by a newer user message, dismissal or expiry, beside
  the structural checks (record, support, lease, no fork). Listing, retryable and the pre-send
  check no longer re-derive work state.
- The pre-send barrier drains again while a provider keeps streaming instead of refusing; both
  providers queue a message sent mid-turn.
- Row activity stays a display-only read of the rows after the marker's journal cursor.
… restart continuation

The drain before the continuation's pre-send check refused the send when provider events were
still arriving at its 2 s bound or the barrier failed. That refusal journaled the continuation as
rejected, so its own message then read as the user moving on and the offer could never be
retried. The drain is now best effort and the pre-send check judges what the journal holds.

Co-Authored-By: Claude <noreply@anthropic.com>
…ing once

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the redesign reads correctly; one gap in the offer-withdrawal rule inline, plus a stale safety comment.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (64f53b1): the offer rule was replaced with a per-stop snapshot and the "is it still owed?" re-derivation was deleted.

  • Offer taken right before each child stops — a new snapshot-before-stop eviction step captures structuredAgentSessionWorkingAtStop (the same working check the sidebar shows) before the provider child stops, and the witness keeps it verbatim once the stop is proven; the old capture-then-re-judge path is gone.
  • Predicate reduced to structural checks plus one withdrawal rule — waitingOnUser, liveWork, owedWork, journalAgreesWorkWasCutOff, journalTurn/newestJournalTurn, childWorkAtStop, providerStopped and turnWasCutOff were removed; a marker now stands unless a newer user message, dismissal, expiry, a fork, or the lease withdraws it.
  • Row activity is display-only — structuredAgentSessionRestartActivity names what the teardown cursor read back and returns undefined for markers with no cursor; nothing about it decides the offer.
  • Admission barrier re-drains — drainAdmissionEvidence keeps flushing while the provider still streams accepted events instead of refusing after a single flush.
  • Witness/teardown rewiring and tests — begin/beforeStop/stopped replace the capture/confirm marker API, the quit phase is begin-resume-markers, and the new offer-after-reattach suite plus the send-after-reply harness case pin the notice-turn behavior.

ℹ️ Nitpicks

  • src/shared/agent-session-resume-marker.ts:1-8 still says "the journal's own turn record has to name the same turn before anything is handed a provider child again." The predicate no longer consults the journal's turn record at all, so the marker's own header now contradicts the rule it documents; the last commit aligned the journalCursor comment but not this one.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

) {
// The user moving on is the one thing that withdraws the offer. Anything the provider does on
// its own after reattaching — a turn it opens, a prompt, restated rows — is not.
if (input.latestUserItemId(marker.sessionId) !== marker.latestUserItemId) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The withdrawal rule keys only on a new user message, so a chat the user has already continued by answering the re-asked prompt stays offered: answering a prompt appends a resolution to the existing item (promptPlan → performPrompt), leaving latestUserItemId unchanged. The removed waitingOnUser/liveWork clauses used to withdraw here; without them a later Resume dispatches a duplicate continuation, and the row still reads "Waiting for your answer" from the cancelled prompt. Reachability is low (auto-resume is off by default, and the continuation wording asks the agent to verify), so treat this as a decision to confirm rather than a blocker.

Technical details
# User progress by answering a prompt does not withdraw a restart offer

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts:106-110` — the sole work-withdrawal clause (`latestUserItemId` mismatch).
- `src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts:54-77` and `structured-agent-session-mutation-plans.ts:123-148` — answering appends `{...prompt, resolution}` to the existing item; no user message is written.
- `structured-agent-session-restart-resume-host.ts:165-171` (`list`) and `:176` (`stillResumable`) both re-run the same predicate, so both see the resolved prompt as still an offer.

## Required outcome
A chat the user has already progressed after restart — by answering the prompt this offer says it lost, or by the resumed agent running a turn in response — should not still be offered, or the PR should state explicitly that only a typed message withdraws and this case is accepted.

## Suggested approach (optional)
A post-cursor row whose prompt resolution is `resolved` (rather than teardown's `cancelled`) is the cheapest discriminator; it is already what the activity read walks.

## Open questions for the human
- Is "the provider re-asks the cancelled prompt and the user answers it" the user moving on, or is the offer meant to survive until they type a message?

… the turn as restated

A reattached provider may restate the offer's turn, so the row's mid-reply label read off the
turn's current state could vanish and count the cut-off reply's own tool calls as background
commands. Also corrects the ineligible-offer comment to match the user-moved-on rule.

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in this delta — the two new commits are a self-contained, well-covered fix, and both new test cases would fail under the old refusal path. Approval stays withheld only because prior Pullfrog threads remain open (see the rolling summary); nothing in these commits re-raises them.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (7e24b39): the restart continuation's admission drain is now best-effort.

  • The admission drain no longer refuses the send — drainAdmissionEvidence in structured-agent-session-turns.ts became Promise<void> and returns on timeout or a failed flush instead of throwing agent_session_admission_evidence_unavailable; dispatchSafely's beforeDispatch wrapper now drains and then always runs the real pre-dispatch check. The error code is fully gone from the tree.
  • Why it matters for retries — the old refusal journaled the continuation's submission as rejected, so a later retry replayed that rejection (same clientOperationId, same fingerprint) and the offer could never be retried. The commit message and the new tests document this.
  • Tests recast to the new behavior — structured-agent-session-operation-settlement.test.ts no longer asserts a refusal; it asserts a send dispatches once past a stalled or failed barrier and never late, plus a new superseded-send case where a stalled refusal write still resolves after the bound. structured-agent-session-restart-offer-after-reattach.test.ts adds a case that keeps the provider streaming (or fails the drain) and still expects outcome: 'continued' with exactly one dispatch.

I verified locally that pnpm test on both touched test files passes (12 tests). I also confirmed the withdrawal check reads only root user message rows (latestStructuredAgentSessionUserItem), which provider streamed events never produce — so a failed drain cannot miss a newer user message and the best-effort behavior cannot manufacture a stale withdrawal decision.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in this delta — the one new commit is a display-only label fix with a test that would fail without it. Approval stays withheld only because prior Pullfrog threads on this PR remain open (see the rolling summary); nothing in this commit re-raises them.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (bc61a10): a single commit that corrects the row's mid-reply label after a provider restatement.

  • The cut-off reply is judged on the settlement's rows, not the restated turn — leadWasMidReply in structured-agent-session-restart-cut-off.ts now takes the revisions since the marker cursor and returns true when any revision for the marker's turn id settled as interrupted/unverifiable. A reattached provider restating that turn as completed or running no longer erases the label; the backwards walk of render items remains only as the fallback for an unreadable cursor read (absent cursor or an epoch mismatch).
  • Why it matters beyond the label — with midReply correctly held true, the reply's own failed tool calls are no longer counted as independent background commands, so the row's activity line stays honest.
  • Doc correction — the failure ledger's ineligible-offer comment now describes the user-moved-on rule (expired, record gone, conversation forked) instead of the removed "finished, or waiting on the user" wording.
  • Test — a new it.each(['completed', 'running']) case seeds the interrupted revision in history and the restatement in items, asserting midReply: true with no tasks.

I verified locally that pnpm test passes on structured-agent-session-restart-cut-off.test.ts and structured-agent-session-restart-failure-filing.test.ts (25 tests), and oxlint is clean on the three changed files. The new case is meaningful: under the pre-fix backwards walk the restated completed/running turn yields midReply: false and the failed tool call is emitted as a task, so the exact-equality assertion fails.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

The marker records the main agent's own state, the pending prompts and the
live child roster at the moment before its child stops; the dialog row,
status bar and candidate read only that snapshot. Deletes the journal
cut-off reader, journalCursor, the offer TTL and the pre-send admission
drain. Superseded offers and chat closes now delete their records.
…ll-working-chats

# Conflicts:
#	src/shared/agent-status-child-work-liveness.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

A pending restart offer is permanently deleted when its chat's journal cannot be restored — not by a user action, which is the only ending this PR intends.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (74b662e): the offer's description moved onto the marker as a stop-time snapshot, and an offer withdrawn by the user's own message is now deleted rather than filtered.

  • The description is now the marker's own snapshot — structuredAgentSessionWorkingAtStop records the lead's own state, the pending prompts and the live child roster from one structuredAgentSessionShownStatus read, and the marker carries activity in place of journalCursor; structured-agent-session-restart-cut-off.ts and its journal re-derivation are deleted.
  • Withdrawal became a deletion ending — structuredAgentSessionResumableSet returns { candidates, superseded }; list() and run() call a new retireSuperseded, which drops the durable record witness-keyed by recordedAt through the capsule's forgetSuperseded (replacing forgetFailures).
  • The admission drain is gone — drainAdmissionEvidence and hasPendingStreamedEvents are removed from the turns, host-mutation and admission types; dispatchSafely now calls the pre-dispatch check without touching the event-stream barrier, pinned by the new dispatches without touching the event-stream barrier test.
  • Offer TTL removed — AGENT_SESSION_RESUME_MARKER_TTL_MS and isExpiredAgentSessionResumeMarker are gone; the capsule keeps only the action-lease reclaim.
  • New extraction and tests — structured-agent-session-restart-offer-records.ts holds the durable read/reveal/retire, and structured-agent-session-restart-offer-endings.test.ts pins that a user send and a chat close delete the offer and any failure record.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: d53fe28f-05f7-4193-8cbc-59843ee1c857

📥 Commits

Reviewing files that changed from the base of the PR and between bc61a10 and 9d82962.

📒 Files selected for processing (33)
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-tabs.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-operation-settlement.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-candidates.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-child-work.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-failure-ledger.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-offer-after-reattach.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-offer-endings.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-offer-records.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-ownership.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-set.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-test-harness.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-teardown.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-shown-work.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
  • src/main/runtime/agent-session-recovery-capsule-entries.ts
  • src/main/runtime/agent-session-recovery-capsule.test.ts
  • src/main/runtime/agent-session-recovery-capsule.ts
  • src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx
  • src/renderer/src/components/native-chat-resume-activity-label.test.ts
  • src/renderer/src/components/native-chat-resume-activity-label.ts
  • src/renderer/src/i18n/locales/en.json
  • src/shared/agent-session-restart-activity.ts
  • src/shared/agent-session-restart-continuation.test.ts
  • src/shared/agent-session-restart-continuation.ts
  • src/shared/agent-session-resume-marker.ts
  • src/shared/agent-status-child-work-liveness.ts
💤 Files with no reviewable changes (3)
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/src/i18n/locales/en.json
Files not reviewed due to moderation or processing errors (2)
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-child-work.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Comment on lines +110 to +113
// The user moving on is the one thing that withdraws the offer. Anything the provider does on
// its own after reattaching — a turn it opens, a prompt, restated rows — is not.
if (input.latestUserItemId(marker.sessionId) !== marker.latestUserItemId) {
superseded.push(marker)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
rg -nP -C4 'revealSession\s*=|revealSession\(' src/main/native-chat/agent-session-wire
fd -i 'structured-agent-session-reveal' src --exec sed -n '1,120p' {}

Repository: stablyai/orca

Length of output: 16602


Preserve restart offers when the session is unreadable.

When sessions does not contain the marker's session, latestUserItemId returns null and itemsFor returns an empty list. A non-null marker value therefore differs from null, so structuredAgentSessionResumableSet adds the marker to superseded. The restart host swallows revealSession failures, so this state can occur without a user action. forgetSuperseded then deletes the durable offer.

Return undefined when the session journal is not readable, and skip supersession for that result. This preserves the offer until a readable journal can decide whether the user sent a newer message.

🐛 Suggested fix
 export type StructuredAgentSessionResumeSetInput = {
   ...
-  latestUserItemId: (sessionId: string) => string | null
+  /** `undefined` when the chat's journal is not readable here: that decides nothing. */
+  latestUserItemId: (sessionId: string) => string | null | undefined
-    if (input.latestUserItemId(marker.sessionId) !== marker.latestUserItemId) {
+    const latestUserItemId = input.latestUserItemId(marker.sessionId)
+    if (latestUserItemId === undefined) {
+      // An unreadable journal decides nothing; the record stays until something can decide.
+      continue
+    }
+    if (latestUserItemId !== marker.latestUserItemId) {
       superseded.push(marker)
       continue
     }
-      latestUserItemId: (sessionId) =>
-        latestStructuredAgentSessionUserItem(itemsFor(sessionId))?.itemId ?? null,
+      latestUserItemId: (sessionId) =>
+        deps.sessions.has(sessionId)
+          ? (latestStructuredAgentSessionUserItem(itemsFor(sessionId))?.itemId ?? null)
+          : undefined,

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review complete after the follow-up fixes.

  • Preserved restart offers when the chat journal cannot be read; unreadable history no longer appears as a newer user message and silently retires the offer.
  • Added the regression test covering that failure path.
  • Updated the affected locale strings and corrected the stale restart-offer comment.
  • Removed unrelated local pnpm lockfile churn from the change.

Validation passed: 14 targeted test files (128 tests), focused offer-ending/resume tests (48 tests), pnpm tc:node, oxlint on changed files, check:code-quality:changed, and git diff --check.

No proven SSH, security, performance, compatibility, cross-platform, or code-organization issues were found in the final review. The changes are pushed in commit c436bbdcf4a.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found in this delta.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (198bd01): a single commit that keeps a restart offer alive when its chat cannot be read here, plus doc and translation catch-ups.

  • An unreadable journal no longer deletes an offer — the candidate reader's itemsFor now returns undefined (instead of []) when the chat is absent from the live session map, and latestUserItemId propagates that undefined; structuredAgentSessionResumableSet only withdraws when the id is defined and disagrees with the marker. A failed restore — corrupt journal, missing journal.db, pre-SQLite remnant — therefore leaves the offer durable rather than retiring it from the capsule.
  • The offer predicate now matches the failure ledger's guard — liveStructuredAgentSessionLatestUserItemId already returned undefined for the same "not readable, decides nothing" case, so the two paths now agree.
  • Test — keeps the offer when the chat cannot be read on this host rejects restoreOne, asserts the offer survives the listing while the mock is active, and re-asserts it after the mock is released. The assertion is real: the interrupted-restart harness builds a fresh host with an empty session map, so under the old []/null path the marker would have been treated as superseded and deleted.
  • Docs and translations — the wire-type comment and the ResumeFailure comment now describe the stop-time snapshot and the close-the-chat ending, and the activity* keys plus the "working" wording were filled in for fr/ja/ko/zh.

Approval is withheld only because earlier pullfrog threads on this PR remain open; nothing in this delta re-raises them.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Full review loop completed at c436bbdcf4a.

Verdict: CLEAN. The independent review found no proven functional, architectural, compatibility, security, performance, cross-platform, or organization issues, and made no additional code changes.

Validation:

  • Readiness checklist completed at the start and end with no candidate findings.
  • Restart/reattach/ownership suites: 44 tests passed.
  • Renderer resume modal/activity/grouping suites: 42 tests passed.
  • Node typecheck, oxlint, changed-code quality checks, and git diff --check passed.
  • Electron CDP validation attached to the intended PR worktree (tope @ brennanb2025/resume-all-working-chats), rendered the real Orca UI, and captured visible evidence with no console errors. Screenshot: Orca rendered UI

The review worker-start path was unavailable before preamble acknowledgement in this runtime; those attempts were recovered and explicitly abandoned. The independent same-model review completed successfully. No mobile or SSH path is touched by this change, so those validations are not applicable.

@brennanb2025

brennanb2025 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Attachments

orca-pr22560-electron-worktree.png
orca-pr22560-electron-worktree.png

orca-pr22560-electron-empty.png
orca-pr22560-electron-empty.png

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review complete for PR #22560.

Status: merge conflicts are resolved and the branch is mergeable. The review found no proven P0, P1, or P2 issue. One in-scope lifecycle fix was added: an admitted send with no active turn can now be settled by idle eviction, while active turns, prompts, and background tasks keep the provider alive. Unused recovery plumbing was removed.

Validation:

  • 164 targeted tests across 10 restart, capsule, lifecycle, and activity-label suites passed.
  • pnpm tc:node passed.
  • git diff --check and formatting/lint hooks passed.
  • Readiness review completed at the start and end with no release-blocking findings.
  • Hidden Electron validation ran with ORCA_BACKGROUND_LAUNCH=1; the intended PR worktree identity was verified and the real renderer had zero console errors. The restart RPC returned no candidates in the disposable profile, so the seeded activity dialog, Resume/Dismiss actions, and activity labels remain unverified. The uncropped real app screenshot is attached in the preceding comment.
  • Linux, Windows, SSH, and mobile paths were not exercised because the changed surfaces are native-chat host/renderer code without those platform branches.

The unrelated dirty pnpm-lock.yaml change was left out of the pushed commits.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

aba58c6 breaks a pinned user-facing guarantee: a message sent while the Claude CLI is still starting no longer counts as owed work, so the idle release can evict the session and refuse the message the user already sent. Two tests in structured-agent-session-starting-release.test.ts fail on this commit.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (c436bbd): aba58c6 plus two merges from main (base noise — the net branch change is the single commit below).

  • Idle release now excludes admitted sends — isWorking is renamed hasOwedWork, and the lifetime predicate moved to a new structuredAgentSessionHasOwedWork, which folds the journal with an empty submissions list so an admitted send with no turn no longer pins an unheld provider child.
  • The provider child phase reaches dispatch — providerChildPhase is threaded from host-mutations.ts through mutation-admission.ts into AgentSessionTurnContext, so a failed dispatch against a starting child is classified rejected rather than unknown.
  • Tests — structured-agent-session-shown-work.test.ts gains the owed-work split; surface-lifetime.test.ts swaps its background-work case for an admitted-send eviction case.

ℹ️ Nitpicks

  • structuredAgentSessionShowsWork now has no production caller (only structured-agent-session-shown-work.test.ts imports it), while the file header still calls it "the one way the product answers it". Either delete it or fold the two predicates onto one projection so there is a single answer.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

… released

Idle release had been switched to the sidebar's working check minus pending sends, which dropped
the rule that a send held while the provider CLI is still starting keeps the session. An unheld
chat left during startup was then evicted and its message refused. Restore the release rule this
branch never needed to change: an open turn, or a pending send while the child is starting.
Also keeps the host file within its line budget.
A forked conversation can never become the marked one again, but its offer was only skipped, so
with no expiry it sat unseen in the recovery file forever. Report it for deletion on the same path
as a newer user message. Also corrects comments that still called listing read-only.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in this delta — the flagged regression is fixed and the fork change is sound. Approval stays withheld only because earlier Pullfrog threads on this PR remain open (see the rolling summary).

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (aba58c6): two commits that restore the idle-release rule and delete forked offers.

  • A held-for-startup send is owed work again — hasOwedWork in structured-agent-session-host-lifetime.ts is back to activeStructuredAgentSessionTurnId(...) !== null || (providerChildPhase === 'starting' && pendingSubmissions().length > 0), so an unheld Claude session still starting with a held message is no longer evicted and refused. structured-agent-session-starting-release.test.ts passes (4/4), along with surface-lifetime (22) and restart-resume (36).
  • The unused predicates are deleted — structuredAgentSessionShowsWork and structuredAgentSessionHasOwedWork are gone, leaving shown-work.ts with only structuredAgentSessionShownStatus. This also clears the prior nitpick that structuredAgentSessionShowsWork had no production caller while the header called it "the one way the product answers it".
  • A forked conversation's offer is retired, not re-filtered forever — structuredAgentSessionResumableSet now pushes a marker whose record head identity root differs from marker.providerHandleRoot into superseded (was a silent continue), so with the TTL removed it cannot sit unseen in the recovery capsule indefinitely. This matches the provider-handle contract, where a root change is a fork and only a leaf advance is a resume, and the comment/doc wording was corrected to say listing is not read-only.

The idle-release deferral question from the initial review no longer applies: reverting hasOwedWork to main's rule (a running turn, or a held send while the child is starting) restores the bounded behavior instead of pinning a child for any non-terminal background task.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…rows

The catalog keeps Agent in English for Japanese, and the localization
gate rejects the translated form.
…sume dialog

The live task roster also lists the foreground command a reply is
running, so a chat stopped mid-command read "Was mid-reply · Monitoring:
<command>". Speak monitoring only for an idle lead, as the sidebar does;
the row's tooltip still names every task.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues — the mid-reply monitoring suppression matches the sidebar's fold. Approval is withheld only because two earlier pullfrog threads on this PR remain open; nothing in this delta re-raises them.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (532daba): one renderer commit that stops the resume dialog calling a mid-reply command "monitoring".

  • Mid-reply watches are suppressed, kept in the tooltip — resumeActivityLabel computes non-agent watches only when activity.state !== 'working', so the foreground command the reply itself was running no longer renders as Monitoring: … in the summary line; the row's detail still lists it.
  • Why it is correct — the recorded state is the MAIN agent's own state, and foldAgentLeadStatus emits workingMode: 'monitoring' only for a settled lead, so the sidebar never calls a mid-reply command monitoring either. claude-background-task-tracker.ts confirms live foreground tasks share the roster the snapshot reads.
  • Test — the new exact-equality case (state: 'working' plus a command task → { summary: 'Was mid-reply', detail: 'Count .plist files' }) fails under the old code, which produced Was mid-reply · Monitoring: Count .plist files.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status — head 780b320dbd

Verdict: ready for review. Three review loops (the last two came back clean), an independent design review, readiness checks at the start and end, and a live run in the app.

Fixed this round

  • A message sent while Claude was still starting could be dropped (the red CI). An earlier commit here had changed when an unwatched chat's agent process gets stopped for being idle, and in doing so dropped the rule that keeps a message sent while Claude is still starting. Idle stopping is back to exactly what main does; this PR no longer touches it.
  • An offer for a conversation forked after the quit could never end. It was hidden and never deleted, which matters now that offers have no expiry. It is now deleted, and only when the fork is proven.
  • The mid-reply row said "Monitoring". A chat stopped mid-reply while running a command read "Was mid-reply · Monitoring: ". The task list includes the reply's own foreground command, so the row now says "Monitoring" only when the main agent had finished, as the sidebar does.
  • Japanese strings failed the localization gate. The new subagent strings now keep "Agent" untranslated, as the rest of the catalog does.
  • Oversized host file. The native-chat host file is back under its line limit.

Live run (macOS, hidden window, real Claude chats, real restart, nothing seeded)

Scenario Result
Main agent finished, background shell still running → offered as "Monitoring: …" Pass
Background subagent left shells running → offered Pass
Waiting on a question → offered as "Waiting for your answer: …" Pass
Mid-reply → offered Pass
Idle chat → not offered Pass
Resume → continuation sent, agent carried on, offer removed Pass
User sends their own message in an offered chat → offer deleted Pass
Dismiss all → recovery file emptied Pass

Resume dialog after restart

More: sidebar before restart · waiting on a question · mid-reply · idle, not offered · resumed chat · after own message · after dismiss

Checks

CI green. pnpm tc, oxlint on every changed file, the changed-code gate, all four localization gates and git diff --check pass. Targeted suites: the native-chat wire, recovery file, shared agent-session, and resume dialog and label suites, plus the close and worktree-teardown suites. Every fix is pinned by a test that fails when the fix is deleted.

Still open

  • Idle stopping of background work (product decision). A chat whose main agent finished and that nobody is viewing is still stopped after 30 idle minutes, as on main, which ends any subagent or background command it left running. The earlier version of this branch kept such chats alive instead; that is now out of this PR so it can be decided on its own.
  • Stale status-bar count (existing behaviour). After you send a message in an offered chat, the "N chats to resume" count updates only when the dialog is next opened. This is the same on main; a small follow-up can make the count refresh immediately.
  • Not exercised: Linux, Windows and SSH hosts. The change has no platform-specific code, and native chat runs on the local host.

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