Files
PaperClipAI/server/src/services/onboarding-first-task-admission.ts
T
DottaandPaperclip 4740d251b7 fix(onboarding): retain plan approval across follow-up decisions (#19043)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Onboarding asks a person to approve a saved plan before execution
tasks can start.
> - A person can then approve a separate proposal for the first
execution step.
> - The creation gate checks only that latest card for the plan target.
It loses the earlier approval of the unchanged plan.
> - Decomposition and native context also omit checkbox plan
confirmations.
> - This pull request validates the latest execution decision and the
latest plan decision together.
> - The benefit is that approved work can start while human, revision,
and proposal gates remain effective.

## Linked Issues or Issue Description

Refs #17914, #18160, and #18283. The related-PR search found no
duplicate repair.

**What happened?**

After a person accepted a checkbox plan card and a later execution
confirmation, native task creation and REST child creation returned 403.
Accepted-plan decomposition returned 422 because its lookup recognized
only standard confirmations. Native task context returned no accepted
plan for the checkbox decision.

**Expected behavior**

The latest execution proposal can use an independently approved,
unchanged plan. A pending or rejected latest proposal must still stop
execution. The current plan must retain valid human approval of its
exact revision.

**Steps to reproduce**

1. Save a plan on a server-created onboarding task.
2. Create a revision-bound checkbox confirmation and accept it as a
human.
3. Create and accept a separate, untargeted confirmation for the first
execution task.
4. Create the child through the native tool or REST API.
5. Separately, try accepted-plan decomposition or read native task
context after step 2.

**Paperclip version or commit**

The workspace's registered hosted release is Core
`09c05c55050fede448196637cbaa543a154da76e`. The three regressions also
failed against local source `3cdb367369f29692b81cf19b514a2abdba90734f`.

**Deployment mode**

Hosted native Runner with ACPX Claude. Local regression proof uses
actual PostgreSQL, authenticated HTTP routes, native tool authority, and
synthetic task data on macOS arm64. No customer data or credentials are
in the fixtures.

## What Changed

- Validate the newest ordinary proposal and the newest plan proposal
separately. Keep the existing human-card and conversational provenance
checks for each.
- Require the accepted plan target to match the current document and
revision. A newer pending, rejected, invalid, or foreign plan decision
cannot fall back to an older approval.
- Share persisted confirmation parsing across admission, decomposition,
and native context. Validate checkbox selections against the saved
options and limits.
- Return `acceptedPlan.approvedOptions` with the user's saved selected
IDs and option text on later wakes, including revision-pinned context.
Keep unchecked defaults out of scope.
- Add behavioral and safety regressions. Document the approval rule.

## Verification

- Three behavioral regressions failed before the repair: child creation
returned 403, decomposition returned 422, and native context returned
`acceptedPlan: null`. All three now pass.
- Two scope-retention regressions failed before the review fix: native
context omitted the selected option after a later approval, with and
without a pinned plan revision. Both now pass. Unchecked defaults stay
excluded.
- Current admission, native-tool, and accepted-confirmation replay
suites: 131 tests passed. Existing first-task completion safety checks:
17 passed, including the atomic decision recheck.
- Safety cases cover missing and rejected plan approval, agent-only
decisions, foreign source runs, changed revisions, invalid checkbox
selections, newer pending or rejected proposals, and task-creation
replay.
- Full repository `pnpm -r typecheck` and `pnpm build` passed locally on
final head `7c0e8ab784`.
- [Full final-head
CI](https://github.com/paperclipai/paperclip/actions/runs/41911580531)
passed: all Vitest shards, Runner checks, eight browser E2E shards,
build, release registry, and canary dry run. The required `verify` and
`e2e` aggregates passed. Storybook checks were skipped for this backend
change; the file-selection job was skipped because full CI ran.
- Earlier local typecheck attempts exposed Copilot verification during
build preparation, stale declarations across the rebase, and two
test-fixture typing errors. The assets were verified, declarations
rebuilt, and fixtures corrected before the complete final-head pass.
- Full local Vitest attempts were interrupted for the rebase and review
repair. The duplicate final local run was stopped after full Linux CI
passed. No complete macOS Vitest-suite pass is claimed.
- A fresh Claude-provider acceptance run remains unverified: the local
Claude account is signed out and no Anthropic API credential is
available. The actual native tool and HTTP paths are verified with real
PostgreSQL persistence. This PR does not deploy or recover the reported
production task.
- Greptile: 5/5 on `7c0e8ab784`, with the review thread resolved.

## Risks

A later acceptance must not revive rejected or changed scope. Admission
validates both current decisions and retains source, company, human
provenance, and document locks. The extra plan-proposal lookup is
bounded and limited to accepted, untargeted first-task proposals.
Ordinary delegation adds no new proof lookup. The native context adds an
optional `approvedOptions` projection. Revision-pinned context now
performs the existing scoped approval read to recover that selection. No
database schema, request, provider, or background worker changes are
included.

## Model Used

OpenAI Codex, exact model `gpt-6.1-sol`, max reasoning effort, with
repository, shell, browser, and GitHub tools. The current session
metadata confirms the model and effort. The context window size is 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
- [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 <noreply@paperclip.ing>
2026-11-04 15:24:58 -06:00

197 lines
11 KiB
TypeScript

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 } from "@paperclipai/shared";
import { badRequest, forbidden } from "../errors.js";
import { readPersistedConfirmationDecision } from "./confirmation-decision.js";
const runOriginFields = {
nativeIssueId: heartbeatRuns.nativeIssueId,
issueId: heartbeatRuns.issueId,
contextIssueId: sql<unknown>`${heartbeatRuns.contextSnapshot}->>'issueId'`,
contextTaskId: sql<unknown>`${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" },
);
type FirstTaskMutationInput = {
companyId: string;
actorAgentId?: string | null;
actorRunId?: string | null;
blockedByIssueIds?: readonly string[];
};
async function readAuthenticatedFirstTaskSources(tx: Db, input: FirstTaskMutationInput, lockSource: boolean, parentIssueId?: string | null) {
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 sourceIssueIds = [runIssueId(observedRun), parentIssueId].filter((id): id is string => Boolean(id));
if (!sourceIssueIds.length) return [];
// Let the UUID column compare identities, including PostgreSQL's accepted
// uppercase, unhyphenated and braced forms. Do not compare raw input strings.
const isRequestedBlocker = input.blockedByIssueIds?.length
? sql<boolean>`${inArray(issues.id, [...input.blockedByIssueIds])}`
: sql<boolean>`false`;
const query = tx.select({ id: issues.id, isRequestedBlocker }).from(issues).where(and(
inArray(issues.id, sourceIssueIds), eq(issues.companyId, input.companyId),
eq(issues.originKind, ONBOARDING_FIRST_TASK_ORIGIN_KIND),
));
return lockSource ? await query.for("update") : await query;
}
function assertNotBlockedByFirstTaskSource(source: { isRequestedBlocker: boolean } | undefined) {
if (source?.isRequestedBlocker) {
throw badRequest(
"First-task execution tasks cannot be blocked by their onboarding task. Remove the onboarding task from blockedByTaskIds (REST: blockedByIssueIds), preserve other required blockers, then use set_dependencies on the onboarding task to wait for the child.",
{ code: "onboarding_first_task_parent_blocker" },
);
}
}
/**
* 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: FirstTaskMutationInput): Promise<void> {
if (!input.actorAgentId || !input.actorRunId) return;
// Match the native authority/interaction lifecycle: issue, then run, then
// interaction/document. Ordinary tasks acquire no additional row locks.
const [source] = await readAuthenticatedFirstTaskSources(tx, input, true);
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();
if (await readOnboardingFirstTaskDecision(tx, { companyId: input.companyId, issueId: source.id, outcomes: ["accepted"] }) !== "accepted") throw acceptanceRequired();
assertNotBlockedByFirstTaskSource(source);
}
/** Blocker edits retain their own authorization; they do not consume a proposal. */
export async function assertOnboardingFirstTaskDependenciesAllowed(tx: Db, input: FirstTaskMutationInput & { parentIssueId: string | null }): Promise<void> {
if (!input.blockedByIssueIds?.length) return;
// The update owner already holds the actor-run authorization lock. A read is
// sufficient for source identity and the locked target's structural parent;
// do not lock a parent after a child. A child's own run has a different source
// but must still not wait on its server-owned onboarding parent.
const sources = await readAuthenticatedFirstTaskSources(tx, input, false, input.parentIssueId);
assertNotBlockedByFirstTaskSource(sources.find(source => source.isRequestedBlocker));
}
async function readLatestProposal(tx: Db, input: { companyId: string; issueId: string }, planOnly = false) {
// 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, input.issueId),
inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]),
sql`NOT (${issueThreadInteractions.payload} ?| ARRAY['toolAction', 'secretProposal', 'connectionAuthorization'])`,
sql`COALESCE(${issueThreadInteractions.payload}->'target'->>'type', '') <> 'custom'`,
planOnly ? sql`${issueThreadInteractions.payload}->'target'->>'type' = 'issue_document'
AND ${issueThreadInteractions.payload}->'target'->>'key' = 'plan'` : undefined,
)).orderBy(desc(issueThreadInteractions.createdAt), desc(issueThreadInteractions.id)).limit(1).for("share");
return proposal;
}
async function readHumanDecision(tx: Db, input: { companyId: string; issueId: string },
proposal: typeof issueThreadInteractions.$inferSelect | undefined) {
if (!proposal) return null;
const decision = readPersistedConfirmationDecision(proposal);
if (!decision) return null;
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) !== input.issueId || origin.agentId !== proposal.createdByAgentId) return null;
} 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) return null;
} else if (!proposal.createdByUserId) return null;
const directHumanDecision = Boolean(proposal.resolvedByUserId && !proposal.resolvedByAgentId && !proposal.resolvedByRunId);
if (!directHumanDecision) {
const commentId = decision.commentId;
if (!commentId || !proposal.resolvedByAgentId || !proposal.resolvedByRunId) return null;
const [comment] = await tx.select().from(issueComments).where(and(
eq(issueComments.id, commentId), eq(issueComments.companyId, input.companyId), eq(issueComments.issueId, input.issueId),
)).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") return null;
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) !== input.issueId
|| (resolver.responsibleUserId && resolver.responsibleUserId !== comment.authorUserId)) return null;
}
return decision;
}
/** Shared persisted-decision proof; creation and completion retain different authority. */
async function readOnboardingFirstTaskDecision(tx: Db, input: {
companyId: string; issueId: string; outcomes: Array<"accepted" | "rejected">;
}): Promise<"accepted" | "rejected" | null> {
const decision = await readHumanDecision(tx, input, await readLatestProposal(tx, input));
if (!decision || !input.outcomes.includes(decision.outcome)) return null;
// Refusal permits acknowledgement and closure, never execution. An agent-only
// plan edit cannot turn that human refusal into permission to execute.
if (decision.outcome === "rejected") return "rejected";
const planDecision = !decision.target
? await readHumanDecision(tx, input, await readLatestProposal(tx, input, true)) : null;
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, input.issueId), eq(issueDocuments.key, "plan"),
)).for("share", { of: documents });
const target = decision.target ?? (plan && planDecision?.outcome === "accepted" ? planDecision.target : null);
if (plan || target) {
if (!target || target.type !== "issue_document" || target.key !== "plan"
|| (target.issueId && target.issueId !== input.issueId) || !plan || plan.revisionId !== target.revisionId
|| (target.documentId && plan.documentId !== target.documentId)) return null;
}
return "accepted";
}
/** Completion may acknowledge a trusted refusal; it never authorizes rejected work. */
export async function onboardingFirstTaskCompletionAllowed(tx: Db, input: {
companyId: string; issueId: string; runId: string;
}): Promise<boolean> {
const [source] = await tx.select({ id: issues.id, originKind: issues.originKind }).from(issues).where(and(
eq(issues.id, input.issueId), eq(issues.companyId, input.companyId),
));
if (!source) return false;
if (source.originKind !== ONBOARDING_FIRST_TASK_ORIGIN_KIND) return true;
// Only the first-task path takes additional authority locks. At status commit
// these are held until the decision is written, in issue/run/proposal order.
const [lockedSource] = await tx.select({ id: issues.id }).from(issues).where(and(
eq(issues.id, source.id), eq(issues.companyId, input.companyId), eq(issues.originKind, ONBOARDING_FIRST_TASK_ORIGIN_KIND),
)).for("update");
if (!lockedSource) return false;
const [run] = await tx.select({ ...runOriginFields, runtimeMode: heartbeatRuns.runtimeMode }).from(heartbeatRuns).where(and(
eq(heartbeatRuns.id, input.runId), eq(heartbeatRuns.companyId, input.companyId),
)).for("share");
if (!run || run.runtimeMode !== "native" || run.nativeIssueId !== source.id) return false;
return await readOnboardingFirstTaskDecision(tx, { companyId: input.companyId, issueId: source.id, outcomes: ["accepted", "rejected"] }) !== null;
}