Skip to content

fix: update permission-broker-response.v1.json#31328

Merged
bartlomieju merged 1 commit intodenoland:mainfrom
bartlomieju:broker_schema
Nov 18, 2025
Merged

fix: update permission-broker-response.v1.json#31328
bartlomieju merged 1 commit intodenoland:mainfrom
bartlomieju:broker_schema

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju commented Nov 18, 2025

It specified "grant" instead of "allow" by mistake.

Copilot AI review requested due to automatic review settings November 18, 2025 09:56
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 18, 2025 •
edited
Loading

Walkthrough

The permission-broker-response schema's result enum has been updated to replace the literal value "grant" with "allow", while keeping "deny" unchanged. This modifies how permission outcomes are represented in the schema definition.

Changes

Cohort / File(s) Summary
Schema definition update
cli/schemas/permission-broker-response.v1.json		 Updated result enum from ["grant", "deny"] to ["allow", "deny"]

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single file change with straightforward value replacement
  • No structural, behavioral, or logic modifications
  • Schema-only update with minimal blast radius

Poem

A rabbit hops through schemas bright, ✨
Where "grant" becomes "allow" tonight,
One tiny change, so clean and neat,
Makes permission wording more sweet! 🐰

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates the schema file being updated and uses a 'fix:' prefix, appropriately describing the primary change of correcting an enum value in the JSON schema.
Description check ✅ Passed The description directly explains the changeset: correcting an enum value from 'grant' to 'allow' in the permission-broker-response schema.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b53514 and 3635e8a.

📒 Files selected for processing (1)
  • cli/schemas/permission-broker-response.v1.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test debug linux-x86_64
  • GitHub Check: test debug linux-aarch64
  • GitHub Check: test debug macos-aarch64
  • GitHub Check: test debug macos-x86_64
  • GitHub Check: test debug windows-x86_64
  • GitHub Check: lint debug windows-x86_64
  • GitHub Check: build libs
  • GitHub Check: lint debug linux-x86_64
  • GitHub Check: lint debug macos-x86_64
🔇 Additional comments (1)
cli/schemas/permission-broker-response.v1.json (1)

14-17: The schema change contradicts the official Deno documentation and introduces a breaking change without corresponding runtime updates.

Official Deno documentation states that permission broker responses must include a result of either "grant" or "deny", not "allow" or "deny" as this PR's schema specifies. This is a critical issue because:

  1. Documentation mismatch: The official specification still references "grant" as the valid response value, not "allow"
  2. Breaking change: Any existing permission broker implementations that follow the official documentation will now be rejected as invalid by the schema
  3. Incomplete change: This PR appears to change only the schema file without evidence that:
    • The Deno runtime has been updated to expect "allow" instead of "grant"
    • The official documentation has been updated accordingly
    • All existing broker implementations have been migrated

This is not a simple schema correction—it requires coordinated changes across the runtime implementation and documentation. The PR needs to be verified to ensure it includes corresponding runtime changes and that all components have been updated consistently.

Comment @coderabbitai help to get the list of available commands and usage tips.

@bartlomieju bartlomieju enabled auto-merge (squash) November 18, 2025 09:57
Copilot AI reviewed Nov 18, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a typo in the permission broker response schema where "grant" was incorrectly specified instead of "allow" as the enum value. This aligns the schema with the actual implementation in the runtime code and test fixtures.

  • Corrected the enum value from "grant" to "allow" in the permission broker response schema

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bartlomieju bartlomieju merged commit 8c61416 into denoland:main Nov 18, 2025
38 of 43 checks passed
bartlomieju added a commit that referenced this pull request Jan 22, 2026
@bartlomieju
fix: update permission-broker-response.v1.json (#31328)
50aba65 
It specified "grant" instead of "allow" by mistake.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@lucacasonato lucacasonato lucacasonato approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bartlomieju @lucacasonato