diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8bfa5e333..c52a42930 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed `/goal` threshold auto-compaction never firing whenever the per-turn supersede/drop-useless prune saved ≥`compaction.thresholdTokens - calculateContextTokens(usage)` tokens. The pre-fix code subtracted prune savings from the threshold input, so a long-running goal session whose visible context (anchored to the same provider billing) sat above `compaction.thresholdTokens` could keep growing past it indefinitely. Threshold maintenance now triggers from the actual last-turn billed context, with the post-prune local estimate kept as a payload-compression floor. ([#3174](https://github.com/can1357/oh-my-pi/issues/3174)) +- Fixed `/goal` threshold auto-compaction skipping real sessions through two separate paths: per-turn supersede/drop-useless pruning no longer deflates the threshold trigger below the last provider-billed context, and active-goal text stops now attempt threshold maintenance before empty/unexpected-stop retry continuations can return from post-turn handling. The threshold decision also logs the billed, stored, resolved, and post-maintenance token counts so future no-start reports identify the exact skip path. ([#3174](https://github.com/can1357/oh-my-pi/issues/3174)) ## [16.1.10] - 2026-06-21 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 7902a02c3..36dac06fa 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2790,9 +2790,10 @@ export class AgentSession { return; } + const activeGoal = this.#goalModeState?.enabled === true && this.#goalModeState.goal.status === "active"; if (this.#assistantEndedWithSuccessfulYield(msg)) { this.#lastSuccessfulYieldToolCallId = undefined; - if (this.#goalModeState?.enabled && this.#goalModeState.goal.status === "active") { + if (activeGoal) { const compactionTask = this.#checkCompaction(msg); this.#trackPostPromptTask(compactionTask); await compactionTask; @@ -2802,6 +2803,19 @@ export class AgentSession { } this.#lastSuccessfulYieldToolCallId = undefined; + let compactionResult = COMPACTION_CHECK_NONE; + let checkedCompaction = false; + if (activeGoal) { + const compactionTask = this.#checkCompaction(msg); + this.#trackPostPromptTask(compactionTask); + compactionResult = await compactionTask; + checkedCompaction = true; + if (compactionResult.deferredHandoff || compactionResult.continuationScheduled) { + await emitAgentEndNotification(); + return; + } + } + if (await this.#handleEmptyAssistantStop(msg)) { await emitAgentEndNotification(); return; @@ -2846,9 +2860,11 @@ export class AgentSession { } this.#resolveRetry(); - const compactionTask = this.#checkCompaction(msg); - this.#trackPostPromptTask(compactionTask); - const compactionResult = await compactionTask; + if (!checkedCompaction) { + const compactionTask = this.#checkCompaction(msg); + this.#trackPostPromptTask(compactionTask); + compactionResult = await compactionTask; + } // Check for incomplete todos only after a final assistant stop, not intermediate tool-use turns. const hasToolCalls = msg.content.some(content => content.type === "toolCall"); if (hasToolCalls) { @@ -8323,7 +8339,7 @@ export class AgentSession { // Stale-result pass runs every turn, before any threshold gating: it is // cheap (bails when no candidate) and independent of the compaction // setting. - await this.#pruneStaleToolResults(); + const supersedeResult = await this.#pruneStaleToolResults(); const compactionSettings = this.settings.getGroup("compaction"); if (!compactionSettings.enabled || compactionSettings.strategy === "off") return COMPACTION_CHECK_NONE; @@ -8331,7 +8347,10 @@ export class AgentSession { // Case 4: Threshold - turn succeeded but context is getting large // Skip if this was an error (non-overflow errors don't have usage data) if (assistantMessage.stopReason === "error") return COMPACTION_CHECK_NONE; - await this.#pruneToolOutputs(); + const pruneResult = await this.#pruneToolOutputs(); + const maintenanceTokensFreed = (supersedeResult?.tokensSaved ?? 0) + (pruneResult?.tokensSaved ?? 0); + const assistantUsageContextTokens = calculateContextTokens(assistantMessage.usage); + const storedContextTokens = this.#estimateStoredContextTokens(); // Pruning frees bytes for the NEXT prompt; it does not change the size of // the prompt the LLM just billed for. Earlier revisions subtracted the // per-turn supersede/prune `tokensSaved` from the threshold input, which @@ -8339,22 +8358,48 @@ export class AgentSession { // indefinitely whenever per-turn pruning saved enough to drop the // post-prune estimate below the user-configured trigger — the visible // context (anchored to the same provider billing) still showed >threshold, - // but `shouldCompact` no-op'd (#3174). Anchor on the last turn's billed - // context tokens, floored by the post-prune stored-conversation estimate - // so a payload-compression hook still can't deflate the trigger. - const contextTokens = compactionContextTokens( - calculateContextTokens(assistantMessage.usage), - this.#estimateStoredContextTokens(), + // but `shouldCompact` no-op'd (#3174). Anchor the initial trigger on the + // last turn's billed context tokens, floored by the post-prune + // stored-conversation estimate so a payload-compression hook still can't + // deflate the trigger. + const contextTokens = compactionContextTokens(assistantUsageContextTokens, storedContextTokens); + const postMaintenanceContextTokens = compactionContextTokens( + Math.max(0, assistantUsageContextTokens - maintenanceTokensFreed), + storedContextTokens, ); - if (shouldCompact(contextTokens, contextWindow, compactionSettings)) { + const thresholdTokens = resolveThresholdTokens(contextWindow, compactionSettings); + const shouldThresholdCompact = shouldCompact(contextTokens, contextWindow, compactionSettings); + logger.debug("Auto-compaction threshold decision", { + phase: "post-agent-end", + goalModeEnabled: this.#goalModeState?.enabled === true, + goalStatus: this.#goalModeState?.goal.status, + stopReason: assistantMessage.stopReason, + sameModel: sameModel === true, + contextWindow, + strategy: compactionSettings.strategy, + thresholdTokens, + assistantUsageContextTokens, + storedContextTokens, + resolvedContextTokens: contextTokens, + postMaintenanceContextTokens, + maintenanceTokensFreed, + shouldCompact: shouldThresholdCompact, + contextPromotionEnabled: this.settings.get("contextPromotion.enabled") === true, + }); + if (shouldThresholdCompact) { // Try promotion first — if a larger model is available, switch instead of compacting const promoted = await this.#tryContextPromotion(assistantMessage); if (!promoted) { return await this.#runAutoCompaction("threshold", false, false, allowDefer, { autoContinue, - triggerContextTokens: contextTokens, + triggerContextTokens: postMaintenanceContextTokens, }); } + logger.debug("Auto-compaction threshold satisfied but context promotion took over", { + contextTokens, + contextWindow, + model: `${assistantMessage.provider}/${assistantMessage.model}`, + }); } return COMPACTION_CHECK_NONE; } @@ -10008,15 +10053,16 @@ export class AgentSession { // situation actually resolves; "idle" is exempt because its 60s+ timer // re-checks usage before re-firing and cannot dead-loop on its own. // - // #2275: the post-shake check MUST be anchored on the same metric that - // triggered compaction. The local estimator (`#estimatePendingPromptTokens`) - // undercounts thinking-signature payloads, so on thinking-heavy sessions it - // reads well below the provider-reported usage that fired the threshold. - // When that estimate slips under the threshold, the fallback never fires - // and the auto-continue prompt re-injects every turn. Prefer the trigger's - // own `contextTokens` (provider-anchored) when the caller supplies it, and - // add hysteresis (80% recovery band) so we don't oscillate at the boundary - // while shake keeps reclaiming a trickle of the previous turn's output. + // #2275: the post-shake check MUST stay provider-anchored when caller + // usage and local estimates diverge. The local estimator undercounts + // thinking-signature payloads, so thinking-heavy sessions can read well + // below the provider usage that fired the threshold. Prefer the caller's + // context figure when supplied, then subtract shake's own savings and add + // hysteresis (80% recovery band) so we don't oscillate at the boundary. + // Threshold callers pass the provider-billed trigger after accounting for + // any supersede/drop-useless pruning that already rewrote the next prompt; + // without that pre-shake savings, shake can fall through to context-full + // even though the post-prune history is already inside the recovery band. const contextWindow = this.model?.contextWindow ?? 0; const compactionSettings = this.settings.getGroup("compaction"); let stillOverThreshold = false; diff --git a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts index cb86914e8..95eb5699d 100644 --- a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts +++ b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts @@ -8,6 +8,7 @@ import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { loadExtensions } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/loader"; import { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/runner"; import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import * as unexpectedStopClassifier from "@oh-my-pi/pi-coding-agent/session/unexpected-stop-classifier"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { getProjectAgentDir, TempDir, withTimeout } from "@oh-my-pi/pi-utils"; @@ -418,6 +419,59 @@ describe("AgentSession auto-compaction queue resume", () => { expect(runtimeSignals).toContain("compaction:start:threshold"); expect(runtimeSignals.some(signal => signal.startsWith("compaction:end:"))).toBe(true); }); + it("runs active-goal threshold compaction before unexpected-stop retry continuation", async () => { + const now = Date.now(); + session.setGoalModeState({ + enabled: true, + mode: "active", + goal: { + id: "goal-unexpected-stop-threshold", + objective: "continue until compacted", + status: "active", + tokensUsed: 0, + timeUsedSeconds: 0, + createdAt: now, + updatedAt: now, + }, + }); + session.settings.set("compaction.thresholdTokens", 76384); + session.settings.set("compaction.thresholdPercent", -1); + session.settings.set("compaction.autoContinue", true); + session.settings.set("contextPromotion.enabled", false); + session.settings.set("features.unexpectedStopDetection", true); + session.settings.set("providers.unexpectedStopModel", "online"); + + vi.spyOn(unexpectedStopClassifier, "classifyUnexpectedStop").mockResolvedValue(true); + vi.spyOn(session.agent, "continue").mockImplementation(async () => { + session.agent.clearAllQueues(); + }); + + const assistantMsg = { + role: "assistant" as const, + content: [{ type: "text" as const, text: "I should continue investigating another module." }], + api: "anthropic-messages" as const, + provider: "anthropic" as const, + model: "claude-sonnet-4-5", + stopReason: "stop" as const, + usage: { + input: 5000, + output: 1000, + cacheRead: 85000, + cacheWrite: 0, + totalTokens: 91000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: now, + }; + + session.agent.emitExternalEvent({ type: "message_end", message: assistantMsg }); + session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMsg] }); + + await session.waitForIdle(); + + expect(getRuntimeSignals()).toContain("compaction:start:threshold"); + }); + it("has isCompacting true when the auto_compaction_start event fires", async () => { // Defect 1: the compaction AbortController (which backs isCompacting) must be // installed before auto_compaction_start is emitted. If it is installed after, diff --git a/packages/coding-agent/test/shake.test.ts b/packages/coding-agent/test/shake.test.ts index 14cc47744..01c7aff07 100644 --- a/packages/coding-agent/test/shake.test.ts +++ b/packages/coding-agent/test/shake.test.ts @@ -347,6 +347,70 @@ describe("AgentSession shake", () => { expect(fullStart).toBeDefined(); }); + it("counts pre-shake prune savings when deciding whether to fall back to context-full", async () => { + session.settings.set("compaction.strategy", "shake"); + session.settings.set("compaction.thresholdTokens", 76384); + session.settings.set("compaction.thresholdPercent", -1); + session.settings.set("compaction.dropUseless", true); + session.settings.set("contextPromotion.enabled", false); + + const now = Date.now(); + sessionManager.appendMessage({ + role: "user", + content: "Investigate every module of the project.", + timestamp: now - 200, + }); + const bigCallId = "call-big-useless-for-shake"; + sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "toolCall", id: bigCallId, name: "search", arguments: { pattern: "TODO" } }], + ...apiInfo, + stopReason: "toolUse", + usage, + timestamp: now - 180, + }); + sessionManager.appendMessage({ + role: "toolResult", + toolCallId: bigCallId, + toolName: "search", + content: [{ type: "text", text: "match line\n".repeat(20000) }], + isError: false, + useless: true, + timestamp: now - 170, + }); + session.agent.replaceMessages(session.buildDisplaySessionContext().messages); + + const shakeSpy = vi + .spyOn(session, "shake") + .mockResolvedValue({ mode: "elide", toolResultsDropped: 1, blocksDropped: 0, tokensFreed: 100 }); + + const assistantMessage: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "trigger" }], + ...apiInfo, + stopReason: "stop", + usage: { + input: 5000, + output: 1000, + cacheRead: 85000, + cacheWrite: 0, + totalTokens: 91000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: now, + }; + + session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); + session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); + await Bun.sleep(50); + + expect(shakeSpy).toHaveBeenCalledTimes(1); + const fullStart = events.find( + event => event.type === "auto_compaction_start" && (event as { action?: string }).action === "context-full", + ); + expect(fullStart).toBeUndefined(); + }); + it("falls back after pre-prompt shake when the floored stored conversation remains over threshold", async () => { session.settings.set("compaction.strategy", "shake"); session.settings.set("compaction.thresholdTokens", 8_000);