diff --git a/packages/op-vscode/src/vscode/pen-editor-provider.ts b/packages/op-vscode/src/vscode/pen-editor-provider.ts index 6b7c65583..1ce8c3edd 100644 --- a/packages/op-vscode/src/vscode/pen-editor-provider.ts +++ b/packages/op-vscode/src/vscode/pen-editor-provider.ts @@ -15,6 +15,7 @@ import type { BridgeOutboundToPage } from "../protocol/bridge"; import { encodeOutbound } from "../protocol/bridge"; import { buildBootHtml, buildWebviewHtml } from "./webview-shell"; import { pickRestartSource, resolveDaemonBinary, type LatestDurable } from "./restart-source"; +import { isShellControl, parseShellReadyOrigin } from "./shell-messages"; const SHELL_READY_TIMEOUT_MS = 5_000; const DAEMON_READY_TIMEOUT_MS = 10_000; @@ -263,6 +264,7 @@ export class PenEditorProvider implements vscode.CustomEditorProvider { const uri = this.backupUri(name); + await ensureParentDir(uri); await vscode.workspace.fs.writeFile(uri, bytes); }, writeBackupFallback: async (bytes) => { @@ -337,6 +339,7 @@ export class PenEditorProvider implements vscode.CustomEditorProvider { const bytes = await this.requireSession(document).backup(() => cancellation.isCancellationRequested); + await ensureParentDir(context.destination); // VS Code: the backup dir may not exist await vscode.workspace.fs.writeFile(context.destination, bytes); document.latestDurable = { source: "backup", json: decode(bytes) }; return { @@ -376,6 +379,13 @@ function decode(bytes: Uint8Array): string { return decoder.decode(bytes); } +/** Ensure a URI's parent directory exists (createDirectory is recursive and a + * no-op if present). VS Code does not guarantee storage / backup-destination + * dirs exist on a fresh profile. */ +async function ensureParentDir(uri: vscode.Uri): Promise { + await vscode.workspace.fs.createDirectory(vscode.Uri.joinPath(uri, "..")); +} + /** Atomic write: temp file + rename, so a crash mid-write can't truncate. */ async function atomicWrite(uri: vscode.Uri, bytes: Uint8Array): Promise { const tmp = uri.with({ path: `${uri.path}.op-tmp` }); @@ -393,22 +403,6 @@ function delay(ms: number): Promise { return new Promise((r) => setTimeout(r, ms)); } -/** Parse an op-shell/ready control message → its reported origin, else undefined. */ -function parseShellReadyOrigin(raw: unknown): string | undefined { - if (typeof raw !== "string") return undefined; - try { - const v = JSON.parse(raw) as { type?: unknown; origin?: unknown }; - if (v.type === "op-shell/ready" && typeof v.origin === "string") return v.origin; - } catch { - /* not a control message */ - } - return undefined; -} - -function isShellControl(raw: unknown): boolean { - return typeof raw === "string" && raw.includes("op-shell/"); -} - function errorPage(detail: string): string { const safe = detail.replace(/[<&]/g, (c) => (c === "<" ? "<" : "&")); return ` diff --git a/packages/op-vscode/src/vscode/shell-messages.test.ts b/packages/op-vscode/src/vscode/shell-messages.test.ts new file mode 100644 index 000000000..64b841deb --- /dev/null +++ b/packages/op-vscode/src/vscode/shell-messages.test.ts @@ -0,0 +1,38 @@ +import { test, expect } from "bun:test"; +import { isShellControl, parseShellReadyOrigin } from "./shell-messages"; + +test("isShellControl matches only a top-level op-shell/ type", () => { + expect(isShellControl(JSON.stringify({ type: "op-shell/ready", origin: "x" }))).toBe(true); + expect(isShellControl(JSON.stringify({ type: "op-bridge/snapshot-result", requestId: "r" }))).toBe(false); +}); + +test("isShellControl does NOT drop a snapshot whose docJson embeds op-shell/", () => { + // Regression: a legitimate snapshot-result carrying "op-shell/" inside its + // docJson must reach the session — a substring check would drop it and hang + // the awaiting save/backup. + const msg = JSON.stringify({ + type: "op-bridge/snapshot-result", + requestId: "r1", + docJson: '{"text":"see op-shell/ready docs"}', + generation: 2, + revision: 1, + }); + expect(msg).toContain("op-shell/"); + expect(isShellControl(msg)).toBe(false); +}); + +test("isShellControl rejects non-string / non-JSON / typeless payloads", () => { + expect(isShellControl(123)).toBe(false); + expect(isShellControl("not json")).toBe(false); + expect(isShellControl(JSON.stringify({ origin: "x" }))).toBe(false); + expect(isShellControl(JSON.stringify({ type: 42 }))).toBe(false); +}); + +test("parseShellReadyOrigin extracts the origin from a ready message", () => { + expect(parseShellReadyOrigin(JSON.stringify({ type: "op-shell/ready", origin: "vscode-webview://x" }))).toBe( + "vscode-webview://x", + ); + expect(parseShellReadyOrigin(JSON.stringify({ type: "op-shell/ready" }))).toBeUndefined(); + expect(parseShellReadyOrigin(JSON.stringify({ type: "op-bridge/opened", generation: 1 }))).toBeUndefined(); + expect(parseShellReadyOrigin("not json")).toBeUndefined(); +}); diff --git a/packages/op-vscode/src/vscode/shell-messages.ts b/packages/op-vscode/src/vscode/shell-messages.ts new file mode 100644 index 000000000..59119d475 --- /dev/null +++ b/packages/op-vscode/src/vscode/shell-messages.ts @@ -0,0 +1,30 @@ +// Pure classification of the shell's control messages (op-shell/*). Kept +// vscode-free and testable: a mis-classification here can drop a legitimate +// snapshot whose docJson happens to contain the text "op-shell/", hanging the +// save/backup that awaits it — so the match must be on the parsed top-level +// `type`, never a substring of the raw payload. + +/** True only when `raw` is a JSON string whose top-level `type` is an + * op-shell/* control type. Business messages (op-bridge/*) and any payload + * that merely embeds "op-shell/" inside its data are NOT control traffic. */ +export function isShellControl(raw: unknown): boolean { + if (typeof raw !== "string") return false; + try { + const v = JSON.parse(raw) as { type?: unknown }; + return typeof v.type === "string" && v.type.startsWith("op-shell/"); + } catch { + return false; + } +} + +/** The origin reported by an op-shell/ready control message, else undefined. */ +export function parseShellReadyOrigin(raw: unknown): string | undefined { + if (typeof raw !== "string") return undefined; + try { + const v = JSON.parse(raw) as { type?: unknown; origin?: unknown }; + if (v.type === "op-shell/ready" && typeof v.origin === "string") return v.origin; + } catch { + /* not a control message */ + } + return undefined; +}