Skip to content

[test-improver] Improve tests for internal/cmd (registerGuardsModeFlag) - #10646

Merged
lpcox merged 2 commits into
mainfrom
test-improver/flags-difc-coverage-f47a1edf1b873816
Aug 4, 2026
Merged

[test-improver] Improve tests for internal/cmd (registerGuardsModeFlag)#10646
lpcox merged 2 commits into
mainfrom
test-improver/flags-difc-coverage-f47a1edf1b873816

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Improved test coverage for internal/cmd/flags_difc.go, specifically registerGuardsModeFlag, which registers the --guards-mode CLI flag and its shell completion function.

File analyzed

  • internal/cmd/flags_difc.go
  • internal/cmd/flags_difc_test.go

Analysis

Ran go test -coverprofile=/tmp/cov.out ./internal/cmd/ + go tool cover -func and found registerGuardsModeFlag at only 66.7% coverage. The uncovered branch was the error path where cmd.RegisterFlagCompletionFunc fails (e.g. when a completion func is already registered for that flag name) — this error was only logged via debugLog.Printf, never returned, so it had no direct test.

Changes

  • Refactor: Extracted registerGuardsModeCompletion(cmd *cobra.Command) out of registerGuardsModeFlag so the completion-registration logic (and its failure path) can be exercised in isolation without needing to re-declare the underlying flag (which would panic via pflag's duplicate-flag registration).
  • New tests (TestRegisterGuardsModeFlag in flags_difc_test.go):
    • registers flag with default value and completion — verifies the flag is registered with difc.DefaultEnforcementMode() as default.
    • registers completion successfully — verifies registerGuardsModeCompletion succeeds on a fresh flag.
    • logs but does not panic when completion func already registered — pre-registers the completion func on the command, then calls registerGuardsModeCompletion again to force cobra's "flag completion already registered" error, asserting it's handled gracefully (logged, not propagated/panicking).

All changes use testify (require/assert) consistent with the rest of the file, and preserve all existing passing tests unchanged.

Coverage

  • registerGuardsModeFlag: 66.7% → 100.0%
  • registerGuardsModeCompletion (new): 100.0%

Verification

go test -v ./internal/cmd/       # PASS, all tests including new TestRegisterGuardsModeFlag subtests
go test -count=3 ./internal/cmd/ # PASS (3x, no flakiness)
go vet ./internal/cmd/           # clean
gofmt -l internal/cmd/           # no output (already formatted)

Cache-memory updated with internal/cmd/flags_difc_test.go to avoid re-selecting this file in future runs.

Generated by Test Improver · auto · 214.8 AIC · ⊞ 8.1K ·

- Extract registerGuardsModeCompletion from registerGuardsModeFlag to make
  the completion-registration error path independently testable.
- Add TestRegisterGuardsModeFlag covering flag defaults, successful
  completion registration, and the previously-untested error branch when
  RegisterFlagCompletionFunc returns 'already registered' (now only logged,
  not propagated).
- registerGuardsModeFlag coverage: 66.7% -> 100.0%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox
lpcox marked this pull request as ready for review August 4, 2026 13:39
Copilot AI balanced review requested due to automatic review settings August 4, 2026 13:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds targeted coverage for --guards-mode flag and completion registration.

Changes:

  • Extracts completion registration into a testable helper.
  • Adds flag registration and duplicate-completion tests.
Show a summary per file
File Description
internal/cmd/flags_difc.go Extracts completion registration logic.
internal/cmd/flags_difc_test.go Adds focused registration tests.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

require.NoError(t, cmd.RegisterFlagCompletionFunc("guards-mode", cobra.FixedCompletions(
difc.ValidModes, cobra.ShellCompDirectiveNoFileComp)))

registerGuardsModeCompletion(cmd)
Comment thread internal/cmd/flags_difc_test.go Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox

lpcox commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

@lpcox

lpcox commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_pull_requests, get_file_contents) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error[-32602]: unknown tool — gateway enforcement BLOCKED
C CLI reads (list_issues, get_file_contents via github CLI) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) refused (gh unauthenticated) BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) refused (gh unauthenticated) BLOCKED

Overall: PASS

Part B: gateway exposes only 22 read-only tools; all write tool names return Error[-32602]: unknown tool — confirming gateway-level enforcement.

References: §30918053207

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc kernel-level isolation)

Part Surface Op Result Expected Status
A MCP reads data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error[-32602]: unknown tool BLOCKED
C CLI reads data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated — refused BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated — refused BLOCKED

Overall: PASS

Gateway enforcement: write tools absent from MCP registry (unknown tool [-32602]). CLI writes blocked by unauthenticated gh. No write leaked.

References: §30918053221

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool [-32602] BLOCKED
C CLI reads (issues/file) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated BLOCKED

Overall: PASS

Note — Part B: Write tools absent from gateway tool list (MCP error [-32602] unknown tool). Gateway enforces read-only at the MCP layer.

Note — Parts D/E: gh CLI unauthenticated; all REST/GraphQL writes rejected before reaching GitHub.

References: §30918053088

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

@lpcox
lpcox merged commit af28a5a into main Aug 4, 2026
38 checks passed
@lpcox
lpcox deleted the test-improver/flags-difc-coverage-f47a1edf1b873816 branch August 4, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants