Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Hey A few things to address before this is ready for review:
If you'd like a hand, here's a ready-to-use agent prompt:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/15a3b915-a669-4c39-8098-69a2f485018c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/15a3b915-a669-4c39-8098-69a2f485018c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/15a3b915-a669-4c39-8098-69a2f485018c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds target-scoped configuration for on.reaction (mirroring status-comment), updates activation reaction condition generation to honor those targets, and computes activation-job permissions from the union of enabled reaction/status-comment targets to avoid over-granting.
Changes:
- Extend reaction parsing to support an object form with
type+{issues, pull-requests, discussions}toggles (and reject “all disabled”). - Generate activation reaction
if:conditions using the configured reaction target toggles. - Scope activation interaction permissions based on enabled reaction/status-comment targets + detected trigger events; update schema and tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/reactions.go | Adds parseReactionConfig to support scalar + object reaction config with target toggles. |
| pkg/workflow/reactions_test.go | Adds unit tests for reaction object parsing and “all targets disabled” validation. |
| pkg/workflow/expression_builder.go | Introduces BuildReactionConditionForTargets and routes default behavior through it. |
| pkg/workflow/expressions_test.go | Adds test ensuring PR-related events are excluded when PR reactions are disabled. |
| pkg/workflow/compiler_types.go | Extends WorkflowData with reaction target toggle fields. |
| pkg/workflow/compiler_safe_outputs.go | Wires reaction object parsing into parseOnSection and stores target toggles in WorkflowData. |
| pkg/workflow/compiler_safe_outputs_test.go | Updates reaction map test to assert successful parsing + defaults/toggles. |
| pkg/workflow/compiler_activation_job.go | Uses reaction target toggles for reaction step condition and permission computation; adds helpers for defaulting toggles. |
| pkg/workflow/activation_permissions_scope_test.go | Adds coverage for permission/condition behavior when PR reactions are disabled; updates helper-call signatures. |
| pkg/parser/schemas/main_workflow_schema.json | Updates workflow schema to allow on.reaction object form with target toggles. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 0
🧪 Test Quality Sentinel ReportTest Quality Score: 80/100✅ Excellent test quality
Test Classification DetailsView All Test Classifications (9 tests)
Build Tag Check ✅All four test files carry Test Inflation — Borderline Note
The 10-point inflation penalty is applied per the formula, but this is not a genuine concern. Test Quality Highlights
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: §24526383893
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 9 new/modified tests enforce behavioral contracts around the new reaction-target scoping and activation permission union logic. No coding-guideline violations.
…ission derivation Generated by the Design Decision Gate workflow to document the architectural decisions implicit in PR #26693. 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 (>100 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft captures the three core decisions in this PR:
It also notes the relationship to ADR-26535 (event-scoped activation permission derivation), which this PR extends. 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: §24526383923 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
|
Activation failed on
pull_requestbecause the reaction step was treated as required and requested permissions without target-level control. This change adds reaction target scoping (mirroringstatus-comment) and derives activation permissions from the combined reaction + status-comment target configuration.Reaction config: add target-scoped object form
on.reactionnow supports object syntax with:type(reaction emoji, defaulteyes)issues,pull-requests,discussions(boolean target toggles)"eyes","+1",1, etc.) unchanged.Activation reaction condition: honor scoped targets
reaction.pull-requests: false, while enabled targets remain active.Activation permissions: compute union of scoped interactions
Schema and compiler model updates
on.reactionobject form.parseOnSection.Focused test coverage updates
In this configuration, activation reaction/status behavior and activation permissions are both derived from these scoped target sets rather than broad defaults.
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:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE x_amd64/vet git rev-�� --show-toplevel x_amd64/vet /usr/bin/git -json GO111MODULE x_amd64/vet git(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE 64/bin/go git rev-�� --show-toplevel sh /usr/bin/git "prettier" --chegit GOPROXY 64/bin/go git(http block)https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq [.object.sha, .object.type] | @tsv -bool -buildtags /usr/local/bin/uname -errorsas -ifaceassert -nilfunc uname -rs -stringintconv -tests /usr/bin/git -json Url: https://gitrev-parse 64/bin/go git(http block)/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq [.object.sha, .object.type] | @tsv -json GO111MODULE /opt/hostedtoolcache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /usr/bin/git GOINSECURE GOMOD GOMODCACHE git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v3/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq [.object.sha, .object.type] | @tsv -json GO111MODULE ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq [.object.sha, .object.type] | @tsv -json GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v5Agent-Logs-Url: REDACTED x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet` (http block)