From 1f5fb31606e4341377c193712a21c1891c88ef4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 13:12:34 +0200 Subject: [PATCH 1/2] test(gap): lock commander's outputError/writeErr indirection (#10711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #10711 reports that a function read from an object property silently drops its own call to a second function passed to it as a parameter — commander's `_displayError` shape, where `outputError(str, write)` invokes the `writeErr` it was handed: this._outputConfiguration.outputError( message, this._outputConfiguration.writeErr); It does not reproduce. The reporter's own isolated repro prints the expected text on all three trees that matter — current main (v0.5.1598), the main commit their branch forks from (8df83f8c12), and their actual tree (PR #10712 on top of #10699, head 463c4fa5) — and real commander 14.0.3 compiled from source via `perry.compilePackages` matches Node 26.5.1 byte for byte across the whole output surface the issue names: `--help`, `--version`, missing required argument, unknown option, unknown command and `program.error()`, under both the default output configuration and a `configureOutput()` override. 32 further shapes of the same indirection agree with Node too. So this adds the regression lock rather than a fix. The shape is worth gating: #10689 — an inherited property read folding to the constant `undefined` on a scalar-replaced object — landed one commit before this issue was filed and is the same family, silent in the same way. The fixture covers the reported form verbatim plus the method-shorthand, class-field, `configureOutput`-override, spread, nested-receiver, cross-object-writer and in-loop spellings. Two of the cases exist to keep the fixture from passing vacuously. One traces `before` / `typeof write` / `after` around the inner call, so "the outer body ran and the inner call evaporated" cannot read as a pass. The other omits the writer entirely and asserts a TypeError: that a missing callee is LOUD is the property that keeps this bug class from ever presenting as a plausible wrong answer. Every writer sinks to stdout because the parity harness merges stdout and stderr into one compared stream; the stream is incidental to the indirection. Refs #10711 --- ...st_gap_10711_property_fn_param_callback.ts | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 test-files/test_gap_10711_property_fn_param_callback.ts diff --git a/test-files/test_gap_10711_property_fn_param_callback.ts b/test-files/test_gap_10711_property_fn_param_callback.ts new file mode 100644 index 0000000000..05bf4c13f5 --- /dev/null +++ b/test-files/test_gap_10711_property_fn_param_callback.ts @@ -0,0 +1,127 @@ +// #10711: a function read from an object property must not drop its own call +// to a second function handed to it as a parameter (also read from an object +// property). +// +// This is commander's `_displayError` shape. `lib/command.js` builds a default +// output configuration holding two function properties — +// +// writeErr: (str) => process.stderr.write(str), +// outputError: (str, write) => write(str), +// +// — and every error path calls +// `this._outputConfiguration.outputError(msg, this._outputConfiguration.writeErr)`: +// an object-property function invoking a SECOND object-property function that +// was passed to it as a parameter. If the inner `write(str)` evaporates, the +// program still runs and still throws the right CommanderError — it just +// prints nothing. A silently dropped call is the worst failure mode there is, +// so this fixture asserts the inner call RAN, not merely that something got +// printed. +// +// Every writer sinks to stdout on purpose. The parity harness merges stdout +// and stderr into one compared stream, so a fixture that used both would race +// on the interleaving; the stream is incidental to the indirection under test. + +function out(s: string): void { + process.stdout.write(s); +} + +// ── 1. The reported shape, verbatim: object-literal arrow properties, both +// reached through one level of plain-function call. ────────────────── +const config: any = { + writeErr: (str: string) => out(str), + outputError: (str: string, write: (s: string) => void) => write(str), +}; + +function fireError(cfg: any, message: string) { + cfg.outputError(message, cfg.writeErr); +} + +fireError(config, "1 verbatim: error: something went wrong\n"); + +// ── 2. The inner call is observably entered and left. Printing "before" and +// "after" around it is what separates "the call ran" from "the outer +// body ran and the inner call vanished" — the two look identical when +// only the payload is checked. ────────────────────────────────────── +const traced: any = { + writeErr: (str: string) => out(" inner: " + str), + outputError: (str: string, write: (s: string) => void) => { + out("2 before, typeof write=" + typeof write + "\n"); + write(str); + out("2 after\n"); + }, +}; +fireError(traced, "payload\n"); + +// ── 3. Method-shorthand spelling of the same object. ──────────────────────── +const shorthand: any = { + writeErr(str: string) { + out(str); + }, + outputError(str: string, write: (s: string) => void) { + write(str); + }, +}; +fireError(shorthand, "3 shorthand: error: something went wrong\n"); + +// ── 4. commander's real home for it: a class field holding the config, the +// receiver reached as `this.` inside a method. ──────────────── +class Reporter { + _outputConfiguration: any = { + writeOut: (str: string) => out(str), + writeErr: (str: string) => out(str), + outputError: (str: string, write: (s: string) => void) => write(str), + getOutHelpWidth: () => 80, + }; + configureOutput(cfg: any): Reporter { + Object.assign(this._outputConfiguration, cfg); + return this; + } + error(message: string): void { + this._outputConfiguration.outputError( + `${message}\n`, + this._outputConfiguration.writeErr, + ); + } +} + +const reporter = new Reporter(); +reporter.error("4 class field: error: something went wrong"); + +// ── 5. `configureOutput` replaces the writer after construction — the call +// must reach the REPLACEMENT, not a value baked in at literal-creation +// time. ──────────────────────────────────────────────────────────── +reporter.configureOutput({ writeErr: (str: string) => out("[override]" + str) }); +reporter.error("5 after configureOutput"); + +// ── 6. Spread-built config, nested receiver, and a writer taken from a +// DIFFERENT object than the one holding `outputError`. ────────────── +const defaults: any = { + writeErr: (str: string) => out(str), + outputError: (str: string, write: (s: string) => void) => write(str), +}; +const nested: any = { io: { ...defaults } }; +nested.io.outputError("6 nested spread: ok\n", nested.io.writeErr); + +const sink: any = { writeErr: (str: string) => out("[other]" + str) }; +nested.io.outputError("6 cross-object writer\n", sink.writeErr); + +// ── 7. Repeated dispatch: the shape must survive a loop, where the call site +// is re-entered and any per-site caching gets a second look. ───────── +for (let i = 0; i < 3; i++) { + fireError(config, "7 loop " + i + "\n"); +} + +// ── 8. And when the writer really is missing, the call must be LOUD. A +// TypeError here is the property that keeps every future instance of +// this bug class from presenting as a plausible wrong answer. (Only the +// error's name is printed: Node names the callee — "write is not a +// function" — where Perry says "value is not a function".) ─────────── +const noWriter: any = { + outputError: (str: string, write: (s: string) => void) => write(str), +}; +try { + fireError(noWriter, "never printed\n"); + out("8 MISSING WRITER SILENTLY DROPPED THE CALL\n"); +} catch (e: any) { + out("8 threw " + e.name + "\n"); +} From 8e80b8cb870c0c17c4472c60c497e1cb43af5d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 13:14:17 +0200 Subject: [PATCH 2/2] changelog: fragment for #10728 (property-fn param callback lock) --- .../10728-property-fn-param-callback-lock.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 changelog.d/10728-property-fn-param-callback-lock.md diff --git a/changelog.d/10728-property-fn-param-callback-lock.md b/changelog.d/10728-property-fn-param-callback-lock.md new file mode 100644 index 0000000000..4a1808439d --- /dev/null +++ b/changelog.d/10728-property-fn-param-callback-lock.md @@ -0,0 +1,32 @@ +### Testing + +- Lock commander's `_displayError` indirection in the gap suite: an object-property + function (`outputError(str, write)`) invoking a second object-property function + handed to it as a parameter (`writeErr`). #10711 reports that Perry silently drops + that inner call and loses commander's error text; it does not reproduce. The + reporter's own isolated repro matches Node 26.5.1 on current `main` (v0.5.1598), on + the `main` commit their branch forks from (`8df83f8c`), and on their actual tree + (PR #10712 over #10699) — each a full `-p perry -p perry-runtime-static + -p perry-stdlib-static` build with `PERRY_RUNTIME_DIR` pinned, so no arm could have + linked a stale archive. Real commander 14.0.3 compiled from source through + `perry.compilePackages` is byte-identical to Node across the whole surface the issue + names — `--help`, `--version`, missing required argument, unknown option, unknown + command and `program.error()` — under the default output configuration and under a + `configureOutput()` override, as are 32 further spellings of the same indirection + (method shorthand, class field, spread, nested receiver, cross-object writer, + computed key, getter, `Object.create` chain, `Object.freeze`, destructuring, three + levels, async caller, nested closure, loop, and the shape inside a CommonJS module). + + The fixture is therefore a regression lock, not a fix, and it passes on unfixed + `main`. The shape still earns a gate: #10689 — an inherited property read folding to + the constant `undefined` on a scalar-replaced object — landed one commit before + #10711 was filed, is the same family, and was silent in the same way, and nothing in + `test-files/` covered this indirection. + + Two cases keep it from passing vacuously. One traces `before` / `typeof write` / + `after` around the inner call so that "the outer body ran and the inner call + evaporated" cannot read as a pass. The other omits the writer and asserts a + `TypeError`, because a missing callee being loud is the property that keeps this bug + class from presenting as a plausible wrong answer rather than a crash. Every writer + sinks to stdout: the parity harness merges stdout and stderr into one compared + stream, so a fixture using both would race on the interleaving.