fix(core): drop undefined permission metadata values before publishing requests - #46906
Open
kitlangton wants to merge 1 commit into
Open
fix(core): drop undefined permission metadata values before publishing requests#46906kitlangton wants to merge 1 commit into
kitlangton wants to merge 1 commit into
Conversation
6 tasks
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.
Why
GET /api/session/:id/permissionreturns 400 while aglob(orgrep) permission is pending whose optional inputs the model omitted. The server logsschema rejection kind=Body. Any client hydrating pending permissions throughpermission.list(session open, reconnect, a second client) gets an error instead of the prompt; only the livepermission.askedevent gets through, so the pending request is invisible to anyone who was not already subscribed.glob.tsandgrep.tscopy optional inputs straight into permission metadata ({ root, path: input.path, hidden: input.hidden, limit: input.limit }), so omitted inputs becomeundefinedvalues inside theSchema.Record(String, Unknown)metadata field.HttpApiEndpointwraps JSON success schemas inSchema.toCodecJson(...), and the JSON codec forSchema.Unknownrejectsundefined(Expected JSON value at ["data"][0]["metadata"]["path"]). The bus event is not JSON-codec encoded, which is why it still arrives.The probe that exposed this is documented in anomalyco/opencode-drive#78 (multi-tool-interleavings).
What Changes
Permission.Servicedropsundefined-valued metadata keys once, where it builds theRequestfromAssertInput, so every tool is covered without touching each call site.Before, with the model calling
glob {"pattern":"**/*.ts"}:GET /api/session/:id/permission{pattern}{ root, path: undefined, hidden: undefined, limit: undefined }metadata.path{pattern, path, limit}{ root, path, hidden: undefined, limit }metadata.hidden{pattern, path, limit, hidden: false}After:
GET /api/session/:id/permission{pattern}{ root }{pattern, path, limit}{ root, path, limit }{pattern, path, limit, hidden: false}Defined values, including
false,0,"", andnull, are kept. Onlyundefinedis dropped, which matches what the wire could ever carry anyway.Scope
One boundary change in
packages/core/src/permission.tsplus a regression test.glob.tsandgrep.tsare unchanged; the publicPermission.Requestschema shape is unchanged, so no client regeneration is needed. A schema-level alternative (making theUnknownrecord tolerateundefinedon encode) was not taken because the metadata field is shared withpermission.askedevents and thesession.permission.createpayload; normalizing at the service keeps one canonical request value for the bus, the HTTP list, and plugin hooks that readexisting.request.Verification
The new test drives the real
Permission.Service(assertwithmetadata: { root, path: undefined, hidden: undefined, limit: undefined }), reads it back throughforSession, and encodes the result withSchema.toCodecJson(Schema.Struct({ data: Schema.Array(Permission.Request) })), the same codecHttpApiEndpointapplies to thesession.permission.listsuccess body. It failed before the fix with the production error and passes after.