[test-improver] Improve tests for auth package

[github-actions]

Test Improvements: internal/auth/header_test.go

File Analyzed

• Test File: internal/auth/header_test.go
• Package: internal/auth
• Lines Changed: +78 / -20

---

1. Fix Bound Asserter Scoping (correctness fix)

All seven table-driven test functions in header_test.go bound assert/require to the outer *testing.T, then used those bound asserters inside t.Run subtests:

// Before (incorrect pattern)
func TestIsMalformedHeader(t *testing.T) {
    assert := assert.New(t)           // bound to OUTER t
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {   // inner t is different
            assert.Equal(tt.want, got)         // silently uses OUTER t!
        })
    }
}

This caused two real problems:

• Wrong failure attribution: test failures were reported against TestIsMalformedHeader instead of TestIsMalformedHeader/Null_byte_is_malformed, making failures much harder to diagnose
• Wrong FailNow target: require.NoError/require.ErrorIs call t.FailNow() on the outer test, which can abort all remaining subtests when only one should stop

Fix: switched all inner assertions to the non-bound form using the subtest's own t:

// After (correct pattern)
func TestIsMalformedHeader(t *testing.T) {
    t.Parallel()
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            t.Parallel()
            assert.Equal(t, tt.want, got)  // uses subtest's t
        })
    }
}

Affected test functions:

• TestIsMalformedHeader
• TestTruncateSecret
• TestParseAuthHeader (also had require.ErrorIs/require.NoError scoping issue)
• TestValidateAgentID
• TestExtractAgentID
• TestExtractSessionID
• TestStripAuthScheme

2. Add t.Parallel() Throughout

All top-level test functions and their subtests now call t.Parallel(). The auth functions under test are pure (no global state mutation), so parallel execution is safe and catches potential data-race issues.

3. New Edge-Case Test Inputs

Added missing inputs that test boundary behaviour of the scheme-stripping logic:

TestParseAuthHeader — 2 new cases:

• "Bearer " (Bearer prefix with empty token) → apiKey="", agentID=""
• "Agent " (Agent prefix with empty value) → apiKey="", agentID=""

TestExtractSessionIDFromHeaders — 3 new cases:

• Both headers empty → ""
• Valid X-Agent-ID with malformed Authorization → returns the valid X-Agent-ID
• X-Agent-ID present alongside a Bearer Authorization → X-Agent-ID wins

TestStripAuthScheme — 2 new cases:

• "Bearer " → scheme "Bearer", value "", matched true
• "Agent " → scheme "Agent", value "", matched true

Test Execution

All tests pass and are stable across multiple runs:

ok  github.com/github/gh-aw-mcpg/internal/auth  0.009s

Subtests now report correctly:

--- PASS: TestIsMalformedHeader/Null_byte_(0x00)_is_malformed (0.00s)
--- PASS: TestParseAuthHeader/Bearer_with_empty_token (0.00s)
--- PASS: TestExtractSessionIDFromHeaders/both_headers_empty_returns_empty_string (0.00s)

Notes on Remaining Uncovered Lines

The two functions below 100% contain legitimately untestable paths:

• ExtractAgentID (83.3%): the if err != nil guard after ParseAuthHeader is dead code — ParseAuthHeader only errors on empty input, which is already handled by the preceding nil-check
• GenerateRandomAgentID (71.4%): the error path requires crypto/rand to fail, which needs dependency injection to test

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

Firewall blocked 1 domain

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

• index.crates.io

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

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · 1.8K AIC · ⊞ 29.5K · ◷

[Copilot]

Pull request overview

This PR improves the internal/auth test suite by fixing incorrect testify bound-asserter usage in subtests, increasing parallelization, and adding coverage for additional auth header edge cases. These changes strengthen failure attribution and exercise more boundary behaviors in auth scheme parsing and session ID extraction.

Changes:

• Replaced bound assert.New(t) / require.New(t) usage with assert.*(t, ...) / require.*(t, ...) inside t.Run subtests.
• Added t.Parallel() to top-level tests and subtests.
• Added new test cases for empty Bearer / Agent values and additional ExtractSessionIDFromHeaders precedence scenarios.

Show a summary per file

File	Description
internal/auth/header_test.go	Refactors subtest assertion scoping, adds parallelization, and expands edge-case coverage for auth header parsing and session ID extraction.

Copilot's findings

Tip

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

• Files reviewed: 1/1 changed files
• Comments generated: 7