mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
fix(native): always advertise run-scoped readers to provider threads (#16995)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs use a Paperclip tool authority to declare tools to the provider thread. Codex runs through runnerd. > - An external-chat wake with a truncated comment batch requires the agent to call `read_current_wake_comments`. A terminal fence fails the run if the agent did not read every bound comment. > - Codex `thread/resume` has no `dynamicTools` parameter. A resumed thread keeps the tool catalog from its first `thread/start`. > - The tool authority declared `read_current_wake_comments` and `read_chat_attachment` only when the current run had a binding. The tool-contract fingerprint already said that both tools are "always advertised", so the code and the contract did not agree. > - When a truncated chat wake resumed a thread that started on an ordinary turn, the agent could not see the reader. The agent blocked, and the fence failed the run with `native_current_wake_comments_unread`. The task then stayed blocked for the board. > - This pull request declares both readers on every turn and rotates threads that started with the old catalog. > - The benefit: truncated GitHub, Slack, and other chat wakes can read their full comment batch on resumed threads. They no longer fail at the fence. ## Linked Issues or Issue Description No public issue exists. This is the bug description, in the fields of `bug_report.yml`. **What happened?** A native Codex run got a GitHub chat wake with a truncated comment batch. The prompt told the agent to call `read_current_wake_comments`. The agent searched for the tool and did not find it. It reported a block (`required_wake_reader_unavailable`). The server sent the reader in the run's authorized tool list, but the provider thread had resumed. The fence then failed the run with `native_current_wake_comments_unread`, and the task became blocked. This happened 6 times on staging for the same task. **Expected behavior** A resumed thread can call `read_current_wake_comments` and `read_chat_attachment` on any turn that has a binding. The run then completes normally. **Steps to reproduce** 1. Start a native Codex session on a task with an ordinary wake. Do not use a truncated chat wake. 2. Send an external-chat message that makes the wake comment batch truncated (`fallbackFetchNeeded: true`). 3. The run resumes the same Codex thread. The model cannot see `read_current_wake_comments`. The run fails with `native_current_wake_comments_unread`. **Paperclip version or commit** Hosted staging release `fc0d390d`. The code is the same on `master` at `3bbc210`. **Deployment mode** Hosted (staging). The native runner uses the `codex_app_server` backend through runnerd in an external sandbox. **Agent adapter(s) involved** Codex (native `paperclip_runner`). Related history: the existing retained-fingerprint table already lists a "conditional current-wake reader" rotation. This change fixes a regression of that contract. I searched open and closed PRs for `current_wake_comments` and "wake comments" and found no duplicate fix. ## What Changed - `PaperclipRunnerToolAuthority.definitions()` now always declares `read_current_wake_comments` and `read_chat_attachment`. Execution stays fail-closed: without the server-verified binding or scope, a call still fails with `paperclip_runner_tool_not_advertised` or `paperclip_runner_chat_attachment_read_scope_unavailable`. - The native tool-contract fingerprint goes from v17 to v18, with the two policy values set to `.v2`. Checkpoints from deployed v17 threads, which may lack the readers, start a fresh provider thread one time with full task context. - New regression test: the catalog for an ordinary turn is equal to the catalog for a truncated chat turn, and it contains both readers. - New resume tests: the deployed v17 local and remote fingerprints do not rebind. The existing test that asserted conditional advertisement now asserts the stable catalog and the fail-closed execution. ## Verification - Red before the fix: `npx vitest run src/services/native-runtime/paperclip-runner-tool-authority.test.ts -t "advertises only real bindings|keeps run-scoped reader declarations stable"` had 2 failures. The unbound catalog had 48 tools and did not include either reader. - Green after the fix: `paperclip-runner-tool-authority.test.ts`, `native-session-resume.test.ts`, `current-wake-comments.test.ts`, `chat-attachment-read.test.ts`, `native-execution-input.test.ts`, and `file-delivery-bridges.test.ts` all pass. - `native-session-executor.test.ts`: 764 pass and 1 fails. The failure (`compares export process compatibility … (changed: false)`, `db.transaction is not a function`) also happens on unchanged `main`, so this change did not cause it. - Consequential assumption: Codex does not refresh dynamic tools on resume. Evidence: `codex app-server generate-json-schema` for codex-cli 0.160.0 shows `ThreadStartParams.dynamicTools`, but `ThreadResumeParams` has no `dynamicTools` field. The failed staging run also showed it: the server listed `read_current_wake_comments` as authorized, but the model's tool search returned no match after `thread/resume`. - Not run locally: the server `tsc --noEmit` step (the sandbox has a 4 GB memory limit and the process was killed) and `dot-runner.test.ts` (`cargo` is not installed). CI runs both. - Not run: a live end-to-end chat wake against a real Codex provider. That needs a staging deployment. ## Risks - The fingerprint bump makes every native session that resumes from a v17 checkpoint start a fresh provider thread one time. Paperclip rebuilds the full task context, but the provider-side conversation history is not carried over. This is the same tradeoff as earlier fingerprint bumps. - Two more tool declarations use a small part of the Codex dynamic-tool budget. The assigned-MCP fit check already includes these definitions. - Follow-up, not in this PR: the native Codex driver reports `toolRefreshOnResume: true`, and the runnerd fake app server accepts `dynamicTools` on `thread/resume`. Real codex-cli 0.160.0 has no such field. Connection-tool continuations that depend on resume refresh need a separate check. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) from Anthropic, running as a Paperclip agent with tool use (shell, file edit, and test execution). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4 files changed
+62
-11
No files matched your search
@@ -1773,6 +1773,13 @@ describe("rebindNativeSessionCheckpoint", () => {
|
||||
retainedFingerprint:
|
||||
"sha256:f12fe3b1bf5d63b7954d8ab16f57764874806ab49acff7c6582c4de007f4ed4d",
|
||||
},
|
||||
{
|
||||
contract: "regressed conditional run-scoped readers",
|
||||
// Deployed v17 threads were opened without read_current_wake_comments
|
||||
// and read_chat_attachment on ordinary turns; resume cannot add them.
|
||||
retainedFingerprint:
|
||||
"sha256:dc495475fb6a4bd034c6dbb2bbe0cf52e7ffc1635838889dc5f7d7251273f119",
|
||||
},
|
||||
{
|
||||
contract: "task-bound human-input description",
|
||||
// Deployed v9 still advertises the generic mock-task question description.
|
||||
@@ -1876,6 +1883,17 @@ describe("rebindNativeSessionCheckpoint", () => {
|
||||
executionTargetKind: "remote",
|
||||
}),
|
||||
).not.toBeNull();
|
||||
// Deployed v17 remote threads were opened without the run-scoped readers.
|
||||
expect(
|
||||
rebindNativeSessionCheckpoint({
|
||||
previousRun: previousRun({
|
||||
nativeToolContractFingerprint:
|
||||
"sha256:96dde421aab01f847468d1a8f12863d32dfdb3038a9aca61e1e6bdc81022d5a1",
|
||||
}),
|
||||
currentExecution: execution(currentRunId),
|
||||
executionTargetKind: "remote",
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
|
||||
|
||||
@@ -34,17 +34,17 @@ export function nativeToolContractFingerprintForTarget(
|
||||
return `sha256:${createHash("sha256")
|
||||
.update(
|
||||
JSON.stringify({
|
||||
schema: "paperclip.native-tool-contract.v17",
|
||||
schema: "paperclip.native-tool-contract.v18",
|
||||
executionTargetKind,
|
||||
advertisementPolicy: {
|
||||
// Direct provider threads retain declarations from thread/start.
|
||||
// Keep this explicit so changing a tool from conditional to stable
|
||||
// advertisement rotates checkpoints even when its schema is unchanged.
|
||||
readCurrentWakeComments: "always_advertised_binding_gated.v1",
|
||||
readCurrentWakeComments: "always_advertised_binding_gated.v2",
|
||||
historicalChatAttachments:
|
||||
"always_advertised_conversation_binding_gated.v1",
|
||||
registerDeliverable: "verified_local_or_remote_workspace.v2",
|
||||
readChatAttachment: "always_advertised_run_scope_local_staging.v1",
|
||||
readChatAttachment: "always_advertised_run_scope_local_staging.v2",
|
||||
structuredHumanInput:
|
||||
"always_advertised_run_issue_agent_binding_gated_current_task_description.v2",
|
||||
semanticCompletion: "finish_accessible_deliverable_evidence.v4",
|
||||
|
||||
@@ -28,7 +28,8 @@ import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.
|
||||
import { PaperclipRunnerSemanticAuthority } from "./runner-semantic-authority.js";
|
||||
import { createAssignedMcpTools } from "./assigned-mcp-tools.js";
|
||||
import type { ToolGatewayService } from "../tool-gateway.js";
|
||||
import { READ_CURRENT_WAKE_COMMENTS_TOOL_NAME } from "./current-wake-comments.js";
|
||||
import { CURRENT_WAKE_COMMENTS_BINDING_SCHEMA, READ_CURRENT_WAKE_COMMENTS_TOOL_NAME } from "./current-wake-comments.js";
|
||||
import { NativeChatAttachmentReadScope, READ_CHAT_ATTACHMENT_TOOL_NAME } from "./chat-attachment-read.js";
|
||||
import { buildLowTrustSourceTrust, buildPromotedSourceTrust, LOW_TRUST_QUARANTINED_BODY } from "../source-trust.js";
|
||||
import { CAPABILITY_SEMANTIC_TOOL_CATALOG, runnerCodexDynamicToolsFit } from "../../vendor/paperclip-runner/index.js";
|
||||
|
||||
@@ -119,12 +120,10 @@ describe("PaperclipRunnerToolAuthority", () => {
|
||||
issueId,
|
||||
runId,
|
||||
});
|
||||
expect(authority.definitions()).toHaveLength(48);
|
||||
expect(authority.definitions()).toHaveLength(50);
|
||||
expect(authority.definitions().map(tool => tool.name)).toEqual(expect.arrayContaining([
|
||||
"get_workspace", "list_workspaces", "select_workspace", "prepare_repository",
|
||||
]));
|
||||
expect(authority.definitions().map(tool => tool.name)).not.toContain("read_chat_attachment");
|
||||
expect(authority.definitions().map(tool => tool.name)).not.toContain("read_current_wake_comments");
|
||||
const questions = authority.definitions().find(tool => tool.name === "request_human_input")!;
|
||||
expect(questions.description).toContain("ask only the next unanswered question");
|
||||
expect(questions.description).toContain("Never fabricate answers");
|
||||
@@ -197,6 +196,39 @@ describe("PaperclipRunnerToolAuthority", () => {
|
||||
arguments: {},
|
||||
}),
|
||||
).rejects.toThrow("paperclip_runner_tool_not_advertised");
|
||||
await expect(
|
||||
authority.execute({
|
||||
tool: READ_CHAT_ATTACHMENT_TOOL_NAME,
|
||||
callId: "attachment-read-without-run-scope",
|
||||
arguments: { sourceCommentId: randomUUID(), attachmentId: randomUUID() },
|
||||
}),
|
||||
).rejects.toThrow("paperclip_runner_chat_attachment_read_scope_unavailable");
|
||||
});
|
||||
|
||||
it("keeps run-scoped reader declarations stable for resumed provider threads", () => {
|
||||
// Codex thread/resume has no dynamicTools parameter: a resumed thread keeps
|
||||
// the catalog from its original thread/start. A truncated chat wake that
|
||||
// resumes a thread opened on an ordinary turn still needs both readers.
|
||||
const ordinary = new PaperclipRunnerToolAuthority(db, { companyId, agentId, issueId, runId });
|
||||
const chatBinding = { companyId, agentId, issueId, runId };
|
||||
const truncatedChat = new PaperclipRunnerToolAuthority(db, {
|
||||
...chatBinding,
|
||||
currentWakeComments: {
|
||||
schema: CURRENT_WAKE_COMMENTS_BINDING_SCHEMA,
|
||||
...chatBinding,
|
||||
provider: "github",
|
||||
commentIds: [randomUUID()],
|
||||
attachmentOmissions: [],
|
||||
bindingDigest: "binding-digest",
|
||||
},
|
||||
chatAttachmentReadScope: new NativeChatAttachmentReadScope({
|
||||
db, binding: chatBinding, workspaceRoot: "/workspace", executionTargetKind: "local",
|
||||
}),
|
||||
});
|
||||
expect(ordinary.definitions().map(tool => tool.name)).toEqual(expect.arrayContaining([
|
||||
READ_CURRENT_WAKE_COMMENTS_TOOL_NAME, READ_CHAT_ATTACHMENT_TOOL_NAME,
|
||||
]));
|
||||
expect(ordinary.definitions()).toEqual(truncatedChat.definitions());
|
||||
});
|
||||
|
||||
it("reads current tool authority without transferring saved profiles or output", async () => {
|
||||
|
||||
@@ -315,12 +315,13 @@ export class PaperclipRunnerToolAuthority {
|
||||
: descriptor.inputSchema,
|
||||
}));
|
||||
// Keep the provider session's direct tool catalog stable across ordinary
|
||||
// and truncated external-chat turns. Execution still fails closed unless
|
||||
// this exact run carries a server-verified current-wake binding.
|
||||
if (this.binding.currentWakeComments) definitions.push(READ_CURRENT_WAKE_COMMENTS_TOOL_DEFINITION);
|
||||
// and truncated external-chat turns: Codex thread/resume cannot add tools,
|
||||
// so a reader missing at thread/start stays missing. Execution still fails
|
||||
// closed unless this exact run carries the server-verified binding/scope.
|
||||
definitions.push(READ_CURRENT_WAKE_COMMENTS_TOOL_DEFINITION);
|
||||
definitions.push(LIST_CHAT_ATTACHMENTS_TOOL_DEFINITION);
|
||||
definitions.push(REUSE_CHAT_ATTACHMENT_TOOL_DEFINITION);
|
||||
if (this.binding.chatAttachmentReadScope) definitions.push(READ_CHAT_ATTACHMENT_TOOL_DEFINITION);
|
||||
definitions.push(READ_CHAT_ATTACHMENT_TOOL_DEFINITION);
|
||||
if (this.binding.taskAttachmentRead) definitions.push(...TASK_ATTACHMENT_DEFINITIONS);
|
||||
definitions.push(...runnerBridgeDefinitions({ workspace: !this.binding.firstPartyOnly && !!this.binding.workspaceRoot && this.binding.workspaceBridge === true, skills: !!this.binding.runtimeContext, api: runnerApiToolsEnabled(this.binding.companyId, this.binding.apiToolsEnabled), mode: workMode }));
|
||||
const connectionTools = this.binding.firstPartyOnly ? [] : [...RUNTIME_CONNECTION_TOOL_DEFINITIONS,
|
||||
|
||||
Reference in new issue
Block a user