Background-work liveness in canonical phase + archive port-lease ordering - #1059
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (37)
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. Comment |
arul28
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@codex review Both P1s addressed: Unsettle leaves schedules paused — valid, and worse than it looked: the scheduler's session pause is persisted, so the schedules stayed disabled across restarts. The scheduler now records which sessions settle paused ( Shell tasks become passive monitors — valid, and it contradicted the classifier's own rule. |
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
…builds monitors Both P1s from Greptile on #1059, verified against the code before fixing. 1. Unsettle left schedules paused forever. The scheduler's session pause is PERSISTED, and settle took one while every unsettle path only cleared lifecycle columns — so a settled-then-unsettled chat kept its monitors, crons, and scheduled turns disabled indefinitely. A durable pause with no undo is just a slower deletion, and the docs already promised the undo. The scheduler now records which sessions settle paused (`settlePausedSessionIds`, persisted beside the pause it annotates). `setSessionPausedForSettle` claims a pause only when the user had not already taken one; `resumeSessionPausedForSettle` puts back exactly that and nothing else; an explicit user toggle drops settle's claim in either direction so a later unsettle cannot override their choice. Every unsettle entry point — registry single/bulk, both IPC handlers, both sync commands — now runs `resumeSettledSessionMachinery`, mirroring the settle wiring. Background work is deliberately not restarted: ADE cannot re-spawn a shell it stopped, and pretending otherwise is worse than leaving it quiet. 2. Generic backgrounded shells were classified as monitors. `local_bash` / `shell` / `background` / `bash` are how a provider says "the agent backgrounded a command" — a `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing them labelled every background build "Monitoring", telling the user nothing was being produced while it was. Mixed is unknown, and this classifier's own stated rule is that unknown is working; including them contradicted it. MONITOR_TASK_TYPES is now only `monitor` / `monitor_mcp` — types whose whole job is to watch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51e843c43e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+666
to
+668
| pausedSessionIds.add(sessionId); | ||
| settlePausedSessionIds.add(sessionId); | ||
| await updatePauseStatuses(sessionId); |
There was a problem hiding this comment.
When a user starts a new turn in a settled chat, agentChatService calls sessionService.clearTurnStartMarkers, which clears settled_at without invoking resumeSettledSessionMachinery. The marker added here therefore remains persisted and the chat appears active again while its schedules stay paused across restarts; only the explicit IPC/RPC unsettle handlers currently perform the resume. Couple the pause ownership to the authoritative lifecycle mutation or invoke the resume from every activity-driven unsettle path.
AGENTS.md reference: AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
Comment on lines
+39382
to
+39386
| emitClaudeBackgroundTaskUpdate(managed, runtime, { | ||
| taskId, | ||
| status: terminalStatus, | ||
| ...(terminalSummary ? { summary: terminalSummary } : {}), | ||
| }); |
There was a problem hiding this comment.
When stopTask is unavailable, times out, or rejects, terminalStatus becomes failed but this update still runs. emitClaudeBackgroundTaskUpdate treats every terminal status as closed and removes the task from liveBackgroundTaskIds, so stopBackgroundWork reports a reduced count even though the process may still be running; the settled row then hides work that can continue consuming tokens or holding resources. Retain the live task on stop failure and only emit a terminal update after confirmed cancellation.
Useful? React with 👍 / 👎.
Comment on lines
+2189
to
+2191
| return (async () => { | ||
| await stopSettledSessionMachinery( | ||
| { |
There was a problem hiding this comment.
This caller-specific teardown leaves the supported CTO operator path unchanged: createCtoOperatorTools still calls deps.sessionService.settleSession directly in its settleSession tool. When the CTO files a chat that owns schedules or background work, neither is paused or stopped, preserving the original hidden-work/token-spend bug for that production path. Route the operator tool through settleTerminalSession, or centralize the teardown in the underlying lifecycle service so callers cannot bypass it.
AGENTS.md reference: AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
… false stops Three P1s from the #1059 re-review — Greptile and Codex independently found the first, which is the highest-signal one. 1. Activity-driven unsettle kept schedules paused (Greptile + Codex). Wiring the resume into each unsettle caller missed the most common unsettle of all: a user sending the next message, which clears settled_at through clearTurnStartMarkers. The chat went active while its monitors, crons, and scheduled turns stayed paused across restarts. The resume now hangs off sessionService's new onSettleCleared hook, fired by every route that clears the column — unsettleSession, unsettleSessions, and clearTurnStartMarkers. Per-caller wiring in the registry, both IPC handlers, and both sync commands is deleted as redundant, so a future caller cannot reintroduce the gap. Settle itself stays explicit per entry point because its teardown has to finish before the write. 2. CTO operator settle bypassed teardown entirely (Codex). createCtoOperatorTools called sessionService.settleSession directly, so a CTO-filed chat kept its schedules armed and its background fleet spending. It now runs the shared teardown first, like every other settle entry point. 3. A failed stop still closed the task row (Codex). When stopTask was missing, timed out, or rejected, the emitted terminal row dropped the task from liveBackgroundTaskIds — which is exactly what the caller measures the stop against, so a stop that did not happen was counted as one that did. The task now stays LIVE on failure; the SDK's next authoritative level drains it if it really ended. Under-reporting is the only safe direction here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Latest round: cutting the scheduled-work pause rather than patching it a fourth time. Settle-clear paths skip schedule resume — valid, and broader than the comment says: Stale resume releases newer pause — also valid; the resume was fire-and-forget and identified ownership only by session id. That's three consecutive rounds finding a defect in the scheduled-work pause specifically, each in a route the previous fix hadn't traced. The pause is persisted, so it needs a complete undo or it silently deletes the user's own monitors and crons. Covering the rest means a pre-read or split statement per output chunk plus per-session serialization — real cost, for the one part of this change that keeps producing bugs. So settle now stops background work only: background shells, subagent fleets, cursor cloud runs. That's the unmanaged, invisible thing this change was about, and it's been stable since iteration 2. ADE's scheduled work is already visible and user-manageable, and Kept from this round: the CTO operator settle now routes through shared teardown, and an unstoppable Claude task stays live instead of being reported as stopped. Stopping scheduled work on settle is still a reasonable feature — it needs its own change with the full clear-path inventory up front. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66e7dba6dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "claude": { | ||
| // Children first: this drains workflow agents, then subagents, then | ||
| // the background shells each of them owns. | ||
| await stopActiveClaudeSubagents( |
There was a problem hiding this comment.
Fresh evidence in the current head is that this new settle path calls stopActiveClaudeSubagents, which first routes background shells through closeOpenClaudeBackgroundTasks; when stopTask is unavailable, rejects, or times out, that helper emits terminal status failed, and emitClaudeBackgroundTaskUpdate removes the task from liveBackgroundTaskIds. The subsequent preservation loop therefore never sees the unstoppable shell, stopped is over-counted, and the settled row can hide a process that is still running.
Useful? React with 👍 / 👎.
Comment on lines
+570
to
+574
| await stopSettledSessionMachinery( | ||
| { | ||
| sessionService: deps.sessionService, | ||
| agentChatService: deps.agentChatService ?? null, | ||
| }, |
There was a problem hiding this comment.
Fresh evidence in the current head is the production construction at apps/ade-cli/src/adeRpcServer.ts:2955-2988: it obtains agentChatService but does not pass the newly added agentChatService dependency to createCtoOperatorTools. Consequently this call receives null when the CTO settle tool is invoked through the desktop socket-backed ADE RPC path, so the row is still settled without stopping its background work. Pass stopBackgroundWork from the already-required service into that construction as the in-process construction does.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
Comment on lines
+39336
to
+39337
| const turnActive = managed.session.status === "active" || Boolean(runtime.activeTurnId); | ||
| if (turnActive) return { stopped: 0, skippedActiveTurn: true }; |
There was a problem hiding this comment.
When PR auto-settlement, the CTO tool, or an RPC caller settles a chat during an active turn, this early return skips all teardown, but the caller still writes settled_at. Chat output deliberately calls setLastOutputPreview without clearing settle markers, and turn completion also preserves them, so any background shell or subagent that survives the foreground turn remains live while the row becomes settled and hidden. Either reject/defer settlement for active turns or arrange a teardown when that turn reaches idle before honoring the settle.
Useful? React with 👍 / 👎.
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
…builds monitors Both P1s from Greptile on #1059, verified against the code before fixing. 1. Unsettle left schedules paused forever. The scheduler's session pause is PERSISTED, and settle took one while every unsettle path only cleared lifecycle columns — so a settled-then-unsettled chat kept its monitors, crons, and scheduled turns disabled indefinitely. A durable pause with no undo is just a slower deletion, and the docs already promised the undo. The scheduler now records which sessions settle paused (`settlePausedSessionIds`, persisted beside the pause it annotates). `setSessionPausedForSettle` claims a pause only when the user had not already taken one; `resumeSessionPausedForSettle` puts back exactly that and nothing else; an explicit user toggle drops settle's claim in either direction so a later unsettle cannot override their choice. Every unsettle entry point — registry single/bulk, both IPC handlers, both sync commands — now runs `resumeSettledSessionMachinery`, mirroring the settle wiring. Background work is deliberately not restarted: ADE cannot re-spawn a shell it stopped, and pretending otherwise is worse than leaving it quiet. 2. Generic backgrounded shells were classified as monitors. `local_bash` / `shell` / `background` / `bash` are how a provider says "the agent backgrounded a command" — a `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing them labelled every background build "Monitoring", telling the user nothing was being produced while it was. Mixed is unknown, and this classifier's own stated rule is that unknown is working; including them contradicted it. MONITOR_TASK_TYPES is now only `monitor` / `monitor_mcp` — types whose whole job is to watch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
… false stops Three P1s from the #1059 re-review — Greptile and Codex independently found the first, which is the highest-signal one. 1. Activity-driven unsettle kept schedules paused (Greptile + Codex). Wiring the resume into each unsettle caller missed the most common unsettle of all: a user sending the next message, which clears settled_at through clearTurnStartMarkers. The chat went active while its monitors, crons, and scheduled turns stayed paused across restarts. The resume now hangs off sessionService's new onSettleCleared hook, fired by every route that clears the column — unsettleSession, unsettleSessions, and clearTurnStartMarkers. Per-caller wiring in the registry, both IPC handlers, and both sync commands is deleted as redundant, so a future caller cannot reintroduce the gap. Settle itself stays explicit per entry point because its teardown has to finish before the write. 2. CTO operator settle bypassed teardown entirely (Codex). createCtoOperatorTools called sessionService.settleSession directly, so a CTO-filed chat kept its schedules armed and its background fleet spending. It now runs the shared teardown first, like every other settle entry point. 3. A failed stop still closed the task row (Codex). When stopTask was missing, timed out, or rejected, the emitted terminal row dropped the task from liveBackgroundTaskIds — which is exactly what the caller measures the stop against, so a stop that did not happen was counted as one that did. The task now stays LIVE on failure; the SDK's next authoritative level drains it if it really ended. Under-reporting is the only safe direction here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/t3-liveness-settle-5db99ac6
branch
from
August 10, 2026 05:10
66e7dba to
cd53e34
Compare
Owner
Author
|
@codex review All three addressed, plus a rebase onto main (#1061 landed under me). Defer active-turn settlement — valid, and the carve-out was simply too wide. A turn's own subagents are work the user can see and are still spared; its detached background work outlives the turn by construction, and an explicit settle is the user saying they're done with it. That now stops mid-turn. Pass teardown control through the ADE RPC operator bridge — valid, and the classic in-process-wired/daemon-missing shape. Keep failed Claude shell stops live — you're right that |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd53e34127
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the background shells each of them owns. Skipped mid-turn — those | ||
| // subagents belong to the turn the user is watching. | ||
| if (!turnActive) { | ||
| await stopActiveClaudeSubagents( |
There was a problem hiding this comment.
Fresh evidence in the current head is that runtimeBackgroundWork now derives user-visible liveness from liveBackgroundTaskIds, but this call first routes idle Claude shells through closeOpenClaudeBackgroundTasks; when stopTask is absent, times out, or rejects, that helper still emits a terminal failed update and deletes the task from the live set. The later retention loop therefore cannot see it, and settlement hides a process that may still be consuming tokens or holding resources. Preserve the live entry and prevent or explicitly report settlement when cancellation is not confirmed.
Useful? React with 👍 / 👎.
| if (!dismissed) return false; | ||
| } | ||
|
|
||
| await stopSettledSessionMachinery( |
There was a problem hiding this comment.
When provider teardown waits on one or more stop calls, a user can start a new turn during this await; clearTurnStartMarkers then runs while no settle marker exists, after which this function writes settled_at for the newly active lifecycle. The fresh turn consequently remains filed as settled when it returns to idle, despite being user activity that should unsettle it. Serialize teardown with lifecycle mutations or condition the final write on the session still having the pre-teardown activity/version; the bulk paths need the same underlying guarantee.
AGENTS.md reference: AGENTS.md:L33-L34
Useful? React with 👍 / 👎.
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
…builds monitors Both P1s from Greptile on #1059, verified against the code before fixing. 1. Unsettle left schedules paused forever. The scheduler's session pause is PERSISTED, and settle took one while every unsettle path only cleared lifecycle columns — so a settled-then-unsettled chat kept its monitors, crons, and scheduled turns disabled indefinitely. A durable pause with no undo is just a slower deletion, and the docs already promised the undo. The scheduler now records which sessions settle paused (`settlePausedSessionIds`, persisted beside the pause it annotates). `setSessionPausedForSettle` claims a pause only when the user had not already taken one; `resumeSessionPausedForSettle` puts back exactly that and nothing else; an explicit user toggle drops settle's claim in either direction so a later unsettle cannot override their choice. Every unsettle entry point — registry single/bulk, both IPC handlers, both sync commands — now runs `resumeSettledSessionMachinery`, mirroring the settle wiring. Background work is deliberately not restarted: ADE cannot re-spawn a shell it stopped, and pretending otherwise is worse than leaving it quiet. 2. Generic backgrounded shells were classified as monitors. `local_bash` / `shell` / `background` / `bash` are how a provider says "the agent backgrounded a command" — a `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing them labelled every background build "Monitoring", telling the user nothing was being produced while it was. Mixed is unknown, and this classifier's own stated rule is that unknown is working; including them contradicted it. MONITOR_TASK_TYPES is now only `monitor` / `monitor_mcp` — types whose whole job is to watch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
… false stops Three P1s from the #1059 re-review — Greptile and Codex independently found the first, which is the highest-signal one. 1. Activity-driven unsettle kept schedules paused (Greptile + Codex). Wiring the resume into each unsettle caller missed the most common unsettle of all: a user sending the next message, which clears settled_at through clearTurnStartMarkers. The chat went active while its monitors, crons, and scheduled turns stayed paused across restarts. The resume now hangs off sessionService's new onSettleCleared hook, fired by every route that clears the column — unsettleSession, unsettleSessions, and clearTurnStartMarkers. Per-caller wiring in the registry, both IPC handlers, and both sync commands is deleted as redundant, so a future caller cannot reintroduce the gap. Settle itself stays explicit per entry point because its teardown has to finish before the write. 2. CTO operator settle bypassed teardown entirely (Codex). createCtoOperatorTools called sessionService.settleSession directly, so a CTO-filed chat kept its schedules armed and its background fleet spending. It now runs the shared teardown first, like every other settle entry point. 3. A failed stop still closed the task row (Codex). When stopTask was missing, timed out, or rejected, the emitted terminal row dropped the task from liveBackgroundTaskIds — which is exactly what the caller measures the stop against, so a stop that did not happen was counted as one that did. The task now stays LIVE on failure; the SDK's next authoritative level drains it if it really ended. Under-reporting is the only safe direction here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/t3-liveness-settle-5db99ac6
branch
from
August 10, 2026 05:31
cd53e34 to
9c1c2d8
Compare
Owner
Author
|
@codex review Both fixed, and rebased again (main moved twice under this branch). Keep failed Claude background stops live — you were right to press on this. I'd only fixed my own sweep loop; Revalidate activity before writing the settle marker — valid race. Settle now snapshots |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1c2d8a06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+303
to
+309
| if (!pinnedActive && atRest && (args.settleOverride === "settled" || args.settledAt)) { | ||
| return { phase: "settled", badge: null }; | ||
| // Deliberately NOT promoted by background work. Settle now tears the | ||
| // session's machinery down (`stopSessionBackgroundWork`), so a settled row | ||
| // with live work is a teardown that has not finished draining, not a state | ||
| // worth advertising — and re-lighting it would let a stubborn monitor | ||
| // out-vote the user's explicit "this is done". | ||
| return { phase: "settled", badge: null, liveness: null }; |
There was a problem hiding this comment.
When an idle Codex chat owns background subagents, or a Claude/Cursor cancellation is unavailable or fails, stopBackgroundWork deliberately leaves the runtime entry live but the caller still writes settledAt; this branch then gives settlement precedence over that live work and renders the row quiet. Fresh evidence in the current head is the explicit default no-op for Codex and the failure continue paths in stopBackgroundWork, so preserving the live IDs alone does not prevent the original hidden-work/token-spend failure. Make unconfirmed teardown observable and reject/defer the settle, or allow retained live work to keep the session visible.
AGENTS.md reference: AGENTS.md:L33-L34
Useful? React with 👍 / 👎.
Comment on lines
+92
to
+94
| const after = args.sessionService.get(args.sessionId); | ||
| if (!after) return false; | ||
| if ((after.lastActivityAt ?? null) !== activityBeforeTeardown) { |
There was a problem hiding this comment.
When a user starts a turn during teardown but the provider has not produced output yet, this comparison still succeeds: clearTurnStartMarkers does not update the persisted last_output_at backing lastActivityAt, and sendMessage updates only the in-memory managed.lastActivityTimestamp. The settle therefore writes its marker over the newly started turn despite this new guard. Use a persisted lifecycle revision or turn-start timestamp that changes synchronously with clearTurnStartMarkers.
AGENTS.md reference: AGENTS.md:L33-L34
Useful? React with 👍 / 👎.
Comment on lines
570
to
577
| await stopSettledSessionMachinery( | ||
| { | ||
| sessionService: deps.sessionService, | ||
| agentChatService: deps.agentChatService ?? null, | ||
| }, | ||
| [sessionId], | ||
| ); | ||
| const ok = deps.sessionService.settleSession(sessionId, { |
There was a problem hiding this comment.
When a user starts or steers a turn while this provider teardown is awaiting, this direct CTO path immediately writes settled_at afterward with no activity revalidation, so the user activity that cleared the lifecycle markers is overwritten. Fresh evidence in the current head is that only settleTerminalSession contains the new guard; the bulk registry/RPC and PR auto-settlement paths use the same teardown-then-write pattern. Route these callers through one guarded lifecycle operation so every settlement entry point has the same concurrency guarantee.
AGENTS.md reference: AGENTS.md:L33-L34
Useful? React with 👍 / 👎.
A session whose foreground turn ended while its background jobs kept going read as idle everywhere a user glances: the Work-tab dot, the TopBar rollup, the dock badge, and the Lanes agent list all showed nothing while agents were mid-run. The "Background work xN" label existed, but only as a label — it never reached the canonical phase those surfaces derive from. canonicalSessionState now promotes a resting session with live background work back to `running`, and reports WHY via a new `liveness` field (turn / background / monitoring). Every existing consumer of the phase inherits the truth without a special case. - Two-state vocabulary: `monitoring` only when watch loops are the SOLE live work, so "still building" and "just watching CI" read differently. - Classification is a denylist (MONITOR_TASK_TYPES / INERT_TASK_TYPES). Unknown task types count as WORKING — an allowlist silently drops a real subagent the first time an SDK renames a type. - Generalized past Claude: codex background subagents and cursor cloud runs now count too. runtimeBackgroundWork() documents what escapes (detached nohup/setsid spawns, user-owned terminals, opencode/droid/pi). - Liveness stays in-memory and empty after restart: orphaned background work is not live work. - A failed, stopped, settled, or hand-raised session still outranks lingering liveness, so a stale "Working" can never mask a failure. - Subagent toolbar badge counts RUNNING subagents, not total tracked — a finished fleet no longer wears a number that only ever grew. - TerminalAttentionSummary.byLaneId removed deliberately: it had no consumer, and laneListSnapshotService already owns the per-lane rollup the Lanes tab and mobile both read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Settle was a pure column write. The row went quiet and everything the session had started kept going — background shells held ports, subagent fleets kept spending tokens, and scheduled work woke the thread hours after the user had declared it done. Archive had the mirror problem: it released the lane's port lease and proxy route while the lane's processes were still bound to those ports, and an archived lane is filtered out of every surface that could have shown the user what to stop. Settle now runs a shared teardown (sessionMachineryTeardown.ts) before the lifecycle write, so a settle can never report success while its monitors are still armed: - pauses the session's scheduled work — pauses, not cancels, so an unsettle brings hand-made schedules back rather than having silently deleted them, - calls the new agentChatService.stopBackgroundWork, which stops every live child BEFORE the parent (stopping only the parent leaves the fleet running and untracked, which is how a "stopped" agent keeps spending), - keeps TERMINAL PANES OPEN. An agent's background shell is thread background work; a pane the user opened is theirs, and closing it on settle would destroy scrollback nobody asked to lose, - leaves an ACTIVE foreground turn alone — its subagents are work the user can see happening, and the row un-settles on its own activity anyway, - is best-effort throughout: a provider that cannot be reached delays nothing and blocks nothing. Wired into every settle entry point: the single/bulk ADE actions, the sessions.settle / settleMany IPC handlers, the session.settle* sync commands, and PR-merge auto-settle — which bypasses settlement blockers and is therefore the path most likely to file a session that is still running something. It composes with that service's session targeting rather than replacing it. laneService.archive is now async and stops the lane's chats, PTYs, watchers and auto-rebase through a shared stopLaneRuntimeWork before the status write, so the port lease its callers release immediately afterwards is released after the processes are gone. archiveAndReclaim uses the same helper; delete keeps its runStep version because the delete dialog reports each step. What escapes is documented rather than pretended away: processes an agent detached with nohup/setsid/disown leave ADE's tree entirely, and Codex background subagents are reported but expose no stop control. No new kill logic is introduced — teardown delegates to ptyService/agentChatService disposal, which already route through the Windows-correct tree-kill helpers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…churn
Findings from the /quality dual-review on this branch, all verified against the
real code paths before applying:
- stopBackgroundWork reported the live work it FOUND as the work it stopped, so
a Codex session (no per-subagent stop control) or a Cursor session with no
cloud agent id claimed a teardown that never happened. It now reports the
measured DROP in live work across the call, which is 0 for those cases by
construction and can never over-report.
- A Claude background task ADE could not stop was closed as "stopped". It now
settles as failed with the reason, matching closeOpenClaudeBackgroundTasks —
both close the row, only one claims ADE did the stopping.
- getSessionSummary emitted backgroundWork: {0,0} on every chat summary. Now
omitted when nothing is live, like every other optional field there.
- NO_BACKGROUND_WORK was a shared mutable object handed out by reference; frozen.
- laneAgents' background hint guarded on a stringly-typed status that chat and
CLI summaries spell differently. Callers now pass turnActive explicitly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…i in the matrix Rebased onto origin/main (P0 #1056, Pi #1054/#1055). - registry's `session.settleSessions` had become an `async` function, which silently converted its argument-validation guard from a synchronous throw into a rejected promise — a contract change for any caller that does not await, caught by registry.test.ts. Only the success path is async now, returned as an explicit promise from a sync body. The success assertion is awaited, because that path genuinely did become asynchronous: the session's monitors have to stop before the settle is written. - runtimeBackgroundWork's switch is now exhaustive over ChatRuntime["kind"] with a `never` check, so a newly landed harness fails to compile until someone classifies it. Pi is listed explicitly: its runtime carries only turn-scoped state (activeTurnId, busy, pendingSteers, activeCompactionId, lease) with no subagent, background-task, or remote-run tracking, and it is absent from SUBAGENT_CAPABILITIES so resolveSubagentCapability already degrades it to the no-op descriptor. Zero is a verified fact about Pi, not an unexamined default. - Settle teardown now runs inside the merged candidateSessionsFor loop, so it targets exactly what the corrected targeting settles: nothing at all when the scope is ambiguous, only declared in-lane sessions when linked, and the bounded sweep minus other PRs' claims otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…removed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…veness contract Pruned/consolidated: the sessions folder had 5 test files against a 3-file budget after this branch added one. deleteTerminalSession.test.ts and sessionMachineryTeardown.test.ts covered the same contract — what happens to a session's machinery at end of life — split across files for dependency reasons, not behavioral ones. Merged into sessionTeardown.test.ts (12 tests), returning the folder to the 4 files it had before this branch. No tests lost. Added, where the failure mode is actually reachable: - agentChatService.test.ts: drives a real background_tasks_changed level and asserts the summary splits it working/monitoring by denylist (local_bash -> monitoring, local_agent AND an unrecognised type -> working), that a live turn makes stopBackgroundWork decline rather than kill it, and that the record is omitted once the level drains rather than riding along as a zero. - laneAgents.test.ts: a resting agent stays live while its background work is, sorts working ahead of monitoring ahead of idle, reports what is still running instead of the finished turn's stale preview, and counts a split-less (older-peer) summary as working rather than passive. Parity: corrected stale prose in attentionItemBuilder that still described the promotion as a sessionStatusPresentation label override rather than a sessionCanonicalState phase promotion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…builds monitors Both P1s from Greptile on #1059, verified against the code before fixing. 1. Unsettle left schedules paused forever. The scheduler's session pause is PERSISTED, and settle took one while every unsettle path only cleared lifecycle columns — so a settled-then-unsettled chat kept its monitors, crons, and scheduled turns disabled indefinitely. A durable pause with no undo is just a slower deletion, and the docs already promised the undo. The scheduler now records which sessions settle paused (`settlePausedSessionIds`, persisted beside the pause it annotates). `setSessionPausedForSettle` claims a pause only when the user had not already taken one; `resumeSessionPausedForSettle` puts back exactly that and nothing else; an explicit user toggle drops settle's claim in either direction so a later unsettle cannot override their choice. Every unsettle entry point — registry single/bulk, both IPC handlers, both sync commands — now runs `resumeSettledSessionMachinery`, mirroring the settle wiring. Background work is deliberately not restarted: ADE cannot re-spawn a shell it stopped, and pretending otherwise is worse than leaving it quiet. 2. Generic backgrounded shells were classified as monitors. `local_bash` / `shell` / `background` / `bash` are how a provider says "the agent backgrounded a command" — a `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing them labelled every background build "Monitoring", telling the user nothing was being produced while it was. Mixed is unknown, and this classifier's own stated rule is that unknown is working; including them contradicted it. MONITOR_TASK_TYPES is now only `monitor` / `monitor_mcp` — types whose whole job is to watch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…use API Missed when setScheduledWorkPaused was split into the settle-scoped setScheduledWorkPausedForSettle; the user-facing toggle keeps its old name and its own callers, which is why only the teardown mocks move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… false stops Three P1s from the #1059 re-review — Greptile and Codex independently found the first, which is the highest-signal one. 1. Activity-driven unsettle kept schedules paused (Greptile + Codex). Wiring the resume into each unsettle caller missed the most common unsettle of all: a user sending the next message, which clears settled_at through clearTurnStartMarkers. The chat went active while its monitors, crons, and scheduled turns stayed paused across restarts. The resume now hangs off sessionService's new onSettleCleared hook, fired by every route that clears the column — unsettleSession, unsettleSessions, and clearTurnStartMarkers. Per-caller wiring in the registry, both IPC handlers, and both sync commands is deleted as redundant, so a future caller cannot reintroduce the gap. Settle itself stays explicit per entry point because its teardown has to finish before the write. 2. CTO operator settle bypassed teardown entirely (Codex). createCtoOperatorTools called sessionService.settleSession directly, so a CTO-filed chat kept its schedules armed and its background fleet spending. It now runs the shared teardown first, like every other settle entry point. 3. A failed stop still closed the task row (Codex). When stopTask was missing, timed out, or rejected, the emitted terminal row dropped the task from liveBackgroundTaskIds — which is exactly what the caller measures the stop against, so a stop that did not happen was counted as one that did. The task now stays LIVE on failure; the SDK's next authoritative level drains it if it really ended. Under-reporting is the only safe direction here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cutting a slice of this PR rather than patching it a fourth time. Greptile's latest round found that `settled_at` is cleared from SEVEN places in sessionService, not the three the onSettleCleared hook covered — including `setLastOutputPreview`, the hot PTY-output path. It also found a TOCTOU where a fire-and-forget resume overlapping a later settle releases the newer pause. That is the third consecutive review round to find a defect in the scheduled-work pause specifically, each in a route the previous fix had not traced. The pause is persisted, so it needs a COMPLETE undo or it silently deletes the user's own monitors and crons. Covering the remaining routes means either a pre-read or a split statement on a per-output-chunk path, plus serializing pause/resume per session — real cost and more machinery, for the part of this change that keeps producing bugs. So settle now stops background work only: background shells, subagent fleets, cursor cloud runs. That was the unmanaged, invisible thing the change was actually about, and it has been stable since the second iteration. Scheduled work in ADE is already visible and user-manageable (scheduledWork / nextWakeAt on the summary, a per-session pause toggle), and canonicalSessionState already handles a settled chat woken by a schedule: green while the turn streams, then re-settled. Leaving it running is the pre-existing, deliberate behavior. Removed: settlePausedSessionIds and the two scheduler methods, setScheduledWorkPausedForSettle, sessionService's onSettleCleared hook and its wiring in main/bootstrap, and resumeSettledSessionMachinery. Kept: the CTO operator settle now routing through shared teardown, and unstoppable Claude tasks staying live rather than being reported as stopped. Stopping scheduled work on settle remains a reasonable feature; it needs its own change with the full clear-path inventory up front, not a bolt-on to this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…op the unhonest count Three P1s from Codex on 66e7dba. 1. Settling during an active turn tore down nothing. stopBackgroundWork returned early on a live turn while the caller still wrote settled_at, so PR auto-settlement, the CTO tool, and RPC callers left background shells running under a row that went quiet when the turn ended. The carve-out was too wide: a turn's own SUBAGENTS are work the user can see and are still spared, but its DETACHED background work outlives the turn by construction and an explicit settle is the user saying they are done with it. That now stops mid-turn; only stopActiveClaudeSubagents and cursor cloud-run cancellation are skipped while a turn runs. 2. The ADE RPC operator bridge never received the teardown control. adeRpcServer's createCtoOperatorTools construction had agentChatService in scope but did not pass it, so the CTO settle tool over the desktop socket filed rows without stopping their background work — the in-process path was fixed and the daemon path was not. Same bug class as every other 'wired in-process, missing from the daemon' regression. 3. The stopped count could not be kept honest. stopActiveClaudeSubagents routes through closeOpenClaudeBackgroundTasks, which closes a shell it FAILED to stop, so any before/after measurement silently counted unstoppable work as stopped. This is the third round to land on that number. It had no consumer anywhere, so it is gone rather than approximated; skippedActiveTurn is the remaining, checkable signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rmed Two P1s from Codex on cd53e34. 1. An unconfirmed stop still closed the task row. closeOpenClaudeBackgroundTasks emits a terminal 'failed' update when stopTask is absent, times out, or rejects, and that removes the task from liveBackgroundTaskIds — which is exactly what runtimeBackgroundWork derives the row's user-visible liveness from. The session therefore went quiet over a shell that may still be running: the precise lie this whole change exists to remove. A stop we attempted and could not confirm now leaves the task LIVE and logs claude_background_stop_unconfirmed; the SDK's next authoritative level drains it if it really ended. Scoped to failed stop ATTEMPTS, so the turn-end close path ('completed', which attempts nothing) is unchanged. 2. Teardown raced the lifecycle write. Provider stop calls take seconds, and a user starting a turn inside that window runs clearTurnStartMarkers against a settle marker that does not exist yet — after which settleTerminalSession wrote settled_at over the freshly-active session, filing a live turn as settled. The settle now snapshots lastActivityAt before teardown and refuses the write if it moved. Real activity outranks a settle request that predates it. It reports true rather than false: the row exists and the request was handled, it simply woke, and false would surface a spurious 'not found'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cutting the second and last slice of settle teardown. Six review rounds, every one of them finding a real defect in this specific mechanism: - an unsettle path that skipped the resume (x3, each a route the previous fix had not traced), - settling mid-turn tearing down nothing while the caller still wrote the marker, - the RPC operator bridge never receiving the teardown control, - a stop count that could not be kept honest, - and finally an activity guard that reads lastActivityAt — which is backed by last_output_at, a column clearTurnStartMarkers never writes. The guard I added last round provably cannot fire. The shape is now unambiguous. Teardown is async; settled_at is written and cleared from seven places. A teardown-then-write settle races real activity, and the failure is not one-sided: a user starting a turn during a provider stop call gets their background work stopped AND no settle. Every guard against it either read a column turn-start does not update, or had to be repeated identically at each settle entry point (settleTerminalSession, bulk registry, both IPC handlers, both sync commands, PR auto-settlement, the CTO tool). Doing this correctly needs a synchronous lifecycle revision that teardown can be serialized against — a different change, designed as one, not a wrapper around the existing write. Shipping the half-working version is worse than the status quo, which is the one thing the brief specifically warned about. What ships instead is the half that has been stable since iteration 2 and is what a user actually sees: live background work promoted into the canonical phase across every glanceable surface, the working/monitoring denylist, cross-runtime generalization, running-count subagent badges, and the archive port-lease ordering fix — archive remains the lifecycle path that does stop processes, and its ordering bug is fixed. Removed: sessionMachineryTeardown, stopBackgroundWork, the activity guard, and the teardown calls in every settle entry point. runtimeBackgroundWork stays; it is the surfacing half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/t3-liveness-settle-5db99ac6
branch
from
August 10, 2026 05:55
9c1c2d8 to
7eff368
Compare
arul28
changed the title
Greptile and Codex both landed on the same branch, and cutting settle teardown made them right: the settled branch suppressed background-work liveness, and its comment justified that with 'settle now tears the session's machinery down' — which stopped being true when the teardown was removed. A settled session can now legitimately still own a live background shell, subagent, or cloud run. The PHASE stays settled: a declared settle is a human judgment call, and re-lighting the row would let a stubborn monitor out-vote the user's explicit 'this is done'. But liveness now reports the truth, so a surface that wants to show 'settled, but something is still running' can. Hiding it behind the phase is the same lie this module exists to prevent, just at the other end of the lifecycle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment on lines
+319
to
+326
| const settledWork = args.backgroundWork; | ||
| return { | ||
| phase: "settled", | ||
| badge: null, | ||
| liveness: totalBackgroundWork(settledWork) <= 0 | ||
| ? null | ||
| : (settledWork?.workingCount ?? 0) > 0 ? "background" : "monitoring", | ||
| }; |
There was a problem hiding this comment.
When a user settles a resting session that still owns a background shell, subagent, monitor, or Cursor cloud run, this branch retains the settled phase and only records the work in liveness. Existing presentation, filing, and attention rollups continue branching on the phase, causing the session to remain in the collapsed settled tier without a working status or contribution to active-work indicators while its background work continues.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/shared/sessionCanonicalState.ts
Line: 319-326
Comment:
**Settled work remains hidden**
When a user settles a resting session that still owns a background shell, subagent, monitor, or Cursor cloud run, this branch retains the `settled` phase and only records the work in `liveness`. Existing presentation, filing, and attention rollups continue branching on the phase, causing the session to remain in the collapsed settled tier without a working status or contribution to active-work indicators while its background work continues.
**Knowledge Base Used:**
- [Chat, terminal, and provider session management](https://app.greptile.com/versic/-/custom-context/knowledge-base/arul28/ade/-/docs/brain-chat-sessions.md)
- [Desktop Renderer UI](https://app.greptile.com/versic/-/custom-context/knowledge-base/arul28/ade/-/docs/desktop-renderer-ui.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Owner
Author
|
Settled work remains hidden — factually correct, and deliberately not taken here. Setting The real fix isn't a presentation tweak either — it's that settle should stop that work, which is the teardown this PR deliberately cuts after six review rounds. That's now a design deliverable, and this finding goes into it as a named constraint: if settle cannot stop the work, the settled row must not silently own it — which is an argument for rejecting/deferring the settle, not for re-lighting the row afterwards. Thanks — this and the |
arul28
added a commit
that referenced
this pull request
Aug 10, 2026
* docs(sessions): design for settle teardown done properly Deliverable for the settle teardown cut from #1059, for review before any reimplementation. Contains the full inventory of all seven paths that write or clear settled_at with file:line and callers (four of the seven clearers are implicit, and one runs per terminal output chunk), a six-case race matrix, and a proposed design: a single settle-writer chokepoint owning a monotonic persisted lifecycle revision, an explicit exclusive 'settling' state, and the rule that an accepted user turn aborts the settle rather than the reverse. Every one of the 16 P1 findings from the six review rounds is mapped to the design constraint it implies and the mechanism that neutralises it, so the review capital is not lost. Three findings are load-bearing: enumerate the writers first, verify the detector actually changes on the event it guards, and put the guarantee in the writer rather than each caller. #1059 did none of the three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(sessions): amend settle-teardown design after review Five amendments from design review of #1067. 1. C4 during settling — the sharpest hole. Teardown itself produces output, so if C4 bumped the guard revision or tripped abort, every real teardown would self-abort and a settle could never land. Added a per-clearer behavior table for the settling window: C3/C6/C7 (turn start, turn failed, attention request) abort because each represents a human decision that outranks settled; C4/C5 (output preview, activity touch) are SWALLOWED — no clear, no revision bump, no abort. Before settling starts all five behave as today. 2. Multi-device authority — VERIFIED BYPASSABLE, now a hard precondition. terminal_sessions is a CRR table and iOS writes settled_at into its own replica optimistically (Database.swift:2128, from SyncService.swift:9286) before sending the remote command. Intent routes through the host chokepoint correctly, but the replica write carries no revision, so a host guard that REJECTS a settle can still be overridden by the phone's row merging upstream. Step 0 of the sequencing is now: make settled_at host-authoritative and replace the optimistic write with local pending UI. 3. Revision column — local-only. C4 writes terminal_sessions per output chunk and cr-sqlite clocks are per column, so a revision column on that table would add a clock entry to the highest-frequency write in the product for a value no other device can use. It goes in a local-only table in LOCAL_ONLY_CRR_EXCLUDED_TABLES, keyed by session id, with an in-memory fallback that degrades identically to the settling state on restart. 4. 3d signed off with both additions: the residue must stay discoverable (diagnostics surface + orphan-reaper eligibility, not just a label, with detached nohup/setsid work reported separately since it is genuinely unreachable), and an analytics event per docs/logging.md — coarse reason and bucketed count, deduplicated per session so a failing fleet is not a burst. 5. Abort result decided: typed settle_aborted_by_activity, never silent success, with the contract documented for all five entry points. PR-merge auto-settle additionally must not mark a PR handled for an aborted session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
arul28
added a commit
that referenced
this pull request
Aug 11, 2026
…p 1) (#1073) * Settle-writer chokepoint and lifecycle revision (settle teardown, step 1) `settled_at` is written and cleared from ten call paths, five of the seven clearers not named "unsettle", one of them running per terminal output chunk. Attaching an async teardown to that was tried and cut in PR #1059 — it produced a P1 every review round, because a decision taken at t0 and applied at t0+T has no way to know the world moved in between. This lands the detector, with no teardown attached. **One writer.** `writeSettleLifecycle` is the only place the settle tuple is assigned; every call site now passes a typed intent (settle / clear / unsettle / override) plus its own non-settle columns. `settleLifecycleChokepoint.test.ts` scans the source and fails if any assignment appears outside the tuple generator — the guarantee belongs to the writer, so adding an eleventh path has to break a test rather than a review. **One revision.** `session_lifecycle_revisions`, a local-only table, bumped by a single `insert … on conflict` immediately adjacent to the column write. Local-only because `terminal_sessions` is a CRR whose settle row is rewritten per output chunk and cr-sqlite clocks are per column — a revision column would put a clock entry on the hottest write in the product for a value no other device can use. An in-memory counter backs it if the table write fails, degrading the same way the settling state will: a lost revision can only cause a settle to be re-taken, never a stale one to be applied. **Correction to the design doc, from doing the enumeration in code.** W3 (`setSettleOverride`) never assigns `settled_at` — it only reads it inside a `case`. But a `'settled'` pin makes a row read as settled anyway, so a revision keyed to `settled_at` alone would have been blind to it: finding 13 of §4 reappearing inside the inventory that was supposed to prevent it. The chokepoint owns the whole tuple instead. §1's counts are corrected with evidence (ten paths, not seven; five implicit clearers, not four). * fix: make the lifecycle revision always advance, and never regress Two flaws in my own first cut, found by tracing the fallback rather than by a test failing. **It could go backwards.** Bumps landed in the table normally and in memory when the table write failed. If the table then started working it began its own count at 1 while memory held more, and the reader preferred the table — handing back a lower number than it had already returned. A revision that moves backwards is worse than none: it lets a stale settle match a token it should have missed. **Taking the max fixed that but let it stall.** The table restarting at 1 under a higher memory value meant a real mutation could produce no observable change — a change the guard cannot see, which is the direction that actually hurts. So the in-process counter now moves on EVERY bump, not only on failure, and the read is the max of both stores. The table survives a restart; the counter guarantees advancement. Monotonic within a process and never regressing across a restart are the only two properties this value needs. Also fixes brittleness in the invariant test: it delimited the tuple generator by a neighbouring identifier, so renaming an unrelated const silently broke it. Explicit `settle-tuple-sql:start/end` sentinels now bound the region. * quality: close the chokepoint's escape hatches, and fix a test that asserted nothing Dual-track review of step 1. Track A confirmed all twelve converted statements are semantically equivalent per path, the CRR exclusion is genuinely applied, and `migrate()` reaches existing databases. The findings worth naming: **The CRR test asserted nothing.** It queried `crsql_master` for a `tbl_ver` key that does not exist in this cr-sqlite build, so it returned null for every table — including `terminal_sessions`, which IS a CRR. It now checks for the `__crsql_clock` sidecar using the repo's own idiom, with a positive control against `terminal_sessions` so the assertion can fail. This is the design's own finding 13 landing on the test meant to enforce it. **The invariant scan was single-file and matched comments.** It read only `sessionService.ts`, so an eleventh writer in a new service would have passed clean. It now walks `apps/desktop/src` and `apps/ade-cli/src`, and strips comments first — the web adapter documents the host's SQL in JSDoc, which the first widened version flagged as three offenders. **The writer still exposed raw SQL.** `where` and `revisionIds` could disagree silently; `extraSet` was a SQL fragment that could have carried a tuple column straight past the chokepoint, with the comma inside a ternary. Now one `sessionIds` array scopes the update AND the bump, and `extraSet` is a `Partial<Record<SettleExtraColumn, SqlValue>>` — writing a tuple column through it is a compile error. `settleMany` keeps its guard predicate as a parameterless `guard`: review argued it was provably redundant, but ADE runs several processes against one database, so another process can invalidate it between the select and the update. **The revision counted attempts, not changes.** `AdeDb.run` discarded the row count. Added `runChanged` (additive; `run` still returns void) and the bump now requires a matched row, so a no-op write cannot spend a revision. Also: revision rows and their in-process entries are reaped on `deleteSession`; the dual-store comment now gives the real reason (cross-process visibility, not just restart survival); §1's line numbers are replaced with method names after the refactor invalidated every one of them; and the "per output chunk" framing is corrected to the measured ~900 ms throttle. Records two gaps step 3 must not assume away: a sibling ADE process can read between the column write and the bump, and a paired desktop peer's settle arrives via `crsql_changes` without moving this host's revision. * test: extract the writer so the invariant is a file boundary The sessions folder was over the per-folder test budget, and my new file was the reason: it was named after a concept rather than a unit, which is the pattern the budget rule exists to catch. Extracting `settleLifecycleWriter.ts` resolves that properly instead of adding an exception. The test becomes a plain colocated `settleLifecycleWriter.test.ts` next to the module it covers, matching the four files already in the folder, and `sessionService.ts` drops 1945 → 1817 lines. It also makes the invariant stronger. The source scan no longer allowlists a region delimited by string offsets inside a 1800-line service — it allowlists a FILE, which no future reordering can silently widen. Verified non-vacuous: re-adding `settled_at = null` to a raw statement in `sessionService.ts` fails the test and names the offending file. Docs: the writer is in the terminals source map with the exact scope of what the revision detects, and §3a records the file-boundary allowlist. * fix(test): the CRR assertion assumed cr-sqlite is always loaded CI caught it: cr-sqlite ships macOS-only binaries, so on the Linux shards the extension is absent and NO table has a `__crsql_clock` sidecar. The positive control I added to stop this test being vacuous was itself the thing that failed. Now the strong pair — token excluded, `terminal_sessions` present — runs where CRR is actually loaded, and is explicitly skipped with the reason where no table has a clock and the assertion could not go red anyway. Same failure mode as the version this replaced, one level up: an assertion that cannot fail is not coverage, and neither is one that fails for the environment rather than the code. * fix: a failed token write must still outrun a sibling process Three review findings (Codex), all verified. **P1 — a real mutation could be invisible.** Another ADE process can push the persisted revision past this process's private counter. If a local mutation's token write then failed, incrementing the private counter alone left `max(persisted, inProcess)` UNCHANGED across a real change — and a teardown holding that token would accept a decision the world had already invalidated. The failure branch now anchors to `max(inProcess, persisted)` before incrementing, so it always outruns what a reader would have seen. The read stays off the output path: only the rare failure branch reads. Reproduced before fixing and pinned by `still advances when a sibling process is ahead and the local write fails` — reverting the anchor fails it with "expected 5 to be greater than 5". **P2 — bulk deletes orphaned tokens.** `laneService` deletes sessions by lane, never touching the token table, which has no foreign key. Swept by absence rather than by id list, so the fix covers that path and any future one — a targeted cascade is what keeps re-introducing this class. **P2 — the scan was case-sensitive.** `SET SETTLED_AT = NULL` is valid SQL and would have passed the load-bearing invariant test. Now case-insensitive, with an uppercase fixture asserting the matcher still bites. * review: assert both scan roots; keep no-op bumps, with the reason Two review findings. **Scan roots** — `fs.existsSync` skipped a missing root silently, and the `> 100 files` floor is satisfied by `apps/desktop/src` alone, so renaming `apps/ade-cli/src` would have quietly retired half the invariant. Both roots are now asserted. **No-op clears spend a revision — accepted, not fixed.** The finding is correct that `sqlite3_changes` counts matched rows rather than differing values, so re-clearing an already-clear tuple advances the revision. I implemented the suggested per-intent predicate and reverted it: putting it in the WHERE gates the caller's own columns too, so `setLastOutputPreview` stopped writing the preview entirely. Four tests caught it immediately. Doing it properly needs either two statements on the output path or a pre-read, both of which the design rules out. Over-bumping is the safe direction — a spent revision costs a re-taken settle, a missed one accepts a stale decision — and step 2's swallow rule is the mechanism that stops a session's own idle output invalidating its teardown, which is the concern behind the finding. Now documented in the writer and pinned by a test that asserts both halves: the revision does move, and the caller's columns still land.
This was referenced Aug 11, 2026
arul28
added a commit
that referenced
this pull request
Aug 11, 2026
…ardown, step 3) (#1076) * feat: attach real settle teardown, and reconcile peer settle-tuple writes Step 3 of the settle-teardown design. The seam is async now. Step 2 shipped a branded synchronous return type that made an awaited teardown a compile error; real stops are async, so that guard had to go. It was a tripwire, not an obstacle: it existed because bolting a deferred teardown onto a synchronous write is what produced a P1 in each of #1059's six rounds, and what changed is the machinery under it. The settling window is exclusive, abortable and in-memory, which is exactly what makes it safe to HOLD across an await — the revision re-check and abort check after the await are the suspension-point guards, and the race matrix covers both. Teardown (sessionSettleTeardown.ts) reuses stopLaneRuntimeWork's SHAPE, not its body: that function disposes chat sessions because it serves lane deletion, and a settle must leave the session usable. Terminals are never touched. Abort is checked before each step, so a turn that wins the race keeps its work. R5 lands as 3d option 3: an unconfirmed stop still settles, with residue recorded — coarse reason, reapable flag, local-only table, cleared implicitly when the session is reactivated — plus one bucketed analytics event per settle. Never silent. Peer tuple writes (R7 + the unsettle mirror) are fixed by finishing host authority, not by adding consensus. db.sync.applyChanges is the one place both the host and peer paths funnel through, so settle-tuple changes are held out of the raw apply and replayed through the chokepoint, gaining the revision and window semantics. Held rather than dropped: a paired desktop's decision is legitimate, it just has to come through the front door. No peer-visible token was built; onRemoteSettleWrite is the telemetry that decides whether one is ever justified. R7/R7b keep their raw db.run bypass and are annotated as to why: they pin the property that motivates the interception. * test: cover the residue table in the CRR-exclusion invariant Step 3 adds a second host-local settle table, so the exclusion test that guards the revision table now guards both. The positive control on terminal_sessions is what keeps the assertion falsifiable. * fix: quality dual-review findings, including two blockers Track A found two blockers, both real. 1. The brain never wired any of this. apps/ade-cli/src/bootstrap.ts built its sessionService with no teardown seam and never registered the apply-layer handler — and in a normal install the brain, not the desktop, is what applies changesets and serves phone sync, remote commands and the PR-merge poller. Teardown was a no-op for almost every settle a user can actually trigger. Both processes now build their hooks from one createSettleTeardownWiring factory so they cannot drift. 2. Holding settle-tuple rows out of crsql_changes breaks LWW convergence. The reviewer proved it against the vendored cr-sqlite build: a column that never enters crsql_changes never raises the local col_version, so this host stays behind the peer permanently and its NEXT genuine decision loses every merge. Two hosts then disagree forever — strictly worse than the bypass being fixed. Reversed: CRR now owns the values and the chokepoint owns the revision, via an observeRemote intent that self-assigns (matches the row, so the revision bumps; changes nothing, so no new column version and no echo). This also dissolves the composed-intent and lost-batch findings, since no intent is reconstructed and nothing is held. Also fixed: stop_and_clear was destroying the user's queued turns on every settle (now stop_only — 3c says losing a settle costs a click, losing the user's work does not); both new analytics properties were silently dropped by the sanitizer, so the telemetry the design leans on did not work, now pinned by a test; unbounded provider awaits could hold the settling window open forever and leave a row permanently unsettleable; count_bucket measured a value that was always 1; session_settle_residue leaked past deleteSession; residue analytics fired for settles that never landed; the settling window could be closed by an owner that no longer held it. Track B: settleSession now delegates to the typed form instead of duplicating it, the provider stop-control fact moved to subagentCapabilities, dead fields (stopped, scheduled_work, reapable) removed, two orphaned JSDoc blocks reattached, stale test title and a dangling comment asserting the opposite of the design deleted, and residue got a read path via the action registry — 'discoverable' was a condition of 3d option 3, not a nice-to-have. * fix: second-round review findings across teardown, reconcile and latency Track A verified two load-bearing claims empirically against the vendored cr-sqlite: the observeRemote self-assignment bumps sqlite3_changes without touching the clock (no echo), and stop_only really does stop background work. The design holds. What it found on top: - session.getSettleResidue was added to CTO_ONLY but not the allowlist, so every call was refused. The read path 3d option 3 was signed off on did not actually exist. - Stale residue survived a clean re-settle: nothing cleared the row when a later teardown confirmed everything, so it kept reporting an old failure with an old timestamp. - A peer write bumped the revision but never tripped the abort. The revision is only re-read AFTER teardown, so teardown ran to completion and interrupted a turn the user had just started on the other device — losing the work AND the settle, which is exactly the R2 shape 3c exists to prevent. - The reconcile handler fired on changes had discarded, so a re-delivered batch abandoned an in-flight settle over a duplicate packet. - A timed-out liveness read was indistinguishable from 'not a chat session', so a slow host settled while claiming a clean teardown — the one outcome residue exists to prevent. - settle_remote_write_reconciled fired on the NORMAL desktop-peer path, one event per session. 'Expected zero' was wrong: a paired desktop replicating its own settles belongs here. Now one batched event per changeset, framed as a rate signal. - Bulk settle was serial, and per session now costs up to 15s. iOS allows 30s for the whole command, so three busy sessions was a guaranteed timeout. Now bounded-concurrent, results reassembled in the caller's order. - Leaked ~50 unref'd timers per settling session; 10Hz polling of an expensive read; lmstudio missing from the provider dimension. Track B: restored the settle methods' locality after my own earlier repair scattered them, put back two invariant comments that repair dropped, moved the duplicated analytics envelope into the shared factory, made residue report a surviving turn separately from surviving jobs, and covered the hand-rolled cr-sqlite pk decoder — the riskiest code in the diff — with a test that drives the real applyChanges path. * fix: third-round review — the dedup guard was inert, and residue could be erased Track A proved my `result.changes > 0` guard does nothing. `crsql_changes` is a VIRTUAL table, so SQLite counts the xUpdate call whether or not cr-sqlite discarded the row as a losing merge — `insert or ignore` never engages, and a re-applied identical changeset still reports one change. Worse, round 2 had just made reconciliation trip the abort, so a duplicate packet (the peer's outbound cursor only advances on an ok ack, so a dropped ack re-sends the same range) would have killed a user's in-flight settle while carrying no new information. Replaced with a real value comparison: capture the column before the apply, report only if it actually moved. The kvDb test now applies the same changeset twice and asserts nothing is reported the second time — it fails against the old guard, which is how the inertness reproduces. Track A also found clearSettleResidue treating 'could not check' as 'confirmed clean'. An empty residue array is also what you get before the chat service exists and when the confirmation read times out, so a settle that verified nothing was deleting an accurate report of work still running. Teardown now returns an explicit `confirmed` flag and only a confirmed-clean settle may erase. The no-op fallbacks in both processes return confirmed: false. Same class, second site: the read-timeout rule was enforced after the first read but not on the confirmation read, so a hung confirm still returned clean. readWork is now a discriminated result, which makes the compiler force both call sites to decide. Track B: replaced the worker pool with chunking — order is preserved for free, and perSession, the queue and the reassembly loop all disappear; hoisted the concurrency constant to module scope; fixed three docblocks my own insertions had detached from their functions; typed ACTIVITY_ABORTS so a future abort reason cannot be silently mis-bucketed; restored the items[0] guard; renamed sessionCount to changesetSessionCount since it counts the changeset, not the reconciliation; made the concurrency test fail by assertion instead of by vitest timeout; and pinned action reachability, which is what H1 slipped through. Not changed: Track B read the settle methods as still scattered. Verified against main — the method order is byte-identical to base, so the interleaving is the pre-existing layout, not damage. Left alone rather than risk a third structural move in this file. * docs: correct settle claims that step 3 made false The terminals README still said settle 'deliberately does NOT stop the session's background work' and that a peer's CRR write is outside the revision's scope. Both were true when written and are not now. Replaced with what actually happens, including why the ordering of steps 0-3 was the thing that made it work. The sync docs were accurate about the phone-only column filter but silent on the desktop-peer path, which is the one a reader would now come looking for. * docs: record the sync bulk-settle shape as an open wire decision 3c's table says the sync entry point should carry the typed outcome additively; it still answers with a bare changed-id array, so an aborted id is indistinguishable from an ineligible one. Not a regression and not silently wrong — iOS's local overlay expires on its own rather than showing a settled row — but step 3 makes aborts likelier, and the fix is a wire-compatibility call that needs the mobile side, so it is written down rather than guessed at. * fix: fourth-round findings — restore the worker pool, and stop over-claiming confirmation Track A verified the value-based dedup guard against the real cr-sqlite across nine scenarios, including the one that actually matters: an exact duplicate arriving in a LATER applyChanges call is not reported, while a genuinely new change still is. It also confirmed the snapshot placement, the absence of SQL injection (the column is narrowed by the type guard before interpolation), and that a numeric val is safe under TEXT affinity. Four fixes from that pass: - Reverted chunking back to the worker pool. I took that simplification last round and it cost real throughput: a chunk barrier idles the other workers until its slowest member finishes, and 'every teardown is bounded' is not 'every teardown takes the same time'. Measured at roughly 65s versus 20s for a 50-session sweep with a quarter of the rows unstoppable — aimed straight at the 30s iOS budget the concurrency exists to protect. The perSession map already gave request order, so the simplification bought nothing. - An abort during the confirmation loop returned confirmed: true. Nothing was confirmed and the work was still running, which is precisely the shape the flag was added to make impossible — one refactor away from erasing an accurate residue record. - A confirmation-read timeout discarded a provider it had already read, losing the analytics dimension for the residue most worth attributing. - The value guard read blobs as null, so a blob that changed looked unchanged. Out-of-contract for any real writer, but the guard it replaced did report it. Both new tests were probed and both initially failed to be meaningful: the provider test was hitting a microtask race where the immediate expire won the FIRST read, and the confirmation test tripped the abort before the loop it was meant to exercise, so it passed against the bug. Fixed both, then re-probed — they now fail against the pre-fix code. * fix: stop the settle pool when a persistence failure propagates CodeRabbit, and it is a failure mode this branch introduced: before step 3, settleMany was one statement for the whole batch, so there was no partial state to lose. Now each session settles individually, and a SQLite lock thrown from one of them rejected Promise.all while the other workers kept shifting the queue — settling sessions the caller had already given up on. The queue is drained on the first failure so no NEW work starts, the sessions already in flight finish rather than being abandoned half-written, and the error is rethrown only once every worker has stopped. Persistence failures still propagate rather than being dressed up as a settle outcome — that distinction was an earlier review finding and it stands. What did settle is durable, and settle is idempotent, so the caller's retry re-reports it instead of double-filing. * fix: honor stop_only everywhere, and see background jobs that outlived a restart Two from Codex, both real, both undermining the point of the feature. stop_only was only honored on the Claude path. The OpenCode, Cursor, Pi and Droid branches of interrupt call cancelQueuedSteers unconditionally and return before the mode is ever consulted — so a settle on those providers silently deleted the user's queued prompts, which is exactly the unrecoverable loss the mode was added to prevent. Gated every one of them. The default is stop_and_clear, so the Stop button is untouched; only teardown asks for stop_only. activeBackgroundTaskCount is derived from the LIVE managed runtime, so a Claude --bg job that survives a brain or app restart reads as zero. Teardown saw a quiet session, skipped interrupt entirely, and filed the row as settled while the daemon job kept running — the precise bug this whole feature exists to fix, reintroduced through a liveness read. The summary already resolved the persisted job for other consumers; it is now on the type and counted as work. Both pinned by tests probed against the pre-fix code. Not fixed: CodeRabbit re-posted the queue-drain comment against the previous head; the drain landed in d499220. Its second half — surfacing the partial outcome instead of throwing — is deliberate: a SQLite lock is not a settle outcome, and settle is idempotent, so the caller's retry re-reports what landed. * fix: gate the persisted background job on daemon liveness Codex, and it is a defect my own previous fix created. claudeBackgroundJobShort is a RECORD, not a liveness signal — it survives the job finishing and survives teardown stopping it. Counting it unconditionally meant every later settle on that session would spend the full confirmation budget and then report residue for a job that no longer exists, while trying to stop it again each time. Now the daemon is asked, through a narrow hasLiveClaudeBackgroundJob exported from agentChatService, and only when the live count already says quiet AND a job is on record — the restart case. That keeps the round-trip off the hot read while still closing the hole where a job outlives its runtime. Also documented, not fixed: a provider stop that overruns its 10s ceiling keeps running, because interrupt takes no abort signal, so a late session-scoped abort could stop a turn the user started after the settle was abandoned. Removing the ceiling is a certain wedge; keeping it is a narrow race needing a 10s+ hang, a new turn inside that window, and the abort still applying. Written up in the design doc (6c-ii) rather than silently traded. Two other comments on this head are stale re-posts: the stop_only gating landed in 86c4c5c (verified present in all seven provider branches), and the queue-drain in d499220. * fix: make background-job liveness required and tri-state Both from this round's review, and both are defects my own liveness fix introduced one commit earlier. hasLiveClaudeBackgroundJob was optional, so a wiring that omitted it read a recorded job as absent and confirmed a clean teardown over work still running — reopening the exact hole the callback was added to close. Now required. And it returned a boolean, which collapsed "the daemon says the job is gone" into the same answer as "the daemon could not be reached" (getLiveClaudeBackgroundSocket catches socket and request failures and returns null). Guessing "finished" is the guess that settles over a running job — the same shape as treating a timed-out liveness read as an idle session, which this branch has now had to fix three times in three places. It returns alive / gone / unknown, and only a definite "gone" counts as no work. Also pinned: interrupt's daemon stop branch is gated on there being no resident Claude runtime, so a resumed session with a live --bg job takes the SDK branch and the job survives. Teardown does not claim that as clean — the confirmation loop still sees the job and reports residue — and there is now a test saying so. Changing that gate would change what the Stop button does, which is not this branch's call. Restored the activeBackgroundTaskCount JSDoc my insertion had detached. * fix: scope the abort check to the window the teardown actually owns Codex, and it is the mirror of a fix already made for closing the window. `end(id, token)` refuses to close a window it does not own, but the abort check still read `abortedBy(id)` — whatever entry currently occupies the id. When `deleteSession` runs mid-teardown, `forget` force-closes the entry; if the id is then recreated and a new settle opens a fresh window, the stale teardown reads the REPLACEMENT, sees "not aborted", and keeps issuing provider stops against the new session's work. `abandoned(id, token)` treats a missing or mismatched entry as abandoned, and both the in-flight check and the post-await check use it. A settle that no longer owns its window must stop as surely as one that was aborted. The other seven comments on this head are threads GitHub re-anchored: stop_only gating (86c4c5c, verified in all seven provider branches), the queue drain (d499220), persisted-job liveness and unknown-as-residue (df2aa0a), the JSDoc association (df2aa0a), the resident-runtime daemon job (pinned as residue rather than silently clean, with a test), and the un-cancellable timed-out interrupt, which is documented as a known limitation in 6c-ii because removing the ceiling trades a narrow race for a certain wedge. * fix: a hung provider stop is a timeout, not a rejection Codex. The 10s ceiling set `stopRejected`, so a provider that never answered was filed identically to one that explicitly refused — in the residue the user reads AND in the settle_teardown_residue analytics dimension. That conflation is precisely what the reason field exists to prevent, and it would have made "how often do stops actually fail in the field" unanswerable, which is the question 3d option 3 added the event to answer. Two existing assertions had encoded the bug rather than catching it: both said a never-resolving interrupt should read "rejected". Corrected, and the fix was probed against them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background-work liveness in canonical phase, and the archive ordering fix
Tier 1B of the 2026-08-09 t3code competitor audit (§4.1–4.2).
Liveness was tracked but invisible
A session whose foreground turn ended while its background shells, monitors, or subagent fleet kept going read as idle everywhere a user glances — the Work-tab dot, the TopBar rollup, the dock badge, and the Lanes agent list all showed nothing while agents were mid-run. A
Background work ×Nlabel existed, but only as a label: it never reached the canonical phase those surfaces derive from.canonicalSessionStatenow promotes a resting session with live background work back torunning, and reports why via a newlivenessfield (turn/background/monitoring). Every existing phase consumer inherits the truth with no special case.monitoringonly when watch loops are the sole live work. One real job among three monitors is still working — the honest summary of a session is its loudest live commitment.MONITOR_TASK_TYPESis onlymonitor/monitor_mcp. Generic backgrounded shells (local_bash,shell,background,bash) are a mixed bag — atail -fand a 20-minutenpm run buildarrive under the same type — and mixed is unknown, and unknown is working. An allowlist would silently drop a real subagent the first time a provider SDK renamed a type.ChatRuntime["kind"], so a newly landed harness fails to compile until someone classifies it. Pi is listed explicitly — verified against its runtime, not assumed.TerminalAttentionSummary.byLaneIdremoved deliberately: zero consumers, andlaneListSnapshotServicealready owns the per-lane rollup.Mobile inherits this for free. iOS never decodes the count; it reads the roster's derived status, and
rosterBuilder.liveChatStatusalready mapsactiveBackgroundTaskCount > 0 → running. Because that count is now cross-runtime, the phone gains Codex/Cursor liveness with zero iOS change.Archive released ports while processes still held them
laneService.archivewas a bare status write, and every caller releases the lane's port lease and proxy route the moment it returns — so the lease went back to the pool while the lane's dev servers and agents were still bound, and the abandoned processes went on holding ports and memory because an archived lane is filtered out of every surface that could have shown them.archiveis now async and stops the lane's chats, PTYs, watchers, and auto-rebase through a sharedstopLaneRuntimeWorkbefore the status write.archiveAndReclaimuses the same helper;deletekeeps itsrunStepversion because the delete dialog reports each step.Why settle teardown is not here
Settle-time teardown was built, then cut after six review rounds found a real P1 in it every single time — an unsettle path that skipped the resume (×3, each a route the previous fix hadn't traced), settling mid-turn tearing down nothing, the RPC operator bridge never receiving the control, a stop count that couldn't be kept honest, and finally an activity guard that reads
lastActivityAt— backed bylast_output_at, a columnclearTurnStartMarkersnever writes. That guard provably could not fire.The shape is unambiguous: teardown is async and
settled_atis written and cleared from seven places. A teardown-then-write settle races real activity, and the failure is not one-sided — a user starting a turn during a provider stop call gets their background work stopped and no settle. Every guard against it either read a column turn-start doesn't update, or had to be repeated identically at seven entry points.Doing it correctly needs a synchronous lifecycle revision that teardown serializes against — a different change, designed as one. A design doc follows this PR; no reimplementation until it's reviewed.
Windows
No new kill logic. Archive teardown delegates to
ptyService/agentChatServicedisposal, which already route throughtaskkill /T /F+child.kill()with the PID-reuse guard. No capability is Windows-blocked.Tests
Consolidated end-of-life teardown coverage; new coverage drives a real
background_tasks_changedlevel and pins the denylist split, the omit-when-drained summary shape, lane-agent liveness and sorting, the older-peer fallback, and the archive stop-before-status ordering.🤖 Generated with Claude Code
Greptile Summary
This PR centralizes background-work liveness in canonical session state, broadens tracking across supported runtimes, and stops lane runtime work before archive resource release. The settled-state fix remains incomplete because existing phase-based surfaces ignore its new liveness field.
Confidence Score: 4/5
The PR is not yet safe to merge because settled sessions with live background work still disappear from active-work status and rollups.
The new liveness field records that a settled session is still working, but canonical phase remains settled and the existing presentation and attention consumers ignore liveness for that phase, leaving the previously reported hidden-work failure outstanding.
Files Needing Attention: apps/desktop/src/shared/sessionCanonicalState.ts, apps/desktop/src/shared/sessionStatusPresentation.ts, apps/desktop/src/renderer/lib/terminalAttention.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Live background work] --> B[canonicalSessionState] B --> C{Session settled?} C -- No --> D[phase: running] C -- Yes --> E[phase: settled<br/>liveness: background] D --> F[Active-work indicators] E --> G[Settled filing] G --> H[Live work omitted from rollups]Prompt To Fix All With AI
Reviews (9): Last reviewed commit: "fix(sessions): a settled row still repor..." | Re-trigger Greptile
Context used (4)
ade codeTUI