Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/session-config-cleanup.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 7 additions & 5 deletions packages/harness/src/core/collector/codex-tailer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/harness/src/core/session-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
);
Expand Down
43 changes: 13 additions & 30 deletions packages/harness/src/core/session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {},
Expand Down Expand Up @@ -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
? {
Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion packages/harness/src/core/subsession-coordinator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
139 changes: 138 additions & 1 deletion packages/harness/src/server/generated-retention.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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");
Expand All @@ -63,17 +67,28 @@ describe("generated-dir retention wiring", () => {
await rm(dir, { recursive: true, force: true });
});

async function boot(): Promise<HarnessServer> {
/** Starts an isolated server with optional launch and adapter overrides. */
async function boot(options: Pick<Parameters<typeof startServer>[0], "buildLaunchOpts" | "adapters"> = {}): Promise<HarnessServer> {
return startServer({
port: 0,
bootToken: "test-token",
telemetryOptIn: false,
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<string> {
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");
Expand Down Expand Up @@ -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);
});
});
25 changes: 15 additions & 10 deletions packages/harness/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HarnessServer> => {
Expand Down Expand Up @@ -1349,10 +1353,10 @@ export const startServer = async (
// Exit-time deletion of generated/<id> (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<string, Promise<void>>();
const generatedRemovals = new Map<string, Promise<void>>();

/**
* The git branch the PRIOR session was last on, from whichever adapter
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
});

Expand Down
Loading