From b8f89bcdc8eddc788fe48edbbc7bfafe041ac83c Mon Sep 17 00:00:00 2001 From: RoboClaw Date: Fri, 18 Sep 2026 09:04:50 -0700 Subject: [PATCH] fix(test): preserve borrowed CLI diagnostic emitters (#151918) * fix(test): preserve borrowed CLI diagnostic emitters Restore explicit receiver forwarding for the captured process emitter and scope the intentional unbound-method lint exception to the preload. Cover ordinary exit and borrowed dispatch without changing production behavior. Co-authored-by: steipete <58493+steipete@users.noreply.github.com> * fix(test): preserve borrowed CLI diagnostic emitters Worked on by: - @steipete Co-authored-by: steipete <58493+steipete@users.noreply.github.com> OpenClaw-Publication: 156b108b-69f4-4e9f-9f4d-bf4c235a99a3 --------- Co-authored-by: steipete <58493+steipete@users.noreply.github.com> --- .oxlintrc.json | 8 ++ .../cli-process-diagnostics.test-support.cjs | 9 +- src/cli/cli-process-diagnostics.test.ts | 130 ++++++++++++++++++ 3 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 src/cli/cli-process-diagnostics.test.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 9928e7cbc062..0abd52ef9c86 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -292,6 +292,14 @@ "typescript/no-explicit-any": "off" } }, + { + "files": ["src/cli/cli-process-diagnostics.test-support.cjs"], + "rules": { + // The saved emitter is always invoked with the caller's receiver via Reflect.apply. + // CJS semantic discovery varies by lint target set; an inline disable can appear unused. + "typescript/unbound-method": "off" + } + }, { "files": ["src/agents/subagents/registry/subagent-registry.ts"], "rules": { diff --git a/src/cli/cli-process-diagnostics.test-support.cjs b/src/cli/cli-process-diagnostics.test-support.cjs index 8a4aaca9315f..19fe66dcf098 100644 --- a/src/cli/cli-process-diagnostics.test-support.cjs +++ b/src/cli/cli-process-diagnostics.test-support.cjs @@ -10,7 +10,8 @@ const startedAt = Date.now(); // An exit-time native wait cannot service the later SIGUSR2 diagnostic request. if (process.execArgv.includes("--trace-exit") && require("node:worker_threads").isMainThread) { const { writeSync } = require("node:fs"); - const emit = process.emit.bind(process); + // Keep the original method unbound: borrowed calls must retain their own receiver. + const emit = process.emit; const writeExitBoundary = (phase, exitCode) => { try { const listeners = phase === "exit-listeners-enter" ? process.listeners("exit") : undefined; @@ -35,12 +36,12 @@ if (process.execArgv.includes("--trace-exit") && require("node:worker_threads"). } }; process.emit = function (event, ...args) { - if (event !== "exit") { - return emit(event, ...args); + if (this !== process || event !== "exit") { + return Reflect.apply(emit, this, [event, ...args]); } writeExitBoundary("exit-listeners-enter", args[0]); try { - const result = emit(event, ...args); + const result = Reflect.apply(emit, this, [event, ...args]); writeExitBoundary("exit-listeners-return", args[0]); return result; } catch (error) { diff --git a/src/cli/cli-process-diagnostics.test.ts b/src/cli/cli-process-diagnostics.test.ts new file mode 100644 index 000000000000..efd272bd3df0 --- /dev/null +++ b/src/cli/cli-process-diagnostics.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it } from "vitest"; +import { formatCliProcessFailure, runCliProcessChild } from "./cli-process-child.test-helpers.js"; + +const diagnosticPrefix = "[cli-process-diagnostics] "; + +function exitBoundaries(stderr: string): unknown[] { + return stderr + .split("\n") + .filter((line) => line.startsWith(`${diagnosticPrefix}{`)) + .map((line) => JSON.parse(line.slice(diagnosticPrefix.length))); +} + +describe.skipIf(process.platform === "win32" || Boolean(process.versions.bun))( + "CLI process exit diagnostics", + () => { + it.each(["natural", "explicit"])("preserves %s process exit", async (mode) => { + const result = await runCliProcessChild({ + nodeArgs: [ + "--trace-exit", + "-e", + ` +process.on('exit', function fixtureExit(code) { + console.log(JSON.stringify({ pid: process.pid, code, receiver: this === process })); +}); +${mode === "explicit" ? "process.exit(3);" : "process.exitCode = 3;"} +`, + ], + env: { ...process.env, NODE_OPTIONS: undefined }, + }); + const failure = formatCliProcessFailure({ reason: `${mode} exit diagnostics`, ...result }); + expect(result.signal, failure).toBeNull(); + expect(result.code, failure).toBe(3); + const output = JSON.parse(result.stdout); + expect(output, failure).toMatchObject({ code: 3, receiver: true }); + expect(exitBoundaries(result.stderr), failure).toMatchObject([ + { + pid: output.pid, + phase: "exit-listeners-enter", + exitCode: 3, + listenerNames: ["fixtureExit"], + }, + { pid: output.pid, phase: "exit-listeners-return", exitCode: 3 }, + ]); + if (mode === "explicit") { + expect( + result.stderr.indexOf("WARNING: Exited the environment with code 3"), + failure, + ).toBeGreaterThan(result.stderr.indexOf('"phase":"exit-listeners-return"')); + } + }); + + it("forwards borrowed receivers, arguments, return values, and thrown errors without exit diagnostics", async () => { + const result = await runCliProcessChild({ + nodeArgs: [ + "--trace-exit", + "-e", + ` +const assert = require('node:assert/strict'); +const { EventEmitter } = require('node:events'); +const receiver = new EventEmitter(); +const event = Symbol('fixture-event'); +const payload = {}; +const failure = new Error('fixture-listener-failed'); +receiver.on(event, function (value, second) { + assert.equal(this, receiver); + assert.equal(value, payload); + assert.equal(second, 7); +}); +receiver.on('exit', function (code) { + assert.equal(this, receiver); + assert.equal(code, 41); +}); +assert.equal(Reflect.apply(process.emit, receiver, [event, payload, 7]), true); +assert.equal(Reflect.apply(process.emit, receiver, ['missing']), false); +assert.equal(Reflect.apply(process.emit, receiver, ['exit', 41]), true); +receiver.on('exit', () => { throw failure; }); +assert.throws(() => Reflect.apply(process.emit, receiver, ['exit', 41]), (error) => error === failure); +process.on(event, function (value) { + assert.equal(this, process); + assert.equal(value, payload); +}); +assert.equal(process.emit(event, payload), true); +assert.equal(process.emit('fixture-missing'), false); +console.log('forwarded'); +`, + ], + env: { ...process.env, NODE_OPTIONS: undefined }, + }); + const failure = formatCliProcessFailure({ reason: "borrowed emit diagnostics", ...result }); + expect(result.signal, failure).toBeNull(); + expect(result.code, failure).toBe(0); + expect(result.stdout, failure).toBe("forwarded\n"); + // Only the child's real exit is instrumented, never a borrowed or non-exit dispatch. + expect(exitBoundaries(result.stderr), failure).toMatchObject([ + { phase: "exit-listeners-enter", exitCode: 0 }, + { phase: "exit-listeners-return", exitCode: 0 }, + ]); + }); + + it("records throwing process exit listeners without replacing their error", async () => { + const result = await runCliProcessChild({ + nodeArgs: [ + "--trace-exit", + "-e", + ` +const assert = require('node:assert/strict'); +const failure = new Error('fixture-exit-failed'); +process.once('exit', function fixtureThrow() { + assert.equal(this, process); + throw failure; +}); +assert.throws(() => process.emit('exit', 9), (error) => error === failure); +console.log('preserved error'); +`, + ], + env: { ...process.env, NODE_OPTIONS: undefined }, + }); + const failure = formatCliProcessFailure({ reason: "throwing exit diagnostics", ...result }); + expect(result.signal, failure).toBeNull(); + expect(result.code, failure).toBe(0); + expect(result.stdout, failure).toBe("preserved error\n"); + expect(exitBoundaries(result.stderr), failure).toMatchObject([ + { phase: "exit-listeners-enter", exitCode: 9 }, + { phase: "exit-listeners-throw", exitCode: 9 }, + { phase: "exit-listeners-enter", exitCode: 0 }, + { phase: "exit-listeners-return", exitCode: 0 }, + ]); + }); + }, +);