fix(routines): clear transient execution failures (#9689)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Scheduled routines track each dispatch in `routine_runs` and link it to an execution issue > - Moving an execution issue to `blocked` or `cancelled` correctly records a failed run state for operator visibility > - When that issue later resumes or completes, the run can retain the earlier failure reason and completion timestamp > - That stale state makes an active or successfully completed routine appear failed > - This pull request reconciles the run back to a live state on resume and preserves cleared failure details as completion context > - The benefit is that routine run status consistently reflects the current execution issue lifecycle without losing useful recovery history ## Linked Issues or Issue Description Refs #9201 ### What happened? A routine execution issue that temporarily moved to `blocked` or `cancelled` caused its linked routine run to become `failed`. If the issue later returned to an active status or reached `done`, the routine run could keep the stale failure reason and terminal timestamp. ### Expected behavior Active execution issues should have an `issue_created` run with no failure or completion timestamp. Completed execution issues should have a `completed` run with no active failure reason, while retaining any earlier transient failure in structured trigger context for diagnosis. ### Steps to reproduce 1. Create a routine run linked to a routine execution issue. 2. Move the issue to `blocked` and synchronize the run state. 3. Move the issue back to `in_progress` or forward to `done` and synchronize again. 4. Observe that the run previously retained stale failed-state fields. ### Environment - Reproduced on `master` at `da549123cc`. - Core server behavior; not adapter-specific. - Covered with the embedded PostgreSQL routines service test harness. ## What Changed - Load the linked routine run while synchronizing execution issue status. - Restore transiently failed runs to `issue_created` when their execution issue resumes active work. - Clear stale failure state when an execution issue completes and retain the earlier failure under `triggerPayload.transientFailure`. - Add regression coverage for both resumed and completed execution issues. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/routines-service.test.ts` — 57 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. ## Risks - Low risk: the change is limited to routine execution issue/run reconciliation. - A completed run now stores a prior failed-state reason as structured transient context instead of leaving `failureReason` populated. - No schema, migration, API contract, or UI behavior changes are included. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex using GPT-5.4 with reasoning, repository tools, GitHub CLI access, code execution, and focused test execution. The runtime did not expose a context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
8161244284
commit
01ddc26a37
|
|
@ -239,6 +239,136 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
.then((rows) => rows[0]!);
|
||||
}
|
||||
|
||||
it("clears transient routine run failures when execution issues resume", async () => {
|
||||
const { companyId, issueSvc, routine, svc } = await seedFixture();
|
||||
const runId = randomUUID();
|
||||
const executionIssue = await issueSvc.create(companyId, {
|
||||
projectId: routine.projectId,
|
||||
title: routine.title,
|
||||
description: routine.description,
|
||||
status: "blocked",
|
||||
priority: routine.priority,
|
||||
assigneeAgentId: routine.assigneeAgentId,
|
||||
originKind: "routine_execution",
|
||||
originId: routine.id,
|
||||
originRunId: runId,
|
||||
});
|
||||
|
||||
await db.insert(routineRuns).values({
|
||||
id: runId,
|
||||
companyId,
|
||||
routineId: routine.id,
|
||||
source: "manual",
|
||||
status: "issue_created",
|
||||
triggeredAt: new Date("2026-07-16T12:00:00.000Z"),
|
||||
linkedIssueId: executionIssue.id,
|
||||
});
|
||||
|
||||
await svc.syncRunStatusForIssue(executionIssue.id);
|
||||
const [failedRun] = await db.select().from(routineRuns).where(eq(routineRuns.id, runId));
|
||||
expect(failedRun).toMatchObject({
|
||||
status: "failed",
|
||||
failureReason: "Execution issue moved to blocked",
|
||||
triggerPayload: {
|
||||
transientFailure: {
|
||||
code: "execution_issue_status",
|
||||
status: "blocked",
|
||||
},
|
||||
},
|
||||
});
|
||||
await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, executionIssue.id));
|
||||
await svc.syncRunStatusForIssue(executionIssue.id);
|
||||
|
||||
const [run] = await db.select().from(routineRuns).where(eq(routineRuns.id, runId));
|
||||
expect(run).toMatchObject({
|
||||
status: "issue_created",
|
||||
failureReason: null,
|
||||
completedAt: null,
|
||||
triggerPayload: {
|
||||
transientFailure: {
|
||||
code: "execution_issue_status",
|
||||
status: "blocked",
|
||||
clearedAt: expect.any(String),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const clearedAt = (run?.triggerPayload as { transientFailure?: { clearedAt?: string } } | null)
|
||||
?.transientFailure?.clearedAt;
|
||||
expect(clearedAt).toEqual(expect.any(String));
|
||||
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, executionIssue.id));
|
||||
await svc.syncRunStatusForIssue(executionIssue.id);
|
||||
|
||||
const [completedRun] = await db.select().from(routineRuns).where(eq(routineRuns.id, runId));
|
||||
expect(completedRun).toMatchObject({
|
||||
status: "completed",
|
||||
failureReason: null,
|
||||
triggerPayload: {
|
||||
transientFailure: {
|
||||
code: "execution_issue_status",
|
||||
status: "blocked",
|
||||
clearedAt,
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(completedRun?.completedAt).toBeInstanceOf(Date);
|
||||
});
|
||||
|
||||
it("moves transient routine run failures into completion context", async () => {
|
||||
const { companyId, issueSvc, routine, svc } = await seedFixture();
|
||||
const runId = randomUUID();
|
||||
const executionIssue = await issueSvc.create(companyId, {
|
||||
projectId: routine.projectId,
|
||||
title: routine.title,
|
||||
description: routine.description,
|
||||
status: "blocked",
|
||||
priority: routine.priority,
|
||||
assigneeAgentId: routine.assigneeAgentId,
|
||||
originKind: "routine_execution",
|
||||
originId: routine.id,
|
||||
originRunId: runId,
|
||||
});
|
||||
|
||||
await db.insert(routineRuns).values({
|
||||
id: runId,
|
||||
companyId,
|
||||
routineId: routine.id,
|
||||
source: "manual",
|
||||
status: "issue_created",
|
||||
triggeredAt: new Date("2026-07-16T12:00:00.000Z"),
|
||||
linkedIssueId: executionIssue.id,
|
||||
triggerPayload: { input: "preserved" },
|
||||
});
|
||||
|
||||
await svc.syncRunStatusForIssue(executionIssue.id);
|
||||
const [failedRun] = await db.select().from(routineRuns).where(eq(routineRuns.id, runId));
|
||||
expect(failedRun).toMatchObject({
|
||||
status: "failed",
|
||||
failureReason: "Execution issue moved to blocked",
|
||||
});
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, executionIssue.id));
|
||||
await svc.syncRunStatusForIssue(executionIssue.id);
|
||||
|
||||
const [run] = await db.select().from(routineRuns).where(eq(routineRuns.id, runId));
|
||||
expect(run).toMatchObject({
|
||||
status: "completed",
|
||||
failureReason: null,
|
||||
triggerPayload: {
|
||||
input: "preserved",
|
||||
transientFailure: {
|
||||
code: "execution_issue_status",
|
||||
status: "blocked",
|
||||
reason: "Execution issue moved to blocked",
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(run?.completedAt).toBeInstanceOf(Date);
|
||||
expect(run?.triggerPayload).toMatchObject({
|
||||
transientFailure: { clearedAt: expect.any(String) },
|
||||
});
|
||||
});
|
||||
|
||||
it("filters listed routines by project", async () => {
|
||||
const { companyId, agentId, projectId, routine, svc } = await seedFixture();
|
||||
const otherProjectId = randomUUID();
|
||||
|
|
|
|||
|
|
@ -83,6 +83,8 @@ const LIVE_HEARTBEAT_RUN_STATUSES = ["queued", "running", "scheduled_retry"];
|
|||
const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]);
|
||||
const MAX_CATCH_UP_RUNS = 25;
|
||||
const MAX_ROUTINE_REVISIONS = 100;
|
||||
const EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE = "execution_issue_status";
|
||||
const EXECUTION_ISSUE_TRANSIENT_FAILURE_STATUSES = ["blocked", "cancelled"] as const;
|
||||
const ACTIVITY_GATE_IGNORED_ACTIONS = [
|
||||
"issue.read_marked",
|
||||
"issue.read_unmarked",
|
||||
|
|
@ -100,6 +102,37 @@ const WEEKDAY_INDEX: Record<string, number> = {
|
|||
Sat: 6,
|
||||
};
|
||||
|
||||
type ExecutionIssueTransientFailureStatus = (typeof EXECUTION_ISSUE_TRANSIENT_FAILURE_STATUSES)[number];
|
||||
|
||||
function executionIssueTransientFailureReason(status: ExecutionIssueTransientFailureStatus) {
|
||||
return `Execution issue moved to ${status}`;
|
||||
}
|
||||
|
||||
function executionIssueTransientFailureStatusFromPayload(payload: unknown): ExecutionIssueTransientFailureStatus | null {
|
||||
if (!payload || typeof payload !== "object" || Array.isArray(payload)) return null;
|
||||
const transientFailure = (payload as Record<string, unknown>).transientFailure;
|
||||
if (!transientFailure || typeof transientFailure !== "object" || Array.isArray(transientFailure)) return null;
|
||||
const record = transientFailure as Record<string, unknown>;
|
||||
if (record.code !== EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE) return null;
|
||||
return EXECUTION_ISSUE_TRANSIENT_FAILURE_STATUSES.find((status) => record.status === status) ?? null;
|
||||
}
|
||||
|
||||
function executionIssueTransientFailureClearedAtFromPayload(payload: unknown): string | null {
|
||||
if (!payload || typeof payload !== "object" || Array.isArray(payload)) return null;
|
||||
const transientFailure = (payload as Record<string, unknown>).transientFailure;
|
||||
if (!transientFailure || typeof transientFailure !== "object" || Array.isArray(transientFailure)) return null;
|
||||
const clearedAt = (transientFailure as Record<string, unknown>).clearedAt;
|
||||
return typeof clearedAt === "string" ? clearedAt : null;
|
||||
}
|
||||
|
||||
function legacyExecutionIssueTransientFailureStatus(
|
||||
failureReason: string | null,
|
||||
): ExecutionIssueTransientFailureStatus | null {
|
||||
return EXECUTION_ISSUE_TRANSIENT_FAILURE_STATUSES.find(
|
||||
(status) => failureReason === executionIssueTransientFailureReason(status),
|
||||
) ?? null;
|
||||
}
|
||||
|
||||
async function resolveCompanyDefaultResponsibleUserId(db: Db, companyId: string) {
|
||||
const company = await db
|
||||
.select({ defaultResponsibleUserId: companies.defaultResponsibleUserId })
|
||||
|
|
@ -3144,17 +3177,73 @@ export function routineService(
|
|||
.where(eq(issues.id, issueId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!issue || issue.originKind !== "routine_execution" || !issue.originRunId) return null;
|
||||
const run = await db
|
||||
.select({
|
||||
id: routineRuns.id,
|
||||
status: routineRuns.status,
|
||||
failureReason: routineRuns.failureReason,
|
||||
triggerPayload: routineRuns.triggerPayload,
|
||||
})
|
||||
.from(routineRuns)
|
||||
.where(eq(routineRuns.id, issue.originRunId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!run) return null;
|
||||
if (issue.status === "done") {
|
||||
const transientFailureStatus = executionIssueTransientFailureStatusFromPayload(run.triggerPayload)
|
||||
?? legacyExecutionIssueTransientFailureStatus(run.failureReason);
|
||||
const transientFailureClearedAt = executionIssueTransientFailureClearedAtFromPayload(run.triggerPayload);
|
||||
return finalizeRun(issue.originRunId, {
|
||||
status: "completed",
|
||||
failureReason: null,
|
||||
completedAt: new Date(),
|
||||
...(transientFailureStatus
|
||||
? {
|
||||
triggerPayload: {
|
||||
...(run.triggerPayload ?? {}),
|
||||
transientFailure: {
|
||||
code: EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE,
|
||||
status: transientFailureStatus,
|
||||
reason: executionIssueTransientFailureReason(transientFailureStatus),
|
||||
clearedAt: transientFailureClearedAt ?? new Date().toISOString(),
|
||||
},
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
}
|
||||
if (issue.status === "blocked" || issue.status === "cancelled") {
|
||||
const failureReason = executionIssueTransientFailureReason(issue.status);
|
||||
return finalizeRun(issue.originRunId, {
|
||||
status: "failed",
|
||||
failureReason: `Execution issue moved to ${issue.status}`,
|
||||
failureReason,
|
||||
completedAt: new Date(),
|
||||
triggerPayload: {
|
||||
...(run.triggerPayload ?? {}),
|
||||
transientFailure: {
|
||||
code: EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE,
|
||||
status: issue.status,
|
||||
reason: failureReason,
|
||||
recordedAt: new Date().toISOString(),
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
const transientFailureStatus = executionIssueTransientFailureStatusFromPayload(run.triggerPayload)
|
||||
?? legacyExecutionIssueTransientFailureStatus(run.failureReason);
|
||||
if (run.status === "failed" && transientFailureStatus) {
|
||||
return finalizeRun(issue.originRunId, {
|
||||
status: "issue_created",
|
||||
failureReason: null,
|
||||
completedAt: null,
|
||||
triggerPayload: {
|
||||
...(run.triggerPayload ?? {}),
|
||||
transientFailure: {
|
||||
code: EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE,
|
||||
status: transientFailureStatus,
|
||||
reason: executionIssueTransientFailureReason(transientFailureStatus),
|
||||
clearedAt: new Date().toISOString(),
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
return null;
|
||||
|
|
|
|||
Loading…
Reference in New Issue