fix: poll issue status instead of heartbeat-run for cancel detection (FAR-60)
The cancel poller was calling GET /api/heartbeat-runs/{runId} which
returned 401 because the adapter key lacks access to the internal
heartbeat-runs endpoint. Switch to GET /api/issues/{issueId}, which
the adapter key can read. Also tighten the trigger condition from
status !== "running" to status === "cancelled" so that other terminal
states (done, blocked, etc.) do not abort the K8s job.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -54,7 +54,7 @@ const HAPPY_JSONL = [
|
|||||||
JSON.stringify({ type: "step_finish", part: { tokens: { input: 100, output: 50, cache: { read: 20 } }, cost: 0.002 } }),
|
JSON.stringify({ type: "step_finish", part: { tokens: { input: 100, output: 50, cache: { read: 20 } }, cost: 0.002 } }),
|
||||||
].join("\n");
|
].join("\n");
|
||||||
|
|
||||||
function makeCtx(configOverrides: Record<string, unknown> = {}): AdapterExecutionContext {
|
function makeCtx(configOverrides: Record<string, unknown> = {}, contextOverrides: Record<string, unknown> = {}): AdapterExecutionContext {
|
||||||
return {
|
return {
|
||||||
runId: "run-test-123",
|
runId: "run-test-123",
|
||||||
agent: { id: "agent-id-test", name: "Test Agent", companyId: "co-1", adapterType: null, adapterConfig: null },
|
agent: { id: "agent-id-test", name: "Test Agent", companyId: "co-1", adapterType: null, adapterConfig: null },
|
||||||
@@ -68,6 +68,7 @@ function makeCtx(configOverrides: Record<string, unknown> = {}): AdapterExecutio
|
|||||||
paperclipWorkspaces: null,
|
paperclipWorkspaces: null,
|
||||||
paperclipRuntimeServiceIntents: null,
|
paperclipRuntimeServiceIntents: null,
|
||||||
paperclipRuntimeServices: null,
|
paperclipRuntimeServices: null,
|
||||||
|
...contextOverrides,
|
||||||
},
|
},
|
||||||
onLog: vi.fn().mockResolvedValue(undefined),
|
onLog: vi.fn().mockResolvedValue(undefined),
|
||||||
} as unknown as AdapterExecutionContext;
|
} as unknown as AdapterExecutionContext;
|
||||||
@@ -883,16 +884,17 @@ describe("execute — external cancel polling", () => {
|
|||||||
delete process.env.PAPERCLIP_API_KEY;
|
delete process.env.PAPERCLIP_API_KEY;
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns errorCode=cancelled and deletes job when heartbeat-run status is not running", async () => {
|
it("returns errorCode=cancelled and deletes job when issue status is cancelled", async () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
|
|
||||||
process.env.PAPERCLIP_API_URL = "http://test-api";
|
process.env.PAPERCLIP_API_URL = "http://test-api";
|
||||||
process.env.PAPERCLIP_API_KEY = "test-key";
|
process.env.PAPERCLIP_API_KEY = "test-key";
|
||||||
|
|
||||||
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({
|
const fetchMock = vi.fn().mockResolvedValue({
|
||||||
ok: true,
|
ok: true,
|
||||||
json: () => Promise.resolve({ status: "cancelled" }),
|
json: () => Promise.resolve({ status: "cancelled" }),
|
||||||
}));
|
});
|
||||||
|
vi.stubGlobal("fetch", fetchMock);
|
||||||
|
|
||||||
let jobDeleted = false;
|
let jobDeleted = false;
|
||||||
const batchApi = makeBatchApi();
|
const batchApi = makeBatchApi();
|
||||||
@@ -909,7 +911,7 @@ describe("execute — external cancel polling", () => {
|
|||||||
});
|
});
|
||||||
vi.mocked(getBatchApi).mockReturnValue(batchApi as unknown as ReturnType<typeof getBatchApi>);
|
vi.mocked(getBatchApi).mockReturnValue(batchApi as unknown as ReturnType<typeof getBatchApi>);
|
||||||
|
|
||||||
const ctx = makeCtx();
|
const ctx = makeCtx({}, { issueId: "issue-test-456" });
|
||||||
const executePromise = execute(ctx);
|
const executePromise = execute(ctx);
|
||||||
|
|
||||||
// Advance in 1-second steps. vi.advanceTimersByTimeAsync fires fake timers
|
// Advance in 1-second steps. vi.advanceTimersByTimeAsync fires fake timers
|
||||||
@@ -928,6 +930,10 @@ describe("execute — external cancel polling", () => {
|
|||||||
expect(batchApi.deleteNamespacedJob).toHaveBeenCalledWith(
|
expect(batchApi.deleteNamespacedJob).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({ name: JOB_NAME, namespace: NAMESPACE, body: { propagationPolicy: "Background" } }),
|
expect.objectContaining({ name: JOB_NAME, namespace: NAMESPACE, body: { propagationPolicy: "Background" } }),
|
||||||
);
|
);
|
||||||
|
expect(fetchMock).toHaveBeenCalledWith(
|
||||||
|
"http://test-api/api/issues/issue-test-456",
|
||||||
|
expect.objectContaining({ headers: expect.objectContaining({ Authorization: "Bearer test-key" }) }),
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not cancel when PAPERCLIP_API_URL is absent", async () => {
|
it("does not cancel when PAPERCLIP_API_URL is absent", async () => {
|
||||||
@@ -941,7 +947,7 @@ describe("execute — external cancel polling", () => {
|
|||||||
expect(result.exitCode).toBe(0);
|
expect(result.exitCode).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not cancel when heartbeat-run status is still running", async () => {
|
it("does not cancel when issue status is not cancelled", async () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
|
|
||||||
process.env.PAPERCLIP_API_URL = "http://test-api";
|
process.env.PAPERCLIP_API_URL = "http://test-api";
|
||||||
@@ -949,10 +955,10 @@ describe("execute — external cancel polling", () => {
|
|||||||
|
|
||||||
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({
|
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({
|
||||||
ok: true,
|
ok: true,
|
||||||
json: () => Promise.resolve({ status: "running" }),
|
json: () => Promise.resolve({ status: "in_progress" }),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const ctx = makeCtx();
|
const ctx = makeCtx({}, { issueId: "issue-test-456" });
|
||||||
const executePromise = execute(ctx);
|
const executePromise = execute(ctx);
|
||||||
|
|
||||||
await vi.advanceTimersByTimeAsync(KEEPALIVE_MS + 500);
|
await vi.advanceTimersByTimeAsync(KEEPALIVE_MS + 500);
|
||||||
|
|||||||
@@ -495,7 +495,7 @@ async function streamAndAwaitJob(
|
|||||||
const logStopSignal = { stopped: false };
|
const logStopSignal = { stopped: false };
|
||||||
const logDedup = new LogLineDedupFilter();
|
const logDedup = new LogLineDedupFilter();
|
||||||
|
|
||||||
const runId = ctx.runId;
|
const issueId = asString(ctx.context.issueId ?? ctx.context.taskId, "").trim();
|
||||||
let lastLogAt = Date.now();
|
let lastLogAt = Date.now();
|
||||||
let keepaliveJobTerminal = false;
|
let keepaliveJobTerminal = false;
|
||||||
let consecutiveTerminalReadings = 0;
|
let consecutiveTerminalReadings = 0;
|
||||||
@@ -525,22 +525,24 @@ async function streamAndAwaitJob(
|
|||||||
})();
|
})();
|
||||||
}, KEEPALIVE_INTERVAL_MS);
|
}, KEEPALIVE_INTERVAL_MS);
|
||||||
|
|
||||||
// External cancel poll: watches Paperclip run status at keepalive cadence.
|
// External cancel poll: watches Paperclip issue status at keepalive cadence.
|
||||||
|
// Polls GET /api/issues/{issueId} (not /api/heartbeat-runs) because the adapter
|
||||||
|
// key has read access to issues but not to the internal heartbeat-runs endpoint.
|
||||||
// Uses await-setTimeout (not setInterval+void) so vi.advanceTimersByTimeAsync
|
// Uses await-setTimeout (not setInterval+void) so vi.advanceTimersByTimeAsync
|
||||||
// can drive it in tests. Fire-and-forget; exits when logStopSignal.stopped.
|
// can drive it in tests. Fire-and-forget; exits when logStopSignal.stopped.
|
||||||
void (async (): Promise<void> => {
|
void (async (): Promise<void> => {
|
||||||
const apiUrl = process.env.PAPERCLIP_API_URL;
|
const apiUrl = process.env.PAPERCLIP_API_URL;
|
||||||
if (!apiUrl || !runId) return;
|
if (!apiUrl || !issueId) return;
|
||||||
while (!logStopSignal.stopped && !cancelSignal.cancelled) {
|
while (!logStopSignal.stopped && !cancelSignal.cancelled) {
|
||||||
await new Promise<void>((resolve) => setTimeout(resolve, KEEPALIVE_INTERVAL_MS));
|
await new Promise<void>((resolve) => setTimeout(resolve, KEEPALIVE_INTERVAL_MS));
|
||||||
if (logStopSignal.stopped || cancelSignal.cancelled) break;
|
if (logStopSignal.stopped || cancelSignal.cancelled) break;
|
||||||
try {
|
try {
|
||||||
const resp = await fetch(`${apiUrl}/api/heartbeat-runs/${runId}`, {
|
const resp = await fetch(`${apiUrl}/api/issues/${issueId}`, {
|
||||||
headers: { Authorization: `Bearer ${process.env.PAPERCLIP_API_KEY ?? ""}` },
|
headers: { Authorization: `Bearer ${process.env.PAPERCLIP_API_KEY ?? ""}` },
|
||||||
});
|
});
|
||||||
if (resp.ok) {
|
if (resp.ok) {
|
||||||
const data = await resp.json() as { status?: string };
|
const data = await resp.json() as { status?: string };
|
||||||
if (typeof data.status === "string" && data.status !== "running") {
|
if (typeof data.status === "string" && data.status === "cancelled") {
|
||||||
cancelSignal.cancelled = true;
|
cancelSignal.cancelled = true;
|
||||||
logStopSignal.stopped = true;
|
logStopSignal.stopped = true;
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user