refactor(core): replace spawn rollback ladder with CleanupStack (#1616)
* refactor(core): replace spawn rollback ladder with CleanupStack Replace the four nested try/catch + cleanupSpawnWorkspaceAndMetadata helper in _spawnInner with a single LIFO CleanupStack. Each side effect (reserved metadata, workspace, prompt files, runtime handle) pushes its undo as soon as the resource exists; on success we dismiss(), on failure we runAll(). Why: adding a new spawn step previously required extending every prior cleanup block. Easy to forget; no compiler check. The stack makes rollback structural — a new step pushes one cleanup, no risk of leaving prior resources behind. runAll() is fault-tolerant by design: a throwing cleanup never short-circuits the rest. Behavior is preserved. Adds characterization tests for the worker spawn rollback paths (none existed before — only spawnOrchestrator was covered): - workspace.create failure cleans reserved metadata - runtime.create failure destroys worktree + cleans metadata - postLaunchSetup failure destroys runtime + worktree + metadata - one cleanup throwing does not skip subsequent cleanups Refs #1603 (PR 1 of the ao spawn refactor plan). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(core): make CleanupStack runAll() terminal, symmetric with dismiss() Address review feedback on PR #1616: previously `dismiss()` → `push()` was a documented no-op but `runAll()` → `push()` would silently queue cleanups that fired on a subsequent `runAll()`. Asymmetric and surprising. Set `this.dismissed = true` at the top of `runAll()` so both terminal states (success via dismiss, failure via runAll) reject further pushes identically. Add a regression test pinning the new symmetric behavior. The "idempotent runAll" test continues to pass (early-return path now fires via the dismissed flag instead of the empty-stack short-circuit). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(core): surface CleanupStack errors and cover postCreate rollback Address review feedback on PR #1616: - Pass an onError callback to cleanupStack.runAll() in _spawnInner that logs cleanup failures via console.error. The previous /* best effort */ pattern silently swallowed errors during rollback; now the same errors are surfaced for debugging without changing behavior (cleanup errors still don't propagate, subsequent cleanups still run). - Add a characterization test for the workspace.postCreate failure path. This was the only rollback path without a test — the stack handled it correctly already, but pinning it down prevents regression. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Harshit Singh Bhandari
GitHub
parent
e465a4702d
commit
71253105cd
|
|
@ -0,0 +1,124 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import { CleanupStack } from "../cleanup-stack.js";
|
||||
|
||||
describe("CleanupStack", () => {
|
||||
it("runs nothing on an empty stack", async () => {
|
||||
const stack = new CleanupStack();
|
||||
await expect(stack.runAll()).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("runs pushed cleanups in LIFO order", async () => {
|
||||
const calls: string[] = [];
|
||||
const stack = new CleanupStack();
|
||||
stack.push(() => {
|
||||
calls.push("a");
|
||||
});
|
||||
stack.push(() => {
|
||||
calls.push("b");
|
||||
});
|
||||
stack.push(() => {
|
||||
calls.push("c");
|
||||
});
|
||||
|
||||
await stack.runAll();
|
||||
|
||||
expect(calls).toEqual(["c", "b", "a"]);
|
||||
});
|
||||
|
||||
it("does not run any cleanups after dismiss()", async () => {
|
||||
const fn = vi.fn();
|
||||
const stack = new CleanupStack();
|
||||
stack.push(fn);
|
||||
|
||||
stack.dismiss();
|
||||
await stack.runAll();
|
||||
|
||||
expect(fn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("awaits async cleanups", async () => {
|
||||
const calls: string[] = [];
|
||||
const stack = new CleanupStack();
|
||||
stack.push(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
calls.push("first-pushed");
|
||||
});
|
||||
stack.push(() => {
|
||||
calls.push("second-pushed");
|
||||
});
|
||||
|
||||
await stack.runAll();
|
||||
|
||||
// LIFO: second-pushed runs first (sync), then async first-pushed completes.
|
||||
expect(calls).toEqual(["second-pushed", "first-pushed"]);
|
||||
});
|
||||
|
||||
it("continues running subsequent cleanups when one throws", async () => {
|
||||
const calls: string[] = [];
|
||||
const stack = new CleanupStack();
|
||||
stack.push(() => {
|
||||
calls.push("a");
|
||||
});
|
||||
stack.push(() => {
|
||||
throw new Error("boom");
|
||||
});
|
||||
stack.push(() => {
|
||||
calls.push("c");
|
||||
});
|
||||
|
||||
await stack.runAll();
|
||||
|
||||
// c runs first (LIFO), middle throws but is swallowed, a still runs.
|
||||
expect(calls).toEqual(["c", "a"]);
|
||||
});
|
||||
|
||||
it("forwards thrown errors to the onError callback when provided", async () => {
|
||||
const errors: unknown[] = [];
|
||||
const stack = new CleanupStack();
|
||||
stack.push(() => {
|
||||
throw new Error("first");
|
||||
});
|
||||
stack.push(async () => {
|
||||
throw new Error("second");
|
||||
});
|
||||
|
||||
await stack.runAll((err) => errors.push(err));
|
||||
|
||||
expect(errors).toHaveLength(2);
|
||||
expect((errors[0] as Error).message).toBe("second"); // LIFO
|
||||
expect((errors[1] as Error).message).toBe("first");
|
||||
});
|
||||
|
||||
it("is a no-op when runAll is called twice", async () => {
|
||||
const fn = vi.fn();
|
||||
const stack = new CleanupStack();
|
||||
stack.push(fn);
|
||||
|
||||
await stack.runAll();
|
||||
await stack.runAll();
|
||||
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("treats push after dismiss as a no-op (cleanup will not run)", async () => {
|
||||
const fn = vi.fn();
|
||||
const stack = new CleanupStack();
|
||||
stack.dismiss();
|
||||
stack.push(fn);
|
||||
|
||||
await stack.runAll();
|
||||
|
||||
expect(fn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats push after runAll as a no-op (symmetric with dismiss)", async () => {
|
||||
const fn = vi.fn();
|
||||
const stack = new CleanupStack();
|
||||
await stack.runAll();
|
||||
stack.push(fn);
|
||||
|
||||
await stack.runAll();
|
||||
|
||||
expect(fn).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -13,6 +13,7 @@ import {
|
|||
readMetadata,
|
||||
readMetadataRaw,
|
||||
} from "../../metadata.js";
|
||||
import { getProjectWorktreesDir } from "../../paths.js";
|
||||
import type {
|
||||
OrchestratorConfig,
|
||||
PluginRegistry,
|
||||
|
|
@ -1287,6 +1288,134 @@ describe("spawn", () => {
|
|||
vi.useRealTimers();
|
||||
}, 20_000);
|
||||
|
||||
describe("rollback on failure", () => {
|
||||
it("cleans up reserved metadata when workspace creation fails", async () => {
|
||||
(mockWorkspace.create as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("workspace creation failed"),
|
||||
);
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow(
|
||||
"workspace creation failed",
|
||||
);
|
||||
|
||||
expect(readMetadataRaw(sessionsDir, "app-1")).toBeNull();
|
||||
expect(mockRuntime.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("destroys the worktree and cleans metadata when runtime creation fails", async () => {
|
||||
// Workspace path must be inside the project's managed worktrees root so
|
||||
// shouldDestroyWorkspacePath() permits destroy. Mock paths under tmpDir
|
||||
// would be skipped as out-of-tree (correct, but not the path we want to
|
||||
// characterize here).
|
||||
const worktreePath = join(getProjectWorktreesDir("my-app"), "app-1");
|
||||
(mockWorkspace.create as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
path: worktreePath,
|
||||
branch: "session/app-1",
|
||||
sessionId: "app-1",
|
||||
projectId: "my-app",
|
||||
});
|
||||
(mockRuntime.create as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("runtime creation failed"),
|
||||
);
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow(
|
||||
"runtime creation failed",
|
||||
);
|
||||
|
||||
expect(mockWorkspace.destroy).toHaveBeenCalledWith(worktreePath);
|
||||
expect(readMetadataRaw(sessionsDir, "app-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("destroys runtime and worktree when post-launch setup fails", async () => {
|
||||
const worktreePath = join(getProjectWorktreesDir("my-app"), "app-1");
|
||||
(mockWorkspace.create as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
path: worktreePath,
|
||||
branch: "session/app-1",
|
||||
sessionId: "app-1",
|
||||
projectId: "my-app",
|
||||
});
|
||||
const postLaunchError = new Error("post-launch setup failed");
|
||||
const agentWithPostLaunch: typeof mockAgent = {
|
||||
...mockAgent,
|
||||
postLaunchSetup: vi.fn().mockRejectedValueOnce(postLaunchError),
|
||||
};
|
||||
const registryWithPostLaunch: PluginRegistry = {
|
||||
...mockRegistry,
|
||||
get: vi.fn().mockImplementation((slot: string) => {
|
||||
if (slot === "runtime") return mockRuntime;
|
||||
if (slot === "agent") return agentWithPostLaunch;
|
||||
if (slot === "workspace") return mockWorkspace;
|
||||
return null;
|
||||
}),
|
||||
};
|
||||
const sm = createSessionManager({ config, registry: registryWithPostLaunch });
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow("post-launch setup failed");
|
||||
|
||||
expect(mockRuntime.destroy).toHaveBeenCalled();
|
||||
expect(mockWorkspace.destroy).toHaveBeenCalledWith(worktreePath);
|
||||
expect(readMetadataRaw(sessionsDir, "app-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("destroys the worktree and cleans metadata when workspace.postCreate fails", async () => {
|
||||
const worktreePath = join(getProjectWorktreesDir("my-app"), "app-1");
|
||||
const workspaceWithPostCreate: typeof mockWorkspace = {
|
||||
...mockWorkspace,
|
||||
create: vi.fn().mockResolvedValueOnce({
|
||||
path: worktreePath,
|
||||
branch: "session/app-1",
|
||||
sessionId: "app-1",
|
||||
projectId: "my-app",
|
||||
}),
|
||||
postCreate: vi.fn().mockRejectedValueOnce(new Error("postCreate hook failed")),
|
||||
};
|
||||
const registryWithPostCreate: PluginRegistry = {
|
||||
...mockRegistry,
|
||||
get: vi.fn().mockImplementation((slot: string) => {
|
||||
if (slot === "runtime") return mockRuntime;
|
||||
if (slot === "agent") return mockAgent;
|
||||
if (slot === "workspace") return workspaceWithPostCreate;
|
||||
return null;
|
||||
}),
|
||||
};
|
||||
const sm = createSessionManager({ config, registry: registryWithPostCreate });
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow("postCreate hook failed");
|
||||
|
||||
expect(workspaceWithPostCreate.destroy).toHaveBeenCalledWith(worktreePath);
|
||||
expect(readMetadataRaw(sessionsDir, "app-1")).toBeNull();
|
||||
// Runtime should not have been created since postCreate failed before runtime.create.
|
||||
expect(mockRuntime.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still cleans subsequent resources when one cleanup step throws", async () => {
|
||||
const worktreePath = join(getProjectWorktreesDir("my-app"), "app-1");
|
||||
(mockWorkspace.create as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
path: worktreePath,
|
||||
branch: "session/app-1",
|
||||
sessionId: "app-1",
|
||||
projectId: "my-app",
|
||||
});
|
||||
// workspace.destroy throws during rollback — metadata cleanup must still run
|
||||
(mockWorkspace.destroy as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("destroy failed"),
|
||||
);
|
||||
(mockRuntime.create as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("runtime creation failed"),
|
||||
);
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow(
|
||||
"runtime creation failed",
|
||||
);
|
||||
|
||||
// Even though workspace.destroy threw, metadata must have been cleaned up.
|
||||
expect(readMetadataRaw(sessionsDir, "app-1")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("displayName derivation", () => {
|
||||
it("persists the issue title as displayName when tracker returns one", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,56 @@
|
|||
/**
|
||||
* LIFO stack of cleanup callbacks for unwinding partial side effects.
|
||||
*
|
||||
* Use inside long initialization sequences (spawn, setup) where each successful
|
||||
* step adds a side effect that must be undone if a *later* step fails. Push a
|
||||
* cleanup as soon as the resource exists; call `dismiss()` once the whole
|
||||
* sequence has succeeded; call `runAll()` from the catch block to unwind.
|
||||
*
|
||||
* `runAll()` is fault-tolerant by design: a cleanup throwing must not skip the
|
||||
* remaining cleanups, otherwise the abstraction is worse than the inline ladder
|
||||
* it replaces. Pass `onError` to observe errors; the default is to swallow them
|
||||
* (matching the existing best-effort pattern in session-manager).
|
||||
*/
|
||||
export type CleanupFn = () => void | Promise<void>;
|
||||
|
||||
export class CleanupStack {
|
||||
private fns: CleanupFn[] = [];
|
||||
private dismissed = false;
|
||||
|
||||
/**
|
||||
* Register a cleanup. Cleanups added after `dismiss()` or `runAll()` will
|
||||
* not run — both are terminal states for the stack.
|
||||
*/
|
||||
push(fn: CleanupFn): void {
|
||||
if (this.dismissed) return;
|
||||
this.fns.push(fn);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark the operation as successful. Subsequent `runAll()` calls do nothing
|
||||
* and subsequent `push()` calls are ignored.
|
||||
*/
|
||||
dismiss(): void {
|
||||
this.dismissed = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run all pushed cleanups in LIFO order. Each cleanup is awaited; throws are
|
||||
* forwarded to `onError` (default: swallowed) so one failing cleanup never
|
||||
* skips the remaining ones. After running, the stack is terminal: subsequent
|
||||
* `push()` calls are no-ops and subsequent `runAll()` calls do nothing —
|
||||
* symmetric with `dismiss()`.
|
||||
*/
|
||||
async runAll(onError?: (err: unknown) => void): Promise<void> {
|
||||
if (this.dismissed) return;
|
||||
this.dismissed = true;
|
||||
while (this.fns.length > 0) {
|
||||
const fn = this.fns.pop()!;
|
||||
try {
|
||||
await fn();
|
||||
} catch (err) {
|
||||
if (onError) onError(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -86,6 +86,7 @@ import {
|
|||
writeWorkspaceOpenCodeAgentsMd,
|
||||
} from "./opencode-agents-md.js";
|
||||
import { writeOpenCodeConfig } from "./opencode-config.js";
|
||||
import { CleanupStack } from "./cleanup-stack.js";
|
||||
import {
|
||||
getOrchestratorSessionId,
|
||||
normalizeOrchestratorSessionStrategy,
|
||||
|
|
@ -1160,40 +1161,47 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
// Get the sessions directory for this project
|
||||
const sessionsDir = getProjectSessionsDir(spawnConfig.projectId);
|
||||
|
||||
// Determine session ID — atomically reserve to prevent concurrent collisions
|
||||
const { sessionId, tmuxName } = await reserveNextSessionIdentity(project, sessionsDir);
|
||||
// CleanupStack: each side effect pushes its undo as soon as the resource
|
||||
// exists. On any failure below we runAll() in LIFO order; on success we
|
||||
// dismiss(). Replaces the previous nested rollback ladder — adding a new
|
||||
// step now requires pushing one cleanup, with no risk of forgetting prior
|
||||
// ones.
|
||||
const cleanupStack = new CleanupStack();
|
||||
try {
|
||||
// Determine session ID — atomically reserve to prevent concurrent collisions
|
||||
const { sessionId, tmuxName } = await reserveNextSessionIdentity(project, sessionsDir);
|
||||
cleanupStack.push(() => deleteMetadata(sessionsDir, sessionId));
|
||||
|
||||
// Determine branch name — explicit branch always takes priority
|
||||
let branch: string;
|
||||
if (spawnConfig.branch) {
|
||||
branch = spawnConfig.branch;
|
||||
} else if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
const fromIssue = resolvedIssue.branchName;
|
||||
branch =
|
||||
fromIssue && isGitBranchNameSafe(fromIssue)
|
||||
? fromIssue
|
||||
: plugins.tracker.branchName(spawnConfig.issueId, project);
|
||||
} else if (spawnConfig.issueId) {
|
||||
// If the issueId is already branch-safe (e.g. "INT-9999"), use as-is.
|
||||
// Otherwise sanitize free-text (e.g. "fix login bug") into a valid slug.
|
||||
const id = spawnConfig.issueId;
|
||||
const isBranchSafe = /^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(id) && !id.includes("..");
|
||||
const slug = isBranchSafe
|
||||
? id
|
||||
: id
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9]+/g, "-")
|
||||
.slice(0, 60)
|
||||
.replace(/^-+|-+$/g, "");
|
||||
branch = `feat/${slug || sessionId}`;
|
||||
} else {
|
||||
branch = `session/${sessionId}`;
|
||||
}
|
||||
// Determine branch name — explicit branch always takes priority
|
||||
let branch: string;
|
||||
if (spawnConfig.branch) {
|
||||
branch = spawnConfig.branch;
|
||||
} else if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
const fromIssue = resolvedIssue.branchName;
|
||||
branch =
|
||||
fromIssue && isGitBranchNameSafe(fromIssue)
|
||||
? fromIssue
|
||||
: plugins.tracker.branchName(spawnConfig.issueId, project);
|
||||
} else if (spawnConfig.issueId) {
|
||||
// If the issueId is already branch-safe (e.g. "INT-9999"), use as-is.
|
||||
// Otherwise sanitize free-text (e.g. "fix login bug") into a valid slug.
|
||||
const id = spawnConfig.issueId;
|
||||
const isBranchSafe = /^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(id) && !id.includes("..");
|
||||
const slug = isBranchSafe
|
||||
? id
|
||||
: id
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9]+/g, "-")
|
||||
.slice(0, 60)
|
||||
.replace(/^-+|-+$/g, "");
|
||||
branch = `feat/${slug || sessionId}`;
|
||||
} else {
|
||||
branch = `session/${sessionId}`;
|
||||
}
|
||||
|
||||
// Create workspace (if workspace plugin is available)
|
||||
let workspacePath = project.path;
|
||||
if (plugins.workspace) {
|
||||
try {
|
||||
// Create workspace (if workspace plugin is available)
|
||||
let workspacePath = project.path;
|
||||
if (plugins.workspace) {
|
||||
const wsInfo = await plugins.workspace.create({
|
||||
projectId: spawnConfig.projectId,
|
||||
project,
|
||||
|
|
@ -1202,110 +1210,54 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
worktreeDir: getProjectWorktreesDir(spawnConfig.projectId),
|
||||
});
|
||||
workspacePath = wsInfo.path;
|
||||
|
||||
// Run post-create hooks — clean up workspace on failure
|
||||
// Only register destroy when the path is inside a managed root —
|
||||
// matches the prior shouldDestroyWorkspacePath gate so we never
|
||||
// destroy a user-owned project directory.
|
||||
if (shouldDestroyWorkspacePath(project, spawnConfig.projectId, workspacePath)) {
|
||||
const ws = plugins.workspace;
|
||||
cleanupStack.push(() => ws.destroy(workspacePath));
|
||||
}
|
||||
if (plugins.workspace.postCreate) {
|
||||
try {
|
||||
await plugins.workspace.postCreate(wsInfo, project);
|
||||
} catch (err) {
|
||||
if (shouldDestroyWorkspacePath(project, spawnConfig.projectId, workspacePath)) {
|
||||
try {
|
||||
await plugins.workspace.destroy(workspacePath);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
await plugins.workspace.postCreate(wsInfo, project);
|
||||
}
|
||||
} catch (err) {
|
||||
// Clean up reserved session ID on workspace failure
|
||||
}
|
||||
|
||||
// Generate prompt with validated issue
|
||||
let issueContext: string | undefined;
|
||||
if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
try {
|
||||
deleteMetadata(sessionsDir, sessionId);
|
||||
issueContext = await plugins.tracker.generatePrompt(spawnConfig.issueId, project);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
// Generate prompt with validated issue
|
||||
let issueContext: string | undefined;
|
||||
if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
try {
|
||||
issueContext = await plugins.tracker.generatePrompt(spawnConfig.issueId, project);
|
||||
} catch {
|
||||
// Non-fatal: continue without detailed issue context
|
||||
// Silently ignore errors - caller can check if issueContext is undefined
|
||||
}
|
||||
}
|
||||
|
||||
const cleanupSpawnWorkspaceAndMetadata = async (
|
||||
promptFile?: string,
|
||||
opencodeConfigFile?: string,
|
||||
): Promise<void> => {
|
||||
if (
|
||||
plugins.workspace &&
|
||||
shouldDestroyWorkspacePath(project, spawnConfig.projectId, workspacePath)
|
||||
) {
|
||||
try {
|
||||
await plugins.workspace.destroy(workspacePath);
|
||||
} catch {
|
||||
/* best effort */
|
||||
// Non-fatal: continue without detailed issue context
|
||||
// Silently ignore errors - caller can check if issueContext is undefined
|
||||
}
|
||||
}
|
||||
try {
|
||||
deleteMetadata(sessionsDir, sessionId);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
if (promptFile) {
|
||||
try {
|
||||
unlinkSync(promptFile);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
if (opencodeConfigFile) {
|
||||
try {
|
||||
unlinkSync(opencodeConfigFile);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const { systemPrompt, taskPrompt } = buildPrompt({
|
||||
project,
|
||||
projectId: spawnConfig.projectId,
|
||||
issueId: spawnConfig.issueId,
|
||||
issueContext,
|
||||
userPrompt: spawnConfig.prompt,
|
||||
});
|
||||
const { systemPrompt, taskPrompt } = buildPrompt({
|
||||
project,
|
||||
projectId: spawnConfig.projectId,
|
||||
issueId: spawnConfig.issueId,
|
||||
issueContext,
|
||||
userPrompt: spawnConfig.prompt,
|
||||
});
|
||||
|
||||
let systemPromptFile: string | undefined;
|
||||
|
||||
// need a seperate config file to pass instructions for opencode session
|
||||
let opencodeConfigFile: string | undefined;
|
||||
|
||||
try {
|
||||
const baseDir = getProjectDir(spawnConfig.projectId);
|
||||
mkdirSync(baseDir, { recursive: true });
|
||||
systemPromptFile = join(baseDir, `worker-prompt-${sessionId}.md`);
|
||||
const systemPromptFile = join(baseDir, `worker-prompt-${sessionId}.md`);
|
||||
writeFileSync(systemPromptFile, systemPrompt, "utf-8");
|
||||
cleanupStack.push(() => unlinkSync(systemPromptFile));
|
||||
|
||||
// need a seperate config file to pass instructions for opencode session
|
||||
let opencodeConfigFile: string | undefined;
|
||||
if (plugins.agent.name === "opencode") {
|
||||
opencodeConfigFile = writeOpenCodeConfig(baseDir, sessionId, [systemPromptFile]);
|
||||
const cfg = opencodeConfigFile;
|
||||
cleanupStack.push(() => unlinkSync(cfg));
|
||||
}
|
||||
} catch (err) {
|
||||
await cleanupSpawnWorkspaceAndMetadata(systemPromptFile, opencodeConfigFile);
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Get agent launch config and create runtime — clean up workspace on failure
|
||||
const opencodeIssueSessionStrategy = project.opencodeIssueSessionStrategy ?? "reuse";
|
||||
let reusedOpenCodeSessionId: string | undefined;
|
||||
try {
|
||||
reusedOpenCodeSessionId =
|
||||
// Get agent launch config and create runtime
|
||||
const opencodeIssueSessionStrategy = project.opencodeIssueSessionStrategy ?? "reuse";
|
||||
const reusedOpenCodeSessionId =
|
||||
plugins.agent.name === "opencode" && spawnConfig.issueId
|
||||
? await resolveOpenCodeSessionReuse({
|
||||
sessionsDir,
|
||||
|
|
@ -1313,30 +1265,25 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
strategy: opencodeIssueSessionStrategy,
|
||||
})
|
||||
: undefined;
|
||||
} catch (err) {
|
||||
await cleanupSpawnWorkspaceAndMetadata(systemPromptFile, opencodeConfigFile);
|
||||
throw err;
|
||||
}
|
||||
const agentLaunchConfig = {
|
||||
sessionId,
|
||||
projectConfig: {
|
||||
...project,
|
||||
agentConfig: {
|
||||
...selection.agentConfig,
|
||||
...(reusedOpenCodeSessionId ? { opencodeSessionId: reusedOpenCodeSessionId } : {}),
|
||||
},
|
||||
},
|
||||
workspacePath,
|
||||
issueId: spawnConfig.issueId,
|
||||
prompt: taskPrompt,
|
||||
systemPromptFile,
|
||||
permissions: selection.permissions,
|
||||
model: selection.model,
|
||||
subagent: spawnConfig.subagent ?? selection.subagent,
|
||||
};
|
||||
|
||||
let handle: RuntimeHandle;
|
||||
try {
|
||||
const agentLaunchConfig = {
|
||||
sessionId,
|
||||
projectConfig: {
|
||||
...project,
|
||||
agentConfig: {
|
||||
...selection.agentConfig,
|
||||
...(reusedOpenCodeSessionId ? { opencodeSessionId: reusedOpenCodeSessionId } : {}),
|
||||
},
|
||||
},
|
||||
workspacePath,
|
||||
issueId: spawnConfig.issueId,
|
||||
prompt: taskPrompt,
|
||||
systemPromptFile,
|
||||
permissions: selection.permissions,
|
||||
model: selection.model,
|
||||
subagent: spawnConfig.subagent ?? selection.subagent,
|
||||
};
|
||||
|
||||
const launchCommand = plugins.agent.getLaunchCommand(agentLaunchConfig);
|
||||
const environment = plugins.agent.getEnvironment(agentLaunchConfig);
|
||||
|
||||
|
|
@ -1344,7 +1291,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
await plugins.agent.preLaunchSetup(workspacePath);
|
||||
}
|
||||
|
||||
handle = await plugins.runtime.create({
|
||||
const handle = await plugins.runtime.create({
|
||||
sessionId: tmuxName ?? sessionId, // Use tmux name for runtime if available
|
||||
workspacePath,
|
||||
launchCommand,
|
||||
|
|
@ -1367,54 +1314,50 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
config.port !== null && { AO_PORT: String(config.port) }),
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
// Clean up workspace, prompt file, and reserved ID if agent config or runtime creation failed
|
||||
await cleanupSpawnWorkspaceAndMetadata(systemPromptFile, opencodeConfigFile);
|
||||
throw err;
|
||||
}
|
||||
const rt = plugins.runtime;
|
||||
cleanupStack.push(() => rt.destroy(handle));
|
||||
|
||||
// Derive a stable display name from task context. Unlike issue-title
|
||||
// enrichment (which is a live tracker API call), this value is captured at
|
||||
// spawn time and persisted, so the dashboard has a good name even when the
|
||||
// tracker is unavailable or the session has no attached PR yet.
|
||||
const displayName = deriveDisplayName({
|
||||
issueTitle: resolvedIssue?.title,
|
||||
prompt: spawnConfig.prompt,
|
||||
});
|
||||
// Derive a stable display name from task context. Unlike issue-title
|
||||
// enrichment (which is a live tracker API call), this value is captured at
|
||||
// spawn time and persisted, so the dashboard has a good name even when the
|
||||
// tracker is unavailable or the session has no attached PR yet.
|
||||
const displayName = deriveDisplayName({
|
||||
issueTitle: resolvedIssue?.title,
|
||||
prompt: spawnConfig.prompt,
|
||||
});
|
||||
|
||||
// Write metadata and run post-launch setup — clean up on failure
|
||||
const createdAt = new Date();
|
||||
const lifecycle = createInitialCanonicalLifecycle("worker", createdAt);
|
||||
lifecycle.runtime.handle = handle;
|
||||
lifecycle.runtime.tmuxName = tmuxName ?? null;
|
||||
// Write metadata and run post-launch setup
|
||||
const createdAt = new Date();
|
||||
const lifecycle = createInitialCanonicalLifecycle("worker", createdAt);
|
||||
lifecycle.runtime.handle = handle;
|
||||
lifecycle.runtime.tmuxName = tmuxName ?? null;
|
||||
|
||||
const session: Session = {
|
||||
id: sessionId,
|
||||
projectId: spawnConfig.projectId,
|
||||
status: deriveLegacyStatus(lifecycle),
|
||||
activity: "active",
|
||||
activitySignal: createActivitySignal("valid", {
|
||||
const session: Session = {
|
||||
id: sessionId,
|
||||
projectId: spawnConfig.projectId,
|
||||
status: deriveLegacyStatus(lifecycle),
|
||||
activity: "active",
|
||||
timestamp: createdAt,
|
||||
source: "runtime",
|
||||
}),
|
||||
lifecycle,
|
||||
branch,
|
||||
issueId: spawnConfig.issueId ?? null,
|
||||
pr: null,
|
||||
workspacePath,
|
||||
runtimeHandle: handle,
|
||||
agentInfo: null,
|
||||
createdAt,
|
||||
lastActivityAt: createdAt,
|
||||
metadata: {
|
||||
...(reusedOpenCodeSessionId ? { opencodeSessionId: reusedOpenCodeSessionId } : {}),
|
||||
...(spawnConfig.prompt ? { userPrompt: spawnConfig.prompt } : {}),
|
||||
...(displayName ? { displayName } : {}),
|
||||
},
|
||||
};
|
||||
activitySignal: createActivitySignal("valid", {
|
||||
activity: "active",
|
||||
timestamp: createdAt,
|
||||
source: "runtime",
|
||||
}),
|
||||
lifecycle,
|
||||
branch,
|
||||
issueId: spawnConfig.issueId ?? null,
|
||||
pr: null,
|
||||
workspacePath,
|
||||
runtimeHandle: handle,
|
||||
agentInfo: null,
|
||||
createdAt,
|
||||
lastActivityAt: createdAt,
|
||||
metadata: {
|
||||
...(reusedOpenCodeSessionId ? { opencodeSessionId: reusedOpenCodeSessionId } : {}),
|
||||
...(spawnConfig.prompt ? { userPrompt: spawnConfig.prompt } : {}),
|
||||
...(displayName ? { displayName } : {}),
|
||||
},
|
||||
};
|
||||
|
||||
try {
|
||||
writeMetadata(sessionsDir, sessionId, {
|
||||
worktree: workspacePath,
|
||||
branch,
|
||||
|
|
@ -1463,71 +1406,74 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
updateMetadata(sessionsDir, sessionId, session.metadata);
|
||||
}
|
||||
invalidateCache();
|
||||
} catch (err) {
|
||||
// Clean up runtime and workspace on post-launch failure
|
||||
try {
|
||||
await plugins.runtime.destroy(handle);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
await cleanupSpawnWorkspaceAndMetadata(systemPromptFile);
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Send the task-specific prompt post-launch for agents that need it
|
||||
// (e.g. Claude Code exits after -p, so we send the prompt after it starts
|
||||
// in interactive mode).
|
||||
// This is intentionally outside the try/catch above — a prompt delivery failure
|
||||
// should NOT destroy the session. The agent is running; user can retry with `ao send`.
|
||||
let promptDelivered = false;
|
||||
if (plugins.agent.promptDelivery === "post-launch" && agentLaunchConfig.prompt) {
|
||||
const maxRetries = 3;
|
||||
const baseDelayMs = 3_000;
|
||||
let lastError: Error | undefined;
|
||||
// Past this point every resource that needed an undo is on disk in its
|
||||
// final form. Dismiss the stack so the prompt-delivery loop below (which
|
||||
// is intentionally non-fatal) cannot trigger a rollback.
|
||||
cleanupStack.dismiss();
|
||||
|
||||
for (let attempt = 1; attempt <= maxRetries; attempt++) {
|
||||
try {
|
||||
// Wait for agent to start and be ready for input
|
||||
// Use exponential backoff: 3s, 6s, 9s between attempts
|
||||
await new Promise((resolve) => setTimeout(resolve, baseDelayMs * attempt));
|
||||
await plugins.runtime.sendMessage(handle, agentLaunchConfig.prompt);
|
||||
promptDelivered = true;
|
||||
break;
|
||||
} catch (err) {
|
||||
lastError = err instanceof Error ? err : new Error(String(err));
|
||||
// Send the task-specific prompt post-launch for agents that need it
|
||||
// (e.g. Claude Code exits after -p, so we send the prompt after it starts
|
||||
// in interactive mode). Prompt delivery failure must NOT destroy the
|
||||
// session — the agent is running; user can retry with `ao send`.
|
||||
let promptDelivered = false;
|
||||
if (plugins.agent.promptDelivery === "post-launch" && agentLaunchConfig.prompt) {
|
||||
const maxRetries = 3;
|
||||
const baseDelayMs = 3_000;
|
||||
let lastError: Error | undefined;
|
||||
|
||||
for (let attempt = 1; attempt <= maxRetries; attempt++) {
|
||||
try {
|
||||
// Wait for agent to start and be ready for input
|
||||
// Use exponential backoff: 3s, 6s, 9s between attempts
|
||||
await new Promise((resolve) => setTimeout(resolve, baseDelayMs * attempt));
|
||||
await plugins.runtime.sendMessage(handle, agentLaunchConfig.prompt);
|
||||
promptDelivered = true;
|
||||
break;
|
||||
} catch (err) {
|
||||
lastError = err instanceof Error ? err : new Error(String(err));
|
||||
console.error(
|
||||
`[session-manager] Prompt delivery attempt ${attempt}/${maxRetries} failed: ${lastError.message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (!promptDelivered) {
|
||||
console.error(
|
||||
`[session-manager] Prompt delivery attempt ${attempt}/${maxRetries} failed: ${lastError.message}`,
|
||||
`[session-manager] FAILED to deliver prompt to session ${sessionId} after ${maxRetries} attempts. ` +
|
||||
`User must send manually with 'ao send'. Last error: ${lastError?.message}`,
|
||||
);
|
||||
}
|
||||
|
||||
session.metadata["promptDelivered"] = String(promptDelivered);
|
||||
} else if (agentLaunchConfig.prompt) {
|
||||
session.metadata["promptDelivered"] = "true";
|
||||
}
|
||||
|
||||
if (!promptDelivered) {
|
||||
console.error(
|
||||
`[session-manager] FAILED to deliver prompt to session ${sessionId} after ${maxRetries} attempts. ` +
|
||||
`User must send manually with 'ao send'. Last error: ${lastError?.message}`,
|
||||
);
|
||||
if (session.metadata["promptDelivered"]) {
|
||||
updateMetadata(sessionsDir, sessionId, session.metadata);
|
||||
invalidateCache();
|
||||
}
|
||||
|
||||
session.metadata["promptDelivered"] = String(promptDelivered);
|
||||
} else if (agentLaunchConfig.prompt) {
|
||||
session.metadata["promptDelivered"] = "true";
|
||||
recordActivityEvent({
|
||||
projectId: spawnConfig.projectId,
|
||||
sessionId,
|
||||
source: "session-manager",
|
||||
kind: "session.spawned",
|
||||
summary: `spawned: ${sessionId}`,
|
||||
data: { agent: plugins.agent.name, branch: session.branch ?? undefined },
|
||||
});
|
||||
|
||||
return session;
|
||||
} catch (err) {
|
||||
// Log cleanup failures so they don't disappear silently. The original
|
||||
// code used /* best effort */ swallows; the stack preserves that
|
||||
// behavior (cleanup errors don't propagate) but surfaces them for debug.
|
||||
await cleanupStack.runAll((cleanupErr) => {
|
||||
console.error("[session-manager] spawn rollback step failed:", cleanupErr);
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
if (session.metadata["promptDelivered"]) {
|
||||
updateMetadata(sessionsDir, sessionId, session.metadata);
|
||||
invalidateCache();
|
||||
}
|
||||
|
||||
recordActivityEvent({
|
||||
projectId: spawnConfig.projectId,
|
||||
sessionId,
|
||||
source: "session-manager",
|
||||
kind: "session.spawned",
|
||||
summary: `spawned: ${sessionId}`,
|
||||
data: { agent: plugins.agent.name, branch: session.branch ?? undefined },
|
||||
});
|
||||
|
||||
return session;
|
||||
}
|
||||
|
||||
async function spawnOrchestrator(orchestratorConfig: OrchestratorSpawnConfig): Promise<Session> {
|
||||
|
|
|
|||
Loading…
Reference in New Issue