Repository navigation
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🔎 PR Code Quality Reviewer is reviewing code quality for this pull request... |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd (bug-fix classification: scalar allowed-repos values were not normalized to arrays before being handed to gh-proxy, which produced a policy string instead of the array gh-proxy expects).
📋 Analysis
Root cause fix
The fix in tools_validation_github.go is precise: normalizeGitHubRepositoryInReposScope now wraps any non-keyword scalar string (owner/repo, owner/*, and the ${{ github.repository }} expression) in a one-element array, while still passing through the all/public keywords unchanged. This matches the root cause described in the linked issue (#70267) rather than papering over a single symptom.
Test coverage (/tdd)
TestGitHubGuardPolicyRepositoryScopes is a strong addition: it exercises both field aliases (allowed-repos/repos), the all/public keywords, the ${{ github.repository }} expression, owner wildcards, prefix wildcards, plain []string and []any YAML-parsed lists -- and runs every case through all three policy builders (MCP gateway, CLI proxy, DIFC proxy) via table-driven subtests. That's exactly the kind of regression coverage that should prevent this class of bug from resurfacing, and the existing hardcoded-JSON assertions in compiler_difc_proxy_test.go / copilot_github_mcp_test.go were correctly updated in lockstep.
Scope
Checked for silent breakage in related call sites (deriveSafeOutputsGuardPolicyFromGitHub, transformRepoPattern, enclave guard-policy builders, docs) -- none depend on the unwrapped scalar shape, so this change is safely isolated to the three policy builders it targets.
No actionable issues found -- this is a tightly scoped, well-tested fix. Approving.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 73.9 AIC · ⌖ 14.8 AIC · ⊞ 10.1K
Comment /matt to run again
There was a problem hiding this comment.
🟢 Approval recommended
The localized fix preserves supported inputs and includes targeted regression coverage, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Fixes #70267 by normalizing scalar repository scopes in gh-aw’s generated guard policies, preventing proxy startup rejection.
Changes:
- Wraps non-keyword strings in arrays while preserving
all,public, and existing lists. - Covers both
allowed-reposand deprecatedreposacross all three policy builders.
| File | Description |
|---|---|
pkg/workflow/tools_validation_github.go |
Normalizes scalar repository scopes. |
pkg/workflow/copilot_github_mcp_test.go |
Updates the expected repository policy format. |
pkg/workflow/compiler_difc_proxy_test.go |
Adds cross-builder regression coverage and updates expectations. |
Review effort: Max
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 3698c90
|
…epos-policy-issue Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Merged current |
|
update spec |
A single repository string in
tools.github.allowed-repospassed strict compilation but produced a policy rejected by gh-proxy at startup.all,public, and existing arrays; apply the same normalization to the deprecatedreposalias.For
allowed-repos: ${{ github.repository }}, the generated policy becomes:{"allow-only":{"min-integrity":"approved","repos":["${{ github.repository }}"]}}