Skip to content

Expose managed approval requirement on permission requests - #2080

Open
joshspicer wants to merge 5 commits into
github:mainfrom
joshspicer:joshspicer/managed-approval-required-sdk
Open

Expose managed approval requirement on permission requests#2080
joshspicer wants to merge 5 commits into
github:mainfrom
joshspicer:joshspicer/managed-approval-required-sdk

Conversation

@joshspicer

@joshspicer joshspicer commented Jul 24, 2026

Copy link
Copy Markdown

Why

Enterprise-managed permission ask rules must be answered by a person. SDK hosts need to distinguish those requests from ordinary permission prompts that client settings may auto-approve.

This is the public SDK contract for:

The initial runtime scope supports managed Shell, Read, Edit, and Domain selectors. Tool and MCP selectors are deferred.

What

Adds optional managedApprovalRequired metadata to the public TypeScript PermissionRequest type.

When true, hosts should bypass automatic approval and present their normal confirmation UI. The runtime remains authoritative for the managed verdict and approve-once-only behavior; the flag is request metadata, not an administrator setting.

The field is overlaid at the hand-authored public type boundary because the runtime schema change has not shipped in a published CLI package yet. This avoids committing unrelated generated-schema drift. A later normal schema regeneration can absorb the field into generated bindings without changing the public TypeScript shape.

Validation

  • npm run build
  • npm run typecheck
  • npm test -- session-event-types.test.ts — 5 passed
  • npm run format:check
  • npm run lint — 0 errors; 3 pre-existing warnings in unrelated tests
  • linked runtime + SDK + VS Code manual validation confirmed managed asks reach client UI and repeated asks do not persist approval

Copilot AI review requested due to automatic review settings July 24, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Exposes managed approval requirements to TypeScript permission handlers.

Changes:

  • Adds optional managedApprovalRequired metadata.
  • Adds package-root type coverage.
Show a summary per file
File Description
nodejs/src/types.ts Overlays managed approval metadata onto permission requests.
nodejs/test/session-event-types.test.ts Verifies the field is publicly importable.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread nodejs/src/types.ts
Comment thread nodejs/src/types.ts Outdated
Comment thread nodejs/src/types.ts Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 18f1bbc1-6001-43e2-b293-724505087f6a
Copilot AI review requested due to automatic review settings July 28, 2026 16:26
@SteveSandersonMS
SteveSandersonMS force-pushed the joshspicer/managed-approval-required-sdk branch from 13c3d37 to 03184d2 Compare July 28, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (1)

nodejs/src/types.ts:1108

  • This overlay only updates the standalone type and PermissionHandler; the public SessionEvent/PermissionRequestedData types still come from the generated schema, where permissionRequest lacks this property. Consequently, an event-only host (the documented path when onPermissionRequest is omitted) cannot compile event.data.permissionRequest.managedApprovalRequired even though the runtime sends it. Please overlay the permission.requested event payload as well so every public permission-request surface exposes the metadata.
export type PermissionRequest = GeneratedPermissionRequest & {
    readonly managedApprovalRequired?: boolean;
};
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c7f00b84-b0a7-4cdf-aca9-ffd49737f26e
Copilot AI review requested due to automatic review settings July 28, 2026 17:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 28, 2026 20:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (1)

nodejs/src/types.ts:1145

  • The behavior change leaves the public README inaccurate: nodejs/README.md:39, :133, and :856-866 still describe approveAll as allowing every request/tool call. With a managed request it now returns no-result, so following those docs can leave execution pending until the consumer explicitly resolves the permission. Please update the permission-handling docs and examples to describe this exception and how hosts should resolve it.
export const approveAll: PermissionHandler = (request) =>
    request.managedApprovalRequired ? { kind: "no-result" } : { kind: "approve-once" };
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@joshspicer
joshspicer marked this pull request as ready for review July 28, 2026 22:24
@joshspicer
joshspicer requested a review from a team as a code owner July 28, 2026 22:24
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 28, 2026 22:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (1)

nodejs/README.md:869

  • The immediately following custom-handler example still returns approve-once for every non-shell request without checking managedApprovalRequired. Copying it therefore auto-approves managed Read, Edit, or Domain asks, contradicting the human-approval requirement documented here. Update that example to route flagged requests through a human confirmation flow (or leave them unanswered).
For requests with `managedApprovalRequired: true`, `approveAll` returns `{ kind: "no-result" }`. The request remains pending and the host must present a human-facing confirmation flow to resolve it explicitly.
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@joshspicer

Copy link
Copy Markdown
Author

Linked end-to-end validation completed against SDK head 0da1c5995fa4, runtime head 18b743d00dcd, and the merged VS Code host integration.

Cross-layer audit of the latest runtime session and AHP transcript confirmed:

  • 12 public SDK permission.requested events exposed managedApprovalRequired: true on the permission request and prompt request
  • all 12 requests completed through the SDK permission callback path
  • AHP rendered 12 human confirmation cards with exactly allow-once and skip; no allow-session option or selection appeared
  • managed denies produced no permission callback/confirmation and failed closed
  • direct allows completed without confirmation
  • the device policy resolved from the supported file source as source: device, deviceManaged: true

The assistant's final table overstated downstream tool success for two approved Domain asks (HTTP redirect/404) and mislabeled the api.github.com prompt; those are reporting/tool-result issues, not SDK permission-routing failures. The SDK contract and host behavior validated successfully.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 29, 2026 00:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants