From 50b932a142216331b8d476042b454bc123739c40 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:30:50 -0500 Subject: [PATCH] fix(server): require approval before onboarding task creation (#17914) https://entire.com/gh/paperclipai/-/trails/78/changes/paperclip/78 ## Thinking Path > - Paperclip manages agent work and human decisions. > - Onboarding asks the first agent to propose a task before it starts work. > - The first-task skill requires human acceptance before task creation. > - The native tool could create and schedule that task without acceptance. > - REST task creation had the same gap. > - This change checks saved acceptance in the shared creation transaction. > - Valid approvals and ordinary delegation keep their existing behavior. ## Linked Issues or Issue Description **What happened?** An agent could create a task from the canonical onboarding first task before the user accepted its proposal. A real journey caught this after the agent read the first-task skill. A local database regression reproduces the missing check in the native tool and both REST creation endpoints. **Expected behavior** New task creation must wait for saved human acceptance of the latest ordinary proposal. A different parent or a standalone task must not bypass that check. **Steps to reproduce** 1. Bind an authenticated agent run to a server-created onboarding first task. 2. Leave its proposal unaccepted. 3. Call native `create_task`, `POST /api/companies/:companyId/issues`, or `POST /api/issues/:issueId/children`. 4. Before this fix, creation succeeds. After this fix, it is denied before a child or wake is committed. **Paperclip version or commit** Reproduced on `3be54534c24fc279533ff421e62a77ad15b72954` before the fix. ## What Changed - Add a shared check for new task creation from an authenticated onboarding run. - Read the latest ordinary confirmation across both confirmation kinds under the source task lock. - Require a human card decision or validated conversational acceptance. Require the current revision for a saved plan. - Preserve ordinary delegation, committed replay, and dispatch recovery. - Add database regressions and update execution and onboarding map documentation. ## Verification - Real PostgreSQL regression before the fix: 17 failures and four passing approval/delegation controls. No skips. - Exact published source: 102 tests passed across admission, conversational confirmation, and project tools, including all 25 admission regressions. Real Linux PostgreSQL, no skips. - `pnpm build` and `pnpm -r typecheck` passed locally. - Map checks and `git diff --check` passed. - `pnpm test:run` was attempted locally. Existing suites failed because the host exhausted PostgreSQL semaphore sets. The remaining owned test process was stopped; its failed log is preserved. No unrelated database was stopped. Current-head Linux CI passed, including build, typecheck, and browser E2E. The first CI attempt lost runners; its evidence is preserved, and interrupted jobs passed one CI rerun. - The live Cloud attempt remains failed. Local tests do not qualify Cloud onboarding, artifact delivery, or native tracing. No deployment or model retry is part of this change. ## Risks A stricter creation check could reject a valid approval. Tests cover direct, conversational, checkbox, dismissed-card, and clock-skew cases. The check applies only to new tasks from the canonical onboarding run. It does not change existing task updates, execution policy, or wake recovery. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The session does not expose a more specific serving model ID or context-window size. ## 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 --- doc/execution-semantics.md | 21 ++ maps/agent-access-map/onboarding.md | 13 + maps/feature-map/onboarding.md | 10 + .../onboarding-first-task-admission.test.ts | 264 ++++++++++++++++++ server/src/services/issues.ts | 7 + .../onboarding-first-task-admission.ts | 130 +++++++++ 6 files changed, 445 insertions(+) create mode 100644 server/src/__tests__/onboarding-first-task-admission.test.ts create mode 100644 server/src/services/onboarding-first-task-admission.ts diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index dc423774d43..e653e027e9e 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -19,6 +19,27 @@ Paperclip separates four concepts that are easy to blur together: The system works best when those are kept separate. +### Onboarding first-task creation + +When an authenticated agent run serves the server-created `onboarding_first_task`, +new task creation requires acceptance of its latest ordinary proposal. The same +transactional admission applies to native `create_task` and REST issue/child +creation. Choosing a different parent, including a standalone task, does not +change the run's source task. + +Acceptance comes from a saved human card decision or a saved conversational +resolution linked to a genuine later user comment on that task. Opening answers +and clarification responses alone, agent-only decisions, and tool/credential +approvals do not authorize the proposal. A newer pending or rejected proposal supersedes an +older acceptance. A saved plan requires approval of its current document revision; +checkbox decisions retain the user's actual selected scope. + +Planning documents, questions and proposal cards remain available before approval. +Ordinary task delegation and previously committed task-creation replay retain +their existing behavior, including dispatch recovery for already admitted work. +This boundary governs new task creation; other execution and resource authorities +continue to apply independently. + ## 2. Assignee Semantics An issue has at most one assignee. diff --git a/maps/agent-access-map/onboarding.md b/maps/agent-access-map/onboarding.md index 62b3cb642c5..9c5dded3eff 100644 --- a/maps/agent-access-map/onboarding.md +++ b/maps/agent-access-map/onboarding.md @@ -174,12 +174,25 @@ Review: `baseline`. API: `present` — representative operation IDs: - `POST /api/companies/{companyId}/issues`. +- `POST /api/issues/{issueId}/children`. Agent access: `available` — Native create_task / agent REST / CLI issue. Policy: Instance setup is operator work; company creation is board; agent creation/task creation retain their individual permissions. +An authenticated run bound to the server-created onboarding first task must +obtain persisted human acceptance of the latest ordinary proposal before creating +new tasks through native `create_task` or REST. Its authenticated run determines +this admission even for an explicit standalone task. A card decision or a saved +conversational resolution with a genuine later same-task user message supplies +acceptance; an agent-only resolution does not. A plan approval must name the +current same-task revision. Newer pending/rejected proposals stop new creation. +Already admitted task replay and ordinary delegation retain their existing rules. +Source: [creation admission](../../server/src/services/onboarding-first-task-admission.ts). +Controls: [native/REST PostgreSQL regressions](../../server/src/__tests__/onboarding-first-task-admission.test.ts). +These controls do not qualify a live Cloud provider journey. + Evidence: `source-only` — [implementation](../../ui/src/components/OnboardingWizard.tsx#L279) at the reviewed commit above. diff --git a/maps/feature-map/onboarding.md b/maps/feature-map/onboarding.md index c7e2fd0c9de..347613e3829 100644 --- a/maps/feature-map/onboarding.md +++ b/maps/feature-map/onboarding.md @@ -132,6 +132,16 @@ parent, and recovery stopped the completion callback before a provider turn. The failed report remains evidence; the revised source still needs live proof. Local cases and automatic selection remain unchanged. +First-task task creation enforces proposal acceptance at the shared server +boundary. Verify that native `create_task` and both REST creation endpoints reject +an authenticated onboarding run before its latest proposal has a saved human +acceptance, including an explicit standalone parent. Then accept a confirmation +or the current plan revision and verify creation succeeds. Opening answers, +agent-only acceptance, stale plans and newer pending/rejected proposals must not +admit new work. [Admission controls](../../server/src/__tests__/onboarding-first-task-admission.test.ts) +exercise native and REST calls against disposable PostgreSQL; live model and +Cloud onboarding remain separate evidence. + For accepted file-producing work, verify exactly one self-assigned child with a self-contained description. The onboarding run must yield through the existing dependency/block disposition so the child's own execution can publish its file. diff --git a/server/src/__tests__/onboarding-first-task-admission.test.ts b/server/src/__tests__/onboarding-first-task-admission.test.ts new file mode 100644 index 00000000000..39759102f07 --- /dev/null +++ b/server/src/__tests__/onboarding-first-task-admission.test.ts @@ -0,0 +1,264 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import { authUsers, companyMemberships, heartbeatRuns, issueComments, issueThreadInteractions, issues } from "@paperclipai/db"; +import { issueThreadInteractionService } from "../services/issue-thread-interactions.js"; +import { documentService } from "../services/documents.js"; +import { resolveConfirmationFromComment } from "../services/confirmation-comment-resolution.js"; +import { PaperclipRunnerToolAuthority } from "../services/native-runtime/paperclip-runner-tool-authority.js"; +import { createLocalAgentJwt } from "../agent-auth-jwt.js"; +import { getEmbeddedPostgresTestSupport } from "./helpers/embedded-postgres.js"; +import { startRunnerApiTestServer } from "./helpers/runner-api-server.js"; + +const support = await getEmbeddedPostgresTestSupport(); +if (!support.supported) console.warn(`Skipping first-task admission database tests: ${support.reason}`); +describe.skipIf(!support.supported)("onboarding first-task execution admission", () => { + let server: Awaited>; + const originalSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET; + beforeAll(async () => { + process.env.PAPERCLIP_AGENT_JWT_SECRET = randomUUID(); + server = await startRunnerApiTestServer(); + }, 60_000); + afterAll(async () => { + await server?.close(); + if (originalSecret === undefined) delete process.env.PAPERCLIP_AGENT_JWT_SECRET; + else process.env.PAPERCLIP_AGENT_JWT_SECRET = originalSecret; + }); + + const fixture = async () => { + const f = await server.fixture({ disableWakeOnDemand: true, connectionScenario: "fresh" }); + await server.db.update(issues).set({ originKind: "onboarding_first_task" }).where(eq(issues.id, f.issueId)); + return f; + }; + + it("rejects native child creation before persisted human proposal acceptance", async () => { + const f = await fixture(); + const before = await f.snapshot(); + const [beforeRun] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + const wakes: unknown[] = []; + const authority = new PaperclipRunnerToolAuthority(server.db, { ...f, enqueueWakeup: async (...args) => { wakes.push(args); return null; } }); + await expect(authority.execute({ tool: "create_task", callId: randomUUID(), arguments: { + title: "Execute proposed work", idempotencyKey: "before-acceptance", status: "todo", + } })).rejects.toThrow(/proposal.*accept/i); + const after = await f.snapshot(); + expect(after.issues).toEqual(before.issues); + expect(after.activity).toEqual(before.activity); + expect(wakes).toEqual([]); + const [afterRun] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + expect(afterRun.resultJson).toEqual(beforeRun.resultJson); + }); + + it.each(["root", "child"])("rejects generic %s creation including explicit standalone parent before acceptance", async path => { + const f = await fixture(); + const before = await f.snapshot(); + const token = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", f.runId)!; + const route = path === "root" ? `/api/companies/${f.companyId}/issues` : `/api/issues/${f.issueId}/children`; + const response = await fetch(`${server.apiUrl}${route}`, { + method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" }, + body: JSON.stringify({ title: "Bypass proposal", parentId: null, assigneeAgentId: f.agentId, status: "todo" }), + }); + const body = await response.json(); + expect(response.status, JSON.stringify(body)).toBe(403); + expect(body.error).toMatch(/proposal.*accept/i); + const after = await f.snapshot(); + expect(after.issues).toEqual(before.issues); + expect(after.activity).toEqual(before.activity); + }); + + const create = (f: Awaited>, key = randomUUID()) => f.authority.execute({ + tool: "create_task", callId: randomUUID(), arguments: { title: "Approved execution", idempotencyKey: key, status: "backlog" }, + }); + const proposal = async (f: Awaited>, checkbox = false) => { + const [issue] = await server.db.select().from(issues).where(eq(issues.id, f.issueId)); + const plan = checkbox ? (await documentService(server.db).upsertIssueDocument({ + issueId: f.issueId, key: "plan", title: "First task plan", format: "markdown", body: "Create and verify the approved deliverable.", + createdByAgentId: f.agentId, createdByRunId: f.runId, + })).document : null; + const svc = issueThreadInteractionService(server.db); + const card = await svc.create(issue, checkbox ? { + kind: "request_checkbox_confirmation", payload: { version: 1, prompt: "Approve this plan?", + options: [{ id: "execute", label: "Execute the plan" }], defaultSelectedOptionIds: ["execute"], minSelected: 1, + target: { type: "issue_document", key: "plan", issueId: f.issueId, documentId: plan!.id, revisionId: plan!.latestRevisionId! } }, + } : { kind: "request_confirmation", payload: { version: 1, prompt: "Create one task for the agreed deliverable?" } }, { agentId: f.agentId }); + return { issue, card, svc, plan }; + }; + const accept = async (f: Awaited>, p: Awaited>) => { + await p.svc.acceptInteraction(p.issue, p.card.id, p.plan ? { selectedOptionIds: ["execute"] } : {}, { userId: f.responsibleUserId! }); + }; + + it.each([false, true])("admits persisted human proposal acceptance (checkbox=%s)", async checkbox => { + const f = await fixture(); + const p = await proposal(f, checkbox); + await accept(f, p); + await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } }); + }); + + it("admits a persisted conversational acceptance with actual same-task human comment provenance", async () => { + const f = await fixture(); + const p = await proposal(f); + const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, + authorUserId: f.responsibleUserId!, authorType: "user", body: "Yes, create that task." }).returning(); + await resolveConfirmationFromComment(server.db, { companyId: f.companyId, issueId: f.issueId, + interactionId: p.card.id, actor: { agentId: f.agentId, runId: f.runId }, input: { commentId: comment.id, decision: "accept" } }); + const [saved] = await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, p.card.id)); + expect(saved).toMatchObject({ status: "accepted", resolvedByAgentId: f.agentId, resolvedByUserId: null, + result: { outcome: "accepted", commentId: comment.id } }); + await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } }); + }); + + it.each(["card", "conversation"])("preserves valid %s acceptance across clocks and a changed responding user", async kind => { + const f = await fixture(); + const originalUserId = randomUUID(); + await server.db.insert(authUsers).values({ id: originalUserId, name: "Original requester", + email: `${originalUserId}@fixture.invalid`, createdAt: new Date(), updatedAt: new Date() }); + await server.db.insert(companyMemberships).values({ companyId: f.companyId, principalType: "user", + principalId: originalUserId, status: "active", membershipRole: "member" }); + await server.db.update(issues).set({ responsibleUserId: originalUserId }).where(eq(issues.id, f.issueId)); + const p = await proposal(f); + if (kind === "card") await accept(f, p); + else { + const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, + authorUserId: f.responsibleUserId!, authorType: "user", body: "Yes, create that task." }).returning(); + await resolveConfirmationFromComment(server.db, { companyId: f.companyId, issueId: f.issueId, + interactionId: p.card.id, actor: { agentId: f.agentId, runId: f.runId }, input: { commentId: comment.id, decision: "accept" } }); + } + // Database-created timestamps and application decision timestamps have + // independent clocks. Persisted authority must not depend on their order. + await server.db.update(issueThreadInteractions).set({ resolvedAt: new Date(new Date(p.card.createdAt).getTime() - 100) }) + .where(eq(issueThreadInteractions.id, p.card.id)); + await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } }); + }); + + it("preserves acceptance of a previously dismissed proposal", async () => { + const f = await fixture(); + const p = await proposal(f); + await p.svc.dismissInteraction(p.issue, p.card.id, { userId: f.responsibleUserId! }); + await expect(create(f)).rejects.toThrow(/proposal.*accept/i); + await accept(f, p); + const [saved] = await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, p.card.id)); + expect(saved.dismissedAt).not.toBeNull(); + await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } }); + }); + + it.each(["opening-answer", "agent-self-accept", "old-comment", "foreign-comment", "derived-comment", "governed-accept", "foreign-source-run"])( + "does not treat %s as persisted human proposal acceptance", async kind => { + const f = await fixture(); + const p = await proposal(f); + if (kind === "opening-answer") { + await server.db.update(issueThreadInteractions).set({ kind: "ask_user_questions", status: "answered", + result: { version: 1, answers: [] }, resolvedByUserId: f.responsibleUserId!, resolvedAt: new Date() }).where(eq(issueThreadInteractions.id, p.card.id)); + } else if (kind === "agent-self-accept") { + await p.svc.acceptInteraction(p.issue, p.card.id, {}, { agentId: f.agentId, runId: f.runId }); + } else if (kind === "governed-accept") { + await accept(f, p); + await server.db.update(issueThreadInteractions).set({ payload: { ...p.card.payload, toolAction: {} } as never }).where(eq(issueThreadInteractions.id, p.card.id)); + } else if (kind === "foreign-source-run") { + const other = await server.fixture({ disableWakeOnDemand: true }); + await accept(f, p); + await server.db.update(issueThreadInteractions).set({ sourceRunId: other.runId }).where(eq(issueThreadInteractions.id, p.card.id)); + } else { + const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, + issueId: kind === "foreign-comment" ? f.blockerId : f.issueId, + authorUserId: f.responsibleUserId!, authorType: "user", body: "A reply", + ...(kind === "old-comment" ? { createdAt: new Date(new Date(p.card.createdAt).getTime() - 1_000) } : {}), + ...(kind === "derived-comment" ? { derivedAuthorAgentId: f.agentId, derivedCreatedByRunId: f.runId } : {}), + }).returning(); + await server.db.update(issueThreadInteractions).set({ status: "accepted", resolvedAt: new Date(), + resolvedByAgentId: f.agentId, resolvedByRunId: f.runId, + result: { version: 1, outcome: "accepted", commentId: comment.id } }).where(eq(issueThreadInteractions.id, p.card.id)); + } + await expect(create(f)).rejects.toThrow(/proposal.*accept/i); + }); + + it.each(["pending", "rejected"])("a newer cross-kind %s proposal supersedes old acceptance", async status => { + const f = await fixture(); + const old = await proposal(f); + await accept(f, old); + const current = await proposal(f, true); + await server.db.update(issueThreadInteractions).set({ createdAt: new Date(new Date(old.card.createdAt).getTime() + 1_000), + ...(status === "rejected" ? { status, resolvedByUserId: f.responsibleUserId!, resolvedAt: new Date(), + result: { version: 1, outcome: "rejected" } as const } : {}), + }).where(eq(issueThreadInteractions.id, current.card.id)); + await expect(create(f)).rejects.toThrow(/proposal.*accept/i); + }); + + it("does not substitute untargeted acceptance for approval of a saved plan", async () => { + const f = await fixture(); + const p = await proposal(f); + await documentService(server.db).upsertIssueDocument({ issueId: f.issueId, key: "plan", format: "markdown", + body: "This saved plan requires acceptance of its exact revision.", createdByAgentId: f.agentId, createdByRunId: f.runId }); + await accept(f, p); + await expect(create(f)).rejects.toThrow(/proposal.*accept/i); + }); + + it.each(["stale", "foreign-target", "empty-selection"])("does not execute a plan with %s approval", async kind => { + const f = await fixture(); + const p = await proposal(f, true); + await accept(f, p); + if (kind === "stale") { + await documentService(server.db).unlockIssueDocument(f.issueId, "plan"); + await documentService(server.db).upsertIssueDocument({ issueId: f.issueId, key: "plan", format: "markdown", + baseRevisionId: p.plan!.latestRevisionId, body: "A revised scope that has not been accepted." }); + } else if (kind === "foreign-target") { + await server.db.update(issueThreadInteractions).set({ payload: { ...p.card.payload, + target: { type: "issue_document", key: "plan", issueId: f.blockerId, revisionId: p.plan!.latestRevisionId! } } as never }).where(eq(issueThreadInteractions.id, p.card.id)); + } else { + await server.db.update(issueThreadInteractions).set({ result: { version: 1, outcome: "accepted", selectedOptionIds: [] } }).where(eq(issueThreadInteractions.id, p.card.id)); + } + await expect(create(f)).rejects.toThrow(/proposal.*accept/i); + }); + + it("keeps ordinary delegation available even when its description invokes the skill", async () => { + const f = await fixture(); + await server.db.update(issues).set({ originKind: "manual", description: "Please read /first-task, then delegate." }).where(eq(issues.id, f.issueId)); + await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } }); + }); + + it("preserves native creation replay and dispatch recovery after a newer pending proposal", async () => { + const f = await fixture(); + const p = await proposal(f); + await accept(f, p); + const wakes: unknown[] = []; + const authority = new PaperclipRunnerToolAuthority(server.db, { ...f, enqueueWakeup: async (...args) => { wakes.push(args); return null; } }); + const input = { tool: "create_task", callId: randomUUID(), arguments: { title: "Approved child", idempotencyKey: "owned", status: "todo" } }; + const first = await authority.execute(input); + expect(wakes).toHaveLength(1); + await proposal(f); + const before = await f.snapshot(); + const [beforeRun] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + expect(await authority.execute({ ...input, callId: randomUUID() })).toEqual(first); + // Existing dispatch recovery remains bounded to this already-admitted child. + expect(wakes).toHaveLength(2); + const after = await f.snapshot(); + expect(after.issues).toEqual(before.issues); + expect(after.activity).toEqual(before.activity); + const [afterRun] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + expect(afterRun.resultJson).toEqual(beforeRun.resultJson); + await expect(authority.execute({ ...input, arguments: { ...input.arguments, idempotencyKey: "new" } })).rejects.toThrow(/proposal.*accept/i); + }); + + + it("replays an owned generic creation after a newer pending proposal", async () => { + const f = await fixture(); + await accept(f, await proposal(f)); + const token = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", f.runId, f.responsibleUserId)!; + const route = `${server.apiUrl}/api/companies/${f.companyId}/issues`; + const body = { title: "Approved child", parentId: f.issueId, idempotencyKey: "owned-rest", assigneeAgentId: f.agentId, status: "backlog" }; + const post = async (input = body) => { + const response = await fetch(route, { method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" }, body: JSON.stringify(input) }); + return { response, data: await response.json() }; + }; + const first = await post(); + expect(first.response.status, JSON.stringify(first.data)).toBe(201); + await proposal(f); + const before = await f.snapshot(); + const replay = await post(); + expect(replay.response.status, JSON.stringify(replay.data)).toBe(200); + expect(replay.data).toMatchObject({ id: first.data.id, deduplicated: true }); + const after = await f.snapshot(); + expect(after.issues).toEqual(before.issues); + const fresh = await post({ ...body, idempotencyKey: "new-rest", title: "Another child" }); + expect(fresh.response.status, JSON.stringify(fresh.data)).toBe(403); + }); + +}); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index d413d0304cf..c6e54798555 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -1,3 +1,4 @@ +import { assertOnboardingFirstTaskCreationAllowed } from "./onboarding-first-task-admission.js"; import { resolveConnectionIntentContinuationSource, resolveItemVerdictContinuationWake } from "./chat-interaction-continuation.js"; import { insertIssueComment } from "./issue-comment-writes.js"; import { measureRunTransaction } from "./execution-transaction-timing.js"; @@ -10147,6 +10148,12 @@ export function issueService(db: Db) { return withRelations; } + await assertOnboardingFirstTaskCreationAllowed(tx as unknown as Db, { + companyId, + actorAgentId: issueData.createdByAgentId, + actorRunId, + }); + const defaultCompanyGoal = await getDefaultCompanyGoal(tx, companyId); let projectWorkspaceId = issueData.projectWorkspaceId ?? null; let executionWorkspaceId = issueData.executionWorkspaceId ?? null; diff --git a/server/src/services/onboarding-first-task-admission.ts b/server/src/services/onboarding-first-task-admission.ts new file mode 100644 index 00000000000..e649120bb6b --- /dev/null +++ b/server/src/services/onboarding-first-task-admission.ts @@ -0,0 +1,130 @@ +import { and, desc, eq, inArray, sql } from "drizzle-orm"; +import { agents, documents, heartbeatRuns, issueComments, issueDocuments, issues, issueThreadInteractions, type Db } from "@paperclipai/db"; +import { ONBOARDING_FIRST_TASK_ORIGIN_KIND, requestCheckboxConfirmationPayloadSchema, requestCheckboxConfirmationResultSchema, + requestConfirmationPayloadSchema, requestConfirmationResultSchema } from "@paperclipai/shared"; +import { forbidden } from "../errors.js"; + +const object = (value: unknown): Record => + value && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; + +const runOriginFields = { + nativeIssueId: heartbeatRuns.nativeIssueId, + issueId: heartbeatRuns.issueId, + contextIssueId: sql`${heartbeatRuns.contextSnapshot}->>'issueId'`, + contextTaskId: sql`${heartbeatRuns.contextSnapshot}->>'taskId'`, + agentId: heartbeatRuns.agentId, + responsibleUserId: heartbeatRuns.responsibleUserId, +}; + +function runIssueId(run: { + nativeIssueId: string | null; issueId: string | null; contextIssueId: unknown; contextTaskId: unknown; +}): string | null { + const id = run.nativeIssueId ?? run.issueId ?? run.contextIssueId ?? run.contextTaskId; + return typeof id === "string" ? id : null; +} + +const acceptanceRequired = () => forbidden( + "First-task proposal requires human acceptance before creating execution tasks.", + { code: "onboarding_first_task_acceptance_required" }, +); + +/** + * Creation admission for a run serving the server-owned onboarding task. This is + * deliberately independent of the requested parent and client origin metadata. + * Call inside the creation transaction, after a side-effect-free replay check. + */ +export async function assertOnboardingFirstTaskCreationAllowed(tx: Db, input: { + companyId: string; + actorAgentId?: string | null; + actorRunId?: string | null; +}): Promise { + if (!input.actorAgentId || !input.actorRunId) return; + const [observedRun] = await tx.select(runOriginFields).from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, input.actorRunId), eq(heartbeatRuns.companyId, input.companyId), + eq(heartbeatRuns.agentId, input.actorAgentId), + )); + if (!observedRun) throw forbidden("Authenticated agent run not found"); + const sourceIssueId = runIssueId(observedRun); + if (!sourceIssueId) return; + // Match the native authority/interaction lifecycle: issue, then run, then + // interaction/document. Ordinary tasks acquire no additional row locks. + const [source] = await tx.select({ id: issues.id }).from(issues).where(and( + eq(issues.id, sourceIssueId), eq(issues.companyId, input.companyId), + eq(issues.originKind, ONBOARDING_FIRST_TASK_ORIGIN_KIND), + )).for("update"); + if (!source) return; + const [run] = await tx.select(runOriginFields).from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, input.actorRunId), eq(heartbeatRuns.companyId, input.companyId), + eq(heartbeatRuns.agentId, input.actorAgentId), + )).for("share"); + if (!run || runIssueId(run) !== source.id) throw acceptanceRequired(); + + // Select the newest proposal before checking its outcome: a later pending or + // rejected proposal must never fall back to an older accepted scope. Governed + // tool/credential actions and custom completion reviews are not proposals. + const [proposal] = await tx.select().from(issueThreadInteractions).where(and( + eq(issueThreadInteractions.companyId, input.companyId), eq(issueThreadInteractions.issueId, source.id), + inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]), + sql`NOT (${issueThreadInteractions.payload} ?| ARRAY['toolAction', 'secretProposal', 'connectionAuthorization'])`, + sql`COALESCE(${issueThreadInteractions.payload}->'target'->>'type', '') <> 'custom'`, + )).orderBy(desc(issueThreadInteractions.createdAt), desc(issueThreadInteractions.id)).limit(1).for("share"); + if (!proposal || proposal.status !== "accepted" || !proposal.resolvedAt) throw acceptanceRequired(); + const checkbox = proposal.kind === "request_checkbox_confirmation"; + const payload = checkbox ? requestCheckboxConfirmationPayloadSchema.safeParse(proposal.payload) + : requestConfirmationPayloadSchema.safeParse(proposal.payload); + const result = checkbox ? requestCheckboxConfirmationResultSchema.safeParse(proposal.result) + : requestConfirmationResultSchema.safeParse(proposal.result); + if (!payload.success || !result.success || result.data.outcome !== "accepted") throw acceptanceRequired(); + if (checkbox) { + const selected = object(result.data).selectedOptionIds; + if (!("options" in payload.data) || !Array.isArray(selected) || selected.length === 0) throw acceptanceRequired(); + const allowed = new Set(payload.data.options.map(option => option.id)); + if (selected.some(id => typeof id !== "string" || !allowed.has(id)) + || selected.length < payload.data.minSelected + || (payload.data.maxSelected != null && selected.length > payload.data.maxSelected)) throw acceptanceRequired(); + } + + if (proposal.sourceRunId) { + const [origin] = await tx.select(runOriginFields).from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, proposal.sourceRunId), eq(heartbeatRuns.companyId, input.companyId), + )).for("share"); + if (!origin || runIssueId(origin) !== source.id || origin.agentId !== proposal.createdByAgentId) throw acceptanceRequired(); + } else if (proposal.createdByAgentId) { + const [creator] = await tx.select({ companyId: agents.companyId }).from(agents).where(eq(agents.id, proposal.createdByAgentId)); + if (creator?.companyId !== input.companyId) throw acceptanceRequired(); + } else if (!proposal.createdByUserId) throw acceptanceRequired(); + + const directHumanDecision = Boolean(proposal.resolvedByUserId && !proposal.resolvedByAgentId && !proposal.resolvedByRunId); + if (!directHumanDecision) { + const commentId = result.data.commentId; + if (!commentId || !proposal.resolvedByAgentId || !proposal.resolvedByRunId) throw acceptanceRequired(); + const [comment] = await tx.select().from(issueComments).where(and( + eq(issueComments.id, commentId), eq(issueComments.companyId, input.companyId), eq(issueComments.issueId, source.id), + )).for("share"); + if (!comment || comment.deletedAt || comment.authorType !== "user" || !comment.authorUserId + || comment.authorAgentId || comment.createdByRunId || comment.derivedAuthorAgentId || comment.derivedCreatedByRunId + || comment.sourceTrust?.disposition === "quarantined" || comment.createdAt < proposal.createdAt + || comment.body.trim() === "/new") throw acceptanceRequired(); + const [resolver] = await tx.select(runOriginFields).from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, proposal.resolvedByRunId), eq(heartbeatRuns.companyId, input.companyId), + eq(heartbeatRuns.agentId, proposal.resolvedByAgentId), + )).for("share"); + if (!resolver || runIssueId(resolver) !== source.id + || (resolver.responsibleUserId && resolver.responsibleUserId !== comment.authorUserId)) throw acceptanceRequired(); + } + + const target = payload.data.target; + const [plan] = await tx.select({ documentId: documents.id, revisionId: documents.latestRevisionId }) + .from(issueDocuments).innerJoin(documents, eq(documents.id, issueDocuments.documentId)).where(and( + eq(issueDocuments.companyId, input.companyId), eq(documents.companyId, input.companyId), + eq(issueDocuments.issueId, source.id), eq(issueDocuments.key, "plan"), + )).for("share", { of: documents }); + // Document writers do not all lock the issue, so hold the document row too. + // A saved plan must be accepted by revision; an untargeted card cannot stand + // in for approval of a different or subsequently edited document. + if (plan || target) { + if (!target || target.type !== "issue_document" || target.key !== "plan" + || (target.issueId && target.issueId !== source.id) || !plan || plan.revisionId !== target.revisionId + || (target.documentId && plan.documentId !== target.documentId)) throw acceptanceRequired(); + } +}