pdf-server: add save_as interact action (#580)
* pdf-server: add save_as interact action
Adds a model-callable save_as action to the interact tool that writes the
viewer's current annotated state (annotations + form values) to a new file.
Reuses the get_pages request/reply pattern: model's interact call blocks
while the viewer builds bytes via getAnnotatedPdfBytes() and posts them
back through a new submit_save_data app-only tool.
Validation runs synchronously before bothering the viewer:
- path must be absolute or file://
- isWritablePath gate (same as save_pdf)
- existence check; fails with a descriptive error unless overwrite: true
The viewer can report build errors back via submit_save_data({error}),
so the model gets a real message instead of the 45s timeout.
* pdf-server: warn model off app-only tools in descriptions
Hosts that don't filter visibility:["app"] tools from the model's tool
list will still surface these. The save_pdf one in particular is a
near-miss for the new save_as interact action — point the model at the
right one.
* pdf-server: save_as without path overwrites original
When path is omitted, save_as falls back to the displayed PDF's local
path (tracked per-viewUUID by display_pdf). Same writability gate as
the viewer's save button: isWritablePath + OS-level W_OK probe, so it
refuses on read-only mounts before the viewer roundtrip.
Requires explicit overwrite: true since the original always exists —
the model can't accidentally clobber it.
Remote PDFs have no source path stored; the error tells the model to
pass an explicit path.
This commit is contained in:
Olivier Chafik
GitHub
parent
38700569e7
commit
4b3f6252a4
1 changed files with 557 additions and 19 deletions
|
|
@ -17,6 +17,7 @@ import {
|
|||
cliLocalFiles,
|
||||
isWritablePath,
|
||||
writeFlags,
|
||||
viewSourcePaths,
|
||||
CACHE_INACTIVITY_TIMEOUT_MS,
|
||||
CACHE_MAX_LIFETIME_MS,
|
||||
CACHE_MAX_PDF_SIZE_BYTES,
|
||||
|
|
@ -1055,6 +1056,294 @@ describe("interact tool", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("save_as", () => {
|
||||
// Roundtrip tests need: writable scope, kick off interact WITHOUT awaiting
|
||||
// (it blocks until the view replies), poll → submit → await. The poll()
|
||||
// call also registers the uuid in viewsPolled, satisfying
|
||||
// ensureViewerIsPolling — without it interact would hang ~8s and fail.
|
||||
|
||||
let tmpDir: string;
|
||||
let savedDirs: Set<string>;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf-saveas-"));
|
||||
savedDirs = new Set(allowedLocalDirs);
|
||||
allowedLocalDirs.add(tmpDir); // make tmpDir a directory root → writable
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
allowedLocalDirs.clear();
|
||||
for (const x of savedDirs) allowedLocalDirs.add(x);
|
||||
viewSourcePaths.clear();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("no path, no source tracked → tells model to provide a path", async () => {
|
||||
// Fresh UUID never seen by display_pdf → viewSourcePaths has no entry.
|
||||
// Same condition as a remote (https://) PDF or a stale viewUUID.
|
||||
const { server, client } = await connect();
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: { viewUUID: "saveas-nosource", action: "save_as" },
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("no local source file");
|
||||
expect(firstText(r)).toContain("Provide an explicit `path`");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("no path, source tracked, overwrite omitted → asks for confirmation", async () => {
|
||||
const { server, client } = await connect();
|
||||
const source = path.join(tmpDir, "original.pdf");
|
||||
fs.writeFileSync(source, "%PDF-1.4\noriginal");
|
||||
viewSourcePaths.set("saveas-noconfirm", source);
|
||||
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: { viewUUID: "saveas-noconfirm", action: "save_as" },
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("overwrites the original");
|
||||
expect(firstText(r)).toContain(source);
|
||||
expect(firstText(r)).toContain("overwrite: true");
|
||||
// Nothing enqueued, file untouched
|
||||
expect(fs.readFileSync(source, "utf8")).toBe("%PDF-1.4\noriginal");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("no path, source not writable → same gate as save button", async () => {
|
||||
const { server, client } = await connect();
|
||||
// Source outside any directory root → isWritablePath false → save button
|
||||
// would be hidden in the viewer. save_as should refuse for the same reason.
|
||||
const outside = path.join(os.tmpdir(), "saveas-outside.pdf");
|
||||
fs.writeFileSync(outside, "x");
|
||||
viewSourcePaths.set("saveas-buttongate", outside);
|
||||
|
||||
try {
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: {
|
||||
viewUUID: "saveas-buttongate",
|
||||
action: "save_as",
|
||||
overwrite: true,
|
||||
},
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("not writable");
|
||||
expect(firstText(r)).toContain("save button is hidden");
|
||||
} finally {
|
||||
fs.rmSync(outside, { force: true });
|
||||
}
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("no path, overwrite: true → roundtrip overwrites the original", async () => {
|
||||
const { server, client } = await connect();
|
||||
const uuid = "saveas-original";
|
||||
const source = path.join(tmpDir, "report.pdf");
|
||||
fs.writeFileSync(source, "%PDF-1.4\noriginal contents");
|
||||
viewSourcePaths.set(uuid, source);
|
||||
|
||||
const interactPromise = client.callTool({
|
||||
name: "interact",
|
||||
arguments: { viewUUID: uuid, action: "save_as", overwrite: true },
|
||||
});
|
||||
|
||||
const cmds = await poll(client, uuid);
|
||||
expect(cmds).toHaveLength(1);
|
||||
expect(cmds[0].type).toBe("save_as");
|
||||
await client.callTool({
|
||||
name: "submit_save_data",
|
||||
arguments: {
|
||||
requestId: cmds[0].requestId as string,
|
||||
data: Buffer.from("%PDF-1.4\nannotated").toString("base64"),
|
||||
},
|
||||
});
|
||||
|
||||
const r = await interactPromise;
|
||||
expect(r.isError).toBeFalsy();
|
||||
expect(firstText(r)).toContain(source);
|
||||
expect(fs.readFileSync(source, "utf8")).toBe("%PDF-1.4\nannotated");
|
||||
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("rejects non-absolute path", async () => {
|
||||
const { server, client } = await connect();
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: {
|
||||
viewUUID: "saveas-rel",
|
||||
action: "save_as",
|
||||
path: "relative.pdf",
|
||||
},
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("absolute");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("rejects non-writable path", async () => {
|
||||
const { server, client } = await connect();
|
||||
// Path outside any directory root → not writable. Validation is sync,
|
||||
// so nothing is enqueued and the queue stays empty.
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: {
|
||||
viewUUID: "saveas-nowrite",
|
||||
action: "save_as",
|
||||
path: "/somewhere/else/out.pdf",
|
||||
},
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("not under a mounted directory root");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("rejects existing file when overwrite is false (default)", async () => {
|
||||
const { server, client } = await connect();
|
||||
const target = path.join(tmpDir, "exists.pdf");
|
||||
fs.writeFileSync(target, "old contents");
|
||||
|
||||
const r = await client.callTool({
|
||||
name: "interact",
|
||||
arguments: {
|
||||
viewUUID: "saveas-exists",
|
||||
action: "save_as",
|
||||
path: target,
|
||||
},
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("already exists");
|
||||
expect(firstText(r)).toContain("overwrite: true");
|
||||
// Existence check is sync — nothing enqueued, file untouched.
|
||||
expect(fs.readFileSync(target, "utf8")).toBe("old contents");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("full roundtrip: enqueue → poll → submit → file written", async () => {
|
||||
const { server, client } = await connect();
|
||||
const uuid = "saveas-roundtrip";
|
||||
const target = path.join(tmpDir, "out.pdf");
|
||||
const pdfBytes = "%PDF-1.4\nfake-annotated-contents\n%%EOF";
|
||||
|
||||
// interact blocks in waitForSaveData until submit_save_data resolves it
|
||||
const interactPromise = client.callTool({
|
||||
name: "interact",
|
||||
arguments: { viewUUID: uuid, action: "save_as", path: target },
|
||||
});
|
||||
|
||||
// Viewer polls → receives the save_as command with a requestId
|
||||
const cmds = await poll(client, uuid);
|
||||
expect(cmds).toHaveLength(1);
|
||||
expect(cmds[0].type).toBe("save_as");
|
||||
const requestId = cmds[0].requestId as string;
|
||||
expect(typeof requestId).toBe("string");
|
||||
|
||||
// Viewer submits bytes
|
||||
const submit = await client.callTool({
|
||||
name: "submit_save_data",
|
||||
arguments: {
|
||||
requestId,
|
||||
data: Buffer.from(pdfBytes).toString("base64"),
|
||||
},
|
||||
});
|
||||
expect(submit.isError).toBeFalsy();
|
||||
|
||||
// interact now unblocks with success
|
||||
const r = await interactPromise;
|
||||
expect(r.isError).toBeFalsy();
|
||||
expect(firstText(r)).toContain("Saved");
|
||||
expect(firstText(r)).toContain(target);
|
||||
expect(fs.readFileSync(target, "utf8")).toBe(pdfBytes);
|
||||
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("overwrite: true replaces an existing file", async () => {
|
||||
const { server, client } = await connect();
|
||||
const uuid = "saveas-overwrite";
|
||||
const target = path.join(tmpDir, "replace.pdf");
|
||||
fs.writeFileSync(target, "old contents");
|
||||
|
||||
const interactPromise = client.callTool({
|
||||
name: "interact",
|
||||
arguments: {
|
||||
viewUUID: uuid,
|
||||
action: "save_as",
|
||||
path: target,
|
||||
overwrite: true,
|
||||
},
|
||||
});
|
||||
|
||||
const cmds = await poll(client, uuid);
|
||||
const requestId = cmds[0].requestId as string;
|
||||
await client.callTool({
|
||||
name: "submit_save_data",
|
||||
arguments: {
|
||||
requestId,
|
||||
data: Buffer.from("%PDF-1.4\nnew").toString("base64"),
|
||||
},
|
||||
});
|
||||
|
||||
const r = await interactPromise;
|
||||
expect(r.isError).toBeFalsy();
|
||||
expect(fs.readFileSync(target, "utf8")).toBe("%PDF-1.4\nnew");
|
||||
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("propagates viewer-reported errors to the model", async () => {
|
||||
const { server, client } = await connect();
|
||||
const uuid = "saveas-viewerr";
|
||||
const target = path.join(tmpDir, "wontwrite.pdf");
|
||||
|
||||
const interactPromise = client.callTool({
|
||||
name: "interact",
|
||||
arguments: { viewUUID: uuid, action: "save_as", path: target },
|
||||
});
|
||||
|
||||
const cmds = await poll(client, uuid);
|
||||
// Viewer hit an error building bytes → reports it instead of timing out
|
||||
await client.callTool({
|
||||
name: "submit_save_data",
|
||||
arguments: {
|
||||
requestId: cmds[0].requestId as string,
|
||||
error: "pdf-lib choked on a comb field",
|
||||
},
|
||||
});
|
||||
|
||||
const r = await interactPromise;
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("pdf-lib choked on a comb field");
|
||||
expect(fs.existsSync(target)).toBe(false);
|
||||
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it("submit_save_data with unknown requestId returns isError", async () => {
|
||||
const { server, client } = await connect();
|
||||
const r = await client.callTool({
|
||||
name: "submit_save_data",
|
||||
arguments: { requestId: "never-created", data: "AAAA" },
|
||||
});
|
||||
expect(r.isError).toBe(true);
|
||||
expect(firstText(r)).toContain("No pending request");
|
||||
await client.close();
|
||||
await server.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe("viewer liveness", () => {
|
||||
// get_screenshot/get_text fail fast when the iframe never polled, instead
|
||||
// of waiting 45s for a viewer that isn't there. Reproduces the case where
|
||||
|
|
|
|||
Loading…
Reference in a new issue