Skip to content

fix(core): drop undefined permission metadata values before publishing requests - #46906

Open
kitlangton wants to merge 1 commit into
v2from
permission-metadata-undefined
Open

fix(core): drop undefined permission metadata values before publishing requests#46906
kitlangton wants to merge 1 commit into
v2from
permission-metadata-undefined

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Why

GET /api/session/:id/permission returns 400 while a glob (or grep) permission is pending whose optional inputs the model omitted. The server logs schema rejection kind=Body. Any client hydrating pending permissions through permission.list (session open, reconnect, a second client) gets an error instead of the prompt; only the live permission.asked event gets through, so the pending request is invisible to anyone who was not already subscribed.

glob.ts and grep.ts copy optional inputs straight into permission metadata ({ root, path: input.path, hidden: input.hidden, limit: input.limit }), so omitted inputs become undefined values inside the Schema.Record(String, Unknown) metadata field. HttpApiEndpoint wraps JSON success schemas in Schema.toCodecJson(...), and the JSON codec for Schema.Unknown rejects undefined (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.Service drops undefined-valued metadata keys once, where it builds the Request from AssertInput, so every tool is covered without touching each call site.

Before, with the model calling glob {"pattern":"**/*.ts"}:

Request Pending request metadata GET /api/session/:id/permission
{pattern} { root, path: undefined, hidden: undefined, limit: undefined } 400 at metadata.path
{pattern, path, limit} { root, path, hidden: undefined, limit } 400 at metadata.hidden
{pattern, path, limit, hidden: false} all defined 200

After:

Request Pending request metadata GET /api/session/:id/permission
{pattern} { root } 200
{pattern, path, limit} { root, path, limit } 200
{pattern, path, limit, hidden: false} all defined 200

Defined values, including false, 0, "", and null, are kept. Only undefined is dropped, which matches what the wire could ever carry anyway.

Scope

One boundary change in packages/core/src/permission.ts plus a regression test. glob.ts and grep.ts are unchanged; the public Permission.Request schema shape is unchanged, so no client regeneration is needed. A schema-level alternative (making the Unknown record tolerate undefined on encode) was not taken because the metadata field is shared with permission.asked events and the session.permission.create payload; normalizing at the service keeps one canonical request value for the bus, the HTTP list, and plugin hooks that read existing.request.

Verification

export PATH=<bun 1.4.0>:$PATH
bun install
cd packages/core && bun test test/permission.test.ts -t "undefined metadata"   # before fix: 1 fail, "Expected JSON value at [\"data\"][0][\"metadata\"][\"path\"]"
cd packages/core && bun test test/permission.test.ts                            # after fix: 104 pass
cd packages/core && bun test test/tool                                          # 331 pass, 19 skip
cd packages/core && bun typecheck
cd packages/server && bun test test/session-instances.test.ts                  # real HttpApi route /api/session/:id/permission, 1 pass

The new test drives the real Permission.Service (assert with metadata: { root, path: undefined, hidden: undefined, limit: undefined }), reads it back through forSession, and encodes the result with Schema.toCodecJson(Schema.Struct({ data: Schema.Array(Permission.Request) })), the same codec HttpApiEndpoint applies to the session.permission.list success body. It failed before the fix with the production error and passes after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant