From 3d29a48e7ae758ab8d2e3cfd90fae2ba8816dd7e Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 5 Apr 2026 03:50:52 +0200 Subject: [PATCH] fix(coding-agent): fixed memory leak by cancelling idle compaction - Fixed memory leak by cancelling idle compaction timer on event controller disposal. - Fixed session resumption to preserve last non-empty session when starting fresh. - Fixed stash detection to use git ref resolution instead of output parsing for reliability. - Fixed secret obfuscation to deobfuscate restored session messages locally while keeping LLM messages obfuscated. - Fixed stash pop operation to preserve staged changes with --index flag after task branch merges. - Changed idle compaction settings from enum to numeric type for flexible configuration. --- .../test/github-copilot-model-limits.test.ts | 12 +- packages/coding-agent/CHANGELOG.md | 8 +- .../src/config/settings-schema.ts | 6 +- packages/coding-agent/src/main.ts | 2 +- .../src/modes/controllers/event-controller.ts | 5 + .../src/modes/interactive-mode.ts | 3 +- packages/coding-agent/src/sdk.ts | 40 ++++-- packages/coding-agent/src/secrets/index.ts | 2 +- .../coding-agent/src/secrets/obfuscator.ts | 10 ++ .../coding-agent/src/session/agent-session.ts | 26 ++-- .../src/session/session-manager.ts | 4 +- packages/coding-agent/src/task/worktree.ts | 2 +- packages/coding-agent/src/utils/git.ts | 17 ++- packages/coding-agent/test/config-cli.test.ts | 32 +++++ .../event-controller-idle-compaction.test.ts | 79 +++++++++++ .../test/sdk-session-isolation.test.ts | 134 ++++++++++++++++++ .../session-manager/file-operations.test.ts | 21 +++ .../coding-agent/test/task/worktree.test.ts | 87 ++++++++++-- 18 files changed, 434 insertions(+), 56 deletions(-) create mode 100644 packages/coding-agent/test/modes/controllers/event-controller-idle-compaction.test.ts diff --git a/packages/ai/test/github-copilot-model-limits.test.ts b/packages/ai/test/github-copilot-model-limits.test.ts index da3fc5424..2ef746a04 100644 --- a/packages/ai/test/github-copilot-model-limits.test.ts +++ b/packages/ai/test/github-copilot-model-limits.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import { Effort } from "../src/model-thinking"; +import { getBundledModel } from "../src/models"; import { githubCopilotModelManagerOptions } from "../src/provider-models/openai-compat"; const originalFetch = global.fetch; @@ -87,7 +88,7 @@ describe("github copilot model limits mapping", () => { const model = models.find(candidate => candidate.id === "gemini-2.5-pro"); expect(model).toBeDefined(); - expect(model?.contextWindow).toBe(1_048_576); + expect(model?.contextWindow).toBe(128_000); expect(model?.maxTokens).toBe(64_000); expect(fetchMock).toHaveBeenCalledTimes(1); }); @@ -137,9 +138,16 @@ describe("github copilot model limits mapping", () => { const model = models.find(candidate => candidate.id === "claude-opus-4.6"); expect(model).toBeDefined(); - expect(model?.contextWindow).toBe(200_000); + expect(model?.contextWindow).toBe(128_000); expect(model?.maxTokens).toBe(16_000); }); + + it("keeps bundled Claude Opus 4.6 Copilot prompt budget truthful offline", () => { + const model = getBundledModel("github-copilot", "claude-opus-4.6"); + + expect(model.contextWindow).toBe(128_000); + expect(model.maxTokens).toBe(64_000); + }); it("inherits bundled GPT-5.4 mini reasoning metadata during discovery", async () => { const { models } = await discoverCopilotModels({ data: [ diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index df95232bf..081af41e2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,10 +1,8 @@ # Changelog ## [Unreleased] - ### Added - - Added idle auto-compaction settings and scheduling so sessions can compact after inactive turns without auto-continuing. - Added `onExternalEditor` callback to extension UI dialog options for handling external editor shortcut in select dialogs - Added external editor shortcut support in plan review selector, allowing users to open and edit the plan in their configured editor @@ -16,6 +14,9 @@ ### Changed +- Changed idle compaction settings (`compaction.idleThresholdTokens` and `compaction.idleTimeoutSeconds`) from enum to numeric type for flexible configuration +- Modified secret obfuscation to deobfuscate restored session messages for local display while keeping outbound LLM messages obfuscated +- Updated stash pop operation to preserve staged changes with `--index` flag when restoring after task branch merges - Changed secret placeholders to deterministic hash-style redaction tokens and deobfuscated assistant output for local display. - Updated hook editor and hook selector components to use `matchesAppExternalEditor` matcher for consistent external editor keybinding detection - Modified plan review flow to read the latest plan content from disk before approval, allowing changes made in external editor to be reflected @@ -37,6 +38,9 @@ ### Fixed +- Fixed idle compaction timer to properly cancel when event controller is disposed, preventing memory leaks +- Fixed session resumption to preserve the last non-empty session when starting a fresh session +- Fixed stash detection to use git ref resolution instead of output parsing for reliable stash state tracking - Fixed isolated task merge-back to preserve task outputs on merge failure and stash dirty worktrees before cherry-pick. - Fixed web search source rendering to truncate long title, metadata, and URL lines before they overflow the UI. - Fixed PR checkout tool to resolve symlinks in worktree paths, ensuring consistent path references in results and metadata diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 3a14fd583..c3bf30380 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -818,8 +818,7 @@ export const SETTINGS_SCHEMA = { }, "compaction.idleThresholdTokens": { - type: "enum", - values: [100000, 200000, 300000, 400000, 500000, 600000, 700000, 800000, 900000] as const, + type: "number", default: 200000, ui: { tab: "context", @@ -830,8 +829,7 @@ export const SETTINGS_SCHEMA = { }, "compaction.idleTimeoutSeconds": { - type: "enum", - values: [60, 120, 300, 600, 1800, 3600] as const, + type: "number", default: 300, ui: { tab: "context", diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 8e31efefc..ba3d664b8 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -289,7 +289,7 @@ async function createSessionManager(parsed: Args, cwd: string): Promise e.type === "message")) { + if (manager.getEntries().length > 0) { parsed.continue = true; } return manager; diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index b12d8f98d..eb8fea139 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -25,6 +25,10 @@ export class EventController { #idleCompactionTimer?: NodeJS.Timeout; constructor(private ctx: InteractiveModeContext) {} + dispose(): void { + this.#cancelIdleCompaction(); + } + #resetReadGroup(): void { this.#lastReadGroup = undefined; } @@ -607,6 +611,7 @@ export class EventController { if (this.#currentContextTokens() < threshold) return; void this.ctx.session.runIdleCompaction(); }, timeoutMs); + this.#idleCompactionTimer.unref?.(); } #currentContextTokens(): number { diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 88ef9291f..b7b790152 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -539,7 +539,7 @@ export class InteractiveMode implements InteractiveModeContext { rebuildChatFromMessages(): void { this.chatContainer.clear(); - const context = this.sessionManager.buildSessionContext(); + const context = this.session.buildDisplaySessionContext(); this.renderSessionContext(context); } @@ -989,6 +989,7 @@ export class InteractiveMode implements InteractiveModeContext { this.#extensionUiController.clearExtensionTerminalInputListeners(); this.#extensionUiController.clearHookWidgets(); this.#observerRegistry.dispose(); + this.#eventController.dispose(); this.statusLine.dispose(); if (this.#resizeHandler) { process.stdout.removeListener("resize", this.#resizeHandler); diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 20b5ec273..e23cb42b7 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -83,7 +83,13 @@ import { } from "./mcp/discoverable-tool-metadata"; import { buildMemoryToolDeveloperInstructions, getMemoryRoot, startMemoryStartupTask } from "./memories"; import asyncResultTemplate from "./prompts/tools/async-result.md" with { type: "text" }; -import { collectEnvSecrets, loadSecrets, obfuscateMessages, SecretObfuscator } from "./secrets"; +import { + collectEnvSecrets, + deobfuscateSessionContext, + loadSecrets, + obfuscateMessages, + SecretObfuscator, +} from "./secrets"; import { AgentSession } from "./session/agent-session"; import { AuthStorage } from "./session/auth-storage"; import { convertToLlm } from "./session/messages"; @@ -693,8 +699,23 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} return hasKey; }; + // Load and create secret obfuscator early so resumed session state and prompt warnings + // reflect actual loaded secrets, not just the setting toggle. + let obfuscator: SecretObfuscator | undefined; + if (settings.get("secrets.enabled")) { + const fileEntries = await logger.timeAsync("loadSecrets", loadSecrets, cwd, agentDir); + const envEntries = collectEnvSecrets(); + const allEntries = [...envEntries, ...fileEntries]; + if (allEntries.length > 0) { + obfuscator = new SecretObfuscator(allEntries); + } + } + const secretsEnabled = obfuscator?.hasSecrets() === true; + // Check if session has existing data to restore - const existingSession = logger.time("loadSession", () => sessionManager.buildSessionContext()); + const existingSession = logger.time("loadSession", () => + deobfuscateSessionContext(sessionManager.buildSessionContext(), obfuscator), + ); const existingBranch = sessionManager.getBranch(); const hasExistingSession = existingBranch.length > 0; const hasThinkingEntry = existingBranch.some(entry => entry.type === "thinking_level_change"); @@ -1278,7 +1299,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} mcpDiscoveryMode: hasDiscoverableMCPTools, mcpDiscoveryServerSummaries: discoverableMCPSummary.servers.map(formatDiscoverableMCPToolServerSummary), eagerTasks, - secretsEnabled: settings.get("secrets.enabled"), + secretsEnabled, }); if (options.systemPrompt === undefined) { @@ -1301,7 +1322,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} mcpDiscoveryMode: hasDiscoverableMCPTools, mcpDiscoveryServerSummaries: discoverableMCPSummary.servers.map(formatDiscoverableMCPToolServerSummary), eagerTasks, - secretsEnabled: settings.get("secrets.enabled"), + secretsEnabled, }); } return options.systemPrompt(defaultPrompt); @@ -1411,17 +1432,6 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} }); }; - // Load and create secret obfuscator if secrets are enabled - let obfuscator: SecretObfuscator | undefined; - if (settings.get("secrets.enabled")) { - const fileEntries = await logger.timeAsync("loadSecrets", loadSecrets, cwd, agentDir); - const envEntries = collectEnvSecrets(); - const allEntries = [...envEntries, ...fileEntries]; - if (allEntries.length > 0) { - obfuscator = new SecretObfuscator(allEntries); - } - } - // Final convertToLlm: chain block-images filter with secret obfuscation const convertToLlmFinal = (messages: AgentMessage[]): Message[] => { const converted = convertToLlmWithBlockImages(messages); diff --git a/packages/coding-agent/src/secrets/index.ts b/packages/coding-agent/src/secrets/index.ts index 68ce08ff4..420150c25 100644 --- a/packages/coding-agent/src/secrets/index.ts +++ b/packages/coding-agent/src/secrets/index.ts @@ -4,7 +4,7 @@ import { YAML } from "bun"; import type { SecretEntry } from "./obfuscator"; import { compileSecretRegex } from "./regex"; -export { obfuscateMessages, type SecretEntry, SecretObfuscator } from "./obfuscator"; +export { deobfuscateSessionContext, obfuscateMessages, type SecretEntry, SecretObfuscator } from "./obfuscator"; /** * Load secrets from project-local and global secrets.yml files. diff --git a/packages/coding-agent/src/secrets/obfuscator.ts b/packages/coding-agent/src/secrets/obfuscator.ts index abed04c6a..b3ea0fea0 100644 --- a/packages/coding-agent/src/secrets/obfuscator.ts +++ b/packages/coding-agent/src/secrets/obfuscator.ts @@ -1,4 +1,5 @@ import type { Message, TextContent } from "@oh-my-pi/pi-ai"; +import type { SessionContext } from "../session/session-manager"; import { compileSecretRegex } from "./regex"; // ═══════════════════════════════════════════════════════════════════════════ @@ -197,6 +198,15 @@ export class SecretObfuscator { } } +export function deobfuscateSessionContext( + sessionContext: SessionContext, + obfuscator: SecretObfuscator | undefined, +): SessionContext { + if (!obfuscator?.hasSecrets()) return sessionContext; + const messages = obfuscator.deobfuscateObject(sessionContext.messages); + return messages === sessionContext.messages ? sessionContext : { ...sessionContext, messages }; +} + // ═══════════════════════════════════════════════════════════════════════════ // Message obfuscation (outbound to LLM) // ═══════════════════════════════════════════════════════════════════════════ diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 9b1012355..c64affdab 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -115,7 +115,7 @@ import planModeToolDecisionReminderPrompt from "../prompts/system/plan-mode-tool type: "text", }; import ttsrInterruptTemplate from "../prompts/system/ttsr-interrupt.md" with { type: "text" }; -import type { SecretObfuscator } from "../secrets/obfuscator"; +import { deobfuscateSessionContext, type SecretObfuscator } from "../secrets/obfuscator"; import { resolveThinkingLevelForModel, toReasoningEffort } from "../thinking"; import type { CheckpointState } from "../tools/checkpoint"; import { outputMeta } from "../tools/output-meta"; @@ -540,7 +540,7 @@ export class AgentSession { this.#defaultSelectedMCPServerNames = new Set(config.defaultSelectedMCPServerNames ?? []); this.#defaultSelectedMCPToolNames = new Set(config.defaultSelectedMCPToolNames ?? []); this.#pruneSelectedMCPToolNames(); - const persistedSelectedMCPToolNames = this.sessionManager.buildSessionContext().selectedMCPToolNames; + const persistedSelectedMCPToolNames = this.buildDisplaySessionContext().selectedMCPToolNames; const currentSelectedMCPToolNames = this.getSelectedMCPToolNames(); const persistInitialMCPToolSelection = config.persistInitialMCPToolSelection ?? this.sessionManager.getBranch().length === 0; @@ -1984,7 +1984,7 @@ export class AgentSession { this.#setDiscoverableMCPTools(this.#collectDiscoverableMCPToolsFromRegistry()); this.#pruneSelectedMCPToolNames(); - if (!this.sessionManager.buildSessionContext().hasPersistedMCPToolSelection) { + if (!this.buildDisplaySessionContext().hasPersistedMCPToolSelection) { this.#selectedMCPToolNames = new Set([ ...this.#selectedMCPToolNames, ...this.#getConfiguredDefaultSelectedMCPToolNames(), @@ -2009,6 +2009,10 @@ export class AgentSession { return this.agent.state.messages; } + buildDisplaySessionContext(): SessionContext { + return deobfuscateSessionContext(this.sessionManager.buildSessionContext(), this.#obfuscator); + } + /** Convert session messages using the same pre-LLM pipeline as the active session. */ async convertMessagesToLlm(messages: AgentMessage[], signal?: AbortSignal): Promise { const transformedMessages = await this.#transformContext(messages, signal); @@ -3497,7 +3501,7 @@ export class AgentSession { } await this.sessionManager.rewriteEntries(); - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); this.agent.replaceMessages(sessionContext.messages); this.#syncTodoPhasesFromBranch(); this.#closeCodexProviderSessionsForHistoryRewrite(); @@ -3623,7 +3627,7 @@ export class AgentSession { preserveData, ); const newEntries = this.sessionManager.getEntries(); - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); this.agent.replaceMessages(sessionContext.messages); this.#syncTodoPhasesFromBranch(); this.#closeCodexProviderSessionsForHistoryRewrite(); @@ -3844,7 +3848,7 @@ export class AgentSession { } // Rebuild agent messages from session - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); this.agent.replaceMessages(sessionContext.messages); this.#syncTodoPhasesFromBranch(); @@ -4730,7 +4734,7 @@ export class AgentSession { preserveData, ); const newEntries = this.sessionManager.getEntries(); - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); this.agent.replaceMessages(sessionContext.messages); this.#syncTodoPhasesFromBranch(); this.#closeCodexProviderSessionsForHistoryRewrite(); @@ -5557,7 +5561,7 @@ export class AgentSession { // Flush pending writes before switching so restore snapshots reflect committed state. await this.sessionManager.flush(); const previousSessionState = this.sessionManager.captureState(); - const previousSessionContext = this.sessionManager.buildSessionContext(); + const previousSessionContext = this.buildDisplaySessionContext(); // switchSession replaces these arrays wholesale during load/rollback, so retaining // the existing message objects is sufficient and avoids structured-clone failures for // extension/custom metadata that is valid to persist but not cloneable. @@ -5586,7 +5590,7 @@ export class AgentSession { await this.sessionManager.setSessionFile(sessionPath); this.agent.sessionId = this.sessionManager.getSessionId(); - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); const didReloadConversationChange = !switchingToDifferentSession && this.#didSessionMessagesChange(previousSessionContext.messages, sessionContext.messages); @@ -5752,7 +5756,7 @@ export class AgentSession { this.agent.sessionId = this.sessionManager.getSessionId(); // Reload messages from entries (works for both file and in-memory mode) - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); await this.#restoreMCPSelectionsForSessionContext(sessionContext); @@ -5923,7 +5927,7 @@ export class AgentSession { } // Update agent state - const sessionContext = this.sessionManager.buildSessionContext(); + const sessionContext = this.buildDisplaySessionContext(); await this.#restoreMCPSelectionsForSessionContext(sessionContext); this.agent.replaceMessages(sessionContext.messages); this.#syncTodoPhasesFromBranch(); diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 47c5765d8..460e9fc87 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -1512,9 +1512,7 @@ export class SessionManager { /** Start a new session. Closes any existing writer first. */ async newSession(options?: NewSessionOptions): Promise { await this.#closePersistWriter(); - const result = this.#newSessionSync(options); - await this.ensureOnDisk(); - return result; + return this.#newSessionSync(options); } /** diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index cdf0fb498..8ebb33dcd 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -551,7 +551,7 @@ export async function mergeTaskBranches( } finally { if (didStash) { try { - await git.stash.pop(repoRoot); + await git.stash.pop(repoRoot, { index: true }); } catch { // Stash-pop conflicts mean the replayed changes clash with the user's // uncommitted edits. Treat this as a merge failure so the caller preserves diff --git a/packages/coding-agent/src/utils/git.ts b/packages/coding-agent/src/utils/git.ts index 0f7521421..44c0a863d 100644 --- a/packages/coding-agent/src/utils/git.ts +++ b/packages/coding-agent/src/utils/git.ts @@ -1115,18 +1115,21 @@ export const cherryPick = Object.assign( // ════════════════════════════════════════════════════════════════════════════ export const stash = { - /** Stash working tree + index changes. Returns true if something was stashed. */ + /** Stash working tree + index changes. Returns true when git created a new stash entry. */ async push(cwd: string, message?: string): Promise { ensureAvailable(); + const previousStash = await ref.resolve(cwd, "refs/stash"); const args = ["stash", "push", "--include-untracked"]; if (message) args.push("-m", message); - const result = await runCommand(cwd, args); - // git stash push exits 0 whether or not it stashed; check output - return result.exitCode === 0 && !result.stdout.includes("No local changes to save"); + await runEffect(cwd, args); + const nextStash = await ref.resolve(cwd, "refs/stash"); + return nextStash !== null && nextStash !== previousStash; }, - /** Pop the most recent stash entry. */ - async pop(cwd: string): Promise { - await runEffect(cwd, ["stash", "pop"]); + /** Pop the most recent stash entry, optionally restoring its staged state. */ + async pop(cwd: string, options?: { index?: boolean }): Promise { + const args = ["stash", "pop"]; + if (options?.index) args.push("--index"); + await runEffect(cwd, args); }, }; diff --git a/packages/coding-agent/test/config-cli.test.ts b/packages/coding-agent/test/config-cli.test.ts index f3407c05a..7e60997ff 100644 --- a/packages/coding-agent/test/config-cli.test.ts +++ b/packages/coding-agent/test/config-cli.test.ts @@ -109,4 +109,36 @@ describe("config CLI schema coverage", () => { expect(parsed.type).toBe("array"); expect(parsed.value).toEqual(["claude-opus-4-6", "gpt-5.3-codex"]); }); + it("sets numeric idle compaction settings from CLI values", async () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + await runConfigCommand({ + action: "set", + key: "compaction.idleThresholdTokens", + value: "300000", + flags: { json: true }, + }); + await runConfigCommand({ + action: "set", + key: "compaction.idleTimeoutSeconds", + value: "600", + flags: { json: true }, + }); + await runConfigCommand({ action: "get", key: "compaction.idleThresholdTokens", flags: { json: true } }); + await runConfigCommand({ action: "get", key: "compaction.idleTimeoutSeconds", flags: { json: true } }); + + const thresholdPayload = logSpy.mock.calls.at(-2)?.[0]; + const timeoutPayload = logSpy.mock.calls.at(-1)?.[0]; + expect(typeof thresholdPayload).toBe("string"); + expect(typeof timeoutPayload).toBe("string"); + expect(JSON.parse(String(thresholdPayload))).toMatchObject({ + key: "compaction.idleThresholdTokens", + type: "number", + value: 300000, + }); + expect(JSON.parse(String(timeoutPayload))).toMatchObject({ + key: "compaction.idleTimeoutSeconds", + type: "number", + value: 600, + }); + }); }); diff --git a/packages/coding-agent/test/modes/controllers/event-controller-idle-compaction.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-idle-compaction.test.ts new file mode 100644 index 000000000..47e189ccf --- /dev/null +++ b/packages/coding-agent/test/modes/controllers/event-controller-idle-compaction.test.ts @@ -0,0 +1,79 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import type { AssistantMessage } from "@oh-my-pi/pi-ai"; +import { _resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; + +function createAssistantMessage(): AssistantMessage { + return { + role: "assistant", + content: [{ type: "text", text: "done" }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-sonnet-4-5", + usage: { + input: 200, + output: 10, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 210, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }; +} + +describe("EventController idle compaction teardown", () => { + beforeEach(async () => { + _resetSettingsForTest(); + await Settings.init({ + inMemory: true, + overrides: { + "compaction.idleEnabled": true, + "compaction.idleThresholdTokens": 100, + "compaction.idleTimeoutSeconds": 60, + }, + }); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + _resetSettingsForTest(); + }); + + it("cancels scheduled idle compaction when disposed", async () => { + const runIdleCompaction = vi.fn(); + const context = { + isInitialized: true, + isBackgrounded: false, + loadingAnimation: undefined, + streamingComponent: undefined, + streamingMessage: undefined, + pendingTools: new Map(), + flushPendingModelSwitch: async () => {}, + ui: { requestRender: vi.fn() }, + chatContainer: { removeChild: vi.fn() }, + statusContainer: { clear: vi.fn() }, + statusLine: { invalidate: vi.fn() }, + updateEditorTopBorder: vi.fn(), + editor: { getText: () => "" }, + sessionManager: { getSessionName: () => undefined }, + session: { + isCompacting: false, + isStreaming: false, + runIdleCompaction, + agent: { state: { messages: [createAssistantMessage()] } }, + }, + } as unknown as InteractiveModeContext; + + const controller = new EventController(context); + await controller.handleEvent({ type: "agent_end", messages: [createAssistantMessage()] }); + controller.dispose(); + vi.advanceTimersByTime(60_000); + + expect(runIdleCompaction).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/coding-agent/test/sdk-session-isolation.test.ts b/packages/coding-agent/test/sdk-session-isolation.test.ts index 20f87d37b..f7fbeb8be 100644 --- a/packages/coding-agent/test/sdk-session-isolation.test.ts +++ b/packages/coding-agent/test/sdk-session-isolation.test.ts @@ -2,9 +2,12 @@ import { afterEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { type AssistantMessage, getBundledModel } from "@oh-my-pi/pi-ai"; import type { Rule } from "@oh-my-pi/pi-coding-agent/capability/rule"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { SecretObfuscator } from "@oh-my-pi/pi-coding-agent/secrets"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { getSessionsDir, Snowflake } from "@oh-my-pi/pi-utils"; function createTtsrRule(name: string): Rule { @@ -23,6 +26,33 @@ function createTtsrRule(name: string): Rule { }; } +const SECRET_ENV_PATTERNS = /(?:KEY|SECRET|TOKEN|PASSWORD|PASS|AUTH|CREDENTIAL|PRIVATE|OAUTH)(?:_|$)/i; + +async function withClearedSecretEnv(run: () => Promise): Promise { + const removed: Array<[string, string]> = []; + for (const [name, value] of Object.entries(process.env)) { + if (!value || value.length < 8) continue; + if (!SECRET_ENV_PATTERNS.test(name)) continue; + removed.push([name, value]); + delete process.env[name]; + } + try { + return await run(); + } finally { + for (const [name, value] of removed) { + process.env[name] = value; + } + } +} + +function getAssistantText(message: AssistantMessage | undefined): string { + if (!message) throw new Error("Expected assistant message"); + return message.content + .filter((block): block is { type: "text"; text: string } => block.type === "text") + .map(block => block.text) + .join(" "); +} + describe("createAgentSession session storage isolation", () => { const tempDirs: string[] = []; @@ -95,4 +125,108 @@ describe("createAgentSession session storage isolation", () => { await session.dispose(); } }); + it("shows redaction guidance only when secrets are actually loaded", async () => { + await withClearedSecretEnv(async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-sdk-secrets-${Snowflake.next()}-`)); + tempDirs.push(tempDir); + const cwd = path.join(tempDir, "project"); + const agentDir = path.join(tempDir, "agent"); + fs.mkdirSync(cwd, { recursive: true }); + + const commonOptions = { + cwd, + agentDir, + settings: Settings.isolated({ "secrets.enabled": true }), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }; + + const withoutSecrets = await createAgentSession(commonOptions); + try { + expect(withoutSecrets.session.systemPrompt).not.toContain("They appear as `#XXXX#` tokens"); + } finally { + await withoutSecrets.session.dispose(); + } + + fs.mkdirSync(path.join(cwd, ".omp"), { recursive: true }); + fs.writeFileSync(path.join(cwd, ".omp", "secrets.yml"), "- type: plain\n content: sdk-secret-token-123456\n"); + + const withSecrets = await createAgentSession(commonOptions); + try { + expect(withSecrets.session.systemPrompt).toContain("They appear as `#XXXX#` tokens"); + } finally { + await withSecrets.session.dispose(); + } + }); + }); + + it("keeps restored assistant messages deobfuscated across reloads", async () => { + await withClearedSecretEnv(async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-sdk-session-secrets-${Snowflake.next()}-`)); + tempDirs.push(tempDir); + const cwd = path.join(tempDir, "project"); + const agentDir = path.join(tempDir, "agent"); + fs.mkdirSync(path.join(cwd, ".omp"), { recursive: true }); + fs.writeFileSync(path.join(cwd, ".omp", "secrets.yml"), "- type: plain\n content: sdk-secret-token-123456\n"); + + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) throw new Error("Expected anthropic model"); + + const obfuscator = new SecretObfuscator([{ type: "plain", content: "sdk-secret-token-123456" }]); + const initialManager = SessionManager.create(cwd, path.join(agentDir, "sessions")); + initialManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: obfuscator.obfuscate("token sdk-secret-token-123456") }], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }); + await initialManager.flush(); + const sessionFile = initialManager.getSessionFile(); + if (!sessionFile) throw new Error("Expected persisted session file"); + await initialManager.close(); + + const resumedManager = await SessionManager.open(sessionFile, path.dirname(sessionFile)); + const { session } = await createAgentSession({ + cwd, + agentDir, + sessionManager: resumedManager, + model, + settings: Settings.isolated({ "secrets.enabled": true }), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + try { + expect(getAssistantText(session.messages.at(-1) as AssistantMessage | undefined)).toContain( + "sdk-secret-token-123456", + ); + await session.reload(); + expect(getAssistantText(session.messages.at(-1) as AssistantMessage | undefined)).toContain( + "sdk-secret-token-123456", + ); + } finally { + await session.dispose(); + } + }); + }); }); diff --git a/packages/coding-agent/test/session-manager/file-operations.test.ts b/packages/coding-agent/test/session-manager/file-operations.test.ts index 736ad72b7..f5e64fcbc 100644 --- a/packages/coding-agent/test/session-manager/file-operations.test.ts +++ b/packages/coding-agent/test/session-manager/file-operations.test.ts @@ -443,4 +443,25 @@ describe("SessionManager legacy session migration persistence", () => { expect(persistedEntries[1].id).toBeDefined(); expect(persistedEntries[1].parentId).toBeNull(); }); + it("keeps the last non-empty session resumable after starting a fresh session", async () => { + const session = SessionManager.create(tempDir, tempDir); + session.appendMessage({ role: "user", content: "hello", timestamp: Date.now() - 1 }); + session.appendMessage(makeAssistantMessage()); + await session.flush(); + + const previousSessionFile = session.getSessionFile(); + if (!previousSessionFile) throw new Error("Expected persisted session file"); + + const freshSessionFile = await session.newSession(); + expect(freshSessionFile).toBeDefined(); + expect(fs.existsSync(freshSessionFile!)).toBe(false); + + const resumed = await SessionManager.continueRecent(tempDir, tempDir); + try { + expect(resumed.getSessionFile()).toBe(previousSessionFile); + } finally { + await resumed.close(); + await session.close(); + } + }); }); diff --git a/packages/coding-agent/test/task/worktree.test.ts b/packages/coding-agent/test/task/worktree.test.ts index b419a0d4e..605ddff18 100644 --- a/packages/coding-agent/test/task/worktree.test.ts +++ b/packages/coding-agent/test/task/worktree.test.ts @@ -1,29 +1,100 @@ -import { describe, expect, it, vi } from "bun:test"; +import { afterEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { getGitNoIndexNullPath, isProjfsUnavailableError, mergeTaskBranches } from "../../src/task/worktree"; const projfsOverlayStartMock = vi.fn(); const projfsOverlayStopMock = vi.fn(); +const tempDirs: string[] = []; vi.mock("@oh-my-pi/pi-natives", () => ({ projfsOverlayStart: projfsOverlayStartMock, projfsOverlayStop: projfsOverlayStopMock, })); -async function loadWorktreeHelpers() { - const { getGitNoIndexNullPath, isProjfsUnavailableError } = await import("../../src/task/worktree"); - return { getGitNoIndexNullPath, isProjfsUnavailableError }; +async function runGit(repo: string, args: string[]): Promise { + const proc = Bun.spawn(["git", ...args], { + cwd: repo, + stderr: "pipe", + stdout: "pipe", + windowsHide: true, + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + if ((exitCode ?? 0) !== 0) { + throw new Error(stderr.trim() || stdout.trim() || `git ${args.join(" ")} failed with exit code ${exitCode ?? 0}`); + } + return stdout.trim(); } +async function createGitRepo(): Promise<{ baseBranch: string; repo: string }> { + const repo = await fs.mkdtemp(path.join(os.tmpdir(), "omp-worktree-")); + tempDirs.push(repo); + await runGit(repo, ["init"]); + await runGit(repo, ["config", "user.email", "test@example.com"]); + await runGit(repo, ["config", "user.name", "Test User"]); + await fs.writeFile(path.join(repo, "merged.txt"), "base version\n"); + await fs.writeFile(path.join(repo, "staged.txt"), "base staged\n"); + await runGit(repo, ["add", "."]); + await runGit(repo, ["commit", "-m", "initial"]); + return { + baseBranch: await runGit(repo, ["branch", "--show-current"]), + repo, + }; +} + +afterEach(async () => { + vi.restoreAllMocks(); + await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); +}); + describe("worktree isolation helpers", () => { - it("returns platform-specific null path for git --no-index diffs", async () => { - const { getGitNoIndexNullPath } = await loadWorktreeHelpers(); + it("returns platform-specific null path for git --no-index diffs", () => { const expected = process.platform === "win32" ? "NUL" : "/dev/null"; expect(getGitNoIndexNullPath()).toBe(expected); }); - it("detects ProjFS prerequisite errors by prefix", async () => { - const { isProjfsUnavailableError } = await loadWorktreeHelpers(); + it("detects ProjFS prerequisite errors by prefix", () => { expect(isProjfsUnavailableError(new Error("PROJFS_UNAVAILABLE: missing feature"))).toBe(true); expect(isProjfsUnavailableError(new Error("fuse-overlay mount failed"))).toBe(false); expect(isProjfsUnavailableError("PROJFS_UNAVAILABLE: not-an-error-instance")).toBe(false); }); + + it("does not pop an unrelated pre-existing stash when the working tree is clean", async () => { + const { repo } = await createGitRepo(); + await fs.writeFile(path.join(repo, "preexisting.txt"), "user stash\n"); + await runGit(repo, ["stash", "push", "--include-untracked", "-m", "preexisting-user-stash"]); + const before = await runGit(repo, ["stash", "list"]); + + const result = await mergeTaskBranches(repo, []); + + expect(result).toEqual({ failed: [], merged: [] }); + expect(await runGit(repo, ["stash", "list"])).toBe(before); + expect(await runGit(repo, ["status", "--porcelain=v1"])).toBe(""); + }); + + it("restores staged changes with index preservation after merging task branches", async () => { + const { baseBranch, repo } = await createGitRepo(); + const taskBranch = "task/merge-staged"; + await runGit(repo, ["checkout", "-b", taskBranch]); + await fs.writeFile(path.join(repo, "merged.txt"), "task branch change\n"); + await runGit(repo, ["add", "merged.txt"]); + await runGit(repo, ["commit", "-m", "task-change"]); + await runGit(repo, ["checkout", baseBranch]); + await fs.writeFile(path.join(repo, "staged.txt"), "local staged change\n"); + await runGit(repo, ["add", "staged.txt"]); + expect(await runGit(repo, ["status", "--porcelain=v1"])).toBe("M staged.txt"); + + const result = await mergeTaskBranches(repo, [{ branchName: taskBranch, taskId: "task-1" }]); + + expect(result).toEqual({ failed: [], merged: [taskBranch] }); + expect(await fs.readFile(path.join(repo, "merged.txt"), "utf8")).toBe("task branch change\n"); + expect(await runGit(repo, ["status", "--porcelain=v1"])).toBe("M staged.txt"); + expect(await runGit(repo, ["diff", "--cached", "--", "staged.txt"])).toContain("+local staged change"); + expect(await runGit(repo, ["stash", "list"])).toBe(""); + }); });