Skip to content

pi coding agent in ade -> Primary - #1054

Merged
arul28 merged 1 commit into
mainfrom
ade/pi-coding-agent-in-ade-b8fbbf2d
Aug 8, 2026
Merged

pi coding agent in ade -> Primary#1054
arul28 merged 1 commit into
mainfrom
ade/pi-coding-agent-in-ade-b8fbbf2d

Conversation

@arul28

@arul28 arul28 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

ADE   Open in ADE  ·  ade/pi-coding-agent-in-ade-b8fbbf2d branch  ·  PR #1054

Greptile Summary

This PR adds Pi as a first-class coding-agent provider across desktop chat, tracked CLI sessions, external-session import, the ADE TUI, model selection, sync, and iOS.

  • Adds a worker-pooled Pi SDK runtime with session leasing, protocol/event mapping, persistence, and provider health checks.
  • Adds Pi model catalogs, authentication and installation affordances, permission modes, logos, and cross-client model pickers.
  • Adds discovery, fork/resume, terminal tracking, and remote import support for native Pi sessions.

Confidence Score: 4/5

The PR should not merge until Pi fork imports retain a deterministic provider-session target for later resumes.

A forked Pi session is persisted without its resulting session identity, so reopening it uses pi --continue and can attach to an unrelated, more recent Pi conversation in the same working directory.

Files Needing Attention: apps/desktop/src/main/services/externalSessions/externalSessionsService.ts and apps/desktop/src/shared/cliLaunch.ts

Important Files Changed

Filename Overview
apps/desktop/src/main/services/chat/agentChatService.ts Integrates Pi chat creation, runtime startup, event streaming, persistence, interruption, resume, and model changes.
apps/desktop/src/main/services/chat/piSdkPool.ts Adds pooled child-worker management, correlated RPC requests, cleanup, and failure propagation for Pi.
apps/desktop/src/main/services/chat/piSdkWorker.ts Hosts Pi SDK sessions and maps initialization, prompting, model changes, aborts, and disposal onto the worker protocol.
apps/desktop/src/main/services/externalSessions/externalSessionsService.ts Adds Pi discovery/import capabilities, but fork imports persist no specific resume target and can later continue the wrong session.
apps/desktop/src/shared/cliLaunch.ts Adds Pi launch/resume commands, model normalization, thinking levels, and permission-derived tool flags.
apps/desktop/src/shared/modelRegistry.ts Adds dynamically registered Pi models with canonical IDs carrying profile, provider, and model identity.
apps/ade-cli/src/tuiClient/adeApi.ts Extends the TUI API path to select and create Pi sessions through canonical model descriptors.
apps/ios/ADE/Views/Work/WorkModelCatalog.swift Adds Pi model metadata parsing and catalog presentation for the iOS Work experience.

Sequence Diagram

sequenceDiagram
  participant Client as Desktop/TUI/iOS
  participant Brain as ADE Brain
  participant Pool as Pi SDK Pool
  participant Worker as Pi Worker
  participant Pi as Pi SDK/Provider
  Client->>Brain: Create Pi chat with model ID
  Brain->>Pool: Acquire session-scoped worker
  Pool->>Worker: Initialize model, tools, and session
  Worker->>Pi: Open or create provider session
  Pi-->>Worker: Session ID and session file
  Worker-->>Brain: Canonical events and pointer state
  Brain-->>Client: Stream chat events
  Client->>Brain: Resume tracked session
  Brain->>Worker: Reopen persisted Pi session
Loading

Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
apps/desktop/src/main/services/externalSessions/externalSessionsService.ts:900
**Fork resume loses Pi identity**

When a user forks a Pi session and later reopens it after another Pi session has run in the same working directory, this branch persists a null target and the resume builder falls back to `pi --continue`, causing ADE to attach to and extend the wrong conversation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat: add Pi as a first-class agent harn..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used (4)

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Aug 8, 2026 5:41pm

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 125 files, which is 25 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 23684b1d-349e-4ac3-a742-fc79d4223e54

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca0930 and cfef213.

⛔ Files ignored due to path filters (1)
  • apps/desktop/src/renderer/assets/provider-logos/pi.svg is excluded by !**/*.svg
📒 Files selected for processing (125)
  • apps/ade-cli/src/adeRpcServer.ts
  • apps/ade-cli/src/services/agentRegistry.test.ts
  • apps/ade-cli/src/services/agentRegistry.ts
  • apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
  • apps/ade-cli/src/tuiClient/adeApi.ts
  • apps/ade-cli/src/tuiClient/app.tsx
  • apps/ade-cli/src/tuiClient/closedCliSessions.ts
  • apps/ade-cli/src/tuiClient/commands.ts
  • apps/ade-cli/src/tuiClient/components/ModelPicker/ModelPickerPane.tsx
  • apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.test.ts
  • apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.ts
  • apps/ade-cli/src/tuiClient/components/ModelPicker/types.ts
  • apps/ade-cli/src/tuiClient/components/RightPane.tsx
  • apps/ade-cli/src/tuiClient/components/SlashPalette.tsx
  • apps/ade-cli/src/tuiClient/externalSessionBrowser.ts
  • apps/ade-cli/src/tuiClient/modelPickerController.ts
  • apps/ade-cli/src/tuiClient/modelState.ts
  • apps/ade-cli/src/tuiClient/planMode.ts
  • apps/ade-cli/src/tuiClient/providerMetadata.ts
  • apps/ade-cli/src/tuiClient/remoteLauncher.ts
  • apps/ade-cli/src/tuiClient/theme.ts
  • apps/ade-cli/src/tuiClient/types.ts
  • apps/desktop/src/main/services/__tests__/piSdk.integration.test.ts
  • apps/desktop/src/main/services/adeActions/registry.ts
  • apps/desktop/src/main/services/ai/aiIntegrationService.ts
  • apps/desktop/src/main/services/ai/aiSettingsStatus.ts
  • apps/desktop/src/main/services/ai/piInstallation.ts
  • apps/desktop/src/main/services/ai/providerConnectionStatus.test.ts
  • apps/desktop/src/main/services/ai/providerConnectionStatus.ts
  • apps/desktop/src/main/services/ai/providerRuntimeHealth.ts
  • apps/desktop/src/main/services/ai/tools/systemPrompt.ts
  • apps/desktop/src/main/services/chat/agentChatService.ts
  • apps/desktop/src/main/services/chat/contextCompactionEmitter.ts
  • apps/desktop/src/main/services/chat/piSdkEnvironment.test.ts
  • apps/desktop/src/main/services/chat/piSdkEnvironment.ts
  • apps/desktop/src/main/services/chat/piSdkEventMapper.ts
  • apps/desktop/src/main/services/chat/piSdkPool.ts
  • apps/desktop/src/main/services/chat/piSdkProtocol.test.ts
  • apps/desktop/src/main/services/chat/piSdkProtocol.ts
  • apps/desktop/src/main/services/chat/piSdkWorker.ts
  • apps/desktop/src/main/services/chat/piSessionLease.test.ts
  • apps/desktop/src/main/services/chat/piSessionLease.ts
  • apps/desktop/src/main/services/chat/threadPointerLedger.ts
  • apps/desktop/src/main/services/config/projectConfigService.ts
  • apps/desktop/src/main/services/externalSessions/discoverPi.test.ts
  • apps/desktop/src/main/services/externalSessions/discoverPi.ts
  • apps/desktop/src/main/services/externalSessions/discoveryUtils.ts
  • apps/desktop/src/main/services/externalSessions/externalSessionsService.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/main/services/pty/ptyService.test.ts
  • apps/desktop/src/main/services/pty/ptyService.ts
  • apps/desktop/src/main/services/sessions/sessionService.ts
  • apps/desktop/src/main/utils/sessionSummary.ts
  • apps/desktop/src/main/utils/terminalSessionSignals.ts
  • apps/desktop/src/main/utils/terminalTuiMarkers.ts
  • apps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatMessageList.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
  • apps/desktop/src/renderer/components/chat/AgentCliAuthCard.tsx
  • apps/desktop/src/renderer/components/prs/state/PrsContext.tsx
  • apps/desktop/src/renderer/components/settings/ChatAppearancePreview.test.tsx
  • apps/desktop/src/renderer/components/settings/ProvidersSection.test.tsx
  • apps/desktop/src/renderer/components/settings/ProvidersSection.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelListRow.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.test.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerContent.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerEmptyState.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerRail.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/ReasoningEffortPicker.test.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/modelCatalog.test.ts
  • apps/desktop/src/renderer/components/shared/ModelPicker/modelCatalog.ts
  • apps/desktop/src/renderer/components/shared/ModelPicker/providerEmptyState.tsx
  • apps/desktop/src/renderer/components/shared/ModelPicker/runtimeCatalogCache.ts
  • apps/desktop/src/renderer/components/shared/ModelPicker/useProviderAuthStatus.test.ts
  • apps/desktop/src/renderer/components/shared/ModelPicker/useProviderAuthStatus.ts
  • apps/desktop/src/renderer/components/shared/ProviderLogos.tsx
  • apps/desktop/src/renderer/components/shared/useOpenProviderSignIn.ts
  • apps/desktop/src/renderer/components/terminals/ToolLogos.tsx
  • apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx
  • apps/desktop/src/renderer/components/terminals/cliLaunch.test.ts
  • apps/desktop/src/renderer/components/terminals/importSessions/ImportSessionBrowser.tsx
  • apps/desktop/src/renderer/components/terminals/importSessions/contract.ts
  • apps/desktop/src/renderer/components/work/PiLoginPromptButton.tsx
  • apps/desktop/src/renderer/lib/modelOptions.ts
  • apps/desktop/src/renderer/lib/nativeLaunchControls.ts
  • apps/desktop/src/renderer/lib/sessions.ts
  • apps/desktop/src/renderer/lib/terminalAttention.ts
  • apps/desktop/src/shared/cliLaunch.ts
  • apps/desktop/src/shared/contextCompaction.ts
  • apps/desktop/src/shared/externalSessionAffordances.ts
  • apps/desktop/src/shared/modelCatalog.test.ts
  • apps/desktop/src/shared/modelCatalog.ts
  • apps/desktop/src/shared/modelRegistry.test.ts
  • apps/desktop/src/shared/modelRegistry.ts
  • apps/desktop/src/shared/orchestrationRuntimePolicy.test.ts
  • apps/desktop/src/shared/orchestrationRuntimePolicy.ts
  • apps/desktop/src/shared/pendingInputLabels.ts
  • apps/desktop/src/shared/types/chat.ts
  • apps/desktop/src/shared/types/config.ts
  • apps/desktop/src/shared/types/externalSessions.ts
  • apps/desktop/src/shared/types/sessions.ts
  • apps/desktop/src/shared/types/sync.ts
  • apps/desktop/tsup.config.ts
  • apps/ios/ADE/Models/RemoteModels.swift
  • apps/ios/ADE/Services/SyncService.swift
  • apps/ios/ADE/Shared/ADESharedTheme.swift
  • apps/ios/ADE/Views/Hub/HubComposerDrawer.swift
  • apps/ios/ADE/Views/PersonalChats/PersonalChatsScreen.swift
  • apps/ios/ADE/Views/Work/WorkBrowserHelpers.swift
  • apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift
  • apps/ios/ADE/Views/Work/WorkContextCompactDivider.swift
  • apps/ios/ADE/Views/Work/WorkExternalSessionAffordances.swift
  • apps/ios/ADE/Views/Work/WorkImportSessionScreen.swift
  • apps/ios/ADE/Views/Work/WorkModelCatalog.swift
  • apps/ios/ADE/Views/Work/WorkModelPickerSheet.swift
  • apps/ios/ADE/Views/Work/WorkModels.swift
  • apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
  • apps/ios/ADE/Views/Work/WorkNewChatSheet.swift
  • apps/ios/ADE/Views/Work/WorkSessionSettingsSheet+Actions.swift
  • apps/ios/ADE/Views/Work/WorkSessionSettingsSheet.swift
  • apps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swift
  • apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift
  • apps/ios/ADETests/ADETests.swift

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.

throw new Error("OpenCode sessions cannot be copied into a different lane folder.");
throw new Error(`${provider === "pi" ? "Pi" : "OpenCode"} sessions cannot be copied into a different lane folder.`);
}
metadataTargetId = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Fork resume loses Pi identity

When a user forks a Pi session and later reopens it after another Pi session has run in the same working directory, this branch persists a null target and the resume builder falls back to pi --continue, causing ADE to attach to and extend the wrong conversation.

Knowledge Base Used: Chat, terminal, and provider session management

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/services/externalSessions/externalSessionsService.ts
Line: 900

Comment:
**Fork resume loses Pi identity**

When a user forks a Pi session and later reopens it after another Pi session has run in the same working directory, this branch persists a null target and the resume builder falls back to `pi --continue`, causing ADE to attach to and extend the wrong conversation.

**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)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

@arul28
arul28 merged commit 5227b9e into main Aug 8, 2026
67 of 69 checks passed
arul28 added a commit that referenced this pull request Aug 10, 2026
…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>
arul28 added a commit that referenced this pull request Aug 10, 2026
…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>
arul28 added a commit that referenced this pull request Aug 10, 2026
…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>
arul28 added a commit that referenced this pull request Aug 10, 2026
…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>
arul28 added a commit that referenced this pull request Aug 10, 2026
…ring (#1059)

* feat(sessions): promote live background work into canonical phase

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>

* feat(sessions): settle stops the machinery it claims to conclude

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>

* fix(sessions): quality pass — honest teardown counts, no zero-record 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>

* fix(sessions): keep bulk-settle validation throwing synchronously + Pi 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>

* docs(sessions): drop reference to the settlement-blocker helper main removed

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

* test(sessions): consolidate end-of-life teardown coverage, pin the liveness 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>

* fix(sessions): give the settle pause an exact undo, and stop calling 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>

* test(prs): update the PR-merge teardown mock for the settle-scoped pause 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>

* fix(sessions): resume at the settle-clearing write, stop bypasses and 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>

* revert(sessions): settle no longer pauses scheduled work

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>

* fix(sessions): stop detached work mid-turn, wire RPC CTO teardown, drop 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>

* fix(sessions): never close a background task whose stop was not confirmed

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>

* revert(sessions): settle no longer stops background work

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>

* fix(sessions): a settled row still reports live background work

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>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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