[codex] prevent invalid agents from receiving assignments and runs (#7663)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The control plane owns agent lifecycle, issue assignment, routine dispatch, heartbeat wakeups, and recovery paths > - Terminated, paused, pending-approval, or otherwise invalid agents should not receive new work or new execution attempts > - The old behavior left eligibility checks spread across routes and services, so assignment and run paths could drift apart > - This pull request centralizes agent lifecycle eligibility and applies it consistently to assignment, invocation, routines, recovery, and UI affordances > - The benefit is safer autonomy: terminated agents stay paused, invalid org-chain agents are surfaced, and active agents keep receiving valid work ## Linked Issues or Issue Description Refs #5103 Related: #1864 Bug fix context: - What happened: agent assignment and heartbeat/run paths did not share one eligibility contract, so invalid lifecycle states could still be considered in some paths. - Expected behavior: terminated agents must never receive new assignments or heartbeat runs, and paused or otherwise invalid agents should be treated as non-invokable consistently. - Steps to reproduce: create or select an agent in an invalid lifecycle state, then attempt assignment, routine dispatch, or heartbeat/recovery wake paths. - Paperclip version/commit: fixed on top of `paperclipai/paperclip` `master` at the PR base. - Deployment mode: applies to the server control plane in local and authenticated deployments. ## What Changed - Added shared agent lifecycle eligibility helpers and exported the related shared types. - Centralized server-side assignability and invokability checks for issue assignment, agent routes, heartbeat dispatch, routines, recovery, and liveness logic. - Hardened issue assignment so invalid assignees are rejected instead of queued for work. - Hardened heartbeat/routine/recovery paths so terminated and otherwise invalid agents are not woken for new runs. - Updated board UI affordances to disable invalid agent actions and surface org-chain warnings where relevant. - Added targeted shared, server, and UI tests for the new eligibility behavior. ## Verification - `pnpm exec vitest run packages/shared/src/agent-eligibility.test.ts server/src/__tests__/agent-invokability.test.ts server/src/__tests__/heartbeat-archived-company-guard.test.ts server/src/__tests__/issue-liveness.test.ts server/src/__tests__/issues-service.test.ts server/src/__tests__/routines-service.test.ts ui/src/lib/company-members.test.ts ui/src/pages/Agents.test.tsx` — 8 files, 144 tests passed. - `pnpm --filter @paperclipai/shared typecheck && pnpm --filter @paperclipai/server typecheck && pnpm --filter @paperclipai/ui typecheck` — passed. - Checked the PR diff does not include `pnpm-lock.yaml` or `.github/workflows` changes. - Checked `ROADMAP.md`; this is a targeted control-plane safety fix and does not duplicate a planned core feature. - Searched GitHub for duplicate or related PRs/issues; closest related items are linked above. - CI and Greptile verification are pending on the opened PR and will be followed up before requesting merge. ## Risks Low to moderate risk. The intended behavioral shift is that invalid agents are refused earlier and more consistently, which could expose existing data with paused, pending, terminated, or broken org-chain assignees. The added tests cover the critical assignment, heartbeat, routine, recovery, shared helper, and UI paths. No database migrations are included. > 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 GPT-5 Codex via the Paperclip `codex_local` adapter, with shell/git/GitHub CLI tool use. Reasoning mode and context window are managed by the adapter runtime and not exposed in this environment. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (not applicable: no design screenshots requested; UI behavior is covered by tests) - [x] I have updated relevant documentation to reflect my changes (not applicable: no user-facing command or schema docs changed) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending Greptile) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
@@ -16,7 +16,13 @@ import {
|
||||
issues,
|
||||
issueComments,
|
||||
} from "@paperclipai/db";
|
||||
import { AGENT_DEFAULT_MAX_CONCURRENT_RUNS, isUuidLike, normalizeAgentUrlKey } from "@paperclipai/shared";
|
||||
import {
|
||||
AGENT_DEFAULT_MAX_CONCURRENT_RUNS,
|
||||
getAgentWorkEligibility,
|
||||
isUuidLike,
|
||||
normalizeAgentUrlKey,
|
||||
type AgentEligibilityAgent,
|
||||
} from "@paperclipai/shared";
|
||||
import { conflict, notFound, unprocessable } from "../errors.js";
|
||||
import { normalizeAgentPermissions } from "./agent-permissions.js";
|
||||
import { REDACTED_EVENT_VALUE, sanitizeRecord } from "../redaction.js";
|
||||
@@ -228,13 +234,45 @@ export function agentService(db: Db) {
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeAgentRow(row: typeof agents.$inferSelect) {
|
||||
function normalizeAgentBaseRow(row: typeof agents.$inferSelect) {
|
||||
return withUrlKey({
|
||||
...row,
|
||||
permissions: normalizeAgentPermissions(row.permissions, row.role),
|
||||
});
|
||||
}
|
||||
|
||||
function toEligibilityAgent(row: Pick<typeof agents.$inferSelect, "id" | "companyId" | "name" | "status" | "reportsTo">): AgentEligibilityAgent {
|
||||
return {
|
||||
id: row.id,
|
||||
companyId: row.companyId,
|
||||
name: row.name,
|
||||
status: row.status,
|
||||
reportsTo: row.reportsTo,
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeAgentRows(rows: (typeof agents.$inferSelect)[], allCompanyRows = rows) {
|
||||
const eligibilityAgents = allCompanyRows.map(toEligibilityAgent);
|
||||
return rows.map((row) => {
|
||||
const base = normalizeAgentBaseRow(row);
|
||||
return {
|
||||
...base,
|
||||
orgChainHealth: getAgentWorkEligibility({
|
||||
agent: toEligibilityAgent(row),
|
||||
agents: eligibilityAgents,
|
||||
}).orgChainHealth,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
function normalizeAgentRow(row: typeof agents.$inferSelect, allCompanyRows?: (typeof agents.$inferSelect)[]) {
|
||||
return normalizeAgentRows([row], allCompanyRows)[0]!;
|
||||
}
|
||||
|
||||
async function listCompanyAgentRows(companyId: string) {
|
||||
return db.select().from(agents).where(eq(agents.companyId, companyId));
|
||||
}
|
||||
|
||||
async function getMonthlySpendByAgentIds(companyId: string, agentIds: string[]) {
|
||||
if (agentIds.length === 0) return new Map<string, number>();
|
||||
const { start, end } = currentUtcMonthWindow();
|
||||
@@ -274,8 +312,17 @@ export function agentService(db: Db) {
|
||||
.where(eq(agents.id, id))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!row) return null;
|
||||
const [hydrated] = await hydrateAgentSpend([row]);
|
||||
return normalizeAgentRow(hydrated);
|
||||
const [companyRows, hydrated] = await Promise.all([
|
||||
listCompanyAgentRows(row.companyId),
|
||||
hydrateAgentSpend([row]).then((rows) => rows[0]!),
|
||||
]);
|
||||
return normalizeAgentRow(hydrated, companyRows);
|
||||
}
|
||||
|
||||
async function requireGetById(id: string) {
|
||||
const agent = await getById(id);
|
||||
if (!agent) throw notFound("Agent not found");
|
||||
return agent;
|
||||
}
|
||||
|
||||
async function ensureManager(companyId: string, managerId: string) {
|
||||
@@ -374,7 +421,7 @@ export function agentService(db: Db) {
|
||||
.where(eq(agents.id, id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
const normalizedUpdated = updated ? normalizeAgentRow(updated) : null;
|
||||
const normalizedUpdated = updated ? await getById(updated.id) : null;
|
||||
|
||||
if (normalizedUpdated && shouldRecordRevision && beforeConfig) {
|
||||
const afterConfig = buildConfigSnapshot(normalizedUpdated);
|
||||
@@ -403,9 +450,12 @@ export function agentService(db: Db) {
|
||||
if (!options?.includeTerminated) {
|
||||
conditions.push(ne(agents.status, "terminated"));
|
||||
}
|
||||
const rows = await db.select().from(agents).where(and(...conditions));
|
||||
const [rows, allCompanyRows] = await Promise.all([
|
||||
db.select().from(agents).where(and(...conditions)),
|
||||
listCompanyAgentRows(companyId),
|
||||
]);
|
||||
const hydrated = await hydrateAgentSpend(rows);
|
||||
return hydrated.map(normalizeAgentRow);
|
||||
return normalizeAgentRows(hydrated, allCompanyRows);
|
||||
},
|
||||
|
||||
getById,
|
||||
@@ -430,7 +480,7 @@ export function agentService(db: Db) {
|
||||
.returning()
|
||||
.then((rows) => rows[0]);
|
||||
|
||||
return normalizeAgentRow(created);
|
||||
return requireGetById(created.id);
|
||||
},
|
||||
|
||||
update: updateAgent,
|
||||
@@ -451,7 +501,7 @@ export function agentService(db: Db) {
|
||||
.where(eq(agents.id, id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
return updated ? normalizeAgentRow(updated) : null;
|
||||
return updated ? getById(updated.id) : null;
|
||||
},
|
||||
|
||||
resume: async (id: string) => {
|
||||
@@ -473,7 +523,7 @@ export function agentService(db: Db) {
|
||||
.where(eq(agents.id, id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
return updated ? normalizeAgentRow(updated) : null;
|
||||
return updated ? getById(updated.id) : null;
|
||||
},
|
||||
|
||||
terminate: async (id: string) => {
|
||||
@@ -540,7 +590,7 @@ export function agentService(db: Db) {
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
if (updated) {
|
||||
return { agent: normalizeAgentRow(updated), activated: true };
|
||||
return { agent: await requireGetById(updated.id), activated: true };
|
||||
}
|
||||
|
||||
const existing = await getById(id);
|
||||
@@ -561,7 +611,7 @@ export function agentService(db: Db) {
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
return updated ? normalizeAgentRow(updated) : null;
|
||||
return updated ? getById(updated.id) : null;
|
||||
},
|
||||
|
||||
listConfigRevisions: async (id: string) =>
|
||||
@@ -670,14 +720,12 @@ export function agentService(db: Db) {
|
||||
},
|
||||
|
||||
orgForCompany: async (companyId: string) => {
|
||||
const rows = await db
|
||||
.select()
|
||||
.from(agents)
|
||||
.where(and(eq(agents.companyId, companyId), ne(agents.status, "terminated")));
|
||||
const normalizedRows = rows.map(normalizeAgentRow);
|
||||
const allCompanyRows = await listCompanyAgentRows(companyId);
|
||||
const rows = allCompanyRows.filter((row) => row.status !== "terminated");
|
||||
const normalizedRows = normalizeAgentRows(rows, allCompanyRows);
|
||||
const byManager = new Map<string | null, typeof normalizedRows>();
|
||||
for (const row of normalizedRows) {
|
||||
const key = row.reportsTo ?? null;
|
||||
const key = row.reportsTo && rows.some((candidate) => candidate.id === row.reportsTo) ? row.reportsTo : null;
|
||||
const group = byManager.get(key) ?? [];
|
||||
group.push(row);
|
||||
byManager.set(key, group);
|
||||
@@ -735,8 +783,7 @@ export function agentService(db: Db) {
|
||||
}
|
||||
|
||||
const rows = await db.select().from(agents).where(eq(agents.companyId, companyId));
|
||||
const matches = rows
|
||||
.map(normalizeAgentRow)
|
||||
const matches = normalizeAgentRows(rows, rows)
|
||||
.filter((agent) => agent.urlKey === urlKey && agent.status !== "terminated");
|
||||
if (matches.length === 1) {
|
||||
return { agent: matches[0] ?? null, ambiguous: false } as const;
|
||||
|
||||
Reference in New Issue
Block a user