mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
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/<id>/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:<attachmentId> 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:<attachmentId>` or `/api/attachments/<attachmentId>/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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
47dccbf3eb
commit
0f84df91da
3 files changed
+43
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<boolean> {
|
||||
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:<attachmentId> 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:<attachmentId> 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
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user