Conversation
This comment has been minimized.
This comment has been minimized.
|
Hey A few things to wrap up before this is ready for review:
If you'd like a prompt to continue, here's one you can use:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/edcc6cac-38c8-4ee5-ae47-94cc8b1fbcd0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/edcc6cac-38c8-4ee5-ae47-94cc8b1fbcd0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/edcc6cac-38c8-4ee5-ae47-94cc8b1fbcd0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates activation-job permission derivation to be trigger-event-aware so issue-only workflows don’t request unnecessary scopes (least privilege), and adds regression coverage to validate the new behavior.
Changes:
- Scoped activation interaction permissions (reaction/status-comment) based on parsed
onevent keys rather than broad grants. - Added structured extraction of trigger event names from
onYAML (with filtering of activation metadata keys). - Updated/added tests to validate least-privilege permissions for issue-only and PR-review-comment workflows.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_activation_job.go | Adds event-aware permission derivation for activation interactions, and introduces on YAML parsing helpers to scope permissions. |
| pkg/workflow/task_and_reaction_permissions_test.go | Updates assertions to ensure issue-only triggers do not grant pull-requests: write / discussions: write. |
| pkg/workflow/activation_permissions_scope_test.go | Adds focused regression tests for issue-only and PR review comment activation permission scoping. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
| func activationEventSet(onSection string) map[string]bool { | ||
| events := make(map[string]bool) | ||
| var onData map[string]any | ||
| if err := yaml.Unmarshal([]byte(onSection), &onData); err != nil { | ||
| compilerActivationJobLog.Printf("Failed to parse on section for activation permission scoping: %v", err) | ||
| return events | ||
| } | ||
|
|
||
| onValue, hasOn := onData["on"] | ||
| if !hasOn { | ||
| return events | ||
| } | ||
|
|
||
| switch v := onValue.(type) { | ||
| case string: | ||
| events[v] = true | ||
| case []any: | ||
| for _, item := range v { | ||
| if eventName, ok := item.(string); ok { | ||
| events[eventName] = true | ||
| } | ||
| } | ||
| case map[string]any: | ||
| for eventName := range v { | ||
| if isActivationMetadataTriggerField(eventName) { | ||
| continue | ||
| } | ||
| events[eventName] = true | ||
| } | ||
| } |
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent test quality
Test Classification DetailsView All Test Classifications (3 tests)
Test Highlights
All three tests:
Test inflation check: 79 new test lines vs 140 changed production lines → 0.56:1 ratio (well under 2:1 threshold ✅) Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24488869997
|
… derivation Generated by the Design Decision Gate workflow. Records the architectural decision to derive activation job permissions from parsed trigger events rather than granting broad write scopes unconditionally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (205 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What the draft coversThe generated ADR captures:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24488870005 Note 🔒 Integrity filter blocked 1 itemThe following item were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
| if hasReaction || hasStatusComment { | ||
| permsMap[PermissionIssues] = PermissionWrite | ||
| permsMap[PermissionPullRequests] = PermissionWrite | ||
| permsMap[PermissionDiscussions] = PermissionWrite |
There was a problem hiding this comment.
@copilot add frontmatter option to disable discussions for status-comments.
There was a problem hiding this comment.
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17dee383-88fc-4612-a885-b768f0497ffd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17dee383-88fc-4612-a885-b768f0497ffd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17dee383-88fc-4612-a885-b768f0497ffd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17dee383-88fc-4612-a885-b768f0497ffd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d99aa4d7-b4fc-420a-989f-6df131fc2cee Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d99aa4d7-b4fc-420a-989f-6df131fc2cee Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add status-comment.issues: Boolean to also selectively disable issue write support. Recompile lock files. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d99aa4d7-b4fc-420a-989f-6df131fc2cee Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 470d515. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
Agent-Logs-Url:--log-level=error bin/git` (http block)
Agent-Logs-Url: REDACTED 64/pkg/tool/linux_amd64/vet` (http block)
Agent-Logs-Url: REDACTED de/node/bin/git` (http block)
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/915874e7-3b67-43fa-8510-40b447231a2f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/915874e7-3b67-43fa-8510-40b447231a2f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/915874e7-3b67-43fa-8510-40b447231a2f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 2a9b73f (feature in 1a9ecde). Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot also add a field to control pull-requests |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/45039d61-768a-4aaf-bc99-a51fdcfc6947 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…dition Agent-Logs-Url: https://github.com/github/gh-aw/sessions/45039d61-768a-4aaf-bc99-a51fdcfc6947 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 3d22416 (follow-up cleanup in d57f5ea). Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…ment object form - Add byok-copilot feature flag subsection to frontmatter.md (from #26544: bundles cli-proxy, COPILOT_API_KEY injection, and latest Copilot CLI install behind a single flag) - Update all github-app examples to use client-id (preferred) instead of app-id (deprecated alias) across auth.mdx, checkout.md, faq.md, imports.md, safe-outputs.md, safe-outputs-pull-requests.md, tokens.md, triggers.md, and glossary.md (from #26551: run gh aw fix to auto-migrate) - Add object-form documentation for status-comment in triggers.md, with issues/pull-requests/discussions toggle fields (from #26535) - Update status-comment description in frontmatter.md to mention object form Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gh aw compilewas over-granting activation permissions (discussions: write,pull-requests: write) for workflows that only operate on issues. This changes activation permission derivation to be event-aware so compiled lock files request only the scopes required by configured triggers.Permission scoping in activation compiler
onevent keys.permissionsblock, andonparsing is invalid/missing/unsupported, activation permissions fall back to broad interaction scopes (matching prior behavior) to avoid under-scoped runtime tokens.pull-requests: writeis no longer granted for status-comments-only flows (it is still granted for reactions that need it).Event parsing and filtering hardening
onYAML instead of relying on loose substring matching.reaction,status-comment,command, etc.) from real event keys.onparsing and explicit logging when synthetic empty-onfallback is used.New frontmatter options: selectively disable status-comment targets
on.status-comment:issues(optional, defaulttrue) controlsissuesandissue_commentstatus comments.pull-requests(optional, defaulttrue) controlspull_requestandpull_request_review_commentstatus comments.discussions(optional, defaulttrue) controlsdiscussionanddiscussion_commentstatus comments.discussions: falsekeeps status comments enabled for issue/PR-related events while excludingdiscussionanddiscussion_comment.issues: falsekeeps status comments enabled for pull-request/discussion-related events while excluding issue events.pull-requests: falsekeeps status comments enabled for issue/discussion-related events while excluding pull request events.issues,pull-requests, anddiscussionsare allfalse, compilation fails with an actionable error instead of silently generating a never-running status-comment condition.Schema updates
main_workflow_schema.jsonsostatus-commentsupports both:issues,pull-requests, anddiscussionsfields (noenabledfield).Regression coverage updates
issues: writepull_request_review_comment+ reaction-only →pull-requests: writeonlystatus-comment.discussions: false→ no discussion status-comment condition and nodiscussions: writestatus-comment.issues: false→ no issue status-comment condition and no issue-derived write scope from status commentsstatus-comment.pull-requests: false→ no pull-request status-comment conditiononparsing fallback → broad interaction permissions are applied with target-toggle awareness.status-commentobject where all targets are disabled → compilation error.> [!WARNING]
>