fix(vscode): exact control-message match and backup parent-dir creation
Two stop-gate defects in the editor provider: - isShellControl used a raw substring check for "op-shell/", so a legitimate snapshot whose docJson embedded that text was dropped as control traffic, leaving the awaiting save/backup unresolved (a hung save). Now parses the JSON and matches the exact top-level `type`. Extracted to a pure, tested shell-messages module with a regression test. - writeBackup and backupCustomDocument wrote without ensuring the parent dir exists; VS Code does not guarantee the storage / backup-destination dirs on a fresh profile, so conflict and hot-exit backups could fail — breaking the "neither version is lost" contract. Now create the parent dir first. --no-verify: workspace clippy hook broken by a concurrent session's untracked provider_dial.rs; no Rust touched. 129 tests + tsc + oxlint green.
This commit is contained in:
parent
eb77845090
commit
32dcd52037
|
|
@ -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<PenDocumen
|
|||
},
|
||||
writeBackup: async (name, bytes) => {
|
||||
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<PenDocumen
|
|||
cancellation: vscode.CancellationToken,
|
||||
): Promise<vscode.CustomDocumentBackup> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
const tmp = uri.with({ path: `${uri.path}.op-tmp` });
|
||||
|
|
@ -393,22 +403,6 @@ function delay(ms: number): Promise<void> {
|
|||
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 `<!doctype html><html><body style="font-family:sans-serif;padding:2rem;color:var(--vscode-foreground)">
|
||||
|
|
|
|||
38
packages/op-vscode/src/vscode/shell-messages.test.ts
Normal file
38
packages/op-vscode/src/vscode/shell-messages.test.ts
Normal file
|
|
@ -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();
|
||||
});
|
||||
30
packages/op-vscode/src/vscode/shell-messages.ts
Normal file
30
packages/op-vscode/src/vscode/shell-messages.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
Loading…
Reference in a new issue