Conversation
|
The following comment was made by an LLM, it may be inaccurate: I found two related PRs that appear to address similar issues with undefined values in permission metadata:
These PRs likely addressed similar problems in the core layer, while the current PR (38918) is fixing the same issue in the opencode (v1) layer. You may want to verify whether these earlier PRs should have also covered the v1 tools, or if there's overlap in the fixes. |
|
Related, no overlap. Both of those change v2 code in Scope differs too. #37650 was reported against v2 glob and grep. On v1 the same defect additionally affects webfetch, websearch and apply_patch, the last nesting |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #38912
Type of change
What does this PR do?
v1 tools write absent optional inputs into permission metadata as
undefined, becausepath: params.pathcreates the key instead of omitting it. Metadata isSchema.Record(Schema.String, Schema.Unknown), andUnknownencodes throughSchema.Json, whose guard rejectsundefinedand recurses. One pending request therefore breaks the wholeGET /permissionresponse, not just its own entry.Five tools hit it: glob, grep, webfetch and websearch flat, plus apply_patch, which nests
movePathinsidemetadata.files[].Fixed where the pending request is built, the only place one is constructed, so it covers both the listing and the
Event.Askedpublish. Recursive because apply_patch nests, and because plugin tools feed the same bag (AskInput.metadatais{ [key: string]: any }), so the producer set is open. Plain objects and arrays only, so aDateis not flattened to{}byObject.entries, and ancestors are tracked the waySchemaAST.isJsondoes, so genuine cycles still fail instead of looping forever.How did you verify your code works?
Three tests boot the real Permission service and encode
list()through the schemaGET /permissiondeclares, so they assert the operation that actually fails rather than a proxy. Reverting only the source change makes the flat and nested tests fail while the negative one still passes. 145 tests pass across the permission, acp and apply_patch suites,tsgo --noEmitis clean, and oxlint reports the same 3 warnings as before.Screenshots / recordings
Not a UI change.
Checklist