Revert #9326 — it breaks cc --help on main (defineProperty TypeError); fixes #9341 - #9345
Conversation
…(from PerryTS#9297) (PerryTS#9326)" This reverts commit 1f8ad06.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughChangesThe PR consolidates array prototype-chain indexing into Array indexing behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to This revert restores the command-line workload but also reintroduces indexed-array writes that can bypass inherited setters or non-writable properties on custom prototypes, causing incorrect runtime behavior for affected programs. Merge should wait for explicit owner acceptance of this bounded compatibility risk or a follow-up fix. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description provides substantial context about the revert, failure, affected issue, rationale, validation, and re-landing conditions. It does not use the repository template headings or checklist, but the required information is mostly present. Full details: Linked Issues checkExplanation The changes directly revert the runtime behavior introduced by Full details: Docstring CoverageExplanation Docstring coverage is 88.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 5 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 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 |
|
The revert did not fix the break — #9326 was not the culprit. Measured on |
Reverts #9326 ("indexed writes honour a custom array prototype", the merge of #9297). Fixes #9341:
claude --helpon current main dies deterministically withrc=1, 5/5 runs — the campaign's primary parity workload is broken and every cc measurement against main is void (including the r6 numbers in
/root/claude-arch/measure_r6.log, which recorded a binary that exits early).Attribution — two independent chains, agreeing
83754818eis verified green; the fix(runtime): no Rust frame is ever a longjmp target — cc --help segfault fixed, parity gate back online (#9305) #9323 merge point re-verified the parity gate; fix(release): prime the macOS x86_64 dependency cache (from #9327) #9328/fix: keep main green after #9329 #9336 are infra/lint-only. That leaves fix(runtime): indexed writes honour a custom array prototype (from #9297) #9326 as the only runtime change in the broken window. Known-good/known-bad binaries preserved at/root/claude-shapedesc/on perrymaster.d1bef4c965, the parent side of the window) is running as confirmation and will be posted to cc --help parity gate down on main: defineProperty-on-non-object at startup — window pins #9326 (indexed writes with a recorded custom array prototype) #9341 when it completes.And this PR is its own third proof: it is a clean
git revert -m 1of exactly the #9326 merge, nothing else — ifcc --helppasses on it, the attribution is settled by construction.Why revert rather than fix-forward
I authored the reverted work, so this is not deflection — the calculus is that a broken primary workload on
maincosts every session more per hour than the fix is worth: it voids all cc measurements, manufactures false blame against any branch measured on top of it, and this is the third cc-breaking regression to reach main this week (the #9169 window, the #9305 setjmp crash, now this).The change also failed in exactly the way its own verification warned about: its 88-fixture corpus and 64-check differential were all green — nothing in its verification ran the cc bundle, and the failing shape (esbuild
__export/namespace machinery at module-graph scale, on the--helppath specifically) is not reducible to the obvious mini-fixture (a direct__defProp/__export/__commonJSrepro passes; I tried).Re-landing conditions
#9220/#9221 remain real bugs and the reverted fix remains substantially correct. It should return once:
cc --helpbyte-parity is run pre-merge on the re-land branch — which this week has earned a place as a required gate for runtime PRs, not a post-hoc measurement.The revert is conflict-free against current main (
6bc775d9b9),cargo fmtclean, runtime typechecks clean.Summary by CodeRabbit
map,forEach,join, andindexOf, when operating on arrays with prototype-provided values.