Files
PaperClipAI/server/src/services/onboarding-first-task-admission.ts
T
DottaandPaperclip c32e1ba6da fix(onboarding): keep approved child task handoffs runnable (#18160)
## Thinking Path

> - Paperclip manages work through tasks, agent runs, dependencies, and
human decisions.
> - The first-task workflow asks for approval before it creates an
execution task.
> - File-producing work must run on a child task before onboarding can
finish.
> - A child blocked by its onboarding source cannot start while the
source waits for that child.
> - This change rejects that dependency during task creation and later
edits, and gives the agent a correction.
> - The shipped recipe shows the dependency direction and requires the
actual approved inputs in the child instructions.

## Linked Issues or Issue Description

**What happened?**

After human approval, an onboarding run could create a child with the
onboarding source in `blockedByTaskIds`. Creation saved a blocked child
with no wake. The parent could then wait for that child. A handoff could
also refer to input from the parent conversation without including the
input in the child description.

**Expected behavior**

The approved child can execute before its waiting parent. Invalid
first-task dependency input fails before task creation and leaves the
same command key available for a corrected call. The child instructions
include the required approved input and constraints.

**Steps to reproduce**

1. Accept a proposal on a server-created first task.
2. Call native `create_task` with `status: "todo"` and the onboarding
source in `blockedByTaskIds`.
3. Observe a blocked child with no scheduled wake. Block the parent on
that child to expose the stalled handoff.

**Paperclip version or commit**

The regression reproduces on `f8484a9f8d1f68e9534fee4f29e00e716dc680f4`.
The repair is based on current master.

**Deployment mode**

Core native and REST task creation. The same server boundary serves
self-hosted and hosted instances.

Related: #18080 fixed remote delivery of the selected first-task skill.
This change handles the later child handoff boundary.

## What Changed

- Reject a task blocked by its authenticated first-task source during
creation and later dependency edits. Later agent edits also check the
locked target's server-owned onboarding parent, protecting a child
running under its own execution. Compare UUID identity in PostgreSQL so
alternate supported UUID spellings cannot bypass the check. The error
explains the correction.
- Add a native child/dependency recipe with self-contained approved
input guidance. Clarify the general `create_task` input descriptions and
update their generated contracts.
- Cover native and REST rejection, corrected retry, preserved valid
dependencies and replay, and an executable recipe with exact input bytes
read from the child's own context. Thread the authenticated run through
native dependency edits. Keep blocker corrections independent of a new
proposal decision.
- Update execution semantics and the existing onboarding map entries.

## Verification

- Red on `f8484a9f`: the new native regression received a created
blocked child instead of the expected rejection.
- Review regressions were red on the original PR head: uppercase,
unhyphenated and braced source UUIDs created blocked children; a later
child PATCH returned 200 and saved the reversed blocker.
- Review follow-up reproduced both native and REST child-own-run
bypasses on `75fa75a6`: the native dependency call applied the reverse
edge; the child's own PATCH returned 200. The regression uses the actual
authenticated child execution and an already checked-out task.
- All 95 tests across first-task admission, assets, and the full native
tool-authority suite pass. The shipped recipe is validated against the
native schemas and exercised through real task/dependency services with
disposable PostgreSQL. Rejected PATCH tests preserve existing
authorization and attempt-audit behavior and require no requested
task-field, relation or wake change. Compatibility controls retain
ordinary manual parent blockers, explicit board edits, other blockers
and empty-list corrections.
- Runner TypeScript build, generated contracts, capability inventory,
protocol manifest, and map checks passed.
- Server typecheck and all 95 focused tests pass with the child-run
follow-up. All latest checks pass on `f24509ae`: 55 successful checks,
one neutral review check, four skipped checks, and a successful legacy
security status. Both automated reviews are 5/5 on this exact commit.
- Prior-head CI failures are preserved: the Runner close fixture had
unconfirmed provider drain while suspension succeeded; its exact
fake-provider subprocess row passed locally with unchanged sources. The
browser outage test lost its title textarea before any outage write. A
bounded local control with a fully loaded composer passed every original
outage assertion. The prior failure is an editor-readiness race; its
exact asynchronous ordering remains unproved. No UI, browser-test, or
timeout change is included. Neither failure is counted as a pass or
silently discarded.
- The first `f24509ae` CI attempt had two interrupted jobs: Typecheck
and General server 9/12. Their logs record runner shutdown signals. The
server job also records lost runner communication. The same-head
failed-job rerun passed both jobs and the required aggregate. It reused
the other successful results. No source, test, or timeout change was
made for the rerun. The failed attempt remains preserved. Full
repository checks pass in CI; the local full repository typecheck, test,
and build commands were not repeated.
- No external model or provider calls are used by these checks.
Real-provider acceptance at this revision remains separate.

## Risks

- The new rejection uses the authenticated run and the server-owned
onboarding origin. Ordinary tasks, other blockers, empty-list
corrections, and already committed creation replay retain their existing
behavior. Blocker edits do not reapply the creation approval gate.
- Nonempty blocker edits perform two indexed identity reads; the task
lookup covers at most the run source and locked structural parent. Other
patches and empty-list corrections add no source lookup. The update
guard adds no parent row lock behind a child. Existing REST
authorization checkout and attempt auditing remain in place before the
domain mutation guard.
- The explicit input recipe guides the model. Schema and persistence
tests do not prove that a future model follows every instruction.
- The tool guidance changes the generated semantic catalog identity. The
maintained generated files are synchronized in this change.

## Model Used

OpenAI `gpt-6.1-sol` through Codex, with `ultra` reasoning effort,
repository tools, code generation, and test execution. The context
window is not exposed by the runtime.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used and stated the available runtime
detail
- [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 described the issue in-PR following the bug issue template
- [x] I have not referenced internal or instance-local Paperclip issues
or links
- [x] My branch name describes the change and contains no internal
ticket ID
- [x] I have run focused 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 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-10-29 10:57:31 -05:00

201 lines
12 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, requestCheckboxConfirmationPayloadSchema, requestCheckboxConfirmationResultSchema,
requestConfirmationPayloadSchema, requestConfirmationResultSchema } from "@paperclipai/shared";
import { badRequest, forbidden } from "../errors.js";
const object = (value: unknown): Record<string, unknown> =>
value && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
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));
}
/** 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> {
// 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'`,
)).orderBy(desc(issueThreadInteractions.createdAt), desc(issueThreadInteractions.id)).limit(1).for("share");
if (!proposal || (proposal.status !== "accepted" && proposal.status !== "rejected") ||
!input.outcomes.includes(proposal.status) || !proposal.resolvedAt) return null;
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 !== proposal.status) return null;
if (checkbox && proposal.status === "accepted") {
const selected = object(result.data).selectedOptionIds;
if (!("options" in payload.data) || !Array.isArray(selected) || selected.length === 0) return null;
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)) 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 = result.data.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;
}
// Refusal permits acknowledgement and closure, never execution. An agent-only
// plan edit cannot turn that human refusal into permission to execute.
if (proposal.status === "rejected") return "rejected";
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, input.issueId), 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 !== 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;
}