fix(core): omit undefined optional keys from glob/grep permission metadata#37965
Open
IbrahimKhan12 wants to merge 1 commit into
Open
fix(core): omit undefined optional keys from glob/grep permission metadata#37965IbrahimKhan12 wants to merge 1 commit into
IbrahimKhan12 wants to merge 1 commit into
Conversation
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Potential Related PR Found:
Why they're related: |
…adata
The glob and grep tools built their permission metadata by assigning
input.path/input.limit/input.include directly. When those optional
inputs are omitted, the decoded input has no such key, so reading it
yields undefined and materializes undefined-valued keys in the metadata
bag (e.g. { root: ".", path: undefined, limit: undefined }).
Permission metadata is Schema.Record(String, Unknown) and is serialized
as JSON for session.permission.list. The undefined entries survive
schema encoding as own properties and break strict JSON encoding of the
response, so a single pending glob/grep permission with omitted optional
inputs can make the whole listing fail.
Only include the optional keys when their value is defined, matching the
webfetch/websearch tools which spread the exact-optional decoded input
and never introduce undefined keys.
Refs anomalyco#37650
IbrahimKhan12
force-pushed
the
search-metadata-omit
branch
from
July 20, 2026 19:14
9bb6dae to
7be147b
Compare
IbrahimKhan12
marked this pull request as ready for review
July 20, 2026 19:14
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
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.
Issue for this PR
Closes #37650
Type of change
What does this PR do?
globandgrepbuild their permission metadata by reading optional inputs directly:The decoded tool input is exact-optional, so when
path/limit/includeare omitted the key is absent andinput.pathreads back asundefined. Assigning it still materializes an own property with anundefinedvalue ({ root: ".", path: undefined, limit: undefined }).Permission metadata is
Schema.Record(Schema.String, Schema.Unknown), andUnknownkeeps thatundefinedas a real own property through schema encoding (verified: the encoded object still has thepathkey; onlyJSON.stringifylater hides it). Serializingsession.permission.listas a strict JSON value then chokes on it, so a single pendingglob/greppermission with omitted optional inputs can fail the whole listing.Fix: only add the optional keys when their value is defined. This matches
webfetch/websearch, which spread the exact-optional decoded input and so never introduceundefinedkeys.Related: #37679 fixes the same issue on the
v2branch by strippingundefinedcentrally inpermission.ts. This PR targetsdevand fixes it at the source (the tools produce clean metadata), leaving theRecord<string, unknown>bag untouched.How did you verify your code works?
Added
test/tool-glob.test.tsandtest/tool-grep.test.tsthat execute each tool without the optional inputs and assert the metadata passed topermission.asserthas noundefined-valued keys (checkingObject.keys, sincetoEqualignoresundefinedproperties). Both fail ondev(keyspath/include/limitpresent) and pass with the fix; a second case confirms provided optional fields are still recorded.bun typecheckand thepackages/corepermission/tool suites pass locally.Screenshots / recordings
Not a UI change.
Checklist