fix(claude-ops): leave a built-in name unresolved when it is bound to a non-constant expression - #5704
Merged
Conversation
… a non-constant expression The name resolvers took the nearest preceding string constant from an index that holds only string bindings, so a nearer binding to a conditional or call (the 2.1.286 skill loader's `Vt=$t?smt(e):e`) was invisible and an unrelated `Vt="string"` far ahead became a phantom built-in command. Every candidate now must be the string constant the module and scope rule says the read sees; otherwise the name stays unresolved. Revalidated against 2.1.286. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton
marked this pull request as ready for review
October 1, 2026 14:17
Contributor
Author
|
@codex review |
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. |
Contributor
|
Claude finished @kyle-sexton's task in 3m 59s —— View job Code review complete
No blocking findings. SummaryTraced the new
One pre-existing, untouched code path worth noting for awareness only (not a finding against this PR): in Verification note: I was unable to execute |
Contributor
|
Claude finished @kyle-sexton's task in 1m 0s —— View job Security review —
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e41ecf9926
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kyle-sexton
enabled auto-merge (squash)
October 1, 2026 14:48
kyle-sexton
added a commit
that referenced
this pull request
Oct 1, 2026
… in its own scope (#5717) Closes #5711 ## Summary On Claude Code 2.1.286 the inventory read the built-in Explore and Plan agents' `disallowedTools` as `partial`. Both definitions spread a shared list (`...pY`). The spread resolver took the nearest `pY=` binding anywhere in the bundle, which on 2.1.286 is an unrelated `pY=p(...)` call, so the shared entries, the Artifact tools among them, were dropped. ## Fix - `_array_names` resolves a `...spread` element through a new `_spread_array` helper. It reads the binding with `_binding_value` at the spread's own offset, the same module and scope rule `_scoped_constant` applies to names and fields (#5619, #5704). Before, it used `_nearest_binding`. - Fail-closed: - When the scoped binding is not an array literal (a call, a conditional, or undeterminable), the list stays `partial`. - When the selected binding is a bare or conditional assignment rather than a declaration (`if(c)pY=["B"]`, `pY=c?["B"]:pY`, `c&&(pY=["B"])`, an unbraced `for(...)pY=["B"]`), or any other write in the same block reaches it, the list stays `partial`. A `var` reached back through a chain of simple declarators counts as a declaration, which keeps Explore and Plan resolving: their `pY` follows a function declaration that has no `;`, and `_declares` misses that statement. - When code off the declaring block's straight line assigns the binding without declaring its own, the list also stays `partial`. That code is a nested block (`if(c){pY=["B"]}`, a loop body), another function (`function init(){pY=["B"]}`), or an expression-bodied arrow (`var f=()=>pY=["B"]`, in either order relative to the declaration). A read can then see B, so the list is not static. This check (`_written_elsewhere`) applies only to the spread path. Applied inside `_binding_value`, it changed many unrelated description fields on both 2.1.285 and 2.1.286 and made the run about 5x slower, so it is not shared with name and field resolution. - Tests in `test_inventory.py`: - A spread whose nearest same-name binding is local to another function resolves to the in-scope array. This test fails on the unfixed resolver. - A spread whose in-scope binding is a conditional stays `partial`. - A spread whose binding another function, a nested `if` or loop block, or an expression-bodied arrow reassigns stays `partial`. - A writer that declares its own local of the same name does not block resolution. - `reference/extraction.md` states the spread rule. claude-ops 0.79.2 -> 0.79.3, with a CHANGELOG entry. ## Verification - `inventory.py --binary-only` on 2.1.286, diffed against the prior run (`.work/cc-2.1.286/inv-final.json`). Only these fields changed: - `builtin_agents/Explore/disallowed_tools`: `[Agent, ExitPlanMode, Edit, Write, NotebookEdit]` -> `[Agent, Artifact, ArtifactComments, ArtifactData, ArtifactCheck, ExitPlanMode, Edit, Write, NotebookEdit]` - `builtin_agents/Explore/disallowed_tools_source`: `partial` -> `literal` - `builtin_agents/Plan/disallowed_tools` and `disallowed_tools_source`: the same change - On 2.1.285, the inventory output is identical to origin/main's. - The reassignment checks (commits f88d9bf and 117a3fb) left the 2.1.285 and 2.1.286 outputs identical to the first fix commit (3075883), and runtime is unchanged (about 22s). - `inventory.py --self-check`: `OK: cli 2.1.286, validated against 2.1.286`, with all six lanes ok. - `python3 -m unittest test_inventory`: `Ran 215 tests ... OK`. - `overlap.py detect`: exit 0, with `discovered: 0` and `resurfaced: 0`. `overlap.py generate --check` reports the docs are in sync. `node scripts/generate-catalog.mjs` produced no diff. - `scripts/run-ruff.sh check` and `format --check` pass on both files. ## Related - #5640: move to a real JavaScript parser, which would replace these scope heuristics. - #5704 / #5700: the same scoped-binding rule, applied to names. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 1, 2026
kyle-sexton
added a commit
that referenced
this pull request
Oct 1, 2026
…e Code 2.1.287 (#5731) Closes #5730 ## Summary Per-release native-surface pass for Claude Code 2.1.287. Every inventory lane extracts ok on 2.1.287, so the inventory is revalidated against it. The one native surface that moved is the hidden built-in command `/plugin-types`, which the 2.1.287 build no longer ships. Its dismissal against `code-metrics:audit-type-debt` was orphaned and is removed. ## Fix - `VALIDATED_AGAINST` in `plugins/claude-ops/skills/inventory/scripts/inventory.py` is now `2.1.287`. - The `plugin-types` / `code-metrics:audit-type-debt` dismissal is removed from `docs/native-surfaces/records.json`. `overlap.py` has no undismiss command, so the record was deleted from the store and `docs/native-surfaces.md` was regenerated with `overlap.py generate`. `node scripts/generate-catalog.mjs` reported the catalog already in sync. - claude-ops is bumped from 0.80.0 to 0.80.1, with a CHANGELOG entry. ## Verification - Binary string search, not variable names: in 2.1.286, `/plugin-types` occurs 10 times and `Write claude-code.d.ts` 2 times. In 2.1.287 both occur 0 times. The only `plugin-types` left in 2.1.287 is the CSP directive name inside a list of `*-src` directives. - Surface diff, 2.1.286 final extraction against 2.1.287: builtin_commands went from 111 to 110 (`plugin-types` removed). Nothing was added or renamed in any lane. The Explore and Plan `disallowed_tools` now read `literal` with the Artifact tools included. That comes from the extractor fix in #5717, not from a change in the binary. - `inventory.py --self-check`: `OK: cli 2.1.287, validated against 2.1.287`, all six lanes ok, exit 0. It was `DEGRADED` (exit 3) before the bump. - `overlap.py detect` on the final extraction: exit 0, integrity ok, discovered 0, resurfaced 0, orphaned dismissals 0, 95 suppressed. - `overlap.py self-check`: exit 3, degraded only by the 2 standing advisories it also reports on main (older recorded extraction versions on rows, upstream SHA not locally decidable). 69 rows checked, 0 problems. - `test_inventory.py`: 215 tests OK. `test_overlap.py`: 184 tests OK. Pinned ruff passes on `inventory.py`. ## Related - #5704: the same pass for 2.1.286. - #5717: the Explore and Plan disallowed-tools extractor fix. - #5730: the native-drift item `native-drift:inventory-degraded:2.1.287:inventory`, filed by this pass and closed by this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Closes #5700
Closes #5701
Summary
On Claude Code 2.1.286 the inventory reported a built-in command
stringthat does not exist. The generic skill loader builds{type:"prompt",name:Vt,...}withVt=$t?smt(e):ein its own scope. The name resolver searched an index that holds only string bindings, so it could not see that nearer conditional binding and picked an unrelatedVt="string"megabytes earlier. That is a wrong value. The reader is supposed to fail closed: a name it cannot resolve stays unresolved.Fix
_scoped_constantchecks it with the module and scope rule that field resolution uses (_binding_value). The candidate must be the string constant that this read sees. A name bound to a conditional, a call or any other non-constant expression stays unresolved, and so does a name whose constant lives in another module. The check applies to commands, bundled skills, subagents and tools:resolve_name_identandresolve_tool_identnow takesrcandbraces._binding_valuegains awindowargument, so a single-character name keeps the existingSHORT_IDENT_LOCALITY_BYTESreach.VALIDATED_AGAINSTis now2.1.286. claude-ops is bumped to 0.79.2 with a CHANGELOG entry, andreference/extraction.mddescribes the check.Verification
--binary-onlyinventories were diffed before (origin/main) and after the fix:validated_againstadvisory text differs.builtin_commands.stringis removed andstringdrops out ofintegrity.undetermined. Nothing else changed in any lane. No other phantom of this bug class turned up.pause-memory(its description is now a getter),plugin-types(now hidden) and Explore/Plandisallowed_tools. The Explore/Plan lists arepartialbecause the...pYspread hits a nearerpY=p(...)call. That is fail-closed (the list is a floor), and this PR leaves it alone.inventory.py --self-checkon the installed CLI:OK: cli 2.1.286, validated against 2.1.286, all six lanesok. Eval-backed facts hold on 2.1.286:usagealiasescost/stats,heapdump/sandboxhidden,teleporthidden and gated,vimremoved_in_docs.test_inventory.py: 209 OK. New fixtures cover a name shadowed by a conditional (the 2.1.286 shape), by a call, by a non-constant in a bundled skill and in a tool, a constant in another module, and a same-scope snake_case rebinding of a tool. Positive controls cover an in-scope constant and an imported one. With the guard stubbed out, 6 of the new tests fail.test_overlap.py184 OK,overlap.test.shpasses,test_native_drift.py40 OK,native_drift.test.shpasses, andscripts/run-ruff.sh checkandformat --checkare clean.Related
🤖 Generated with Claude Code