mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 17:51:58 +01:00
fix(runner): recognize verified task document evidence (#18661)
## Thinking Path > - Paperclip manages AI agents and keeps their results attached to tasks. > - Native tools return links to saved task documents. > - Completion feedback asks agents to include those links. > - The file-delivery validator treated every such relative link as a workspace path. > - A valid document and CSV result could therefore fail completion when it cited the supplied document link. > - This change derives document links from the same verified publication projection for guidance and validation. > - Requested files still need their own verified delivery evidence. ## Linked Issues or Issue Description **What happened?** The native document tool and completion feedback produce a link such as `/EX/issues/EX-1#document-output`. When a task also requests a downloadable CSV, citing that link causes completion to reject it as a workspace-only file. This occurs even when the document is published and the file has a valid current-run delivery receipt. **Expected behavior** Accept a server-derived document link only when it names the current company/task and a currently attached revision with a matching native publication receipt. Keep the separate requested-file requirement and all foreign, stale, missing, and workspace-file guards. **Steps to reproduce** 1. Save a task document through the native `write_document` tool. 2. Publish a CSV through `register_deliverable`. 3. Ask completion feedback for the final-response guidance. 4. Cite its returned document link alongside the file receipt in the next completion report. 5. The old validator rejects the valid document link as a workspace-only file. **Paperclip version or commit** Current source checkout. The regression exercises the real native document producer and completion validator. **Deployment mode** Local source verification on macOS arm64 with Node 24.22.1, pinned pnpm 9.15.4, locked dependencies, temporary PostgreSQL, and a controlled remote file-reader boundary. No live provider or hosted deployment is included. ## What Changed - Derive the canonical document href in the existing company/task/current-revision publication query. - Use that same verified href in completion guidance and reference validation. - Keep file registration and hash/size receipt requirements separate from document evidence. - Cover evidence, artifact, and criterion references; foreign companies/tasks; bogus keys; local paths; and lost current-revision publication proof. - Exercise actual native document writes and CSV registration, including attachment download query and fragment links. - Update the execution contract and the document feature/access maps. ## Verification - Red before the production edit: native completion generated a real saved-document link, then rejected the same link alongside a valid published file. The failure was the workspace-only-file guard. - Green: 210 tests across native completion feedback, explicit output validation, and the existing native file-handoff suite pass. These use real PostgreSQL and native tool receipts; the remote file reader is controlled. - A saved document link alone still fails the explicit CSV requirement. A document whose current revision lacks its publication receipt is rejected. Foreign task/company links, missing keys, malformed links, and workspace paths remain rejected. - Full workspace typecheck, server typecheck, and production build pass. - `node scripts/check-maps.mjs`, `node scripts/check-module-boundaries.mjs`, and `git diff --check` pass. Map checks establish structure only. - Full `pnpm test:run` is running separately. There is no full-suite verdict yet. CI and review are pending. - Hosted acceptance and any recovery of existing tasks remain separate steps. This PR does not change final-response recovery or scheduling. ## Risks - An overly broad URL exception could admit foreign or unpublished output. The validator accepts only exact hrefs from the authenticated publication projection. - Document evidence could hide a missing requested file. A separate regression keeps that delivery gate effective. - The shared projection adds current task/company joins. Existing document feedback and publication tests cover those consumers. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with reasoning, code editing, shell execution, and automated tests. The context-window size is not exposed in this session. ## 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 - [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>
This commit is contained in:
1 parent
aa6c9c3f25
commit
cd12432947
6 files changed
+89
-14
No files matched your search
@@ -2480,6 +2480,13 @@ accessible repository work products do not require an attachment. Publication
|
||||
failure calls for continued work or a concrete blocker, not a human confirmation
|
||||
that the task is complete.
|
||||
|
||||
Native completion reports can cite the relative task-document links returned by
|
||||
the server. Each link must resolve to this company's current task and an attached
|
||||
document whose current revision has a matching native publication receipt.
|
||||
The same server-owned link projection supplies completion guidance and evidence
|
||||
validation. A published document link can accompany a requested file, but it
|
||||
does not replace that file's verified delivery evidence.
|
||||
|
||||
For an explicit file output in the current request, an empty report, a
|
||||
verification-only reference, or an unregistered URL cannot satisfy delivery.
|
||||
The report must cite an attachment verified by the current run's durable
|
||||
|
||||
@@ -44,6 +44,14 @@ CLI / MCP.
|
||||
Policy: Task/resource visibility, mutation/checkout ownership, revision/lock guards and
|
||||
permitted deliverable context.
|
||||
|
||||
Native completion evidence accepts the server's relative link only for a document
|
||||
attached to the current company/task with a matching current-revision publication
|
||||
receipt. It can accompany a separately verified requested file; it cannot replace
|
||||
that file. The [native feedback regression](../../server/src/services/native-runtime/native-completion-feedback.test.ts)
|
||||
exercises the native write/register/finish boundary with local PostgreSQL and a
|
||||
controlled remote file reader. It does not qualify the whole document API or a
|
||||
live provider run.
|
||||
|
||||
Evidence: `source-only` — [implementation](../../server/src/routes/issues.ts#L10299) at
|
||||
the reviewed commit above.
|
||||
|
||||
|
||||
@@ -94,6 +94,13 @@ Manual: Edit a paragraph, save, reload, and compare revisions. Add an annotation
|
||||
|
||||
Automated: [attachment routes](../../server/src/__tests__/issue-attachment-routes.test.ts) and [work products](../../server/src/__tests__/work-products.test.ts) cover data/access behavior; [artifact arrival browser suite](../../tests/e2e/artifact-tab-arrival.spec.ts) targets arrival. [Artifact uploads](../../tests/e2e/artifact-uploads.spec.ts) drives file selection/drop, filename collisions, retry, and preservation of paused/unassigned tasks.
|
||||
|
||||
[Native completion feedback](../../server/src/services/native-runtime/native-completion-feedback.test.ts)
|
||||
checks a real saved-document link alongside a verified CSV attachment, including
|
||||
attachment download query and fragment links. It rejects foreign task/company
|
||||
links, missing documents, unproven current revisions, and workspace files. The
|
||||
document alone cannot satisfy the requested file. These local tool/Postgres
|
||||
checks do not establish hosted delivery or browser rendering.
|
||||
|
||||
Manual: Upload a small Markdown file and CSV through Artifacts. Markdown becomes
|
||||
a versioned task document; other files become attachments. Read the document,
|
||||
download the CSV and compare bytes, then reload. Upload the same Markdown filename
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { agents, companies, createDb, documentRevisions, executionWorkspaces, executionWorkspaceRepositories, heartbeatRuns, issues, issueThreadInteractions, issueTreeHolds, issueDocuments } from "@paperclipai/db";
|
||||
@@ -19,11 +19,11 @@ describe("native final-response feedback", () => {
|
||||
let db: ReturnType<typeof createDb>;
|
||||
beforeAll(async () => { temporary = await startRecoveryPostgres("native-final-response-"); db = createDb(temporary.connectionString); });
|
||||
afterAll(async () => { await temporary?.cleanup(); });
|
||||
async function fixture() {
|
||||
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID();
|
||||
await db.insert(companies).values({ id: companyId, name: "Feedback", issuePrefix: `FB${companyId.slice(0, 8).toUpperCase()}` });
|
||||
async function fixture(existingCompanyId?: string) {
|
||||
const companyId = existingCompanyId ?? randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID();
|
||||
if (!existingCompanyId) await db.insert(companies).values({ id: companyId, name: "Feedback", issuePrefix: `FB${companyId.slice(0, 8).toUpperCase()}` });
|
||||
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "paperclip_runner", status: "active" });
|
||||
await db.insert(issues).values({ id: issueId, companyId, identifier: `FB${companyId.slice(0, 8).toUpperCase()}-1`, title: "Save the task document", status: "in_progress", assigneeAgentId: agentId });
|
||||
await db.insert(issues).values({ id: issueId, companyId, identifier: `FB${companyId.slice(0, 8).toUpperCase()}-${existingCompanyId ? 2 : 1}`, title: "Save the task document", status: "in_progress", assigneeAgentId: agentId });
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, nativeIssueId: issueId, status: "running", runtimeMode: "native", contextSnapshot: { issueId } });
|
||||
await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, issueId));
|
||||
const authority = new PaperclipRunnerToolAuthority(db, { companyId, agentId, issueId, runId });
|
||||
@@ -32,6 +32,17 @@ describe("native final-response feedback", () => {
|
||||
} }) as { document: { id: string; latestRevisionId: string }; documentHref: string };
|
||||
return { companyId, agentId, issueId, runId, saved };
|
||||
}
|
||||
async function publishedFileReport(value: Awaited<ReturnType<typeof fixture>>): Promise<PrpStructuredRunResult> {
|
||||
await db.update(issues).set({ description: "Save a document on this task. Export the results as a CSV." }).where(eq(issues.id, value.issueId));
|
||||
const bytes = Buffer.from("category,total\nverified,3\n");
|
||||
const authority = new PaperclipRunnerToolAuthority(db, { companyId: value.companyId, agentId: value.agentId,
|
||||
issueId: value.issueId, runId: value.runId, workspaceRoot: "/workspace/test", executionTargetKind: "remote", readRemoteTaskFile: async () => bytes });
|
||||
const receipt = await authority.execute({ tool: "register_deliverable", callId: "csv", arguments: {
|
||||
idempotencyKey: "csv", filename: "results.csv", contentType: "text/csv", contentRef: "results.csv",
|
||||
byteSize: bytes.length, sha256: createHash("sha256").update(bytes).digest("hex"), title: "Results CSV",
|
||||
} }) as { downloadPath: string };
|
||||
return { ...done, evidence: [{ ref: `${receipt.downloadPath}#download` }] };
|
||||
}
|
||||
it("rejects a duplicate approval report for the exact pending tool-action card", async () => {
|
||||
const value = await fixture(), interactionId = randomUUID();
|
||||
await db.insert(issueThreadInteractions).values({ id: interactionId, companyId: value.companyId, issueId: value.issueId,
|
||||
@@ -211,6 +222,44 @@ describe("native final-response feedback", () => {
|
||||
expect(feedback).toContain("When compatible with the requested response format");
|
||||
expect(feedback).not.toContain("publish secrets");
|
||||
});
|
||||
it.each(["evidence", "artifacts", "criterion"] as const)("accepts its generated saved-document link in %s alongside a published file", async location => {
|
||||
const value = await fixture();
|
||||
const report = await publishedFileReport(value);
|
||||
const feedback = await nativeCompletionFeedback(db, value.runId, report);
|
||||
const href = /\[Saved document\]\(([^)]+)\)/u.exec(feedback)?.[1];
|
||||
expect(href).toBe(value.saved.documentHref);
|
||||
const referenced = location === "evidence" ? { ...report, evidence: [...report.evidence, { ref: href! }] }
|
||||
: location === "artifacts" ? { ...report, artifacts: [{ kind: "document", ref: href! }] }
|
||||
: { ...report, completionClaim: { ...report.completionClaim, criteria: [{ criterionId: "document", status: "satisfied" as const, evidenceRefs: [href!] }] } };
|
||||
await expect(nativeCompletionFeedback(db, value.runId, referenced))
|
||||
.resolves.toContain("Completion report accepted");
|
||||
});
|
||||
it("retains foreign task, foreign company, bogus link, and local file guards", async () => {
|
||||
const current = await fixture(), otherTask = await fixture(current.companyId), otherCompany = await fixture();
|
||||
const report = await publishedFileReport(current);
|
||||
for (const ref of [otherTask.saved.documentHref, otherCompany.saved.documentHref,
|
||||
current.saved.documentHref.replace("#document-output", "#document-missing"),
|
||||
current.saved.documentHref.replace("/issues/", "/not-issues/"),
|
||||
"//files.example.test/results.csv", "/tmp/results.csv", "results.csv"]) {
|
||||
await expect(nativeCompletionFeedback(db, current.runId, { ...report, evidence: [...report.evidence, { ref }] }))
|
||||
.rejects.toThrow("workspace-only file");
|
||||
}
|
||||
});
|
||||
it("does not accept the link after its current revision loses publication proof", async () => {
|
||||
const value = await fixture(), report = await publishedFileReport(value);
|
||||
const referenced = { ...report, evidence: [...report.evidence, { ref: value.saved.documentHref }] };
|
||||
await expect(nativeCompletionFeedback(db, value.runId, referenced)).resolves.toContain("Completion report accepted");
|
||||
await documentService(db).upsertIssueDocument({ format: "markdown", issueId: value.issueId, key: "output", title: "Replacement",
|
||||
body: "Revision without a matching native publication receipt", baseRevisionId: value.saved.document.latestRevisionId,
|
||||
createdByAgentId: value.agentId, createdByRunId: value.runId });
|
||||
await expect(nativeCompletionFeedback(db, value.runId, referenced)).rejects.toThrow("requested task document is only a workspace file");
|
||||
});
|
||||
it("does not use a published document link as the requested downloadable file", async () => {
|
||||
const value = await fixture();
|
||||
await db.update(issues).set({ description: "Save a document on this task. Export the results as a CSV." }).where(eq(issues.id, value.issueId));
|
||||
await expect(nativeCompletionFeedback(db, value.runId, { ...done, evidence: [{ ref: value.saved.documentHref }] }))
|
||||
.rejects.toThrow("no accessible delivery evidence");
|
||||
});
|
||||
it("does not link stale saved revisions", async () => {
|
||||
const value = await fixture();
|
||||
await documentService(db).upsertIssueDocument({ format: "markdown", issueId: value.issueId, key: "output", title: "Updated", body: "New version",
|
||||
|
||||
@@ -17,7 +17,6 @@ import { and, eq, inArray, notInArray } from "drizzle-orm";
|
||||
import {
|
||||
approvals,
|
||||
agents,
|
||||
companies,
|
||||
heartbeatRuns,
|
||||
completionContracts,
|
||||
issueApprovals,
|
||||
@@ -40,10 +39,7 @@ function record(value: unknown): Record<string, unknown> {
|
||||
/** Reconstruct links from current task state, never from provider-supplied URLs. */
|
||||
async function savedDocumentLinks(db: Db, run: typeof heartbeatRuns.$inferSelect, issue: typeof issues.$inferSelect) {
|
||||
const saved = await publishedTaskDocuments(db, { companyId: run.companyId, issueId: issue.id });
|
||||
if (!saved.length) return [];
|
||||
const [company] = await db.select({ issuePrefix: companies.issuePrefix }).from(companies).where(eq(companies.id, run.companyId));
|
||||
if (!company) return [];
|
||||
return saved.map(document => `[Saved document](/${encodeURIComponent(company.issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}#document-${encodeURIComponent(document.key)})`);
|
||||
return saved.map(document => `[Saved document](${document.href})`);
|
||||
}
|
||||
|
||||
/** Read current constraints before accepting the report, not a premature status commit. */
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { assets, documents, documentRevisions, heartbeatRuns, issueDocuments, issueAttachments, issueWorkProducts, mcpAttachmentUploads, dotRunnerAssignments, type Db } from "@paperclipai/db";
|
||||
import { assets, companies, documents, documentRevisions, heartbeatRuns, issueDocuments, issueAttachments, issues, issueWorkProducts, mcpAttachmentUploads, dotRunnerAssignments, type Db } from "@paperclipai/db";
|
||||
import type { PrpStructuredRunResult } from "../../vendor/paperclip-runner/index.js";
|
||||
|
||||
function evidenceRefs(value: unknown): string[] {
|
||||
@@ -184,8 +184,11 @@ export function explicitlyRequestsTaskDocumentOutput(objective: string): boolean
|
||||
*/
|
||||
export async function publishedTaskDocuments(db: Db, binding: { companyId: string; issueId: string }) {
|
||||
const saved = await db.select({ id: documents.id, revisionId: documents.latestRevisionId,
|
||||
key: issueDocuments.key, resultJson: heartbeatRuns.resultJson })
|
||||
key: issueDocuments.key, resultJson: heartbeatRuns.resultJson,
|
||||
issuePrefix: companies.issuePrefix, issueIdentifier: issues.identifier })
|
||||
.from(issueDocuments)
|
||||
.innerJoin(issues, and(eq(issues.id, issueDocuments.issueId), eq(issues.companyId, binding.companyId)))
|
||||
.innerJoin(companies, eq(companies.id, issues.companyId))
|
||||
.innerJoin(documents, and(eq(documents.id, issueDocuments.documentId), eq(documents.companyId, binding.companyId)))
|
||||
.innerJoin(documentRevisions, and(eq(documentRevisions.id, documents.latestRevisionId),
|
||||
eq(documentRevisions.documentId, documents.id), eq(documentRevisions.companyId, binding.companyId)))
|
||||
@@ -197,7 +200,8 @@ export async function publishedTaskDocuments(db: Db, binding: { companyId: strin
|
||||
const receipt = record(value), result = record(receipt.result), published = record(result.document);
|
||||
return receipt.operationId === "write_document" && ["applied", "duplicate"].includes(String(result.disposition))
|
||||
&& published.id === document.id && published.latestRevisionId === document.revisionId;
|
||||
})).map(({ id, revisionId, key }) => ({ id, revisionId, key }));
|
||||
})).map(({ id, revisionId, key, issuePrefix, issueIdentifier }) => ({ id, revisionId, key,
|
||||
href: `/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issueIdentifier ?? binding.issueId)}#document-${encodeURIComponent(key)}` }));
|
||||
}
|
||||
|
||||
/** Files cited as completed output must be reachable outside the agent workspace. */
|
||||
@@ -209,17 +213,21 @@ export async function validateNativeDeliverableEvidence(
|
||||
if (result.reportedWorkDisposition !== "done") return;
|
||||
const fileRequested = explicitlyRequestsFileOutput(binding.objective);
|
||||
const taskDocumentRequested = explicitlyRequestsTaskDocumentOutput(binding.objective);
|
||||
const publishedTaskDocument = taskDocumentRequested && (await publishedTaskDocuments(db, binding)).length > 0;
|
||||
const artifactRefs = new Set(evidenceRefs(result.artifacts));
|
||||
const refs = new Set([
|
||||
...evidenceRefs(result.evidence),
|
||||
...artifactRefs,
|
||||
...result.completionClaim.criteria.flatMap(({ evidenceRefs }) => evidenceRefs),
|
||||
]);
|
||||
const citesTaskDocument = [...refs].some(ref => /^\/[^/?#]+\/issues\/[^/?#]+#document-[^/?#]+$/u.test(ref.trim()));
|
||||
const publishedDocuments = taskDocumentRequested || citesTaskDocument ? await publishedTaskDocuments(db, binding) : [];
|
||||
const publishedTaskDocument = taskDocumentRequested && publishedDocuments.length > 0;
|
||||
const documentHrefs = new Set(publishedDocuments.map(document => document.href));
|
||||
let registeredAttachment = false;
|
||||
for (const value of refs) {
|
||||
if (typeof value !== "string") continue;
|
||||
const ref = value.trim();
|
||||
if (documentHrefs.has(ref)) continue;
|
||||
const attachmentPath = /^\/api\/attachments\/([^/?#]+)\/content(?:[?#].*)?$/u.exec(ref);
|
||||
if (ref.startsWith("deliverable:") || attachmentPath) {
|
||||
const id = attachmentPath?.[1] ?? ref.slice("deliverable:".length);
|
||||
|
||||
Reference in new issue
Block a user