mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-15 01:52:55 +01:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a person clicks Retry on a stopped task, Paperclip saves the
request as a held wake and dispatches it right away, unless admission is
held. Admission is held during a task drain, for example while a
deployment restarts the server
> - A Retry held by a drain is released later by the deferred-receipt
worker. That worker shares one queue with every other held wake: comment
waits, execution waits, and interrupted queues
> - On a busy instance that queue holds hundreds of passive waits that
come due every 30 seconds. A Retry is checked in due-time order behind
all of them. Ending a drain does not wake the worker either
> - In a real case, a Retry clicked during a deployment drain was still
held 40 minutes later, after the drain expired and the server restarted.
It ran only when the person clicked Retry again
> - This pull request checks operator Retries ahead of passive waits,
and wakes the worker as soon as a drain stops or expires
> - The benefit is that a Retry held by a drain starts within one worker
pass after the drain ends
## Linked Issues or Issue Description
**What happened?**
A person clicked Retry on a blocked task 14 seconds after a deployment's
task drain started. The Retry was saved as `deferred_issue_execution`,
and the immediate dispatch returned early because of the drain. The
drain had a 20-minute TTL, and the deployment restarted the server about
6 minutes in. The held Retry still did not start, and its `updated_at`
never changed. Forty minutes later the person clicked Retry again, and
the new request started a run and merged with the held one.
That instance had 390 held wakes: 271 execution waits and 116 others,
against 6 task Retries. The execution waits come due every 30 seconds.
Measured on the instance, the deferred-receipt worker took about 7
minutes to get through that backlog once.
**Expected behavior**
A Retry that a drain held starts soon after the drain stops or expires,
or after the server restarts, whatever the size of the passive backlog.
**Steps to reproduce**
1. Start a non-idle task drain (`POST /instance/task-drain`).
2. Click Retry on a stopped task. The receipt is
`deferred_issue_execution`.
3. Stop the drain. On `master`, the deferred-receipt worker only notices
on its next 30-second re-check. It then checks the Retry in due-time
order behind every passive wait that came due before it.
**Paperclip version or commit**
`master` at 8148fc0b1e.
## What Changed
- `server/src/services/scoped-deadline-work.ts`:
- New optional `priority(entry)`. Due entries are sorted by priority
first, then by due time and id as before.
- New `refresh(hint)`. It clears a scope's reconstruction delay and
invalidates it. Failure backoff is kept.
- New optional `onStop` hook, called when the coordinator stops the
worker.
- `server/src/services/heartbeat/deferred-receipt-work.ts`:
- A `task_retry` receipt that a person requested is an operator Retry,
with priority 1. Passive receipts keep priority 0, so their order does
not change.
- Each deferred scan loads operator Retries first, with their own query,
then pages by id as before. A Retry is therefore in memory in the first
pass even beyond the first 100-row page. A receipt loaded twice merges
by id.
- New optional `owner.subscribeRefresh`. The work calls `refresh()` on
all three of its queues when it fires, and releases the subscription
from `onStop`, so the coordinator's own stop removes it.
- `server/src/services/heartbeat.ts`: passes `subscribeRefresh` that
listens to task-drain changes and expiry. Before this change, a deploy
drain left the worker waiting for its 30-second re-check, behind the
scan's reconstruction delay.
- `server/src/__tests__/deferred-execution-receipt-work.test.ts`, four
new tests:
- A real Retry, requested during a deploy drain, starts a run (to
`succeeded`) within 5 seconds after the drain stops.
- An operator Retry whose id sorts after 120 passive execution waits
(more than one load page) is resumed first, in the first worker pass.
- A held Retry inserted with no notification right after a full scan is
resumed as soon as the drain refresh fires.
- Stopping only the coordinator removes the drain listener.
## Verification
- `npx vitest run src/__tests__/deferred-execution-receipt-work.test.ts
-t "held by a deploy drain|ahead of a backlog|ordinary committed task
Retry"`: 3 pass. The third is the existing ordinary Retry dispatch test.
- Red checks, removing one piece at a time:
- Without the drain subscription, the drain test fails: the Retry is
still `deferred_issue_execution` with no run after 5 seconds.
- Without the priority, or without the prioritized load, the backlog
test fails: the first resumed request is the passive `00000000-…` wake.
- With `invalidate()` instead of `refresh()`, the post-scan test fails
after 5 seconds.
- Without the `onStop` release, the listener test fails.
- Full `deferred-execution-receipt-work.test.ts`: 50 pass.
- `chat-channels.integration.test.ts -t "still batches several replies
in one finding thread"`: it timed out once on CI and passes 2 of 2 times
locally with this change.
- `scoped-deadline-work`, `heartbeat-task-drain`,
`heartbeat-task-drain-admission-release`, `cloud-control-task-drain`,
`task-admission-deadline`, `onboarding-first-task-admission` and
`delivery-work-coordinator` suites: 164 pass.
- `pnpm run typecheck` in `server` (layered): clean.
## Risks
- Operator Retries are a small set; there were 6 on the instance above.
They now take the first slots of a worker pass, so passive waits can
move by at most that many places. Passive waits keep their own order.
- A Retry that keeps failing its check still gets the existing failure
backoff, so it cannot take every pass.
- A drain start also wakes the worker. That pass ends at once, because
the worker's run gate still reads the drain.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) via Claude Code, with tool use
(shell, file editing, vitest runs against embedded PostgreSQL, read-only
production log and database analysis).
## 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>
300 lines
16 KiB
TypeScript
300 lines
16 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import type { DeadlineReconciliationSnapshot } from "./deadline-reconciliation.js";
|
|
import { takeCompanyWorkBatch } from "./company-work-batch.js";
|
|
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
|
import type { DeliveryQueue } from "./delivery-work-notifications.js";
|
|
|
|
type ScheduledEntry = { id: string; companyId: string; dueAt: number };
|
|
|
|
/** Coalesced scope reconstruction and fair deadline dispatch. Domain owners
|
|
* supply persistence, eligibility, and execution; this helper owns no SQL. */
|
|
export function registerScopedDeadlineWork<Entry extends ScheduledEntry, Scope>(
|
|
coordinator: ReturnType<typeof createDeliveryWorkCoordinator>,
|
|
options: {
|
|
queue: DeliveryQueue;
|
|
batchSize?: number;
|
|
minimumPassIntervalMs?: number;
|
|
concurrentChecks?: {
|
|
limit: number;
|
|
key(entry: Entry): string | readonly string[];
|
|
// A pass otherwise ends only after its slowest check. Owners whose checks
|
|
// can legitimately run for minutes may let idle slots keep admitting
|
|
// entries that become due meanwhile, so one slow resource cannot hold
|
|
// every other resource's saved deadline until it finishes. check() must
|
|
// revalidate each entry against durable state, because an entry admitted
|
|
// this way is not reconstructed first.
|
|
admitDueWhileActive?: boolean;
|
|
};
|
|
reconstructionIntervalMs?: number | ((scope: Scope | undefined) => number);
|
|
beforePass?(): Promise<void>;
|
|
decodeScope(hint: string): Scope | undefined;
|
|
matches(entry: Entry, scope?: Scope): boolean;
|
|
load(scope?: Scope, signal?: AbortSignal): Promise<Entry[] | { rows: Entry[] | null; complete: boolean }>;
|
|
// Owners may retain pending queue priority while refreshing eligibility.
|
|
mergeEntry?(pending: Entry, loaded: Entry): Entry;
|
|
priority?(entry: Entry): number;
|
|
check(entry: Entry, signal?: AbortSignal): Promise<Entry | null>;
|
|
onStop?(): void;
|
|
},
|
|
canRun: () => boolean,
|
|
) {
|
|
if (options.concurrentChecks && (!Number.isInteger(options.concurrentChecks.limit) || options.concurrentChecks.limit < 1)) {
|
|
throw new RangeError("Concurrent check limit must be a positive integer");
|
|
}
|
|
const ownerRevision = randomUUID();
|
|
let revision = 0, running = false, stopped = false, lastRunFailed = false;
|
|
const dirty = new Set<string>(["*"]);
|
|
const versions = new Map<string, number>();
|
|
const scans = new Map<string, { version: number; seen: Set<string> }>();
|
|
const entries = new Map<string, Entry>();
|
|
const failures = new Map<string, { count: number; retryAt: number }>();
|
|
const scopeFailures = new Map<string, { count: number; retryAt: number }>();
|
|
const reconstructAfter = new Map<string, number>();
|
|
let earliest = 0;
|
|
let lastCompany: string | null = null;
|
|
function failed(map: typeof failures, id: string) {
|
|
const count = Math.min((map.get(id)?.count ?? 0) + 1, 5);
|
|
map.set(id, { count, retryAt: Date.now() + Math.min(300_000, 30_000 * 2 ** (count - 1)) });
|
|
}
|
|
function dirtyScopes() {
|
|
return [...dirty].filter(hint => hint !== "*" || !scopeFailures.has(hint))
|
|
.filter(hint => !scans.has(hint) || scopeFailures.has(hint))
|
|
.filter(hint => (reconstructAfter.get(hint) ?? 0) <= Date.now() || scopeFailures.has(hint))
|
|
.map(hint => { try { return options.decodeScope(hint); } catch { return undefined; } });
|
|
}
|
|
function next() {
|
|
if (!canRun()) return dirty.size > 0 || entries.size > 0 ? Date.now() + 30_000 : null;
|
|
let nextAt: number | null = null;
|
|
const take = (at: number) => { nextAt = nextAt === null ? at : Math.min(nextAt, at); };
|
|
for (const hint of dirty) take(Math.max(earliest, reconstructAfter.get(hint) ?? 0, scopeFailures.get(hint)?.retryAt ?? 0));
|
|
const unavailable = dirtyScopes();
|
|
for (const entry of entries.values()) if (!unavailable.some(scope => options.matches(entry, scope)))
|
|
take(Math.max(earliest, entry.dueAt, failures.get(entry.id)?.retryAt ?? 0));
|
|
return nextAt;
|
|
}
|
|
const worker = coordinator.register(options.queue, {
|
|
retryMs: 30_000,
|
|
pausePassiveMaintenance: true,
|
|
onStop: () => { stopped = true; revision++; options.onStop?.(); },
|
|
onSettled: hints => { revision++; for (const hint of hints ?? ["*"]) {
|
|
dirty.add(hint); versions.set(hint, (versions.get(hint) ?? 0) + 1);
|
|
} },
|
|
run: async signal => {
|
|
revision++; running = true;
|
|
try {
|
|
if (!canRun() || Date.now() < earliest) return;
|
|
for (const [hint, until] of reconstructAfter) {
|
|
if (until <= Date.now()) reconstructAfter.delete(hint);
|
|
}
|
|
const ready = [...dirty].filter(hint => Math.max(reconstructAfter.get(hint) ?? 0, scopeFailures.get(hint)?.retryAt ?? 0) <= Date.now());
|
|
if (ready.length === 0 && ![...entries.values()].some(entry => Math.max(entry.dueAt, failures.get(entry.id)?.retryAt ?? 0) <= Date.now())) return;
|
|
earliest = Date.now() + (options.minimumPassIntervalMs ?? 1000);
|
|
try { await options.beforePass?.(); } catch (error) {
|
|
for (const hint of ready) failed(scopeFailures, hint);
|
|
for (const entry of entries.values()) if (entry.dueAt <= Date.now()) failed(failures, entry.id);
|
|
throw error;
|
|
}
|
|
const errors: unknown[] = [];
|
|
const requested = ready.includes("*")
|
|
? ["*", ...ready.filter(hint => hint !== "*")].slice(0, 25)
|
|
: ready.slice(0, 25);
|
|
for (const hint of requested) dirty.delete(hint);
|
|
for (let index = 0; index < requested.length; index++) {
|
|
if (signal.aborted || !canRun()) { for (const hint of requested.slice(index)) dirty.add(hint); return; }
|
|
const hint = requested[index];
|
|
let scope: Scope | undefined;
|
|
let decoded = false;
|
|
try {
|
|
scope = options.decodeScope(hint); decoded = true;
|
|
const scan = scans.get(hint) ?? { version: versions.get(hint) ?? 0, seen: new Set<string>() };
|
|
scans.set(hint, scan);
|
|
const loaded = await options.load(scope, signal);
|
|
const result = Array.isArray(loaded) ? { rows: loaded, complete: true } : loaded;
|
|
for (const entry of result.rows ?? []) {
|
|
const pending = entries.get(entry.id);
|
|
entries.set(entry.id, pending && options.mergeEntry ? options.mergeEntry(pending, entry) : entry);
|
|
for (const [other, progress] of scans) if (options.matches(entry, options.decodeScope(other))) progress.seen.add(entry.id);
|
|
}
|
|
if (!result.complete) { dirty.add(hint); continue; }
|
|
if (result.rows !== null) for (const entry of entries.values()) if (options.matches(entry, scope) && !scan.seen.has(entry.id)) {
|
|
entries.delete(entry.id); failures.delete(entry.id);
|
|
}
|
|
scans.delete(hint);
|
|
const reconstructionInterval = typeof options.reconstructionIntervalMs === "function"
|
|
? options.reconstructionIntervalMs(scope) : (options.reconstructionIntervalMs ?? 0);
|
|
if (reconstructionInterval > 0) {
|
|
reconstructAfter.set(hint, Date.now() + reconstructionInterval);
|
|
}
|
|
if ((versions.get(hint) ?? 0) !== scan.version) dirty.add(hint); else versions.delete(hint);
|
|
if (!scope) scopeFailures.clear(); else scopeFailures.delete(hint);
|
|
} catch (error) {
|
|
const retryHint = decoded ? hint : "*";
|
|
if (!decoded) { scans.delete(hint); versions.delete(hint); }
|
|
dirty.add(retryHint); failed(scopeFailures, retryHint); errors.push(error);
|
|
}
|
|
}
|
|
const unavailable = dirtyScopes();
|
|
const due = [...entries.values()].filter(entry => !unavailable.some(scope => options.matches(entry, scope))
|
|
&& Math.max(entry.dueAt, failures.get(entry.id)?.retryAt ?? 0) <= Date.now())
|
|
.sort((a, b) => (options.priority?.(b) ?? 0) - (options.priority?.(a) ?? 0)
|
|
|| a.dueAt - b.dueAt || a.id.localeCompare(b.id));
|
|
const batch = takeCompanyWorkBatch(due, options.batchSize ?? 25, lastCompany); lastCompany = batch.lastCompany;
|
|
// An owner may share more than one resource (for example a task and an
|
|
// endpoint). Merge intersecting keys before dispatch so waiting siblings
|
|
// do not occupy the bounded check slots ahead of independent work.
|
|
const parents = batch.entries.map((_, index) => index);
|
|
const root = (index: number): number => {
|
|
while (parents[index] !== index) index = parents[index];
|
|
return index;
|
|
};
|
|
const owners = new Map<string, number>();
|
|
for (const [index, entry] of batch.entries.entries()) {
|
|
const configured = options.concurrentChecks?.key(entry) ?? "";
|
|
const keys = typeof configured === "string" ? [configured] : configured;
|
|
for (const key of keys) {
|
|
const previous = owners.get(key);
|
|
if (previous === undefined) owners.set(key, index);
|
|
else parents[root(index)] = root(previous);
|
|
}
|
|
}
|
|
const groups = new Map<number, Entry[]>();
|
|
for (const [index, entry] of batch.entries.entries()) {
|
|
const key = root(index);
|
|
const group = groups.get(key) ?? [];
|
|
group.push(entry); groups.set(key, group);
|
|
}
|
|
const pending = [...groups.values()];
|
|
let nextGroup = 0;
|
|
const admitDueWhileActive = options.concurrentChecks?.admitDueWhileActive === true;
|
|
const keysOf = (entry: Entry) => {
|
|
const configured = options.concurrentChecks?.key(entry) ?? "";
|
|
return typeof configured === "string" ? [configured] : [...configured];
|
|
};
|
|
// Entries owned by this pass (queued in a group or running) and the
|
|
// resource keys they hold. Every batch group reserves its keys until the
|
|
// whole group finishes, so admission can neither overlap a queued
|
|
// sibling nor jump ahead of it. Admission never overlaps either set.
|
|
const owned = new Set(batch.entries.map(entry => entry.id));
|
|
const busyKeys = new Map<string, number>();
|
|
const reserve = (keys: readonly string[]) => {
|
|
for (const key of keys) busyKeys.set(key, (busyKeys.get(key) ?? 0) + 1);
|
|
};
|
|
const release = (keys: readonly string[]) => {
|
|
for (const key of keys) {
|
|
const count = (busyKeys.get(key) ?? 1) - 1;
|
|
if (count > 0) busyKeys.set(key, count); else busyKeys.delete(key);
|
|
}
|
|
};
|
|
const groupKeys = pending.map(group => [...new Set(group.flatMap(keysOf))]);
|
|
for (const keys of groupKeys) reserve(keys);
|
|
const lastChecked = new Map<string, number>();
|
|
const spacing = options.minimumPassIntervalMs ?? 1000;
|
|
let running = 0;
|
|
let changed: (() => void) | null = null;
|
|
const notify = () => { const resolve = changed; changed = null; resolve?.(); };
|
|
const checkEntry = async (entry: Entry) => {
|
|
running += 1;
|
|
try {
|
|
const current = await options.check(entry, signal);
|
|
if (current) entries.set(entry.id, current); else entries.delete(entry.id);
|
|
failures.delete(entry.id);
|
|
} catch (error) { failed(failures, entry.id); errors.push(error); }
|
|
finally {
|
|
running -= 1;
|
|
lastChecked.set(entry.id, Date.now());
|
|
notify();
|
|
}
|
|
};
|
|
const nextAdmission = () => {
|
|
// Mid-pass settlements mark scopes dirty, but reconstruction waits for
|
|
// the next pass. Admission therefore relies on check() revalidating
|
|
// the entry against durable state, as the option requires.
|
|
const now = Date.now();
|
|
let wakeAt: number | null = null;
|
|
let candidate: Entry | null = null;
|
|
for (const item of entries.values()) {
|
|
if (owned.has(item.id)) continue;
|
|
if (keysOf(item).some(key => busyKeys.has(key))) continue;
|
|
const at = Math.max(item.dueAt, failures.get(item.id)?.retryAt ?? 0, (lastChecked.get(item.id) ?? -Infinity) + spacing);
|
|
if (at > now) { wakeAt = wakeAt === null ? at : Math.min(wakeAt, at); continue; }
|
|
if (!candidate || item.dueAt < candidate.dueAt || (item.dueAt === candidate.dueAt && item.id < candidate.id)) candidate = item;
|
|
}
|
|
return { candidate, wakeAt };
|
|
};
|
|
const waitForChange = (wakeAt: number | null) => new Promise<void>(resolve => {
|
|
const timer = wakeAt === null ? undefined : setTimeout(finish, Math.max(0, wakeAt - Date.now()));
|
|
function finish() {
|
|
if (timer !== undefined) clearTimeout(timer);
|
|
signal.removeEventListener("abort", finish);
|
|
if (changed === finish) changed = null;
|
|
resolve();
|
|
}
|
|
const previous = changed;
|
|
changed = previous ? () => { previous(); finish(); } : finish;
|
|
signal.addEventListener("abort", finish, { once: true });
|
|
});
|
|
const checkGroups = async () => {
|
|
while (!signal.aborted && canRun()) {
|
|
const index = nextGroup++;
|
|
const group = pending[index];
|
|
if (group) {
|
|
try {
|
|
for (const entry of group) {
|
|
if (signal.aborted || !canRun()) return;
|
|
try { await checkEntry(entry); } finally { owned.delete(entry.id); }
|
|
}
|
|
} finally { release(groupKeys[index]); notify(); }
|
|
continue;
|
|
}
|
|
if (!admitDueWhileActive) return;
|
|
const { candidate, wakeAt } = nextAdmission();
|
|
if (candidate) {
|
|
const keys = keysOf(candidate);
|
|
owned.add(candidate.id); reserve(keys);
|
|
try { await checkEntry(candidate); } finally { owned.delete(candidate.id); release(keys); notify(); }
|
|
continue;
|
|
}
|
|
// Nothing is admissible now. Stay available only while another
|
|
// check still holds this pass open; otherwise the pass ends and the
|
|
// coordinator schedules the next saved deadline as usual.
|
|
if (running === 0) return;
|
|
await waitForChange(wakeAt);
|
|
}
|
|
};
|
|
// With admission enabled, spare slots start too: a pass that begins with
|
|
// one slow group must still admit other resources that come due later.
|
|
const workers = admitDueWhileActive ? options.concurrentChecks!.limit
|
|
: Math.min(options.concurrentChecks?.limit ?? 1, pending.length);
|
|
const checks = await Promise.allSettled(Array.from({ length: workers }, checkGroups));
|
|
for (const result of checks) if (result.status === "rejected") errors.push(result.reason);
|
|
if (errors.length) throw new AggregateError(errors, `${options.queue} could not settle`);
|
|
lastRunFailed = false;
|
|
} catch (error) { lastRunFailed = true; throw error; }
|
|
finally { running = false; revision++; }
|
|
},
|
|
// Retain scopes/deadlines for resume without retaining a settled write's
|
|
// idle hold during drain. The coordinator still fences unresolved XIDs.
|
|
hasPending: async () => canRun() && dirty.size > 0,
|
|
nextRunAt: next,
|
|
nextRunAtOnError: next,
|
|
});
|
|
function invalidate(hint = "*") {
|
|
revision++;
|
|
dirty.add(hint); versions.set(hint, (versions.get(hint) ?? 0) + 1);
|
|
worker.wake();
|
|
}
|
|
return { ...worker,
|
|
reconciliationSnapshot: (): DeadlineReconciliationSnapshot => ({
|
|
revision: `${ownerRevision}:${revision}`,
|
|
reconciled: !stopped && !running && !lastRunFailed && dirty.size === 0 && scans.size === 0
|
|
&& failures.size === 0 && scopeFailures.size === 0,
|
|
}),
|
|
wake() { revision++; worker.wake(); },
|
|
invalidate,
|
|
refresh(hint = "*") {
|
|
reconstructAfter.delete(hint);
|
|
invalidate(hint);
|
|
},
|
|
};
|
|
}
|