Include firewall domain decisions in logs JSON by Copilot · Pull Request #59575 · github/gh-aw

Copilot · 2026-09-09T00:39:34Z

The logs JSON output lacked a normalized domain inventory showing whether firewall requests were allowed or blocked.

Domain reporting
- Add deterministic firewall_log.domains records.
- Include domain, acceptance state, and request count.
- Emit separate records when a domain has both allowed and blocked requests.

"domains": [
  { "domain": "api.github.com:443", "accepted": true, "arity": 2 },
  { "domain": "example.com:443", "accepted": false, "arity": 1 }
]

Compatibility
- Preserve existing aggregate counters, domain lists, and requests_by_domain.
- Add a patch changeset for the expanded JSON output.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-09-09T00:44:15Z

✅ PR Code Quality Reviewer completed the code quality review.

Reviewed PR #59575 and found no actionable review comments to publish.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-09-09T00:44:22Z

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

No ADR enforcement needed: PR does not have the implementation label and has <=100 new lines of code in business logic directories.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-09-09T00:44:23Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-09-09T00:44:23Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-09-09T00:45:12Z

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #59575

Copilot

🟡 Changes recommended

Placeholder keys must be excluded from normalized domain records before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds deterministic firewall-domain decisions and request counts to the logs JSON output.

Changes:

Adds sorted domain records with acceptance state and request count.
Tests mixed allowed/blocked decisions and serialization.
Adds a patch changeset.

File summaries

File	Review
`pkg/cli/logs_report_test.go`	Tests aggregation, ordering, and JSON output.
`pkg/cli/logs_report_firewall.go`	Builds normalized records, but must exclude placeholder domains such as `(unknown)` and `-`.
`.changeset/patch-logs-firewall-domains.md`	Documents the output enhancement.

Review details

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

+	for _, domain := range sliceutil.SortedKeys(allRequestsByDomain) {
+		stats := allRequestsByDomain[domain]


github-actions · 2026-09-09T00:49:06Z

Comment Memory

reviewed_at: 2026-09-09T00:45:43Z
review_event: COMMENT
top_themes:
  - no actionable blocking issues in changed lines
files_reviewed:
  - .changeset/patch-logs-firewall-domains.md
  - pkg/cli/logs_report_firewall.go
  - pkg/cli/logs_report_test.go
comment_count: 0

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 30 AIC · ⌖ 7.32 AIC · ⊞ 21.8K · ◷
_{Comment /review to run again}

github-actions

Verdict

Non-blocking change. The new firewall_log.domains payload matches the PR description by emitting one record per domain/decision pair, and I did not find a correctness or merge-blocking issue in the changed lines.

Themes

The added aggregation is deterministic because it iterates sorted domain keys.
Existing aggregate counters and requests_by_domain remain intact, so the JSON expansion is additive rather than a breaking replacement.
The updated test covers the mixed allow/block case that this change introduces.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 30 AIC · ⌖ 7.32 AIC · ⊞ 21.8K
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /grill-with-docs and /tdd on this small, well-tested feature addition.

📋 Key Themes & Highlights

Key Themes

Naming: Arity (see inline comment) is a nonstandard term for a request count and may confuse API consumers.

Positive Highlights

✅ Deterministic ordering via sliceutil.SortedKeys keeps JSON output stable for diffing/tests.
✅ Test in logs_report_test.go exercises the mixed allowed/blocked domain case and asserts both the struct value and the marshaled JSON substring.
✅ Backward compatible: existing requests_by_domain, allowed_domains, blocked_domains fields are preserved untouched; new field is omitempty.
✅ Changeset included for the patch release.

No blocking issues found; the single naming suggestion is non-blocking.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 34.8 AIC · ⌖ 14.7 AIC · ⊞ 10.3K
_{Comment /matt to run again}

github-actions · 2026-09-09T00:49:32Z

+type FirewallDomainRecord struct {
+	Domain   string `json:"domain"`
+	Accepted bool   `json:"accepted"`
+	Arity    int    `json:"arity"`


[/grill-with-docs] Arity is an unusual name for "number of requests" — arity normally means the number of arguments a function takes. This could confuse consumers of the JSON output.

💡 Suggestion

Consider renaming to something that matches the domain vocabulary already used elsewhere in this file, e.g. Count or Requests (compare DomainRequestStats.Allowed/Blocked and TotalRequests/AllowedRequests naming). The PR body's example JSON also uses arity, so the public JSON key would need to change too if you rename.

@copilot please address this.

github-actions

Reviewed the firewall domain-decisions JSON change (backend, no UI). Logic, tests, and changeset are solid. One non-blocking naming clarity issue flagged inline: arity is a confusing name for a request count.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 25.3 AIC · ⌖ 13.3 AIC · ⊞ 8.3K

github-actions · 2026-09-09T00:51:37Z

+type FirewallDomainRecord struct {
+	Domain   string `json:"domain"`
+	Accepted bool   `json:"accepted"`
+	Arity    int    `json:"arity"`


The field name arity is unclear for API consumers — it actually represents a request count for a domain+decision pair, not mathematical arity (number of arguments/operands). Consider renaming to something self-explanatory like count or request_count (and the Go field to Count) so downstream JSON consumers don't need to guess its meaning from source code.

@copilot please address this.

Add firewall domains to logs JSON

014b9ae

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI assigned Copilot and pelikhan Sep 9, 2026

Copilot AI changed the title ~~Include firewall domains in logs JSON output~~ Include firewall domain decisions in logs JSON Sep 9, 2026

Copilot finished work on behalf of pelikhan September 9, 2026 00:39

Copilot AI requested a review from pelikhan September 9, 2026 00:39

Copilot created this pull request from a session on behalf of pelikhan September 9, 2026 00:39 View session

pelikhan marked this pull request as ready for review September 9, 2026 00:41

Copilot AI balanced review requested due to automatic review settings September 9, 2026 00:41

Copilot started reviewing on behalf of pelikhan September 9, 2026 00:42 View session

pelikhan closed this Sep 9, 2026

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread pkg/cli/logs_report_firewall.go

Comment on lines +156 to +157

for _, domain := range sliceutil.SortedKeys(allRequestsByDomain) {

stats := allRequestsByDomain[domain]

github-actions Bot reviewed Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

Include firewall domain decisions in logs JSON - #59575

Include firewall domain decisions in logs JSON#59575
pelikhan with Copilot wants to merge 1 commit into
mainfrom
copilot/add-firewall-domain-info-logs

Copilot AI commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Sep 9, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		for _, domain := range sliceutil.SortedKeys(allRequestsByDomain) {
		stats := allRequestsByDomain[domain]

Uh oh!

Conversation

Copilot AI commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

github-actions Bot commented Sep 9, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading