Skip to content

fix: add onPermissionRequest handler to sessions across various docum…#892

Open
horihiro wants to merge 3 commits intogithub:mainfrom
horihiro:fix/typescript-sample
Open

fix: add onPermissionRequest handler to sessions across various docum…#892
horihiro wants to merge 3 commits intogithub:mainfrom
horihiro:fix/typescript-sample

Conversation

@horihiro
Copy link

This pull request updates documentation examples across multiple files to consistently demonstrate how to handle permission requests when creating Copilot sessions. The primary change is the addition of the onPermissionRequest parameter—often using the new approveAll helper—to all relevant TypeScript/Node.js sample code. This ensures users see the recommended way to handle permission prompts in their own implementations.

The most important changes include:

Documentation consistency and clarity:

Import updates for new helper:

  • Updated all relevant TypeScript examples to import approveAll from @github/copilot-sdk alongside CopilotClient (and defineTool where used), ensuring the helper is available in code samples. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Sample code modernization:

  • Refactored session creation in documentation to use the new permission handling pattern, replacing or supplementing older examples that lacked permission request handling. [1] [2]

These changes improve the clarity and correctness of the documentation, ensuring users follow best practices for permission handling in Copilot SDK integrations.…entation and examples

@horihiro horihiro requested a review from a team as a code owner March 19, 2026 21:00
Copilot AI review requested due to automatic review settings March 19, 2026 21:00
Copy link
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 updates the Node.js/TypeScript documentation and examples to consistently show handling of session permission prompts by adding onPermissionRequest (often via the approveAll helper) when creating Copilot sessions.

Changes:

  • Add onPermissionRequest to session creation examples across docs and the Node example.
  • Update imports in TypeScript snippets to include approveAll where referenced.
  • Modernize/standardize session config snippets to reflect the required permission-handling pattern.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
nodejs/examples/basic-example.ts Adds approveAll and passes it via onPermissionRequest when creating a session.
nodejs/README.md Updates multiple README snippets to include onPermissionRequest and introduces approveAll usage.
docs/troubleshooting/debugging.md Adds onPermissionRequest to a debugging snippet for session creation.
docs/troubleshooting/compatibility.md Adds onPermissionRequest to an infinite-sessions configuration example.
docs/setup/scaling.md Adds onPermissionRequest to scaling-oriented session creation examples.
docs/setup/local-cli.md Updates local CLI setup snippet to import/use approveAll and adds onPermissionRequest to another example.
docs/setup/github-oauth.md Updates OAuth setup snippet to import/use approveAll and pass onPermissionRequest.
docs/setup/bundled-cli.md Updates bundled CLI setup snippet to import/use approveAll and adds onPermissionRequest to more configs.
docs/setup/backend-services.md Updates backend-service examples to import/use approveAll and pass onPermissionRequest.
docs/setup/azure-managed-identity.md Updates managed identity example to import/use approveAll and pass onPermissionRequest.
docs/hooks/user-prompt-submitted.md Adds onPermissionRequest to multiple hook examples.
docs/hooks/session-lifecycle.md Adds onPermissionRequest to session lifecycle hook examples (incl. one using approveAll).
docs/hooks/pre-tool-use.md Adds onPermissionRequest to pre-tool-use hook examples.
docs/hooks/post-tool-use.md Adds onPermissionRequest to post-tool-use hook examples.
docs/hooks/index.md Updates hooks overview snippet to import/use approveAll and adds onPermissionRequest elsewhere.
docs/hooks/error-handling.md Adds onPermissionRequest to error-handling hook examples.
docs/getting-started.md Updates getting-started snippets to import/use approveAll and add onPermissionRequest.
docs/features/steering-and-queueing.md Updates import to include approveAll for permission-handling consistency.
docs/features/skills.md Adds onPermissionRequest to a skills configuration example.
docs/features/session-persistence.md Updates import to include approveAll and adds onPermissionRequest to a persistence example.
docs/features/mcp.md Updates MCP examples to import/use approveAll and pass onPermissionRequest.
docs/features/custom-agents.md Adds onPermissionRequest to custom agent session creation examples.
docs/auth/byok.md Updates BYOK examples to import/use approveAll and pass onPermissionRequest in multiple configs.

return null;
},
},
onPermissionRequest: approveAll
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This snippet uses onPermissionRequest: approveAll, but approveAll isn’t defined or imported within the code block. Since docs code blocks are validated as standalone TypeScript files, this will fail compilation. Either add the appropriate import (approveAll from @github/copilot-sdk) to the block, or switch this line to an inline handler like async () => ({ kind: "approved" }) to keep the snippet self-contained.

Suggested change
onPermissionRequest: approveAll
onPermissionRequest: async () => ({ kind: "approved" })

Copilot uses AI. Check for mistakes.
nodejs/README.md Outdated

```typescript
import type { PermissionRequest, PermissionRequestResult } from "@github/copilot-sdk";
import type { PermissionRequest, PermissionRequestResult, approveAll } from "@github/copilot-sdk";
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

In this snippet, approveAll is a runtime value export, but it’s being imported via import type { ... }, which will fail TypeScript compilation. Import approveAll with a normal import { approveAll } ... (and keep PermissionRequest/PermissionRequestResult as type-only if desired).

Suggested change
import type { PermissionRequest, PermissionRequestResult, approveAll } from "@github/copilot-sdk";
import { approveAll } from "@github/copilot-sdk";
import type { PermissionRequest, PermissionRequestResult } from "@github/copilot-sdk";

Copilot uses AI. Check for mistakes.
Comment on lines 700 to 726
@@ -712,6 +722,7 @@ const session = await client.createSession({

return { kind: "approved" };
},
onPermissionRequest: approveAll
});
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This createSession object literal defines onPermissionRequest twice (custom handler at the top, then onPermissionRequest: approveAll later). Duplicate keys are a TypeScript error and it’s unclear which behavior the example intends. Remove one of the two, or split into two separate examples (custom handler vs approveAll).

Copilot uses AI. Check for mistakes.
Comment on lines +246 to +252
model: "gpt-4.1",
streaming: true,
});
}, onPermissionRequest: approveAll);
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This createSession call is syntactically invalid: onPermissionRequest is being passed outside the config object (}, onPermissionRequest: approveAll);). Move onPermissionRequest: approveAll inside the object literal, and ensure approveAll is imported in this snippet (the import currently only brings in CopilotClient).

Copilot uses AI. Check for mistakes.

```typescript
import { CopilotClient } from "@github/copilot-sdk";
import { CopilotClient, approveAll } from "@github/copilot-sdk";
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice there's a mix of approveAll and inline handlers (async () => ({ kind: "approved" })) across many of these docs. For this one in particular it seems strange to import approveAll and then not use it.

Would you be willing to check across all these docs and standardize on one approach? Ideally let's use the approveAll import for all of them.

@SteveSandersonMS
Copy link
Contributor

SteveSandersonMS commented Mar 20, 2026

@horihiro Looks like there are a few syntactical errors across these changes (see the comments). Would you be open to updating this to address them? Thanks!

(Note: I clicked the close button by mistake, so have also reopened this PR!)

@horihiro
Copy link
Author

@SteveSandersonMS Unified all onPermissionRequest configuration values to the imported approveAll.

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