mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-16 15:15:35 +02:00
fix(server): report failing workspace jobs as 422 instead of 500 (#15941)
A non-zero workspace job exit is the operator job's own result. Map it to 422 workspace_job_failed with the exit code instead of a 500 that the error handler reports to Sentry as a server crash. A job without a command is also 422. Other workspace command failures are unchanged. Co-Authored-By: Amy (Opus) <claude@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+95
-17
No files matched your search
@@ -50,6 +50,7 @@ import {
|
|||||||
resolveRuntimeProvisionCommand,
|
resolveRuntimeProvisionCommand,
|
||||||
resolveWorkspaceRuntimeReadinessTimeoutSec,
|
resolveWorkspaceRuntimeReadinessTimeoutSec,
|
||||||
resolveShell,
|
resolveShell,
|
||||||
|
runWorkspaceJobForControl,
|
||||||
sanitizeRuntimeServiceBaseEnv,
|
sanitizeRuntimeServiceBaseEnv,
|
||||||
setWorkspaceRuntimeExposureDepsForTests,
|
setWorkspaceRuntimeExposureDepsForTests,
|
||||||
startRuntimeServicesForWorkspaceControl,
|
startRuntimeServicesForWorkspaceControl,
|
||||||
@@ -80,6 +81,7 @@ import {
|
|||||||
import { resolvePaperclipConfigPath } from "../paths.ts";
|
import { resolvePaperclipConfigPath } from "../paths.ts";
|
||||||
import type { WorkspaceOperation } from "@paperclipai/shared";
|
import type { WorkspaceOperation } from "@paperclipai/shared";
|
||||||
import type { WorkspaceOperationRecorder } from "../services/workspace-operations.ts";
|
import type { WorkspaceOperationRecorder } from "../services/workspace-operations.ts";
|
||||||
|
import { HttpError } from "../errors.ts";
|
||||||
import { deriveWorktreeInstanceId } from "../services/workspace-instance-cleanup.ts";
|
import { deriveWorktreeInstanceId } from "../services/workspace-instance-cleanup.ts";
|
||||||
import {
|
import {
|
||||||
getEmbeddedPostgresTestSupport,
|
getEmbeddedPostgresTestSupport,
|
||||||
@@ -10194,3 +10196,55 @@ describe("realizeExecutionWorkspace with an exact existing branch", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("runWorkspaceJobForControl", () => {
|
||||||
|
const actor = { id: null, name: "Board", companyId: "company-1" };
|
||||||
|
|
||||||
|
it("reports a failing operator job as an unprocessable job result, not a server error", async () => {
|
||||||
|
const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-workspace-job-failure-"));
|
||||||
|
const { recorder, operations } = createWorkspaceOperationRecorderDouble();
|
||||||
|
const jobScript = "console.error('Restored patch differs from exact source'); process.exit(3);";
|
||||||
|
|
||||||
|
try {
|
||||||
|
const failure = await runWorkspaceJobForControl({
|
||||||
|
actor,
|
||||||
|
issue: null,
|
||||||
|
workspace: buildWorkspace(workspaceRoot),
|
||||||
|
command: {
|
||||||
|
name: "Recover retained files",
|
||||||
|
command: `${JSON.stringify(process.execPath)} -e ${JSON.stringify(jobScript)}`,
|
||||||
|
},
|
||||||
|
recorder,
|
||||||
|
}).then(() => null, (error: unknown) => error);
|
||||||
|
|
||||||
|
expect(failure).toBeInstanceOf(HttpError);
|
||||||
|
expect(failure).toMatchObject({
|
||||||
|
status: 422,
|
||||||
|
message: 'Workspace job "Recover retained files" failed: Restored patch differs from exact source',
|
||||||
|
details: { code: "workspace_job_failed", exitCode: 3 },
|
||||||
|
});
|
||||||
|
expect(operations).toHaveLength(1);
|
||||||
|
expect(operations[0]?.result).toMatchObject({ status: "failed", exitCode: 3 });
|
||||||
|
} finally {
|
||||||
|
await fs.rm(workspaceRoot, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a job without a command as unprocessable", async () => {
|
||||||
|
const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-workspace-job-missing-"));
|
||||||
|
const { recorder, operations } = createWorkspaceOperationRecorderDouble();
|
||||||
|
|
||||||
|
try {
|
||||||
|
await expect(runWorkspaceJobForControl({
|
||||||
|
actor,
|
||||||
|
issue: null,
|
||||||
|
workspace: buildWorkspace(workspaceRoot),
|
||||||
|
command: { name: "Empty job" },
|
||||||
|
recorder,
|
||||||
|
})).rejects.toMatchObject({ status: 422, message: 'Workspace job "Empty job" is missing command' });
|
||||||
|
expect(operations).toHaveLength(0);
|
||||||
|
} finally {
|
||||||
|
await fs.rm(workspaceRoot, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -34,7 +34,7 @@ import {
|
|||||||
} from "@paperclipai/shared";
|
} from "@paperclipai/shared";
|
||||||
import { and, desc, eq, gte, inArray, isNull, lte, ne, or } from "drizzle-orm";
|
import { and, desc, eq, gte, inArray, isNull, lte, ne, or } from "drizzle-orm";
|
||||||
import { asNumber, asString, parseObject, renderTemplate } from "../adapters/utils.js";
|
import { asNumber, asString, parseObject, renderTemplate } from "../adapters/utils.js";
|
||||||
import { conflict } from "../errors.js";
|
import { conflict, unprocessable } from "../errors.js";
|
||||||
import { resolveHomeAwarePath } from "../home-paths.js";
|
import { resolveHomeAwarePath } from "../home-paths.js";
|
||||||
import { hasVerifiedWorktreeSeedManifest, isVerifiedWorktreeSeedManifest } from "../worktree-seed-manifest.js";
|
import { hasVerifiedWorktreeSeedManifest, isVerifiedWorktreeSeedManifest } from "../worktree-seed-manifest.js";
|
||||||
import {
|
import {
|
||||||
@@ -3055,6 +3055,19 @@ function resolveRepoManagedWorkspaceCommand(command: string, repoRoot: string) {
|
|||||||
return command;
|
return command;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A configured workspace command ran to completion but exited non-zero. The message carries the
|
||||||
|
* command's own output. The default `Error` name is kept so existing reports keep their grouping.
|
||||||
|
*/
|
||||||
|
export class WorkspaceCommandExitError extends Error {
|
||||||
|
readonly exitCode: number | null;
|
||||||
|
|
||||||
|
constructor(message: string, exitCode: number | null) {
|
||||||
|
super(message);
|
||||||
|
this.exitCode = exitCode;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function runWorkspaceCommand(input: {
|
async function runWorkspaceCommand(input: {
|
||||||
command: string;
|
command: string;
|
||||||
resolvedCommand?: string;
|
resolvedCommand?: string;
|
||||||
@@ -3075,10 +3088,11 @@ async function runWorkspaceCommand(input: {
|
|||||||
if (proc.code === 0) return;
|
if (proc.code === 0) return;
|
||||||
|
|
||||||
const details = [proc.stderr.trim(), proc.stdout.trim()].filter(Boolean).join("\n");
|
const details = [proc.stderr.trim(), proc.stdout.trim()].filter(Boolean).join("\n");
|
||||||
throw new Error(
|
throw new WorkspaceCommandExitError(
|
||||||
details.length > 0
|
details.length > 0
|
||||||
? `${input.label} failed: ${details}`
|
? `${input.label} failed: ${details}`
|
||||||
: `${input.label} failed with exit code ${proc.code ?? -1}`,
|
: `${input.label} failed with exit code ${proc.code ?? -1}`,
|
||||||
|
proc.code,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3211,10 +3225,11 @@ async function recordWorkspaceCommandOperation(
|
|||||||
if (code === 0) return operation;
|
if (code === 0) return operation;
|
||||||
|
|
||||||
const details = [stderr.trim(), stdout.trim()].filter(Boolean).join("\n");
|
const details = [stderr.trim(), stdout.trim()].filter(Boolean).join("\n");
|
||||||
throw new Error(
|
throw new WorkspaceCommandExitError(
|
||||||
details.length > 0
|
details.length > 0
|
||||||
? `${input.label} failed: ${details}`
|
? `${input.label} failed: ${details}`
|
||||||
: `${input.label} failed with exit code ${code ?? -1}`,
|
: `${input.label} failed with exit code ${code ?? -1}`,
|
||||||
|
code,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -5139,23 +5154,32 @@ export async function runWorkspaceJobForControl(input: {
|
|||||||
adapterEnv: input.adapterEnv ?? {},
|
adapterEnv: input.adapterEnv ?? {},
|
||||||
});
|
});
|
||||||
if (!resolved.command) {
|
if (!resolved.command) {
|
||||||
throw new Error(`Workspace job "${resolved.name}" is missing command`);
|
throw unprocessable(`Workspace job "${resolved.name}" is missing command`);
|
||||||
}
|
}
|
||||||
|
|
||||||
await ensureServerWorkspaceLinksCurrent(resolved.cwd);
|
await ensureServerWorkspaceLinksCurrent(resolved.cwd);
|
||||||
return await recordWorkspaceCommandOperation(input.recorder, {
|
try {
|
||||||
phase: "workspace_provision",
|
return await recordWorkspaceCommandOperation(input.recorder, {
|
||||||
command: resolved.command,
|
phase: "workspace_provision",
|
||||||
cwd: resolved.cwd,
|
command: resolved.command,
|
||||||
env: resolved.env,
|
cwd: resolved.cwd,
|
||||||
label: `Workspace job "${resolved.name}"`,
|
env: resolved.env,
|
||||||
metadata: {
|
label: `Workspace job "${resolved.name}"`,
|
||||||
workspaceCommandKind: "job",
|
metadata: {
|
||||||
workspaceCommandName: resolved.name,
|
workspaceCommandKind: "job",
|
||||||
...(input.metadata ?? {}),
|
workspaceCommandName: resolved.name,
|
||||||
},
|
...(input.metadata ?? {}),
|
||||||
successMessage: `Completed workspace job "${resolved.name}"\n`,
|
},
|
||||||
});
|
successMessage: `Completed workspace job "${resolved.name}"\n`,
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
// A job is an operator-authored command: a non-zero exit is its result, already recorded on
|
||||||
|
// the operation, not a server fault.
|
||||||
|
if (error instanceof WorkspaceCommandExitError) {
|
||||||
|
throw unprocessable(error.message, { code: "workspace_job_failed", exitCode: error.exitCode });
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function resolveServiceScopeId(input: {
|
function resolveServiceScopeId(input: {
|
||||||
|
|||||||
Reference in new issue
Block a user