mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents save Markdown documents through native tools and REST routes. > - The native write_document handler trims the supplied body. > - This removes final newlines and can change Markdown whitespace. > - This pull request preserves the original body after required-content validation. > - Agents can now write and read the same Markdown string across these paths. ## Linked Issues or Issue Description **What happened?** The native write_document tool removes leading and trailing whitespace. A body with one final newline loses that newline on read-back. The REST document PUT and write_task_document preserve it. **Expected behavior** The native write_document tool must preserve the supplied Markdown body exactly. Required-content checks and key and title normalization must stay in place. REST-backed paths retain surrounding whitespace and keep their existing escaped-line-break conversion for single-line payloads. **Steps to reproduce** 1. Write a disposable document through write_document with a final newline. 2. Read the document and compare the full body with the supplied string. 3. Repeat through write_task_document and REST. These paths preserve the string. Related work: Refs #18824. That change aligns document keys. This change preserves body strings. A search of open and closed PRs found no duplicate fix. ## What Changed - Use a separate untrimmed required-string helper for the document body. Keep the existing requiredString callback signature and all other field normalization. - Add real tool-handler, database, and authenticated HTTP tests for create, update, read-back, and replay. Cover leading spaces, hard breaks, blank lines, final newlines, no final newline, Unicode, and mixed line endings. - Keep blank and invalid bodies rejected. Document whitespace preservation and the existing REST escaped-line-break exception. - Fix the Runner support archive-scanner shutdown defect exposed by CI: rejected ZIP scanners now receive SIGKILL so an ignored SIGTERM cannot leave publication waiting for process closure. Keep all secret and archive limits unchanged. A real child-process regression fails before the change and passes afterward. - Address all three initial review findings: callback type compatibility, contribution comment policy, and the REST documentation limit. ## Verification Final tested head: `c9ff8cf3f83aa19e5d8313c65816a3a2e6562368`. - All applicable [CI checks](https://github.com/paperclipai/paperclip/actions/runs/43019885807) passed. This includes typecheck, build, Runner support, Rust, native Runner, general tests, serialized suites, all eight E2E shards, and the required aggregate gates. - [Greptile review](https://github.com/paperclipai/paperclip/runs/130520170309) passed on this head with confidence 5/5. All three review threads are resolved. The independent Paperclip review also reports 5/5 with no findings. - Red at `7368a0b15c2f02107778a961144d86e2eb59a952`: `pnpm --dir server exec vitest run src/services/native-runtime/task-document-body.integration.test.ts` failed because write_document removed the final newline. Cross-task and REST cases passed. - Green on the final source: 176 tests passed across the document body, document key, native tool authority, task scope, document service, and document route suites. The tests cover create, update, exact read-back, replay, blank rejection, revision guards, and company scope. - The ZIP shutdown regression failed before repair with “scanner did not stop”. After repair, 54 tests passed across zip-inspection-shutdown, private-reporter-artifacts, manual-private-cell-report-assets, and private-evidence-budget. The regression uses a real process that ignores SIGTERM. - Earlier CI runs had two archive-related timeouts. The source repair above replaces blind reruns. The Runner support suite and aggregate verify gate now pass. - A disposable live native document reproduced the original whitespace loss. REST preserved the same string. The fixed handler passed local integration tests. The running service has not been deployed. - Local environment: Linux x64, Node 25.9.0, Vitest 5.0.3, and embedded PostgreSQL. Local Rust preparation was unavailable because cargo is absent. Final-head CI passed Rust verification. `pnpm verify --explain` requested broad CI because evidence.ts is a global input. Required CI supplied that coverage. ## Risks Low risk. The change retains required-content validation. The product change affects document body whitespace handling. The CI repair forcefully stops only already-rejected archive scanners; it retains fail-closed publication and all existing scan limits. It does not change schema, authorization, revision guards, receipts, or persistence. ## Model Used CI repair continuation: GPT-6.1, using repository tools and code execution. OpenAI Codex, GPT-6 (the exact serving model ID and context window were not exposed in this session). Used reasoning, repository tools, and code 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 - [x] 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>
74 lines
4.1 KiB
TypeScript
74 lines
4.1 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { afterAll, beforeAll, expect, it } from "vitest";
|
|
import { startRunnerApiTestServer } from "../../__tests__/helpers/runner-api-server.js";
|
|
import { createLocalAgentJwt } from "../../agent-auth-jwt.js";
|
|
import { documentService } from "../documents.js";
|
|
|
|
let server: Awaited<ReturnType<typeof startRunnerApiTestServer>>;
|
|
const previousSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET;
|
|
beforeAll(async () => {
|
|
process.env.PAPERCLIP_AGENT_JWT_SECRET = randomUUID();
|
|
server = await startRunnerApiTestServer();
|
|
}, 60_000);
|
|
afterAll(async () => {
|
|
await server?.close();
|
|
if (previousSecret === undefined) delete process.env.PAPERCLIP_AGENT_JWT_SECRET;
|
|
else process.env.PAPERCLIP_AGENT_JWT_SECRET = previousSecret;
|
|
});
|
|
|
|
const bodies = [
|
|
"# Final newline\n",
|
|
" Leading whitespace\n\nA Markdown hard break \nnext line\n\n",
|
|
"No final newline",
|
|
"\t日本語 café 🧭\r\n\r\nSecond line \r\n\n",
|
|
];
|
|
|
|
it.each(["write_document", "write_task_document", "rest"] as const)(
|
|
"%s preserves exact Markdown on create, update, and read-back", async surface => {
|
|
const fixture = await server.fixture({ disableWakeOnDemand: true });
|
|
const issueId = surface === "write_task_document" ? fixture.blockerId : fixture.issueId;
|
|
const service = documentService(server.db);
|
|
const token = createLocalAgentJwt(fixture.agentId, fixture.companyId, "paperclip_runner", fixture.runId);
|
|
const headers = { Authorization: `Bearer ${token}`, "Content-Type": "application/json" };
|
|
for (const [index, body] of bodies.entries()) {
|
|
const key = `exact-body-${index}`;
|
|
let baseRevisionId: string | null = null;
|
|
for (const phase of ["create", "update"] as const) {
|
|
const supplied = phase === "create" ? body : `Updated\n${body}`;
|
|
const args = { key, title: " Exact body ", body: supplied, baseRevisionId };
|
|
const call = { tool: surface, callId: randomUUID(), arguments: surface === "write_document"
|
|
? { ...args, idempotencyKey: randomUUID() } : { ...args, taskId: issueId } };
|
|
if (surface === "rest") {
|
|
const response = await fetch(`${server.apiUrl}/api/issues/${issueId}/documents/${key}`, {
|
|
method: "PUT", headers, body: JSON.stringify({ ...args, format: "markdown" }),
|
|
});
|
|
expect(response.status).toBe(phase === "create" ? 201 : 200);
|
|
expect((await response.json()).body).toBe(supplied);
|
|
} else {
|
|
const result = await fixture.authority.execute(call);
|
|
expect(result).toMatchObject(surface === "write_document"
|
|
? { document: { body: supplied, title: "Exact body" } } : { ok: true, status: phase === "create" ? 201 : 200 });
|
|
expect(await fixture.authority.execute({ ...call, callId: surface === "write_document" ? randomUUID() : call.callId })).toEqual(result);
|
|
}
|
|
const saved = await service.getIssueDocumentByKey(issueId, key);
|
|
expect(saved?.body).toBe(supplied);
|
|
expect(saved?.latestRevisionNumber).toBe(phase === "create" ? 1 : 2);
|
|
const read = await fixture.authority.execute({ tool: surface === "write_task_document" ? "read_task_document" : "read_document",
|
|
callId: randomUUID(), arguments: surface === "write_task_document" ? { taskId: issueId, key } : { key } }) as { document?: { body: string }; data?: { body: string } };
|
|
expect(surface === "write_task_document" ? read.data?.body : read.document?.body).toBe(supplied);
|
|
baseRevisionId = saved!.latestRevisionId;
|
|
}
|
|
}
|
|
}, 30_000,
|
|
);
|
|
|
|
it("retains required body validation for write_document", async () => {
|
|
const fixture = await server.fixture({ disableWakeOnDemand: true });
|
|
for (const body of [undefined, null, 42, "", " \n\t"]) {
|
|
await expect(fixture.authority.execute({ tool: "write_document", callId: randomUUID(), arguments: {
|
|
key: "invalid-body", title: "Invalid body", body, baseRevisionId: null, idempotencyKey: randomUUID(),
|
|
} })).rejects.toThrow("paperclip_runner_tool_input_invalid");
|
|
}
|
|
expect(await documentService(server.db).getIssueDocumentByKey(fixture.issueId, "invalid-body")).toBeNull();
|
|
});
|