chore: merge train 224 (v0.5.1603) - #10742
Merged
Merged
Conversation
added 10 commits
September 19, 2026 16:44
#10713: `--no-raise-vs <base>` compared the merge base's recorded baseline with the checked-out one and never scanned the tree, so a branch that added findings without touching the baseline compared 561 against 561 and printed "no ceiling raised" while `--check` failed on the same worktree with `REGRESSION: 563 findings exceeds baseline 561`. It now scans the worktree and compares the measured total and per-file counts against the base's recorded ceilings, and prints the resolved base SHA with both totals. #10713, second hole: the dispatch was `if args.no_raise_vs:`, so the empty string an unset $BASE_SHA expands to was falsy, the mode was never entered, and the script fell through to the plain report and exited 0. Now `is not None`, with `resolve_ref` rejecting an empty ref the way raw_handle_debt.py's `git_show` rejects an unfetched one. #10715: `LET_BIND` was matched per line, so a binding rustfmt broke after the `=` -- a function of indentation depth and identifier length, not of anything about the code -- was never tracked. A `let` is now folded back into one statement first. Statements containing a brace stay line-oriented on purpose, so a closure body's own bindings do not go dark. The measured surface moves 558 -> 581 across 85 files (+34 newly visible, -11 false positives in ioredis.rs where a wrapped SHADOWING `let` failed to reset the identity). The baseline is deliberately NOT re-pinned: it still records 561, so both forms are red pending an audited schema migration. Each fix plants the defect it fixes in `--self-test`, verified by reverting each one in isolation. The old self-test passed on the day the live check was fooled, which was the point. Refs #10713, #10715.
The wrapped-`let` fold changes what the detector can count, so the recorded 561 and the measured 581 are two different yardsticks. That is what the script's audited-migration exemption is for, and it is the same situation as the 1 -> 2 migration. The ratchet is unchanged: it still fails on finding 582, verified by planting one. The exemption becomes an explicit AUDITED_MIGRATIONS list naming each migration and its reason, instead of a hard-coded (1, BASELINE_SCHEMA) pair. Every unlisted schema change is still rejected, and --self-test now asserts that 2 -> 4 is refused and that BASELINE_SCHEMA cannot be bumped without naming its own migration -- otherwise a renumber would exempt every PR from the ratchet. 581 = 558 + 34 newly visible - 11 false positives. Two of the 34 were inspected and are genuine unrooted-across-allocation shapes; the other 32 are unaudited exposure surface, not known bugs.
`raw_handle_debt.py --no-raise-vs` compares ceilings strictly per path, so a pure file move — which the 2000-line cap forces regularly — reads as new debt: the bare run demands the emptied source's line be deleted, and the merge-base run then rejects the destination as "was not listed at the merge base", though the total never moved and the bodies are byte-identical. A ledger entry may now carry `# moved-from: <path>`. The destination is credited with what the source actually surrendered between the base and head ledgers, and nothing else: the total check is untouched, the credit is bounded by a real reduction in the same diff, two destinations sharing one source drain one pool, and the annotation goes inert once the move lands. A malformed entry comment is now a parse failure rather than an ignored comment, and `--update` carries surviving annotations through (writer split out as `render_ledger` so the round trip is assertable). `--self-test` +12 cases: undeclared move still rejected, declared move and a 1+1 three-way split pass, laundering / over-draw / double-spend / stale annotation / self-reference each rejected by their own diagnostic. No ceiling changed.
…10581) The fixture raced a `* * * * * *` CronJob against a fixed `Date.now() + 10_000`. The deadline is a timeout, not a barrier: when it expired first the loop exited and the fixture printed `false` on two lines expected to read `true`, dropping `tick 1`/`tick 2` as well — a four-line divergence the harness reports as a `parity_fail`, i.e. as a miscompile. It is inside pr-gate's gap shards and absent from gap_snapshot.json, and it already held #10530 out of a train. The wait now has no deadline, so the printed text is a function of CronJob's behaviour alone: the ticks arrive, or PERRY_RUN_TIMEOUT kills the run and the harness classifies that as a crash/timeout rather than a parity mismatch. No fallback bound — any bound that prints or throws on expiry is the same defect with a longer fuse, and the old 10s was unreachable anyway because PERRY_RUN_TIMEOUT is also 10s. The never-started job now prints `neverTicks === 0` from a real counter instead of a hardcoded `true`, checked after the barrier. Output bytes unchanged. Verified with an identical 11s event-loop stall injected into both the old and new fixtures: under Node 26.5.1 and Perry v0.5.1598 the old one diverges and the new one is byte-identical to the unstalled oracle. Harness run exits 0 with journal status `pass`; 8 Node runs gave one distinct output in 1.86-2.04s.
#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 (8df83f8), and their actual tree (PR #10712 on top of #10699, head 463c4fa) — 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
This was referenced Sep 19, 2026
Closed
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge train 224 — four tooling and test PRs validated together as one tree.
Trains land as their own PR, so the source PRs are closed, not merged, and their close-keywords never fire. Issues resolved are listed at the bottom.
Contents
fix(tooling): close two blind spots inunrooted_local_shape.pytooling: let the raw-handle ledger declare a relocationtest: maketest_gap_cron_cronjobwait on a barrier, not a deadlinetest(gap): lock commander'soutputError/writeErrindirectionWhy #10719 matters more than "a script fix"
unrooted_local_shape.py --no-raise-vsnever scanned a line of source. It read the merge base's recorded baseline and the checked-out one and compared those two numbers — so a branch that adds findings without touching the baseline compared 561 against 561 and exited 0. Every green that arm ever produced was vacuous, including the one it produced on #10668.A second defect in the same dispatch:
--no-raise-vs ""was tested for truthiness, so the empty string an unset$BASE_SHAexpands to skipped the mode entirely and fell through to an ordinary report, exit 0.The detector itself was also line-oriented, so any
letbinding that rustfmt wrapped across lines was invisible. Folding the statement first moves the true count to 580, which is +34 newly visible across 15 files and −11 false positives inioredis.rswhere a wrapped shadowing binding matched nothing and every use of the fresh name was reported against the dead one.The baseline number in this train is 580, not #10719's 581 — deliberately
#10719 re-pinned at 581 measured against
4715bc2fa1. Since then #10668 landed in train 221 (3983b7a5eb, rootingresponse_headers.rsarray pointers), removing one finding, so the baseline conflicted. Resolved by re-deriving with the PR's own new detector against the assembled tree —--update-baseline, giving schema 3, total 580, 84 files, with--checkagreeing at rc=0 — rather than hand-merging two numbers that were measured by different detectors against different trees.An auditor diffing this train against #10719 will see
"total": 581as the one line that differs. That is intended.The schema 2 → 3 bump is an audited migration, the same mechanism the 1 → 2 migration used, because the old 561 was produced by a weaker detector: comparing 580 against it compares two yardsticks. The ratchet is not loosened — it still fails at 581. Worth noting the old baseline also carried 3 findings of slack nobody had noticed (it recorded 561 while
mainmeasured 558), so on that axis the gate got tighter.Validation
Assembled on main and proven: every PR fully represented by patch-id and by subject+author. All nine cheap gates green — including, notably, the two ratchets this train itself modifies:
raw_handle_debt.pyruns clean under #10721's new relocation-aware logic, and both unrooted gates run clean against the re-derived baseline.Issues resolved
Closes #10713
Closes #10715
Closes #10583
Closes #10581