[codex] Improve agent runtime recovery and governance (#4086)

## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies.
> - The heartbeat runtime, agent import path, and agent configuration
defaults determine whether work is dispatched safely and predictably.
> - Several accumulated fixes all touched agent execution recovery, wake
routing, import behavior, and runtime concurrency defaults.
> - Those changes need to land together so the heartbeat service and
agent creation defaults stay internally consistent.
> - This pull request groups the runtime/governance changes from the
split branch into one standalone branch.
> - The benefit is safer recovery for stranded runs, bounded high-volume
reads, imported-agent approval correctness, skill-template support, and
a clearer default concurrency policy.

## What Changed

- Fixed stranded continuation recovery so successful automatic retries
are requeued instead of incorrectly blocking the issue.
- Bounded high-volume issue/log reads across issue, heartbeat, agent,
project, and workspace paths.
- Fixed imported-agent approval and instruction-path permission
handling.
- Quarantined seeded worktree execution state during worktree
provisioning.
- Queued approval follow-up wakes and hardened SQL_ASCII heartbeat
output handling.
- Added reusable agent instruction templates for hiring flows.
- Set the default max concurrent agent runs to five and updated related
UI/tests/docs.

## Verification

- `pnpm install --frozen-lockfile`
- `pnpm exec vitest run server/src/__tests__/company-portability.test.ts
server/src/__tests__/heartbeat-process-recovery.test.ts
server/src/__tests__/heartbeat-comment-wake-batching.test.ts
server/src/__tests__/heartbeat-list.test.ts
server/src/__tests__/issues-service.test.ts
server/src/__tests__/agent-permissions-routes.test.ts
packages/adapter-utils/src/server-utils.test.ts
ui/src/lib/new-agent-runtime-config.test.ts`
- Split integration check: merged this branch first, followed by the
other [PAP-1614](/PAP/issues/PAP-1614) branches, with no merge
conflicts.
- Confirmed this branch does not include `pnpm-lock.yaml`.

## Risks

- Medium risk: touches heartbeat recovery, queueing, and issue list
bounds in central runtime paths.
- Imported-agent and concurrency default behavior changes may affect
existing automation that assumes one-at-a-time default runs.
- 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 Codex, GPT-5.4 tool-enabled coding model, agentic
code-editing/runtime with local shell and GitHub CLI access; exact
context window and reasoning mode are not exposed by the Paperclip
harness.

## 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 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
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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 2026-04-20 06:19:48 -05:00 committed by GitHub
parent 057fee4836
commit 16b2b84d84
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
38 changed files with 1569 additions and 240 deletions

View file

@ -34,6 +34,7 @@ const mockAgentService = vi.hoisted(() => ({
getById: vi.fn(),
list: vi.fn(),
create: vi.fn(),
activatePendingApproval: vi.fn(),
updatePermissions: vi.fn(),
getChainOfCommand: vi.fn(),
resolveByReference: vi.fn(),
@ -108,6 +109,7 @@ function registerModuleMocks() {
companySkillService: () => mockCompanySkillService,
budgetService: () => mockBudgetService,
heartbeatService: () => mockHeartbeatService,
ISSUE_LIST_DEFAULT_LIMIT: 500,
issueApprovalService: () => mockIssueApprovalService,
issueService: () => mockIssueService,
logActivity: mockLogActivity,
@ -166,6 +168,7 @@ describe("agent permission routes", () => {
mockAgentService.getChainOfCommand.mockResolvedValue([]);
mockAgentService.resolveByReference.mockResolvedValue({ ambiguous: false, agent: baseAgent });
mockAgentService.create.mockResolvedValue(baseAgent);
mockAgentService.activatePendingApproval.mockResolvedValue(baseAgent);
mockAgentService.updatePermissions.mockResolvedValue(baseAgent);
mockAccessService.getMembership.mockResolvedValue({
id: "membership-1",
@ -480,6 +483,7 @@ describe("agent permission routes", () => {
heartbeat: {
enabled: false,
intervalSec: 3600,
maxConcurrentRuns: 5,
},
},
}),
@ -517,12 +521,73 @@ describe("agent permission routes", () => {
heartbeat: {
enabled: false,
intervalSec: 3600,
maxConcurrentRuns: 5,
},
},
}),
);
});
it("allows board users to directly approve pending agents", async () => {
const pendingAgent = {
...baseAgent,
status: "pending_approval",
};
const approvedAgent = {
...baseAgent,
status: "idle",
};
mockAgentService.getById.mockResolvedValue(pendingAgent);
mockAgentService.activatePendingApproval.mockResolvedValue({
agent: approvedAgent,
activated: true,
});
const app = await createApp({
type: "board",
userId: "board-user",
source: "local_implicit",
isInstanceAdmin: true,
companyIds: [companyId],
});
const res = await request(app)
.post(`/api/agents/${agentId}/approve`)
.send({});
expect(res.status).toBe(200);
expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith(agentId);
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
companyId,
actorType: "user",
actorId: "board-user",
action: "agent.approved",
entityType: "agent",
entityId: agentId,
details: { source: "agent_detail" },
}));
});
it("rejects direct approval for agents that are not pending approval", async () => {
const app = await createApp({
type: "board",
userId: "board-user",
source: "local_implicit",
isInstanceAdmin: true,
companyIds: [companyId],
});
const res = await request(app)
.post(`/api/agents/${agentId}/approve`)
.send({});
expect(res.status).toBe(409);
expect(mockAgentService.activatePendingApproval).not.toHaveBeenCalled();
expect(mockLogActivity).not.toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
action: "agent.approved",
}));
});
it("exposes explicit task assignment access on agent detail", async () => {
mockAccessService.listPrincipalGrants.mockResolvedValue([
{
@ -615,6 +680,12 @@ describe("agent permission routes", () => {
status: "todo",
},
]);
expect(mockIssueService.list).toHaveBeenCalledWith(companyId, {
touchedByUserId: "board-user",
inboxArchivedByUserId: "board-user",
status: "backlog,todo,in_progress,in_review,blocked,done",
limit: 500,
});
});
it("rejects heartbeat cancellation outside the caller company scope", async () => {

View file

@ -1539,13 +1539,13 @@ describe("company portability", () => {
expect(routineSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({
projectId: "project-created",
title: "Monday Review",
assigneeAgentId: null,
assigneeAgentId: "agent-created",
priority: "high",
status: "paused",
concurrencyPolicy: "always_enqueue",
catchUpPolicy: "enqueue_missed_with_cap",
}), expect.any(Object));
expect(result.warnings).toContain(
expect(result.warnings).not.toContain(
"Task monday-review assignee claudecoder is pending_approval; imported work was left unassigned.",
);
expect(routineSvc.createTrigger).toHaveBeenCalledTimes(2);
@ -2132,6 +2132,7 @@ describe("company portability", () => {
runtimeConfig: {
heartbeat: {
enabled: false,
maxConcurrentRuns: 5,
},
},
});
@ -2210,6 +2211,7 @@ describe("company portability", () => {
runtimeConfig: {
heartbeat: {
enabled: false,
maxConcurrentRuns: 5,
},
},
}));
@ -2489,7 +2491,7 @@ describe("company portability", () => {
expect(agentSvc.create).not.toHaveBeenCalled();
});
it("imports new agents through approval and adapter-config normalization", async () => {
it("imports new agents as active while preserving future hire approval settings", async () => {
const portability = companyPortabilityService({} as any);
const exported = await portability.exportBundle("company-1", {
include: {
@ -2549,7 +2551,10 @@ describe("company portability", () => {
adapterConfig: expect.objectContaining({
normalized: true,
}),
status: "pending_approval",
status: "idle",
}));
expect(companySvc.create).toHaveBeenCalledWith(expect.objectContaining({
requireBoardApprovalForNewAgents: true,
}));
});
@ -2614,4 +2619,154 @@ describe("company portability", () => {
},
}));
});
it("nameOverrides applied after collision detection do not re-validate uniqueness", async () => {
const portability = companyPortabilityService({} as any);
const exported = await portability.exportBundle("company-1", {
include: { company: false, agents: true, projects: false, issues: false },
});
// Simulate existing agents so collision detection triggers rename
agentSvc.list.mockResolvedValue([
{ id: "existing-1", name: "ClaudeCoder", status: "idle", role: "engineer", adapterType: "claude_local", adapterConfig: {}, runtimeConfig: {}, budgetMonthlyCents: 0, permissions: {}, metadata: null },
]);
const preview = await portability.previewImport({
source: { type: "inline", rootPath: exported.rootPath, files: exported.files },
include: { company: false, agents: true, projects: false, issues: false },
target: { mode: "existing_company", companyId: "company-1" },
agents: ["claudecoder"],
collisionStrategy: "rename",
nameOverrides: { claudecoder: "ClaudeCoder" },
});
// The override reverts the renamed agent back to its original collision name.
// This is a known limitation: nameOverrides bypass collision checks.
const plan = preview.plan.agentPlans.find((p) => p.slug === "claudecoder");
expect(plan).toBeDefined();
expect(plan!.action).toBe("create");
expect(plan!.plannedName).toBe("ClaudeCoder");
});
it("handles circular reportsTo chains without infinite recursion during export", async () => {
const portability = companyPortabilityService({} as any);
agentSvc.list.mockResolvedValue([
{
id: "agent-a", name: "AgentA", status: "idle", role: "engineer", title: null, icon: null,
reportsTo: "agent-b", capabilities: null, adapterType: "claude_local",
adapterConfig: {}, runtimeConfig: {}, budgetMonthlyCents: 0, permissions: {}, metadata: null,
},
{
id: "agent-b", name: "AgentB", status: "idle", role: "manager", title: null, icon: null,
reportsTo: "agent-a", capabilities: null, adapterType: "claude_local",
adapterConfig: {}, runtimeConfig: {}, budgetMonthlyCents: 0, permissions: {}, metadata: null,
},
]);
agentInstructionsSvc.exportFiles.mockResolvedValue({
files: { "AGENTS.md": "Instructions" }, entryFile: "AGENTS.md", warnings: [],
});
// Export should complete without infinite recursion in org chart building
const exported = await portability.exportBundle("company-1", {
include: { company: true, agents: true, projects: false, issues: false },
});
expect(exported.manifest.agents).toHaveLength(2);
// Both agents should appear in the export
const slugs = exported.manifest.agents.map((a) => a.slug);
expect(slugs).toContain("agenta");
expect(slugs).toContain("agentb");
});
it("resolves issue assignee to existing agent when agent is skipped", async () => {
const portability = companyPortabilityService({} as any);
projectSvc.list.mockResolvedValue([{
id: "project-1", companyId: "company-1", name: "TestProject", urlKey: "testproject",
description: null, leadAgentId: null, targetDate: null, color: null, status: "planned",
executionWorkspacePolicy: null, archivedAt: null, workspaces: [],
}]);
issueSvc.list.mockResolvedValue([{
id: "issue-1", companyId: "company-1", title: "Test task", identifier: "PAP-1",
description: "A test task", status: "todo", priority: "medium",
assigneeAgentId: "agent-1", projectId: "project-1", projectWorkspaceId: null,
goalId: null, parentId: null, billingCode: null, labelIds: [],
executionWorkspaceSettings: null, assigneeAdapterOverrides: null, metadata: null,
}]);
const exported = await portability.exportBundle("company-1", {
include: { company: false, agents: true, projects: true, issues: true },
});
// Re-import into same company with skip collision strategy
// Both agents exist so both will be skipped; the existing agent should resolve for issue assignment
agentSvc.list.mockResolvedValue([
{ id: "agent-1", name: "ClaudeCoder", status: "idle", role: "engineer", adapterType: "claude_local", adapterConfig: {}, runtimeConfig: {}, budgetMonthlyCents: 0, permissions: {}, metadata: null },
{ id: "agent-2", name: "CMO", status: "idle", role: "cmo", adapterType: "claude_local", adapterConfig: {}, runtimeConfig: {}, budgetMonthlyCents: 0, permissions: {}, metadata: null },
]);
projectSvc.list.mockResolvedValue([]);
issueSvc.list.mockResolvedValue([]);
projectSvc.create.mockResolvedValue({ id: "project-new", companyId: "company-1", urlKey: "testproject" });
issueSvc.create.mockResolvedValue({ id: "issue-new", identifier: "PAP-100" });
const result = await portability.importBundle({
source: { type: "inline", rootPath: exported.rootPath, files: exported.files },
include: { company: false, agents: true, projects: true, issues: true },
target: { mode: "existing_company", companyId: "company-1" },
agents: "all",
collisionStrategy: "skip",
}, "user-1");
// Both agents should be skipped (already exist)
const agentResult = result.agents.find((a) => a.slug === "claudecoder");
expect(agentResult).toBeDefined();
expect(agentResult!.action).toBe("skipped");
// Issue should still be created and reference the existing agent
expect(issueSvc.create).toHaveBeenCalled();
const issueCreateCall = issueSvc.create.mock.calls[0];
// The assigneeAgentId should resolve to the existing agent via existingSlugToAgentId
expect(issueCreateCall[1]).toEqual(expect.objectContaining({
assigneeAgentId: "agent-1",
}));
});
it("handles a package with only skills (no agents or projects)", async () => {
const portability = companyPortabilityService({} as any);
const exported = await portability.exportBundle("company-1", {
include: { company: false, agents: false, projects: false, issues: false, skills: true },
expandReferencedSkills: true,
});
expect(exported.manifest.agents).toHaveLength(0);
expect(exported.manifest.projects).toHaveLength(0);
expect(exported.manifest.issues).toHaveLength(0);
// Skills should still be exported
expect(exported.manifest.skills.length).toBeGreaterThanOrEqual(0);
});
it("preview import detects no agents to import when agents are excluded", async () => {
const portability = companyPortabilityService({} as any);
const exported = await portability.exportBundle("company-1", {
include: { company: true, agents: true, projects: false, issues: false },
});
agentSvc.list.mockResolvedValue([]);
const preview = await portability.previewImport({
source: { type: "inline", rootPath: exported.rootPath, files: exported.files },
include: { company: false, agents: false, projects: false, issues: false },
target: { mode: "existing_company", companyId: "company-1" },
agents: "all",
collisionStrategy: "rename",
});
expect(preview.plan.agentPlans).toHaveLength(0);
expect(preview.plan.projectPlans).toHaveLength(0);
expect(preview.plan.issuePlans).toHaveLength(0);
});
});

View file

@ -236,6 +236,115 @@ describe("heartbeat comment wake batching", () => {
}
});
it("defers approval-approved wakes for a running issue so the assignee resumes after the run", async () => {
const companyId = randomUUID();
const agentId = randomUUID();
const issueId = randomUUID();
const runId = randomUUID();
const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
const heartbeat = heartbeatService(db);
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix,
requireBoardApprovalForNewAgents: false,
});
await db.insert(agents).values({
id: agentId,
companyId,
name: "CEO",
role: "ceo",
status: "running",
adapterType: "process",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
});
await db.insert(heartbeatRuns).values({
id: runId,
companyId,
agentId,
invocationSource: "assignment",
triggerDetail: "system",
status: "running",
contextSnapshot: {
issueId,
taskId: issueId,
wakeReason: "issue_assigned",
},
});
await db.insert(issues).values({
id: issueId,
companyId,
title: "Hire an agent",
status: "blocked",
priority: "medium",
assigneeAgentId: agentId,
executionRunId: runId,
executionAgentNameKey: "ceo",
executionLockedAt: new Date(),
issueNumber: 1,
identifier: `${issuePrefix}-1`,
});
const followupRun = await heartbeat.wakeup(agentId, {
source: "automation",
triggerDetail: "system",
reason: "approval_approved",
payload: {
issueId,
approvalId: "approval-1",
approvalStatus: "approved",
},
contextSnapshot: {
issueId,
taskId: issueId,
approvalId: "approval-1",
approvalStatus: "approved",
wakeReason: "approval_approved",
},
requestedByActorType: "user",
requestedByActorId: "local-board",
});
expect(followupRun).toBeNull();
const deferred = await db
.select()
.from(agentWakeupRequests)
.where(
and(
eq(agentWakeupRequests.companyId, companyId),
eq(agentWakeupRequests.agentId, agentId),
eq(agentWakeupRequests.status, "deferred_issue_execution"),
),
)
.then((rows) => rows[0] ?? null);
expect(deferred).not.toBeNull();
expect(deferred?.reason).toBe("issue_execution_deferred");
expect(deferred?.payload).toMatchObject({
issueId,
approvalId: "approval-1",
approvalStatus: "approved",
});
expect((deferred?.payload as Record<string, unknown>)._paperclipWakeContext).toMatchObject({
issueId,
taskId: issueId,
approvalId: "approval-1",
approvalStatus: "approved",
wakeReason: "approval_approved",
});
const runs = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId));
expect(runs).toHaveLength(1);
expect(runs[0]?.id).toBe(runId);
});
it("batches deferred comment wakes and forwards the ordered batch to the next run", async () => {
const gateway = await createControlledGatewayServer();
const companyId = randomUUID();

View file

@ -5,7 +5,7 @@ import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { heartbeatService } from "../services/heartbeat.ts";
import { boundHeartbeatRunEventPayloadForStorage, heartbeatService } from "../services/heartbeat.ts";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
@ -202,3 +202,25 @@ describeEmbeddedPostgres("heartbeat list", () => {
expect(result).not.toHaveProperty("nestedHuge");
});
});
describe("heartbeat run event payload bounding", () => {
it("truncates oversized adapter metadata before storage", () => {
const payload = boundHeartbeatRunEventPayloadForStorage({
adapterType: "codex_local",
prompt: "x".repeat(40_000),
context: {
issueId: "issue-1",
memory: "y".repeat(40_000),
},
});
expect(payload.adapterType).toBe("codex_local");
expect(typeof payload.prompt).toBe("string");
expect((payload.prompt as string).length).toBeLessThan(20_000);
expect(payload.prompt).toContain("[truncated");
expect(payload.context).toMatchObject({
issueId: "issue-1",
});
expect(JSON.stringify(payload).length).toBeLessThan(45_000);
});
});

View file

@ -702,6 +702,56 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
}
});
it("does not continue seeded in-progress work that has no run linkage", async () => {
const companyId = randomUUID();
const agentId = randomUUID();
const issueId = randomUUID();
const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix,
requireBoardApprovalForNewAgents: false,
});
await db.insert(agents).values({
id: agentId,
companyId,
name: "CodexCoder",
role: "engineer",
status: "idle",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
});
await db.insert(issues).values({
id: issueId,
companyId,
title: "Seeded in-flight work",
status: "in_progress",
priority: "medium",
assigneeAgentId: agentId,
checkoutRunId: null,
executionRunId: null,
issueNumber: 1,
identifier: `${issuePrefix}-1`,
startedAt: new Date("2026-03-19T00:00:00.000Z"),
});
const heartbeat = heartbeatService(db);
const result = await heartbeat.reconcileStrandedAssignedIssues();
expect(result.dispatchRequeued).toBe(0);
expect(result.continuationRequeued).toBe(0);
expect(result.escalated).toBe(0);
expect(result.skipped).toBe(1);
const runs = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId));
expect(runs).toHaveLength(0);
const [issue] = await db.select().from(issues).where(eq(issues.id, issueId));
expect(issue?.status).toBe("in_progress");
expect(issue?.executionRunId).toBeNull();
});
it("classifies actionable plan-only recovery and enqueues one liveness continuation", async () => {
mockAdapterExecute.mockResolvedValueOnce({
exitCode: 0,
@ -824,6 +874,39 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
expect(comments[0]?.body).toContain("Latest retry failure: `process_lost` - run failed before issue advanced.");
});
it("re-enqueues continuation when the latest automatic continuation succeeded without closing the issue", async () => {
const { agentId, issueId, runId } = await seedStrandedIssueFixture({
status: "in_progress",
runStatus: "succeeded",
retryReason: "issue_continuation_needed",
});
const heartbeat = heartbeatService(db);
const result = await heartbeat.reconcileStrandedAssignedIssues();
expect(result.continuationRequeued).toBe(1);
expect(result.escalated).toBe(0);
expect(result.issueIds).toEqual([issueId]);
const issue = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null);
expect(issue?.status).toBe("in_progress");
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId));
expect(comments).toHaveLength(0);
const runs = await db
.select()
.from(heartbeatRuns)
.where(eq(heartbeatRuns.agentId, agentId));
expect(runs).toHaveLength(2);
const retryRun = runs.find((row) => row.id !== runId);
expect(retryRun?.id).toBeTruthy();
expect((retryRun?.contextSnapshot as Record<string, unknown>)?.retryReason).toBe("issue_continuation_needed");
if (retryRun) {
await waitForRunToSettle(heartbeat, retryRun.id);
}
});
it("does not reconcile user-assigned work through the agent stranded-work recovery path", async () => {
const { issueId, runId } = await seedStrandedIssueFixture({
status: "todo",

View file

@ -22,12 +22,20 @@ import {
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { instanceSettingsService } from "../services/instance-settings.ts";
import { issueService } from "../services/issues.ts";
import { clampIssueListLimit, ISSUE_LIST_MAX_LIMIT, issueService } from "../services/issues.ts";
import { buildProjectMentionHref } from "@paperclipai/shared";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
describe("issue list limit helpers", () => {
it("clamps untrusted issue-list limits to the server maximum", () => {
expect(clampIssueListLimit(0)).toBe(1);
expect(clampIssueListLimit(25.9)).toBe(25);
expect(clampIssueListLimit(ISSUE_LIST_MAX_LIMIT + 10)).toBe(ISSUE_LIST_MAX_LIMIT);
});
});
async function ensureIssueRelationsTable(db: ReturnType<typeof createDb>) {
await db.execute(sql.raw(`
CREATE TABLE IF NOT EXISTS "issue_relations" (