mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
## Thinking Path > - Paperclip manages AI agents and their shared work. > - Skill snapshots need safe cleanup after their retention grace period. > - The retention worker scans large directories in pages. > - Another company's grace deadline can expire while a page is still running. > - The shared deadline helper then uses the worker's 30-second failure retry for successful continuation. > - This pull request gives this worker a one-second continuation delay. > - Directory scans keep moving while failure retries retain their existing delay. ## Linked Issues or Issue Description Refs #18266. Its unrelated fixture checks exposed this scheduling bug. **What happened?** A successful directory page can wait 30 seconds before the next page when another company's grace deadline becomes overdue during the scan. **Expected behavior** Continue successful paging within one second. Process the overdue company. Stop scheduling when all work settles. Keep the existing 30-second failure retry. **Steps to reproduce** Hold the second successful page with a deferred promise. Advance the fake clock past another company's saved five-second deadline. Release the page. Before this fix, the next wake is still 25 seconds away. The regression also requires all pages, the overdue rescan, and an idle worker after settlement. **Paperclip version or commit** Reproduced on `b3c2172d417ea6b783e0c796aaed4a91e1f9d381`. **Deployment mode** Core retention worker. The deterministic test uses the real coordinator, deadline helper, and retention worker with controlled service results. It needs no external database or provider. ## What Changed - Set `continuationMs: 1000` on the skill retention worker. - Add a controlled regression for a grace deadline that expires during a successful page. - Keep the shared deadline helper and the worker's failure retry unchanged. ## Verification - Red: the new regression fails on unchanged production code with a 25,000 ms remaining delay. The existing control passes. - Green: retention worker, deadline worker, and scoped deadline worker suites pass: 7 tests. - `pnpm build` and `pnpm -r typecheck` pass. All required checks pass on the current head. Greptile reports 5/5 with no review threads. - The full local test suite is not complete. The bundled macOS PostgreSQL runtime lacks `libzstd.1.dylib`, as confirmed in the existing host diagnostic. The deterministic worker tests do not need PostgreSQL. Full repository tests must pass in CI. - No live Cloud, provider, model, or deployment test is selected. ## Risks Successful overdue continuation can run sooner. The existing one-second page interval still applies. The test requires the remaining pages, the overdue rescan, and no more work after settlement. Failure retry remains 30 seconds. This change adds no schema, API, or product command. ## Model Used OpenAI `gpt-6.1-sol`, ultra reasoning, through Codex. Parent runtime metadata confirms this model. The context window is not exposed. Terminal tools assisted source review and testing. A separate agent reviewed the source and regression. ## 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>
152 lines
6.8 KiB
TypeScript
152 lines
6.8 KiB
TypeScript
import path from "node:path";
|
|
import { resolvePaperclipInstanceRoot } from "../home-paths.js";
|
|
import { logger } from "../middleware/logger.js";
|
|
import { registerDeadlineWork } from "./deadline-work.js";
|
|
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
|
import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
|
|
import { onSkillVersionCacheChange } from "./skill-version-retention-notifications.js";
|
|
import type { skillVersionRetentionService, SkillVersionPruneResult } from "./skill-version-retention.js";
|
|
|
|
type RetentionService = ReturnType<typeof skillVersionRetentionService>;
|
|
type ActiveCompany = { id: string; cursor: SkillVersionPruneResult["cursor"]; nextRunAt: number | null; rescanOwed: boolean };
|
|
|
|
export function registerSkillVersionRetentionWork(
|
|
coordinator: ReturnType<typeof createDeliveryWorkCoordinator>,
|
|
service: RetentionService,
|
|
canRun: () => boolean,
|
|
) {
|
|
const root = path.resolve(resolvePaperclipInstanceRoot(), "skills");
|
|
const dirty = new Set<string>();
|
|
const deadlines = new Map<string, number>();
|
|
const failures = new Map<string, number>();
|
|
const freshScanAfter = new Map<string, number>();
|
|
const cacheChanges = new Map<string, Map<string, number>>();
|
|
const cacheDeadline = (id: string) => {
|
|
let next = Infinity;
|
|
for (const at of cacheChanges.get(id)?.values() ?? []) next = Math.min(next, at);
|
|
return next;
|
|
};
|
|
let discoveryAfter = 0, discoveryLoading = false, discoveryAgain = false;
|
|
let discovery: { after?: string; retryAt: number } | null = { retryAt: 0 };
|
|
let active: ActiveCompany | null = null;
|
|
let continuationAt = 0;
|
|
const readyAt = (id: string) => Math.max(failures.get(id) ?? 0,
|
|
Math.min(deadlines.get(id) ?? Infinity, cacheDeadline(id)) <= Date.now() ? 0 : freshScanAfter.get(id) ?? 0);
|
|
const changed = (hints?: ReadonlySet<string>) => {
|
|
for (const hint of hints ?? ["*"]) {
|
|
if (hint === "*") {
|
|
if (!discovery) discovery = { retryAt: discoveryAfter };
|
|
else if (discoveryLoading || discovery.after !== undefined) discoveryAgain = true;
|
|
} else dirty.add(hint);
|
|
}
|
|
};
|
|
const worker = registerDeadlineWork(coordinator, DELIVERY_QUEUES.skillVersionRetention, {
|
|
canRun,
|
|
retryMs: 30_000,
|
|
continuationMs: 1000,
|
|
notificationCoalesceMs: 1000,
|
|
onSettled: changed,
|
|
run: async signal => {
|
|
const admitted = () => !signal.aborted && canRun();
|
|
for (const [id, at] of freshScanAfter) if (at <= Date.now()) freshScanAfter.delete(id);
|
|
for (const [id, at] of deadlines) if (at <= Date.now()) {
|
|
dirty.add(id);
|
|
if (active?.id === id) active.rescanOwed = true;
|
|
freshScanAfter.delete(id);
|
|
deadlines.delete(id);
|
|
}
|
|
for (const [id, entries] of cacheChanges) {
|
|
for (const [directory, at] of entries) if (at <= Date.now()) {
|
|
dirty.add(id);
|
|
if (active?.id === id) active.rescanOwed = true;
|
|
freshScanAfter.delete(id);
|
|
entries.delete(directory);
|
|
}
|
|
if (entries.size === 0) cacheChanges.delete(id);
|
|
}
|
|
let inspected = 0;
|
|
let companies = 0;
|
|
const scope = discovery;
|
|
if (scope && scope.retryAt <= Date.now() && admitted()) {
|
|
discoveryLoading = true;
|
|
try {
|
|
const rows = await service.listCompanies(scope.after);
|
|
for (const row of rows) dirty.add(row.id);
|
|
if (discovery === scope) {
|
|
if (rows.length === 51) discovery = { after: rows.at(-1)!.id, retryAt: 0 };
|
|
else {
|
|
discoveryAfter = Date.now() + 30_000;
|
|
discovery = discoveryAgain ? { retryAt: discoveryAfter } : null;
|
|
discoveryAgain = false;
|
|
}
|
|
}
|
|
} catch (err) {
|
|
if (discovery === scope) scope.retryAt = Date.now() + 30_000;
|
|
logger.warn({ err }, "skill cache retention company discovery failed");
|
|
} finally { discoveryLoading = false; }
|
|
}
|
|
while (companies < 50 && inspected < 50 && admitted()) {
|
|
if (!active) {
|
|
const id = [...dirty].find(id => readyAt(id) <= Date.now());
|
|
if (!id) break;
|
|
dirty.delete(id);
|
|
active = { id, cursor: null, nextRunAt: null, rescanOwed: false };
|
|
deadlines.delete(id);
|
|
cacheChanges.delete(id);
|
|
}
|
|
const company = active;
|
|
try {
|
|
const result = await service.pruneCompany(company.id, {
|
|
cursor: company.cursor, maxEntries: 50 - inspected, shouldContinue: admitted,
|
|
});
|
|
if (result.checked > 0 || result.failed > 0 || Object.keys(result.skipped).length > 0) {
|
|
logger.info({ ...result, companyId: company.id, dryRun: service.dryRun }, "skill version snapshot retention");
|
|
}
|
|
inspected += result.checked;
|
|
companies++;
|
|
if (result.nextRunAt !== null) company.nextRunAt = Math.min(company.nextRunAt ?? Infinity, result.nextRunAt);
|
|
company.cursor = result.cursor;
|
|
failures.delete(company.id);
|
|
if (result.interrupted || !admitted()) {
|
|
company.cursor = null;
|
|
company.nextRunAt = null;
|
|
continuationAt = Date.now() + 1000;
|
|
return;
|
|
}
|
|
if (result.truncated) break;
|
|
if (company.rescanOwed) freshScanAfter.delete(company.id);
|
|
else freshScanAfter.set(company.id, Date.now() + 30_000);
|
|
if (company.nextRunAt !== null) deadlines.set(company.id, company.nextRunAt);
|
|
active = null;
|
|
} catch (err) {
|
|
dirty.add(company.id);
|
|
failures.set(company.id, Date.now() + 30_000);
|
|
active = null;
|
|
companies++;
|
|
logger.warn({ err, companyId: company.id }, "skill cache retention company inspection failed");
|
|
}
|
|
}
|
|
continuationAt = Date.now() + 1000;
|
|
},
|
|
next: async () => {
|
|
let next: number | null = null;
|
|
const take = (at: number) => { next = Math.min(next ?? Infinity, at); };
|
|
if (discovery) take(Math.max(continuationAt, discovery.retryAt));
|
|
if (active) take(continuationAt);
|
|
for (const id of dirty) take(Math.max(continuationAt, readyAt(id)));
|
|
for (const [id, at] of deadlines) take(Math.max(at, failures.get(id) ?? 0));
|
|
for (const id of cacheChanges.keys()) take(Math.max(cacheDeadline(id), failures.get(id) ?? 0));
|
|
return next;
|
|
},
|
|
});
|
|
const unsubscribe = onSkillVersionCacheChange(change => {
|
|
if (change.root !== root) return;
|
|
let entries = cacheChanges.get(change.companyId);
|
|
if (!entries) { entries = new Map(); cacheChanges.set(change.companyId, entries); }
|
|
entries.set(change.directory, change.occurredAt + service.graceMs);
|
|
if (change.source === "publication") dirty.add(change.companyId);
|
|
worker.wake();
|
|
});
|
|
return { ...worker, unsubscribe, dispose: () => { unsubscribe(); cacheChanges.clear(); return service.dispose(); } };
|
|
}
|