diffs: remove standalone path footguns and add format passthrough regression

This commit is contained in:
Gustavo Madeira Santana
2026-03-02 02:15:55 -05:00
parent 30e831cb24
commit b753edbc73
3 changed files with 98 additions and 48 deletions

View File

@@ -92,10 +92,11 @@ describe("DiffArtifactStore", () => {
await expect(store.readHtml(artifact.id)).rejects.toThrow("escapes store root");
});
it("allocates standalone file paths outside artifact metadata", async () => {
const filePath = store.allocateStandaloneFilePath();
expect(filePath).toMatch(/preview\.png$/);
expect(filePath).toContain(rootDir);
it("creates standalone file artifacts with managed metadata", async () => {
const standalone = await store.createStandaloneFileArtifact();
expect(standalone.filePath).toMatch(/preview\.png$/);
expect(standalone.filePath).toContain(rootDir);
expect(Date.parse(standalone.expiresAt)).toBeGreaterThan(Date.now());
});
it("expires standalone file artifacts using ttl metadata", async () => {
@@ -127,8 +128,8 @@ describe("DiffArtifactStore", () => {
const imagePath = store.allocateImagePath(artifact.id, "pdf");
expect(imagePath).toMatch(/preview\.pdf$/);
const standaloneImagePath = store.allocateStandaloneImagePath();
expect(standaloneImagePath).toMatch(/preview\.png$/);
const standalone = await store.createStandaloneFileArtifact();
expect(standalone.filePath).toMatch(/preview\.png$/);
const updated = await store.updateImagePath(artifact.id, imagePath);
expect(updated.filePath).toBe(imagePath);
@@ -144,9 +145,9 @@ describe("DiffArtifactStore", () => {
});
const artifactPdf = store.allocateFilePath(artifact.id, "pdf");
const standalonePdf = store.allocateStandaloneFilePath("pdf");
const standalonePdf = await store.createStandaloneFileArtifact({ format: "pdf" });
expect(artifactPdf).toMatch(/preview\.pdf$/);
expect(standalonePdf).toMatch(/preview\.pdf$/);
expect(standalonePdf.filePath).toMatch(/preview\.pdf$/);
});
it("throttles cleanup sweeps across repeated artifact creation", async () => {

View File

@@ -31,6 +31,8 @@ type StandaloneFileMeta = {
filePath: string;
};
type ArtifactMetaFileName = "meta.json" | "file-meta.json";
export class DiffArtifactStore {
private readonly rootDir: string;
private readonly logger?: PluginLogger;
@@ -123,11 +125,6 @@ export class DiffArtifactStore {
return path.join(this.artifactDir(id), `preview.${format}`);
}
allocateStandaloneFilePath(format: DiffOutputFormat = "png"): string {
const id = crypto.randomBytes(10).toString("hex");
return path.join(this.artifactDir(id), `preview.${format}`);
}
async createStandaloneFileArtifact(
params: CreateStandaloneFileArtifactParams = {},
): Promise<{ id: string; filePath: string; expiresAt: string }> {
@@ -162,10 +159,6 @@ export class DiffArtifactStore {
return this.allocateFilePath(id, format);
}
allocateStandaloneImagePath(format: DiffOutputFormat = "png"): string {
return this.allocateStandaloneFilePath(format);
}
scheduleCleanup(): void {
this.maybeCleanupExpired();
}
@@ -237,60 +230,76 @@ export class DiffArtifactStore {
return this.resolveWithinRoot(id);
}
private metaPath(id: string): string {
return path.join(this.artifactDir(id), "meta.json");
}
private standaloneMetaPath(id: string): string {
return path.join(this.artifactDir(id), "file-meta.json");
}
private async writeMeta(meta: DiffArtifactMeta): Promise<void> {
await fs.writeFile(this.metaPath(meta.id), JSON.stringify(meta, null, 2), "utf8");
await this.writeJsonMeta(meta.id, "meta.json", meta);
}
private async readMeta(id: string): Promise<DiffArtifactMeta | null> {
try {
const raw = await fs.readFile(this.metaPath(id), "utf8");
return JSON.parse(raw) as DiffArtifactMeta;
} catch (error) {
if (isFileNotFound(error)) {
return null;
}
this.logger?.warn(`Failed to read diff artifact metadata for ${id}: ${String(error)}`);
const parsed = await this.readJsonMeta(id, "meta.json", "diff artifact");
if (!parsed) {
return null;
}
return parsed as DiffArtifactMeta;
}
private async writeStandaloneMeta(meta: StandaloneFileMeta): Promise<void> {
await fs.writeFile(this.standaloneMetaPath(meta.id), JSON.stringify(meta, null, 2), "utf8");
await this.writeJsonMeta(meta.id, "file-meta.json", meta);
}
private async readStandaloneMeta(id: string): Promise<StandaloneFileMeta | null> {
const parsed = await this.readJsonMeta(id, "file-meta.json", "standalone diff");
if (!parsed) {
return null;
}
try {
const raw = await fs.readFile(this.standaloneMetaPath(id), "utf8");
const parsed = JSON.parse(raw) as Partial<StandaloneFileMeta>;
const value = parsed as Partial<StandaloneFileMeta>;
if (
parsed.kind !== "standalone_file" ||
typeof parsed.id !== "string" ||
typeof parsed.createdAt !== "string" ||
typeof parsed.expiresAt !== "string" ||
typeof parsed.filePath !== "string"
value.kind !== "standalone_file" ||
typeof value.id !== "string" ||
typeof value.createdAt !== "string" ||
typeof value.expiresAt !== "string" ||
typeof value.filePath !== "string"
) {
return null;
}
return {
kind: parsed.kind,
id: parsed.id,
createdAt: parsed.createdAt,
expiresAt: parsed.expiresAt,
filePath: this.normalizeStoredPath(parsed.filePath, "filePath"),
kind: value.kind,
id: value.id,
createdAt: value.createdAt,
expiresAt: value.expiresAt,
filePath: this.normalizeStoredPath(value.filePath, "filePath"),
};
} catch (error) {
this.logger?.warn(`Failed to normalize standalone diff metadata for ${id}: ${String(error)}`);
return null;
}
}
private metaFilePath(id: string, fileName: ArtifactMetaFileName): string {
return path.join(this.artifactDir(id), fileName);
}
private async writeJsonMeta(
id: string,
fileName: ArtifactMetaFileName,
data: unknown,
): Promise<void> {
await fs.writeFile(this.metaFilePath(id, fileName), JSON.stringify(data, null, 2), "utf8");
}
private async readJsonMeta(
id: string,
fileName: ArtifactMetaFileName,
context: string,
): Promise<unknown | null> {
try {
const raw = await fs.readFile(this.metaFilePath(id, fileName), "utf8");
return JSON.parse(raw) as unknown;
} catch (error) {
if (isFileNotFound(error)) {
return null;
}
this.logger?.warn(`Failed to read standalone diff metadata for ${id}: ${String(error)}`);
this.logger?.warn(`Failed to read ${context} metadata for ${id}: ${String(error)}`);
return null;
}
}

View File

@@ -123,6 +123,25 @@ vi.mock("../agents/openclaw-tools.js", () => {
return { ok: true };
},
},
{
name: "diffs_compat_test",
parameters: {
type: "object",
properties: {
mode: { type: "string" },
fileFormat: { type: "string" },
},
additionalProperties: false,
},
execute: async (_toolCallId: string, args: unknown) => {
const input = (args ?? {}) as Record<string, unknown>;
return {
ok: true,
observedFormat: input.format,
observedFileFormat: input.fileFormat,
};
},
},
];
return {
@@ -546,4 +565,25 @@ describe("POST /tools/invoke", () => {
expect(crashBody.error?.type).toBe("tool_error");
expect(crashBody.error?.message).toBe("tool execution failed");
});
it("passes deprecated format alias through invoke payloads even when schema omits it", async () => {
cfg = {
...cfg,
agents: {
list: [{ id: "main", default: true, tools: { allow: ["diffs_compat_test"] } }],
},
};
const res = await invokeToolAuthed({
tool: "diffs_compat_test",
args: { mode: "file", format: "pdf" },
sessionKey: "main",
});
expect(res.status).toBe(200);
const body = await res.json();
expect(body.ok).toBe(true);
expect(body.result?.observedFormat).toBe("pdf");
expect(body.result?.observedFileFormat).toBeUndefined();
});
});