mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 05:56:19 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The control plane must keep each active issue on a clear execution or recovery path. > - A missing issue disposition can require more than one bounded repair attempt. > - A server restart could lose that repair path or move source ownership to the recovery owner. > - A parked or expired retry could also make the user interface show a false healthy state. > - Concurrent recovery loops must not schedule the same repair attempt twice. > - This pull request keeps retry state durable, makes scheduling atomic, and keeps source ownership stable. > - The benefit is that recovery continues after a restart and operators see the correct state. ## Linked Issues or Issue Description **What happened?** A run that ended without a valid issue disposition could lose its repair path after a server restart. Manager recovery could also change the source owner. In addition, a parked or expired retry could make the issue look healthy when no active work existed. Concurrent reconciliation could also schedule the same repair attempt twice. **Expected behavior** Paperclip must keep bounded source and manager repair attempts across restarts. Recovery ownership must stay separate from source issue ownership. The server and user interface must report only a live retry as active work. Each repair attempt must be scheduled at most once per company. **Steps to reproduce** 1. Start an agent run on an issue. 2. End the run without a valid issue disposition. 3. Let the first repair attempt schedule a retry. 4. Restart the server, let the retry time pass without a live run, or start two reconciliation loops together. 5. Observe that the repair path can stop, the issue can show a false healthy state, or duplicate retries can be created. **Paperclip version or commit** The problem existed on `master` before candidate head `d8e620fe86bade7df18decac332007f5821ae04f`. **Deployment mode** The problem affects self-hosted servers and local builds that use automatic recovery. ## What Changed - Persist bounded source-owner and manager repair lineages with stable fingerprints and retry limits. - Resume incomplete disposition repairs after a server restart. - Keep recovery ownership separate from source issue ownership and enforce source mutation authority. - Project live retry evidence into issue and blocker summaries. - Show recovery owner, return owner, attempt count, and retry state in the board user interface. - Treat expired or parked retries as attention states unless a queued or running attempt exists. - Atomically deduplicate disposition-repair wake requests with a company-scoped partial unique index. - Reuse the winning run when concurrent reconciliation loses the uniqueness race, without duplicate scheduling activity. - Honor disabled on-demand wake policy before recovery scheduling and again before delayed retry promotion. - Keep the new index migration safe for lagging seeded databases that already contain the index. - Add server and user interface tests for recovery, restart, ownership, retry, concurrency, and blocker states. - Update the implementation and execution semantics documents. ## Verification - Focused server recovery and ownership suites: 282 tests passed on the repaired base candidate. - Focused user interface recovery suites: 128 tests passed on the repaired base candidate. - Atomic-deduplication schema and recovery suites: 111 tests passed on the first Greptile repair. - Recovery and scheduled-retry wake-policy suites: 126 tests passed at `d8e620fe86bade7df18decac332007f5821ae04f`. - The exact lagging-source migration-order test passed after the index migration became idempotent: 1 test passed and 62 unrelated tests were skipped. - `@paperclipai/db` and `@paperclipai/server` typechecks passed at the current head. - Migration generation and migration safety checks passed for migration `0226_tan_colossus.sql`. - `pnpm check:token-gates` passed on the repaired base candidate. - `pnpm -r typecheck` passed on the repaired base candidate. - `pnpm build` passed on the repaired base candidate. - `pnpm test:run` passed 4,540 tests on the repaired base candidate. Four fixed-port cases met listeners that already existed on the host. - The two unchanged fixed-port files passed in an isolated network namespace: 129 tests passed and 27 tests were skipped. - Independent Security and QA reviews approved `63c0423aab54c66f2293a20b0fb3f3b013ee3ba8`; exact-head re-review is required after automated checks settle on `d8e620fe86bade7df18decac332007f5821ae04f`. ## Risks - Recovery orchestration affects issue liveness and ownership. The new paths use bounded attempts, stable fingerprints, row locks, authority checks, and database uniqueness. - A conservative attention state can show more warnings when a scheduled retry has no queued or running attempt. It does not hide stopped work. - Migration `0226_tan_colossus.sql` creates a partial unique index on a known-large table. Migrations run transactionally, so `CONCURRENTLY` is unavailable. The matching disposition-repair key namespace is introduced by this release, so deployed databases have no matching rows before the index is added. > 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 from the GPT-5 model family used agentic reasoning, tool use, and code execution. The runtime did not expose the exact model ID or context window. - Anthropic Claude Opus 5 used a 1M context window, tool use, and code execution for part of the user interface repair, as recorded in the commit history. ## 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> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
416 lines
14 KiB
TypeScript
416 lines
14 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import {
|
|
formatRecoveryAttemptLabel,
|
|
formatRecoveryLineageSummary,
|
|
formatRecoveryRetryOffset,
|
|
readRecoveryRetryLineage,
|
|
type RecoveryLineageInput,
|
|
} from "./recovery-lineage";
|
|
|
|
const NOW = new Date("2026-08-18T12:00:00.000Z");
|
|
|
|
function at(offsetMs: number) {
|
|
return new Date(NOW.getTime() + offsetMs).toISOString();
|
|
}
|
|
|
|
function sourceLaneAction(overrides: Partial<RecoveryLineageInput> = {}): RecoveryLineageInput {
|
|
return {
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
retryAgentId: "agent-owner",
|
|
attempt: 2,
|
|
maxAttempts: 5,
|
|
baseBackoffMs: 60_000,
|
|
jitterMs: 3_000,
|
|
retryAt: at(3 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
evidence: {},
|
|
attemptCount: 2,
|
|
maxAttempts: 5,
|
|
timeoutAt: at(3 * 60_000),
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function recoveryLaneAction(overrides: Partial<RecoveryLineageInput> = {}): RecoveryLineageInput {
|
|
return {
|
|
wakePolicy: {
|
|
type: "bounded_recovery_owner",
|
|
ownerAgentId: "agent-manager",
|
|
attempt: 1,
|
|
maxAttempts: 3,
|
|
retryAt: at(60_000),
|
|
scheduledRunId: "run-9",
|
|
preservesSourceAssignee: true,
|
|
},
|
|
evidence: { sourceAttemptCount: 5, sourceMaxAttempts: 5 },
|
|
attemptCount: 1,
|
|
maxAttempts: 3,
|
|
timeoutAt: at(60_000),
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
// Liveness is a question about now, so every case pins the clock. Fixtures date their retry
|
|
// times from NOW; left on the wall clock, a "future" retry silently becomes a past one and the
|
|
// suite would assert the opposite of what it claims.
|
|
beforeEach(() => {
|
|
vi.useFakeTimers();
|
|
vi.setSystemTime(NOW);
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.useRealTimers();
|
|
});
|
|
|
|
describe("readRecoveryRetryLineage", () => {
|
|
it("returns null when the action carries no bounded lineage", () => {
|
|
expect(readRecoveryRetryLineage({ wakePolicy: null })).toBeNull();
|
|
expect(readRecoveryRetryLineage({ wakePolicy: { type: "wake_owner" } })).toBeNull();
|
|
expect(readRecoveryRetryLineage({ wakePolicy: { type: "monitor" } })).toBeNull();
|
|
});
|
|
|
|
it("reads the source-owner lane with its stored attempt and due time", () => {
|
|
const lineage = readRecoveryRetryLineage(sourceLaneAction());
|
|
expect(lineage).not.toBeNull();
|
|
expect(lineage!.lane).toBe("source_owner");
|
|
expect(lineage!.attempt).toBe(2);
|
|
expect(lineage!.maxAttempts).toBe(5);
|
|
expect(lineage!.attemptsRemaining).toBe(3);
|
|
expect(lineage!.exhausted).toBe(false);
|
|
expect(lineage!.nextRetryAt).toBe(at(3 * 60_000));
|
|
expect(lineage!.retryAgentId).toBe("agent-owner");
|
|
expect(lineage!.hasDurablePath).toBe(true);
|
|
// The source lane is the original owner retrying itself, so it reports itself as
|
|
// preserving the deliverable even without an explicit policy flag.
|
|
expect(lineage!.preservesSourceAssignee).toBe(true);
|
|
expect(lineage!.sourceAttempt).toBe(2);
|
|
expect(lineage!.sourceMaxAttempts).toBe(5);
|
|
});
|
|
|
|
it("marks a source lane with no attempts left as exhausted and drops its stale due time", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 5,
|
|
maxAttempts: 5,
|
|
retryAt: at(-60_000),
|
|
},
|
|
}),
|
|
);
|
|
expect(lineage!.exhausted).toBe(true);
|
|
expect(lineage!.attemptsRemaining).toBe(0);
|
|
expect(lineage!.nextRetryAt).toBeNull();
|
|
expect(lineage!.hasDurablePath).toBe(false);
|
|
});
|
|
|
|
it("reads the manager lane and keeps the source lane's spent attempts from evidence", () => {
|
|
const lineage = readRecoveryRetryLineage(recoveryLaneAction());
|
|
expect(lineage!.lane).toBe("recovery_owner");
|
|
expect(lineage!.attempt).toBe(1);
|
|
expect(lineage!.maxAttempts).toBe(3);
|
|
expect(lineage!.retryAgentId).toBe("agent-manager");
|
|
expect(lineage!.preservesSourceAssignee).toBe(true);
|
|
expect(lineage!.sourceAttempt).toBe(5);
|
|
expect(lineage!.sourceMaxAttempts).toBe(5);
|
|
expect(lineage!.hasDurablePath).toBe(true);
|
|
});
|
|
|
|
it("treats a board escalation from this contract as an exhausted lane", () => {
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: {
|
|
type: "board_escalation",
|
|
reason: "recovery_owner_retry_exhausted",
|
|
attempt: 3,
|
|
maxAttempts: 3,
|
|
preservesSourceAssignee: true,
|
|
},
|
|
evidence: { sourceAttemptCount: 5, sourceMaxAttempts: 5 },
|
|
});
|
|
expect(lineage!.lane).toBe("board");
|
|
expect(lineage!.exhausted).toBe(true);
|
|
expect(lineage!.nextRetryAt).toBeNull();
|
|
expect(lineage!.hasDurablePath).toBe(false);
|
|
expect(lineage!.sourceMaxAttempts).toBe(5);
|
|
});
|
|
|
|
it("ignores an unrelated board escalation that is not part of the owner-sticky contract", () => {
|
|
expect(
|
|
readRecoveryRetryLineage({
|
|
wakePolicy: { type: "board_escalation", reason: "provider_quota" },
|
|
evidence: {},
|
|
}),
|
|
).toBeNull();
|
|
});
|
|
|
|
it("falls back to the action's own attempt fields when the policy omits them", () => {
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: { type: "bounded_owner_disposition_repair" },
|
|
attemptCount: 3,
|
|
maxAttempts: 5,
|
|
timeoutAt: at(120_000),
|
|
});
|
|
expect(lineage!.attempt).toBe(3);
|
|
expect(lineage!.maxAttempts).toBe(5);
|
|
expect(lineage!.nextRetryAt).toBe(at(120_000));
|
|
});
|
|
|
|
it("reports no durable path when nothing is scheduled yet", () => {
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
retryAgentId: "agent-owner",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
},
|
|
});
|
|
expect(lineage!.exhausted).toBe(false);
|
|
expect(lineage!.hasDurablePath).toBe(false);
|
|
});
|
|
|
|
it("ignores an unparseable stored due time", () => {
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: "not-a-date",
|
|
},
|
|
});
|
|
expect(lineage!.nextRetryAt).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("durable-path liveness", () => {
|
|
it("keeps a future retry durable and not expired", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
}),
|
|
)!;
|
|
expect(lineage.retryExpired).toBe(false);
|
|
expect(lineage.hasDurablePath).toBe(true);
|
|
expect(lineage.liveRunId).toBeNull();
|
|
});
|
|
|
|
it("treats a retry time in the past as expired with no durable path", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
timeoutAt: at(-5 * 60_000),
|
|
}),
|
|
)!;
|
|
// Attempts remain on paper, but the one that was promised never ran.
|
|
expect(lineage.exhausted).toBe(false);
|
|
expect(lineage.attemptsRemaining).toBe(4);
|
|
expect(lineage.retryExpired).toBe(true);
|
|
expect(lineage.hasDurablePath).toBe(false);
|
|
});
|
|
|
|
it("does not let a bare scheduled run id stand in for liveness", () => {
|
|
// The id only records that a run was created once. Without a liveness signal it proves
|
|
// nothing, which is why the past-due lane above stays expired despite carrying one.
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
scheduledRunId: "run-2",
|
|
},
|
|
})!;
|
|
expect(lineage.scheduledRunId).toBe("run-2");
|
|
expect(lineage.liveRunId).toBeNull();
|
|
expect(lineage.hasDurablePath).toBe(false);
|
|
});
|
|
|
|
it("holds a past retry durable while a verified live run is executing it", () => {
|
|
const action = sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
timeoutAt: at(-5 * 60_000),
|
|
});
|
|
for (const status of ["queued", "running"] as const) {
|
|
const lineage = readRecoveryRetryLineage(action, {
|
|
scheduledRetry: { runId: "run-2", status },
|
|
})!;
|
|
expect(lineage.liveRunId).toBe("run-2");
|
|
// The due time is behind us precisely because the run picked it up.
|
|
expect(lineage.retryExpired).toBe(false);
|
|
expect(lineage.hasDurablePath).toBe(true);
|
|
}
|
|
});
|
|
|
|
it("rejects a scheduled run that is no longer live", () => {
|
|
const action = sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
timeoutAt: at(-5 * 60_000),
|
|
});
|
|
for (const status of ["cancelled", "scheduled_retry"] as const) {
|
|
const lineage = readRecoveryRetryLineage(action, {
|
|
scheduledRetry: { runId: "run-2", status },
|
|
})!;
|
|
expect(lineage.liveRunId).toBeNull();
|
|
expect(lineage.retryExpired).toBe(true);
|
|
expect(lineage.hasDurablePath).toBe(false);
|
|
}
|
|
});
|
|
|
|
it("ignores a live run that is not the one this lane parked", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
timeoutAt: at(-5 * 60_000),
|
|
}),
|
|
{ scheduledRetry: { runId: "run-somebody-else", status: "running" } },
|
|
)!;
|
|
expect(lineage.liveRunId).toBeNull();
|
|
expect(lineage.hasDurablePath).toBe(false);
|
|
});
|
|
|
|
it("keeps an exhausted lane dead even while a live run is reported", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 5,
|
|
maxAttempts: 5,
|
|
retryAt: at(-60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
}),
|
|
{ scheduledRetry: { runId: "run-2", status: "running" } },
|
|
)!;
|
|
expect(lineage.exhausted).toBe(true);
|
|
expect(lineage.hasDurablePath).toBe(false);
|
|
// An exhausted lane reports no upcoming attempt, so it is not also "missed".
|
|
expect(lineage.retryExpired).toBe(false);
|
|
});
|
|
|
|
it("does not call a barely-due retry expired while surfaces still read it as now", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5_000),
|
|
},
|
|
}),
|
|
)!;
|
|
expect(formatRecoveryRetryOffset(lineage)).toBe("now");
|
|
expect(lineage.retryExpired).toBe(false);
|
|
expect(lineage.hasDurablePath).toBe(true);
|
|
});
|
|
|
|
it("honours an injected now", () => {
|
|
const action = sourceLaneAction();
|
|
expect(readRecoveryRetryLineage(action, { now: NOW.getTime() })!.retryExpired).toBe(false);
|
|
expect(
|
|
readRecoveryRetryLineage(action, { now: NOW.getTime() + 60 * 60_000 })!.retryExpired,
|
|
).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("recovery lineage formatting", () => {
|
|
it("formats the attempt budget", () => {
|
|
expect(formatRecoveryAttemptLabel(readRecoveryRetryLineage(sourceLaneAction())!)).toBe(
|
|
"Attempt 2 of 5",
|
|
);
|
|
});
|
|
|
|
it("clamps a spent attempt count to the budget", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: { type: "bounded_owner_disposition_repair", attempt: 6, maxAttempts: 5 },
|
|
}),
|
|
)!;
|
|
expect(formatRecoveryAttemptLabel(lineage)).toBe("Attempt 5 of 5");
|
|
});
|
|
|
|
it("formats the next due time relative to now", () => {
|
|
const lineage = readRecoveryRetryLineage(sourceLaneAction())!;
|
|
expect(formatRecoveryRetryOffset(lineage)).toBe("in 3m");
|
|
expect(formatRecoveryLineageSummary(lineage)).toBe("Attempt 2 of 5 · next try in 3m");
|
|
});
|
|
|
|
it("reports a missed retry instead of an upcoming one (PAP-17561 regression)", () => {
|
|
// The reported false healthy state was exactly "Attempt 1 of 5 · next try 5m ago".
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 1,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
timeoutAt: at(-5 * 60_000),
|
|
}),
|
|
)!;
|
|
const summary = formatRecoveryLineageSummary(lineage);
|
|
expect(summary).toBe("Attempt 1 of 5 · retry missed 5m ago");
|
|
expect(summary).not.toContain("next try");
|
|
});
|
|
|
|
it("says the attempt is running when a live run is verified", () => {
|
|
const lineage = readRecoveryRetryLineage(
|
|
sourceLaneAction({
|
|
wakePolicy: {
|
|
type: "bounded_owner_disposition_repair",
|
|
attempt: 2,
|
|
maxAttempts: 5,
|
|
retryAt: at(-5 * 60_000),
|
|
scheduledRunId: "run-2",
|
|
},
|
|
}),
|
|
{ scheduledRetry: { runId: "run-2", status: "running" } },
|
|
)!;
|
|
expect(formatRecoveryLineageSummary(lineage)).toBe("Attempt 2 of 5 · attempt running now");
|
|
});
|
|
|
|
it("says retries are used up when a lane is exhausted", () => {
|
|
const lineage = readRecoveryRetryLineage({
|
|
wakePolicy: {
|
|
type: "board_escalation",
|
|
reason: "recovery_owner_retry_exhausted",
|
|
attempt: 3,
|
|
maxAttempts: 3,
|
|
preservesSourceAssignee: true,
|
|
},
|
|
})!;
|
|
expect(formatRecoveryRetryOffset(lineage)).toBeNull();
|
|
expect(formatRecoveryLineageSummary(lineage)).toBe("Attempt 3 of 3 · retries used up");
|
|
});
|
|
});
|