mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
## Thinking Path > - Paperclip manages work and human decisions for AI-agent companies. > - Automatic decision queues store membership for pending attention sources. > - The old attention maintenance scan created that membership as a side effect. > - Moving approval retention out of the scan also removed approval queue discovery. > - Queue API consumers do not build the attention feed first, so they can miss pending PR approvals. > - This pull request gives approval queue discovery its own worker and commit signals. > - The worker becomes quiet after it saves every missing membership. ## Linked Issues or Issue Description Refs #17497. Stacked on #17508, following #17505. This is a correctness dependency for the approval retention migration in #17497. Do not treat that migration as complete without this queue-population path. A real PostgreSQL regression found no PR queue when a pending approval had a linked PR work product and the maintenance feed excluded approvals. Related: #13364 proposes pruning stale seeded memberships. This PR preserves existing membership retention and does not implement pruning. #15149 documents the decision-queue APIs. ## What Changed - Add an approval queue worker that selects missing PR-queue memberships from existing durable records. Bound each pass to 100 rows. Continue past failed rows and revisit them on retry. - Reuse the queue service's seed writer through a normalized source contract. Keep queue rules, uniqueness, and audit writes in that service. Keep feed readers compatible. - Wake from committed approval and link changes, issue/company visibility, PR work-product changes and imports, and queue-rule changes. Reuse the existing transaction boundaries. - Recheck approval status under a row lock. Serialize queue-rule changes with the seed writer. Preserve required recovery during drain and shutdown. - Register startup reconstruction under the existing maintenance switch. Leave other attention-source seeding with its current owner until its own migration. - Document the complete producer audit and stack dependency. Add a validated external PostgreSQL option to the existing queue route suite so it runs on hosts without embedded database support. ## Verification Current tested revision: `7948136ef8`. This PR targets master after #17508 merged as `8406047b4e`. Normal ancestry reconciliation preserved the complete tested integration tree, before the review repairs below. Only the original 25 seeding paths differ from master, and database schema/migration metadata is identical. - **56 cases pass in the three complete affected suites:** 28 approval seeding, 6 queue routes and 22 task-watchdog cases. Server typecheck and production build pass. - Three real PostgreSQL retry regressions failed then passed: new matching arrivals cannot postpone a recovered failure indefinitely; successful partial progress resets old failure penalties; an empty cursor rollover does not reset outage backoff. Exact one- and two-second retry assertions, eventual complete recovery and existing capped-backoff/company isolation tests pass. - Three genuinely empty external databases failed before setup repair because tables were missing. Each suite now applies migrations itself. All 55 cases passed on a second set of fresh databases; adding the final rollover guard then passed all 56 cases using those now-migrated fixtures. No production migration or table is added. - The initial expanded fresh run passed 54 cases but failed a cleanup expectation that omitted a required cursor rollover pass. The fixture now permits that explicit continuation while preserving exact deadline and complete-membership assertions. All failure records remain in `doc/idle-approval-queue-seeding.md`. - The prior integrated revision `4e725e2055` passed **522 cases in fifteen complete producer/consumer suites**, plus types/build. Its unchanged consumer code and historical deadlock, cross-company amplification, backoff, gateway integration and setup evidence remain documented; those counts are not relabeled as current acceptance. Commands: server Vitest for approval queue seeding, queue routes and task watchdogs; server typecheck/build. Fresh exact-head Greptile/Ponytail reviews and all CI checks remain required. No full local repository-suite, deployed suspension or cost claim is made. ## Risks - Signals cover the supported in-process service writers. Independent SQL writers and multiple server replicas need a separate notification contract. - The source predicate must stay aligned with unscoped attention maintenance. The first linked issue, company scope, pending status, and execution visibility are tested. A pending approval itself keeps its linked external conversation active. - Existing enabled seed rules can re-add manually removed matching memberships. This preserves the old scan's behavior. Completed or archived membership is not pruned. - Queue seeding and retention are separate owners. Other attention sources still need their own event-driven discovery before the aggregate scan can be retired. - This PR does not extend Cloud wake coverage, relax sleep admission, or claim measured host/database suspension. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The exact serving model identifier and context window were not exposed. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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>
107 lines
5.3 KiB
TypeScript
107 lines
5.3 KiB
TypeScript
import { and, asc, desc, eq, gt, lte, sql, type SQL } from "drizzle-orm";
|
|
import { approvals, companies, issues, issueWorkProducts, type Db } from "@paperclipai/db";
|
|
import { firstApprovalIssueId } from "./approval-visibility.js";
|
|
import { executionIssueCondition } from "./issue-visibility.js";
|
|
import { pullRequestSeedProductCondition } from "./decision-queue-seed-rules.js";
|
|
import { decisionQueueService } from "./decision-queues.js";
|
|
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
|
import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
|
|
|
|
const linkedIssue = firstApprovalIssueId();
|
|
|
|
export function approvalQueueSeedingService(db: Db) {
|
|
const pending = new Map<string | undefined, { cursor?: string; end?: string; generation: number; failures: number; retryAt: number }>([[undefined, { generation: 0, failures: 0, retryAt: 0 }]]);
|
|
function request(companyIds: ReadonlySet<string> | undefined) {
|
|
for (const companyId of companyIds ?? []) {
|
|
const existing = pending.get(companyId);
|
|
if (existing) existing.generation++;
|
|
else pending.set(companyId, { generation: 0, failures: 0, retryAt: 0 });
|
|
}
|
|
}
|
|
function candidates(database: Db, companyId: string | undefined, condition?: SQL, order = asc) {
|
|
return database.select({ id: approvals.id, companyId: approvals.companyId, issueId: linkedIssue }).from(approvals)
|
|
.innerJoin(companies, eq(companies.id, approvals.companyId))
|
|
.where(and(eq(approvals.status, "pending"), eq(companies.status, "active"), companyId === undefined ? undefined : eq(approvals.companyId, companyId), condition,
|
|
sql`exists (select 1 from ${issues} where ${issues.id} = ${linkedIssue}
|
|
and ${issues.companyId} = ${approvals.companyId} and ${executionIssueCondition()}
|
|
and exists (select 1 from ${issueWorkProducts} where ${pullRequestSeedProductCondition(approvals.companyId)}
|
|
and ${issueWorkProducts.issueId} = ${issues.id}))`,
|
|
sql`not exists (select 1 from decision_queues queue where queue.company_id = ${approvals.companyId} and queue.key = 'prs'
|
|
and (not queue.seed_rules_enabled or exists (select 1 from decision_queue_items item where item.queue_id = queue.id
|
|
and item.company_id = ${approvals.companyId} and item.source_kind = 'approval' and item.source_id = ${approvals.id}::text)))`,
|
|
)).orderBy(order(approvals.id));
|
|
}
|
|
async function sweep(signal?: AbortSignal) {
|
|
const scope = [...pending.entries()].find(([, state]) => state.retryAt <= Date.now());
|
|
if (!scope) return;
|
|
const [companyId, state] = scope;
|
|
try {
|
|
const generation = state.generation;
|
|
state.end ??= (await candidates(db, companyId, undefined, desc).limit(1))[0]?.id;
|
|
const rows = state.end ? await candidates(db, companyId, and(lte(approvals.id, state.end),
|
|
state.cursor ? gt(approvals.id, state.cursor) : undefined)).limit(100) : [];
|
|
const errors: unknown[] = [];
|
|
for (const row of rows) {
|
|
if (signal?.aborted) break;
|
|
try {
|
|
await db.transaction(async tx => {
|
|
const database = tx as unknown as Db;
|
|
const [current] = await candidates(database, row.companyId, eq(approvals.id, row.id)).for("update", { of: approvals });
|
|
if (!current) return;
|
|
await decisionQueueService(database).materializeSeedSources(current.companyId, [{
|
|
sourceKind: "approval", sourceId: current.id, issueId: current.issueId,
|
|
}]);
|
|
});
|
|
} catch (error) { errors.push(error); }
|
|
state.cursor = row.id;
|
|
}
|
|
if (rows.length < 100) { state.cursor = undefined; state.end = undefined; }
|
|
const remaining = await candidates(db, companyId).limit(1);
|
|
if (!signal?.aborted && generation === state.generation && remaining.length === 0) pending.delete(companyId);
|
|
if (errors.length === 1) throw errors[0];
|
|
if (errors.length) throw new AggregateError(errors, "Approval queue seeding failed");
|
|
if (rows.length > 0 || remaining.length === 0) state.failures = 0;
|
|
state.retryAt = 0;
|
|
} catch (error) {
|
|
state.failures = Math.min(state.failures + 1, 7);
|
|
state.retryAt = Date.now() + Math.min(60_000, 1_000 * 2 ** (state.failures - 1));
|
|
throw error;
|
|
} finally {
|
|
if (pending.has(companyId)) {
|
|
pending.delete(companyId);
|
|
pending.set(companyId, state);
|
|
}
|
|
}
|
|
}
|
|
function next() {
|
|
let earliest: number | null = null;
|
|
for (const state of pending.values()) earliest = earliest === null ? state.retryAt : Math.min(earliest, state.retryAt);
|
|
return earliest;
|
|
}
|
|
return { sweep, next, request };
|
|
}
|
|
|
|
export function registerApprovalQueueSeedingWork(
|
|
coordinator: ReturnType<typeof createDeliveryWorkCoordinator>,
|
|
service: ReturnType<typeof approvalQueueSeedingService>,
|
|
canRun: () => boolean,
|
|
) {
|
|
let earliest = 0;
|
|
return coordinator.register(DELIVERY_QUEUES.approvalQueueSeeding, {
|
|
retryMs: 1_000,
|
|
pausePassiveMaintenance: true,
|
|
onSettled: service.request,
|
|
run: async signal => {
|
|
if (!canRun()) return;
|
|
if (Date.now() < earliest) return;
|
|
earliest = Date.now() + 1_000;
|
|
await service.sweep(signal);
|
|
},
|
|
hasPending: async () => (service.next() ?? Infinity) <= Date.now(),
|
|
nextRunAt: () => {
|
|
const next = service.next();
|
|
return next === null ? null : Math.max(earliest, next);
|
|
},
|
|
});
|
|
}
|