Make safe-outputs.add-comment discussions permission opt-in by default#39051
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot the add-comment description should be dynamically updated to reflect the available permissions in JavaScript when starting the safe output MCP |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…scription Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #39051 does not have the 'implementation' label and has only 31 new lines in business logic directories (≤100 threshold), with no custom .design-gate.yml config. |
There was a problem hiding this comment.
Pull request overview
This pull request flips safe-outputs.add-comment so discussions: write is no longer requested by default, making Discussions commenting explicitly opt-in (safe-outputs.add-comment.discussions: true). It updates the permission computation, test expectations, runtime tool description generation, and regenerated lock workflows to match the new default.
Changes:
- Changed
add-commentpermission derivation sodiscussions: writeis granted only whendiscussions: true. - Updated/added tests and tool-description logic to reflect the new default-disabled discussions behavior (including dynamic description rewriting).
- Regenerated many
.lock.ymlworkflows to dropdiscussions: writewhere it was only present due to the old default.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/add_comment.go | Makes discussions permission opt-in for add-comment and updates config semantics/docs. |
| pkg/workflow/safe_outputs_handler_registry.go | Emits discussions into handler config for add_comment. |
| pkg/workflow/safe_outputs_permissions_test.go | Updates permission expectation tests for new add-comment default (no discussions). |
| pkg/workflow/add_comment_discussions_integration_test.go | Updates integration expectations for discussions default/behavior. |
| pkg/workflow/notify_comment_test.go | Updates conclusion-job permission expectations around add-comment defaults. |
| pkg/workflow/compiler_safe_outputs_job_test.go | Adjusts consolidated safe_outputs job permission assertions impacted by add-comment defaults. |
| pkg/workflow/compiler_safe_outputs_config_test.go | Adds a boolean-field handler-config test for add_comment discussions. |
| pkg/workflow/compile_outputs_comment_test.go | Updates generated lock permissions substring assertion for new default. |
| pkg/workflow/js/safe_outputs_tools.json | Updates embedded add_comment tool description to reflect opt-in discussions. |
| actions/setup/js/safe_outputs_tools.json | Updates runtime tools source description for add_comment to reflect opt-in discussions. |
| actions/setup/js/generate_safe_outputs_tools.cjs | Dynamically rewrites add_comment description to reflect discussion capability. |
| actions/setup/js/generate_safe_outputs_tools.test.cjs | Adds tests for dynamic add_comment description behavior (enabled vs default-disabled). |
| docs/src/content/docs/specs/safe-outputs-specification.md | Updates add_comment tool schema description note for new default. |
| .github/workflows/workflow-health-manager.lock.yml | Removes discussions: write from job permissions where it was only due to old add-comment default. |
| .github/workflows/visual-regression-checker.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/unbloat-docs.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/test-quality-sentinel.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/technical-doc-writer.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/sub-issue-closer.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/static-analysis-report.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/stale-pr-cleanup.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-workflow-call.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-test-tools.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-temporary-id.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-service-ports.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-project.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-pi.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-opencode.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-multi-pr.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-gemini.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-crush.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-claude.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-ci.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-antigravity.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-agent-scoped-approved.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-agent-public-none.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-agent-public-approved.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-agent-all-none.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/smoke-agent-all-merged.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/scout.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/ruflo-backed-task.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/refiner.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/q.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/pr-triage-agent.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/pr-sous-chef.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/necromancer.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/mattpocock-skills-reviewer.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/issue-triage-agent.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/issue-monster.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/draft-pr-cleanup.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/discussion-task-miner.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/dev-hawk.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/design-decision-gate.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/dependabot-repair.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/daily-fact.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/craft.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/contribution-check.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/cloclo.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/ci-doctor.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/brave.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/archie.lock.yml | Same: regenerated permissions without discussions write. |
| .github/workflows/approach-validator.lock.yml | Same: regenerated permissions without discussions write. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 65/65 changed files
- Comments generated: 2
| ```json | ||
| { | ||
| "name": "add_comment", | ||
| "description": "Add a comment to an existing issue, pull request, or discussion. IMPORTANT: Comments are subject to validation constraints enforced by the MCP server - maximum 65536 characters for the complete comment (including footer which is added automatically), 10 mentions (@username), and 50 links. Exceeding these limits will result in an immediate error with specific guidance. NOTE: By default, this tool requires discussions:write permission. If your GitHub App lacks Discussions permission, set 'discussions: false' in the workflow's safe-outputs.add-comment configuration to exclude this permission.", | ||
| "description": "Add a comment to an existing issue, pull request, or discussion. IMPORTANT: Comments are subject to validation constraints enforced by the MCP server - maximum 65536 characters for the complete comment (including footer which is added automatically), 10 mentions (@username), and 50 links. Exceeding these limits will result in an immediate error with specific guidance. NOTE: By default, this tool does not require discussions:write permission. Set 'discussions: true' in the workflow's safe-outputs.add-comment configuration to enable discussion comments and request this permission.", | ||
| "inputSchema": { |
| // Check permissions based on what safe-outputs are configured | ||
| // When add-comment is configured, it requires issues and discussions permissions | ||
| // When add-comment is configured by default, it requires issues permission | ||
| // (PR comments are issue comments, so only issues: write is needed, not pull-requests: write) | ||
| // When only missing_tool/noop is configured, minimal permissions are needed | ||
| if tt.addCommentConfig { |
|
@copilot review all comments and address unresolved review feedback.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in the latest commit. The TypeScript error was a missing parentheses in the JSDoc type cast on line 53 of |
…ier strips Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in the latest commit. The JSDoc inline cast ( |
add-commentpreviously requesteddiscussions: writeunless explicitly disabled, which breaks in environments where Discussions permission is unavailable. This change flips the default so discussion-comment capability is only enabled when explicitly requested.Permission default change
AddCommentsConfigsemantics sodiscussionsis opt-in.buildAddCommentPermissionsnow grantsdiscussions: writeonly whendiscussions: true.Behavior and expectation updates in tests
discussionsabsent ⇒ no discussions permission).add_commenttool-description behavior for bothdiscussions: trueand default-disabled cases.Tooling/docs parity
safe-outputs.add-comment.discussions: trueadd_commentdescription to reflect available discussion permissions (including whenreply_to_iddiscussion-threading support is advertised).