fix(t2): stop delivery receipts asserting outcomes nobody observed #483
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,16 @@ import { parseCmuxStatusFrame } from "./cmux-status-frame.js"; | |
| import { isCmuxAccessControlDenied } from "./cmux-access-control.js"; | ||
|
|
||
| const execFileAsync = promisify(execFile); | ||
| /** | ||
| * Hard ceiling on one CLI-fallback `cmux` invocation. | ||
| * | ||
| * AIDEV-NOTE (T2 #450): the socket transport is bounded by its own | ||
| * REQUEST_TIMEOUT_MS, but this fallback had none -- a wedged `cmux` | ||
| * subprocess held its caller (notably the delivery snapshot read) forever. | ||
| * Matches the socket client's 10s budget so neither transport can outlast the | ||
| * other. | ||
| */ | ||
| export const CMUX_CLI_EXEC_TIMEOUT_MS = 10_000; | ||
| const STANDARD_BUNDLED_CMUX = | ||
| "/Applications/cmux.app/Contents/Resources/bin/cmux"; | ||
|
|
||
|
|
@@ -55,6 +65,8 @@ interface CmuxClientOptions { | |
| bin?: string; | ||
| env?: NodeJS.ProcessEnv; | ||
| existsSync?: (path: string) => boolean; | ||
| /** Hard ceiling on one CLI-fallback exec; defaults to CMUX_CLI_EXEC_TIMEOUT_MS. */ | ||
| execTimeoutMs?: number; | ||
| } | ||
|
|
||
| interface CmuxIdentifyResult { | ||
|
|
@@ -75,13 +87,18 @@ export class CmuxClient { | |
| private bin?: string; | ||
| private env?: NodeJS.ProcessEnv; | ||
| private existsSync: (path: string) => boolean; | ||
| private execTimeoutMs: number; | ||
| private observerTransportGeneration = 0; | ||
|
|
||
| constructor(opts?: CmuxClientOptions) { | ||
| this.exec = opts?.exec; | ||
| this.bin = opts?.bin; | ||
| this.env = opts?.env; | ||
| this.existsSync = opts?.existsSync ?? fs.existsSync; | ||
| this.execTimeoutMs = Math.max( | ||
| 1, | ||
| opts?.execTimeoutMs ?? CMUX_CLI_EXEC_TIMEOUT_MS, | ||
| ); | ||
|
Comment on lines
+98
to
+101
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- src/cmux-client.ts ---'
sed -n '1,190p' src/cmux-client.ts
printf '%s\n' '--- CmuxClient construction and injected executor references ---'
rg -n -C 3 'new CmuxClient|CmuxClient|execTimeoutMs|ExecFn|execFileAsync' --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: EtanHey/cmuxlayer Length of output: 50374 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- timeout-related tests and option plumbing ---'
rg -n -C 4 'execTimeoutMs|CMUX_CLI_EXEC_TIMEOUT_MS|timeout.*exec|exec.*timeout' src tests README.md package.json 2>/dev/null || true
printf '%s\n' '--- factory option construction ---'
sed -n '20,115p' src/cmux-client-factory.ts
sed -n '3268,3300p' src/server.ts
printf '%s\n' '--- injected executor declarations and call sites outside tests ---'
rg -n -C 2 'exec\?: ExecFn|opts\?\.exec|new CmuxClient\(\{|createServer\(\{' src --glob '*.ts'Repository: EtanHey/cmuxlayer Length of output: 13693 🌐 Web query:
💡 Result: In Node.js, the Citations:
🏁 Script executed: #!/bin/bash
set -eu
node - <<'JS'
const { execFile } = require("node:child_process");
console.log("node", process.version);
for (const value of [NaN, Infinity, -Infinity, -1, 0, 1, 150]) {
await new Promise((resolve) => {
const started = Date.now();
const child = execFile(
process.execPath,
["-e", "setTimeout(() => process.exit(0), 50)"],
{ timeout: value },
(error) => {
console.log(JSON.stringify({
input: String(value),
mathMax: Math.max(1, value),
errorCode: error?.code ?? null,
errorName: error?.name ?? null,
timedOut: error?.killed === true || error?.signal === "SIGTERM",
elapsedMs: Date.now() - started,
}));
resolve();
},
);
child.on("error", () => {});
});
}
JSRepository: EtanHey/cmuxlayer Length of output: 1005 🏁 Script executed: #!/bin/bash
set -eu
node --input-type=module - <<'JS'
import { execFile } from "node:child_process";
console.log("node", process.version);
for (const value of [NaN, Infinity, -Infinity, -1, 0, 1, 150]) {
await new Promise((resolve) => {
const started = Date.now();
const child = execFile(
process.execPath,
["-e", "setTimeout(() => process.exit(0), 50)"],
{ timeout: value },
(error) => {
console.log(JSON.stringify({
input: String(value),
mathMax: Math.max(1, value),
errorCode: error?.code ?? null,
errorName: error?.name ?? null,
timedOut: error?.killed === true || error?.signal === "SIGTERM",
elapsedMs: Date.now() - started,
}));
resolve();
},
);
child.on("error", () => {});
});
}
JSRepository: EtanHey/cmuxlayer Length of output: 1037 🏁 Script executed: #!/bin/bash
set -eu
node --input-type=module - <<'JS'
import { execFile } from "node:child_process";
console.log("node", process.version);
for (const value of [NaN, Infinity, -Infinity, -1, 0, 1, 1.5, 150]) {
const result = {
input: String(value),
mathMax: Math.max(1, value),
};
try {
await new Promise((resolve, reject) => {
const child = execFile(
process.execPath,
["-e", "setTimeout(() => process.exit(0), 50)"],
{ timeout: value },
(error) => {
result.errorCode = error?.code ?? null;
result.errorName = error?.name ?? null;
result.timedOut =
error?.killed === true || error?.signal === "SIGTERM";
resolve();
},
);
child.on("error", reject);
});
} catch (error) {
result.errorCode = error?.code ?? null;
result.errorName = error?.name ?? null;
result.threwSynchronously = true;
}
console.log(JSON.stringify(result));
}
JSRepository: EtanHey/cmuxlayer Length of output: 974 Validate
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| setEnv(env: NodeJS.ProcessEnv | undefined): void { | ||
|
|
@@ -121,6 +138,10 @@ export class CmuxClient { | |
| ) | ||
| : await execFileAsync(bin, cliArgs, { | ||
| ...(env ? { env } : {}), | ||
| timeout: this.execTimeoutMs, | ||
|
coderabbitai[bot] marked this conversation as resolved.
There was a problem hiding this comment. 🟡 Medium When 🚀 Reply "fix it for me" or copy this AI Prompt for your agent:There was a problem hiding this comment. 🟠 High
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| // N2: a subprocess that ignores SIGTERM would still hang the | ||
| // promise the timeout exists to bound. | ||
| killSignal: "SIGKILL", | ||
| }); | ||
| return stdout; | ||
| } catch (error) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
src/agent-engine.ts:7115A deduplicated local ticket is treated as proof that external escalation already happened, so a later occurrence is never sent to
deliveryIssueFilerand is recorded asticket_escalated: falseeven when the earlier filer was absent, failed, or succeeded before its receipt update persisted. Track durable external-escalation state separately from local-ticket creation instead of inferring it fromwritten.created.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: