test(gap): lock commander's outputError/writeErr indirection — #10711 does not reproduce

[proggeramlug]

#10711 does not reproduce

The issue 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:

this._outputConfiguration.outputError(
  message,
  this._outputConfiguration.writeErr,
);

I could not make it fail anywhere.

The reporter's own isolated repro, verbatim

const config: any = {
  writeErr: (str: string) => process.stderr.write(str),
  outputError: (str: string, write: (s: string) => void) => write(str),
};
function fireError(cfg: any, message: string) {
  cfg.outputError(message, cfg.writeErr);
}
fireError(config, "error: something went wrong\n");

tree	result
Node 26.5.1 (node --experimental-strip-types)	error: something went wrong
main @ 4715bc2f (v0.5.1598, train 220)	error: something went wrong
main @ 8df83f8c — the commit the reporter's branch forks from	error: something went wrong
PR #10712 head 463c4fa5 (on #10699) — the reporter's actual tree	error: something went wrong

All three were full cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static
builds with PERRY_RUNTIME_DIR pinned to the matching release/, so no arm could have
linked a stale .a. Byte-identical to Node in every case, and the text really is on
stderr (2>/dev/null swallows it, 2>&1 >/dev/null keeps it).

Real commander, compiled from source

commander 14.0.3 via perry.compilePackages, with the real JS class confirmed in use
(_outputConfiguration is a live object, outputError/writeErr are functions,
constructor.name === "Command"). Diffed against Node across the entire surface the
issue says is lost — --help, --version, missing required argument, unknown option,
unknown command, program.error() — under both the default output configuration and a
configureOutput() override that proves the two-level indirection actually ran:

=== DIFF ===
IDENTICAL

on main and on the reporter's tree alike.

Everything else

32 further spellings of the same indirection all match Node: method shorthand, class
field, Object.assign override, spread-built config, nested receiver a.b.outputError(m, a.b.writeErr),
cross-object writer, computed keys, array-element receiver, getter-provided writer,
Object.create prototype chain, Object.freeze, destructuring, three-level chaining,
two plain-function hops, async caller, nested closure, in a loop, and the shape inside a
CommonJS module reached through createRequire. A missing writer throws TypeError
rather than being dropped (Perry says "value is not a function" where Node names the
callee — a separate, cosmetic message gap).

What does lose commander's output today

Compiled from source, commander is fine. What is not fine on main is the path you get
when commander is not listed in perry.compilePackages:

note: serving `commander` from the bundled native binding `perry-ext-commander`
      (a partial drop-in), ignoring your installed `node_modules/commander`.

Against that shim the same program prints nothing at all — no error text, and no
CommanderError either, so the catch never runs:

=== NODE ===                          === PERRY (native commander shim) ===
error: unknown option '--nope'        (no output)
caught code=commander.unknownOption
error: custom failure
caught code=demo.custom

and --help / --version are empty with program.args === undefined. That is the
pre-existing native-binding gap PR #10712 exists to delete, and it is the only
configuration I found where commander's output disappears.

This matters for triage: "codes right, text missing" is a blend of the two paths, not
something either produces on its own. The compiled-source path gets both right; the
shim gets both wrong. A build where part of the program reaches the shim and part
reaches the real source — which is what a stale libperry_stdlib.a produces on a branch
that removes perry-ext-commander and edits crates/perry-stdlib/Cargo.toml — would
look exactly like the report.

What this PR does

Adds the regression lock, not a fix. The shape earns a gate: #10689 — an inherited
property read folding to the constant undefined on a scalar-replaced object — landed
one commit before this issue was filed, is the same family, and was silent in exactly
the same way. Nothing in test-files/ covered this indirection before.

Two cases exist so the fixture cannot pass 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;
• one 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.

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. The stream is
incidental to the indirection under test.

This fixture passes on unfixed main — it does not discriminate, because there is
nothing to discriminate. That is the finding, and it is why this is filed as a lock
rather than a fix.

Suggested next step for #10711

Ask the reporter for the exact build that produced the empty output. The most likely
explanation is the trap CLAUDE.md documents: perry-runtime/perry-stdlib are
rlib-only, so cargo build -p perry-runtime -p perry-stdlib does not refresh
libperry_{runtime,stdlib}.a and perry compile links a stale archive. PR #10712
removes perry-ext-commander and edits crates/perry-stdlib/Cargo.toml, so a stale
stdlib archive on that branch is exactly the situation where the compiler and the linked
runtime disagree about who owns commander.

I'd hold #10711 open until that is ruled out rather than close it on this PR, so no
close keyword here.

Verification

• Gap suite: PERRY_SKIP_BUILD=1 PERRY_BIN=… ./run_parity_tests.sh --filter test_gap_
• ./scripts/check_file_size.sh — clean
• No compiler code changed, so the suite's before/after differs only by this fixture.

Summary by CodeRabbit

• Tests
  • Added regression coverage for callbacks obtained from object properties and passed into other object-property functions.
  • Verified behavior across object literals, methods, class fields, spread-built configurations, replaced writers, nested receivers, and repeated calls.
  • Confirmed missing writers raise a visible TypeError instead of silently skipping the call.
• Documentation
  • Documented the regression test and its parity results across supported call patterns.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5047d9f9-30b0-4344-aaa7-b3bcce68efc1

📥 Commits

Reviewing files that changed from the base of the PR and between 4715bc2 and 8e80b8c.

📒 Files selected for processing (2)

• changelog.d/10728-property-fn-param-callback-lock.md
• test-files/test_gap_10711_property_fn_param_callback.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

The pull request adds a regression fixture for property-function callback indirection and documents that the reported defect does not reproduce on current sources. The fixture covers multiple object shapes, writer replacement, repeated dispatch, and missing-writer errors.

Changes

Property callback regression lock

Layer / File(s)	Summary
Fixture and regression record test-files/test_gap_10711_property_fn_param_callback.ts, changelog.d/10728-property-fn-param-callback-lock.md	The fixture tests eight property-function callback scenarios, including replacement writers, nested spread configuration, repeated dispatch, and a missing writer that must throw TypeError. The changelog records the parity results and the stdout-based guards.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the regression-lock test, the callback indirection under test, and issue #10711. It is concise and related to the primary change.
Description check	✅ Passed	The description provides a detailed summary, explains the changes and related issue, documents verification commands, and clarifies that the PR adds a regression lock rather than a fix. It does not us…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Landed in merge train 224 (#10742), released as v0.5.1603 — main is now d4ef732ab9.

Closing rather than merging is how trains work here: the PRs were cherry-picked onto one tree, validated together, and landed under the train's own commit, so GitHub cannot mark this one merged even though your change is on main.

One deliberate divergence, for #10719 only: the train carries "total": 580 where the PR carries 581. #10668 landed in train 221 after your measurement and removed one finding, so the baseline conflicted. I resolved it by re-deriving with your own new detector against the assembled tree (--update-baseline, schema 3, total 580, 84 files, --check agreeing at rc=0) rather than hand-merging two numbers taken by different detectors against different trees. The schema 2 → 3 migration is intact.

Validation: all nine cheap gates, cargo check --workspace --all-targets under -D warnings, the release build of all five pinned artifacts, every unit suite, and a 3-area gap sweep with zero unexplained regressions, each area asserted to have run a non-zero number of tests. lint completed its full 6-of-6 compile tier with no failure outside the known-red public-baseline step.