From 0f84df91daee64c86dbfaca44fdd54b86c6507d3 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 9 Nov 2026 09:44:17 -0800 Subject: [PATCH] fix(native): accept a completed child's file in the parent's completion report (#20325) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In onboarding, the agent plans the user's first task, creates one child task for the work, and waits for it > - The `first-task` skill tells the agent to publish a requested file on the child task. When the parent wakes, the agent links the child's file and does not publish a copy > - Native `register_deliverable` is bound to its current task, so the parent cannot register the child's file again > - The native completion check required each cited attachment to be on the task being completed > - So when the parent cited the child's download link, `paperclip_finish` failed, and the agent reported the finished onboarding task as blocked > - This pull request lets a completion report cite an attachment on a direct child task that is done > - The benefit is that onboarding closes after its child task delivers a file, and the user does not see a false "blocked" status ## Linked Issues or Issue Description Related: #18258 (onboarding artifact completion instructions). **What happened?** An onboarding run created a child task to build and publish a project. The child task registered `project.zip` with `register_deliverable` and was marked done. The parent woke with reason `issue_children_completed`. The agent called `paperclip_finish` with `reportedWorkDisposition: "done"` and cited the child's `/api/attachments//content` link. The call failed two times with: ``` Completion cites no registered attachment on this task. Use register_deliverable for the requested file and cite deliverable: from its receipt. ``` The skill does not let the agent register the file on the parent, so the agent called `paperclip_block` with "Paperclip completion validation" as the owner. The parent task stayed `blocked`. Nothing wakes it again. **Expected behavior** The parent completes and links the child's file. **Steps to reproduce** 1. Create a task with a child task. 2. In a native run on the child, register a file with `register_deliverable`, and mark the child done. 3. In a native run on the parent, call `paperclip_finish` with `reportedWorkDisposition: "done"` and the child's `deliverable:` or `/api/attachments//content` as evidence. 4. The call fails with "Completion cites no registered attachment on this task". ## What Changed - `server/src/services/native-runtime/native-deliverable-feedback.ts`: when a cited attachment is not on the task, `validateNativeDeliverableEvidence` also looks for it on a direct child task with status `done`. If it is there, the citation is accepted. - The child's file is context, the same as an attachment from an earlier run on the same task. It does not count as this run's publication of a file that the parent's own request names. - With `lockSources`, the finalization path locks the attachment and child task rows with `FOR SHARE NOWAIT`, as it does for other sources. - The error text now says "on this task or its completed child tasks". - This applies to the `paperclip_finish` check and to the two finalization checks, because they use the same function. - `doc/AGENT-ARTIFACTS.md`: the native runner section states that a completion report can cite a file on a completed direct child task. ## Verification - Extended "prepares one verified same-run attachment and replays without duplicates" in `server/src/services/native-runtime/native-runner-file-handoff.test.ts`: - An attachment on an unrelated task is still rejected. - An attachment on a child task that is not done is still rejected. - An attachment on a done child task is accepted, through `paperclip_finish` and through the locked finalization path. - When the parent's own request names a file, the child's file alone does not satisfy it. - Without the source change, the test fails with the same error as the run above. - `npx vitest run` on `chat-attachment-reuse`, `native-completion-feedback`, `native-completion-format-feedback`, `native-deliverable-feedback`, and `native-runner-file-handoff` tests in `server/src/services/native-runtime`: 281 passed. - `npx vitest run src/__tests__/native-result-commit-work.test.ts src/__tests__/native-status-arbiter-corpus.test.ts` in `server`: 167 passed. - `npx tsc --noEmit -p server/tsconfig.json` passes. ## Risks - Only direct children with status `done` are accepted. A grandchild's file, or a file on a child that is not done, is still rejected. - A parent that names a requested file in its own request still needs its own delivery evidence. This keeps the current rule for earlier outputs. - A task that is already blocked by this error does not change. The user can mark it done, or comment so the agent runs again. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (code reading, database queries, shell commands, and test runs). ## 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 - [ ] All Paperclip CI gates are green - [ ] 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/AGENT-ARTIFACTS.md | 6 ++++++ .../native-deliverable-feedback.ts | 17 ++++++++++++++- .../native-runner-file-handoff.test.ts | 21 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/doc/AGENT-ARTIFACTS.md b/doc/AGENT-ARTIFACTS.md index f5c12fdaf9c..cb87f4d8bb9 100644 --- a/doc/AGENT-ARTIFACTS.md +++ b/doc/AGENT-ARTIFACTS.md @@ -27,6 +27,12 @@ remote workspace. Supply a workspace-relative `contentRef`, basename `filename`, The tool verifies the file, stores an attachment and artifact work product, and binds it to the response. Generic API tools and a legacy API key are unnecessary. +`register_deliverable` stores the file on its current task. A completion report +can also cite a file on a direct child task that is done, because the child's +own completion verified that file. A parent that delegated a file to a child +links the child's file and does not register a copy. The child's file does not +count as delivery of a file that the parent's own request names. + Named JSON file outputs and explicit publication requests require the same verified delivery evidence as other file outputs. A saved summary document does not replace a requested downloadable JSON file. Reading JSON inputs or discussing diff --git a/server/src/services/native-runtime/native-deliverable-feedback.ts b/server/src/services/native-runtime/native-deliverable-feedback.ts index bad131e5af9..ba36256ccd5 100644 --- a/server/src/services/native-runtime/native-deliverable-feedback.ts +++ b/server/src/services/native-runtime/native-deliverable-feedback.ts @@ -324,6 +324,20 @@ export async function publishedTaskDocuments(db: Db, binding: { companyId: strin href: `/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issueIdentifier ?? binding.issueId)}#document-${encodeURIComponent(key)}` })); } +/** A parent that delegated a file to a child links the child's delivered file + * instead of publishing a copy, because native register_deliverable is bound to + * its current task. A completed direct child's attachment is a valid citation, + * but not this run's publication of a file that the parent itself requests. + */ +async function completedChildAttachment(db: Db, binding: { companyId: string; issueId: string; lockSources?: boolean }, id: string): Promise { + const query = db.select({ id: issueAttachments.id }).from(issueAttachments) + .innerJoin(issues, and(eq(issues.id, issueAttachments.issueId), eq(issues.companyId, binding.companyId), + eq(issues.parentId, binding.issueId), eq(issues.status, "done"))) + .where(and(eq(issueAttachments.id, id), eq(issueAttachments.companyId, binding.companyId))).limit(1); + const [attachment] = await (binding.lockSources ? query.for("share", { of: [issueAttachments, issues], noWait: true }) : query); + return attachment !== undefined; +} + /** Files cited as completed output must be reachable outside the agent workspace. */ export async function validateNativeDeliverableEvidence( db: Db, @@ -358,7 +372,8 @@ export async function validateNativeDeliverableEvidence( const [attachment] = uuid.test(id) ? await (binding.lockSources ? query.for("share", { of: [issueAttachments, assets], noWait: true }) : query) : []; if (!attachment) { - throw new NativeDeliverableEvidenceError("Completion cites no registered attachment on this task. Use register_deliverable for the requested file and cite deliverable: from its receipt. No human completion approval was created."); + if (uuid.test(id) && await completedChildAttachment(db, binding, id)) continue; + throw new NativeDeliverableEvidenceError("Completion cites no registered attachment on this task or its completed child tasks. Use register_deliverable for the requested file and cite deliverable: from its receipt. No human completion approval was created."); } // A prior output (or user input) can be useful context, but does not prove // this run published the newly requested output. The receipt survives a diff --git a/server/src/services/native-runtime/native-runner-file-handoff.test.ts b/server/src/services/native-runtime/native-runner-file-handoff.test.ts index a01a17d0dd7..e7c166fd71c 100644 --- a/server/src/services/native-runtime/native-runner-file-handoff.test.ts +++ b/server/src/services/native-runtime/native-runner-file-handoff.test.ts @@ -32,6 +32,7 @@ import { issueComments, issues, issueWorkProducts, + type Db, } from "@paperclipai/db"; import { startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js"; @@ -45,6 +46,7 @@ import { stageNativeRunnerAttachmentBytes, stageNativeRunnerWakeAttachments, } from "./native-runner-file-handoff.js"; +import { validateNativeDeliverableEvidence } from "./native-deliverable-feedback.js"; import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.js"; describe("native runner file handoff", () => { @@ -461,7 +463,26 @@ describe("native runner file handoff", () => { try { await expect(nativeCompletionFeedback(db, runId, doneReport([`deliverable:${first.entityRefs[0]}`]))) .rejects.toThrow("registered attachment on this task"); + // A parent links the file its completed child delivered. It cannot + // register that file again, because register_deliverable is task-bound. + await db.update(issues).set({ parentId: issueId }).where(eq(issues.id, otherIssueId)); + await expect(nativeCompletionFeedback(db, runId, doneReport([`deliverable:${first.entityRefs[0]}`]))) + .rejects.toThrow("registered attachment on this task"); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, otherIssueId)); + await db.update(issues).set({ title: "Review the delegated work" }).where(eq(issues.id, issueId)); + await expect(nativeCompletionFeedback(db, runId, doneReport([`/api/attachments/${first.entityRefs[0]}/content`]))) + .resolves.toContain("Completion report accepted"); + await expect(db.transaction(tx => validateNativeDeliverableEvidence(tx as unknown as Db, { + companyId, issueId, runId, objective: "Review the delegated work", semanticToolReceipts: {}, + lockSources: true, authoritativeDone: true, + }, doneReport([`deliverable:${first.entityRefs[0]}`])))).resolves.toBeUndefined(); + // The child's file is context, not this run's publication of a file the + // parent's own request names. + await db.update(issues).set({ title: "Prepare a requested file" }).where(eq(issues.id, issueId)); + await expect(nativeCompletionFeedback(db, runId, doneReport([`deliverable:${first.entityRefs[0]}`]))) + .rejects.toThrow("no accessible delivery evidence"); } finally { + await db.update(issues).set({ title: "Prepare a requested file" }).where(eq(issues.id, issueId)); await db.update(issueAttachments).set({ issueId }).where(eq(issueAttachments.id, first.entityRefs[0])); await db.delete(issues).where(eq(issues.id, otherIssueId)); }