Files
PaperClipAI/server/src/services/approvals.ts
T
DottaandPaperclip 4e27a82a42 fix(server): wake approval queue seeding from committed changes (#17510)
## 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>
2026-10-27 20:33:01 -05:00

206 lines
8.3 KiB
TypeScript

import { notifyToolReviewChange } from "./tool-review-work-notifications.js";
import { writeApprovalChange } from "./approval-work-notifications.js";
import { approvalResolutionRecords, resolvableApprovalStatuses as resolvableStatuses } from "../lib/approval-records.js";
import { notifyHireApproved } from "./hire-hook.js";
import { and, asc, eq, inArray, sql, type SQL } from "drizzle-orm";
import type { Db } from "@paperclipai/db";
import { approvalComments, approvals } from "@paperclipai/db";
import { unprocessable } from "../errors.js";
import { redactCurrentUserText } from "../log-redaction.js";
import { createAgentLifecycle } from "./agent-lifecycle.js";
import { instanceSettingsService } from "./instance-settings.js";
export function approvalRecords(db: Db) {
const instanceSettings = instanceSettingsService(db);
const { getExistingApproval, resolveApproval } = approvalResolutionRecords(db);
function redactApprovalComment<T extends { body: string }>(comment: T, censorUsernameInLogs: boolean): T {
return {
...comment,
body: redactCurrentUserText(comment.body, { enabled: censorUsernameInLogs }),
};
}
return {
getExistingApproval, resolveApproval,
list: (companyId: string, status?: string, readCondition?: SQL<boolean>) => {
const conditions = [eq(approvals.companyId, companyId)];
if (status) conditions.push(eq(approvals.status, status));
if (readCondition) conditions.push(readCondition);
return db.select().from(approvals).where(and(...conditions));
},
getById: (id: string) =>
db
.select()
.from(approvals)
.where(eq(approvals.id, id))
.then((rows) => rows[0] ?? null),
findOpenHireApprovalForAgent: async (companyId: string, agentId: string) => {
const rows = await db
.select()
.from(approvals)
.where(
and(
eq(approvals.companyId, companyId),
eq(approvals.type, "hire_agent"),
inArray(approvals.status, resolvableStatuses),
sql`${approvals.payload} ->> 'agentId' = ${agentId}`,
),
);
return rows[0] ?? null;
},
create: (companyId: string, data: Omit<typeof approvals.$inferInsert, "companyId">) =>
writeApprovalChange(db, companyId, tx => tx
.insert(approvals)
.values({ ...data, companyId })
.returning()
.then((rows) => rows[0])),
// Cancel an open (pending/revision_requested) approval without a board
// decision — e.g. when its paired agent is terminated during duplicate
// cleanup. Idempotent: a no-op on already-resolved approvals.
cancel: async (id: string, reason?: string | null) => {
const now = new Date();
const [existing] = await db.select({ companyId: approvals.companyId }).from(approvals).where(eq(approvals.id, id));
if (!existing) return null;
const updated = await writeApprovalChange(db, existing.companyId, async tx => {
await notifyToolReviewChange(tx, existing.companyId);
return tx.update(approvals)
.set({
status: "cancelled",
decisionNote: reason ?? null,
decidedAt: now,
updatedAt: now,
})
.where(and(eq(approvals.id, id), inArray(approvals.status, resolvableStatuses)))
.returning()
.then((rows) => rows[0] ?? null);
});
return updated;
},
requestRevision: async (id: string, decidedByUserId: string, decisionNote?: string | null) => {
const existing = await getExistingApproval(id);
if (existing.status !== "pending") {
throw unprocessable("Only pending approvals can request revision");
}
const now = new Date();
return writeApprovalChange(db, existing.companyId, tx => tx
.update(approvals)
.set({
status: "revision_requested",
decidedByUserId,
decisionNote: decisionNote ?? null,
decidedAt: now,
updatedAt: now,
})
.where(eq(approvals.id, id))
.returning()
.then((rows) => rows[0]));
},
resubmit: async (id: string, payload?: Record<string, unknown>) => {
const existing = await getExistingApproval(id);
if (existing.status !== "revision_requested") {
throw unprocessable("Only revision requested approvals can be resubmitted");
}
const now = new Date();
return writeApprovalChange(db, existing.companyId, tx => tx
.update(approvals)
.set({
status: "pending",
payload: payload ?? existing.payload,
decisionNote: null,
decidedByUserId: null,
decidedAt: null,
updatedAt: now,
})
.where(eq(approvals.id, id))
.returning()
.then((rows) => rows[0]));
},
listComments: async (approvalId: string) => {
const existing = await getExistingApproval(approvalId);
const { censorUsernameInLogs } = await instanceSettings.getGeneral();
return db
.select()
.from(approvalComments)
.where(
and(
eq(approvalComments.approvalId, approvalId),
eq(approvalComments.companyId, existing.companyId),
),
)
.orderBy(asc(approvalComments.createdAt))
.then((comments) => comments.map((comment) => redactApprovalComment(comment, censorUsernameInLogs)));
},
addComment: async (
approvalId: string,
body: string,
actor: { agentId?: string; userId?: string },
) => {
const existing = await getExistingApproval(approvalId);
const currentUserRedactionOptions = {
enabled: (await instanceSettings.getGeneral()).censorUsernameInLogs,
};
const redactedBody = redactCurrentUserText(body, currentUserRedactionOptions);
return db
.insert(approvalComments)
.values({
companyId: existing.companyId,
approvalId,
authorAgentId: actor.agentId ?? null,
authorUserId: actor.userId ?? null,
body: redactedBody,
})
.returning()
.then((rows) => redactApprovalComment(rows[0], currentUserRedactionOptions.enabled));
},
};
}
export function approvalService(db: Db) {
const { getExistingApproval, resolveApproval, ...records } = approvalRecords(db);
async function reconcileApprovedBuiltInAgent(companyId: string, payload: Record<string, unknown>, database: Db) {
const sourceBuiltInAgentKey = typeof payload.sourceBuiltInAgentKey === "string" ? payload.sourceBuiltInAgentKey : null;
if (!sourceBuiltInAgentKey) return;
const { builtInAgentService } = await import("./built-in-agents.js");
await builtInAgentService(database).ensure(companyId, sourceBuiltInAgentKey);
}
async function decideHire(target: string | { approvalId: string }, status: "approved" | "rejected", userId: string, note?: string | null) {
const lifecycle = createAgentLifecycle(db);
const result = await (status === "approved" ? lifecycle.approveHire : lifecycle.rejectHire)(target, userId, note);
if (result?.hireApprovedAgentId && result.approval) {
await reconcileApprovedBuiltInAgent(result.approval.companyId, result.approval.payload, db);
void notifyHireApproved(db, { companyId: result.approval.companyId, agentId: result.hireApprovedAgentId,
source: "approval", sourceId: result.approval.id, approvedAt: result.approval.decidedAt ?? new Date() }).catch(() => {});
}
return result;
}
async function decide(id: string, status: "approved" | "rejected", userId: string, note?: string | null) {
const approval = await getExistingApproval(id);
if (approval.type === "hire_agent") {
const result = await decideHire({ approvalId: id }, status, userId, note);
return { approval: result!.approval!, applied: result!.applied };
}
return resolveApproval(id, status, userId, note);
}
return { ...records,
approveHire: (agentId: string, userId: string, note?: string | null) => decideHire(agentId, "approved", userId, note),
rejectHire: (agentId: string, userId: string, note?: string | null) => decideHire(agentId, "rejected", userId, note),
approve: (id: string, userId: string, note?: string | null) => decide(id, "approved", userId, note),
reject: (id: string, userId: string, note?: string | null) => decide(id, "rejected", userId, note),
};
}