fix(launch): accepted empty daemon regex matches
Preserved zero-width readiness and wait matches across the daemon wire protocol, and isolated malformed completion events from unrelated pending RPCs. Fixes #7908
This commit is contained in:
@@ -19,6 +19,8 @@
|
||||
|
||||
- Fixed `/reload-plugins` and the Agent Control Center leaving the running session's `task` tool on its create-time agent definitions; refreshed `.omp/agents/*.md` names and descriptions now reach existing tools without restarting ([#7940](https://github.com/can1357/oh-my-pi/issues/7940)).
|
||||
- Fixed legacy Pi extensions failing to load when they call `pi.unregisterProvider()`, and made provider replacement calls take effect immediately after extension runtime initialization ([#7914](https://github.com/can1357/oh-my-pi/issues/7914)).
|
||||
- Fixed zero-width daemon readiness and wait regex matches being rejected by the hub wire decoder; persisted empty-match completion replays can now be acknowledged without rejecting later side-effecting RPCs ([#7908](https://github.com/can1357/oh-my-pi/issues/7908)).
|
||||
|
||||
- Fixed proxy discovery preferring the bundled catalog name over the proxy-reported name, so `omp models refresh` now updates stale display names (e.g. a proxy serving `longcat-2.0` as `"LongCat"` no longer shows the raw id).
|
||||
- Fixed the compiled binary build on Windows: `Bun.Glob.scan` yields backslash-separated paths, which the legacy Pi virtual module used verbatim for export keys and generated identifiers, producing invalid JavaScript.
|
||||
- Fixed Ctrl+O (`app.tools.expand`) not expanding truncated tool output while a tool-approval prompt or other selection dialog held keyboard focus, by promoting the shortcut to a global input listener that fires regardless of focus (it still defers to fullscreen overlays and the tree selector's own Ctrl+O filter cycle) ([#7837](https://github.com/can1357/oh-my-pi/issues/7837)).
|
||||
|
||||
@@ -367,7 +367,17 @@ class SocketDaemonClient implements DaemonBrokerClient {
|
||||
try {
|
||||
message = parseDaemonWireMessage(decoded);
|
||||
} catch (error) {
|
||||
this.#rejectPending(error instanceof Error ? error : new Error(String(error)));
|
||||
const parseError = error instanceof Error ? error : new Error(String(error));
|
||||
if (
|
||||
typeof decoded === "object" &&
|
||||
decoded !== null &&
|
||||
"event" in decoded &&
|
||||
decoded.event === "daemon-completed"
|
||||
) {
|
||||
logger.warn("Ignoring malformed daemon completion", { error: parseError.message });
|
||||
continue;
|
||||
}
|
||||
this.#rejectPending(parseError);
|
||||
continue;
|
||||
}
|
||||
if ("event" in message) {
|
||||
|
||||
@@ -168,6 +168,11 @@ function optionalString(value: unknown, label: string): string | undefined {
|
||||
return stringValue(value, label);
|
||||
}
|
||||
|
||||
function optionalRawString(value: unknown, label: string): string | undefined {
|
||||
if (value === undefined) return undefined;
|
||||
return rawString(value, label);
|
||||
}
|
||||
|
||||
function booleanValue(value: unknown, label: string): boolean {
|
||||
if (typeof value !== "boolean") throw new Error(`${label} must be a boolean`);
|
||||
return value;
|
||||
@@ -272,7 +277,7 @@ export function parseDaemonSnapshot(value: unknown): DaemonSnapshot {
|
||||
restartCount: numberValue(source.restartCount, "daemon.restartCount"),
|
||||
outputBytes: numberValue(source.outputBytes, "daemon.outputBytes"),
|
||||
owner: optionalString(source.owner, "daemon.owner"),
|
||||
readyMatch: optionalString(source.readyMatch, "daemon.readyMatch"),
|
||||
readyMatch: optionalRawString(source.readyMatch, "daemon.readyMatch"),
|
||||
readyPending: source.readyPending === undefined ? undefined : readyPendingList(source.readyPending),
|
||||
persist: booleanValue(source.persist, "daemon.persist"),
|
||||
detached: source.detached === undefined ? false : booleanValue(source.detached, "daemon.detached"),
|
||||
@@ -427,7 +432,7 @@ export function parseDaemonRpcResult(operation: DaemonOperation, value: unknown)
|
||||
return {
|
||||
op: "wait",
|
||||
daemon: parseDaemonSnapshot(source.daemon),
|
||||
matched: optionalString(source.matched, "result.matched"),
|
||||
matched: optionalRawString(source.matched, "result.matched"),
|
||||
timedOut: booleanValue(source.timedOut, "result.timedOut"),
|
||||
};
|
||||
case "send":
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { type DaemonOperation, parseDaemonRpcResult, parseDaemonWireRequest } from "../../src/launch/protocol";
|
||||
import {
|
||||
type DaemonOperation,
|
||||
parseDaemonRpcResult,
|
||||
parseDaemonSnapshot,
|
||||
parseDaemonWireRequest,
|
||||
} from "../../src/launch/protocol";
|
||||
|
||||
const operation: Extract<DaemonOperation, { op: "logs" }> = {
|
||||
op: "logs",
|
||||
@@ -18,6 +23,18 @@ const baseResult = {
|
||||
state: "running" as const,
|
||||
};
|
||||
|
||||
const baseSnapshot = {
|
||||
name: "web",
|
||||
id: "daemon-1",
|
||||
state: "ready" as const,
|
||||
createdAt: 1,
|
||||
startedAt: 1,
|
||||
restartCount: 0,
|
||||
outputBytes: 5,
|
||||
persist: false,
|
||||
detached: false,
|
||||
};
|
||||
|
||||
describe("launch logs protocol", () => {
|
||||
it("decodes terminal rows without changing their bytes", () => {
|
||||
const terminalRows = ["\x1b[0m\x1b[1;38;5;2mready", "", "界e\u0301"];
|
||||
@@ -74,3 +91,31 @@ describe("launch logs compatibility", () => {
|
||||
expect("terminalText" in result ? result.terminalText : undefined).toBe("progress\rready");
|
||||
});
|
||||
});
|
||||
|
||||
describe("regex-derived protocol fields", () => {
|
||||
it("preserves an empty readiness match", () => {
|
||||
expect(parseDaemonSnapshot({ ...baseSnapshot, readyMatch: "" }).readyMatch).toBe("");
|
||||
});
|
||||
|
||||
it("preserves an empty wait pattern match", () => {
|
||||
const waitOperation: Extract<DaemonOperation, { op: "wait" }> = {
|
||||
op: "wait",
|
||||
name: "web",
|
||||
for: "ready",
|
||||
pattern: "^",
|
||||
timeoutMs: 1_000,
|
||||
};
|
||||
expect(
|
||||
parseDaemonRpcResult(waitOperation, {
|
||||
daemon: baseSnapshot,
|
||||
matched: "",
|
||||
timedOut: false,
|
||||
}),
|
||||
).toEqual({
|
||||
op: "wait",
|
||||
daemon: baseSnapshot,
|
||||
matched: "",
|
||||
timedOut: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -171,6 +171,61 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("daemon broker", () => {
|
||||
it("keeps a valid RPC response authoritative after a malformed completion", async () => {
|
||||
const projectDir = await tempDir("omp-daemon-malformed-completion-project-");
|
||||
const runtimeDir = await tempDir("omp-daemon-malformed-completion-runtime-");
|
||||
const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 });
|
||||
const server = net.createServer(socket => {
|
||||
let buffer = "";
|
||||
socket.setEncoding("utf8");
|
||||
socket.on("data", chunk => {
|
||||
buffer += chunk;
|
||||
const newline = buffer.indexOf("\n");
|
||||
if (newline < 0) return;
|
||||
const request: unknown = JSON.parse(buffer.slice(0, newline));
|
||||
if (
|
||||
typeof request !== "object" ||
|
||||
request === null ||
|
||||
!("id" in request) ||
|
||||
typeof request.id !== "string"
|
||||
) {
|
||||
socket.destroy(new Error("request id missing"));
|
||||
return;
|
||||
}
|
||||
socket.write(
|
||||
`${JSON.stringify({
|
||||
event: "daemon-completed",
|
||||
completionId: "malformed-completion",
|
||||
owner: "completion-owner",
|
||||
daemon: null,
|
||||
})}\n`,
|
||||
);
|
||||
socket.write(
|
||||
`${JSON.stringify({
|
||||
id: request.id,
|
||||
ok: true,
|
||||
result: { projectDir },
|
||||
})}\n`,
|
||||
);
|
||||
});
|
||||
});
|
||||
const listening = Promise.withResolvers<void>();
|
||||
server.once("error", listening.reject);
|
||||
server.listen(daemonBrokerEndpoint(projectDir, runtimeDir), listening.resolve);
|
||||
await listening.promise;
|
||||
try {
|
||||
expect(await client.request({ op: "ping" })).toEqual({ op: "ping", projectDir });
|
||||
} finally {
|
||||
client.close();
|
||||
const closed = Promise.withResolvers<void>();
|
||||
server.close(error => {
|
||||
if (error) closed.reject(error);
|
||||
else closed.resolve();
|
||||
});
|
||||
await closed.promise;
|
||||
}
|
||||
});
|
||||
|
||||
it("shares PTY output and input across project clients", async () => {
|
||||
const projectDir = await tempDir("omp-daemon-project-");
|
||||
const runtimeDir = await tempDir("omp-daemon-runtime-");
|
||||
@@ -756,6 +811,90 @@ esac
|
||||
}
|
||||
}, 12_000);
|
||||
|
||||
it("replays a zero-width completion without poisoning the next start", async () => {
|
||||
const projectDir = await tempDir("omp-daemon-empty-ready-project-");
|
||||
const runtimeDir = await tempDir("omp-daemon-empty-ready-runtime-");
|
||||
const markerPath = path.join(projectDir, "victim-ran");
|
||||
const owner = "empty-ready-owner";
|
||||
const first = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 });
|
||||
let recovered: DaemonBrokerClient | undefined;
|
||||
let victimError: Error | undefined;
|
||||
try {
|
||||
first.onCompletion(owner, () => {
|
||||
throw new Error("leave completion pending for reconnect");
|
||||
});
|
||||
await first.request({ op: "ping" });
|
||||
await first
|
||||
.request({
|
||||
op: "start",
|
||||
spec: {
|
||||
name: "empty-ready-poison",
|
||||
application: process.execPath,
|
||||
args: ["-e", 'console.log("READY")'],
|
||||
env: {},
|
||||
cwd: projectDir,
|
||||
pty: false,
|
||||
ready: { log: "^", timeoutMs: 5_000 },
|
||||
restart: "no",
|
||||
persist: false,
|
||||
detached: false,
|
||||
},
|
||||
owner,
|
||||
})
|
||||
.catch(() => undefined);
|
||||
const metaPath = path.join(runtimeDir, "daemons", "empty-ready-poison", "meta.json");
|
||||
expect(
|
||||
await waitUntil(async () => {
|
||||
const metadata: unknown = await Bun.file(metaPath).json();
|
||||
return (
|
||||
typeof metadata === "object" &&
|
||||
metadata !== null &&
|
||||
"completionPending" in metadata &&
|
||||
metadata.completionPending === true
|
||||
);
|
||||
}, 3_000),
|
||||
).toBeTrue();
|
||||
first.close();
|
||||
|
||||
const completions: DaemonSnapshot[] = [];
|
||||
recovered = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 });
|
||||
recovered.onCompletion(owner, notification => {
|
||||
completions.push(notification.daemon);
|
||||
});
|
||||
const victim = await recovered
|
||||
.request({
|
||||
op: "start",
|
||||
spec: {
|
||||
name: "empty-ready-victim",
|
||||
application: process.execPath,
|
||||
args: ["-e", `await Bun.write(${JSON.stringify(markerPath)}, "yes"); console.log("SECOND")`],
|
||||
env: {},
|
||||
cwd: projectDir,
|
||||
pty: false,
|
||||
ready: { log: "SECOND", timeoutMs: 5_000 },
|
||||
restart: "no",
|
||||
persist: false,
|
||||
detached: false,
|
||||
},
|
||||
})
|
||||
.catch(error => {
|
||||
victimError = error instanceof Error ? error : new Error(String(error));
|
||||
return undefined;
|
||||
});
|
||||
|
||||
expect(await waitUntil(() => Bun.file(markerPath).exists(), 3_000)).toBeTrue();
|
||||
expect(await Bun.file(markerPath).text()).toBe("yes");
|
||||
expect(victimError).toBeUndefined();
|
||||
if (victim?.op !== "start") throw new Error("victim start result missing");
|
||||
expect(victim.daemon).toMatchObject({ name: "empty-ready-victim", readyMatch: "SECOND" });
|
||||
expect(await waitUntil(() => completions.length === 1, 2_000)).toBeTrue();
|
||||
expect(completions[0]).toMatchObject({ name: "empty-ready-poison", readyMatch: "", state: "exited" });
|
||||
} finally {
|
||||
first.close();
|
||||
if (recovered) await shutdown(recovered);
|
||||
}
|
||||
}, 12_000);
|
||||
|
||||
it("replays a recovered non-detached daemon exit", async () => {
|
||||
if (process.platform === "win32") return;
|
||||
const projectDir = await tempDir("omp-daemon-attached-recovery-project-");
|
||||
|
||||
Reference in New Issue
Block a user