diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index de39acbaa0..3bbc855773 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,9 @@ ### 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. 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)). - Shared RPC hosts now cut a socket peer that stops reading (a write not accepted within 4 seconds) with an `overflow` record `stalled, resync required` instead of letting it hold the session worker's output credit until the 5-second `session_worker_credit_timeout` quarantined a healthy session mid-turn; a cut or overflowed peer no longer withholds session credit or fails the shared host writer ([#1529](https://github.com/code-yeongyu/senpi/pull/1529)). ### 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 f4ae906bef..698181dc39 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -212,8 +212,12 @@ 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. Acquired 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 +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 explicitly. Close or an opening deadline requests worker termination, but does not release reservations or worker capacity until the actual exit event. A syscall that cannot yet be interrupted can therefore keep an entry internally quarantined after the routing handle has closed. `list_sessions` continues to publish `closing`, not a @@ -1063,6 +1067,56 @@ 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"}` | +| `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 cannot be admitted by the destination model's resume/compaction policy. | The full budget projection | + +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. + +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. + +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 +{ + "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). 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 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 f6f2a49031..0fc795ba42 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 { @@ -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; @@ -246,25 +246,48 @@ export class AgentSessionRuntime { projectTrustContextFactory?: (cwd: string) => ProjectTrustContext; }, ): Promise<{ cancelled: boolean }> { - 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); + const isSelfResume = + previousSessionFile !== undefined && + 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); - 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, - }), - ); + // Build and admit the actual destination, including its model selection, + // settings, prompt and tools. Persistence and destructive shutdown 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, + }); + let acceptance: ReturnType | undefined; + try { + // 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); + } finally { + if (this.session !== result.session) { + try { + await result.session.disposeCandidate(); + } finally { + acceptance?.rollback(); + } + } + } await this.finishSessionReplacement(options?.withSession); return { cancelled: false }; } diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 216da44836..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; } @@ -2928,8 +2933,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(); @@ -2951,7 +2958,14 @@ export class AgentSession { this._unsubscribeWakeSources?.(); this._unsubscribeWakeSources = undefined; this._eventListeners = []; - cleanupSessionResources(this.sessionId); + if (options?.releaseProviderResources !== false) cleanupSessionResources(this.sessionId); + } + + /** Invalidate only an unstarted destination's registrations, not live-session resources. */ + async disposeCandidate(): Promise { + // 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. */ @@ -7142,6 +7156,7 @@ export class AgentSession { }, registerLazyToolActivator: (activator) => { this._lazyToolActivators.push(activator); + this._toolSearchService = getToolSearchServiceForActivator(activator) ?? this._toolSearchService; }, getCommands, setModel: async (model) => { @@ -7491,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(); @@ -7735,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 57a46c8548..1e6c51715a 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,3 +1,212 @@ +## 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 + +- `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. + +### 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 + +- `packages/coding-agent/src/core/sdk.ts`: retain upstream's mandatory-compaction admission for restored contexts that fit the raw model window, and dispose candidate resources only when admission genuinely rejects. + +### Why + +- `packages/coding-agent/src/core/sdk.ts`: unconditional candidate cleanup would destroy a runtime that upstream now intentionally admits for pre-first-turn compaction. + +### Why an extension could not handle it + +- `packages/coding-agent/src/core/sdk.ts`: SDK admission owns the candidate before extension lifecycle startup. + +### Expected merge conflict zones + +- `packages/coding-agent/src/core/sdk.ts`: model-usability catch and resume-compaction requirement. + +## 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 + +- `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 + +- `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 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`: 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 + +- `packages/coding-agent/src/core/session-manager.ts`, `packages/coding-agent/src/core/session-write-reservation.ts`, `packages/coding-agent/src/core/agent-session-runtime.ts`, `packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/src/core/sdk.ts`: prepared writer ownership and pre-start candidate disposal are core lifecycle responsibilities before extension startup. + +### Expected merge conflict zones + +- `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-only invalidation and failed model admission. + +## Resume admission uses the prepared destination runtime (2026-09-08) + +### What changed + +- `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 + +- `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 + +- `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 + +- `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`. + ## Registration-time shared-host capability (2026-09-09) ### What changed @@ -33,6 +242,7 @@ ### Expected merge conflict zones - MEDIUM in `sdk.ts` startup admission and `agent-session.ts` pre-provider compaction gate; LOW in the interactive event switch. + ## Size-adaptive summarization duration budget setting (2026-09-08) ### 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/extensions/builtin/mcp/changes.md b/packages/coding-agent/src/core/extensions/builtin/mcp/changes.md index 0945fbd5bd..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,5 +1,41 @@ +## 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) + +### What changed + +- `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 + +- `packages/coding-agent/src/core/extensions/builtin/mcp/index.ts`: repeated cancelled or budget-rejected resumes otherwise retain candidate APIs and listeners on the process-global MCP service; disposing that singleton instead would break the active session. + +### Why an extension could not handle it + +- `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`: attachment-time wire subscription, session shutdown ownership and inventory bridge cleanup. + ## Explicit pgrep match-all pattern for process-tree collection (2026-08-12) ### What changed 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 213dc19c82..8dc4c20fbc 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: { @@ -55,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 — @@ -111,7 +104,18 @@ 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. + 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); @@ -167,6 +171,12 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex wrapAsync( "mcp.session_shutdown", async (event) => { + // An unstarted candidate owns only its factory-time subscriptions, not + // the shared service currently attached to the live runtime. + if (!attachPromise) { + disposeControlInventory(); + return; + } if (event.reason === "reload" && !sessionOwned) return; disposeControlInventory(); await service.handleSessionShutdown(event); 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..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,65 @@ # 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 + +- `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 + +- `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..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 { getToolSearchService, 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; @@ -70,8 +75,11 @@ export default function toolSearchExtension(pi: ExtensionAPI): void | Promise pi.setActiveTools([...names]), }; const sessionOwned = hasProviderScope(); - const service = sessionOwned ? new ToolSearchService(runtime) : getToolSearchService(runtime); - if (sessionOwned) installScopedToolSearchService(service); + // 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) 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 e05a45a6d4..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,10 +218,29 @@ 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; -/** Make a session-owned service visible to later builtins loaded in the same provider scope. */ +/** Publish the local runtime's catalog only once its session has been accepted. */ +export function installLocalToolSearchService(value: ToolSearchService): void { + service = value; +} + +/** 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/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 754571d94a..1e59867825 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -555,6 +555,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} !session.settingsManager.getCompactionEnabled() || error.projection.liveContextTokens > error.projection.contextWindow ) { + await session.disposeCandidate(); throw error; } session.admitResumeCompactionRequired(error.projection); diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index ff96845ad6..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, @@ -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"; @@ -823,12 +824,35 @@ 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; private sessionDir: string; private cwd: string; private persist: boolean; + private deferredPersistence?: { + rewrite: boolean; + entries: SessionEntry[]; + snapshots: Map; + }; private flushed: boolean = false; private fileEntries: FileEntry[] = []; private byId: Map = new Map(); @@ -870,11 +894,15 @@ 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: [], snapshots: new Map() } + : undefined; + if (persist && !deferredPersistence && this.sessionDir && !existsSync(this.sessionDir)) { mkdirSync(this.sessionDir, { recursive: true }); } @@ -907,8 +935,18 @@ export class SessionManager { this._setSessionFile(sessionFile); } + private _reserveWrite(path: string): void { + if (this.deferredPersistence) { + if (!this.deferredPersistence.snapshots.has(path)) { + this.deferredPersistence.snapshots.set(path, readSessionFingerprint(path)); + } + return; + } + reserveSessionWrite(path); + } + private _setSessionFile(sessionFile: string, preloadedFileEntries?: FileEntry[]): void { - if (this.persist) reserveSessionWrite(resolvePath(sessionFile)); + if (this.persist) this._reserveWrite(resolvePath(sessionFile)); this.sessionFile = resolvePath(sessionFile); this.mirrorTrimmed = false; this.residentStore.clear(); @@ -936,7 +974,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; @@ -985,7 +1025,7 @@ export class SessionManager { if (this.persist) { const fileTimestamp = timestamp.replace(/[:.]/g, "-"); const path = join(this.getSessionDir(), `${fileTimestamp}_${this.sessionId}.jsonl`); - reserveSessionWrite(path); + this._reserveWrite(path); this.sessionFile = path; } return this.sessionFile; @@ -1031,6 +1071,10 @@ export class SessionManager { private _rewriteFile(): void { if (!this.persist || !this.sessionFile) return; + if (this.deferredPersistence) { + this.deferredPersistence.rewrite = true; + return; + } reserveSessionWrite(this.sessionFile); const fd = openSync(this.sessionFile, "w"); try { @@ -1072,6 +1116,10 @@ export class SessionManager { _persist(entry: SessionEntry): void { if (!this.persist || !this.sessionFile) return; + if (this.deferredPersistence) { + this.deferredPersistence.entries.push(entry); + return; + } reserveSessionWrite(this.sessionFile); const persistedEntry = this.residentStore.materialize(entry); @@ -1102,7 +1150,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); @@ -1292,7 +1342,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; @@ -1586,16 +1637,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); @@ -1781,11 +1837,11 @@ export class SessionManager { parentId = labelEntry.id; } - reserveSessionWrite(newSessionFile); + this._reserveWrite(newSessionFile); 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; @@ -1850,8 +1906,92 @@ 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; + beginCommit: () => { commit: () => void; rollback: () => void }; + } { + const sessionManager = SessionManager._open(path, sessionDir, cwdOverride, true); + const pending = sessionManager.deferredPersistence; + return { + sessionManager, + beginCommit: () => { + 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 destructive session shutdown. + let release = reserveSessionWrite(file); + const rollback = () => { + release?.(); + release = undefined; + }; + const revalidate = () => { + try { + const expected = pending?.snapshots.get(file); + const current = readSessionFingerprint(file); + if (current !== expected) { + throw new SessionResumeConflictError(file); + } + } catch (error) { + rollback(); + throw error; + } + }; + revalidate(); + return { + rollback, + commit: () => { + // 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; + mkdirSync(sessionManager.sessionDir, { recursive: true }); + if (pending?.rewrite) { + sessionManager._rewriteFile(); + } else { + if (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); + } + // 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(); + }, + }; + }, + }; + } + + private static _open( + path: string, + sessionDir: string | undefined, + cwdOverride: string | undefined, + deferredPersistence: boolean, + ): SessionManager { const resolvedPath = resolvePath(path); - reserveSessionWrite(resolvedPath); + if (!deferredPersistence) reserveSessionWrite(resolvedPath); let header: SessionHeader | null = null; let preloadedFileEntries: FileEntry[] | undefined; if (cwdOverride === undefined && existsSync(resolvedPath)) { @@ -1868,14 +2008,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/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/core/session-write-reservation.ts b/packages/coding-agent/src/core/session-write-reservation.ts index a05eeefbcc..edebbc11b3 100644 --- a/packages/coding-agent/src/core/session-write-reservation.ts +++ b/packages/coding-agent/src/core/session-write-reservation.ts @@ -1,12 +1,12 @@ /** Installed only inside a shared-host session isolate, before constructing any writer. */ -let reserve: ((path: string) => void) | undefined; +let reserve: ((path: string) => (() => void) | undefined) | undefined; -export function installSessionWriteReservation(reservation: (path: string) => void): void { +export function installSessionWriteReservation(reservation: (path: string) => (() => void) | undefined): void { if (reserve) throw new Error("Session write reservation already installed"); reserve = reservation; } -/** Synchronous SessionManager entry points must obtain the host grant before touching a writer. */ -export function reserveSessionWrite(path: string): void { - reserve?.(path); +/** Obtain the host grant before touching a writer. Only a newly acquired grant can be rolled back. */ +export function reserveSessionWrite(path: string): (() => void) | undefined { + return reserve?.(path); } diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 5d339b2db5..ea4427cc8d 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -1,3 +1,39 @@ +## 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 + +### What changed + +- `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 + +- 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-09 - Surface required compaction after oversized resume diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 28723fa05c..f8d4b82d92 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, @@ -108,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"; @@ -7570,21 +7572,45 @@ 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 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 || + overrideError instanceof SessionResumeConflictError + ) { + return this.cancelResumeWithRecoverableError(overrideError); + } + return this.handleFatalRuntimeError("Failed to resume session", overrideError); } - this.showStatus("Resumed session in current cwd"); - return result; + } + if (error instanceof ModelUsabilityBudgetError || error instanceof SessionResumeConflictError) { + return this.cancelResumeWithRecoverableError(error); } return this.handleFatalRuntimeError("Failed to resume session", error); } } + /** 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 }; + } + 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 d5a8f689e2..acf722c9fd 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -1,5 +1,63 @@ +## 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) + +### What changed + +- `packages/coding-agent/src/modes/rpc/session-worker-protocol.ts`, `packages/coding-agent/src/modes/rpc/session-worker-client.ts`, `packages/coding-agent/src/modes/rpc/session-worker.ts` and `packages/coding-agent/src/modes/rpc/worker-session-registry.ts`: internal reservation acknowledgments distinguish new grants from already-owned paths. Failed/cancelled candidate acceptance can synchronously release only its newly acquired canonical grant. Accepted and previously owned writer reservations retain the existing worker-exit lifetime. + +### Why + +- `packages/coding-agent/src/modes/rpc/session-worker-protocol.ts`, `packages/coding-agent/src/modes/rpc/session-worker-client.ts`, `packages/coding-agent/src/modes/rpc/session-worker.ts` and `packages/coding-agent/src/modes/rpc/worker-session-registry.ts`: a cancelled resume must not block other clients from opening its unused target, and cancelling a resume of the live session must not release its existing writer ownership. + +### Why an extension could not handle it + +- `packages/coding-agent/src/modes/rpc/session-worker-protocol.ts`, `packages/coding-agent/src/modes/rpc/session-worker-client.ts`, `packages/coding-agent/src/modes/rpc/session-worker.ts` and `packages/coding-agent/src/modes/rpc/worker-session-registry.ts`: the transport host owns canonical grants across worker isolates; extension events cannot release that private registry state. + +### Expected merge conflict zones + +- `packages/coding-agent/src/modes/rpc/session-worker-protocol.ts`: internal release message. +- `packages/coding-agent/src/modes/rpc/session-worker-client.ts`: reservation acknowledgment and release dispatch. +- `packages/coding-agent/src/modes/rpc/session-worker.ts`: synchronous grant/rollback exchange. +- `packages/coding-agent/src/modes/rpc/worker-session-registry.ts`: grant ownership and release callback. + +## 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`. + ## Cut stalled socket peers before they consume the session worker credit (2026-09-10) ### 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 0519b4186a..4771589107 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, @@ -54,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"; @@ -1671,13 +1673,33 @@ 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 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 + : conflict + ? { sessionFile: conflict.sessionFile } + : 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..8e32ff172c 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-client.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-client.ts @@ -11,9 +11,11 @@ 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"; +import { SessionResumeConflictError } from "../../core/session-resume-conflict.ts"; import type { JsonAgentSessionEvent } from "../json-event.ts"; import { attachJsonlLineReader, serializeJsonLine } from "./jsonl.ts"; import type { @@ -1109,6 +1111,14 @@ export class RpcClient { errorResponse.errorData as ConstructorParameters[0], ); } + if (errorResponse.errorCode === "model_usability_budget" && errorResponse.errorData) { + throw new ModelUsabilityBudgetError( + 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/src/modes/rpc/session-worker-client.ts b/packages/coding-agent/src/modes/rpc/session-worker-client.ts index 6ba14e4e41..c29e8c2a96 100644 --- a/packages/coding-agent/src/modes/rpc/session-worker-client.ts +++ b/packages/coding-agent/src/modes/rpc/session-worker-client.ts @@ -48,12 +48,13 @@ export class SessionWorkerClient { private latestDisplay?: Extract; private readonly callbacks: { - reserve: (path: string) => boolean; + reserve: (path: string) => boolean | "acquired"; + release: (path: string) => void; exit: () => void; failure: (error: string) => void; }; - constructor(callbacks: { reserve: (path: string) => boolean; exit: () => void; failure: (error: string) => void }) { + constructor(callbacks: SessionWorkerClient["callbacks"]) { this.callbacks = callbacks; this.exited = new Promise((resolve) => { this.worker.once("exit", () => { @@ -186,8 +187,15 @@ export class SessionWorkerClient { this.requests.receive(message); return; } - case "reserve": - this.acknowledge(message.signal, !this.stopped && this.callbacks.reserve(message.path)); + case "reserve": { + const grant = this.callbacks.reserve(message.path); + Atomics.store(new Int32Array(message.signal), 1, grant === "acquired" ? 1 : 0); + this.acknowledge(message.signal, grant !== false); + return; + } + case "release_reservation": + this.callbacks.release(message.path); + this.acknowledge(message.signal, true); return; case "snapshot": this.snapshot = message.snapshot; diff --git a/packages/coding-agent/src/modes/rpc/session-worker-protocol.ts b/packages/coding-agent/src/modes/rpc/session-worker-protocol.ts index f71ae7149f..407248a016 100644 --- a/packages/coding-agent/src/modes/rpc/session-worker-protocol.ts +++ b/packages/coding-agent/src/modes/rpc/session-worker-protocol.ts @@ -43,6 +43,7 @@ export type SessionWorkerToHost = | { type: "ready"; request: number; snapshot: WorkerSnapshot } | { type: "result"; request: number; error?: string } | { type: "reserve"; path: string; signal: SharedArrayBuffer } + | { type: "release_reservation"; path: string; signal: SharedArrayBuffer } | { type: "snapshot"; snapshot: WorkerSnapshot; signal: SharedArrayBuffer; settled?: boolean } | { type: "control_done"; control: "display" | "cancel_ui" } | { diff --git a/packages/coding-agent/src/modes/rpc/session-worker.ts b/packages/coding-agent/src/modes/rpc/session-worker.ts index ad1b96ab83..e8ae27284f 100644 --- a/packages/coding-agent/src/modes/rpc/session-worker.ts +++ b/packages/coding-agent/src/modes/rpc/session-worker.ts @@ -53,9 +53,13 @@ function exchange( if (result !== 1) failWorker("session_worker_credit_timeout"); } -installSessionWriteReservation((path) => - exchange((signal) => ({ type: "reserve", path: canonicalPath(path), signal }), "session_path_in_use"), -); +installSessionWriteReservation((path) => { + const canonical = canonicalPath(path); + const signal = new SharedArrayBuffer(8); + exchange((signal) => ({ type: "reserve", path: canonical, signal }), "session_path_in_use", signal); + if (Atomics.load(new Int32Array(signal), 1) !== 1) return undefined; + return () => exchange((signal) => ({ type: "release_reservation", path: canonical, signal })); +}); class WorkerEventWriter extends SessionEventWriter { constructor() { diff --git a/packages/coding-agent/src/modes/rpc/worker-session-registry.ts b/packages/coding-agent/src/modes/rpc/worker-session-registry.ts index 23aeb396aa..6c426f3ef9 100644 --- a/packages/coding-agent/src/modes/rpc/worker-session-registry.ts +++ b/packages/coding-agent/src/modes/rpc/worker-session-registry.ts @@ -51,6 +51,9 @@ export class WorkerSessionRegistry { }; const worker = new SessionWorkerClient({ reserve: (path) => this.reserve(handle, path), + release: (path) => { + if (this.reservations.get(path) === handle) this.reservations.delete(path); + }, exit: () => { if (this.entries.get(handle) !== entry) return; entry.state = "closed"; @@ -191,7 +194,7 @@ export class WorkerSessionRegistry { return { ...result, attached: true }; } - private reserve(handle: string, path: string): boolean { + private reserve(handle: string, path: string): boolean | "acquired" { const entry = this.entries.get(handle); if (!entry) return false; const owner = this.reservations.get(path); @@ -201,7 +204,7 @@ export class WorkerSessionRegistry { for (const current of this.reservations.values()) if (current === handle) count++; if (count >= SESSION_WORKER_LIMITS.reservations) return false; this.reservations.set(path, handle); - return true; + return "acquired"; } private openResult(handle: string, entry: RpcSessionEntry): OpenRpcSession { 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..1606ae4998 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -1,9 +1,14 @@ -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 { afterEach, describe, expect, it, vi } from "vitest"; import { type CreateAgentSessionRuntimeFactory, createAgentSessionFromServices, @@ -11,6 +16,11 @@ 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 { 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 { AgentToolResult, @@ -39,7 +49,17 @@ 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; + destinationCompactionEnabled?: boolean; + destinationSystemPrompt?: string; + destinationToolDescription?: string; + }, ) { const tempDir = options?.cwd ?? join(tmpdir(), `pi-runtime-suite-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -49,6 +69,10 @@ 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 }, ], }); faux.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two"), fauxAssistantMessage("three")]); @@ -59,7 +83,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: [ @@ -91,7 +115,31 @@ 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 !== undefined || + options?.destinationCompactionEnabled !== undefined + ? { + compaction: { + ...(options.destinationReserveTokens !== undefined + ? { reserveTokens: options.destinationReserveTokens } + : {}), + ...(options.destinationCompactionEnabled !== undefined + ? { enabled: options.destinationCompactionEnabled } + : {}), + }, + } + : {}), + }); + } return { ...(await createAgentSessionFromServices({ services, @@ -99,6 +147,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, @@ -207,6 +267,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) => { @@ -652,4 +767,361 @@ describe("AgentSessionRuntime characterization", () => { expect(runtime.session.model?.id).toBe("faux-2"); 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 shutdownSessions: string[] = []; + const { runtime, faux, tempDir } = await createRuntimeForTest( + (pi) => { + pi.on("session_before_switch", (event) => { + events.push(event); + }); + pi.on("session_shutdown", (event, ctx) => { + events.push(event); + shutdownSessions.push(ctx.sessionManager.getSessionId()); + }); + }, + { bootstrapModelId: forced }, + ); + const target = SessionManager.create(tempDir, join(tempDir, "targets")); + target.appendModelChange(faux.getModel().provider, stored); + // Beyond the smaller model's window, not merely eligible for upstream resume compaction. + target.appendMessage({ role: "user", content: "x".repeat(70_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([{ type: "session_before_switch", reason: "resume", targetSessionFile: path }]); + expect(shutdownSessions).toEqual([]); + expect(shutdownSessions).not.toContain(original.sessionId); + 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) }, + ])("uses destination $name for resume admission", async (options) => { + const events: RecordedSessionEvent[] = []; + const requiresCompaction = "destinationReserveTokens" in options; + const { runtime, tempDir } = await createRuntimeForTest( + (pi) => { + pi.on("session_before_switch", (event) => { + events.push(event); + }); + }, + { ...options, destinationCompactionEnabled: requiresCompaction }, + ); + 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(); + if (requiresCompaction) { + // Upstream now admits this shortfall for mandatory compaction; retain the exact destination budget. + expect(await runtime.switchSession(path)).toEqual({ cancelled: false }); + const notices: unknown[] = []; + const unsubscribe = runtime.session.subscribe((event) => notices.push(event)); + unsubscribe(); + expect(notices).toContainEqual( + expect.objectContaining({ + type: "resume_compaction_required", + projection: expect.objectContaining({ compactionReserveTokens: 100_000, usable: false }), + }), + ); + expect(events).toEqual([{ type: "session_before_switch", reason: "resume", targetSessionFile: path }]); + return; + } + await expect(runtime.switchSession(path)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + expect(events).toEqual([{ type: "session_before_switch", reason: "resume", targetSessionFile: path }]); + expect(runtime.session).toBe(original); + await expect(original.prompt("still usable")).resolves.toBeUndefined(); + }); + + // PR #1473: candidate cleanup must not invoke process-global shutdown hooks. + it.each(["cancelled", "rejected"])( + "discards %s candidates without retaining listeners or disturbing live globals", + async (outcome) => { + const service = getMcpService(); + const listeners = new Set[0]>(); + const subscribe = service.onWireStatusChanged.bind(service); + const observation = vi.spyOn(service, "onWireStatusChanged").mockImplementation((listener) => { + listeners.add(listener); + const unsubscribe = subscribe(listener); + return () => { + listeners.delete(listener); + unsubscribe(); + }; + }); + const killAll = vi.spyOn(cursorCliChildRegistry, "killAll"); + const shutdowns: number[] = []; + let factories = 0; + try { + const { runtime, tempDir } = await createRuntimeForTest( + (pi) => { + const candidate = factories++; + pi.on("session_before_switch", () => ({ cancel: outcome === "cancelled" })); + pi.on("session_shutdown", () => { + shutdowns.push(candidate); + }); + }, + 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"); + writeFileSync( + target, + JSON.stringify({ + type: "session", + version: 3, + id: "mcp-candidate", + timestamp: new Date(0).toISOString(), + cwd: tempDir, + }), + ); + const bytes = readFileSync(target); + for (let attempt = 1; attempt <= 3; attempt++) { + if (outcome === "rejected") { + await expect(runtime.switchSession(target)).rejects.toBeInstanceOf(ModelUsabilityBudgetError); + } else { + expect(await runtime.switchSession(target)).toEqual({ cancelled: true }); + } + expect(killAll).not.toHaveBeenCalled(); + expect(isNativeBypass()).toBe(true); + expect(shutdowns).toEqual([]); + expect(factories).toBe(outcome === "cancelled" ? 1 : attempt + 1); + expect(listeners).toEqual(baseline); + expect(service.getSnapshot()).toMatchObject({ + disposed: false, + sessionStartCount: 1, + lastSessionStartReason: "startup", + hasSessionContext: true, + }); + expect(getMcpService()).toBe(service); + expect(runtime.session).toBe(live); + expect(readFileSync(target)).toEqual(bytes); + } + const errors: unknown[] = []; + live.extensionRunner.onError((error) => errors.push(error)); + await expect(live.prompt("still usable after cancelled resumes")).resolves.toBeUndefined(); + await expect(service.refreshWireStatusSnapshot(live.sessionId)).resolves.toMatchObject({ servers: [] }); + expect(errors).toEqual([]); + expect(listeners).toEqual(baseline); + } finally { + observation.mockRestore(); + killAll.mockRestore(); + setNativeBypass(false); + } + }, + ); + + // 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. + 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 })); + + // 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... + 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(); + expect(extensionErrors).toEqual([]); + }); + + // 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[] = []; + // 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; + 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); + + // 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); + await expect(runtime.session.prompt("still here")).resolves.toBeUndefined(); + }); }); 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 new file mode 100644 index 0000000000..6ad7d07c34 --- /dev/null +++ b/packages/coding-agent/test/suite/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; + cancelResumeWithRecoverableError: (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 cancelResumeWithRecoverableError = Object.getOwnPropertyDescriptor( + InteractiveMode.prototype, + "cancelResumeWithRecoverableError", + )?.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(() => ({})), + cancelResumeWithRecoverableError: vi.fn(function (this: ResumeContext, error: ModelUsabilityBudgetError) { + return cancelResumeWithRecoverableError.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/suite/issue-1473-runtime-support.ts b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts new file mode 100644 index 0000000000..3a90daecfc --- /dev/null +++ b/packages/coding-agent/test/suite/issue-1473-runtime-support.ts @@ -0,0 +1,104 @@ +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 | Promise = () => {}, + 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: [ + async (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, + })), + }); + await 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-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 new file mode 100644 index 0000000000..fb80824bf8 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-cancelled-resume-reservations.test.ts @@ -0,0 +1,135 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { startWorkerHost } from "../rpc-worker-host-support.ts"; + +// PR #1473: exercise the real sharedHost worker registry, not a reservation mock. +it.each(["unterminated", "legacy", "empty", "missing"])( + "lets another client open and attach a cancelled %s resume target", + async (kind) => { + const host = await startWorkerHost( + `export default function(pi) { + pi.on("session_before_switch", () => ({ cancel: true })); + }`, + { socket: true }, + ); + try { + const target = join(host.scratch, "cancelled.jsonl"); + const bytes = + kind === "empty" + ? "" + : JSON.stringify({ + type: "session", + version: kind === "legacy" ? 1 : 3, + id: "cancelled", + timestamp: new Date(0).toISOString(), + cwd: host.cwd, + }); + if (kind !== "missing") await writeFile(target, bytes); + const a = await host.connect(); + const b = await host.connect(); + const c = await host.connect(); + const opened = await a.request({ type: "open_session", cwd: host.cwd }); + expect(opened.success).toBe(true); + for (let attempt = 0; attempt < 3; attempt++) { + const cancelled = await a.request({ + type: "switch_session", + sessionId: opened.data?.sessionId, + sessionPath: target, + }); + expect(cancelled).toMatchObject({ success: true, data: { cancelled: true } }); + if (kind === "missing") await expect(readFile(target)).rejects.toMatchObject({ code: "ENOENT" }); + else expect(await readFile(target, "utf8")).toBe(bytes); + } + const selfCancelled = await a.request({ + type: "switch_session", + sessionId: opened.data?.sessionId, + sessionPath: opened.data?.state?.sessionFile, + }); + expect(selfCancelled).toMatchObject({ success: true, data: { cancelled: true } }); + const selfAttached = await c.request({ + type: "open_session", + cwd: host.cwd, + sessionPath: opened.data?.state?.sessionFile, + }); + expect(selfAttached).toMatchObject({ + success: true, + data: { attached: true, sessionId: opened.data?.sessionId }, + }); + const other = await b.request({ type: "open_session", cwd: host.cwd, sessionPath: target }); + expect(other).toMatchObject({ success: true }); + expect(other.data?.sessionId).not.toBe(opened.data?.sessionId); + const attached = await c.request({ type: "open_session", cwd: host.cwd, sessionPath: target }); + expect(attached).toMatchObject({ success: true, data: { attached: true, sessionId: other.data?.sessionId } }); + const live = await a.request({ type: "get_state", sessionId: opened.data?.sessionId }); + expect(live).toMatchObject({ success: true, data: { sessionId: opened.data?.state?.sessionId } }); + } finally { + await host.dispose(); + } + }, + 60_000, +); + +// A factory mutates the target only to deterministically exercise the real acceptance failure path. +it.each(["owned", "changed"])( + "rejects an %s acceptance after veto without destructive shutdown", + async (failure) => { + const host = await startWorkerHost(` + import { appendFileSync, existsSync, unlinkSync, writeFileSync } from "node:fs"; + export default function(pi) { + if (existsSync("mutate-next")) { + unlinkSync("mutate-next"); + appendFileSync("target.jsonl", "\\n"); + } + pi.on("session_before_switch", () => { writeFileSync("switch-fired", "1"); }); + pi.on("session_shutdown", () => { writeFileSync("shutdown-fired", "1"); }); + } + `); + try { + const target = join(host.cwd, "target.jsonl"); + const bytes = JSON.stringify({ + type: "session", + version: 3, + id: "destination", + timestamp: new Date(0).toISOString(), + cwd: host.cwd, + }); + await writeFile(target, bytes); + const live = await host.request({ type: "open_session", cwd: host.cwd }); + expect(live.success).toBe(true); + if (failure === "owned") { + expect(await host.request({ type: "open_session", cwd: host.cwd, sessionPath: target })).toMatchObject({ + success: true, + }); + } else { + await writeFile(join(host.cwd, "mutate-next"), "1"); + } + const before = await readFile(target); + const rejected = await host.request({ + type: "switch_session", + sessionId: live.data?.sessionId, + sessionPath: target, + }); + expect(rejected).toMatchObject({ success: false }); + if (failure === "owned") { + expect(rejected.error).toContain("session_path_in_use"); + expect(await readFile(target)).toEqual(before); + } else { + expect(rejected.error).toContain("Session file changed while preparing resume"); + // Revalidation failed after acquiring a new grant. It must be released without worker exit. + expect(await host.request({ type: "open_session", cwd: host.cwd, sessionPath: target })).toMatchObject({ + success: true, + }); + } + 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 }, + }); + } finally { + await host.dispose(); + } + }, + 60_000, +); 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..7a00e91520 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-deferred-legacy-content.test.ts @@ -0,0 +1,164 @@ +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 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"); + 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(1); + expect(shutdowns).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 new file mode 100644 index 0000000000..fe8a3a865c --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-prepared-session-writers.test.ts @@ -0,0 +1,122 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +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> = []; +afterEach(() => { + vi.restoreAllMocks(); + for (const cleanup of cleanups.splice(0)) cleanup(); +}); + +function fixture() { + const cwd = mkdtempSync(join(tmpdir(), "prepared-writers-")); + cleanups.push(() => rmSync(cwd, { recursive: true, force: true })); + const path = join(cwd, "target.jsonl"); + const bytes = JSON.stringify({ + type: "session", + version: 3, + id: "target", + timestamp: new Date(0).toISOString(), + cwd, + }); + writeFileSync(path, bytes); + return { cwd, path, bytes }; +} + +// PR #1473: all instance entry points must preserve the prepared manager's deferred ownership. +it.each(["open", "set", "new", "branch", "write", "reload"])( + "defers writer ownership through prepared %s and reserves only the actual accepted destination", + (operation) => { + const { cwd, path, bytes } = fixture(); + const reserve = vi.spyOn(reservations, "reserveSessionWrite"); + const prepared = SessionManager.prepareOpen(path); + const manager = prepared.sessionManager; + switch (operation) { + case "set": { + const other = join(cwd, "other.jsonl"); + writeFileSync(other, bytes); + manager.setSessionFile(other); + break; + } + case "new": + manager.newSession(); + break; + case "branch": { + const leaf = manager.appendMessage(fauxAssistantMessage("stored")); + manager.createBranchedSession(leaf); + break; + } + case "write": + manager.appendMessage(fauxAssistantMessage("stored")); + break; + case "reload": + manager.reloadFromDisk(); + break; + case "open": + break; + } + expect(reserve).not.toHaveBeenCalled(); + expect(readFileSync(path, "utf8")).toBe(bytes); + const acceptance = prepared.beginCommit(); + expect(reserve).toHaveBeenCalledExactlyOnceWith(manager.getSessionFile()); + expect(readFileSync(path, "utf8")).toBe(bytes); + acceptance.commit(); + }, +); + +it("revalidates the accepted destination and releases the grant when its snapshot changed", () => { + const { path, bytes } = fixture(); + const prepared = SessionManager.prepareOpen(path); + const release = vi.fn(); + vi.spyOn(reservations, "reserveSessionWrite").mockReturnValue(release); + writeFileSync(path, `${bytes}\n`); + expect(() => prepared.beginCommit()).toThrow("Session file changed while preparing resume"); + expect(release).toHaveBeenCalledOnce(); + expect(readFileSync(path, "utf8")).toBe(`${bytes}\n`); +}); + +it("releases the acceptance grant on cancellation without writing the target", () => { + const { path, bytes } = fixture(); + const prepared = SessionManager.prepareOpen(path); + const release = vi.fn(); + vi.spyOn(reservations, "reserveSessionWrite").mockReturnValue(release); + const acceptance = prepared.beginCommit(); + acceptance.rollback(); + 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..f9a2707548 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-conflict.test.ts @@ -0,0 +1,181 @@ +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: 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(); + let factories = 0; + const host = await resumeRuntime(async () => { + if (factories++ > 0) { + 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, 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", (event) => { + writeFileSync("conflict-target", event.targetSessionFile); + }); + } + `, + { 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..af76ed757c --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-resume-lifecycle.test.ts @@ -0,0 +1,215 @@ +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 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)", + 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, +); 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-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(); + } +}); 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); +}); 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(); + } + }, +); 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 new file mode 100644 index 0000000000..c1e9e63414 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/issue-1473-upstream-compaction-admission.test.ts @@ -0,0 +1,60 @@ +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 { SessionManager } from "../../../src/core/session-manager.ts"; +import { resumeRuntime } from "../issue-1473-runtime-support.ts"; + +// PR #1473 must preserve upstream 60c436d40 while cleaning up genuinely rejected candidates. +it.each([ + { name: "compactable context", size: 60_000, enabled: true, accepted: true }, + { name: "context beyond model window", size: 70_000, enabled: true, accepted: false }, + { 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 } })); + const target = SessionManager.create(host.cwd, join(host.cwd, "targets")); + target.appendMessage({ role: "user", content: "x".repeat(size), timestamp: 0 }); + target.appendMessage(fauxAssistantMessage("stored")); + const path = target.getSessionFile(); + if (!path) throw new Error("Persistent fixture has no session file"); + const before = readFileSync(path); + const original = host.runtime.session; + + if (accepted) { + await expect(host.runtime.switchSession(path)).resolves.toEqual({ cancelled: false }); + expect(host.runtime.session).not.toBe(original); + expect(host.runtime.session.sessionFile).toBe(path); + const events: unknown[] = []; + const unsubscribe = host.runtime.session.subscribe((event) => events.push(event)); + unsubscribe(); + expect(events).toContainEqual( + expect.objectContaining({ + type: "resume_compaction_required", + projection: expect.objectContaining({ admission: "resume", usable: false }), + }), + ); + 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(1); + expect(shutdowns).toBe(0); + expect(readFileSync(path)).toEqual(before); + } + } finally { + await host.dispose(); + } +}); diff --git a/packages/coding-agent/test/suite/rpc-client-budget-error.test.ts b/packages/coding-agent/test/suite/rpc-client-budget-error.test.ts new file mode 100644 index 0000000000..65994c67ad --- /dev/null +++ b/packages/coding-agent/test/suite/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/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"),