[test-improver] Improve tests for server requireGuardPolicyIfGuardEnabled#2096
Draft
github-actions[bot] wants to merge 1 commit intomainfrom
Draft
[test-improver] Improve tests for server requireGuardPolicyIfGuardEnabled#2096github-actions[bot] wants to merge 1 commit intomainfrom
github-actions[bot] wants to merge 1 commit intomainfrom
Conversation
Add five new test cases covering previously untested code paths in requireGuardPolicyIfGuardEnabled: - NilGuard: verifies the nil-guard early return (g == nil) - NoopGuard: verifies the noop-guard early return (g.Name() == "noop") - WithValidGlobalPolicy: verifies the non-nil policy path (guard is kept) - WithInvalidGlobalPolicy: verifies error propagation from resolveGuardPolicy - UnknownServerID: verifies fallback to noop for an unrecognised server ID Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox
added a commit
that referenced
this pull request
Mar 19, 2026
…sue (#2158) ## Summary Adds a **Guard Filtering Summary** section to the repo-assist Monthly Activity Issue so maintainers can see what objects the guard policy blocked during each run. ## What it looks like When the guard filters objects, the monthly activity issue will include: ```markdown ## Guard Filtering Summary | Type | Count | Resources | |------|-------|-----------| | Issues | 7 | #1711, #2049, #2086, #2087, #2089, #2093, #2100 | | PRs | 7 | #2037, #2042, #2061, #2063, #2064, #2092, #2096 | | Other | 2 | actions_list, get_repository_tree | **Policy**: `repos: [github/*], min-integrity: merged` **Total filtered**: 54 items across 17 tool calls ``` When no filtering occurs, it states "No objects were filtered by the guard policy." ## How it works 1. **New section in issue template** — "Guard Filtering Summary" sits between "Future Work" and "Run History" 2. **New step 6** — Agent reads `/tmp/gh-aw/mcp-logs/rpc-messages.jsonl` via bash, parses `DIFC_FILTERED` entries, groups by type (issues/PRs/other), deduplicates across tool calls 3. **Python one-liner** — Extracts resource descriptions, groups into a JSON summary the agent uses to populate the template ## Motivation From [run 23274488766](https://github.com/github/gh-aw-mcpg/actions/runs/23274488766), 54 objects were silently filtered with `min-integrity: merged`. The agent reported "GitHub API access to private repo issues unavailable" without understanding why. This change gives both the agent and maintainers explicit visibility into guard policy impact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
File Analyzed
internal/server/require_guard_policy_test.gointernal/serverImprovements Made
1. Increased Coverage
requireGuardPolicyIfGuardEnabledinunified.gohas five distinct code paths. The original three tests only exercised two of them (thepolicy == nilbranches). Three paths had zero direct coverage:nilguardg == nilat line 681g.Name() == "noop"at line 681return g, nilat line 704return nil, errat line 687cfg.Servers2. New Tests Added
✅
TestRequireGuardPolicyIfGuardEnabled_NilGuard— verifies that anilguard is returned immediately without any policy lookup (early-returng == nilbranch).✅
TestRequireGuardPolicyIfGuardEnabled_NoopGuard— verifies that a noop guard passes through unchanged (early-returng.Name() == "noop"branch), usingguard.NewNoopGuard().✅
TestRequireGuardPolicyIfGuardEnabled_WithValidGlobalPolicy— verifies that whencfg.GuardPolicyis a validallow-onlypolicy the non-noop guard is kept (return g, nilat line 704).✅
TestRequireGuardPolicyIfGuardEnabled_WithInvalidGlobalPolicy— verifies that aGuardPolicywith an invalidmin-integritylevel causesresolveGuardPolicyto return an error, which is propagated as(nil, err).✅
TestRequireGuardPolicyIfGuardEnabled_UnknownServerID— verifies that calling with a server ID not present incfg.Serversfalls back to noop (nil policy + no guard-policies entry).3. Test Patterns Consistent with Existing File
All new tests follow the established patterns in the file:
requirefor fatal assertions (error presence / guard non-nil)assertfor value comparisonsmockGuardhelper structguard.NewNoopGuard()used directly (already imported)Why These Changes?
requireGuardPolicyIfGuardEnabledis the gating function that decides whether an active (non-noop) guard is allowed to enforce policy for a given backend server. Its five branches each have distinct runtime consequences for DIFC enforcement. The three previously untested paths include the only path that returns an error, the only path where anilguard is a legitimate input, and the primary "happy path" where a configured global policy keeps the guard active. Covering these paths ensures regressions in the guard-policy resolution logic are caught by tests rather than surfacing at runtime.Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests