refactor: extract classifyOrphan helper with decision matrix (#8)
Co-Authored-By: Claude Sonnet <noreply@anthropic.com> Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
+35
-35
@@ -15,7 +15,7 @@ vi.mock("./k8s-client.js", () => ({
|
|||||||
resetCache: vi.fn(),
|
resetCache: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const { isK8s404, buildPartialRunError, isReattachableOrphan, describePodTerminatedError, streamPodLogsOnce } = await import("./execute.js");
|
const { isK8s404, buildPartialRunError, classifyOrphan, describePodTerminatedError, streamPodLogsOnce } = await import("./execute.js");
|
||||||
|
|
||||||
function makeJob(opts: {
|
function makeJob(opts: {
|
||||||
runId?: string;
|
runId?: string;
|
||||||
@@ -146,59 +146,59 @@ describe("buildPartialRunError", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("isReattachableOrphan", () => {
|
describe("classifyOrphan", () => {
|
||||||
const agentId = "agent-abc";
|
|
||||||
const taskId = "task-xyz";
|
const taskId = "task-xyz";
|
||||||
const sessionId = "sess-123";
|
const sessionId = "sess-123";
|
||||||
|
|
||||||
it("returns true when agent/task/session all match and Job is not terminal", () => {
|
// --- Happy path: reattach ---
|
||||||
const job = makeJob({ agentId, taskId, sessionId, runId: "old-run" });
|
it("returns reattach when taskId matches and both sessionIds match", () => {
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(true);
|
const job = makeJob({ taskId, sessionId });
|
||||||
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("reattach");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when the Job is already Complete", () => {
|
it("returns reattach when taskId matches and expected sessionId is null (missing on current side)", () => {
|
||||||
const job = makeJob({ agentId, taskId, sessionId, runId: "old-run", terminal: true });
|
const job = makeJob({ taskId, sessionId });
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
expect(classifyOrphan(job, { taskId, sessionId: null })).toBe("reattach");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when expected taskId is null (caller couldn't derive one)", () => {
|
it("returns reattach when taskId matches and job has no session-id label (missing on job side)", () => {
|
||||||
const job = makeJob({ agentId, taskId, sessionId });
|
const job = makeJob({ taskId });
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId: null, sessionId })).toBe(false);
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("reattach");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when expected sessionId is null", () => {
|
it("returns reattach when taskId matches and neither side has a sessionId", () => {
|
||||||
const job = makeJob({ agentId, taskId, sessionId });
|
const job = makeJob({ taskId });
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId: null })).toBe(false);
|
expect(classifyOrphan(job, { taskId, sessionId: null })).toBe("reattach");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when agent id doesn't match", () => {
|
// --- Block: task unknown ---
|
||||||
const job = makeJob({ agentId: "agent-other", taskId, sessionId });
|
it("returns block_task_unknown when expected taskId is null", () => {
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
const job = makeJob({ taskId, sessionId });
|
||||||
|
expect(classifyOrphan(job, { taskId: null, sessionId })).toBe("block_task_unknown");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when task id doesn't match", () => {
|
it("returns block_task_unknown when job has no task-id label", () => {
|
||||||
const job = makeJob({ agentId, taskId: "task-other", sessionId });
|
const job = makeJob({ sessionId });
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("block_task_unknown");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when session id doesn't match", () => {
|
// --- Block: task mismatch ---
|
||||||
const job = makeJob({ agentId, taskId, sessionId: "sess-other" });
|
it("returns block_task_mismatch when both sides have taskId but they differ", () => {
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
const job = makeJob({ taskId: "task-other", sessionId });
|
||||||
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("block_task_mismatch");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when the Job is from a different adapter type", () => {
|
// --- Block: session mismatch ---
|
||||||
const job = makeJob({ agentId, taskId, sessionId, adapterType: "claude_local" });
|
it("returns block_session_mismatch when taskId matches but sessionIds differ", () => {
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
const job = makeJob({ taskId, sessionId: "sess-other" });
|
||||||
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("block_session_mismatch");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns false when Job has no task-id label (labels were introduced in FAR-124)", () => {
|
// --- Terminal orphan (caller filters these before classifyOrphan) ---
|
||||||
const job = makeJob({ agentId, sessionId });
|
it("returns reattach for terminal job (caller is responsible for filtering terminals)", () => {
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
const job = makeJob({ taskId, sessionId, terminal: true });
|
||||||
});
|
// classifyOrphan does not check terminal status — that is the caller's job
|
||||||
|
expect(classifyOrphan(job, { taskId, sessionId })).toBe("reattach");
|
||||||
it("returns false when Job has no session-id label", () => {
|
|
||||||
const job = makeJob({ agentId, taskId });
|
|
||||||
expect(isReattachableOrphan(job, { agentId, taskId, sessionId })).toBe(false);
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+38
-29
@@ -89,30 +89,46 @@ export function buildPartialRunError(
|
|||||||
: `Claude exited with code ${exitCode ?? -1}`;
|
: `Claude exited with code ${exitCode ?? -1}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type OrphanClassification =
|
||||||
|
| "reattach"
|
||||||
|
| "block_session_mismatch"
|
||||||
|
| "block_task_mismatch"
|
||||||
|
| "block_task_unknown";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Evaluate an orphaned K8s Job (one whose `paperclip.io/run-id` label does
|
* Classify a non-terminal orphaned K8s Job (one whose `paperclip.io/run-id`
|
||||||
* not match the current runId) as a potential reattach target. A Job is
|
* label does not match the current runId but does belong to this agent) as a
|
||||||
* reattachable when it belongs to the same agent, same task, and same resume
|
* reattach candidate or a block reason.
|
||||||
* session as the current run — meaning the previous Paperclip instance was
|
*
|
||||||
* mid-stream on the exact piece of work this new run was dispatched to do.
|
* Decision matrix:
|
||||||
|
* - taskId mismatch (both present, different values) → block_task_mismatch
|
||||||
|
* - taskId missing on either side → block_task_unknown
|
||||||
|
* - taskId match + both have sessionId + sessionIds differ → block_session_mismatch
|
||||||
|
* - taskId match + one or both sides missing sessionId → reattach (reconcile)
|
||||||
|
* - taskId match + both have sessionId + sessionIds match → reattach (happy path)
|
||||||
|
*
|
||||||
* Exported for unit tests.
|
* Exported for unit tests.
|
||||||
*/
|
*/
|
||||||
export function isReattachableOrphan(
|
export function classifyOrphan(
|
||||||
job: k8s.V1Job,
|
job: k8s.V1Job,
|
||||||
expected: { agentId: string; taskId: string | null; sessionId: string | null },
|
expected: { taskId: string | null; sessionId: string | null },
|
||||||
): boolean {
|
): OrphanClassification {
|
||||||
if (!expected.taskId || !expected.sessionId) return false;
|
|
||||||
const labels = job.metadata?.labels ?? {};
|
const labels = job.metadata?.labels ?? {};
|
||||||
if (labels["paperclip.io/adapter-type"] !== "claude_k8s") return false;
|
const jobTaskId = labels["paperclip.io/task-id"] ?? null;
|
||||||
if (labels["paperclip.io/agent-id"] !== expected.agentId) return false;
|
const jobSessionId = labels["paperclip.io/session-id"] ?? null;
|
||||||
if (labels["paperclip.io/task-id"] !== expected.taskId) return false;
|
|
||||||
if (labels["paperclip.io/session-id"] !== expected.sessionId) return false;
|
// taskId missing on either side
|
||||||
const conditions = job.status?.conditions ?? [];
|
if (!expected.taskId || !jobTaskId) return "block_task_unknown";
|
||||||
const terminal = conditions.some(
|
|
||||||
(c) => (c.type === "Complete" || c.type === "Failed") && c.status === "True",
|
// taskId mismatch
|
||||||
);
|
if (expected.taskId !== jobTaskId) return "block_task_mismatch";
|
||||||
if (terminal) return false;
|
|
||||||
return true;
|
// taskId matches — check sessionId
|
||||||
|
if (expected.sessionId && jobSessionId && expected.sessionId !== jobSessionId) {
|
||||||
|
return "block_session_mismatch";
|
||||||
|
}
|
||||||
|
|
||||||
|
return "reattach";
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -553,18 +569,11 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||||||
(j) => (j.metadata?.labels?.["paperclip.io/run-id"] ?? "") === runId,
|
(j) => (j.metadata?.labels?.["paperclip.io/run-id"] ?? "") === runId,
|
||||||
);
|
);
|
||||||
|
|
||||||
// Pick the most recent reattachable orphan — same agent + task + session,
|
// Pick the most recent reattachable orphan — same task + session, not
|
||||||
// not terminal. Only one target is chosen; any other orphans get
|
// terminal. Only one target is chosen; any other orphans get cleaned up.
|
||||||
// cleaned up as before.
|
|
||||||
if (reattachOrphanedJobs && orphaned.length > 0) {
|
if (reattachOrphanedJobs && orphaned.length > 0) {
|
||||||
const candidates = orphaned
|
const candidates = orphaned
|
||||||
.filter((j) =>
|
.filter((j) => classifyOrphan(j, { taskId: currentTaskLabel, sessionId: currentSessionLabel }) === "reattach")
|
||||||
isReattachableOrphan(j, {
|
|
||||||
agentId,
|
|
||||||
taskId: currentTaskLabel,
|
|
||||||
sessionId: currentSessionLabel,
|
|
||||||
}),
|
|
||||||
)
|
|
||||||
.sort((a, b) => {
|
.sort((a, b) => {
|
||||||
const at = new Date(a.metadata?.creationTimestamp ?? 0).getTime();
|
const at = new Date(a.metadata?.creationTimestamp ?? 0).getTime();
|
||||||
const bt = new Date(b.metadata?.creationTimestamp ?? 0).getTime();
|
const bt = new Date(b.metadata?.creationTimestamp ?? 0).getTime();
|
||||||
|
|||||||
Reference in New Issue
Block a user