diff --git a/.changeset/session-config-cleanup.md b/.changeset/session-config-cleanup.md new file mode 100644 index 000000000..b1404682d --- /dev/null +++ b/.changeset/session-config-cleanup.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Prevent repeated exit-status broadcasts from deleting configuration regenerated during session resume, including sessions restored after restart or imported from history. Failed resume preparation also cleans up regenerated configuration. diff --git a/packages/harness/src/core/collector/codex-tailer.test.ts b/packages/harness/src/core/collector/codex-tailer.test.ts index 0ce90dd61..38d0531dc 100644 --- a/packages/harness/src/core/collector/codex-tailer.test.ts +++ b/packages/harness/src/core/collector/codex-tailer.test.ts @@ -83,13 +83,15 @@ describe("tailCodexRollout", () => { it("translates a user_message into UserPromptSubmit", async () => { await writeFile(rolloutPath, sessionMetaLine("agent-1", "/tmp/proj", "2026-01-01T00:00:00.000Z")); - start(); - await sleep(POLL_MS * 4); + start({ startFromBeginning: true }); + await vi.waitFor(() => { + expect(onEvent).toHaveBeenCalledWith("SessionStart", expect.anything()); + }); await appendFile(rolloutPath, userMessageLine("build me a leasing workflow")); - await sleep(POLL_MS * 4); - - expect(onEvent).toHaveBeenCalledWith("UserPromptSubmit", { prompt: "build me a leasing workflow" }); + await vi.waitFor(() => { + expect(onEvent).toHaveBeenCalledWith("UserPromptSubmit", { prompt: "build me a leasing workflow" }); + }); }); it("pairs function_call + function_call_output into a single PostToolUse", async () => { diff --git a/packages/harness/src/core/session-manager.test.ts b/packages/harness/src/core/session-manager.test.ts index 34022a58b..cba9cf517 100644 --- a/packages/harness/src/core/session-manager.test.ts +++ b/packages/harness/src/core/session-manager.test.ts @@ -5409,7 +5409,13 @@ describe("SessionManager", () => { expect(onAgentMapSessionExit).toHaveBeenCalledOnce(); expect(onAgentMapSessionExit).toHaveBeenCalledWith(session.id); - expect(manager.get(session.id)).toEqual(beforeResume); + // Preparation now enters the starting lifetime, so its failure clears + // the old exit result while retaining identity and last real activity. + expect(manager.get(session.id)).toEqual({ + ...beforeResume, + exitCode: null, + exitTail: null, + }); expect(spawns).toHaveLength(1); }, ); diff --git a/packages/harness/src/core/session-manager.ts b/packages/harness/src/core/session-manager.ts index b71bf3f7b..23f92fa59 100644 --- a/packages/harness/src/core/session-manager.ts +++ b/packages/harness/src/core/session-manager.ts @@ -1427,6 +1427,11 @@ export class SessionManager { }); } + /** + * Resumes a stored conversation after the adapter confirms it can reopen it. + * Claims the starting state before preparation. Setup failures run normal + * exit cleanup and preserve the previous last-activity timestamp. + */ async resume( id: string, trusted: TrustedSessionResumeOptions = {}, @@ -1517,15 +1522,15 @@ export class SessionManager { // they must see this lifecycle as starting, not schedule cleanup against // files that the resumed process is currently regenerating. const lastActiveBeforeResume = session.lastActiveAt; - const statusBeforeResume = session.status; - const exitCodeBeforeResume = session.exitCode; session.status = "starting"; session.exitCode = null; session.lastActiveAt = this.now(); - let opts: LaunchOpts; - let spec: SpawnSpec; - let mcpCredentialLaunch: McpCredentialLaunch | undefined; + // A failed pre-PTY attempt is not activity; the failure path restores this + // timestamp so the dead pane's elapsed time still reflects real work. try { + await this.persist(); + this.emitStatus(session); + // Preparation belongs to this lifetime so failure runs normal exit cleanup. const launchContext = trusted.promptAppendix || trusted.focusedContext || agentMapIdentity ? { @@ -1542,34 +1547,12 @@ export class SessionManager { const built = await (launchContext ? this.buildLaunchOpts(id, session, launchContext) : this.buildLaunchOpts(id, session)); - ({ mcpCredentialLaunch, ...opts } = { + const { mcpCredentialLaunch, ...opts } = { harnessSessionId: id, cwd: session.cwd, ...built, - }); - spec = adapter.resume(session.agentSessionId, opts); - } catch (error) { - // Resume preparation may rotate project capabilities or write generated - // launch state before the process exists. No starting state was exposed - // or persisted yet, so restore the exact prior record while releasing - // any prepared authority. - session.status = statusBeforeResume; - session.exitCode = exitCodeBeforeResume; - session.lastActiveAt = lastActiveBeforeResume; - await Promise.resolve(this.onAgentMapSessionExit?.(id)).catch(() => {}); - throw error; - } - // The prior value is kept so the failure path below can put it back: - // `lastActiveAt` is stamped only to keep sweepDeadSessions() from reaping - // this record - // during the pre-pty window (it reaps non-exited records with no pty once - // they're older than the grace period). If the resume never produces a - // pty, that stamp is not activity and must not survive — otherwise a - // session idle since last night reports "Ran for 6h 25m" purely because - // someone clicked Resume. - try { - await this.persist(); - this.emitStatus(session); + }; + const spec = adapter.resume(session.agentSessionId, opts); // Schema-aware and strict: the caller leaves a valid current file // untouched, translates a valid legacy file, and reconstructs anything // missing/invalid from this session plus the live registry. Await it in diff --git a/packages/harness/src/core/subsession-coordinator.test.ts b/packages/harness/src/core/subsession-coordinator.test.ts index 739f72049..ce5b2432f 100644 --- a/packages/harness/src/core/subsession-coordinator.test.ts +++ b/packages/harness/src/core/subsession-coordinator.test.ts @@ -496,7 +496,14 @@ describe("SubsessionCoordinator", () => { delegationKey, outcome: `Implement ${delegationKey}`, })) } }; const limited = newCoordinator("limited-wait", { readinessTimeoutMs: 250, batchWaitTimeoutMs: 100 }); - const first = await limited.execute(caller, batch); + // Expire the batch after the first child starts, independent of disk speed. + vi.useFakeTimers({ toFake: ["Date"] }); + const deadline = Date.now() + 100; + const stopClock = manager.onStatusChange((session) => { + if (session.id !== caller.sessionId && session.status === "running") + vi.setSystemTime(deadline); + }); + const first = await limited.execute(caller, batch).finally(stopClock); expect(first.results).toHaveLength(3); expect(first.results.map(({ error }) => error?.code)).toEqual(["readiness_timeout", "readiness_timeout", "readiness_timeout"]); expect(first.results.map(({ sessionState }) => sessionState)).toEqual(["awaiting-ready", "reserved", "reserved"]); diff --git a/packages/harness/src/server/definition-list-enrichment.test.ts b/packages/harness/src/server/definition-list-enrichment.test.ts index ad05e99c0..fd6bf0a98 100644 --- a/packages/harness/src/server/definition-list-enrichment.test.ts +++ b/packages/harness/src/server/definition-list-enrichment.test.ts @@ -29,8 +29,10 @@ vi.mock("@sapiom/mcp/auth", () => ({ }), writeCredentials: vi.fn(async () => {}), clearCredentials: vi.fn(async () => {}), + credentialsFilePath: vi.fn(), })); +import { credentialsFilePath } from "@sapiom/mcp/auth"; import type { HarnessAdapter, LaunchOpts, @@ -90,6 +92,9 @@ describe("definition list enrichment wiring (SAP-3214)", () => { tempDir = await fs.mkdtemp( path.join(os.tmpdir(), "harness-definition-list-enrichment-"), ); + vi.mocked(credentialsFilePath).mockReturnValue( + path.join(tempDir, "credentials.json"), + ); previousAgentsUrl = process.env.SAPIOM_AGENTS_URL; api = { listStatus: 200, diff --git a/packages/harness/src/server/generated-retention.test.ts b/packages/harness/src/server/generated-retention.test.ts index 3e8763a42..1cc65651a 100644 --- a/packages/harness/src/server/generated-retention.test.ts +++ b/packages/harness/src/server/generated-retention.test.ts @@ -10,8 +10,11 @@ import { access, mkdir, mkdtemp, rm, utimes, writeFile } from "node:fs/promises" import { tmpdir } from "node:os"; import { join } from "node:path"; +vi.mock("../core/inject/retention.js", { spy: true }); + import { startServer, type HarnessServer } from "./index.js"; import type { HarnessAdapter, LaunchOpts, SpawnSpec } from "../shared/types.js"; +import { removeGeneratedSessionDir } from "../core/inject/retention.js"; const DAY_MS = 24 * 60 * 60 * 1000; @@ -49,6 +52,7 @@ describe("generated-dir retention wiring", () => { let server: HarnessServer | undefined; beforeEach(async () => { + vi.mocked(removeGeneratedSessionDir).mockClear(); dir = await mkdtemp(join(tmpdir(), "harness-retention-wiring-")); generatedRoot = join(dir, "generated"); cwd = join(dir, "project"); @@ -63,7 +67,8 @@ describe("generated-dir retention wiring", () => { await rm(dir, { recursive: true, force: true }); }); - async function boot(): Promise { + /** Starts an isolated server with optional launch and adapter overrides. */ + async function boot(options: Pick[0], "buildLaunchOpts" | "adapters"> = {}): Promise { return startServer({ port: 0, bootToken: "test-token", @@ -71,9 +76,19 @@ describe("generated-dir retention wiring", () => { autoCreateSession: false, adapters: { "claude-code": fakeClaudeAdapter() }, stateRoot: dir, + ...options, }); } + /** Writes a real MCP configuration file so tests can verify its deletion. */ + async function writeMcpConfig(id: string): Promise { + const sessionDir = join(generatedRoot, id); + await mkdir(sessionDir, { recursive: true }); + const mcpConfigFile = join(sessionDir, "mcp-config.json"); + await writeFile(mcpConfigFile, '{"mcpServers":{}}'); + return mcpConfigFile; + } + it("sweeps stale orphaned dirs at boot and keeps fresh ones", async () => { const staleDir = join(generatedRoot, "orphan-from-a-crash"); const freshDir = join(generatedRoot, "recent-orphan"); @@ -113,4 +128,126 @@ describe("generated-dir retention wiring", () => { { timeout: 10_000, interval: 100 }, ); }, 15_000); + + it("ignores repeated exited metadata broadcasts while resume regenerates configuration", async () => { + let builds = 0; + server = await boot({ + buildLaunchOpts: async (id) => { + const mcpConfigFile = await writeMcpConfig(id); + if (++builds === 2) { + // Resume has awaited the prior exit's cleanup. A workspace scan + // may publish metadata while the new configuration is being built. + expect(server!.sessionManager.get(id)?.status).toBe("starting"); + server!.sessionManager.setBoundWorkflowPath(id, cwd); + // No second removal may begin against the regenerated files. + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(1); + } + return { mcpConfigFile }; + }, + }); + const session = await server.sessionManager.create({ cwd, harness: "claude-code" }); + await server.sessionManager.setAgentSessionId(session.id, "retention-rollout"); + await server.sessionManager.kill(session.id); + await vi.waitFor(async () => { + expect(await exists(join(generatedRoot, session.id))).toBe(false); + }); + // A completed removal still guards later metadata from the same lifetime. + server.sessionManager.setBoundWorkflowPath(session.id, cwd); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(1); + await server.sessionManager.resume(session.id); + expect(await exists(join(generatedRoot, session.id, "mcp-config.json"))).toBe(true); + + // Regenerating configuration resets the guard, so its next exit still cleans + // up normally rather than retaining credentials indefinitely. + await server.sessionManager.kill(session.id); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(2); + await vi.waitFor(async () => { + expect(await exists(join(generatedRoot, session.id))).toBe(false); + }); + }); + + it.each(["restart", "adopted history"])("protects regenerated configuration when resuming after %s", async (source) => { + const buildLaunchOpts = async (id: string) => { + const mcpConfigFile = await writeMcpConfig(id); + if (server!.sessionManager.get(id)) { + // Restored/imported history also enters its new lifetime before + // generation, so metadata broadcasts cannot remove these files. + expect(server!.sessionManager.get(id)?.status).toBe("starting"); + const removalsBeforeMetadata = vi.mocked(removeGeneratedSessionDir).mock.calls.length; + server!.sessionManager.setBoundWorkflowPath(id, cwd); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(removalsBeforeMetadata); + } + return { mcpConfigFile }; + }; + server = await boot({ buildLaunchOpts }); + let id: string; + if (source === "restart") { + const session = await server.sessionManager.create({ cwd, harness: "claude-code" }); + id = session.id; + await server.sessionManager.setAgentSessionId(id, "restored-rollout"); + await server.sessionManager.kill(id); + await vi.waitFor(async () => expect(await exists(join(generatedRoot, id))).toBe(false)); + await server.sessionManager.flush(); + await server.close(); + await server.sessionManager.flush(); + vi.mocked(removeGeneratedSessionDir).mockClear(); + server = await boot({ buildLaunchOpts }); + } else { + const session = await server.sessionManager.registerHistorical({ + harness: "claude-code", + cwd, + agentSessionId: "imported-rollout", + title: "Imported session", + lastActiveAt: "2026-01-01T00:00:00.000Z", + }); + id = session.id; + } + expect(server.sessionManager.get(id)?.status).toBe("exited"); + await server.sessionManager.resume(id); + expect(await exists(join(generatedRoot, id, "mcp-config.json"))).toBe(true); + const removalsBeforeExit = vi.mocked(removeGeneratedSessionDir).mock.calls.length; + await server.sessionManager.kill(id); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(removalsBeforeExit + 1); + await vi.waitFor(async () => expect(await exists(join(generatedRoot, id))).toBe(false)); + }); + + it.each(["configuration", "adapter"])("cleans regenerated files when resume fails during %s", async (failure) => { + let fail = true; + const adapter = fakeClaudeAdapter(); + const resume = adapter.resume; + adapter.resume = (agentSessionId, opts) => { + if (fail && failure === "adapter") throw new Error("resume preparation failed"); + return resume(agentSessionId, opts); + }; + server = await boot({ + adapters: { "claude-code": adapter }, + buildLaunchOpts: async (id) => { + const mcpConfigFile = await writeMcpConfig(id); + if (fail && failure === "configuration") throw new Error("resume preparation failed"); + return { mcpConfigFile }; + }, + }); + const session = await server.sessionManager.registerHistorical({ + harness: "claude-code", + cwd, + agentSessionId: "failed-resume-rollout", + title: "Imported session", + lastActiveAt: "2026-01-01T00:00:00.000Z", + }); + await expect(server.sessionManager.resume(session.id)).rejects.toThrow("resume preparation failed"); + expect(server.sessionManager.get(session.id)).toMatchObject({ + status: "exited", + lastActiveAt: "2026-01-01T00:00:00.000Z", + }); + await vi.waitFor(async () => expect(await exists(join(generatedRoot, session.id))).toBe(false)); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(1); + + // The failed attempt must not prevent a retry or that lifetime's cleanup. + fail = false; + await server.sessionManager.resume(session.id); + expect(await exists(join(generatedRoot, session.id, "mcp-config.json"))).toBe(true); + await server.sessionManager.kill(session.id); + await vi.waitFor(async () => expect(await exists(join(generatedRoot, session.id))).toBe(false)); + expect(removeGeneratedSessionDir).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/harness/src/server/index.ts b/packages/harness/src/server/index.ts index 1bbea2d5b..b00f2d428 100644 --- a/packages/harness/src/server/index.ts +++ b/packages/harness/src/server/index.ts @@ -680,6 +680,10 @@ function createDefaultBuildLaunchOpts( }; } +/** + * Starts Studio's HTTP server and session lifecycle services. + * Call the returned close() method to stop listeners and release resources. + */ export const startServer = async ( options: HarnessServerOptions, ): Promise => { @@ -1349,10 +1353,10 @@ export const startServer = async ( // Exit-time deletion of generated/ (see the onStatusChange handler // below) can race a fast resume(): resume regenerates the dir via // buildLaunchOpts, and the rm scheduled at the previous exit could still - // be in flight. Serialize by awaiting any pending removal for this id - // before (re)generating its files. + // be in flight. Keep each removal until the next config build: its presence + // guards repeated exited broadcasts, and its promise serializes regeneration. const generatedRoot = options.generatedRoot ?? statePaths.generated; - const pendingGeneratedRemovals = new Map>(); + const generatedRemovals = new Map>(); /** * The git branch the PRIOR session was last on, from whichever adapter @@ -1482,12 +1486,14 @@ export const startServer = async ( options.sapiomDevMcp, options.loadSystemPrompt ?? fetchSystemPromptForActiveEnvironment, ); + /** Waits for prior cleanup before preparing this run's files and capabilities. */ const buildLaunchOpts: LaunchOptsBuilder = async ( harnessSessionId, req, context, ) => { - await pendingGeneratedRemovals.get(harnessSessionId); + await generatedRemovals.get(harnessSessionId); + generatedRemovals.delete(harnessSessionId); // Scope/bootstrap ownership is already resolved; prepare the user's new // project before config generation and PTY spawn, never during resume. const initialPrompt = context?.resume @@ -2172,17 +2178,16 @@ export const startServer = async ( // The generated config dir is dead once the pty is: every file in it // is regenerated by buildLaunchOpts on resume, and the agent's last // emit.cjs execution (SessionEnd) happens before its process exits. + // Metadata broadcasts can repeat status=exited. Schedule removal once + // per lifetime; resume starts its next lifetime before regenerating + // configuration and awaits this removal before writing new files. + if (generatedRemovals.has(session.id)) return; const removal = removeGeneratedSessionDir(session.id, { generatedRoot }) .then(() => undefined) .catch((err: unknown) => { console.error("[harness] generated-dir cleanup failed:", err); - }) - .finally(() => { - if (pendingGeneratedRemovals.get(session.id) === removal) { - pendingGeneratedRemovals.delete(session.id); - } }); - pendingGeneratedRemovals.set(session.id, removal); + generatedRemovals.set(session.id, removal); } });