From 99e79846816bd2b96b3c5702977136cc4cf6d1ab Mon Sep 17 00:00:00 2001 From: Tinycute00 <178717298+Tinycute00@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:21:00 +0800 Subject: [PATCH 01/11] fix(coding-agent): run resume budget check before session teardown Resuming a session whose restored transcript exceeds the current model's context budget threw ModelUsabilityBudgetError only from createAgentSession, which runs after switchSession has already torn down the live session and invalidated its extension runner. A resume that was always going to be rejected therefore destroyed the session the user was still in, and the next input crashed with the stale-extension-context error. In interactive mode the error was routed to handleFatalRuntimeError -> process.exit(1), so the user was silently dropped to the shell. AgentSessionRuntime.switchSession now runs assertSessionAdmissible before teardownCurrent, mirroring the post-teardown admission check in createAgentSession against the current model. A rejected resume becomes a clean no-op that leaves the live session intact. InteractiveMode.handleResumeSession now catches ModelUsabilityBudgetError, shows it via showError, and returns cancelled instead of exiting the process. --- .../src/core/agent-session-runtime.ts | 17 ++++++ packages/coding-agent/src/core/changes.md | 18 ++++++ .../src/modes/interactive/changes.md | 18 ++++++ .../src/modes/interactive/interactive-mode.ts | 5 ++ .../test/suite/agent-session-runtime.test.ts | 55 +++++++++++++++++++ 5 files changed, 113 insertions(+) diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 4fcc44cbd2..5e22627058 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -3,6 +3,7 @@ import { basename, join, resolve } from "node:path"; import { resolvePath } from "../utils/paths.ts"; import type { AgentSession } from "./agent-session.ts"; import type { AgentSessionRuntimeDiagnostic, AgentSessionServices } from "./agent-session-services.ts"; +import { estimateTokens } from "./compaction/compaction.ts"; import type { ProjectTrustContext, ReplacedSessionContext, @@ -253,6 +254,7 @@ export class AgentSessionRuntime { const previousSessionFile = this.session.sessionFile; const sessionManager = SessionManager.open(sessionPath, undefined, options?.cwdOverride); assertSessionCwdExists(sessionManager, this.cwd); + await this.assertSessionAdmissible(sessionManager); await this.teardownCurrent("resume", sessionManager.getSessionFile()); await this.apply( await this.createRuntime({ @@ -268,6 +270,21 @@ export class AgentSessionRuntime { return { cancelled: false }; } + /** + * Re-run the resume admission check (model usability budget) against the + * target session's context without replacing the current session. Mirrors + * the check createAgentSession performs after teardown; running it early + * keeps a rejected resume from invalidating the live session. + */ + async assertSessionAdmissible(sessionManager: SessionManager): Promise { + const existingSession = sessionManager.buildSessionContext(); + if (existingSession.messages.length === 0) return; + const model = this.session.model; + if (!model) return; + const liveContextTokens = existingSession.messages.reduce((total, message) => total + estimateTokens(message), 0); + this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" }); + } + async newSession(options?: { parentSession?: string; setup?: (sessionManager: SessionManager) => Promise; diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index d42fdd9dae..7b633f9f19 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,21 @@ +## Resume admission runs before teardown so a rejected resume keeps the live session (2026-09-08) + +### What changed + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` now calls a new `assertSessionAdmissible(sessionManager)` before `teardownCurrent("resume", ...)`. The method rebuilds the target session context, sums its live-context tokens with `estimateTokens` (imported from `./compaction/compaction.ts`), and runs `this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" })` against the current model - mirroring the post-teardown check `createAgentSession` performs in `sdk.ts`. When the target is empty or the session has no model it is a no-op. + +### Why + +- Resuming a session whose restored transcript exceeds the current model's context budget threw `ModelUsabilityBudgetError` only from `createAgentSession`, which runs after `teardownCurrent` has already disposed the live session and invalidated its extension runner. The user's still-active session was destroyed by a resume that was always going to be rejected, and the next input crashed with "This extension ctx is stale after session replacement or reload". Running the same admission check before teardown makes a rejected resume a clean no-op that leaves the live session intact. + +### Why an extension could not handle it + +- The teardown/create ordering lives inside the core `AgentSessionRuntime.switchSession` state machine. No extension hook fires between `teardownCurrent` (which disposes the session and invalidates the runner) and `createRuntime`, so no extension can intercept the rejection before the live session is torn down. + +### Expected merge conflict zones + +- LOW: the added `assertSessionAdmissible` call in `switchSession`, the new method placed after `switchSession`, and the `estimateTokens` import line. + ## Insufficient accepted compaction keeps its blocked state, #7921 case 6 (2026-09-07) ### What changed diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index dab9282119..18c1a5e457 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -1,4 +1,22 @@ +## 2026-09-08 - Over-budget resume shows an error instead of exiting the process + +### What changed + +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: `handleResumeSession`'s catch block now handles `ModelUsabilityBudgetError` (imported from `../../core/extensions/builtin/compaction/model-usability-budget.ts`) before the `handleFatalRuntimeError` fallback. It renders the message through `showError` and returns `{ cancelled: true }` instead of routing to `handleFatalRuntimeError`, which calls `process.exit(1)`. + +### Why + +- A `/resume` rejected by the model usability budget is an expected, recoverable outcome, not a fatal runtime fault. Routing it to `handleFatalRuntimeError` tore the TUI down and called `process.exit(1)` before the error text could repaint, so the user was silently dropped to the shell. Showing the error and cancelling keeps the interactive session running with a visible explanation. + +### Why an extension could not handle it + +- The resume error is caught inside the interactive mode's own `handleResumeSession` control flow; the process-exit decision is core interactive-mode code with no extension hook between the caught error and `handleFatalRuntimeError`. + +### Expected merge conflict zones + +- LOW: the new `ModelUsabilityBudgetError` branch in `handleResumeSession` and the added import line. + ## 2026-09-07 - Add a workflow tip for the report-bug skill ### What changed diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index e1f3822f4e..d3273deb48 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -74,6 +74,7 @@ import { computeCacheWaste, detectCacheMiss, } from "../../core/cache-stats.ts"; +import { ModelUsabilityBudgetError } from "../../core/extensions/builtin/compaction/model-usability-budget.ts"; import type { AutocompleteProviderFactory, EditorFactory, @@ -7576,6 +7577,10 @@ export class InteractiveMode { this.showStatus("Resumed session in current cwd"); return result; } + if (error instanceof ModelUsabilityBudgetError) { + this.showError(`Failed to resume session: ${error.message}`); + return { cancelled: true }; + } return this.handleFatalRuntimeError("Failed to resume session", error); } } diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index 30a7cc1c11..6dfae88480 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -11,6 +11,7 @@ import { createAgentSessionServices, } from "../../src/core/agent-session-runtime.ts"; import { AuthStorage } from "../../src/core/auth-storage.ts"; +import { ModelUsabilityBudgetError } from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; import { SessionManager } from "../../src/core/session-manager.ts"; import type { AgentToolResult, @@ -652,4 +653,58 @@ describe("AgentSessionRuntime characterization", () => { expect(runtime.session.model?.id).toBe("faux-2"); expect(runtime.session.thinkingLevel).toBe("off"); }); + + // Regression: a resume rejected by the model usability budget must run its + // admission check BEFORE teardown, so the live session the user is still in is + // never disposed/invalidated by a resume that will fail anyway. + it("rejects an over-budget resume without invalidating the live session", async () => { + const { runtime } = await createRuntimeForTest(() => {}); + await runtime.session.prompt("hello"); + const originalSession = runtime.session; + const originalSessionFile = runtime.session.sessionFile; + + // Seed a target session whose restored transcript far exceeds the current + // model's context window (faux default contextWindow is 128000 tokens). + const targetDir = join(tmpdir(), `pi-runtime-overbudget-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(targetDir, { recursive: true }); + const targetSession = SessionManager.create(targetDir); + const targetModel = runtime.session.model!; + targetSession.appendMessage({ + role: "user", + content: [{ type: "text", text: "x".repeat(4_000_000) }], + timestamp: Date.now() - 1, + }); + // A trailing assistant message flushes the buffered transcript to disk so the + // re-opened session manager actually reports the over-budget live context. + targetSession.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "ok" }], + api: targetModel.api, + provider: targetModel.provider, + model: targetModel.id, + stopReason: "stop", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: Date.now(), + }); + const targetSessionFile = targetSession.getSessionFile(); + cleanups.push(() => rmSync(targetDir, { recursive: true, force: true })); + + await expect(runtime.switchSession(targetSessionFile!)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + + // The live session object and its file are unchanged... + expect(runtime.session).toBe(originalSession); + expect(runtime.session.sessionFile).toBe(originalSessionFile); + // ...and it is still usable: teardown never disposed it, so its extension + // runner was never invalidated (pre-fix, teardown ran first and the next + // input crashed with the stale-context error). + expect(runtime.session.extensionRunner.isActive).toBe(true); + await expect(runtime.session.prompt("still here")).resolves.toBeUndefined(); + }); }); From 0b85a15f29fbeaf8253a3ac263d80cf20437d26c Mon Sep 17 00:00:00 2001 From: Tinycute00 <178717298+Tinycute00@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:00:04 +0800 Subject: [PATCH 02/11] fix(coding-agent): check resume budget against the restored model before the switch event Resolve the model a resume will actually restore (the destination session's stored model via resolveStoredModelReference when present and authorized, else the live model) and run the admission check against it, so a resume that the active model would fit but the restored model cannot is rejected before the live session is torn down. Move the non-mutating preflight (SessionManager.open, cwd existence, admission) ahead of the session_before_switch emit so a rejected resume is a true no-op and never lets handlers mutate live-session state. Give the cwd-override retry in handleResumeSession the same recoverable budget handling via a shared cancelResumeWithBudgetError helper instead of letting a second-attempt budget error escape as an unhandled rejection. Preserve the budget rejection's typed identity across the shared-host RPC boundary: connection-handler emits a model_usability_budget error code with the projection, and rpc-client reconstructs ModelUsabilityBudgetError so the TUI's instanceof check still holds client-side. --- packages/coding-agent/CHANGELOG.md | 2 + .../src/core/agent-session-runtime.ts | 40 ++++++- packages/coding-agent/src/core/changes.md | 9 +- .../src/modes/interactive/changes.md | 2 +- .../src/modes/interactive/interactive-mode.ts | 35 ++++-- .../coding-agent/src/modes/rpc/changes.md | 19 +++ .../src/modes/rpc/connection-handler.ts | 16 ++- .../coding-agent/src/modes/rpc/rpc-client.ts | 6 + .../interactive-mode-resume-budget.test.ts | 111 ++++++++++++++++++ .../test/rpc-client-budget-error.test.ts | 72 ++++++++++++ .../test/suite/agent-session-runtime.test.ts | 79 +++++++++++++ 11 files changed, 369 insertions(+), 22 deletions(-) create mode 100644 packages/coding-agent/test/interactive-mode-resume-budget.test.ts create mode 100644 packages/coding-agent/test/rpc-client-budget-error.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ff87444e1a..02e59f648e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,8 @@ ### Fixed +- Resume preflight now runs the model-budget admission check before session teardown, against the model the resume will actually restore, so a `/resume` that exceeds the target model's context window shows an error and keeps the live session running instead of tearing it down and silently exiting (exit 1). The check runs before the switch lifecycle event so a rejected resume is a true no-op, the cwd-override retry gets the same recoverable handling, and the rejection keeps its typed identity across the shared-host RPC boundary. + ### Removed ## [2026.9.7-2] - 2026-09-07 diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 5e22627058..811b4c760d 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -1,5 +1,6 @@ import { copyFileSync, existsSync, mkdirSync } from "node:fs"; import { basename, join, resolve } from "node:path"; +import type { Api, Model } from "@earendil-works/pi-ai"; import { resolvePath } from "../utils/paths.ts"; import type { AgentSession } from "./agent-session.ts"; import type { AgentSessionRuntimeDiagnostic, AgentSessionServices } from "./agent-session-services.ts"; @@ -11,6 +12,7 @@ import type { SessionStartEvent, } from "./extensions/index.ts"; import { type ExtensionRunner, emitSessionShutdownEvent } from "./extensions/runner.ts"; +import { resolveStoredModelReference } from "./model-resolver.ts"; import type { CreateAgentSessionResult } from "./sdk.ts"; import { assertSessionCwdExists } from "./session-cwd.ts"; import { SessionManager } from "./session-manager.ts"; @@ -246,15 +248,20 @@ export class AgentSessionRuntime { projectTrustContextFactory?: (cwd: string) => ProjectTrustContext; }, ): Promise<{ cancelled: boolean }> { + // Run all non-mutating preflight checks before firing session_before_switch: + // SessionManager.open, cwd existence, and the model-budget admission check are + // pure reads, so a rejected resume stays a true no-op and never lets handlers + // (e.g. a side-query abort or widget removal) mutate the live session first. + const sessionManager = SessionManager.open(sessionPath, undefined, options?.cwdOverride); + assertSessionCwdExists(sessionManager, this.cwd); + this.assertSessionAdmissible(sessionManager); + const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); if (beforeResult.cancelled) { return beforeResult; } const previousSessionFile = this.session.sessionFile; - const sessionManager = SessionManager.open(sessionPath, undefined, options?.cwdOverride); - assertSessionCwdExists(sessionManager, this.cwd); - await this.assertSessionAdmissible(sessionManager); await this.teardownCurrent("resume", sessionManager.getSessionFile()); await this.apply( await this.createRuntime({ @@ -275,16 +282,39 @@ export class AgentSessionRuntime { * target session's context without replacing the current session. Mirrors * the check createAgentSession performs after teardown; running it early * keeps a rejected resume from invalidating the live session. + * + * The check runs against the model the resume will actually restore, not the + * live session's model: createAgentSession restores existingSession.model from + * the destination file, so checking the active model would pass preflight when + * the active window is larger, tear down the live session, then re-throw from + * the post-teardown check - the exact destructive failure this guards against. */ - async assertSessionAdmissible(sessionManager: SessionManager): Promise { + assertSessionAdmissible(sessionManager: SessionManager): void { const existingSession = sessionManager.buildSessionContext(); if (existingSession.messages.length === 0) return; - const model = this.session.model; + const model = this.resolveResumeModel(existingSession.model); if (!model) return; const liveContextTokens = existingSession.messages.reduce((total, message) => total + estimateTokens(message), 0); this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" }); } + /** + * Resolve the model a resume would restore, mirroring createAgentSession's + * restore path in sdk.ts: prefer the destination session's stored model when it + * resolves and its provider is authorized, otherwise fall back to the live + * session's active model (the same fallback createAgentSession lands on). + */ + private resolveResumeModel(storedModel: { provider: string; modelId: string } | null): Model | undefined { + if (storedModel) { + const modelRuntime = this.session.modelRuntime; + const restored = resolveStoredModelReference(storedModel.provider, storedModel.modelId, modelRuntime); + if (restored && modelRuntime.hasConfiguredAuth(restored.model.provider)) { + return restored.model; + } + } + return this.session.model; + } + async newSession(options?: { parentSession?: string; setup?: (sessionManager: SessionManager) => Promise; diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 7b633f9f19..8dca8059f7 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,12 +1,13 @@ -## Resume admission runs before teardown so a rejected resume keeps the live session (2026-09-08) +## Resume admission runs before teardown and before the switch event, against the restored model (2026-09-08) ### What changed -- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` now calls a new `assertSessionAdmissible(sessionManager)` before `teardownCurrent("resume", ...)`. The method rebuilds the target session context, sums its live-context tokens with `estimateTokens` (imported from `./compaction/compaction.ts`), and runs `this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" })` against the current model - mirroring the post-teardown check `createAgentSession` performs in `sdk.ts`. When the target is empty or the session has no model it is a no-op. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` now opens the target `SessionManager`, runs `assertSessionCwdExists`, and calls the synchronous `assertSessionAdmissible(sessionManager)` *before* `emitBeforeSwitch("resume", ...)` and `teardownCurrent("resume", ...)`. All three are non-mutating reads, so a rejected resume is a true no-op that never fires the switch lifecycle event (no side-query abort, no widget removal) and never disposes the live session. +- `assertSessionAdmissible` sums the target's live-context tokens with `estimateTokens` (imported from `./compaction/compaction.ts`) and runs `this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" })`. The `model` is now the model the resume will actually restore, resolved by the new private `resolveResumeModel`: it mirrors `createAgentSession`'s restore path in `sdk.ts` - `resolveStoredModelReference(existingSession.model.provider, existingSession.model.modelId, this.session.modelRuntime)` when the destination carries a stored model whose provider is authorized (`modelRuntime.hasConfiguredAuth`), else the live session's active model. When the target is empty or no model resolves it is a no-op. ### Why -- Resuming a session whose restored transcript exceeds the current model's context budget threw `ModelUsabilityBudgetError` only from `createAgentSession`, which runs after `teardownCurrent` has already disposed the live session and invalidated its extension runner. The user's still-active session was destroyed by a resume that was always going to be rejected, and the next input crashed with "This extension ctx is stale after session replacement or reload". Running the same admission check before teardown makes a rejected resume a clean no-op that leaves the live session intact. +- Resuming a session whose restored transcript exceeds the *restored* model's context budget threw `ModelUsabilityBudgetError` only from `createAgentSession`, which runs after `teardownCurrent` has already disposed the live session and invalidated its extension runner. The user's still-active session was destroyed by a resume that was always going to be rejected, and the next input crashed with "This extension ctx is stale after session replacement or reload". Checking the *active* model was not enough: if the active model has a bigger window than the restored one, preflight passed, teardown destroyed the live session, and the post-teardown check re-threw - the exact destructive failure. Running the same admission check against the restored model, before the switch event and teardown, makes a rejected resume a clean no-op. ### Why an extension could not handle it @@ -14,7 +15,7 @@ ### Expected merge conflict zones -- LOW: the added `assertSessionAdmissible` call in `switchSession`, the new method placed after `switchSession`, and the `estimateTokens` import line. +- LOW: the reordered preflight block in `switchSession`, the `assertSessionAdmissible`/`resolveResumeModel` methods placed after `switchSession`, and the `estimateTokens` / `resolveStoredModelReference` / `Model, Api` import lines. ## Insufficient accepted compaction keeps its blocked state, #7921 case 6 (2026-09-07) diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 18c1a5e457..ad3d28a5f1 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -3,7 +3,7 @@ ### What changed -- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: `handleResumeSession`'s catch block now handles `ModelUsabilityBudgetError` (imported from `../../core/extensions/builtin/compaction/model-usability-budget.ts`) before the `handleFatalRuntimeError` fallback. It renders the message through `showError` and returns `{ cancelled: true }` instead of routing to `handleFatalRuntimeError`, which calls `process.exit(1)`. +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: `handleResumeSession` now handles `ModelUsabilityBudgetError` (imported from `../../core/extensions/builtin/compaction/model-usability-budget.ts`) on both switch attempts. A new `cancelResumeWithBudgetError` helper renders the message through `showError` and returns `{ cancelled: true }` instead of routing to `handleFatalRuntimeError` (which calls `process.exit(1)`). The `MissingSessionCwdError` cwd-override retry is now wrapped in its own `try/catch` so a budget rejection from the second `switchSession` gets the same recoverable treatment instead of escaping as an unhandled rejection. ### Why diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index d3273deb48..5b4e27a83a 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -7566,25 +7566,40 @@ export class InteractiveMode { this.showStatus("Resume cancelled"); return { cancelled: true }; } - const result = await this.runtimeHost.switchSession(sessionPath, { - cwdOverride: selectedCwd, - withSession: options?.withSession, - projectTrustContextFactory: (cwd) => this.createProjectTrustContext(cwd), - }); - if (result.cancelled) { + try { + const result = await this.runtimeHost.switchSession(sessionPath, { + cwdOverride: selectedCwd, + withSession: options?.withSession, + projectTrustContextFactory: (cwd) => this.createProjectTrustContext(cwd), + }); + if (result.cancelled) { + return result; + } + this.showStatus("Resumed session in current cwd"); return result; + } catch (overrideError: unknown) { + // The cwd-override retry can also be rejected by the model usability + // budget; give it the same recoverable treatment as the first attempt + // instead of letting it escape as an unhandled rejection. + if (overrideError instanceof ModelUsabilityBudgetError) { + return this.cancelResumeWithBudgetError(overrideError); + } + return this.handleFatalRuntimeError("Failed to resume session", overrideError); } - this.showStatus("Resumed session in current cwd"); - return result; } if (error instanceof ModelUsabilityBudgetError) { - this.showError(`Failed to resume session: ${error.message}`); - return { cancelled: true }; + return this.cancelResumeWithBudgetError(error); } return this.handleFatalRuntimeError("Failed to resume session", error); } } + /** Render an over-budget resume rejection and keep the live session running. */ + private cancelResumeWithBudgetError(error: ModelUsabilityBudgetError): { cancelled: boolean } { + this.showError(`Failed to resume session: ${error.message}`); + return { cancelled: true }; + } + private getLoginProviderOptions(authType?: "oauth" | "api_key"): AuthSelectorProvider[] { const options: AuthSelectorProvider[] = []; for (const provider of this.session.modelRuntime.getProviders()) { diff --git a/packages/coding-agent/src/modes/rpc/changes.md b/packages/coding-agent/src/modes/rpc/changes.md index 97423f5beb..72533b63cc 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -1,5 +1,24 @@ # changes +## Budget rejection keeps its typed identity across the RPC boundary (2026-09-08) + +### What changed + +- `connection-handler.ts`: the command error path now classifies `ModelUsabilityBudgetError` (imported from `../../core/extensions/builtin/compaction/model-usability-budget.ts`) alongside `MissingSessionCwdError`, emitting `errorCode: "model_usability_budget"` with the error's `projection` as `errorData` on the wire. +- `rpc-client.ts`: `getData` reconstructs `ModelUsabilityBudgetError` from that typed code + projection before falling back to a plain `Error`, mirroring the existing `missing_session_cwd` reconstruction. + +### Why + +- With `experimental.sharedHost`, `switchSession` runs on the host and its result crosses the RPC seam. `getData` rebuilt any non-missing-cwd failure as a plain `Error`, so the interactive resume handler's `instanceof ModelUsabilityBudgetError` check failed client-side and the TUI still routed an over-budget `/resume` to `process.exit(1)`. Classifying on a typed error code (never a message substring) preserves the identity so the client shows the error and keeps the live session, matching the in-process path. + +### Why an extension could not handle it + +- Error serialization/deserialization across the RPC transport is core host/client plumbing with no extension hook; only the connection handler and client can preserve a typed error's identity over the wire. + +### Expected merge conflict zones + +- LOW: the classification block in `connection-handler.ts`'s command catch, its new import, and the added branch + import in `rpc-client.ts`'s `getData`. + ## Watchdog reads the ownership token before it removes the scratch directory (2026-09-07) ### What changed diff --git a/packages/coding-agent/src/modes/rpc/connection-handler.ts b/packages/coding-agent/src/modes/rpc/connection-handler.ts index e308a3683c..56255e1576 100644 --- a/packages/coding-agent/src/modes/rpc/connection-handler.ts +++ b/packages/coding-agent/src/modes/rpc/connection-handler.ts @@ -35,6 +35,7 @@ import { subscribeProviderAccountEvents, } from "../../core/extensions/builtin/claude-sdk-oauth/account-events.ts"; import { CLAUDE_SDK_OAUTH_PROVIDER_ID } from "../../core/extensions/builtin/claude-sdk-oauth/account-management.ts"; +import { ModelUsabilityBudgetError } from "../../core/extensions/builtin/compaction/model-usability-budget.ts"; import { isMcpControlInventoryChanged, MCP_CONTROL_INVENTORY_CHANGED_EVENT, @@ -1669,13 +1670,24 @@ export function createRpcConnectionHandler( } catch (commandError: unknown) { const missingCwd = commandError instanceof Error && commandError.name === "MissingSessionCwdError" && "issue" in commandError; + // Preserve the budget-rejection identity across the RPC boundary: the client + // reconstructs ModelUsabilityBudgetError from this typed code + projection so + // its instanceof check (and the TUI's recoverable handling) still works, + // rather than seeing a plain Error and exiting. + const budgetError = commandError instanceof ModelUsabilityBudgetError ? commandError : undefined; + const errorCode = missingCwd ? "missing_session_cwd" : budgetError ? "model_usability_budget" : undefined; + const errorData = missingCwd + ? (commandError as { issue: unknown }).issue + : budgetError + ? budgetError.projection + : undefined; output( error( command.id, command.type, commandError instanceof Error ? commandError.message : String(commandError), - missingCwd ? "missing_session_cwd" : undefined, - missingCwd ? commandError.issue : undefined, + errorCode, + errorData, ), ); await waitForRpcBackpressure(); diff --git a/packages/coding-agent/src/modes/rpc/rpc-client.ts b/packages/coding-agent/src/modes/rpc/rpc-client.ts index 307b9d0a40..b2611237ef 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-client.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-client.ts @@ -11,6 +11,7 @@ import type { ImageContent } from "@earendil-works/pi-ai"; import type { PromptDisposition, SessionStats } from "../../core/agent-session.ts"; import type { BashResult } from "../../core/bash-executor.ts"; import type { CompactionResult } from "../../core/compaction/index.ts"; +import { ModelUsabilityBudgetError } from "../../core/extensions/builtin/compaction/model-usability-budget.ts"; import type { ServiceTier } from "../../core/extensions/builtin/service-tier.ts"; import { MissingSessionCwdError } from "../../core/session-cwd.ts"; import type { SessionEntry, SessionTreeNode } from "../../core/session-manager.ts"; @@ -1109,6 +1110,11 @@ export class RpcClient { errorResponse.errorData as ConstructorParameters[0], ); } + if (errorResponse.errorCode === "model_usability_budget" && errorResponse.errorData) { + throw new ModelUsabilityBudgetError( + errorResponse.errorData as ConstructorParameters[0], + ); + } throw new Error(errorResponse.error); } // Type assertion: we trust response.data matches T based on the command sent. diff --git a/packages/coding-agent/test/interactive-mode-resume-budget.test.ts b/packages/coding-agent/test/interactive-mode-resume-budget.test.ts new file mode 100644 index 0000000000..1736128f62 --- /dev/null +++ b/packages/coding-agent/test/interactive-mode-resume-budget.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from "vitest"; +import { ModelUsabilityBudgetError } from "../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { MissingSessionCwdError } from "../src/core/session-cwd.ts"; +import { InteractiveMode } from "../src/modes/interactive/interactive-mode.ts"; + +type HandleResumeSession = ( + this: ResumeContext, + sessionPath: string, + options?: unknown, +) => Promise<{ cancelled: boolean }>; + +interface ResumeContext { + clearStatusIndicator: () => void; + runtimeHost: { switchSession: (sessionPath: string, options?: unknown) => Promise<{ cancelled: boolean }> }; + showStatus: (message: string) => void; + showError: (message: string) => void; + handleFatalRuntimeError: (prefix: string, error: unknown) => Promise; + promptForMissingSessionCwd: (error: MissingSessionCwdError) => Promise; + createProjectTrustContext: (cwd: string) => unknown; + cancelResumeWithBudgetError: (error: ModelUsabilityBudgetError) => { cancelled: boolean }; +} + +function getHandleResumeSession(): HandleResumeSession { + const descriptor = Object.getOwnPropertyDescriptor(InteractiveMode.prototype, "handleResumeSession"); + if (!descriptor || typeof descriptor.value !== "function") { + throw new Error("InteractiveMode.handleResumeSession is not available"); + } + return descriptor.value as HandleResumeSession; +} + +function makeBudgetError(): ModelUsabilityBudgetError { + return new ModelUsabilityBudgetError({ + model: "faux/faux-small", + contextWindow: 8192, + liveContextTokens: 60000, + systemPromptTokens: 3748, + activeToolSchemaTokens: 4538, + outputReserveTokens: 2048, + compactionReserveTokens: 1024, + speculationLeadTokens: 0, + safetyMarginTokens: 8192, + safetyMarginProfile: "default", + requiredTokens: 79550, + shortfallTokens: 71358, + usable: false, + admission: "resume", + }); +} + +function makeContext(overrides: Partial): ResumeContext { + const cancelResumeWithBudgetError = Object.getOwnPropertyDescriptor( + InteractiveMode.prototype, + "cancelResumeWithBudgetError", + )?.value as (this: ResumeContext, error: ModelUsabilityBudgetError) => { cancelled: boolean }; + const base: ResumeContext = { + clearStatusIndicator: vi.fn(), + runtimeHost: { switchSession: vi.fn(async () => ({ cancelled: false })) }, + showStatus: vi.fn(), + showError: vi.fn(), + handleFatalRuntimeError: vi.fn(async () => { + throw new Error("handleFatalRuntimeError should not be reached"); + }) as unknown as (prefix: string, error: unknown) => Promise, + promptForMissingSessionCwd: vi.fn(async () => "/tmp/override-cwd"), + createProjectTrustContext: vi.fn(() => ({})), + cancelResumeWithBudgetError: vi.fn(function (this: ResumeContext, error: ModelUsabilityBudgetError) { + return cancelResumeWithBudgetError.call(this, error); + }), + ...overrides, + }; + return base; +} + +describe("InteractiveMode.handleResumeSession budget handling", () => { + it("cancels and shows the error when the first switch is over budget", async () => { + const handleResumeSession = getHandleResumeSession(); + const budgetError = makeBudgetError(); + const ctx = makeContext({ + runtimeHost: { switchSession: vi.fn(async () => Promise.reject(budgetError)) }, + }); + + const result = await handleResumeSession.call(ctx, "/tmp/session.jsonl"); + + expect(result).toEqual({ cancelled: true }); + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("Failed to resume session")); + expect(ctx.handleFatalRuntimeError).not.toHaveBeenCalled(); + }); + + it("cancels and shows the error when the cwd-override retry is over budget", async () => { + const handleResumeSession = getHandleResumeSession(); + const missingCwd = new MissingSessionCwdError({ + sessionFile: "/tmp/session.jsonl", + sessionCwd: "/tmp/gone", + fallbackCwd: "/tmp/here", + }); + const budgetError = makeBudgetError(); + const switchSession = vi + .fn<(sessionPath: string, options?: unknown) => Promise<{ cancelled: boolean }>>() + .mockRejectedValueOnce(missingCwd) + .mockRejectedValueOnce(budgetError); + const ctx = makeContext({ runtimeHost: { switchSession } }); + + const result = await handleResumeSession.call(ctx, "/tmp/session.jsonl"); + + expect(result).toEqual({ cancelled: true }); + expect(switchSession).toHaveBeenCalledTimes(2); + // The second call carried the chosen cwd override. + expect(switchSession.mock.calls[1]?.[1]).toMatchObject({ cwdOverride: "/tmp/override-cwd" }); + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("Failed to resume session")); + expect(ctx.handleFatalRuntimeError).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/coding-agent/test/rpc-client-budget-error.test.ts b/packages/coding-agent/test/rpc-client-budget-error.test.ts new file mode 100644 index 0000000000..6ba96c3e39 --- /dev/null +++ b/packages/coding-agent/test/rpc-client-budget-error.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { + ModelUsabilityBudgetError, + type ModelUsabilityBudgetProjection, +} from "../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { MissingSessionCwdError } from "../src/core/session-cwd.ts"; +import { RpcClient } from "../src/modes/rpc/rpc-client.ts"; + +type RpcClientPrivate = { + getData: (response: unknown) => T; +}; + +const projection: ModelUsabilityBudgetProjection = { + model: "faux/faux-small", + contextWindow: 8192, + liveContextTokens: 60000, + systemPromptTokens: 3748, + activeToolSchemaTokens: 4538, + outputReserveTokens: 2048, + compactionReserveTokens: 1024, + speculationLeadTokens: 0, + safetyMarginTokens: 8192, + safetyMarginProfile: "default", + requiredTokens: 79550, + shortfallTokens: 71358, + usable: false, + admission: "resume", +}; + +describe("RpcClient budget error identity", () => { + it("reconstructs ModelUsabilityBudgetError from a typed error response", () => { + const client = new RpcClient(); + const getData = (client as unknown as RpcClientPrivate).getData.bind(client); + + let thrown: unknown; + try { + getData({ + type: "response", + command: "switch_session", + success: false, + error: new ModelUsabilityBudgetError(projection).message, + errorCode: "model_usability_budget", + errorData: projection, + }); + } catch (error: unknown) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(ModelUsabilityBudgetError); + expect((thrown as ModelUsabilityBudgetError).projection).toEqual(projection); + expect(thrown).not.toBeInstanceOf(MissingSessionCwdError); + }); + + it("falls back to a plain Error when no typed code is present", () => { + const client = new RpcClient(); + const getData = (client as unknown as RpcClientPrivate).getData.bind(client); + + expect(() => + getData({ + type: "response", + command: "switch_session", + success: false, + error: "boom", + }), + ).toThrow(/boom/); + try { + getData({ type: "response", command: "switch_session", success: false, error: "boom" }); + } catch (error: unknown) { + expect(error).not.toBeInstanceOf(ModelUsabilityBudgetError); + } + }); +}); diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index 6dfae88480..c69f7c167a 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -11,6 +11,7 @@ import { createAgentSessionServices, } from "../../src/core/agent-session-runtime.ts"; import { AuthStorage } from "../../src/core/auth-storage.ts"; +import { estimateTokens } from "../../src/core/compaction/compaction.ts"; import { ModelUsabilityBudgetError } from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; import { SessionManager } from "../../src/core/session-manager.ts"; import type { @@ -50,6 +51,9 @@ describe("AgentSessionRuntime characterization", () => { models: [ { id: "faux-1", reasoning: true }, { id: "faux-2", reasoning: false }, + // A deliberately tiny context window so a transcript that fits the default + // 128000-token model is over budget once resumed against this model. + { id: "faux-small", reasoning: false, contextWindow: 8192, maxTokens: 2048 }, ], }); faux.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two"), fauxAssistantMessage("three")]); @@ -707,4 +711,79 @@ describe("AgentSessionRuntime characterization", () => { expect(runtime.session.extensionRunner.isActive).toBe(true); await expect(runtime.session.prompt("still here")).resolves.toBeUndefined(); }); + + // Regression: the admission check must run against the model the resume will + // actually restore (the destination session's stored model), not the live + // session's model. When the active model has a bigger window than the restored + // one, checking the active model passes preflight, tears down the live session, + // then re-throws from the post-teardown check - the destructive failure. + it("rejects a resume over the restored model's budget even when the active model would fit", async () => { + const emittedBeforeSwitch: RecordedSessionEvent[] = []; + const { runtime, faux } = await createRuntimeForTest((pi: ExtensionAPI) => { + pi.on("session_before_switch", (event) => { + emittedBeforeSwitch.push(event); + }); + }); + // Active session runs on the default 128000-token model. + await runtime.session.prompt("hello"); + const originalSession = runtime.session; + const originalSessionFile = runtime.session.sessionFile; + + // The destination session stores the tiny 8192-token model. Its transcript is + // ~60000 tokens: comfortably inside the active model's window, far past the + // restored model's. The user + assistant model entry make faux-small the + // restored model on resume. + const smallModel = faux.getModel("faux-small")!; + const targetDir = join( + tmpdir(), + `pi-runtime-restored-model-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(targetDir, { recursive: true }); + const targetSession = SessionManager.create(targetDir); + targetSession.appendMessage({ + role: "user", + content: [{ type: "text", text: "x".repeat(60_000) }], + timestamp: Date.now() - 1, + }); + targetSession.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "ok" }], + api: smallModel.api, + provider: smallModel.provider, + model: smallModel.id, + stopReason: "stop", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: Date.now(), + }); + const targetSessionFile = targetSession.getSessionFile(); + cleanups.push(() => rmSync(targetDir, { recursive: true, force: true })); + + // Confirm the transcript fits the active model: checking it would pass. + const activeTokens = SessionManager.open(targetSessionFile!) + .buildSessionContext() + .messages.reduce((total, message) => total + estimateTokens(message), 0); + expect(() => + originalSession.assertModelUsable(originalSession.model, activeTokens, { + includeSpeculationLead: false, + admission: "resume", + }), + ).not.toThrow(); + + await expect(runtime.switchSession(targetSessionFile!)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + + // Non-mutating preflight ran before the switch lifecycle event, so a rejected + // resume is a true no-op: no session_before_switch was ever emitted. + expect(emittedBeforeSwitch).toEqual([]); + expect(runtime.session).toBe(originalSession); + expect(runtime.session.sessionFile).toBe(originalSessionFile); + expect(runtime.session.extensionRunner.isActive).toBe(true); + await expect(runtime.session.prompt("still here")).resolves.toBeUndefined(); + }); }); From b5156dd649c1b1554f07e3f722a762991b9f4d41 Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Wed, 9 Sep 2026 06:50:52 +0800 Subject: [PATCH 03/11] fix(coding-agent): admit prepared resumes without disrupting live sessions --- packages/coding-agent/docs/rpc.md | 37 ++++ .../src/core/agent-session-runtime.ts | 85 +++------ .../coding-agent/src/core/agent-session.ts | 6 +- packages/coding-agent/src/core/changes.md | 21 ++- .../extensions/builtin/tool-search/changes.md | 21 +++ .../extensions/builtin/tool-search/index.ts | 7 +- .../extensions/builtin/tool-search/service.ts | 5 + packages/coding-agent/src/core/sdk.ts | 15 +- .../coding-agent/src/core/session-manager.ts | 56 +++++- .../test/suite/agent-session-runtime.test.ts | 178 +++++++++++++++++- .../interactive-mode-resume-budget.test.ts | 6 +- .../rpc-client-budget-error.test.ts | 6 +- .../test/tool-search/native-anthropic.test.ts | 2 +- 13 files changed, 347 insertions(+), 98 deletions(-) rename packages/coding-agent/test/{ => suite}/interactive-mode-resume-budget.test.ts (94%) rename packages/coding-agent/test/{ => suite}/rpc-client-budget-error.test.ts (89%) diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index c8f64357a7..39c7db777e 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -997,6 +997,43 @@ If an extension cancelled the switch: {"type": "response", "command": "switch_session", "success": true, "data": {"cancelled": true}} ``` +A rejected switch answers with `success: false` plus a typed `errorCode` and structured `errorData`: + +| `errorCode` | Meaning | `errorData` | +|---|---|---| +| `missing_session_cwd` | The session's stored cwd no longer exists. Retry the command with `cwdOverride`. | `{"sessionFile", "sessionCwd", "fallbackCwd"}` | +| `model_usability_budget` | The stored transcript does not fit the context budget of the model the switch would run on. | The full budget projection | + +Both rejections are decided before the switch lifecycle event and live-session teardown, so the current session stays usable. A cancelled or rejected switch does not write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. + +```json +{ + "type": "response", + "command": "switch_session", + "success": false, + "error": "Model anthropic/claude-opus-4-5 cannot host this session ...", + "errorCode": "model_usability_budget", + "errorData": { + "model": "anthropic/claude-opus-4-5", + "contextWindow": 200000, + "liveContextTokens": 260000, + "systemPromptTokens": 3748, + "activeToolSchemaTokens": 4538, + "outputReserveTokens": 32000, + "compactionReserveTokens": 1024, + "speculationLeadTokens": 0, + "safetyMarginTokens": 16384, + "safetyMarginProfile": "anthropic", + "requiredTokens": 317694, + "shortfallTokens": 117694, + "usable": false, + "admission": "resume" + } +} +``` + +For a non-empty `switch_session` target, `admission` is `resume` and `speculationLeadTokens` is zero. The shared projection also supports `start` (including empty targets) and `switch` (model changes). `requiredTokens` is the sum of `liveContextTokens`, `systemPromptTokens`, `activeToolSchemaTokens`, `outputReserveTokens`, `compactionReserveTokens`, `speculationLeadTokens`, and `safetyMarginTokens`. `shortfallTokens = max(0, requiredTokens - contextWindow)`; `usable` is true exactly when that shortfall is zero. `model` and `safetyMarginProfile` identify the selection and margin policy; they are not token components. Clients should branch on `errorCode`, not parse the human-readable `error` text. + #### fork Create a new fork from a previous user message on the active branch. Can be cancelled by a `session_before_fork` extension event handler. Returns the text of the message being forked from. diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 811b4c760d..4e3e6cccdd 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -1,10 +1,8 @@ import { copyFileSync, existsSync, mkdirSync } from "node:fs"; import { basename, join, resolve } from "node:path"; -import type { Api, Model } from "@earendil-works/pi-ai"; import { resolvePath } from "../utils/paths.ts"; import type { AgentSession } from "./agent-session.ts"; import type { AgentSessionRuntimeDiagnostic, AgentSessionServices } from "./agent-session-services.ts"; -import { estimateTokens } from "./compaction/compaction.ts"; import type { ProjectTrustContext, ReplacedSessionContext, @@ -12,7 +10,6 @@ import type { SessionStartEvent, } from "./extensions/index.ts"; import { type ExtensionRunner, emitSessionShutdownEvent } from "./extensions/runner.ts"; -import { resolveStoredModelReference } from "./model-resolver.ts"; import type { CreateAgentSessionResult } from "./sdk.ts"; import { assertSessionCwdExists } from "./session-cwd.ts"; import { SessionManager } from "./session-manager.ts"; @@ -248,73 +245,33 @@ export class AgentSessionRuntime { projectTrustContextFactory?: (cwd: string) => ProjectTrustContext; }, ): Promise<{ cancelled: boolean }> { - // Run all non-mutating preflight checks before firing session_before_switch: - // SessionManager.open, cwd existence, and the model-budget admission check are - // pure reads, so a rejected resume stays a true no-op and never lets handlers - // (e.g. a side-query abort or widget removal) mutate the live session first. - const sessionManager = SessionManager.open(sessionPath, undefined, options?.cwdOverride); + const previousSessionFile = this.session.sessionFile; + const prepared = SessionManager.prepareOpen(sessionPath, undefined, options?.cwdOverride); + const { sessionManager } = prepared; assertSessionCwdExists(sessionManager, this.cwd); - this.assertSessionAdmissible(sessionManager); - - const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); - if (beforeResult.cancelled) { - return beforeResult; + // Build and admit the actual destination, including its model selection, + // settings, prompt and tools. Persistence and switch lifecycle stay deferred. + const result = await this.createRuntime({ + cwd: sessionManager.getCwd(), + agentDir: this.services.agentDir, + sessionManager, + sessionStartEvent: { type: "session_start", reason: "resume", previousSessionFile }, + projectTrustContext: options?.projectTrustContextFactory?.(sessionManager.getCwd()), + launchProfile: this._launchProfile, + }); + try { + const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); + if (beforeResult.cancelled) return beforeResult; + prepared.commit(); + await this.teardownCurrent("resume", sessionManager.getSessionFile()); + await this.apply(result); + } finally { + if (this.session !== result.session) result.session.dispose({ releaseProviderResources: false }); } - - const previousSessionFile = this.session.sessionFile; - await this.teardownCurrent("resume", sessionManager.getSessionFile()); - await this.apply( - await this.createRuntime({ - cwd: sessionManager.getCwd(), - agentDir: this.services.agentDir, - sessionManager, - sessionStartEvent: { type: "session_start", reason: "resume", previousSessionFile }, - projectTrustContext: options?.projectTrustContextFactory?.(sessionManager.getCwd()), - launchProfile: this._launchProfile, - }), - ); await this.finishSessionReplacement(options?.withSession); return { cancelled: false }; } - /** - * Re-run the resume admission check (model usability budget) against the - * target session's context without replacing the current session. Mirrors - * the check createAgentSession performs after teardown; running it early - * keeps a rejected resume from invalidating the live session. - * - * The check runs against the model the resume will actually restore, not the - * live session's model: createAgentSession restores existingSession.model from - * the destination file, so checking the active model would pass preflight when - * the active window is larger, tear down the live session, then re-throw from - * the post-teardown check - the exact destructive failure this guards against. - */ - assertSessionAdmissible(sessionManager: SessionManager): void { - const existingSession = sessionManager.buildSessionContext(); - if (existingSession.messages.length === 0) return; - const model = this.resolveResumeModel(existingSession.model); - if (!model) return; - const liveContextTokens = existingSession.messages.reduce((total, message) => total + estimateTokens(message), 0); - this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" }); - } - - /** - * Resolve the model a resume would restore, mirroring createAgentSession's - * restore path in sdk.ts: prefer the destination session's stored model when it - * resolves and its provider is authorized, otherwise fall back to the live - * session's active model (the same fallback createAgentSession lands on). - */ - private resolveResumeModel(storedModel: { provider: string; modelId: string } | null): Model | undefined { - if (storedModel) { - const modelRuntime = this.session.modelRuntime; - const restored = resolveStoredModelReference(storedModel.provider, storedModel.modelId, modelRuntime); - if (restored && modelRuntime.hasConfiguredAuth(restored.model.provider)) { - return restored.model; - } - } - return this.session.model; - } - async newSession(options?: { parentSession?: string; setup?: (sessionManager: SessionManager) => Promise; diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index c0f236b2ed..4bacbd79a1 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -2910,8 +2910,10 @@ export class AgentSession { /** * Remove all listeners and disconnect from agent. * Call this when completely done with the session. + * Unstarted resume candidates do not own provider resources keyed by the + * persisted session ID, which may still belong to a live runtime. */ - dispose(): void { + dispose(options?: { releaseProviderResources?: boolean }): void { try { this._probeBackScheduler.cancel("dispose"); this.abortRetry(); @@ -2933,7 +2935,7 @@ export class AgentSession { this._unsubscribeWakeSources?.(); this._unsubscribeWakeSources = undefined; this._eventListeners = []; - cleanupSessionResources(this.sessionId); + if (options?.releaseProviderResources !== false) cleanupSessionResources(this.sessionId); } /** Live in-session activity signals; see `session-activity.ts` for the contract. */ diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 7247073fcb..be29ae603d 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,21 +1,30 @@ -## Resume admission runs before teardown and before the switch event, against the restored model (2026-09-08) +## Resume admission uses the prepared destination runtime (2026-09-08) ### What changed -- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` now opens the target `SessionManager`, runs `assertSessionCwdExists`, and calls the synchronous `assertSessionAdmissible(sessionManager)` *before* `emitBeforeSwitch("resume", ...)` and `teardownCurrent("resume", ...)`. All three are non-mutating reads, so a rejected resume is a true no-op that never fires the switch lifecycle event (no side-query abort, no widget removal) and never disposes the live session. -- `assertSessionAdmissible` sums the target's live-context tokens with `estimateTokens` (imported from `./compaction/compaction.ts`) and runs `this.session.assertModelUsable(model, liveContextTokens, { includeSpeculationLead: false, admission: "resume" })`. The `model` is now the model the resume will actually restore, resolved by the new private `resolveResumeModel`: it mirrors `createAgentSession`'s restore path in `sdk.ts` - `resolveStoredModelReference(existingSession.model.provider, existingSession.model.modelId, this.session.modelRuntime)` when the destination carries a stored model whose provider is authorized (`modelRuntime.hasConfiguredAuth`), else the live session's active model. When the target is empty or no model resolves it is a no-op. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` prepares the actual factory result before `session_before_switch` and teardown, then applies that same result after cancellation succeeds. It no longer approximates SDK model selection with the live runtime, settings, prompt or tools. Cancelled prepared sessions are disposed without starting them. +- `packages/coding-agent/src/core/session-manager.ts`: `prepareOpen` defers newline repair, migration/empty-file rewrites and initialization appends until an admitted switch is accepted. Normal `open` behavior is unchanged; an ordinary accepted resume appends pending entries without rewriting existing transcript bytes. +- `packages/coding-agent/src/core/sdk.ts`: dispose the newly constructed destination if its authoritative model-budget admission throws, releasing its subscriptions rather than leaking a rejected session. +- `packages/coding-agent/src/core/agent-session.ts`: unstarted candidates can dispose subscriptions without releasing provider resources by persisted session ID; those resources may still belong to a live runtime of the same session. ### Why -- Resuming a session whose restored transcript exceeds the *restored* model's context budget threw `ModelUsabilityBudgetError` only from `createAgentSession`, which runs after `teardownCurrent` has already disposed the live session and invalidated its extension runner. The user's still-active session was destroyed by a resume that was always going to be rejected, and the next input crashed with "This extension ctx is stale after session replacement or reload". Checking the *active* model was not enough: if the active model has a bigger window than the restored one, preflight passed, teardown destroyed the live session, and the post-teardown check re-threw - the exact destructive failure. Running the same admission check against the restored model, before the switch event and teardown, makes a rejected resume a clean no-op. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: CLI/launch-profile model choices outrank stored models, and unavailable stored models fall back through destination settings. Admission must also use destination tool schemas, system prompt and compaction settings. Rejection must not emit the switch event, abort side work or invalidate the live session. +- `packages/coding-agent/src/core/session-manager.ts`: opening a target was not a read-only operation; cancelled resumes could repair or rewrite it. Deferred persistence preserves byte identity on cancellation and rejection. +- `packages/coding-agent/src/core/sdk.ts`: moving construction before teardown requires cleaning up rejected construction without disposing the outgoing runtime. +- `packages/coding-agent/src/core/agent-session.ts`: cancelling a resume of the current session must not close the live session's provider connections. ### Why an extension could not handle it -- The teardown/create ordering lives inside the core `AgentSessionRuntime.switchSession` state machine. No extension hook fires between `teardownCurrent` (which disposes the session and invalidates the runner) and `createRuntime`, so no extension can intercept the rejection before the live session is torn down. +- `packages/coding-agent/src/core/agent-session-runtime.ts`, `packages/coding-agent/src/core/session-manager.ts`, `packages/coding-agent/src/core/sdk.ts`, and `packages/coding-agent/src/core/agent-session.ts`: destination construction, budget admission, persistence and provider-resource ownership precede extension lifecycle startup and are owned by the core replacement state machine. ### Expected merge conflict zones -- LOW: the reordered preflight block in `switchSession`, the `assertSessionAdmissible`/`resolveResumeModel` methods placed after `switchSession`, and the `estimateTokens` / `resolveStoredModelReference` / `Model, Api` import lines. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` ordering and removal of the duplicate admission/model resolver. +- `packages/coding-agent/src/core/session-manager.ts`: constructor, persistence guards and `open`/`prepareOpen`. +- `packages/coding-agent/src/core/sdk.ts`: authoritative post-construction budget check. +- `packages/coding-agent/src/core/agent-session.ts`: optional provider-resource release in `dispose`. + ## Same-model recovery for a native tool-search 400 (2026-09-08) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md index 55ea25e10c..6d9bd377a3 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md @@ -1,5 +1,26 @@ # Tool Search Builtin Changes +## 2026-09-08 - Keep rejected resume candidates out of the live catalog (PR #1473) + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts` creates a separate service for each local extension generation and publishes it only at `session_start`. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts` provides the local installation seam; provider-scoped installation remains unchanged. + +### Why + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts` previously rebound the global service during destination loading. Rejecting and disposing that candidate left the live session's context and provider-request hooks pointing at a stale extension API. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts` must keep the accepted catalog available to MCP and core consumers while another candidate is prepared. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts` and `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts` own this builtin's factory-time shared state; another extension cannot undo captured runtime bindings reliably. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: default factory and service import. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts`: local/scoped service ownership helpers. + ## 2026-09-08 - Wire the native 400 fallback into a session recovery signal (senpi #1481/#1482) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts index e42254216e..3f125f346c 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts @@ -1,7 +1,7 @@ import { bindToProviderScope } from "@earendil-works/pi-ai/node/provider-scope"; import type { ExtensionAPI, ExtensionFactory } from "../../types.ts"; import { AnthropicNativeToolSearchAdapter, isMcpNativeToolSearchEnabled } from "./native-search.ts"; -import { getToolSearchService, installScopedToolSearchService, ToolSearchService } from "./service.ts"; +import { installLocalToolSearchService, installScopedToolSearchService, ToolSearchService } from "./service.ts"; import { createToolSearchTool, TOOL_SEARCH_TOOL_NAME } from "./tool.ts"; export function createToolSearchExtension(service: ToolSearchService): ExtensionFactory { @@ -70,8 +70,11 @@ export default function toolSearchExtension(pi: ExtensionAPI): void | Promise pi.setActiveTools([...names]), }; const sessionOwned = hasProviderScope(); - const service = sessionOwned ? new ToolSearchService(runtime) : getToolSearchService(runtime); + // A prepared resume is not the active local runtime yet. Its callbacks must + // never rebind the live catalog to an extension generation that may be rejected. + const service = new ToolSearchService(runtime); if (sessionOwned) installScopedToolSearchService(service); + else pi.on("session_start", () => installLocalToolSearchService(service)); return createToolSearchExtension(service)(pi); } diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts index e05a45a6d4..a3a38a8c1d 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts @@ -221,6 +221,11 @@ function isValidDocument(doc: ToolSearchDocument, source: ToolSearchSource): boo const scopedService = new AsyncLocalStorage(); let service: ToolSearchService | null = null; +/** Publish the local runtime's catalog only once its session has been accepted. */ +export function installLocalToolSearchService(value: ToolSearchService): void { + service = value; +} + /** Make a session-owned service visible to later builtins loaded in the same provider scope. */ export function installScopedToolSearchService(value: ToolSearchService): void { scopedService.enterWith(value); diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index ae54c5dc0f..1e1bb2ff49 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -541,11 +541,16 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const liveContextTokens = hasExistingSession ? existingSession.messages.reduce((total, message) => total + estimateTokens(message), 0) : 0; - session.assertModelUsable( - undefined, - liveContextTokens, - hasExistingSession ? { includeSpeculationLead: false, admission: "resume" } : { admission: "start" }, - ); + try { + session.assertModelUsable( + undefined, + liveContextTokens, + hasExistingSession ? { includeSpeculationLead: false, admission: "resume" } : { admission: "start" }, + ); + } catch (error) { + session.dispose({ releaseProviderResources: false }); + throw error; + } sessionRef.current = session; const extensionsResult = resourceLoader.getExtensions(); diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index e769ec5724..bbf106631b 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -828,6 +828,7 @@ export class SessionManager { private sessionDir: string; private cwd: string; private persist: boolean; + private deferredPersistence?: { rewrite: boolean; entries: SessionEntry[] }; private flushed: boolean = false; private fileEntries: FileEntry[] = []; private byId: Map = new Map(); @@ -869,11 +870,13 @@ export class SessionManager { persist: boolean, newSessionOptions?: NewSessionOptions, preloadedFileEntries?: FileEntry[], + deferredPersistence = false, ) { this.cwd = resolvePath(cwd); this.sessionDir = normalizePath(sessionDir); this.persist = persist; - if (persist && this.sessionDir && !existsSync(this.sessionDir)) { + this.deferredPersistence = deferredPersistence ? { rewrite: false, entries: [] } : undefined; + if (persist && !deferredPersistence && this.sessionDir && !existsSync(this.sessionDir)) { mkdirSync(this.sessionDir, { recursive: true }); } @@ -1027,6 +1030,10 @@ export class SessionManager { private _rewriteFile(): void { if (!this.persist || !this.sessionFile) return; + if (this.deferredPersistence) { + this.deferredPersistence.rewrite = true; + return; + } const fd = openSync(this.sessionFile, "w"); try { for (const entry of this.fileEntries) { @@ -1067,6 +1074,10 @@ export class SessionManager { _persist(entry: SessionEntry): void { if (!this.persist || !this.sessionFile) return; + if (this.deferredPersistence) { + this.deferredPersistence.entries.push(entry); + return; + } const persistedEntry = this.residentStore.materialize(entry); const hasAssistant = this.fileEntries.some((e) => e.type === "message" && e.message.role === "assistant"); @@ -1843,6 +1854,45 @@ export class SessionManager { * @param cwdOverride Optional cwd override instead of the session header cwd. */ static open(path: string, sessionDir?: string, cwdOverride?: string): SessionManager { + return SessionManager._open(path, sessionDir, cwdOverride, false); + } + + /** Prepare a resume without repairing, migrating or appending to its file until admitted. */ + static prepareOpen( + path: string, + sessionDir?: string, + cwdOverride?: string, + ): { + sessionManager: SessionManager; + commit: () => void; + } { + const sessionManager = SessionManager._open(path, sessionDir, cwdOverride, true); + const pending = sessionManager.deferredPersistence; + return { + sessionManager, + commit: () => { + sessionManager.deferredPersistence = undefined; + mkdirSync(sessionManager.sessionDir, { recursive: true }); + if (pending?.rewrite) { + sessionManager._rewriteFile(); + } else { + const file = sessionManager.getSessionFile(); + if (file && existsSync(file)) { + const content = readFileSync(file); + if (content.length > 0 && content[content.length - 1] !== 10) appendFileSync(file, "\n"); + } + for (const entry of pending?.entries ?? []) sessionManager._persist(entry); + } + }, + }; + } + + private static _open( + path: string, + sessionDir: string | undefined, + cwdOverride: string | undefined, + deferredPersistence: boolean, + ): SessionManager { const resolvedPath = resolvePath(path); let header: SessionHeader | null = null; let preloadedFileEntries: FileEntry[] | undefined; @@ -1860,14 +1910,14 @@ export class SessionManager { } // This process opens the session for append; normalize a final unterminated // JSONL entry here, never from read-only loadEntriesFromFile callers. - if (existsSync(resolvedPath)) { + if (!deferredPersistence && existsSync(resolvedPath)) { const content = readFileSync(resolvedPath); if (content.length > 0 && content[content.length - 1] !== 10) appendFileSync(resolvedPath, "\n"); } const cwd = cwdOverride ?? (header ? getSessionHeaderCwd(header) : undefined) ?? process.cwd(); // If no sessionDir provided, derive from file's parent directory const dir = sessionDir ? normalizePath(sessionDir) : resolve(resolvedPath, ".."); - return new SessionManager(cwd, dir, resolvedPath, true, undefined, preloadedFileEntries); + return new SessionManager(cwd, dir, resolvedPath, true, undefined, preloadedFileEntries, deferredPersistence); } /** diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index c69f7c167a..cf5a61be48 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -1,7 +1,12 @@ -import { existsSync, mkdirSync, realpathSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, parse } from "node:path"; -import { fauxAssistantMessage, fauxToolCall, registerFauxProvider } from "@earendil-works/pi-ai/compat"; +import { + fauxAssistantMessage, + fauxToolCall, + registerFauxProvider, + registerSessionResourceCleanup, +} from "@earendil-works/pi-ai/compat"; import { Type } from "typebox"; import { afterEach, describe, expect, it } from "vitest"; import { @@ -41,7 +46,16 @@ describe("AgentSessionRuntime characterization", () => { async function createRuntimeForTest( extensionFactory: ExtensionFactory, - options?: { cwd?: string; bootstrapModel?: boolean; bootstrapThinkingLevel?: boolean }, + options?: { + cwd?: string; + bootstrapModel?: boolean; + bootstrapModelId?: string; + bootstrapThinkingLevel?: boolean; + destinationDefaultModel?: string; + destinationReserveTokens?: number; + destinationSystemPrompt?: string; + destinationToolDescription?: string; + }, ) { const tempDir = options?.cwd ?? join(tmpdir(), `pi-runtime-suite-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -51,6 +65,7 @@ describe("AgentSessionRuntime characterization", () => { models: [ { id: "faux-1", reasoning: true }, { id: "faux-2", reasoning: false }, + { id: "faux-medium", reasoning: false, contextWindow: 65536, maxTokens: 2048 }, // A deliberately tiny context window so a transcript that fits the default // 128000-token model is over budget once resumed against this model. { id: "faux-small", reasoning: false, contextWindow: 8192, maxTokens: 2048 }, @@ -64,7 +79,7 @@ describe("AgentSessionRuntime characterization", () => { const runtimeOptions = { agentDir: tempDir, authStorage, - model: options?.bootstrapModel === false ? undefined : faux.getModel(), + model: options?.bootstrapModel === false ? undefined : faux.getModel(options?.bootstrapModelId ?? "faux-1"), thinkingLevel: options?.bootstrapThinkingLevel === false ? undefined : undefined, resourceLoaderOptions: { extensionFactories: [ @@ -96,7 +111,21 @@ describe("AgentSessionRuntime characterization", () => { const services = await createAgentSessionServices({ ...runtimeOptions, cwd, + resourceLoaderOptions: { + ...runtimeOptions.resourceLoaderOptions, + systemPrompt: sessionStartEvent?.reason === "resume" ? options?.destinationSystemPrompt : undefined, + }, }); + if (sessionStartEvent?.reason === "resume") { + services.settingsManager.applyOverrides({ + ...(options?.destinationDefaultModel + ? { defaultProvider: faux.getModel().provider, defaultModel: options.destinationDefaultModel } + : {}), + ...(options?.destinationReserveTokens + ? { compaction: { reserveTokens: options.destinationReserveTokens } } + : {}), + }); + } return { ...(await createAgentSessionFromServices({ services, @@ -104,6 +133,18 @@ describe("AgentSessionRuntime characterization", () => { sessionStartEvent, model: runtimeOptions.model, thinkingLevel: runtimeOptions.thinkingLevel, + customTools: + sessionStartEvent?.reason === "resume" && options?.destinationToolDescription + ? [ + { + name: "destination_tool", + label: "Destination tool", + description: options.destinationToolDescription, + parameters: Type.Object({}), + execute: async () => ({ content: [], details: {} }), + }, + ] + : undefined, })), services, diagnostics: services.diagnostics, @@ -658,6 +699,116 @@ describe("AgentSessionRuntime characterization", () => { expect(runtime.session.thinkingLevel).toBe("off"); }); + // PR #1473: explicit factory choices take precedence over stored models. + it.each([ + { forced: "faux-medium", stored: "faux-1", rejected: true }, + { forced: "faux-1", stored: "faux-medium", rejected: false }, + ])("admits the factory's $forced model rather than stored $stored", async ({ forced, stored, rejected }) => { + const events: RecordedSessionEvent[] = []; + const { runtime, faux, tempDir } = await createRuntimeForTest( + (pi) => { + pi.on("session_before_switch", (event) => { + events.push(event); + }); + pi.on("session_shutdown", (event) => { + events.push(event); + }); + }, + { bootstrapModelId: forced }, + ); + const target = SessionManager.create(tempDir, join(tempDir, "targets")); + target.appendModelChange(faux.getModel().provider, stored); + target.appendMessage({ role: "user", content: "x".repeat(60_000), timestamp: 1 }); + target.appendMessage({ ...fauxAssistantMessage("stored"), provider: faux.getModel().provider, model: stored }); + const path = target.getSessionFile(); + if (!path) throw new Error("missing target file"); + const bytes = readFileSync(path); + const original = runtime.session; + if (rejected) { + await expect(runtime.switchSession(path)).rejects.toMatchObject({ + projection: { model: `${faux.getModel().provider}/${forced}`, admission: "resume" }, + }); + expect(events).toEqual([]); + expect(readFileSync(path)).toEqual(bytes); + expect(runtime.session).toBe(original); + await expect(original.prompt("still usable")).resolves.toBeUndefined(); + } else { + expect(await runtime.switchSession(path)).toEqual({ cancelled: false }); + expect(runtime.session.model?.id).toBe(forced); + expect(SessionManager.open(path).buildSessionContext().messages).toEqual(runtime.session.messages); + } + }); + + // PR #1473: destination services, not the live session, determine the whole budget. + it.each([ + { name: "stored-model fallback", bootstrapModel: false, destinationDefaultModel: "faux-medium" }, + { name: "compaction settings", destinationReserveTokens: 100_000 }, + { name: "system prompt", destinationSystemPrompt: "p".repeat(400_000) }, + { name: "tool schemas", destinationToolDescription: "t".repeat(400_000) }, + ])("rejects using destination $name before lifecycle effects", async (options) => { + const events: RecordedSessionEvent[] = []; + const { runtime, tempDir } = await createRuntimeForTest((pi) => { + pi.on("session_before_switch", (event) => { + events.push(event); + }); + }, options); + const target = SessionManager.create(tempDir, join(tempDir, "targets")); + target.appendModelChange("missing-provider", "missing-model"); + target.appendMessage({ role: "user", content: "x".repeat(60_000), timestamp: 1 }); + target.appendMessage({ ...fauxAssistantMessage("stored"), provider: "missing-provider", model: "missing-model" }); + const path = target.getSessionFile(); + if (!path) throw new Error("missing target file"); + const original = runtime.session; + const tokens = target.buildSessionContext().messages.reduce((sum, message) => sum + estimateTokens(message), 0); + expect(() => + original.assertModelUsable(original.model, tokens, { includeSpeculationLead: false, admission: "resume" }), + ).not.toThrow(); + await expect(runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + expect(events).toEqual([]); + expect(runtime.session).toBe(original); + await expect(original.prompt("still usable")).resolves.toBeUndefined(); + }); + + // PR #1473: cancellation must not repair, initialize, or migrate the target. + it("keeps live provider resources when a resume of the same session is cancelled", async () => { + const { runtime } = await createRuntimeForTest((pi) => { + pi.on("session_before_switch", () => ({ cancel: true })); + }); + await runtime.session.prompt("persist source"); + const path = runtime.session.sessionFile; + if (!path) throw new Error("missing session file"); + const released: Array = []; + const unregister = registerSessionResourceCleanup((id) => released.push(id)); + try { + expect(await runtime.switchSession(path)).toEqual({ cancelled: true }); + expect(released).toEqual([]); + } finally { + unregister(); + } + }); + + // PR #1473: cancellation must not repair, initialize, or migrate the target. + it.each(["unterminated", "legacy", "empty"])("leaves a cancelled %s target byte-identical", async (kind) => { + const { runtime, tempDir } = await createRuntimeForTest((pi) => { + pi.on("session_before_switch", () => ({ cancel: true })); + }); + const target = join(tempDir, "cancelled.jsonl"); + const header = { + type: "session", + version: kind === "legacy" ? 1 : 3, + id: "cancelled", + timestamp: "2026-09-08T00:00:00.000Z", + cwd: tempDir, + }; + const bytes = kind === "empty" ? "" : JSON.stringify(header); + writeFileSync(target, bytes); + const original = runtime.session; + expect(await runtime.switchSession(target)).toEqual({ cancelled: true }); + expect(readFileSync(target, "utf8")).toBe(bytes); + expect(runtime.session).toBe(original); + expect(original.extensionRunner.isActive).toBe(true); + }); + // Regression: a resume rejected by the model usability budget must run its // admission check BEFORE teardown, so the live session the user is still in is // never disposed/invalidated by a resume that will fail anyway. @@ -700,6 +851,9 @@ describe("AgentSessionRuntime characterization", () => { const targetSessionFile = targetSession.getSessionFile(); cleanups.push(() => rmSync(targetDir, { recursive: true, force: true })); + // PR #1473: a successful faux reply must not hide stale builtin callbacks. + const extensionErrors: unknown[] = []; + runtime.session.extensionRunner.onError((error) => extensionErrors.push(error)); await expect(runtime.switchSession(targetSessionFile!)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); // The live session object and its file are unchanged... @@ -710,6 +864,7 @@ describe("AgentSessionRuntime characterization", () => { // input crashed with the stale-context error). expect(runtime.session.extensionRunner.isActive).toBe(true); await expect(runtime.session.prompt("still here")).resolves.toBeUndefined(); + expect(extensionErrors).toEqual([]); }); // Regression: the admission check must run against the model the resume will @@ -719,11 +874,16 @@ describe("AgentSessionRuntime characterization", () => { // then re-throws from the post-teardown check - the destructive failure. it("rejects a resume over the restored model's budget even when the active model would fit", async () => { const emittedBeforeSwitch: RecordedSessionEvent[] = []; - const { runtime, faux } = await createRuntimeForTest((pi: ExtensionAPI) => { - pi.on("session_before_switch", (event) => { - emittedBeforeSwitch.push(event); - }); - }); + // No explicit factory model, so the destination's stored model is what the + // resume restores - the case this admission check has to judge. + const { runtime, faux } = await createRuntimeForTest( + (pi: ExtensionAPI) => { + pi.on("session_before_switch", (event) => { + emittedBeforeSwitch.push(event); + }); + }, + { bootstrapModel: false }, + ); // Active session runs on the default 128000-token model. await runtime.session.prompt("hello"); const originalSession = runtime.session; diff --git a/packages/coding-agent/test/interactive-mode-resume-budget.test.ts b/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts similarity index 94% rename from packages/coding-agent/test/interactive-mode-resume-budget.test.ts rename to packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts index 1736128f62..2ed70ab69d 100644 --- a/packages/coding-agent/test/interactive-mode-resume-budget.test.ts +++ b/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from "vitest"; -import { ModelUsabilityBudgetError } from "../src/core/extensions/builtin/compaction/model-usability-budget.ts"; -import { MissingSessionCwdError } from "../src/core/session-cwd.ts"; -import { InteractiveMode } from "../src/modes/interactive/interactive-mode.ts"; +import { ModelUsabilityBudgetError } from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { MissingSessionCwdError } from "../../src/core/session-cwd.ts"; +import { InteractiveMode } from "../../src/modes/interactive/interactive-mode.ts"; type HandleResumeSession = ( this: ResumeContext, diff --git a/packages/coding-agent/test/rpc-client-budget-error.test.ts b/packages/coding-agent/test/suite/rpc-client-budget-error.test.ts similarity index 89% rename from packages/coding-agent/test/rpc-client-budget-error.test.ts rename to packages/coding-agent/test/suite/rpc-client-budget-error.test.ts index 6ba96c3e39..65994c67ad 100644 --- a/packages/coding-agent/test/rpc-client-budget-error.test.ts +++ b/packages/coding-agent/test/suite/rpc-client-budget-error.test.ts @@ -2,9 +2,9 @@ import { describe, expect, it } from "vitest"; import { ModelUsabilityBudgetError, type ModelUsabilityBudgetProjection, -} from "../src/core/extensions/builtin/compaction/model-usability-budget.ts"; -import { MissingSessionCwdError } from "../src/core/session-cwd.ts"; -import { RpcClient } from "../src/modes/rpc/rpc-client.ts"; +} from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { MissingSessionCwdError } from "../../src/core/session-cwd.ts"; +import { RpcClient } from "../../src/modes/rpc/rpc-client.ts"; type RpcClientPrivate = { getData: (response: unknown) => T; diff --git a/packages/coding-agent/test/tool-search/native-anthropic.test.ts b/packages/coding-agent/test/tool-search/native-anthropic.test.ts index 8b8e006f73..9846b2ee6c 100644 --- a/packages/coding-agent/test/tool-search/native-anthropic.test.ts +++ b/packages/coding-agent/test/tool-search/native-anthropic.test.ts @@ -414,6 +414,7 @@ describe("native 400 pending recovery signal", () => { ], }); try { + await harness.getExtensionRunner().emit({ type: "session_start", reason: "startup" }); const service = getToolSearchService(); service.feed( "mcp", @@ -432,7 +433,6 @@ describe("native 400 pending recovery signal", () => { ], { activate: () => {} }, ); - await harness.getExtensionRunner().emit({ type: "session_start", reason: "startup" }); const payload = { model: "claude-fable-5-1", tools: [] as unknown[], messages: [] }; const injected = await harness.getExtensionRunner().emitBeforeProviderRequest(payload, undefined, { model: getModel("anthropic", "claude-fable-5-1"), From 4f7b567437cfe8d1b0402027cf8d66e08608db43 Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Wed, 9 Sep 2026 13:20:48 +0800 Subject: [PATCH 04/11] fix(sessions): isolate discarded resume cleanup --- .../src/core/agent-session-runtime.ts | 9 ++- .../coding-agent/src/core/agent-session.ts | 11 +-- packages/coding-agent/src/core/changes.md | 29 ++++++- .../core/extensions/builtin/mcp/changes.md | 6 +- .../src/core/extensions/builtin/mcp/index.ts | 13 ++-- .../test/suite/agent-session-runtime.test.ts | 78 +++++++++++++++++-- ...473-cancelled-resume-reservations.test.ts} | 0 ...sue-1473-prepared-session-writers.test.ts} | 0 8 files changed, 122 insertions(+), 24 deletions(-) rename packages/coding-agent/test/suite/regressions/{1473-cancelled-resume-reservations.test.ts => issue-1473-cancelled-resume-reservations.test.ts} (100%) rename packages/coding-agent/test/suite/regressions/{1473-prepared-session-writers.test.ts => issue-1473-prepared-session-writers.test.ts} (100%) diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 3ef5f7ec52..f598791de1 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -1,6 +1,6 @@ import { copyFileSync, existsSync, mkdirSync } from "node:fs"; import { basename, join, resolve } from "node:path"; -import { resolvePath } from "../utils/paths.ts"; +import { canonicalizePath, resolvePath } from "../utils/paths.ts"; import type { AgentSession } from "./agent-session.ts"; import type { AgentSessionRuntimeDiagnostic, AgentSessionServices } from "./agent-session-services.ts"; import type { @@ -247,6 +247,11 @@ export class AgentSessionRuntime { }, ): Promise<{ cancelled: boolean }> { const previousSessionFile = this.session.sessionFile; + const isSelfResume = + previousSessionFile !== undefined && + canonicalizePath(resolve(sessionPath)) === canonicalizePath(resolve(previousSessionFile)); + // Settling active work would append to this same file after taking the candidate snapshot. + if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; const prepared = SessionManager.prepareOpen(sessionPath, undefined, options?.cwdOverride); const { sessionManager } = prepared; assertSessionCwdExists(sessionManager, this.cwd); @@ -267,6 +272,8 @@ export class AgentSessionRuntime { acceptance = prepared.beginCommit(); const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); if (beforeResult.cancelled) return beforeResult; + // Preparation and veto handlers can yield while a new turn starts on the live session. + if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; acceptance.commit(); await this.teardownCurrent("resume", sessionManager.getSessionFile()); await this.apply(result); diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 839c8a2ded..7550e26eb5 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -2938,14 +2938,11 @@ export class AgentSession { if (options?.releaseProviderResources !== false) cleanupSessionResources(this.sessionId); } - /** Shut down an unstarted destination without releasing the live session's provider resources. */ + /** Invalidate only an unstarted destination's registrations, not live-session resources. */ async disposeCandidate(): Promise { - try { - // This is discarded resume preparation, not a process quit or resource reload. - await emitSessionShutdownEvent(this._extensionRunner, { type: "session_shutdown", reason: "resume" }); - } finally { - this.dispose({ releaseProviderResources: false }); - } + // Normal shutdown handlers may mutate process-global state owned by the live session. + // Runner invalidation removes this candidate's tracked subscriptions without dispatching them. + this.dispose({ releaseProviderResources: false }); } /** Live in-session activity signals; see `session-activity.ts` for the contract. */ diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 2e75cf0091..c9a81e1c02 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,26 @@ +## Contained resume lifecycle corrections (2026-09-09) + +### What changed + +- `packages/coding-agent/src/core/agent-session.ts`: discarded unstarted candidates invalidate only their own tracked registrations, without broadcasting `session_shutdown` or releasing live provider resources. MCP defers its direct service listener until attachment. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: busy self-resumes return cancellation before preparation; canonical path comparison covers aliases, and activity is rechecked after asynchronous preparation/veto handling before acceptance. +- Regression filenames now use `issue-1473-` rather than a bare PR-number prefix. Deterministic runtime coverage asserts untouched Cursor/image globals, MCP listener baseline, and complete tool-result context after a cancelled busy self-resume. + +### Why + +- `packages/coding-agent/src/core/agent-session.ts`: broadcasting shutdown from a candidate called process-global Cursor `killAll` and reset native-image bypass owned by the still-live session. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: aborting busy self-resumes persisted final entries after the candidate snapshot, leaving the replacement context stale. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/agent-session.ts`: candidate invalidation is a core ownership boundary; it must not dispatch destructive live-session lifecycle handlers. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: only the runtime can reject before snapshot construction and before teardown persists to the same file. The separate trust/admission lifecycle-contract question is unchanged. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/agent-session.ts`: `disposeCandidate`. +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` admission and acceptance guards. + ## Cancellation-safe candidate ownership (2026-09-09) ### What changed @@ -5,12 +28,12 @@ - `packages/coding-agent/src/core/session-manager.ts`: all prepared-manager writer entry points defer reservations, including open, set/reload, new, branch and write paths. Acceptance acquires the actual destination grant and revalidates its bytes before switch handlers; persistence remains deferred until the veto passes. - `packages/coding-agent/src/core/session-write-reservation.ts`: a newly acquired grant returns a rollback callback; an already-owned live grant does not. - `packages/coding-agent/src/core/agent-session-runtime.ts`: cancelled or failed acceptance rolls back new ownership after candidate cleanup; reservation denial precedes destructive switch handlers and live teardown. -- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: discarded candidates run their own `session_shutdown` handlers with the resume reason before invalidation, including budget-admission failure, without releasing provider resources belonging to the live session. +- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: discarded candidates use candidate-only invalidation, including budget-admission failure, without normal shutdown dispatch or releasing provider resources belonging to the live session. ### Why - `packages/coding-agent/src/core/session-manager.ts`, `packages/coding-agent/src/core/session-write-reservation.ts` and `packages/coding-agent/src/core/agent-session-runtime.ts`: a cancelled shared-host resume retained a destination reservation until worker exit, preventing another client from opening the target. Snapshot revalidation prevents committing admission based on changed destination bytes. -- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: ordinary disposal skipped extension shutdown, retaining candidate MCP subscriptions on the live singleton. +- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: candidate cleanup must release tracked registrations without destructive global shutdown; the MCP builtin now defers its untracked singleton listener until attachment. ### Why an extension could not handle it @@ -21,7 +44,7 @@ - `packages/coding-agent/src/core/session-manager.ts`: deferred persistence, reservation entry points and `prepareOpen` acceptance. - `packages/coding-agent/src/core/session-write-reservation.ts`: host grant callback. - `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` acceptance and discard finally block. -- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: candidate shutdown and failed model admission. +- `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: candidate-only invalidation and failed model admission. ## Resume admission uses the prepared destination runtime (2026-09-08) diff --git a/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md b/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md index 0666779ba5..be2299dbe1 100644 --- a/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md @@ -4,7 +4,7 @@ ### What changed -- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: shutdown of an unattached candidate removes its factory-time inventory request and wire-status subscriptions without invoking shutdown on the service attached to the live runtime. Attached-session switch, reload and quit behavior is unchanged. +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: direct wire-status subscription is deferred until attachment, before inventory can be emitted, so an unstarted candidate never retains an API on the live service. Its factory-time `pi.events` inventory-request subscription is removed by existing runner invalidation, without normal shutdown dispatch. Attached-session switch, reload and quit behavior is unchanged. ### Why @@ -12,11 +12,11 @@ ### Why an extension could not handle it -- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: this builtin owns the subscriptions and knows whether its instance attached the shared service. Core must emit candidate shutdown before invalidating its runner. +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: the fix is implemented in this builtin: only attachment needs the direct service listener. Existing runner invalidation cleans up candidate-owned event-bus registrations; core must not emit normal shutdown for unstarted candidates because other builtins mutate live process-global state. ### Expected merge conflict zones -- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: session shutdown ownership and inventory bridge cleanup. +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: attachment-time wire subscription, session shutdown ownership and inventory bridge cleanup. ## Explicit pgrep match-all pattern for process-tree collection (2026-08-12) diff --git a/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts b/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts index 907c4c393a..9a46e82fd3 100644 --- a/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts @@ -35,15 +35,12 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex data.respond(service.refreshWireStatusSnapshot(data.sessionId)); }, ); - const unsubscribeWireStatus = service.onWireStatusChanged((sessionId, snapshot) => { - if (sessionId === undefined || sessionId !== attachedSessionId) return; - pi.events.emit(MCP_CONTROL_INVENTORY_CHANGED_EVENT, { sessionId, snapshot }); - }); + let unsubscribeWireStatus: (() => void) | undefined; const disposeControlInventory = (): void => { if (controlInventoryDisposed) return; controlInventoryDisposed = true; unsubscribeControlInventoryRequest(); - unsubscribeWireStatus(); + unsubscribeWireStatus?.(); }; const sink = { logger: { @@ -112,6 +109,12 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex // session_start always starts a fresh attach (reloads must re-sync config). const attach = (event: SessionStartEvent, ctx: ExtensionContext): Promise => { attachedSessionId = ctx.sessionManager?.getSessionId?.(); + // Unstarted candidates must not retain APIs on the live singleton. Unlike + // pi.events subscriptions, this direct listener is not tracked by runner invalidation. + unsubscribeWireStatus ??= service.onWireStatusChanged((sessionId, snapshot) => { + if (sessionId === undefined || sessionId !== attachedSessionId) return; + pi.events.emit(MCP_CONTROL_INVENTORY_CHANGED_EVENT, { sessionId, snapshot }); + }); attachPromise = (async () => { await service.attachSession(event, ctx, pi); refreshMcpInstructionsForSession(service); diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index f33a1ab755..51b3e89009 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -18,6 +18,8 @@ import { import { AuthStorage } from "../../src/core/auth-storage.ts"; import { estimateTokens } from "../../src/core/compaction/compaction.ts"; import { ModelUsabilityBudgetError } from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { cursorCliChildRegistry } from "../../src/core/extensions/builtin/cursor-cli-oauth/diagnostics.ts"; +import { isNativeBypass, setNativeBypass } from "../../src/core/extensions/builtin/imagegen/state.ts"; import { getMcpService } from "../../src/core/extensions/builtin/mcp/service.ts"; import { SessionManager } from "../../src/core/session-manager.ts"; import type { @@ -254,6 +256,61 @@ describe("AgentSessionRuntime characterization", () => { expect(outgoingEntries.map((entry) => entry.message.role)).toEqual(["user", "assistant", "toolResult"]); }); + // PR #1473: a busy self-resume must not replace the live context with a pre-tool snapshot. + it("cancels a busy self-resume and retains the completing tool result", async () => { + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + const aborted = vi.fn(); + let factories = 0; + const { runtime, faux } = await createRuntimeForTest((pi) => { + factories++; + pi.registerTool({ + name: "block", + label: "Block", + description: "Waits for explicit release", + parameters: Type.Object({}), + execute: async (_id, _params, signal) => { + const onAbort = () => { + aborted(); + release.resolve(); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + started.resolve(); + try { + await release.promise; + return { content: [{ type: "text" as const, text: "released" }], details: {} }; + } finally { + signal?.removeEventListener("abort", onAbort); + } + }, + }); + }); + await runtime.session.prompt("persist source"); + const live = runtime.session; + const path = live.sessionFile; + if (!path) throw new Error("missing session file"); + faux.setResponses([ + fauxAssistantMessage(fauxToolCall("block", {}, { id: "self-resume-tool" }), { stopReason: "toolUse" }), + fauxAssistantMessage("complete"), + ]); + const prompt = live.prompt("start blocked tool"); + try { + await started.promise; + expect(live.isSessionBusy).toBe(true); + expect(await runtime.switchSession(path)).toEqual({ cancelled: true }); + expect(runtime.session).toBe(live); + expect(factories).toBe(1); + expect(aborted).not.toHaveBeenCalled(); + } finally { + release.resolve(); + await prompt; + } + expect(runtime.session.messages).toEqual(SessionManager.open(path).buildSessionContext().messages); + expect(runtime.session.messages.filter((message) => message.role === "toolResult")).toMatchObject([ + { toolCallId: "self-resume-tool", isError: false }, + ]); + }, 15_000); + it("emits session_before_switch and session_start for new and resume flows", async () => { const events: RecordedSessionEvent[] = []; const { runtime } = await createRuntimeForTest((pi: ExtensionAPI) => { @@ -731,8 +788,8 @@ describe("AgentSessionRuntime characterization", () => { await expect(runtime.switchSession(path)).rejects.toMatchObject({ projection: { model: `${faux.getModel().provider}/${forced}`, admission: "resume" }, }); - expect(events).toEqual([{ type: "session_shutdown", reason: "resume" }]); - expect(shutdownSessions).toEqual([target.getSessionId()]); + expect(events).toEqual([]); + expect(shutdownSessions).toEqual([]); expect(shutdownSessions).not.toContain(original.sessionId); expect(readFileSync(path)).toEqual(bytes); expect(runtime.session).toBe(original); @@ -774,9 +831,9 @@ describe("AgentSessionRuntime characterization", () => { await expect(original.prompt("still usable")).resolves.toBeUndefined(); }); - // PR #1473: factory-time MCP subscriptions belong to each candidate, not the singleton owner. + // PR #1473: candidate cleanup must not invoke process-global shutdown hooks. it.each(["cancelled", "rejected"])( - "shuts down %s candidates without retaining listeners or disposing live MCP", + "discards %s candidates without retaining listeners or disturbing live globals", async (outcome) => { const service = getMcpService(); const listeners = new Set[0]>(); @@ -789,6 +846,7 @@ describe("AgentSessionRuntime characterization", () => { unsubscribe(); }; }); + const killAll = vi.spyOn(cursorCliChildRegistry, "killAll"); const shutdowns: number[] = []; let factories = 0; try { @@ -803,6 +861,11 @@ describe("AgentSessionRuntime characterization", () => { outcome === "rejected" ? { destinationSystemPrompt: "p".repeat(400_000) } : undefined, ); const live = runtime.session; + expect(live.extensionRunner.getExtensionIdentities().map((extension) => extension.path)).toEqual( + expect.arrayContaining(["", "", ""]), + ); + // Model the still-live native image lane without replacing its real shutdown handler. + setNativeBypass(true); const baseline = new Set(listeners); expect(baseline.size).toBe(1); const target = join(tempDir, "mcp-cancelled.jsonl"); @@ -823,7 +886,10 @@ describe("AgentSessionRuntime characterization", () => { } else { expect(await runtime.switchSession(target)).toEqual({ cancelled: true }); } - expect(shutdowns).toEqual(Array.from({ length: attempt }, (_, index) => index + 1)); + expect(killAll).not.toHaveBeenCalled(); + expect(isNativeBypass()).toBe(true); + expect(shutdowns).toEqual([]); + expect(factories).toBe(attempt + 1); expect(listeners).toEqual(baseline); expect(service.getSnapshot()).toMatchObject({ disposed: false, @@ -843,6 +909,8 @@ describe("AgentSessionRuntime characterization", () => { expect(listeners).toEqual(baseline); } finally { observation.mockRestore(); + killAll.mockRestore(); + setNativeBypass(false); } }, ); diff --git a/packages/coding-agent/test/suite/regressions/1473-cancelled-resume-reservations.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts similarity index 100% rename from packages/coding-agent/test/suite/regressions/1473-cancelled-resume-reservations.test.ts rename to packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts diff --git a/packages/coding-agent/test/suite/regressions/1473-prepared-session-writers.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts similarity index 100% rename from packages/coding-agent/test/suite/regressions/1473-prepared-session-writers.test.ts rename to packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts From a7686fd85f27e6f6e2fb47820586aa61f037c91a Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Wed, 9 Sep 2026 23:16:11 +0800 Subject: [PATCH 05/11] fix(sessions): preserve staged resume data and recover conflicts Keep materialized history until persistence, revalidate changed targets, retain typed recovery across TUI/RPC, normalize busy self-resume paths, and defer MCP native-search ownership until attachment. Admission ordering is unchanged. Verified with 26 named regressions, 10285 passing package tests, check/build, 10 real RPC scenarios and 12 xterm TUI scenarios. Ancillary text-tool-leak smoke failures reproduce identically at the unchanged base. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 3 +- packages/coding-agent/docs/rpc.md | 3 +- .../src/core/agent-session-runtime.ts | 2 +- packages/coding-agent/src/core/changes.md | 56 ++++++ .../core/extensions/builtin/mcp/changes.md | 18 ++ .../src/core/extensions/builtin/mcp/index.ts | 9 +- .../coding-agent/src/core/session-manager.ts | 87 ++++++--- .../src/core/session-resume-conflict.ts | 10 + .../src/modes/interactive/changes.md | 18 ++ .../src/modes/interactive/interactive-mode.ts | 22 ++- .../coding-agent/src/modes/rpc/changes.md | 18 ++ .../src/modes/rpc/connection-handler.ts | 14 +- .../coding-agent/src/modes/rpc/rpc-client.ts | 4 + .../interactive-mode-resume-budget.test.ts | 10 +- .../test/suite/issue-1473-runtime-support.ts | 101 ++++++++++ ...issue-1473-deferred-legacy-content.test.ts | 159 ++++++++++++++++ .../issue-1473-mcp-candidate-gate.test.ts | 94 ++++++++++ ...ssue-1473-prepared-session-writers.test.ts | 31 +++ .../issue-1473-resume-conflict.test.ts | 176 ++++++++++++++++++ .../issue-1473-resume-lifecycle.test.ts | 83 +++++++++ 20 files changed, 871 insertions(+), 47 deletions(-) create mode 100644 packages/coding-agent/src/core/session-resume-conflict.ts create mode 100644 packages/coding-agent/test/suite/issue-1473-runtime-support.ts create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-mcp-candidate-gate.test.ts create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c2c6169deb..0afb08ef7d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,7 @@ ### Fixed - Resume preflight checks the actual destination runtime's model budget before switch handlers or teardown, preserving the live session on rejection, including cwd-override retries and shared-host RPC. Cancelled candidates leave target files unchanged, release tentative writer reservations, and run their own MCP listener cleanup without shutting down the live service. +- Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). ### Removed @@ -8639,4 +8640,4 @@ Initial public release. - Git branch display in footer - Message queueing during streaming responses - OAuth integration for Gmail and Google Calendar access -- HTML export with syntax highlighting and collapsible sections \ No newline at end of file +- HTML export with syntax highlighting and collapsible sections diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index 85d6a9c0ea..bcb955f385 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -1066,9 +1066,10 @@ A rejected switch answers with `success: false` plus a typed `errorCode` and str | `errorCode` | Meaning | `errorData` | |---|---|---| | `missing_session_cwd` | The session's stored cwd no longer exists. Retry the command with `cwdOverride`. | `{"sessionFile", "sessionCwd", "fallbackCwd"}` | +| `session_resume_conflict` | The target changed after the resume snapshot was read. Retry to admit the current bytes. | `{"sessionFile"}` | | `model_usability_budget` | The stored transcript does not fit the context budget of the model the switch would run on. | The full budget projection | -Both rejections are decided before the switch lifecycle event and live-session teardown, so the current session stays usable. A cancelled or rejected switch does not write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. +Missing-cwd and budget rejections are decided before the switch lifecycle event and live-session teardown, so the current session stays usable. Resume conflicts are checked both before the switch lifecycle event and again after its awaited handlers, immediately before persistence. A conflict preserves the live session and any intervening target writes; the rejected candidate does not persist. A cancelled or rejected switch does not itself write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. ```json { diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index f598791de1..2521d2e24f 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -249,7 +249,7 @@ export class AgentSessionRuntime { const previousSessionFile = this.session.sessionFile; const isSelfResume = previousSessionFile !== undefined && - canonicalizePath(resolve(sessionPath)) === canonicalizePath(resolve(previousSessionFile)); + canonicalizePath(resolvePath(sessionPath)) === canonicalizePath(resolvePath(previousSessionFile)); // Settling active work would append to this same file after taking the candidate snapshot. if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; const prepared = SessionManager.prepareOpen(sessionPath, undefined, options?.cwdOverride); diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index c9a81e1c02..668e1576f1 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,59 @@ +## Preserve materialized staged history until persistence (2026-09-09) + +### What changed + +- `packages/coding-agent/src/core/session-manager.ts`: prepared managers keep full materialized loaded and appended entries, including branch replacements, and defer mirror trimming. Successful durable acceptance restores the existing bounded resident cache and releases full staged snapshots/read caches. Compaction-aware reads materialize evicted persisted entries directly instead of re-evicting them during the same read. + +### Why + +- `packages/coding-agent/src/core/session-manager.ts`: v1/v2 migration is deferred, so old disk IDs cannot recover content evicted before rewrite; a single 65MiB string or aggregate over 64MiB otherwise undercounts admission and permanently serializes resident markers. Re-externalizing disk fallbacks also loses over-budget strings on post-persistence reads. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/session-manager.ts`: migration, snapshot admission, persistence and cache ownership are internal manager responsibilities. No cache limits are raised, and no recovery relies on migrated old IDs. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/session-manager.ts`: loaded/appended/branched resident forms, deferred mirror trim, prepareOpen commit, and _getCompactEntries materialization. + +## Revalidate prepared resume at persistence (2026-09-09) + +### What changed + +- `packages/coding-agent/src/core/session-manager.ts`: acceptance revalidates destination bytes synchronously again inside commit, immediately before persistence. Both checks release newly acquired grants on conflict; rollback is idempotent. +- `packages/coding-agent/src/core/session-resume-conflict.ts`: typed recoverable SessionResumeConflictError carries the destination sessionFile. + +### Why + +- `packages/coding-agent/src/core/session-manager.ts` and `packages/coding-agent/src/core/session-resume-conflict.ts`: asynchronous veto handlers can invalidate the admitted snapshot, including legacy rewrites and empty/missing destinations, without making the live runtime unusable. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/session-manager.ts` and `packages/coding-agent/src/core/session-resume-conflict.ts`: only the prepared writer can validate the snapshot adjacent to synchronous persistence and roll back its grant. The pre-veto check and admission ordering remain intact. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/session-manager.ts`: prepareOpen acceptance closure. +- `packages/coding-agent/src/core/session-resume-conflict.ts`: new typed error. + +## Compatible staged-resume safety (2026-09-09) + +### What changed + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: busy self-resume identity uses SessionManager resolvePath normalization before canonicalization, including file URLs and tilde paths. + +### Why + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: lexical node:path resolution missed accepted input aliases and allowed teardown to overtake a completing tool. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: the guard must run before candidate construction. Exact admission still precedes before-switch; the separate lifecycle decision is unresolved. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: switchSession identity guard. + ## Contained resume lifecycle corrections (2026-09-09) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md b/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md index be2299dbe1..2ee5ea9ef4 100644 --- a/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md @@ -1,3 +1,21 @@ +## Install native search ownership only on attachment (2026-09-09) + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: native tool-search gate registration moves from the extension factory to actual attachment, alongside the deferred wire listener. + +### Why + +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: constructing a false/unconfigured candidate replaced a true live gate in both async-local and independent process-fallback contexts. Discarding the candidate could not undo this global mutation. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: this builtin owns registration; attaching the accepted service is the correct ownership boundary, not candidate construction or cleanup. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: factory and attach closure. Single-flight attachment and candidate-only cleanup are unchanged. + # mcp Extension Changes ## Discarded candidates release only their own MCP subscriptions (2026-09-09) diff --git a/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts b/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts index 9a46e82fd3..8dc4c20fbc 100644 --- a/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/mcp/index.ts @@ -52,10 +52,6 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex registerMcpCommands(pi, service); - installMcpNativeToolSearchGate(() => { - const setting = service.getNativeToolSearchSetting(); - return setting === true || setting === "auto"; - }); // skills-carry-MCP (todo 37): skills declaring MCP servers (mcp.json // sidecar or SKILL.md frontmatter) register lazily with tools hidden; // loading a skill — /skill: input or the model reading its SKILL.md — @@ -108,6 +104,11 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex // the first turn's payload deterministically carries the MCP tool set. // session_start always starts a fresh attach (reloads must re-sync config). const attach = (event: SessionStartEvent, ctx: ExtensionContext): Promise => { + // Candidate factories must not replace the active async-context or process fallback gate. + installMcpNativeToolSearchGate(() => { + const setting = service.getNativeToolSearchSetting(); + return setting === true || setting === "auto"; + }); attachedSessionId = ctx.sessionManager?.getSessionId?.(); // Unstarted candidates must not retain APIs on the live singleton. Unlike // pi.events subscriptions, this direct listener is not tracked by runner invalidation. diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 7c53ebca3a..2d54533535 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -20,6 +20,7 @@ import { APP_NAME, getAgentDir as getDefaultAgentDir, getSessionsDir } from "../ import { normalizePath, resolvePath } from "../utils/paths.ts"; import { listSessionInfos, listSessionsFromDir, type SessionListProgress } from "./session-discovery.ts"; import { type ResidentStoreStats, ResidentStringStore } from "./session-resident-store.ts"; +import { SessionResumeConflictError } from "./session-resume-conflict.ts"; import { reserveSessionWrite } from "./session-write-reservation.ts"; export type { SessionListProgress } from "./session-discovery.ts"; @@ -955,7 +956,9 @@ export class SessionManager { this._rewriteFile(); } - this.fileEntries = this.fileEntries.map((entry) => this.residentStore.externalize(entry)); + this.fileEntries = this.fileEntries.map((entry) => + this.deferredPersistence ? this.residentStore.materialize(entry) : this.residentStore.externalize(entry), + ); this._buildIndex(); this.mutationCount++; this.flushed = true; @@ -1129,7 +1132,9 @@ export class SessionManager { } private _appendEntry(entry: SessionEntry): void { - const residentEntry = this.residentStore.externalize(entry); + const residentEntry = this.deferredPersistence + ? this.residentStore.materialize(entry) + : this.residentStore.externalize(entry); this.fileEntries.push(residentEntry); this.byId.set(residentEntry.id, residentEntry); this.entryOrdersById.set(residentEntry.id, this.fileEntries.length - 1); @@ -1319,7 +1324,8 @@ export class SessionManager { } private _trimMirrorAfterCompaction(compaction: CompactionEntry): void { - if (!this.persist) return; + // Prepared history is not durable yet; trimming would discard rewrite input. + if (!this.persist || this.deferredPersistence) return; const firstKeptIndex = this.fileEntries.findIndex((entry) => entry.id === compaction.firstKeptEntryId); const compactionIndex = this.fileEntries.findIndex((entry) => entry.id === compaction.id); if (firstKeptIndex < 0 || compactionIndex < firstKeptIndex) return; @@ -1613,16 +1619,21 @@ export class SessionManager { return undefined; }); } - if (missingEntryIds.size > 0 && this.sessionFile) { - const persistedById = new Map(this._loadFullHistoryEntries().map((entry) => [entry.id, entry])); - for (let index = 0; index < entries.length; index++) { - const entry = entries[index]!; - if (!missingEntryIds.has(entry.id)) continue; - const persisted = persistedById.get(entry.id); - if (persisted) entries[index] = this.residentStore.externalize(persisted) as SessionEntry; - } - } - const materialized = entries.map((entry) => this.residentStore.materialize(entry) as SessionEntry); + const persistedById = + missingEntryIds.size > 0 && this.sessionFile + ? new Map( + this._loadFullHistoryEntries() + .filter((entry): entry is SessionEntry => entry.type !== "session") + .map((entry) => [entry.id, entry]), + ) + : undefined; + // Materialize disk fallbacks directly into this read, not back into the bounded + // cache: one 65MiB string (or the aggregate) would immediately evict itself again. + const materialized = entries.map( + (entry) => + (missingEntryIds.has(entry.id) ? persistedById?.get(entry.id) : undefined) ?? + this.residentStore.materialize(entry), + ); for (const entry of materialized) { if (entry.type !== "message") continue; const order = this.entryOrdersById.get(entry.id); @@ -1812,7 +1823,7 @@ export class SessionManager { this.residentStore.clear(); this.mirrorTrimmed = false; this.fileEntries = [header, ...pathWithoutLabels, ...labelEntries].map((entry) => - this.residentStore.externalize(entry), + this.deferredPersistence ? this.residentStore.materialize(entry) : this.residentStore.externalize(entry), ); this.sessionId = newSessionId; this.sessionFile = newSessionFile; @@ -1898,20 +1909,30 @@ export class SessionManager { if (!file) throw new Error("Prepared session is missing its destination file"); // Admission ran on a read-only snapshot. Revalidate the actual destination // under its canonical host grant before any destructive switch handler. - const release = reserveSessionWrite(file); - try { - const expected = pending?.snapshots.get(file); - const current = existsSync(file) ? readFileSync(file) : undefined; - if (expected === undefined ? current !== undefined : !current?.equals(expected)) { - throw new Error(`Session file changed while preparing resume: ${file}`); - } - } catch (error) { + let release = reserveSessionWrite(file); + const rollback = () => { release?.(); - throw error; - } + release = undefined; + }; + const revalidate = () => { + try { + const expected = pending?.snapshots.get(file); + const current = existsSync(file) ? readFileSync(file) : undefined; + if (expected === undefined ? current !== undefined : !current?.equals(expected)) { + throw new SessionResumeConflictError(file); + } + } catch (error) { + rollback(); + throw error; + } + }; + revalidate(); return { - rollback: () => release?.(), + rollback, commit: () => { + // Veto handlers awaited since beginCommit may have changed the file. + // Keep this final check and persistence synchronous, before live teardown. + revalidate(); sessionManager.deferredPersistence = undefined; mkdirSync(sessionManager.sessionDir, { recursive: true }); if (pending?.rewrite) { @@ -1923,6 +1944,22 @@ export class SessionManager { } for (const entry of pending?.entries ?? []) sessionManager._persist(entry); } + // Only durable entries may use an evictable cache. Admission and any + // legacy rewrite above consumed the full materialized staged history. + if (sessionManager.flushed) { + const leafId = sessionManager.leafId; + sessionManager.fileEntries = sessionManager.fileEntries.map((entry) => + sessionManager.residentStore.externalize(entry), + ); + sessionManager._buildIndex(); + sessionManager.leafId = leafId; + sessionManager.mutationCount++; + sessionManager.entriesCache = null; + sessionManager.branchCache = null; + sessionManager.compactEntriesCache = null; + } + pending?.entries.splice(0); + pending?.snapshots.clear(); }, }; }, diff --git a/packages/coding-agent/src/core/session-resume-conflict.ts b/packages/coding-agent/src/core/session-resume-conflict.ts new file mode 100644 index 0000000000..61644d542a --- /dev/null +++ b/packages/coding-agent/src/core/session-resume-conflict.ts @@ -0,0 +1,10 @@ +/** Recoverable rejection: resume admission no longer describes the destination bytes. */ +export class SessionResumeConflictError extends Error { + readonly sessionFile: string; + + constructor(sessionFile: string) { + super(`Session file changed while preparing resume: ${sessionFile}`); + this.name = "SessionResumeConflictError"; + this.sessionFile = sessionFile; + } +} diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 87d1991e99..b933c1a3e7 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -1,3 +1,21 @@ +## Recoverable resume conflicts (2026-09-09) + +### What changed + +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: handle SessionResumeConflictError recoverably for both direct and cwd-override retry resume, sharing the existing budget error rendering helper. + +### Why + +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: a changed target is a rejected candidate, not a fatal error in the still-live session. Error identity must survive transport without parsing message text. + +### Why an extension could not handle it + +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: core transport and interactive exception routing own these boundaries, outside extension error handling. + +### Expected merge conflict zones + +- `packages/coding-agent/src/modes/interactive/interactive-mode.ts`: imports and typed resume error handling. Existing budget rejection behavior is retained. + ## 2026-09-08 - Over-budget resume shows an error instead of exiting the process diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index c56d7c70f9..dfac46dba0 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -109,6 +109,7 @@ import type { ResourceDiagnostic } from "../../core/resource-loader.ts"; import { formatMissingSessionCwdPrompt, MissingSessionCwdError } from "../../core/session-cwd.ts"; import { createSessionLogger, type SessionLogger } from "../../core/session-log.ts"; import { type SessionEntry, SessionManager, sessionEntryToContextMessages } from "../../core/session-manager.ts"; +import { SessionResumeConflictError } from "../../core/session-resume-conflict.ts"; import type { FullscreenExitOutput, TuiMode } from "../../core/settings-manager.ts"; import { BUILTIN_SLASH_COMMANDS } from "../../core/slash-commands.ts"; import type { SourceInfo } from "../../core/source-info.ts"; @@ -7579,24 +7580,29 @@ export class InteractiveMode { this.showStatus("Resumed session in current cwd"); return result; } catch (overrideError: unknown) { - // The cwd-override retry can also be rejected by the model usability - // budget; give it the same recoverable treatment as the first attempt + // The cwd-override retry can also be rejected by admission or a + // changed target; give it the same recoverable treatment as the first attempt // instead of letting it escape as an unhandled rejection. - if (overrideError instanceof ModelUsabilityBudgetError) { - return this.cancelResumeWithBudgetError(overrideError); + if ( + overrideError instanceof ModelUsabilityBudgetError || + overrideError instanceof SessionResumeConflictError + ) { + return this.cancelResumeWithRecoverableError(overrideError); } return this.handleFatalRuntimeError("Failed to resume session", overrideError); } } - if (error instanceof ModelUsabilityBudgetError) { - return this.cancelResumeWithBudgetError(error); + if (error instanceof ModelUsabilityBudgetError || error instanceof SessionResumeConflictError) { + return this.cancelResumeWithRecoverableError(error); } return this.handleFatalRuntimeError("Failed to resume session", error); } } - /** Render an over-budget resume rejection and keep the live session running. */ - private cancelResumeWithBudgetError(error: ModelUsabilityBudgetError): { cancelled: boolean } { + /** Render a recoverable resume rejection and keep the live session running. */ + private cancelResumeWithRecoverableError(error: ModelUsabilityBudgetError | SessionResumeConflictError): { + cancelled: boolean; + } { this.showError(`Failed to resume session: ${error.message}`); return { cancelled: true }; } diff --git a/packages/coding-agent/src/modes/rpc/changes.md b/packages/coding-agent/src/modes/rpc/changes.md index 0f908ba89d..2736e3c6ce 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -1,3 +1,21 @@ +## Recoverable resume conflicts (2026-09-09) + +### What changed + +- `packages/coding-agent/src/modes/rpc/connection-handler.ts` and `packages/coding-agent/src/modes/rpc/rpc-client.ts`: serialize session_resume_conflict with sessionFile and reconstruct SessionResumeConflictError through the real worker/connection/socket/client path. + +### Why + +- `packages/coding-agent/src/modes/rpc/connection-handler.ts` and `packages/coding-agent/src/modes/rpc/rpc-client.ts`: a changed target is a rejected candidate, not a fatal error in the still-live session. Error identity must survive transport without parsing message text. + +### Why an extension could not handle it + +- `packages/coding-agent/src/modes/rpc/connection-handler.ts` and `packages/coding-agent/src/modes/rpc/rpc-client.ts`: core transport and interactive exception routing own these boundaries, outside extension error handling. + +### Expected merge conflict zones + +- `packages/coding-agent/src/modes/rpc/connection-handler.ts` and `packages/coding-agent/src/modes/rpc/rpc-client.ts`: imports and typed resume error handling. Existing budget rejection behavior is retained. + # changes ## Roll back unaccepted candidate writer grants (2026-09-09) diff --git a/packages/coding-agent/src/modes/rpc/connection-handler.ts b/packages/coding-agent/src/modes/rpc/connection-handler.ts index 53cdd12499..4771589107 100644 --- a/packages/coding-agent/src/modes/rpc/connection-handler.ts +++ b/packages/coding-agent/src/modes/rpc/connection-handler.ts @@ -55,6 +55,7 @@ import type { WorkingIndicatorOptions, } from "../../core/extensions/index.ts"; import { FooterDataProvider } from "../../core/footer-data-provider.ts"; +import { SessionResumeConflictError } from "../../core/session-resume-conflict.ts"; import { getSupportedThinkingLevels } from "../../core/thinking-levels.ts"; import { ProjectTrustStore } from "../../core/trust-manager.ts"; import { type Theme, theme } from "../interactive/theme/theme.ts"; @@ -1677,12 +1678,21 @@ export function createRpcConnectionHandler( // its instanceof check (and the TUI's recoverable handling) still works, // rather than seeing a plain Error and exiting. const budgetError = commandError instanceof ModelUsabilityBudgetError ? commandError : undefined; - const errorCode = missingCwd ? "missing_session_cwd" : budgetError ? "model_usability_budget" : undefined; + const conflict = commandError instanceof SessionResumeConflictError ? commandError : undefined; + const errorCode = missingCwd + ? "missing_session_cwd" + : budgetError + ? "model_usability_budget" + : conflict + ? "session_resume_conflict" + : undefined; const errorData = missingCwd ? (commandError as { issue: unknown }).issue : budgetError ? budgetError.projection - : undefined; + : conflict + ? { sessionFile: conflict.sessionFile } + : undefined; output( error( command.id, diff --git a/packages/coding-agent/src/modes/rpc/rpc-client.ts b/packages/coding-agent/src/modes/rpc/rpc-client.ts index b2611237ef..8e32ff172c 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-client.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-client.ts @@ -15,6 +15,7 @@ import { ModelUsabilityBudgetError } from "../../core/extensions/builtin/compact import type { ServiceTier } from "../../core/extensions/builtin/service-tier.ts"; import { MissingSessionCwdError } from "../../core/session-cwd.ts"; import type { SessionEntry, SessionTreeNode } from "../../core/session-manager.ts"; +import { SessionResumeConflictError } from "../../core/session-resume-conflict.ts"; import type { JsonAgentSessionEvent } from "../json-event.ts"; import { attachJsonlLineReader, serializeJsonLine } from "./jsonl.ts"; import type { @@ -1115,6 +1116,9 @@ export class RpcClient { errorResponse.errorData as ConstructorParameters[0], ); } + if (errorResponse.errorCode === "session_resume_conflict" && errorResponse.errorData) { + throw new SessionResumeConflictError((errorResponse.errorData as { sessionFile: string }).sessionFile); + } throw new Error(errorResponse.error); } // Type assertion: we trust response.data matches T based on the command sent. diff --git a/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts b/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts index 2ed70ab69d..6ad7d07c34 100644 --- a/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts +++ b/packages/coding-agent/test/suite/interactive-mode-resume-budget.test.ts @@ -17,7 +17,7 @@ interface ResumeContext { handleFatalRuntimeError: (prefix: string, error: unknown) => Promise; promptForMissingSessionCwd: (error: MissingSessionCwdError) => Promise; createProjectTrustContext: (cwd: string) => unknown; - cancelResumeWithBudgetError: (error: ModelUsabilityBudgetError) => { cancelled: boolean }; + cancelResumeWithRecoverableError: (error: ModelUsabilityBudgetError) => { cancelled: boolean }; } function getHandleResumeSession(): HandleResumeSession { @@ -48,9 +48,9 @@ function makeBudgetError(): ModelUsabilityBudgetError { } function makeContext(overrides: Partial): ResumeContext { - const cancelResumeWithBudgetError = Object.getOwnPropertyDescriptor( + const cancelResumeWithRecoverableError = Object.getOwnPropertyDescriptor( InteractiveMode.prototype, - "cancelResumeWithBudgetError", + "cancelResumeWithRecoverableError", )?.value as (this: ResumeContext, error: ModelUsabilityBudgetError) => { cancelled: boolean }; const base: ResumeContext = { clearStatusIndicator: vi.fn(), @@ -62,8 +62,8 @@ function makeContext(overrides: Partial): ResumeContext { }) as unknown as (prefix: string, error: unknown) => Promise, promptForMissingSessionCwd: vi.fn(async () => "/tmp/override-cwd"), createProjectTrustContext: vi.fn(() => ({})), - cancelResumeWithBudgetError: vi.fn(function (this: ResumeContext, error: ModelUsabilityBudgetError) { - return cancelResumeWithBudgetError.call(this, error); + cancelResumeWithRecoverableError: vi.fn(function (this: ResumeContext, error: ModelUsabilityBudgetError) { + return cancelResumeWithRecoverableError.call(this, error); }), ...overrides, }; diff --git a/packages/coding-agent/test/suite/issue-1473-runtime-support.ts b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts new file mode 100644 index 0000000000..26e141b15b --- /dev/null +++ b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts @@ -0,0 +1,101 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fauxAssistantMessage, registerFauxProvider } from "@earendil-works/pi-ai/compat"; +import { + type CreateAgentSessionRuntimeFactory, + createAgentSessionFromServices, + createAgentSessionRuntime, + createAgentSessionServices, +} from "../../src/core/agent-session-runtime.ts"; +import type { ExtensionAPI } from "../../src/core/extensions/types.ts"; +import { SessionManager } from "../../src/core/session-manager.ts"; + +export async function resumeRuntime(extension: (pi: ExtensionAPI) => void = () => {}, contextWindow?: number) { + const cwd = mkdtempSync(join(tmpdir(), "pr1473-runtime-")); + const faux = registerFauxProvider( + contextWindow ? { models: [{ id: "large", contextWindow, maxTokens: 1024 }] } : {}, + ); + faux.setResponses([fauxAssistantMessage("stored"), fauxAssistantMessage("follow-up")]); + let factories = 0; + const createRuntime: CreateAgentSessionRuntimeFactory = async ({ cwd, sessionManager, sessionStartEvent }) => { + factories++; + const services = await createAgentSessionServices({ + cwd, + agentDir: cwd, + resourceLoaderOptions: { + noSkills: true, + noPromptTemplates: true, + noThemes: true, + extensionFactories: [ + (pi) => { + const model = faux.getModel(); + pi.registerProvider(model.provider, { + baseUrl: model.baseUrl, + apiKey: "faux-key", + api: faux.api, + models: faux.models.map((m) => ({ + id: m.id, + name: m.name, + api: m.api, + reasoning: m.reasoning, + input: m.input, + cost: m.cost, + contextWindow: m.contextWindow, + maxTokens: m.maxTokens, + })), + }); + extension(pi); + }, + ], + }, + }); + return { + ...(await createAgentSessionFromServices({ + services, + sessionManager, + sessionStartEvent, + model: faux.getModel(), + })), + services, + diagnostics: services.diagnostics, + }; + }; + const runtime = await createAgentSessionRuntime(createRuntime, { + cwd, + agentDir: cwd, + sessionManager: SessionManager.create(cwd, join(cwd, "sessions")), + }); + await runtime.session.bindExtensions({}); + runtime.setRebindSession(async (session) => { + await session.bindExtensions({}); + }); + return { + runtime, + faux, + cwd, + factories: () => factories, + async dispose() { + try { + await runtime.dispose(); + } finally { + faux.unregister(); + rmSync(cwd, { recursive: true, force: true }); + } + }, + }; +} + +export async function deadline(promise: Promise): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error("PR1473 event deadline")), 10_000); + }), + ]); + } finally { + clearTimeout(timer); + } +} diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts new file mode 100644 index 0000000000..628323a1be --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts @@ -0,0 +1,159 @@ +import { createHash } from "node:crypto"; +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; +import { expect, it } from "vitest"; +import { ModelUsabilityBudgetError } from "../../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { loadEntriesFromFile, SessionManager } from "../../../src/core/session-manager.ts"; +import { getMessageText } from "../harness.ts"; +import { resumeRuntime } from "../issue-1473-runtime-support.ts"; + +const MiB = 1024 * 1024; +const hash = (text: string | Buffer) => createHash("sha256").update(text).digest("hex"); +const cases = [ + { version: 1, sizes: [65], layout: "single65MiB" }, + { version: 1, sizes: [33, 33], layout: "aggregate66MiB" }, + { version: 2, sizes: [65], layout: "single65MiB" }, + { version: 2, sizes: [33, 33], layout: "aggregate66MiB" }, +]; +function fixture(path: string, cwd: string, version: number, sizes: number[]) { + const timestamp = new Date(0).toISOString(); + const texts = sizes.map((size, i) => `${String.fromCharCode(65 + i)} `.repeat((size * MiB) / 2)); + const messages = [ + ...texts.map((text) => ({ role: "user", content: text, timestamp: 0 })), + fauxAssistantMessage("durable tail"), + ]; + const entries = messages.map((message, i) => ({ + type: "message", + id: `entry${i}`, + parentId: i ? `entry${i - 1}` : null, + timestamp, + message, + })); + const compact = { + type: "compaction", + id: "compact", + parentId: `entry${entries.length - 1}`, + timestamp, + summary: "summary", + tokensBefore: 1, + ...(version === 1 ? { firstKeptEntryIndex: 1 } : { firstKeptEntryId: "entry0" }), + }; + writeFileSync( + path, + [ + JSON.stringify({ type: "session", version, id: "legacy", timestamp, cwd }), + ...entries.map((e) => JSON.stringify(e)), + JSON.stringify(compact), + ].join("\n"), + ); + return texts.map((text) => ({ length: text.length, sha: hash(text) })); +} +function userContent(messages: readonly unknown[]) { + return messages + .filter((m) => typeof m === "object" && m !== null && "role" in m && m.role === "user") + .map((m) => { + const text = getMessageText(m); + return { length: text.length, sha: hash(text) }; + }); +} + +// PR #1473 ghN2N: use actual 64MiB overflow, never a raised or mocked resident budget. +it.each(cases)( + "PR1473 ghN2N: deferred legacy rewrite preserves evicted transcript content (v$version $layout)", + async ({ version, sizes }) => { + let cancel = true; + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", () => ({ cancel })); + }, 128_000_000); + try { + const path = join(host.cwd, "legacy.jsonl"); + const expected = fixture(path, host.cwd, version, sizes); + const before = hash(readFileSync(path)); + const prepared = SessionManager.prepareOpen(path); + const manager = prepared.sessionManager; + // Admission must see materialized content before migrated IDs exist on disk. + expect.soft(userContent(manager.buildSessionContext().messages)).toEqual(expected); + const cancelled = prepared.beginCommit(); + cancelled.rollback(); + expect(hash(readFileSync(path))).toBe(before); + const live = host.runtime.session; + expect(await host.runtime.switchSession(path)).toEqual({ cancelled: true }); + expect(host.runtime.session).toBe(live); + expect(hash(readFileSync(path))).toBe(before); + cancel = false; + expect(await host.runtime.switchSession(path)).toEqual({ cancelled: false }); + expect.soft(userContent(host.runtime.session.messages)).toEqual(expected); + const persisted = loadEntriesFromFile(path); + expect(persisted[0]).toMatchObject({ type: "session", version: 3 }); + expect(userContent(persisted.flatMap((e) => (e.type === "message" ? [e.message] : [])))).toEqual(expected); + const ids = new Set(persisted.map((e) => e.id)); + for (const entry of persisted) { + if (entry.type === "session") continue; + if (entry.parentId !== null) expect(ids.has(entry.parentId)).toBe(true); + if (entry.type === "compaction") expect(ids.has(entry.firstKeptEntryId)).toBe(true); + } + expect(readFileSync(path, "utf8").includes("senpi-resident-string:v1:")).toBe(false); + const accepted = host.runtime.session.sessionManager; + expect(accepted.getResidentStoreStats().blobBytes).toBeLessThanOrEqual(64 * MiB); + expect(accepted.getResidentStoreStats().blobCount).toBe(sizes.length === 1 ? 0 : 1); + expect(userContent(accepted.buildSessionContext().messages)).toEqual(expected); + expect(userContent(SessionManager.open(path).buildSessionContext().messages)).toEqual(expected); + } finally { + await host.dispose(); + } + }, + 60_000, +); + +// Actual SDK admission must reject the full large context; markers used to undercount it. +it("PR1473 ghN2N: legacy budget rejection emits no switch event and preserves bytes", async () => { + let vetoes = 0; + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", () => { + vetoes++; + }); + }); + try { + const path = join(host.cwd, "rejected.jsonl"); + fixture(path, host.cwd, 1, [65]); + const before = hash(readFileSync(path)); + const live = host.runtime.session; + await expect(host.runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + expect(vetoes).toBe(0); + expect(host.runtime.session).toBe(live); + expect(hash(readFileSync(path))).toBe(before); + } finally { + await host.dispose(); + } +}, 60_000); + +it("PR1473 ghN2N: staged appends remain materialized through persistence", async () => { + const host = await resumeRuntime(); + try { + const path = join(host.cwd, "append.jsonl"); + writeFileSync( + path, + JSON.stringify({ + type: "session", + version: 1, + id: "pending", + timestamp: new Date(0).toISOString(), + cwd: host.cwd, + }), + ); + const prepared = SessionManager.prepareOpen(path); + const text = "P".repeat(65 * MiB); + prepared.sessionManager.appendMessage({ role: "user", content: text, timestamp: 0 }); + prepared.sessionManager.appendMessage(fauxAssistantMessage("tail")); + const expected = [{ length: text.length, sha: hash(text) }]; + expect.soft(userContent(prepared.sessionManager.buildSessionContext().messages)).toEqual(expected); + prepared.beginCommit().commit(); + expect(userContent(loadEntriesFromFile(path).flatMap((e) => (e.type === "message" ? [e.message] : [])))).toEqual( + expected, + ); + expect(prepared.sessionManager.getResidentStoreStats().blobBytes).toBeLessThanOrEqual(64 * MiB); + } finally { + await host.dispose(); + } +}, 60_000); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-mcp-candidate-gate.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-mcp-candidate-gate.test.ts new file mode 100644 index 0000000000..26fd564db7 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-mcp-candidate-gate.test.ts @@ -0,0 +1,94 @@ +import { AsyncResource } from "node:async_hooks"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { ProviderScope, runWithProviderScope } from "@earendil-works/pi-ai/node/provider-scope"; +import { expect, it } from "vitest"; +import { createEventBus } from "../../../src/core/event-bus.ts"; +import { createMcpExtension } from "../../../src/core/extensions/builtin/mcp/index.ts"; +import { McpService } from "../../../src/core/extensions/builtin/mcp/service.ts"; +import { + installMcpNativeToolSearchGate, + isMcpNativeToolSearchEnabled, +} from "../../../src/core/extensions/builtin/tool-search/native-search.ts"; +import { createExtensionRuntime, loadExtensionFromFactory } from "../../../src/core/extensions/loader.ts"; +import type { Extension } from "../../../src/core/extensions/types.ts"; + +// Created before any gate installation: runInAsyncScope reads the process fallback, not an inherited gate. +const independent = new AsyncResource("pr1473-independent-gate"); + +// PR #1473 ghN2J: actual scoped services, real config load and extension attachment, no gate mocks. +it.each([true, false])( + "PR1473 ghN2J: discarded candidates preserve the active MCP native gate (%s)", + async (active) => { + const root = mkdtempSync(join(tmpdir(), "pr1473-mcp-")); + const liveService = new McpService(); + const candidateService = new McpService(); + const liveScope = new ProviderScope(); + const candidateScope = new ProviderScope(); + const candidateRuntime = createExtensionRuntime(); + const makeCwd = (name: string, enabled: boolean) => { + const cwd = join(root, name); + mkdirSync(join(cwd, ".senpi"), { recursive: true }); + writeFileSync( + join(cwd, ".senpi", "mcp.json"), + JSON.stringify({ settings: { nativeToolSearch: enabled }, mcpServers: {} }), + ); + return cwd; + }; + const liveCwd = makeCwd("live", active); + const candidateCwd = makeCwd("candidate", !active); + const attach = async (extension: Extension, cwd: string) => { + for (const handler of extension.handlers.get("session_start") ?? []) { + await handler({ type: "session_start", reason: "resume" }, { cwd, isProjectTrusted: () => true }); + } + }; + try { + await runWithProviderScope(liveScope, async () => { + const live = await loadExtensionFromFactory( + createMcpExtension(liveService), + liveCwd, + createEventBus(), + createExtensionRuntime(), + "", + ); + await attach(live, liveCwd); + expect(liveService.getNativeToolSearchSetting()).toBe(active); + expect(isMcpNativeToolSearchEnabled()).toBe(active); + expect(independent.runInAsyncScope(isMcpNativeToolSearchEnabled)).toBe(active); + await runWithProviderScope(candidateScope, async () => { + await loadExtensionFromFactory( + createMcpExtension(candidateService), + candidateCwd, + createEventBus(), + candidateRuntime, + "", + ); + expect(candidateService.getSnapshot().sessionStartCount).toBe(0); + candidateRuntime.invalidate(); + expect.soft(isMcpNativeToolSearchEnabled()).toBe(active); + }); + expect.soft(independent.runInAsyncScope(isMcpNativeToolSearchEnabled)).toBe(active); + const accepted = await runWithProviderScope(candidateScope, () => + loadExtensionFromFactory( + createMcpExtension(candidateService), + candidateCwd, + createEventBus(), + createExtensionRuntime(), + "", + ), + ); + await attach(accepted, candidateCwd); + expect(candidateService.getNativeToolSearchSetting()).toBe(!active); + expect(isMcpNativeToolSearchEnabled()).toBe(!active); + expect(independent.runInAsyncScope(isMcpNativeToolSearchEnabled)).toBe(!active); + }); + } finally { + await Promise.all([liveService.dispose("quit"), candidateService.dispose("quit")]); + liveScope.close(); + candidateScope.close(); + independent.runInAsyncScope(() => installMcpNativeToolSearchGate(() => false)); + rmSync(root, { recursive: true, force: true }); + } + }, +); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts index 395e3fd8f0..fe8a3a865c 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; import { afterEach, expect, it, vi } from "vitest"; import { SessionManager } from "../../../src/core/session-manager.ts"; +import { SessionResumeConflictError } from "../../../src/core/session-resume-conflict.ts"; import * as reservations from "../../../src/core/session-write-reservation.ts"; const cleanups: Array<() => void> = []; @@ -89,3 +90,33 @@ it("releases the acceptance grant on cancellation without writing the target", ( expect(release).toHaveBeenCalledOnce(); expect(readFileSync(path, "utf8")).toBe(bytes); }); + +// PR #1473 ghN2H: awaited veto handlers can change the destination after the first grant check. +it.each(["normal", "legacy", "empty", "missing"])( + "PR1473 ghN2H: commit rejects a target changed after beginCommit (%s)", + (kind) => { + const { path, bytes } = fixture(); + if (kind === "legacy") writeFileSync(path, bytes.replace('"version":3', '"version":1')); + if (kind === "empty") writeFileSync(path, ""); + if (kind === "missing") rmSync(path); + const prepared = SessionManager.prepareOpen(path); + prepared.sessionManager.appendMessage(fauxAssistantMessage("candidate-only")); + const release = vi.fn(); + vi.spyOn(reservations, "reserveSessionWrite").mockReturnValue(release); + const acceptance = prepared.beginCommit(); + const changed = `${bytes}\n${JSON.stringify({ type: "session_info", id: "external", parentId: null, timestamp: new Date(0).toISOString(), name: "external-write" })}\n`; + writeFileSync(path, changed); + let failure: unknown; + try { + acceptance.commit(); + } catch (error) { + failure = error; + } + expect(failure).toBeInstanceOf(SessionResumeConflictError); + expect(failure).toMatchObject({ sessionFile: path }); + expect(readFileSync(path, "utf8")).toBe(changed); + expect(release).toHaveBeenCalledOnce(); + acceptance.rollback(); + expect(release).toHaveBeenCalledOnce(); + }, +); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts new file mode 100644 index 0000000000..47dfaec0d1 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts @@ -0,0 +1,176 @@ +import { appendFileSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { expect, it } from "vitest"; +import { SessionResumeConflictError } from "../../../src/core/session-resume-conflict.ts"; +import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; +import { RpcClient } from "../../../src/modes/rpc/rpc-client.ts"; +import { deadline, resumeRuntime } from "../issue-1473-runtime-support.ts"; +import { startWorkerHost } from "../rpc-worker-host-support.ts"; + +function resumeSurface(runtimeHost: unknown, cwd: string) { + const errors: string[] = []; + let fatals = 0; + let retries = 0; + const ctx = Object.assign(Object.create(InteractiveMode.prototype) as object, { + runtimeHost, + clearStatusIndicator() {}, + showStatus() {}, + showError(message: string) { + errors.push(message); + }, + async handleFatalRuntimeError(_prefix: string, error: unknown) { + fatals++; + throw error; + }, + async promptForMissingSessionCwd() { + retries++; + return cwd; + }, + createProjectTrustContext() { + return undefined; + }, + }); + const handle = Object.getOwnPropertyDescriptor(InteractiveMode.prototype, "handleResumeSession")?.value as ( + this: object, + path: string, + ) => Promise<{ cancelled: boolean }>; + return { resume: (path: string) => handle.call(ctx, path), errors, fatals: () => fatals, retries: () => retries }; +} + +// PR #1473 ghN2K/ghN2H: real candidate + awaited veto mutation, not a mocked switch rejection. +it.each(["direct", "cwd retry"])("PR1473 ghN2K: %s resume conflict remains recoverable", async (route) => { + const retry = route === "cwd retry"; + let target = ""; + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", async () => { + entered.resolve(); + await release.promise; + }); + }); + let attempt: Promise<{ cancelled: boolean }> | undefined; + try { + await host.runtime.session.prompt("persist live"); + const live = host.runtime.session; + target = join(host.cwd, "target.jsonl"); + const bytes = JSON.stringify({ + type: "session", + version: 1, + id: "target", + timestamp: new Date(0).toISOString(), + cwd: retry ? join(host.cwd, "gone") : host.cwd, + }); + writeFileSync(target, bytes); + const surface = resumeSurface(host.runtime, host.cwd); + attempt = surface.resume(target); + // Observe rejection immediately so a failed RED assertion cannot leave an unhandled promise. + const outcome = attempt.then( + (result) => ({ result }), + (error: unknown) => ({ error }), + ); + await deadline(entered.promise); + appendFileSync(target, "\n"); + release.resolve(); + expect(await deadline(outcome)).toEqual({ result: { cancelled: true } }); + expect(surface.fatals()).toBe(0); + expect(surface.retries()).toBe(retry ? 1 : 0); + expect(surface.errors).toHaveLength(1); + expect(readFileSync(target, "utf8")).toBe(`${bytes}\n`); + expect(host.runtime.session).toBe(live); + await live.prompt("follow-up after conflict"); + expect(live.messages.at(-1)).toMatchObject({ role: "assistant", content: [{ type: "text", text: "follow-up" }] }); + } finally { + release.resolve(); + if (attempt) + await attempt.catch((error: unknown) => { + expect(error).toBeInstanceOf(SessionResumeConflictError); + }); + await host.dispose(); + } +}); + +// PR #1473 ghN2K: a real isolate serializes via connection-handler; RpcClient consumes the socket reply. +it("PR1473 ghN2K: shared-host conflict retains typed identity", async () => { + const fauxModule = fileURLToPath(new URL("../../../../ai/src/providers/faux.ts", import.meta.url)); + const host = await startWorkerHost( + String.raw` + import { appendFileSync } from "node:fs"; + import { fauxProvider, fauxAssistantMessage } from ${JSON.stringify(fauxModule)}; + export default function(pi) { + const faux = fauxProvider({ provider: "pr1473", models: [{ id: "pr1473-faux" }] }); + faux.setResponses([fauxAssistantMessage("PR1473_FOLLOWUP")]); + pi.registerProvider(faux.provider); + pi.on("session_before_switch", async (event) => { + await Promise.resolve(); + appendFileSync(event.targetSessionFile, "\n"); + }); + } + `, + { socket: true }, + ); + const client = new RpcClient({ socketPath: join(host.scratch, "rpc.sock") }); + try { + const target = join(host.cwd, "target.jsonl"); + const bytes = JSON.stringify({ + type: "session", + version: 1, + id: "target", + timestamp: new Date(0).toISOString(), + cwd: host.cwd, + }); + writeFileSync(target, bytes); + await client.start(); + const opened = await client.openSession({ cwd: host.cwd, provider: "pr1473", modelId: "pr1473-faux" }); + const wire = await host.connect(); + const response = await wire.request({ type: "switch_session", sessionId: opened.sessionId, sessionPath: target }); + expect + .soft(response) + .toMatchObject({ success: false, errorCode: "session_resume_conflict", errorData: { sessionFile: target } }); + await expect(client.switchSession(target)).rejects.toBeInstanceOf(SessionResumeConflictError); + expect(readFileSync(target, "utf8")).toBe(`${bytes}\n\n`); + for (const retry of [false, true]) { + const uiTarget = join(host.cwd, retry ? "retry.jsonl" : "direct.jsonl"); + const uiBytes = JSON.stringify({ + type: "session", + version: 1, + id: "ui-target", + timestamp: new Date(0).toISOString(), + cwd: retry ? join(host.cwd, "gone") : host.cwd, + }); + writeFileSync(uiTarget, uiBytes); + const surface = resumeSurface(client, host.cwd); + expect(await surface.resume(uiTarget)).toEqual({ cancelled: true }); + expect(surface.errors).toHaveLength(1); + expect(surface.fatals()).toBe(0); + expect(surface.retries()).toBe(retry ? 1 : 0); + expect(readFileSync(uiTarget, "utf8")).toBe(`${uiBytes}\n`); + } + expect((await client.getState()).sessionId).toBe(opened.state.sessionId); + const settled = Promise.withResolvers(); + const unsubscribe = client.onEvent((event) => { + if (event.type === "agent_settled") settled.resolve(); + }); + try { + await client.prompt("follow-up after conflict", { sessionTitlePrompt: false }); + await deadline(settled.promise); + } finally { + unsubscribe(); + } + expect(JSON.stringify(await client.getMessages())).toContain("PR1473_FOLLOWUP"); + // A different worker can now acquire the changed destination: candidate-only rollback crossed IPC. + expect( + await wire.request({ + type: "open_session", + cwd: host.cwd, + sessionPath: target, + provider: "pr1473", + modelId: "pr1473-faux", + }), + ).toMatchObject({ success: true }); + } finally { + await client.stop(); + await host.dispose(); + } +}, 60_000); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts new file mode 100644 index 0000000000..c4fdce50f7 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts @@ -0,0 +1,83 @@ +import { symlinkSync } from "node:fs"; +import { join, relative } from "node:path"; +import { pathToFileURL } from "node:url"; +import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai/compat"; +import { Type } from "typebox"; +import { expect, it, vi } from "vitest"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { deadline, resumeRuntime } from "../issue-1473-runtime-support.ts"; + +// PR #1473 ghN2F: the identity guard must use the same normalization as opening the target. +it.each(["file URL", "tilde", "symlink"])( + "PR1473 ghN2F: normalized busy self-resume preserves the completing tool (%s)", + async (alias) => { + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let aborts = 0; + let vetoes = 0; + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", () => { + vetoes++; + }); + pi.registerTool({ + name: "block", + label: "Block", + description: "Wait for explicit release", + parameters: Type.Object({}), + execute: async (_id, _params, signal) => { + const onAbort = () => { + aborts++; + release.resolve(); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + started.resolve(); + try { + await release.promise; + return { content: [{ type: "text", text: "released" }], details: {} }; + } finally { + signal?.removeEventListener("abort", onAbort); + } + }, + }); + }); + let prompt: Promise | undefined; + try { + await host.runtime.session.prompt("persist source"); + const live = host.runtime.session; + const path = live.sessionFile!; + const link = join(host.cwd, "alias.jsonl"); + symlinkSync(path, link); + vi.stubEnv("HOME", host.cwd); + vi.stubEnv("USERPROFILE", host.cwd); + const input = + alias === "file URL" + ? pathToFileURL(path).href + : alias === "tilde" + ? `~/${relative(host.cwd, path)}` + : link; + host.faux.setResponses([ + fauxAssistantMessage(fauxToolCall("block", {}, { id: "completing-tool" }), { stopReason: "toolUse" }), + fauxAssistantMessage("complete"), + ]); + prompt = live.prompt("start tool"); + await deadline(started.promise); + expect(await host.runtime.switchSession(input)).toEqual({ cancelled: true }); + expect(host.runtime.session).toBe(live); + expect(host.factories()).toBe(1); + expect(vetoes).toBe(0); + expect(aborts).toBe(0); + release.resolve(); + await deadline(prompt); + expect(live.messages).toEqual(SessionManager.open(path).buildSessionContext().messages); + expect(live.messages.filter((m) => m.role === "toolResult")).toMatchObject([ + { toolCallId: "completing-tool", isError: false, content: [{ type: "text", text: "released" }] }, + ]); + } finally { + release.resolve(); + if (prompt) await deadline(prompt); + vi.unstubAllEnvs(); + await host.dispose(); + } + }, + 30_000, +); From 0672e9bf4b40f2f5d6930e9260b99018a9535822 Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 06:21:11 +0800 Subject: [PATCH 06/11] fix(sessions): veto resume before destination preparation Run the cancellable check before destination trust, snapshot and factories while retaining exact SDK admission before outgoing shutdown. Preserve active btw work on cancellation or rejection and keep accepted replacement cleanup in session_shutdown. Record the approved event contract, keep post-snapshot conflicts recoverable, and cover async veto writes plus work beginning during veto or preparation. Verified check/build, 10302 package tests, 12 real RPC scenarios and 16 xterm TUI scenarios. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/docs/extensions.md | 4 + packages/coding-agent/docs/rpc.md | 4 +- .../src/core/agent-session-runtime.ts | 22 +-- packages/coding-agent/src/core/changes.md | 24 +++ .../core/extensions/builtin/btw/changes.md | 18 ++ .../src/core/extensions/builtin/btw/index.ts | 4 - .../coding-agent/src/core/session-manager.ts | 4 +- .../test/suite/agent-session-runtime.test.ts | 15 +- .../test/suite/issue-1473-runtime-support.ts | 9 +- .../regressions/issue-1473-btw-resume.test.ts | 110 ++++++++++++ ...1473-cancelled-resume-reservations.test.ts | 6 +- ...issue-1473-deferred-legacy-content.test.ts | 9 +- .../issue-1473-resume-conflict.test.ts | 21 ++- .../issue-1473-resume-lifecycle.test.ts | 132 ++++++++++++++ .../issue-1473-resume-trust.test.ts | 170 ++++++++++++++++++ ...1473-upstream-compaction-admission.test.ts | 8 +- 17 files changed, 521 insertions(+), 41 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-btw-resume.test.ts create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-resume-trust.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0ba7ba0c33..3927bb6b40 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,7 +10,7 @@ ### Fixed -- Resume preflight checks the actual destination runtime's model budget before switch handlers or teardown, preserving the live session on rejection, including cwd-override retries and shared-host RPC. Cancelled candidates leave target files unchanged, release tentative writer reservations, and run their own MCP listener cleanup without shutting down the live service. +- Resume runs the cancellable before-switch check before destination trust, snapshot and factory preparation, then checks the exact destination SDK budget before outgoing shutdown. Vetoed resumes construct no candidates; cancelled and rejected resumes preserve active `/btw` work and the live session, including cwd-override retries and shared-host RPC. Rejected candidates leave target files unchanged and release their own registrations and tentative writer reservations without shutting down the live service. - Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). ### Removed diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 6912512f18..eae5e48d66 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -462,6 +462,10 @@ pi.on("session_info_changed", async (event, ctx) => { Fired before starting a new session (`/new`) or switching sessions (`/resume`). +This is a cancellable check, not a cleanup event. For `/resume`, it runs before reading the destination snapshot, resolving destination project trust, or constructing its runtime. Returning `{ cancel: true }` prevents all of that destination preparation. Writes completed by an awaited handler are included in the subsequent snapshot. + +After the veto accepts, senpi checks the actual destination model, settings, prompt and tools against SDK admission (including mandatory resume compaction when eligible), then acquires the writer grant and synchronously revalidates and persists the candidate. A later admission, missing-cwd, or conflict rejection can therefore follow this event without any replacement. Do not abort side work or release live resources here; `session_shutdown` runs only when an accepted replacement tears down the outgoing session. + ```typescript pi.on("session_before_switch", async (event, ctx) => { // event.reason - "new" or "resume" diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index 3e992d1ced..d9ef5e6331 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -1071,7 +1071,9 @@ A rejected switch answers with `success: false` plus a typed `errorCode` and str When compaction is enabled and the restored context fits the raw model window, a budget shortfall can instead be admitted with `resume_compaction_required`. The first prompt must complete the required compaction and satisfy the remaining budget before a normal provider turn. -Missing-cwd and budget rejections are decided before the switch lifecycle event and live-session teardown, so the current session stays usable. Resume conflicts are checked both before the switch lifecycle event and again after its awaited handlers, immediately before persistence. A conflict preserves the live session and any intervening target writes; the rejected candidate does not persist. A cancelled or rejected switch does not itself write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. +The order is: cancellable `session_before_switch` check, destination snapshot and trust/factory preparation, exact SDK budget admission, writer grant with synchronous final revalidation/persistence, then outgoing `session_shutdown` and replacement. A veto prevents destination trust prompts, trust persistence and factory execution. Writes completed by an awaited veto are included in the snapshot; writes during later factory preparation produce a recoverable conflict. + +Missing-cwd, budget and conflict rejections may follow the cancellable check, but never cause outgoing shutdown or invalidate the current session. Cleanup belongs in `session_shutdown`, not `session_before_switch`; active `/btw` work survives cancelled and rejected resumes. A conflict preserves any intervening target writes, and a cancelled or rejected switch does not itself write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. ```json { diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 2521d2e24f..0fc795ba42 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -77,9 +77,9 @@ function extractUserMessageText(content: string | Array<{ type: string; text?: s /** * Owns the current AgentSession plus its cwd-bound services. * - * Session replacement methods tear down the current runtime first, then create - * and apply the next runtime. If creation fails, the error is propagated to the - * caller. The caller is responsible for user-facing error handling. + * Resume checks the outgoing veto, then prepares and admits the destination + * before tearing down the live runtime. Other replacements create their next + * runtime after teardown. Callers handle propagated errors on their UI surface. */ export class AgentSessionRuntime { private rebindSession?: (session: AgentSession) => Promise; @@ -252,11 +252,16 @@ export class AgentSessionRuntime { canonicalizePath(resolvePath(sessionPath)) === canonicalizePath(resolvePath(previousSessionFile)); // Settling active work would append to this same file after taking the candidate snapshot. if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; + // This is a cancellable check, not cleanup: veto before destination reads, + // trust prompts or factories. Writes completed by the veto belong in the snapshot. + const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); + if (beforeResult.cancelled) return beforeResult; + if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; const prepared = SessionManager.prepareOpen(sessionPath, undefined, options?.cwdOverride); const { sessionManager } = prepared; assertSessionCwdExists(sessionManager, this.cwd); // Build and admit the actual destination, including its model selection, - // settings, prompt and tools. Persistence and switch lifecycle stay deferred. + // settings, prompt and tools. Persistence and destructive shutdown stay deferred. const result = await this.createRuntime({ cwd: sessionManager.getCwd(), agentDir: this.services.agentDir, @@ -267,13 +272,10 @@ export class AgentSessionRuntime { }); let acceptance: ReturnType | undefined; try { - // A denied writer grant must not run destructive before-switch handlers. - // The acceptance grant is reversible until this candidate becomes current. - acceptance = prepared.beginCommit(); - const beforeResult = await this.emitBeforeSwitch("resume", sessionPath); - if (beforeResult.cancelled) return beforeResult; - // Preparation and veto handlers can yield while a new turn starts on the live session. + // Preparation can yield while a new turn starts on the live session. if (isSelfResume && this.session.isSessionBusy) return { cancelled: true }; + // The grant is reversible; final revalidation and persistence do not yield. + acceptance = prepared.beginCommit(); acceptance.commit(); await this.teardownCurrent("resume", sessionManager.getSessionFile()); await this.apply(result); diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 0e3a6c8a3c..004db40547 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,27 @@ +## Approved veto-first resume lifecycle (2026-09-10) + +This approved order supersedes the historical admission-before-veto and no-before-switch-on-rejection statements below. The five staged-data, identity, conflict, ownership and MCP fixes remain intact. + +### What changed + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: after normalized busy-self rejection, await the cancellable before-switch check before the target snapshot, trust context and actual factory. Recheck busy self-resume after both awaited veto and preparation; retain exact SDK admission (including upstream mandatory compaction), candidate-only disposal, writer rollback and synchronous final revalidation/persistence before outgoing shutdown. +- `packages/coding-agent/src/core/session-manager.ts`: update acceptance comments to distinguish writer validation from the earlier cancellable veto; persistence and materialized retention are unchanged. + +### Why + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: cancellation previously prompted/persisted destination trust and executed factories; the old snapshot also rejected writes completed by an awaited veto. Before-switch is a check, never a cleanup commitment. Missing-cwd, budget and conflict rejection may follow it without outgoing shutdown. +- `packages/coding-agent/src/core/session-manager.ts`: callers must not interpret the grant as preceding the public veto; writes during factory preparation still conflict, and the final synchronous check protects direct prepared-writer callers too. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: only the replacement owner can place the veto before destination trust/factory execution while preserving authoritative SDK admission and live-session ownership. +- `packages/coding-agent/src/core/session-manager.ts`: prepared writer ordering is an internal persistence contract. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/agent-session-runtime.ts`: `switchSession` veto, busy guards, preparation and acceptance block. Fork behavior is unchanged. +- `packages/coding-agent/src/core/session-manager.ts`: `prepareOpen` acceptance comments only. + ## Preserve upstream resume-compaction admission (2026-09-09) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/btw/changes.md b/packages/coding-agent/src/core/extensions/builtin/btw/changes.md index 28006d0a7c..edd0b1a6be 100644 --- a/packages/coding-agent/src/core/extensions/builtin/btw/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/btw/changes.md @@ -1,5 +1,23 @@ # changes — btw +## Preserve side queries through resume checks (2026-09-10) + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/btw/index.ts`: remove the redundant `session_before_switch` dismiss. The existing `session_shutdown` handler remains the cleanup point for an accepted replacement; fork handling is unchanged. + +### Why + +- `packages/coding-agent/src/core/extensions/builtin/btw/index.ts`: before-switch is a cancellable check that now precedes exact destination admission, not a commitment to replace the session. Cancelled and truly budget-rejected resumes must leave the active side query running. This supersedes the historical before-switch abort statement below. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/extensions/builtin/btw/index.ts`: another extension cannot reverse this builtin's private AbortController or restore its dismissed widget. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/extensions/builtin/btw/index.ts`: lifecycle handler registration only; no new public event. + ## 2026-08-13 - Preserve provider-header deletion markers ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/btw/index.ts b/packages/coding-agent/src/core/extensions/builtin/btw/index.ts index 957c56b6fd..a8bde64b8a 100644 --- a/packages/coding-agent/src/core/extensions/builtin/btw/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/btw/index.ts @@ -26,10 +26,6 @@ export default function btwExtension(pi: ExtensionAPI) { if (current.panel) ctx.ui.setWidget(WIDGET_KEY, undefined); } - pi.on("session_before_switch", (_event, ctx) => { - dismiss(ctx, { abort: true }); - }); - pi.on("session_before_fork", (_event, ctx) => { dismiss(ctx, { abort: true }); }); diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 2d54533535..0fe9228869 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1908,7 +1908,7 @@ export class SessionManager { const file = sessionManager.getSessionFile(); if (!file) throw new Error("Prepared session is missing its destination file"); // Admission ran on a read-only snapshot. Revalidate the actual destination - // under its canonical host grant before any destructive switch handler. + // under its canonical host grant before destructive session shutdown. let release = reserveSessionWrite(file); const rollback = () => { release?.(); @@ -1930,7 +1930,7 @@ export class SessionManager { return { rollback, commit: () => { - // Veto handlers awaited since beginCommit may have changed the file. + // A prepared-writer caller may have changed the file since beginCommit. // Keep this final check and persistence synchronous, before live teardown. revalidate(); sessionManager.deferredPersistence = undefined; diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index b44a9c12fa..1606ae4998 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -800,7 +800,7 @@ describe("AgentSessionRuntime characterization", () => { await expect(runtime.switchSession(path)).rejects.toMatchObject({ projection: { model: `${faux.getModel().provider}/${forced}`, admission: "resume" }, }); - expect(events).toEqual([]); + expect(events).toEqual([{ type: "session_before_switch", reason: "resume", targetSessionFile: path }]); expect(shutdownSessions).toEqual([]); expect(shutdownSessions).not.toContain(original.sessionId); expect(readFileSync(path)).toEqual(bytes); @@ -857,7 +857,7 @@ describe("AgentSessionRuntime characterization", () => { return; } await expect(runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); - expect(events).toEqual([]); + expect(events).toEqual([{ type: "session_before_switch", reason: "resume", targetSessionFile: path }]); expect(runtime.session).toBe(original); await expect(original.prompt("still usable")).resolves.toBeUndefined(); }); @@ -884,7 +884,7 @@ describe("AgentSessionRuntime characterization", () => { const { runtime, tempDir } = await createRuntimeForTest( (pi) => { const candidate = factories++; - pi.on("session_before_switch", () => ({ cancel: true })); + pi.on("session_before_switch", () => ({ cancel: outcome === "cancelled" })); pi.on("session_shutdown", () => { shutdowns.push(candidate); }); @@ -920,7 +920,7 @@ describe("AgentSessionRuntime characterization", () => { expect(killAll).not.toHaveBeenCalled(); expect(isNativeBypass()).toBe(true); expect(shutdowns).toEqual([]); - expect(factories).toBe(attempt + 1); + expect(factories).toBe(outcome === "cancelled" ? 1 : attempt + 1); expect(listeners).toEqual(baseline); expect(service.getSnapshot()).toMatchObject({ disposed: false, @@ -1115,9 +1115,10 @@ describe("AgentSessionRuntime characterization", () => { await expect(runtime.switchSession(targetSessionFile!)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); - // Non-mutating preflight ran before the switch lifecycle event, so a rejected - // resume is a true no-op: no session_before_switch was ever emitted. - expect(emittedBeforeSwitch).toEqual([]); + // The approved veto-first contract permits a cancellable check, never live teardown. + expect(emittedBeforeSwitch).toEqual([ + { type: "session_before_switch", reason: "resume", targetSessionFile: targetSessionFile }, + ]); expect(runtime.session).toBe(originalSession); expect(runtime.session.sessionFile).toBe(originalSessionFile); expect(runtime.session.extensionRunner.isActive).toBe(true); diff --git a/packages/coding-agent/test/suite/issue-1473-runtime-support.ts b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts index 26e141b15b..3a90daecfc 100644 --- a/packages/coding-agent/test/suite/issue-1473-runtime-support.ts +++ b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts @@ -11,7 +11,10 @@ import { import type { ExtensionAPI } from "../../src/core/extensions/types.ts"; import { SessionManager } from "../../src/core/session-manager.ts"; -export async function resumeRuntime(extension: (pi: ExtensionAPI) => void = () => {}, contextWindow?: number) { +export async function resumeRuntime( + extension: (pi: ExtensionAPI) => void | Promise = () => {}, + contextWindow?: number, +) { const cwd = mkdtempSync(join(tmpdir(), "pr1473-runtime-")); const faux = registerFauxProvider( contextWindow ? { models: [{ id: "large", contextWindow, maxTokens: 1024 }] } : {}, @@ -28,7 +31,7 @@ export async function resumeRuntime(extension: (pi: ExtensionAPI) => void = () = noPromptTemplates: true, noThemes: true, extensionFactories: [ - (pi) => { + async (pi) => { const model = faux.getModel(); pi.registerProvider(model.provider, { baseUrl: model.baseUrl, @@ -45,7 +48,7 @@ export async function resumeRuntime(extension: (pi: ExtensionAPI) => void = () = maxTokens: m.maxTokens, })), }); - extension(pi); + await extension(pi); }, ], }, diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-btw-resume.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-btw-resume.test.ts new file mode 100644 index 0000000000..c0c387cf9e --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-btw-resume.test.ts @@ -0,0 +1,110 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { createAssistantMessageEventStream, fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; +import { expect, it } from "vitest"; +import { ModelUsabilityBudgetError } from "../../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { deadline, resumeRuntime } from "../issue-1473-runtime-support.ts"; + +// PR #1473: exercise builtin /btw through a real runtime provider, holding its stream by events. +it.each(["cancelled", "budget-rejected", "accepted"])( + "PR1473 btw: active query follows %s replacement", + async (outcome) => { + const events: string[] = []; + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", () => { + events.push("veto"); + return { cancel: outcome === "cancelled" }; + }); + pi.on("session_shutdown", () => { + events.push("shutdown"); + }); + }, 65_536); + const started = Promise.withResolvers(); + const aborted = Promise.withResolvers(); + const stream = createAssistantMessageEventStream(); + let aborts = 0; + let sideQuery: Promise | undefined; + try { + await host.runtime.session.prompt("persist source"); + const live = host.runtime.session; + const model = host.faux.getModel(); + await host.runtime.services.modelRuntime.registerProvider(model.provider, { + api: model.api, + apiKey: "faux-key", + baseUrl: model.baseUrl, + models: [model], + streamSimple: (_model, _context, options) => { + const signal = options?.signal; + if (!signal) throw new Error("Missing side-query cancellation signal"); + signal.addEventListener( + "abort", + () => { + aborts++; + stream.push({ + type: "error", + reason: "aborted", + error: fauxAssistantMessage("", { stopReason: "aborted" }), + }); + stream.end(); + aborted.resolve(); + }, + { once: true }, + ); + stream.push({ + type: "text_delta", + contentIndex: 0, + delta: "active", + partial: fauxAssistantMessage("active"), + }); + started.resolve(signal); + return stream; + }, + }); + const history = [...live.messages]; + sideQuery = live.prompt("/btw remain active"); + const signal = await deadline(started.promise); + const target = SessionManager.create(host.cwd, join(host.cwd, "targets")); + target.appendMessage({ + role: "user", + content: outcome === "budget-rejected" ? "x".repeat(70_000) : "valid", + timestamp: 0, + }); + target.appendMessage(fauxAssistantMessage("stored")); + const path = target.getSessionFile(); + if (!path) throw new Error("Missing target file"); + const bytes = readFileSync(path); + if (outcome === "budget-rejected") { + await expect(host.runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + } else { + expect(await host.runtime.switchSession(path)).toEqual({ cancelled: outcome === "cancelled" }); + } + if (outcome === "accepted") { + await deadline(aborted.promise); + expect(signal.aborted).toBe(true); + expect(aborts).toBe(1); + expect(events).toEqual(["veto", "shutdown"]); + expect(host.runtime.session).not.toBe(live); + } else { + expect.soft(signal.aborted).toBe(false); + expect.soft(aborts).toBe(0); + expect.soft(events).toEqual(["veto"]); + expect(host.runtime.session).toBe(live); + expect(readFileSync(path)).toEqual(bytes); + expect(live.messages).toEqual(history); + stream.push({ type: "done", reason: "stop", message: fauxAssistantMessage("completed side query") }); + stream.end(); + } + await deadline(sideQuery); + } finally { + stream.push({ type: "done", reason: "stop", message: fauxAssistantMessage("cleanup") }); + stream.end(); + try { + if (sideQuery) await deadline(sideQuery); + } finally { + await host.dispose(); + } + } + }, + 30_000, +); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts index 09c3e27e41..fb80824bf8 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts @@ -72,7 +72,7 @@ it.each(["unterminated", "legacy", "empty", "missing"])( // A factory mutates the target only to deterministically exercise the real acceptance failure path. it.each(["owned", "changed"])( - "rejects an %s acceptance before destructive switch handlers", + "rejects an %s acceptance after veto without destructive shutdown", async (failure) => { const host = await startWorkerHost(` import { appendFileSync, existsSync, unlinkSync, writeFileSync } from "node:fs"; @@ -82,6 +82,7 @@ it.each(["owned", "changed"])( appendFileSync("target.jsonl", "\\n"); } pi.on("session_before_switch", () => { writeFileSync("switch-fired", "1"); }); + pi.on("session_shutdown", () => { writeFileSync("shutdown-fired", "1"); }); } `); try { @@ -120,7 +121,8 @@ it.each(["owned", "changed"])( success: true, }); } - await expect(readFile(join(host.cwd, "switch-fired"))).rejects.toMatchObject({ code: "ENOENT" }); + expect(await readFile(join(host.cwd, "switch-fired"), "utf8")).toBe("1"); + await expect(readFile(join(host.cwd, "shutdown-fired"))).rejects.toMatchObject({ code: "ENOENT" }); expect(await host.request({ type: "get_state", sessionId: live.data?.sessionId })).toMatchObject({ success: true, data: { sessionId: live.data?.state?.sessionId }, diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts index 628323a1be..7a00e91520 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts @@ -107,12 +107,16 @@ it.each(cases)( ); // Actual SDK admission must reject the full large context; markers used to undercount it. -it("PR1473 ghN2N: legacy budget rejection emits no switch event and preserves bytes", async () => { +it("PR1473 ghN2N: legacy budget rejection emits veto without shutdown and preserves bytes", async () => { let vetoes = 0; + let shutdowns = 0; const host = await resumeRuntime((pi) => { pi.on("session_before_switch", () => { vetoes++; }); + pi.on("session_shutdown", () => { + shutdowns++; + }); }); try { const path = join(host.cwd, "rejected.jsonl"); @@ -120,7 +124,8 @@ it("PR1473 ghN2N: legacy budget rejection emits no switch event and preserves by const before = hash(readFileSync(path)); const live = host.runtime.session; await expect(host.runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); - expect(vetoes).toBe(0); + expect(vetoes).toBe(1); + expect(shutdowns).toBe(0); expect(host.runtime.session).toBe(live); expect(hash(readFileSync(path))).toBe(before); } finally { diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts index 47dfaec0d1..f9a2707548 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts @@ -38,17 +38,18 @@ function resumeSurface(runtimeHost: unknown, cwd: string) { return { resume: (path: string) => handle.call(ctx, path), errors, fatals: () => fatals, retries: () => retries }; } -// PR #1473 ghN2K/ghN2H: real candidate + awaited veto mutation, not a mocked switch rejection. +// PR #1473 ghN2K/ghN2H: mutate during actual factory preparation, AFTER the accepted veto/snapshot. it.each(["direct", "cwd retry"])("PR1473 ghN2K: %s resume conflict remains recoverable", async (route) => { const retry = route === "cwd retry"; let target = ""; const entered = Promise.withResolvers(); const release = Promise.withResolvers(); - const host = await resumeRuntime((pi) => { - pi.on("session_before_switch", async () => { + let factories = 0; + const host = await resumeRuntime(async () => { + if (factories++ > 0) { entered.resolve(); await release.promise; - }); + } }); let attempt: Promise<{ cancelled: boolean }> | undefined; try { @@ -96,15 +97,19 @@ it("PR1473 ghN2K: shared-host conflict retains typed identity", async () => { const fauxModule = fileURLToPath(new URL("../../../../ai/src/providers/faux.ts", import.meta.url)); const host = await startWorkerHost( String.raw` - import { appendFileSync } from "node:fs"; + import { appendFileSync, existsSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { fauxProvider, fauxAssistantMessage } from ${JSON.stringify(fauxModule)}; export default function(pi) { + if (existsSync("conflict-target")) { + const target = readFileSync("conflict-target", "utf8"); + unlinkSync("conflict-target"); + appendFileSync(target, "\n"); + } const faux = fauxProvider({ provider: "pr1473", models: [{ id: "pr1473-faux" }] }); faux.setResponses([fauxAssistantMessage("PR1473_FOLLOWUP")]); pi.registerProvider(faux.provider); - pi.on("session_before_switch", async (event) => { - await Promise.resolve(); - appendFileSync(event.targetSessionFile, "\n"); + pi.on("session_before_switch", (event) => { + writeFileSync("conflict-target", event.targetSessionFile); }); } `, diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts index c4fdce50f7..af76ed757c 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts @@ -7,6 +7,138 @@ import { expect, it, vi } from "vitest"; import { SessionManager } from "../../../src/core/session-manager.ts"; import { deadline, resumeRuntime } from "../issue-1473-runtime-support.ts"; +// PR #1473 ghN2H: a veto is allowed to finish writing before the destination is snapshotted. +it("PR1473 ghN2H: resume observes writes completed by its awaited veto", async () => { + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + let path = ""; + const host = await resumeRuntime((pi) => { + pi.on("session_before_switch", async () => { + entered.resolve(); + await release.promise; + SessionManager.open(path).appendMessage({ role: "user", content: "VETO_WRITE", timestamp: 1 }); + }); + }); + let attempt: Promise<{ cancelled: boolean }> | undefined; + try { + const target = SessionManager.create(host.cwd, join(host.cwd, "targets")); + target.appendMessage(fauxAssistantMessage("stored")); + const file = target.getSessionFile(); + if (!file) throw new Error("Missing target file"); + path = file; + attempt = host.runtime.switchSession(path); + const outcome = attempt.then( + (result) => ({ result }), + (error: unknown) => ({ error }), + ); + await deadline(entered.promise); + release.resolve(); + expect(await deadline(outcome)).toEqual({ result: { cancelled: false } }); + expect(host.runtime.session.messages).toContainEqual({ role: "user", content: "VETO_WRITE", timestamp: 1 }); + expect(host.runtime.session.messages).toEqual(SessionManager.open(path).buildSessionContext().messages); + } finally { + release.resolve(); + if (attempt) await Promise.allSettled([attempt]); + await host.dispose(); + } +}, 30_000); + +// PR #1473: both awaited boundaries must recheck self-resume before persistence or teardown. +it.each(["veto", "factory"])( + "PR1473 busy: self-resume preserves work started during %s", + async (stage) => { + const entered = Promise.withResolvers(); + const releasePreparation = Promise.withResolvers(); + const toolStarted = Promise.withResolvers(); + const releaseTool = Promise.withResolvers(); + let factories = 0; + let shutdowns = 0; + let aborts = 0; + const host = await resumeRuntime(async (pi) => { + if (factories++ > 0 && stage === "factory") { + entered.resolve(); + await releasePreparation.promise; + } + pi.on("session_before_switch", async () => { + if (stage === "veto") { + entered.resolve(); + await releasePreparation.promise; + } + }); + pi.on("session_shutdown", () => { + shutdowns++; + }); + pi.registerTool({ + name: "block", + label: "Block", + description: "Wait for explicit release", + parameters: Type.Object({}), + execute: async (_id, _params, signal) => { + const onAbort = () => { + aborts++; + releaseTool.resolve(); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + toolStarted.resolve(); + try { + await releaseTool.promise; + return { content: [{ type: "text", text: "BUSY_RESULT" }], details: {} }; + } finally { + signal?.removeEventListener("abort", onAbort); + } + }, + }); + }); + let prompt: Promise | undefined; + let attempt: Promise<{ cancelled: boolean }> | undefined; + try { + await host.runtime.session.prompt("persist source"); + const live = host.runtime.session; + const path = live.sessionFile; + if (!path) throw new Error("Missing source file"); + attempt = host.runtime.switchSession(path); + const result = attempt.then( + (value) => ({ value }), + (error: unknown) => ({ error }), + ); + await deadline(entered.promise); + host.faux.setResponses([ + fauxAssistantMessage(fauxToolCall("block", {}, { id: "busy-call" }), { stopReason: "toolUse" }), + fauxAssistantMessage("complete"), + ]); + prompt = live.prompt("start tool during resume"); + await deadline(toolStarted.promise); + releasePreparation.resolve(); + expect(await deadline(result)).toEqual({ value: { cancelled: true } }); + expect(host.runtime.session).toBe(live); + expect(factories).toBe(stage === "veto" ? 1 : 2); + expect(shutdowns).toBe(0); + expect(aborts).toBe(0); + releaseTool.resolve(); + await deadline(prompt); + expect(live.messages).toEqual(SessionManager.open(path).buildSessionContext().messages); + expect(live.messages).toContainEqual( + expect.objectContaining({ + role: "toolResult", + toolCallId: "busy-call", + isError: false, + content: [{ type: "text", text: "BUSY_RESULT" }], + }), + ); + } finally { + releasePreparation.resolve(); + releaseTool.resolve(); + try { + if (attempt) await deadline(Promise.allSettled([attempt])); + if (prompt) await deadline(prompt); + } finally { + await host.dispose(); + } + } + }, + 30_000, +); + // PR #1473 ghN2F: the identity guard must use the same normalization as opening the target. it.each(["file URL", "tilde", "symlink"])( "PR1473 ghN2F: normalized busy self-resume preserves the completing tool (%s)", diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-resume-trust.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-resume-trust.test.ts new file mode 100644 index 0000000000..b2aed8d382 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-trust.test.ts @@ -0,0 +1,170 @@ +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { fauxAssistantMessage, fauxProvider } from "@earendil-works/pi-ai/compat"; +import { expect, it } from "vitest"; +import { parseArgs } from "../../../src/cli/args.ts"; +import { createAgentSessionRuntime } from "../../../src/core/agent-session-runtime.ts"; +import type { ProjectTrustContext } from "../../../src/core/extensions/types.ts"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { ProjectTrustStore } from "../../../src/core/trust-manager.ts"; +import { createCliRuntimeFactory } from "../../../src/main.ts"; + +// PR #1473 gdAkN: exercise the CLI's real trust/resource/factory wiring, not a trust mock. +it.each([true, false])( + "PR1473 gdAkN: destination trust and factories respect veto (cancel=%s)", + async (cancel) => { + const root = mkdtempSync(join(tmpdir(), "pr1473-trust-")); + const cwd = join(root, "source"); + const destination = join(root, "destination"); + const agentDir = join(root, "agent"); + const extensions = join(destination, ".senpi", "extensions"); + for (const dir of [cwd, agentDir, extensions]) mkdirSync(dir, { recursive: true }); + const marker = join(destination, "factory-ran"); + const fauxModule = fileURLToPath(new URL("../../../../ai/src/providers/faux.ts", import.meta.url)); + writeFileSync( + join(extensions, "destination.js"), + `import { appendFileSync } from "node:fs"; +import { fauxProvider, fauxAssistantMessage, fauxToolCall } from ${JSON.stringify(fauxModule)}; +export default function(pi) { + appendFileSync(${JSON.stringify(marker)}, "factory\\n"); + const faux = fauxProvider({ provider: "pr1473-destination", models: [{ id: "destination-model" }] }); + faux.setResponses([ + fauxAssistantMessage(fauxToolCall("destination_tool", {}, { id: "destination-call" }), { stopReason: "toolUse" }), + fauxAssistantMessage("DESTINATION_PROVIDER_OK") + ]); + pi.registerProvider(faux.provider); + pi.registerTool({ name: "destination_tool", label: "Destination", description: "Return destination sentinel", + parameters: { type: "object", properties: {}, required: [] }, + execute: async () => ({ content: [{ type: "text", text: "DESTINATION_TOOL_OK" }], details: {} }) }); +}`, + ); + writeFileSync( + join(destination, ".senpi", "settings.json"), + JSON.stringify({ defaultProvider: "pr1473-destination", defaultModel: "destination-model" }), + ); + const faux = fauxProvider({ provider: "pr1473-source", models: [{ id: "source-model" }] }); + faux.setResponses([fauxAssistantMessage("SOURCE_OK"), fauxAssistantMessage("SOURCE_FOLLOWUP")]); + writeFileSync( + join(agentDir, "settings.json"), + JSON.stringify({ defaultProvider: "pr1473-source", defaultModel: "source-model" }), + ); + let factories = 0; + const events: string[] = []; + const factory = createCliRuntimeFactory( + { + cwd, + agentDir, + appMode: "interactive", + parsed: parseArgs(["--no-skills", "--no-prompt-templates", "--no-themes", "--no-context-files"]), + }, + { + extensionFactories: [ + (pi) => { + pi.registerProvider(faux.provider); + pi.on("session_before_switch", () => { + events.push("veto"); + return { cancel }; + }); + pi.on("session_shutdown", () => { + events.push("shutdown"); + }); + }, + ], + }, + ); + const runtime = await createAgentSessionRuntime( + async (options) => { + factories++; + return factory(options); + }, + { cwd, agentDir, sessionManager: SessionManager.create(cwd, join(root, "sessions")) }, + ); + let prompts = 0; + let contexts = 0; + const trustContext = (targetCwd: string): ProjectTrustContext => { + contexts++; + return { + cwd: targetCwd, + mode: "tui", + hasUI: true, + ui: { + select: async (_title, options) => { + prompts++; + return options[0]; + }, + confirm: async () => false, + input: async () => undefined, + notify: () => {}, + }, + }; + }; + try { + await runtime.session.bindExtensions({}); + runtime.setRebindSession(async (session) => { + await session.bindExtensions({}); + }); + expect(runtime.diagnostics.filter((entry) => entry.type === "error")).toEqual([]); + await runtime.session.prompt("persist source", { sessionTitlePrompt: false }); + const live = runtime.session; + const parent = live.sessionFile; + if (!parent) throw new Error("Missing source file"); + const parentBytes = readFileSync(parent); + const target = SessionManager.create(destination, join(root, "targets")); + target.newSession({ parentSession: parent }); + target.appendMessage({ role: "user", content: "destination history", timestamp: 0 }); + target.appendMessage({ + ...fauxAssistantMessage("stored"), + provider: "pr1473-destination", + model: "destination-model", + }); + const path = target.getSessionFile(); + if (!path) throw new Error("Missing destination file"); + const bytes = readFileSync(path); + expect(await runtime.switchSession(path, { projectTrustContextFactory: trustContext })).toEqual({ + cancelled: cancel, + }); + if (cancel) { + expect.soft(prompts).toBe(0); + expect.soft(contexts).toBe(0); + expect.soft(factories).toBe(1); + expect.soft(existsSync(marker)).toBe(false); + expect.soft(existsSync(join(agentDir, "trust.json"))).toBe(false); + expect.soft(new ProjectTrustStore(agentDir).get(destination)).toBe(null); + expect(events).toEqual(["veto"]); + expect(readFileSync(path)).toEqual(bytes); + expect(runtime.session).toBe(live); + await live.prompt("still here", { sessionTitlePrompt: false }); + expect(live.messages.at(-1)).toMatchObject({ content: [{ type: "text", text: "SOURCE_FOLLOWUP" }] }); + } else { + expect(prompts).toBe(1); + expect(contexts).toBe(1); + expect(factories).toBe(2); + expect(readFileSync(marker, "utf8")).toBe("factory\n"); + expect(new ProjectTrustStore(agentDir).get(destination)).toBe(true); + expect(events).toEqual(["veto", "shutdown"]); + expect(runtime.session.model).toMatchObject({ provider: "pr1473-destination", id: "destination-model" }); + expect(runtime.session.getActiveToolNames()).toContain("destination_tool"); + expect(runtime.diagnostics.filter((entry) => entry.type === "error")).toEqual([]); + await runtime.session.prompt("use destination tool", { sessionTitlePrompt: false }); + expect(runtime.session.messages).toContainEqual( + expect.objectContaining({ + role: "toolResult", + toolName: "destination_tool", + isError: false, + content: [{ type: "text", text: "DESTINATION_TOOL_OK" }], + }), + ); + expect(runtime.session.messages.at(-1)).toMatchObject({ + content: [{ type: "text", text: "DESTINATION_PROVIDER_OK" }], + }); + expect(readFileSync(parent)).toEqual(parentBytes); + } + } finally { + await runtime.dispose(); + rmSync(root, { recursive: true, force: true }); + } + }, + 60_000, +); diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-upstream-compaction-admission.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-upstream-compaction-admission.test.ts index f17faa4e21..c1e9e63414 100644 --- a/packages/coding-agent/test/suite/regressions/issue-1473-upstream-compaction-admission.test.ts +++ b/packages/coding-agent/test/suite/regressions/issue-1473-upstream-compaction-admission.test.ts @@ -13,10 +13,14 @@ it.each([ { name: "disabled compaction", size: 60_000, enabled: false, accepted: false }, ])("PR1473 upstream: preserves admission for $name", async ({ size, enabled, accepted }) => { let vetoes = 0; + let shutdowns = 0; const host = await resumeRuntime((pi) => { pi.on("session_before_switch", () => { vetoes++; }); + pi.on("session_shutdown", () => { + shutdowns++; + }); }, 65_536); try { writeFileSync(join(host.cwd, "settings.json"), JSON.stringify({ compaction: { enabled } })); @@ -42,10 +46,12 @@ it.each([ }), ); expect(vetoes).toBe(1); + expect(shutdowns).toBe(1); } else { await expect(host.runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); expect(host.runtime.session).toBe(original); - expect(vetoes).toBe(0); + expect(vetoes).toBe(1); + expect(shutdowns).toBe(0); expect(readFileSync(path)).toEqual(before); } } finally { From 36c4da66ab8e9060a3432058d579a1fddeee86aa Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 09:23:19 +0800 Subject: [PATCH 07/11] fix(tool-search): defer scoped catalog installation until start Match local catalog ownership by installing provider-scoped catalogs only at session_start. Discarded candidates no longer replace live lazy-tool activation or native-injection diagnostics. Verified two faithful RED failures and three GREEN cases, 90 related controls, check/build, 10305 package tests, actual Node API behavior and both shared-worker TUI recovery paths. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 1 + .../extensions/builtin/tool-search/changes.md | 19 ++++ .../extensions/builtin/tool-search/index.ts | 4 +- .../extensions/builtin/tool-search/service.ts | 2 +- .../issue-1473-tool-search-candidate.test.ts | 92 +++++++++++++++++++ 5 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-tool-search-candidate.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3927bb6b40..968f33ef5c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixed +- Keep provider-scoped tool-search catalogs bound to the active session until a resumed session starts, preserving lazy-tool activation and native-request diagnostics after discarded candidates ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). - Resume runs the cancellable before-switch check before destination trust, snapshot and factory preparation, then checks the exact destination SDK budget before outgoing shutdown. Vetoed resumes construct no candidates; cancelled and rejected resumes preserve active `/btw` work and the live session, including cwd-override retries and shared-host RPC. Rejected candidates leave target files unchanged and release their own registrations and tentative writer reservations without shutting down the live service. - Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md index 6d9bd377a3..245f84dc2d 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md @@ -1,5 +1,24 @@ # Tool Search Builtin Changes +## 2026-09-10 - Keep discarded candidates out of scoped tool-search state + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: publish provider-scoped catalogs on `session_start`, matching local catalog installation. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts`: document the accepted-session installation boundary. + +### Why + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: candidate construction previously replaced the current async tool-search store before acceptance. Invalidating a rejected candidate left live lazy-tool activation and native diagnostics pointing at that candidate. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: the builtin owns scoped catalog installation before another extension can recover the live store. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: factory installation and session-start registration order. + ## 2026-09-08 - Keep rejected resume candidates out of the live catalog (PR #1473) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts index 3f125f346c..8a5ac8d3b8 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts @@ -70,10 +70,10 @@ export default function toolSearchExtension(pi: ExtensionAPI): void | Promise pi.setActiveTools([...names]), }; const sessionOwned = hasProviderScope(); - // A prepared resume is not the active local runtime yet. Its callbacks must + // A prepared resume is not the active runtime yet. Its callbacks must // never rebind the live catalog to an extension generation that may be rejected. const service = new ToolSearchService(runtime); - if (sessionOwned) installScopedToolSearchService(service); + if (sessionOwned) pi.on("session_start", () => installScopedToolSearchService(service)); else pi.on("session_start", () => installLocalToolSearchService(service)); return createToolSearchExtension(service)(pi); } diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts index a3a38a8c1d..747fa6d3e2 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts @@ -226,7 +226,7 @@ export function installLocalToolSearchService(value: ToolSearchService): void { service = value; } -/** Make a session-owned service visible to later builtins loaded in the same provider scope. */ +/** Publish the accepted session's catalog in the current async provider scope. */ export function installScopedToolSearchService(value: ToolSearchService): void { scopedService.enterWith(value); } diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-tool-search-candidate.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-tool-search-candidate.test.ts new file mode 100644 index 0000000000..c390f111d8 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-tool-search-candidate.test.ts @@ -0,0 +1,92 @@ +import { AsyncResource } from "node:async_hooks"; +import { ProviderScope, runWithProviderScope } from "@earendil-works/pi-ai/node/provider-scope"; +import { Type } from "typebox"; +import { expect, it } from "vitest"; +import { createEventBus } from "../../../src/core/event-bus.ts"; +import toolSearchExtension from "../../../src/core/extensions/builtin/tool-search/index.ts"; +import { getToolSearchService } from "../../../src/core/extensions/builtin/tool-search/service.ts"; +import { createExtensionRuntime, loadExtensionFromFactory } from "../../../src/core/extensions/loader.ts"; +import type { Extension, ToolInfo } from "../../../src/core/extensions/types.ts"; + +function toolRuntime(name: string) { + const runtime = createExtensionRuntime(); + let active: string[] = []; + runtime.getAllTools = (): ToolInfo[] => [ + { + name, + label: name, + description: `${name} searchable tool`, + parameters: Type.Object({}), + sourceInfo: { path: `/test/${name}.ts`, source: "test", scope: "temporary", origin: "top-level" }, + exposure: "search", + searchKeywords: [], + allowLazyActivation: true, + }, + ]; + runtime.getActiveTools = () => [...active]; + runtime.setActiveTools = (names) => { + active = [...names]; + }; + return runtime; +} + +async function attach(extension: Extension) { + for (const handler of extension.handlers.get("session_start") ?? []) { + await handler({ type: "session_start", reason: "resume" }, { sessionManager: { getEntries: () => [] } }); + } +} + +// PR1473 g4Vu9: actual factory, scoped service, lazy activation and native diagnostics. +it.each(["discarded catalog", "discarded diagnostic", "accepted"])( + "PR1473 g4Vu9: %s retains the correct tool-search owner", + async (probe) => { + const resource = new AsyncResource("pr1473-tool-search"); + const scope = new ProviderScope(); + const liveRuntime = toolRuntime("live_hidden"); + const candidateRuntime = toolRuntime("candidate_hidden"); + try { + await resource.runInAsyncScope(() => + runWithProviderScope(scope, async () => { + const live = await loadExtensionFromFactory( + toolSearchExtension, + process.cwd(), + createEventBus(), + liveRuntime, + "", + ); + await attach(live); + const liveService = getToolSearchService(); + expect(liveService.activateTool("live_hidden")).toBe(true); + liveRuntime.setActiveTools([]); + liveService.noteNativeInjectionFailure("live-native-failure"); + + const candidate = await loadExtensionFromFactory( + toolSearchExtension, + process.cwd(), + createEventBus(), + candidateRuntime, + "", + ); + if (probe === "accepted") { + await attach(candidate); + expect(getToolSearchService().activateTool("candidate_hidden")).toBe(true); + expect(candidateRuntime.getActiveTools()).toContain("candidate_hidden"); + expect(getToolSearchService().activateTool("live_hidden")).toBe(false); + } else { + candidateRuntime.invalidate(); + if (probe === "discarded catalog") { + expect(getToolSearchService().activateTool("live_hidden")).toBe(true); + expect(liveRuntime.getActiveTools()).toContain("live_hidden"); + } else { + expect(getToolSearchService().takeNativeInjectionFailure()).toBe("live-native-failure"); + } + } + }), + ); + } finally { + liveRuntime.invalidate(); + candidateRuntime.invalidate(); + resource.emitDestroy(); + } + }, +); From be41e0df4a7f7a187f917da76ca29db641a45e21 Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 09:54:27 +0800 Subject: [PATCH 08/11] docs(rpc): align protocol overview with switch ordering Describe the cancellable handler before candidate snapshot and admission, and final revalidation before outgoing shutdown. This matches switchSession and the command-specific protocol section; no behavior or JSON examples change. Verified by source/document comparison and git diff --check. Code at parent 36c4da66a has green full local and GitHub CI; no prose-pinning test is added. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/docs/rpc.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index d9ef5e6331..ddf0030d0b 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -212,8 +212,9 @@ and only then constructs its session writer and runtime. A conflicting alias att explicitly before opening another writer. SessionManager writes, switches, forks, new sessions and imports obtain the same grant before writer creation or -append-side normalization. Resume candidate preparation acquires no writer reservation. After exact admission, -acceptance obtains a reversible grant and revalidates the destination before switch handlers; cancellation or failure +append-side normalization. Resume runs cancellable `session_before_switch` handlers before opening the destination +snapshot and preparing the candidate, which acquires no writer reservation. After exact admission, acceptance obtains +a reversible grant and revalidates the destination before outgoing shutdown; cancellation or failure releases only a newly acquired candidate grant, leaving existing live ownership intact. Accepted writer paths are conservatively retained for that worker's entire lifetime, including superseded paths after a switch. Each worker may reserve at most 64 paths; an exhausted reservation budget fails From bee8f7b2f5a573a96aeb99696b8f4fb904ee52af Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 11:46:07 +0800 Subject: [PATCH 09/11] fix(sessions): bound prepared resume fingerprints Stream content fingerprints instead of retaining a second full transcript buffer. Preserve same-size content conflict detection, metadata-only acceptance, synchronous writer checks and large legacy content. Verified faithful retention RED, 21 focused tests, a 65 MiB Node API probe, full check/build, 10311 package tests and the current RPC/TUI matrix. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/src/core/changes.md | 18 +++++ .../coding-agent/src/core/session-manager.ts | 28 +++++-- .../issue-1473-snapshot-retention.test.ts | 79 +++++++++++++++++++ 4 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-snapshot-retention.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 968f33ef5c..2a350f88a1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,7 +12,7 @@ - Keep provider-scoped tool-search catalogs bound to the active session until a resumed session starts, preserving lazy-tool activation and native-request diagnostics after discarded candidates ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). - Resume runs the cancellable before-switch check before destination trust, snapshot and factory preparation, then checks the exact destination SDK budget before outgoing shutdown. Vetoed resumes construct no candidates; cancelled and rejected resumes preserve active `/btw` work and the live session, including cwd-override retries and shared-host RPC. Rejected candidates leave target files unchanged and release their own registrations and tentative writer reservations without shutting down the live service. -- Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). +- Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated with bounded content fingerprints before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). ### Removed diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 004db40547..eb4ab6579d 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,21 @@ +## Bounded prepared-resume content fingerprints (2026-09-10) + +### What changed + +- `packages/coding-agent/src/core/session-manager.ts`: retain streamed SHA256 fingerprints rather than full-file buffers for prepared-writer conflict checks. Preparation and both synchronous acceptance checks use bounded scratch space; absent files remain distinct from empty files. + +### Why + +- `packages/coding-agent/src/core/session-manager.ts`: retaining a second complete transcript during destination construction defeats bounded resident storage. Content fingerprints still detect same-size edits with unchanged timestamps and permit metadata-only changes. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/session-manager.ts`: deferred snapshots, writer grants and final content validation belong to the internal persistence boundary. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/session-manager.ts`: deferred snapshot values, `_reserveWrite`, and `prepareOpen` revalidation. Materialized history, migration, grant rollback and persistence ordering are unchanged. + ## Approved veto-first resume lifecycle (2026-09-10) This approved order supersedes the historical admission-before-veto and no-before-switch-on-rejection statements below. The five staged-data, identity, conflict, ownership and MCP fixes remain intact. diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 0fe9228869..abdcaaa8ed 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1,6 +1,6 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core"; import type { ImageContent, Message, TextContent, ThinkingSelection, Usage } from "@earendil-works/pi-ai"; -import { randomBytes, randomUUID } from "crypto"; +import { createHash, randomBytes, randomUUID } from "crypto"; import { appendFileSync, closeSync, @@ -824,6 +824,24 @@ export function setSessionEntryLoaderForTesting(loader: typeof loadEntriesFromFi }; } +/** Fingerprint exact file bytes without retaining another transcript-sized buffer. */ +function readSessionFingerprint(path: string): string | undefined { + if (!existsSync(path)) return undefined; + const fd = openSync(path, "r"); + try { + const hash = createHash("sha256"); + const buffer = Buffer.allocUnsafe(64 * 1024); + while (true) { + const bytesRead = readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead === 0) break; + hash.update(buffer.subarray(0, bytesRead)); + } + return hash.digest("hex"); + } finally { + closeSync(fd); + } +} + export class SessionManager { private sessionId: string = ""; private sessionFile: string | undefined; @@ -833,7 +851,7 @@ export class SessionManager { private deferredPersistence?: { rewrite: boolean; entries: SessionEntry[]; - snapshots: Map; + snapshots: Map; }; private flushed: boolean = false; private fileEntries: FileEntry[] = []; @@ -920,7 +938,7 @@ export class SessionManager { private _reserveWrite(path: string): void { if (this.deferredPersistence) { if (!this.deferredPersistence.snapshots.has(path)) { - this.deferredPersistence.snapshots.set(path, existsSync(path) ? readFileSync(path) : undefined); + this.deferredPersistence.snapshots.set(path, readSessionFingerprint(path)); } return; } @@ -1917,8 +1935,8 @@ export class SessionManager { const revalidate = () => { try { const expected = pending?.snapshots.get(file); - const current = existsSync(file) ? readFileSync(file) : undefined; - if (expected === undefined ? current !== undefined : !current?.equals(expected)) { + const current = readSessionFingerprint(file); + if (current !== expected) { throw new SessionResumeConflictError(file); } } catch (error) { diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-snapshot-retention.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-snapshot-retention.test.ts new file mode 100644 index 0000000000..b3e2b8c266 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-snapshot-retention.test.ts @@ -0,0 +1,79 @@ +import { mkdtempSync, readFileSync, rmSync, statSync, utimesSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { serialize } from "node:v8"; +import { afterEach, expect, it } from "vitest"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { SessionResumeConflictError } from "../../../src/core/session-resume-conflict.ts"; + +const roots: string[] = []; +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +function fixture(text: string) { + const cwd = mkdtempSync(join(tmpdir(), "pr1473-snapshot-")); + roots.push(cwd); + const path = join(cwd, "target.jsonl"); + const bytes = [ + JSON.stringify({ type: "session", version: 3, id: "target", timestamp: new Date(0).toISOString(), cwd }), + JSON.stringify({ + type: "message", + id: "message", + parentId: null, + timestamp: new Date(0).toISOString(), + message: { role: "user", content: text, timestamp: 0 }, + }), + "", + ].join("\n"); + writeFileSync(path, bytes); + return { path, bytes }; +} + +// PR #1473 g6BJr: conflict metadata must not retain a second transcript-sized representation. +it("keeps prepared snapshot metadata bounded when the transcript is large", () => { + // Given a real transcript substantially larger than the snapshot metadata budget. + const { path, bytes } = fixture("payload ".repeat(256 * 1024)); + + // When the actual manager stages that transcript without accepting it. + const prepared = SessionManager.prepareOpen(path); + const snapshots = prepared.sessionManager["deferredPersistence"]?.snapshots; + + // Then retained conflict metadata is bounded independently of transcript content. + expect(snapshots).toBeDefined(); + expect(serialize(snapshots).byteLength).toBeLessThan(4096); + expect(readFileSync(path, "utf8")).toBe(bytes); +}); + +// PR #1473: bounded fingerprints must retain byte-level, not just stat-level, conflict detection. +it("rejects same-size changed content even when the original modification time is restored", () => { + // Given an accepted writer grant for a staged file. + const { path, bytes } = fixture("before"); + const original = statSync(path); + const prepared = SessionManager.prepareOpen(path); + const acceptance = prepared.beginCommit(); + const changed = bytes.replace('"content":"before"', '"content":"after!"'); + expect(Buffer.byteLength(changed)).toBe(Buffer.byteLength(bytes)); + + // When an external writer changes content without changing length or final mtime. + writeFileSync(path, changed); + utimesSync(path, original.atime, original.mtime); + + // Then commit rejects and preserves the external bytes. + expect(() => acceptance.commit()).toThrow(SessionResumeConflictError); + expect(readFileSync(path, "utf8")).toBe(changed); +}); + +// PR #1473: metadata-only changes are not content conflicts. +it("accepts identical content after only file timestamps change", () => { + // Given a staged, unchanged transcript. + const { path, bytes } = fixture("unchanged"); + const prepared = SessionManager.prepareOpen(path); + + // When only timestamps change before acceptance. + utimesSync(path, new Date(0), new Date(1000)); + prepared.beginCommit().commit(); + + // Then acceptance preserves the complete original bytes. + expect(readFileSync(path, "utf8")).toBe(bytes); +}); From 7a686a02882735003191d581e626d5596e0b54f3 Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 11:47:44 +0800 Subject: [PATCH 10/11] fix(tool-search): retain the SDK session catalog owner Carry the existing lazy activator service into its owning AgentSession for unknown-tool resolution and native-injection diagnostics. Direct SDK sessions work without session_start while discarded candidates remain out of global catalogs. Verified three faithful RED cases, 101 focused tests, actual Node SDK dispatch and native recovery, full check/build, 10311 package tests and the final RPC/TUI matrix. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/core/agent-session.ts | 13 +- packages/coding-agent/src/core/changes.md | 18 ++ .../extensions/builtin/tool-search/changes.md | 20 ++ .../extensions/builtin/tool-search/index.ts | 9 +- .../extensions/builtin/tool-search/service.ts | 16 +- .../issue-1473-sdk-tool-search-owner.test.ts | 178 ++++++++++++++++++ 7 files changed, 249 insertions(+), 7 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/issue-1473-sdk-tool-search-owner.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2a350f88a1..6a80d97e11 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,7 +10,7 @@ ### Fixed -- Keep provider-scoped tool-search catalogs bound to the active session until a resumed session starts, preserving lazy-tool activation and native-request diagnostics after discarded candidates ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). +- Keep provider-scoped tool-search catalogs bound to the active session until a resumed session starts, preserving lazy-tool activation and native-request diagnostics after discarded candidates. Direct SDK sessions retain their own catalog and diagnostics without requiring extension startup or taking another session's global ownership ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). - Resume runs the cancellable before-switch check before destination trust, snapshot and factory preparation, then checks the exact destination SDK budget before outgoing shutdown. Vetoed resumes construct no candidates; cancelled and rejected resumes preserve active `/btw` work and the live session, including cwd-override retries and shared-host RPC. Rejected candidates leave target files unchanged and release their own registrations and tentative writer reservations without shutting down the live service. - Staged resumes preserve large legacy transcript content through migration, keep the active MCP native-search setting until attachment, and recognize file-URL and tilde aliases for busy self-resumes. Concurrent destination changes are revalidated with bounded content fingerprints before persistence and reported as recoverable errors in the TUI and RPC ([#1473](https://github.com/code-yeongyu/senpi/pull/1473) by [@Tinycute00](https://github.com/Tinycute00)). diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 1524773b3c..63724cdfb7 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -127,7 +127,11 @@ import { import { WAKE_SOURCE_STATE_EVENT } from "./extensions/builtin/monitor-state-event.ts"; import { CODEX_RESPONSES_API, type ServiceTier } from "./extensions/builtin/service-tier.ts"; import { deriveExtensionRegistrationId } from "./extensions/builtin/tool-search/engine/marker.ts"; -import { getToolSearchService } from "./extensions/builtin/tool-search/service.ts"; +import { + getToolSearchService, + getToolSearchServiceForActivator, + type ToolSearchService, +} from "./extensions/builtin/tool-search/service.ts"; import { type ContextUsage, ExecuteToolError, @@ -1195,6 +1199,7 @@ export class AgentSession { // Tool registry for extension getTools/setTools private _toolRegistry: Map = new Map(); private _lazyToolActivators: LazyToolActivator[] = []; + private _toolSearchService: ToolSearchService | undefined; private _toolDefinitions: Map = new Map(); private _toolPromptSnippets: Map = new Map(); private _toolPromptGuidelines: Map = new Map(); @@ -1452,7 +1457,7 @@ export class AgentSession { this.agent.resolveUnknownToolCall = (toolName) => { let service: ReturnType; try { - service = getToolSearchService(); + service = this._toolSearchService ?? getToolSearchService(); } catch { return undefined; } @@ -7151,6 +7156,7 @@ export class AgentSession { }, registerLazyToolActivator: (activator) => { this._lazyToolActivators.push(activator); + this._toolSearchService = getToolSearchServiceForActivator(activator) ?? this._toolSearchService; }, getCommands, setModel: async (model) => { @@ -7500,6 +7506,7 @@ export class AgentSession { previousActiveToolRegistrationIds?: ReadonlyMap; }): void { this._delegatedCompactionKey = undefined; + this._toolSearchService = undefined; const autoResizeImages = this.settingsManager.getImageAutoResize(); const shellCommandPrefix = this.settingsManager.getShellCommandPrefix(); const shellPath = this.settingsManager.getShellPath(); @@ -7744,7 +7751,7 @@ export class AgentSession { private _takeNativeToolSearchInjectionFailure(): string | null { try { - return getToolSearchService().takeNativeInjectionFailure(); + return (this._toolSearchService ?? getToolSearchService()).takeNativeInjectionFailure(); } catch { return null; } diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index eb4ab6579d..1e6c51715a 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,21 @@ +## Session-local SDK tool-search ownership (2026-09-10) + +### What changed + +- `packages/coding-agent/src/core/agent-session.ts`: retain the tool-search owner associated with the session's existing lazy activator registration, clear it during runtime reconstruction, and prefer it for unknown-tool resolution and native-injection diagnostics. + +### Why + +- `packages/coding-agent/src/core/agent-session.ts`: documented direct SDK callers need not bind extensions or emit `session_start`. Their captured provider hooks can inject deferred tools while global lookup is absent or belongs to another accepted session. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/agent-session.ts`: core owns unknown-tool dispatch and native-error recovery. Internal callback ownership connects those consumers to the same service without changing public extension APIs or prematurely publishing a resume candidate. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/agent-session.ts`: lazy registration binding, runtime reconstruction, unknown-tool resolution and native-injection failure consumption. + ## Bounded prepared-resume content fingerprints (2026-09-10) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md index 245f84dc2d..a54c6812c0 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/changes.md @@ -1,5 +1,25 @@ # Tool Search Builtin Changes +## 2026-09-10 - Retain the unbound SDK session's tool-search owner (PR #1473) + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts`: privately associate each catalog with its lazy activator using weak keys, so AgentSession can retain the service delivered through its existing registration wiring. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: register that associated activator. Global and provider-scoped publication still occurs only at `session_start`. + +### Why + +- Direct `createAgentSession()` callers need not call `bindExtensions()`. Their provider hooks can inject deferred tools before `session_start`, while global catalog lookup previously rejected those calls or consumed another session's native-injection diagnostic. + +### Why an extension could not handle it + +- The builtin owns the service captured by its provider hooks and lazy activator; core needs that same owner before resolving an unknown tool or consuming a native-injection failure. The existing callback registration carries this internal association without a public lifecycle change. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts`: ownership helpers beside local/scoped installation. +- `packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts`: lazy activator registration and service imports. + ## 2026-09-10 - Keep discarded candidates out of scoped tool-search state ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts index 8a5ac8d3b8..26612c22d7 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/index.ts @@ -1,7 +1,12 @@ import { bindToProviderScope } from "@earendil-works/pi-ai/node/provider-scope"; import type { ExtensionAPI, ExtensionFactory } from "../../types.ts"; import { AnthropicNativeToolSearchAdapter, isMcpNativeToolSearchEnabled } from "./native-search.ts"; -import { installLocalToolSearchService, installScopedToolSearchService, ToolSearchService } from "./service.ts"; +import { + createToolSearchActivator, + installLocalToolSearchService, + installScopedToolSearchService, + ToolSearchService, +} from "./service.ts"; import { createToolSearchTool, TOOL_SEARCH_TOOL_NAME } from "./tool.ts"; export function createToolSearchExtension(service: ToolSearchService): ExtensionFactory { @@ -15,7 +20,7 @@ export function createToolSearchExtension(service: ToolSearchService): Extension pi.on("context", (event) => { service.maybeRehydrateFromHistory(event.messages); }); - pi.registerLazyToolActivator((toolName) => service.activateTool(toolName)); + pi.registerLazyToolActivator(createToolSearchActivator(service)); let toolRegistered = false; service.bindToolRegistrar(() => { if (toolRegistered) return; diff --git a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts index 747fa6d3e2..4befc966c5 100644 --- a/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts +++ b/packages/coding-agent/src/core/extensions/builtin/tool-search/service.ts @@ -1,6 +1,6 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { basename, extname } from "node:path"; -import type { ExtensionAPI, ToolInfo } from "../../types.ts"; +import type { ExtensionAPI, LazyToolActivator, ToolInfo } from "../../types.ts"; import { type Bm25Result, type Bm25SearchOptions, buildBm25Index } from "./engine/bm25.ts"; import type { ToolSearchDocument, ToolSearchSource } from "./engine/document.ts"; import { deriveExtensionRegistrationId, rehydrate } from "./engine/marker.ts"; @@ -218,6 +218,20 @@ function isValidDocument(doc: ToolSearchDocument, source: ToolSearchSource): boo return doc.source === source && doc.name.length > 0 && doc.registrationId.length > 0; } +// bindCore transfers this callback to its owning session even before session_start. +// Weak keys keep discarded extension generations out of the shared active catalog. +const activatorServices = new WeakMap(); + +export function createToolSearchActivator(value: ToolSearchService): LazyToolActivator { + const activate: LazyToolActivator = (name) => value.activateTool(name); + activatorServices.set(activate, value); + return activate; +} + +export function getToolSearchServiceForActivator(activate: LazyToolActivator): ToolSearchService | undefined { + return activatorServices.get(activate); +} + const scopedService = new AsyncLocalStorage(); let service: ToolSearchService | null = null; diff --git a/packages/coding-agent/test/suite/regressions/issue-1473-sdk-tool-search-owner.test.ts b/packages/coding-agent/test/suite/regressions/issue-1473-sdk-tool-search-owner.test.ts new file mode 100644 index 0000000000..f78ab6638b --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-sdk-tool-search-owner.test.ts @@ -0,0 +1,178 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fauxAssistantMessage, fauxToolCall, registerFauxProvider } from "@earendil-works/pi-ai/compat"; +import { Type } from "typebox"; +import { expect, it } from "vitest"; +import type { AgentSession } from "../../../src/core/agent-session.ts"; +import { AuthStorage } from "../../../src/core/auth-storage.ts"; +import toolSearchExtension from "../../../src/core/extensions/builtin/tool-search/index.ts"; +import { + getToolSearchService, + resetToolSearchServiceForTests, +} from "../../../src/core/extensions/builtin/tool-search/service.ts"; +import { createAgentSession } from "../../../src/core/sdk.ts"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { SettingsManager } from "../../../src/core/settings-manager.ts"; +import { createInMemoryModelRegistry } from "../../model-runtime-test-utils.ts"; +import { createTestExtensionsResult, createTestResourceLoader } from "../../utilities.ts"; + +async function createSdkFixture() { + resetToolSearchServiceForTests(); + const cwd = mkdtempSync(join(tmpdir(), "pr1473-sdk-owner-")); + const faux = registerFauxProvider({ + api: "anthropic-messages", + provider: "pr1473-sdk", + models: [{ id: "primary" }, { id: "fallback" }], + }); + const sessions: AgentSession[] = []; + const started: string[] = []; + const authStorage = AuthStorage.inMemory(); + const modelRegistry = await createInMemoryModelRegistry(authStorage); + modelRegistry.registerProvider(faux.getModel().provider, { + api: faux.api, + apiKey: "offline-fixture", + baseUrl: faux.getModel().baseUrl, + models: faux.models.map((model) => ({ ...model, compat: { supportsToolReferences: true } })), + }); + return { + faux, + started, + async createSession(name: string) { + const extensionsResult = await createTestExtensionsResult( + [ + { factory: toolSearchExtension, path: "" }, + (pi) => { + pi.on("session_start", () => { + started.push(name); + }); + pi.registerTool({ + name, + label: name, + description: name, + exposure: "search", + parameters: Type.Object({ city: Type.String() }), + execute: async (_id, params) => ({ + content: [{ type: "text", text: `${name}:${params.city}` }], + details: { owner: name }, + }), + }); + }, + ], + cwd, + ); + const { session } = await createAgentSession({ + cwd, + agentDir: join(cwd, "agent"), + model: { ...faux.getModel(), compat: { supportsToolReferences: true } }, + modelRegistry, + authStorage, + resourceLoader: createTestResourceLoader({ extensionsResult }), + sessionManager: SessionManager.inMemory(cwd), + settingsManager: SettingsManager.inMemory({ + compaction: { enabled: false }, + retry: { + enabled: true, + fallbackChains: { "pr1473-sdk/primary": ["pr1473-sdk/fallback"] }, + }, + }), + noTools: "builtin", + autoTitleSessions: false, + }); + sessions.push(session); + return session; + }, + async cleanup() { + for (const session of sessions) await session.disposeCandidate(); + faux.unregister(); + resetToolSearchServiceForTests(); + rmSync(cwd, { recursive: true, force: true }); + }, + }; +} + +// PR1473 PRRT_kwDORgY43c6g6BJn: documented SDK flow does not call bindExtensions/session_start. +it.each([false, true])("executes the SDK's deferred tool with unrelated global owner=%s", async (withLiveOwner) => { + // Given an unbound SDK session, optionally beside an accepted live session. + const fixture = await createSdkFixture(); + try { + const live = withLiveOwner ? await fixture.createSession("live_hidden") : undefined; + await live?.bindExtensions({}); + const session = await fixture.createSession("sdk_hidden"); + const payloads: unknown[] = []; + fixture.faux.setResponses([ + async (_context, options, _state, model) => { + payloads.push(await options?.onPayload?.({ tools: [] }, model, { model, headers: {} })); + return fauxAssistantMessage(fauxToolCall("sdk_hidden", { city: "Seoul" }), { stopReason: "toolUse" }); + }, + fauxAssistantMessage("done"), + ]); + expect(session.getActiveToolNames()).not.toContain("sdk_hidden"); + + // When the provider calls the schema injected by this session's captured hook. + await session.prompt("call the deferred tool"); + + // Then normal agent-core dispatch executes this session's registered tool. + expect(payloads[0]).toMatchObject({ + tools: expect.arrayContaining([ + { name: "sdk_hidden", description: "sdk_hidden", input_schema: expect.any(Object), defer_loading: true }, + ]), + }); + expect(session.messages.filter((message) => message.role === "toolResult")).toMatchObject([ + { toolName: "sdk_hidden", isError: false, content: [{ type: "text", text: "sdk_hidden:Seoul" }] }, + ]); + expect(fixture.started).toEqual(withLiveOwner ? ["live_hidden"] : []); + if (live) { + expect(live.getActiveToolNames()).not.toContain("sdk_hidden"); + expect( + getToolSearchService() + .getCatalog() + .map((doc) => doc.name), + ).toEqual(["live_hidden"]); + } else { + expect(() => getToolSearchService()).toThrow(); + } + } finally { + await fixture.cleanup(); + } +}); + +it("recovers the SDK's native 400 without consuming the live owner's diagnostic", async () => { + // Given separate accepted-live and unbound-SDK native injection owners. + const fixture = await createSdkFixture(); + try { + const live = await fixture.createSession("live_hidden"); + await live.bindExtensions({}); + const liveService = getToolSearchService(); + liveService.noteNativeInjectionFailure("live-owner-sentinel"); + const session = await fixture.createSession("sdk_hidden"); + const payloads: unknown[] = []; + fixture.faux.setResponses([ + async (_context, options, _state, model) => { + payloads.push(await options?.onPayload?.({ tools: [] }, model, { model, headers: {} })); + await options?.onResponse?.({ status: 400, headers: {} }, model); + return fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "invalid_request_error: rejected tool reference", + }); + }, + async (_context, options, _state, model) => { + payloads.push(await options?.onPayload?.({ tools: [] }, model, { model, headers: {} })); + return fauxAssistantMessage("recovered"); + }, + ]); + + // When this SDK request is rejected and retried. + await session.prompt("recover native injection"); + + // Then retry stays on the same model, disables only local injection, and preserves the live signal. + expect(fixture.faux.getCallLog().map((call) => call.modelId)).toEqual(["primary", "primary"]); + expect(payloads[0]).toMatchObject({ + tools: expect.arrayContaining([expect.objectContaining({ name: "sdk_hidden", defer_loading: true })]), + }); + expect(payloads[1]).toEqual({ tools: [] }); + expect(liveService.takeNativeInjectionFailure()).toBe("live-owner-sentinel"); + } finally { + await fixture.cleanup(); + } +}); From 210fb93d5e5eca033c5ec5c91c4d7255bef3a70b Mon Sep 17 00:00:00 2001 From: Tinycute00 Date: Thu, 10 Sep 2026 11:49:41 +0800 Subject: [PATCH 11/11] docs(rpc): clarify finite resume admission and projections Distinguish SDK/factory admission from accepted-runtime startup mutations. Document ordinary budget sums, compaction-aware maxima and the separate compaction-required SDK fallback diagnostic. Verified against implementation and real startup probes; optional recommendation errors retain the admitted fallback. JSON examples and runtime behavior are unchanged. No prose-pinning tests. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/docs/rpc.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index ddf0030d0b..42763efc53 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -1074,7 +1074,9 @@ When compaction is enabled and the restored context fits the raw model window, a The order is: cancellable `session_before_switch` check, destination snapshot and trust/factory preparation, exact SDK budget admission, writer grant with synchronous final revalidation/persistence, then outgoing `session_shutdown` and replacement. A veto prevents destination trust prompts, trust persistence and factory execution. Writes completed by an awaited veto are included in the snapshot; writes during later factory preparation produce a recoverable conflict. -Missing-cwd, budget and conflict rejections may follow the cancellable check, but never cause outgoing shutdown or invalidate the current session. Cleanup belongs in `session_shutdown`, not `session_before_switch`; active `/btw` work survives cancelled and rejected resumes. A conflict preserves any intervening target writes, and a cancelled or rejected switch does not itself write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. +Admission covers the model, prompt and active tools assembled by the destination SDK and factories at that point. `session_start` runs on the accepted replacement; later extension model, tool and prompt changes are not previewed by this check. Startup handler failures are reported through the host's extension-error path, not treated as retroactive resume cancellation. For example, a recommended-model change rejected by its own budget check retains the admitted fallback model. This preflight does not guarantee that arbitrary later configuration changes fit the model window. + +Missing-cwd, budget and conflict rejections may follow the cancellable check, but never cause outgoing shutdown or invalidate the current session. Cleanup belongs in `session_shutdown`, not `session_before_switch`; active `/btw` work survives cancelled and rejected admissions. A conflict preserves any intervening target writes, and cancellation or failed admission does not itself write to the target session file. Clients can pick a different session or change the destination configuration and retry. Changing the live model with `set_model` does not override a destination's stored model or a model forced by CLI `--model` / the launch profile; change that startup selection when retrying with a larger model. ```json { @@ -1102,7 +1104,13 @@ Missing-cwd, budget and conflict rejections may follow the cancellable check, bu } ``` -For a non-empty `switch_session` target, `admission` is `resume` and `speculationLeadTokens` is zero. The shared projection also supports `start` (including empty targets) and `switch` (model changes). `requiredTokens` is the sum of `liveContextTokens`, `systemPromptTokens`, `activeToolSchemaTokens`, `outputReserveTokens`, `compactionReserveTokens`, `speculationLeadTokens`, and `safetyMarginTokens`. `shortfallTokens = max(0, requiredTokens - contextWindow)`; `usable` is true exactly when that shortfall is zero. `model` and `safetyMarginProfile` identify the selection and margin policy; they are not token components. Clients should branch on `errorCode`, not parse the human-readable `error` text. +For a non-empty `switch_session` target, `admission` is `resume` and `speculationLeadTokens` is zero. The shared projection also supports `start` (including empty targets) and `switch` (model changes). For ordinary admission, `requiredTokens` is the sum of `liveContextTokens`, `systemPromptTokens`, `activeToolSchemaTokens`, `outputReserveTokens`, `compactionReserveTokens`, `speculationLeadTokens`, and `safetyMarginTokens`. `shortfallTokens = max(0, requiredTokens - contextWindow)`; `usable` is true exactly when that shortfall is zero. + +If that sum exceeds the window during a compaction-enabled resume, the projection also checks two requirements: compaction needs `liveContextTokens + systemPromptTokens + activeToolSchemaTokens + compactionReserveTokens + safetyMarginTokens`; the post-compaction turn needs the effective retained-history allowance plus all the non-history components in the ordinary sum. The retained-history allowance is derived from the compaction settings and model window, not included as a projection field. If both requirements fit, `requiredTokens` becomes their maximum, `shortfallTokens` is zero, and `usable` is true. The original component fields are retained, so their sum need not equal `requiredTokens` in this case. + +Separately, the SDK can admit a resume whose projection remains unusable when compaction is enabled and `liveContextTokens <= contextWindow`. Its `resume_compaction_required` event preserves that diagnostic projection, including the positive shortfall; the event does not claim that a normal turn already fits. Required compaction and budget validation must succeed before the first normal provider turn. + +`model` and `safetyMarginProfile` identify the selection and margin policy; they are not token components. Treat the returned `requiredTokens` as authoritative rather than recomputing it from the component fields. Clients should distinguish the `resume_compaction_required` event from a failed response and branch on `errorCode`, not parse the human-readable `error` text. #### fork