diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 611baa29d..8d3aa7e94 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed +- Explicit `/compact` now works on Claude SDK-owned sessions after a rejected smaller-model switch, so the recommended recovery from a failed downswitch is usable again ([#1423](https://github.com/code-yeongyu/senpi/pull/1423) by [@realsigridjin](https://github.com/realsigridjin)). Automatic threshold, overflow, pre-prompt, and speculative compaction stay SDK-owned, and failed manual attempts are now recorded by the compaction circuit breaker instead of being invisible to it. - A plain `--session ` resume on the `claude-sdk-oauth` provider no longer re-sends the whole conversation with `Session continuity lost (options_changed)` after a restart (code-yeongyu/oh-my-openagent#7884). A restored session binding whose system prompt or toolset fingerprint drifted - an engine upgrade, a prompt-content change, or the UTC date rolling over - now reattaches to the existing SDK session and sends only the new turn, the same way a live session already did; only an account or model identity change still cold-seeds. The continuity observation names which half drifted (`system_prompt_changed` / `toolset_changed`) instead of the bare `options_changed`. - The `Current date:` line is normalized out of the `claude-sdk-oauth` prompt fingerprint even when extension prompt sections follow the `Current working directory:` line, which is the shape every real session has. Previously the normalization only matched when that line ended the prompt, so the fingerprint changed at every UTC midnight. diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md index 8ec10e71f..c1ceb8fd2 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md @@ -1,5 +1,25 @@ # changes.md — builtin compaction policy +## Allow explicit manual compaction on SDK-owned automatic lanes (2026-09-07) + +### What changed + +- `lane-policy.ts` exposes one reason-aware `ownsCompaction` predicate: manual requests are senpi-owned for recovery even on SDK-native lanes; automatic threshold, overflow, pre-prompt, and speculative routes remain SDK-owned. The predicate is used for the before-compact admission and failure-accounting sites; the message-end degradation site is automatic-only and remains unchanged. +- Failed manual compactions are recorded by the circuit breaker, including on SDK-native lanes; manual requests no longer bypass the breaker. +- Concurrent SDK/native and senpi work is protected by the existing speculative generation and message-revision checks before generated results are applied. + +### Why + +- A rejected downsizing leaves the larger model selected so the user can compact first. Cancelling explicit `/compact` with `external-owner` blocked that recovery; manual requests must reach the existing summary generation and persistence path without weakening model admission. + +### Why an extension could not handle it + +- The cancellation is owned by this builtin hook. Another extension cannot safely undo its rejection or replace the coordinated compaction lifecycle. + +### Expected merge conflict zones + +- LOW: `packages/coding-agent/src/core/extensions/builtin/compaction/index.ts` around the SDK-native lane guard in `session_before_compact`. + ## Omit speculation lead from resumed-session admission (2026-09-03) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/circuit-breaker.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/circuit-breaker.ts index 81c408c0a..224ab0512 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/circuit-breaker.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/circuit-breaker.ts @@ -54,6 +54,14 @@ export function isTripped(state: CompactionExtensionState, now: number): boolean return state.trippedAt !== null && now < state.trippedAt + COOLDOWN_MS; } +/** + * A tripped breaker halts *automatic* compaction only. An explicit `/compact` is the + * user's escape hatch - and on an SDK-owned lane it is the documented recovery from a + * rejected model downswitch - so refusing it during the cooldown would strand the very + * session the recovery exists for. Manual failures are still recorded (see the + * `ownsCompaction` failure-accounting site in `index.ts`), so they count toward the + * trip that protects the automatic routes. + */ export function shouldBypass(_state: CompactionExtensionState, opts?: ShouldBypassOptions): boolean { if (opts?.manual === true) return true; if (opts?.reason === "manual") return true; diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts index f1b73a0f8..f323865e5 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts @@ -556,7 +556,7 @@ export default function compactionExtension( let warmJobConsumed = false; invalidateSpeculativeCompaction(ctx); try { - if (lanePolicy.disablesSenpiCompaction(ctx)) { + if (!lanePolicy.ownsCompaction(ctx, event.reason)) { return { cancel: true, rejectionCause: "external-owner", @@ -763,7 +763,7 @@ export default function compactionExtension( return; } if (compactEvent.rejectionCause === "external-owner") return; - if (!lanePolicy.disablesSenpiCompaction(ctx)) { + if (lanePolicy.ownsCompaction(ctx, compactEvent.reason)) { state = breaker.recordFailure(state, Date.now(), { route: compactEvent.reason }); } }); diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/lane-policy.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/lane-policy.ts index dd4e6ddf9..cc32233e5 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/lane-policy.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/lane-policy.ts @@ -14,8 +14,10 @@ * This module also owns the shape of the mirrored `compact_boundary` ledger * entry, so the SDK's native compactions stay visible in senpi history. */ + import type { AgentMessage } from "@earendil-works/pi-agent-core"; import type { AssistantMessageDiagnostic } from "@earendil-works/pi-ai"; +import type { CompactionReason } from "../../types.ts"; import { CLAUDE_SDK_OAUTH_PROVIDER_ID } from "../claude-sdk-oauth/account-management.ts"; import type { ClaudeSdkOauthProviderSettings } from "../claude-sdk-oauth/settings.ts"; import { loadClaudeSdkOauthProviderSettingsFromDisk } from "../claude-sdk-oauth/settings.ts"; @@ -57,6 +59,8 @@ export interface LaneContext { export interface CompactionLanePolicy { /** True when the SDK owns this lane's context and senpi compaction must stand down. */ disablesSenpiCompaction(context: LaneContext): boolean; + /** Manual requests are explicitly owned by senpi for recovery, even on SDK lanes. */ + ownsCompaction(context: LaneContext, reason: CompactionReason): boolean; } export interface CompactBoundaryEntry { @@ -90,28 +94,36 @@ export function createCompactionLanePolicy( const load = options.loadProviderSettings ?? loadClaudeSdkOauthProviderSettingsFromDisk; let cachedCwd: string | undefined; let cachedResumeMode: string | undefined; - return { - disablesSenpiCompaction(context: LaneContext): boolean { - if (context.model?.provider !== CLAUDE_SDK_OAUTH_PROVIDER_ID) return false; - // A configured compaction model override makes senpi own summarization - // for the lane, so the SDK-native stand-down no longer applies. This is - // the escape hatch for lanes whose SDK never fires native compaction. - if (context.getCompactionSettings?.().model) return false; - // Per-cwd cache is the intended contract (pinned by lane-policy.test.ts): - // resumeMode is read once per cwd. A mid-session switch takes effect on - // the next cwd or session. - if (cachedCwd !== context.cwd) { - try { - cachedResumeMode = load(context.cwd).resumeMode; - } catch { - // A settings read failure must never silently disable senpi compaction: - // fail closed by keeping senpi's own compaction fully active. - cachedCwd = undefined; - return false; - } - cachedCwd = context.cwd; + // Declared as a local so `ownsCompaction` never depends on `this`: the policy object + // is routinely destructured at call sites, which would otherwise unbind the receiver. + const disablesSenpiCompaction = (context: LaneContext): boolean => { + if (context.model?.provider !== CLAUDE_SDK_OAUTH_PROVIDER_ID) return false; + // A configured compaction model override makes senpi own summarization + // for the lane, so the SDK-native stand-down no longer applies. This is + // the escape hatch for lanes whose SDK never fires native compaction. + if (context.getCompactionSettings?.().model) return false; + // Per-cwd cache is the intended contract (pinned by lane-policy.test.ts): + // resumeMode is read once per cwd. A mid-session switch takes effect on + // the next cwd or session. + if (cachedCwd !== context.cwd) { + try { + cachedResumeMode = load(context.cwd).resumeMode; + } catch { + // A settings read failure must never silently disable senpi compaction: + // fail closed by keeping senpi's own compaction fully active. + cachedCwd = undefined; + return false; } - return isSdkNativeCompactionLane({ model: context.model, resumeMode: cachedResumeMode }); + cachedCwd = context.cwd; + } + return isSdkNativeCompactionLane({ model: context.model, resumeMode: cachedResumeMode }); + }; + return { + disablesSenpiCompaction, + ownsCompaction(context: LaneContext, reason: CompactionReason): boolean { + // Manual is senpi-owned everywhere: it is the user's explicit recovery path, + // including on an SDK-native lane whose automatic routes stay SDK-owned. + return reason === "manual" || !disablesSenpiCompaction(context); }, }; } diff --git a/packages/coding-agent/test/claude-sdk-oauth-compaction-alignment.test.ts b/packages/coding-agent/test/claude-sdk-oauth-compaction-alignment.test.ts index 5cec021c9..d37fd06f4 100644 --- a/packages/coding-agent/test/claude-sdk-oauth-compaction-alignment.test.ts +++ b/packages/coding-agent/test/claude-sdk-oauth-compaction-alignment.test.ts @@ -215,17 +215,23 @@ describe("claude-sdk-oauth lane: senpi compaction stands down", () => { expect(harness.registration.state.callCount).toBe(0); }); - it("cancels a requested senpi compaction with the lane reason", async () => { - const harness = createHarness({ provider: "claude-sdk-oauth" }); - - const result = await harness.sessionBeforeCompact(beforeCompactEvent(), harness.ctx); - - expect(result).toMatchObject({ - cancel: true, - reason: SDK_NATIVE_LANE_REJECTION_REASON, - rejectionCause: "external-owner", - }); - }); + it.each(["threshold", "overflow", "pre_prompt"] as const)( + "cancels automatic %s compaction with the lane reason", + async (reason) => { + const harness = createHarness({ provider: "claude-sdk-oauth" }); + + const result = await harness.sessionBeforeCompact( + { ...beforeCompactEvent(), reason, willRetry: reason === "overflow" }, + harness.ctx, + ); + + expect(result).toMatchObject({ + cancel: true, + reason: SDK_NATIVE_LANE_REJECTION_REASON, + rejectionCause: "external-owner", + }); + }, + ); it("leaves context messages untouched while the same load reduces them for other providers", () => { const reductionMessages = () => [ diff --git a/packages/coding-agent/test/suite/sdk-manual-compaction-recovery.test.ts b/packages/coding-agent/test/suite/sdk-manual-compaction-recovery.test.ts new file mode 100644 index 000000000..22ea84850 --- /dev/null +++ b/packages/coding-agent/test/suite/sdk-manual-compaction-recovery.test.ts @@ -0,0 +1,104 @@ +import { fauxAssistantMessage } from "@earendil-works/pi-ai"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import compactionExtension from "../../src/core/extensions/builtin/compaction/index.ts"; +import { ModelUsabilityBudgetError } from "../../src/core/extensions/builtin/compaction/model-usability-budget.ts"; +import { SessionManager } from "../../src/core/session-manager.ts"; +import { createHarness, type Harness } from "./harness.ts"; + +const harnesses: Harness[] = []; +afterEach(() => { + for (const harness of harnesses.splice(0)) harness.cleanup(); + vi.restoreAllMocks(); +}); + +/** + * `claude-sdk-oauth` is the regression case: it fails without the manual ownership + * exemption, with `rejectionCause: "external-owner"`. `faux` is a deliberate control + * for the lane the exemption must NOT change - it reaches the same persisted-summary + * outcome through the ordinary senpi-owned path, and it fails if the shared + * `ownsCompaction` predicate ever regresses the non-SDK branch. + */ +describe("explicit compaction recovers a rejected model downswitch", () => { + it.each(["claude-sdk-oauth", "faux"])("persists a usable manual summary on %s", async (provider) => { + // Given the real builtin, no compaction-model override, and an oversized live transcript. + vi.spyOn(Date, "now").mockReturnValue(1_800_000_000_000); + const harness = await createHarness({ + provider, + models: [ + { id: "million", contextWindow: 1_000_000, maxTokens: 32_000 }, + { id: "target", contextWindow: 272_000, maxTokens: 32_000 }, + ], + settings: { compaction: { enabled: true, keepRecentTokens: 1 } }, + extensionFactories: [compactionExtension], + persistSession: true, + }); + harnesses.push(harness); + const source = harness.getModel(); + const target = harness.getModel("target"); + if (!target) throw new Error("missing target fixture"); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "historical context ".repeat(30_000) }], + timestamp: 1, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("historical response", { timestamp: 2 }), + api: source.api, + provider, + model: source.id, + usage: { + input: 845_096, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 845_096, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }); + const keptEntryId = harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "KEEP_AFTER_COMPACTION" }], + timestamp: 3, + }); + harness.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + await expect(harness.session.setModel(target)).rejects.toMatchObject({ + name: ModelUsabilityBudgetError.name, + projection: { usable: false, contextWindow: 272_000 }, + }); + expect(harness.session.model?.id).toBe("million"); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([]); + const summary = "MANUAL_COMPACTION_RECOVERY_SUMMARY"; + harness.setResponses([fauxAssistantMessage(summary, { timestamp: 4 })]); + + // When explicit /compact's session entry point runs, the harness is already subscribed + // to lifecycle events. Await its terminal promise, not elapsed time or polling. + const outcome = await harness.session.compact().then( + (result) => ({ status: "compacted" as const, result }), + (error: unknown) => ({ status: "rejected" as const, error }), + ); + + // Then cancellation is not recovery: require an actual result and persisted usable history. + expect(outcome, JSON.stringify(harness.eventsOfType("compaction_end"))).toMatchObject({ status: "compacted" }); + if (outcome.status !== "compacted") throw outcome.error; + expect(outcome.result).toMatchObject({ summary, firstKeptEntryId: keptEntryId }); + expect(harness.eventsOfType("compaction_end")).toEqual([ + expect.objectContaining({ reason: "manual", accepted: true, aborted: false }), + ]); + const sessionFile = harness.sessionManager.getSessionFile(); + if (!sessionFile) throw new Error("missing persisted session fixture"); + const reopened = SessionManager.open(sessionFile); + expect(reopened.getEntries().filter((entry) => entry.type === "compaction")).toEqual([ + expect.objectContaining({ summary, firstKeptEntryId: keptEntryId }), + ]); + expect(reopened.buildSessionContext().messages).toEqual(harness.session.messages); + expect(harness.session.messages).toEqual([ + expect.objectContaining({ role: "compactionSummary", summary }), + expect.objectContaining({ role: "user", content: [{ type: "text", text: "KEEP_AFTER_COMPACTION" }] }), + ]); + await harness.session.setModel(target); + expect(harness.session.model?.id).toBe("target"); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([ + expect.objectContaining({ provider, modelId: "target" }), + ]); + }); +});