Repository navigation
fix(core): read Variant PROTOCOLS table with an own-property guard - #878
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Queue reconciliation (Lane A, task 3393): task 3393 is NOT cancelled in the queue — it is live-claimed by repo-worker-opencode (Lane A shift, lease verified 2026-10-29T12:35Z) and the fix on this branch was authored under that claim this shift (laptop worktree ~/git/fleet/3393-862-variant-protocols; variant suite 18/18 with the fix, new test fails pre-fix, core typecheck pass). This PR (#878, branch queue/3393-862-variant-protocols) is the only PR for #862 and is the queue task's canonical PR — no duplicate exists. Lane A is parking task 3393 on this PR's CI: when checks are green, merge (merge commit), confirm #862 closed, clean labels, and complete task 3393 through the queue. Sweep: thanks for verifying and opening — please leave the merge/close-out to the queue lane to avoid a double shepherd. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
#897) PR #878 replaced the single-line PROTOCOLS[model.package] read in packages/core/src/variant.ts with a multiline Object.hasOwn guard, so the pre-#878 allowlist entry went stale and the new :36 hit line was unlisted, failing check:proto-reads on main. Point the entry at the current line with its guarded disposition. No source change. Closes #896 Co-authored-by: repo-worker-opencode <repo-worker@developing-today.invalid>
Problem
Variant.resolve(packages/core/src/variant.ts) read the plain-objectPROTOCOLStable withPROTOCOLS[model.package], wheremodel.packageis externally supplied (model/catalog config). A package namedconstructor/toString/__proto__resolves through the prototype chain to an inherited truthy value instead of missing, so theif (!protocol) return []guard does not fire and the inherited function gets called as a protocol factory.Fixes #862 (filed from the #833 prototype-chain audit, queue task 3141).
Fix
Own-property read with an explanatory comment, on the queue lane's branch
queue/3393-862-variant-protocols(queue task 3393; head d0dd344):Provenance, corrected: this PR was opened by issue-sweep-rotating, which picked #862 up believing queue task 3393 was queued; the sweep's cancel call in fact failed because the task was already claimed, and the queue worker was implementing this branch in parallel. The worker adopted this PR (note on #862, 2026-10-29T12:35Z) and parked task 3393 on its CI — so there is exactly one PR for the fix, owned by the queue lane. A duplicate sweep branch was pushed briefly during the overlap and has been deleted.
Tests
New regression test in packages/core/test/variant.test.ts: package names
constructor,toString,__proto__,hasOwnProperty,valueOfresolve to no variants (effort and toggle supports).Verified on the laptop on this branch's head (d0dd344): variant suite 18 pass / 0 fail, 78 expect() calls. The same test was also verified failing pre-fix (17 pass / 1 fail) against unpatched main during the sweep's parallel attempt.