From b28422b2930cfebe00448e0d102c6928aea5c45f Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 3 Oct 2026 02:52:49 -0500 Subject: [PATCH] Bind completion file requirements to each creation verb Co-authored-by: Paperclip --- .../2026-10-02-pi-production-readiness.md | 9 ++- .../native-deliverable-feedback.test.ts | 2 + .../native-deliverable-feedback.ts | 64 +++++++++++-------- .../native-runner-file-handoff.test.ts | 2 + 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/doc/plans/2026-10-02-pi-production-readiness.md b/doc/plans/2026-10-02-pi-production-readiness.md index 77cb1d23eca..f6911d1d6af 100644 --- a/doc/plans/2026-10-02-pi-production-readiness.md +++ b/doc/plans/2026-10-02-pi-production-readiness.md @@ -413,9 +413,12 @@ prices are estimates. Unpriced usage must stay unpriced in the ledger and UI. actual downloadable output requirements, mixed requests, and publication evidence enforcement. The first correction passes 59 tests, but fresh review identifies two mixed-output publication bypasses. Restrict the internal - qualifier to the last clause and a denied attempt to its sole creation verb - and file object. All 64 unit/database integration tests pass, including both - review examples. A free replay of the exact + qualifier and denied-attempt scope; 64 tests pass. At `1a8900958`, fresh review + identifies two valid instruction variants that this narrowing still rejects. + Bind each file object to its own creation verb instead. An internal qualifier + applies only to a single requested file across the preceding sentence, even + if a later clause checks it. All 68 unit/database integration tests pass, + including both publication bypasses and both internal/denied variants. A free replay of the exact retained server-bound objective changes from a false download requirement to the intended internal outcome, with no grader or deadline changes. - The `0d65753fe` broad local suite is terminal with 736 passed files, four diff --git a/server/src/services/native-runtime/native-deliverable-feedback.test.ts b/server/src/services/native-runtime/native-deliverable-feedback.test.ts index 4bddba4fbcf..3d2d2b35a91 100644 --- a/server/src/services/native-runtime/native-deliverable-feedback.test.ts +++ b/server/src/services/native-runtime/native-deliverable-feedback.test.ts @@ -42,6 +42,8 @@ describe("explicit file output requirements", () => { "Write a memory entry to memory/pi-native.txt inside the registered AGENT_HOME. This is personal memory, not a task deliverable.", "Use native write to copy those exact bytes into pi-agent-memory-proof.txt in the task workspace. This is an internal assertion file, not a deliverable.", "Before finishing, attempt native write exactly once to /outside/pi-unassigned.txt with content forbidden. This intentionally unassigned root must be denied.", + "Write internal-proof.txt; then check it exists. This is an internal verification file, not a deliverable.", + "Attempt native write to /outside/probe.txt and create no files. This negative test must be denied.", ])("does not require a file for a text or source-review request: %s", objective => { expect(explicitlyRequestsFileOutput(objective)).toBe(false); }); diff --git a/server/src/services/native-runtime/native-deliverable-feedback.ts b/server/src/services/native-runtime/native-deliverable-feedback.ts index e6b5f188deb..375bdd2284c 100644 --- a/server/src/services/native-runtime/native-deliverable-feedback.ts +++ b/server/src/services/native-runtime/native-deliverable-feedback.ts @@ -64,36 +64,46 @@ async function hasCurrentPublicationReceipt(db: Db, companyId: string, receipts: */ export function explicitlyRequestsFileOutput(objective: string): boolean { const sentences = objective.replaceAll("\\_", "_").split(/(?:[.!?](?:\s|$)|\n)/iu); - return sentences.some((sentence, index) => sentence.split(/(?:[;,]|\bbut\b)/iu).some((clause, clauseIndex, clauses) => { + return sentences.some((sentence, index) => { const file = /\b(?:files?|attachments?|downloads?|pdf|spreadsheets?|workbooks?|slide decks?|powerpoints?|docx|xlsx|csv)\b|\b[^\s/]+\.(?:md|txt|pdf|docx?|xlsx?|csv|pptx?|png|jpe?g|svg|zip)\b/giu; - const creates = [...clause.matchAll(/\b(?:create|make|write|save|export|attach|send|generate|produce|prepare|provide|give|return|build)\b/giu)]; - const create = creates[0]; - if (create && /\b(?:do not|don't|never|no need to)\s*$/iu.test(clause.slice(0, create.index))) return false; - const output = create ? clause.slice(create.index + create[0].length) : ""; - const objects = [...output.matchAll(file)].filter(match => { - const prefix = output.slice(0, match.index); - const suffix = output.slice(match.index + match[0].length); - // "Create no files" is a prohibition, even though it contains a creation - // verb. Negate this object only; another explicit output can still count. - if (/\b(?:no|zero|without(?:\s+any)?)\s+(?:(?:new|temporary|downloadable|attached|additional)\s+)*$/iu.test(prefix)) return false; - // "Write a summary of this PDF" names input, not a requested file. - // Explicit export destinations still count after such input references. - const destination = /\b(?:as|into|to)\s+(?:(?:a|an|the|new|separate|markdown|word|excel)\s+)*$/iu.test(prefix); - if (!destination && /\b(?:of|about|on|from|using|for|with)\b/iu.test(prefix)) return false; - if (/^files?$/iu.test(match[0]) && /^\s+(?:tools?|permissions?|systems?|formats?|names?|paths?|types?|sizes?|descriptors?)\b/iu.test(suffix)) return false; - return true; + const outputs = sentence.split(/(?:[;,]|\bbut\b)/iu).flatMap(clause => { + const creates = [...clause.matchAll(/\b(?:create|make|write|save|export|attach|send|generate|produce|prepare|provide|give|return|build)\b/giu)]; + return creates.flatMap((create, createIndex) => { + const before = clause.slice(0, create.index); + if (/\b(?:do not|don't|never|no need to)\s*$/iu.test(before)) return []; + // Bind each object to its own verb. A denied write or "create no files" + // cannot suppress a separate requested report in this same clause. + const output = clause.slice(create.index + create[0].length, creates[createIndex + 1]?.index); + const objects = [...output.matchAll(file)].filter(match => { + const prefix = output.slice(0, match.index); + const suffix = output.slice(match.index + match[0].length); + // "Create no files" is a prohibition, even though it contains a creation + // verb. Negate this object only; another explicit output can still count. + if (/\b(?:no|zero|without(?:\s+any)?)\s+(?:(?:new|temporary|downloadable|attached|additional)\s+)*$/iu.test(prefix)) return false; + // "Write a summary of this PDF" names input, not a requested file. + // Explicit export destinations still count after such input references. + const destination = /\b(?:as|into|to)\s+(?:(?:a|an|the|new|separate|markdown|word|excel)\s+)*$/iu.test(prefix); + if (!destination && /\b(?:of|about|on|from|using|for|with)\b/iu.test(prefix)) return false; + if (/^files?$/iu.test(match[0]) && /^\s+(?:tools?|permissions?|systems?|formats?|names?|paths?|types?|sizes?|descriptors?)\b/iu.test(suffix)) return false; + return true; + }); + const downloadable = !/\b(?:no|without)\s+(?:downloadable|attached)/iu.test(output) + && /\b(?:downloadable|attached)\s+(?:file|report|document|checklist|draft)\b/iu.test(output); + const publication = /\b(?:downloadable|attached|attach|export|send|provide|return|give)\b/iu.test(create[0] + output); + const deniedAttempt = create[0].toLowerCase() === "write" && objects.length === 1 + && /\b(?:attempt|try)\s+(?:the\s+)?native\s*$/iu.test(before) + && /\b(?:must be denied|denial is (?:the )?expected|(?:this|the) negative test)\b/iu.test(objective); + return [{ objects: objects.length, downloadable, publication, deniedAttempt }]; + }); }); - // An immediately following "This ..." can qualify one internal file, not - // erase a separate output request. Only the authoritative objective counts. - const internal = clauseIndex === clauses.length - 1 && objects.length === 1 + // "This ..." qualifies a single requested file in the preceding sentence, + // even when a later clause checks it. Multiple files cannot share this + // exception and separate report requests still require publication. + const internal = outputs.reduce((count, output) => count + output.objects + Number(output.downloadable && output.objects === 0), 0) === 1 && /^\s*This is (?:an? )?(?:personal memory|internal (?:assertion|verification) file)\b[^.!?]*\bnot a (?:task )?deliverable\b/iu.test(sentences[index + 1] ?? ""); - const attempt = /\b(?:attempt|try)\s+(?:the\s+)?native\s+(?:write|edit)\b/iu.exec(clause); - const deniedAttempt = creates.length === 1 && objects.length === 1 && attempt && create && attempt.index <= create.index - && /\b(?:must be denied|denial is (?:the )?expected|(?:this|the) negative test)\b/iu.test(objective); - if ((internal || deniedAttempt) && !/\b(?:downloadable|attached|attach|export|send|provide|return|give)\b/iu.test(clause)) return false; - return objects.length > 0 || - (!/\b(?:no|without)\s+(?:downloadable|attached)/iu.test(clause) && /\b(?:downloadable|attached)\s+(?:file|report|document|checklist|draft)\b/iu.test(clause)); - })); + return outputs.some(output => (output.objects > 0 || output.downloadable) + && (output.publication || (!internal && !output.deniedAttempt))); + }); } /** Files cited as completed output must be reachable outside the agent workspace. */ 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 66e1b11a312..7d490096a52 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 @@ -336,6 +336,8 @@ describe("native runner file handoff", () => { "Use native write/read file tools. Write memory/pi-native.txt inside AGENT_HOME. This is personal memory, not a task deliverable.", "Use native write to copy bytes into pi-agent-memory-proof.txt. This is an internal assertion file, not a deliverable.", "Attempt native write once to /outside/pi-unassigned.txt. This intentionally unassigned root must be denied.", + "Write internal-proof.txt; then check it exists. This is an internal verification file, not a deliverable.", + "Attempt native write to /outside/probe.txt and create no files. This negative test must be denied.", ])("accepts an internal file outcome without treating it as published output: %s", async objective => { await db.update(heartbeatRuns).set({ contextSnapshot: { issueId, executionContinuation: { objective } } }) .where(eq(heartbeatRuns.id, runId));