[repository-quality] Repository Quality Improvement Report - Integration Test Binary-Path Duplication & Brittleness (2026-09-22)

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Integration Test Binary-Path Duplication & Brittleness

Analysis Date: 2026-09-22
Focus Area: Integration Test Binary-Path Duplication & Brittleness (custom)
Strategy Type: Custom
Custom Area: Yes — the repo already has a canonical GetBinaryPath()/logAndValidateBinaryPath() helper in pkg/cli/mcp_helpers.go for production code, but 32+ integration tests across 8 files independently re-implement a hardcoded "../../gh-aw" lookup-and-skip pattern instead of using a shared test helper. This is a concrete, previously unexamined maintainability/brittleness gap distinct from the last 10 runs (which covered linter CI gating, WASM docs, release tooling, nolint suppressions, and slash-command triggers).

Executive Summary

Ten pkg/cli integration test files (mcp_server_tools_test.go, mcp_server_compile_test.go, mcp_server_json_integration_test.go, mcp_server_operations_test.go, status_command_test.go, mcp_server_error_codes_test.go, mcp_server_fix_test.go, mcp_server_inspect_test.go, mcp_server_stdio_integration_test.go, mcp_server_add_test.go) contain 32 near-identical copies of a three-line block that hardcodes the built binary's relative path as "../../gh-aw" (or "../../gh-aw.exe" on Windows), checks os.Stat, and calls t.Skip("Skipping test: gh-aw binary not found. Run 'make build' first.") if missing. This pattern is duplicated 32 times across roughly 3,300 lines of test code.

The duplication creates three concrete risks: (1) the relative path ../../gh-aw silently breaks if any of these test files are ever moved to a subpackage, since Go resolves relative paths from the test binary's working directory, not from the source file location; (2) any future change to the binary output name/location (e.g., Makefile BINARY_NAME or a build-tag-specific output path) requires 32 manual edits with no compiler-enforced consistency; (3) tests silently no-op (pass green) rather than fail loudly when the binary is absent, which can mask genuinely broken integration coverage in CI if the make build step is ever skipped or reordered ahead of go test -tags integration.

Recommended fix: introduce a single testutil.RequireGhAwBinary(t *testing.T) string (or equivalent pkg/cli-local helper) that centralizes path resolution (honoring runtime.GOOS for .exe), and replace all 32 call sites with a one-line call. This reduces duplicated logic by roughly 90 lines, removes the risk of path drift, and gives a single point of control for future binary-location changes.

Full Analysis Report

Focus Area: Integration Test Binary-Path Duplication & Brittleness

Current State Assessment

The pkg/cli package already has a canonical, well-documented helper for binary path resolution used in production code (GetBinaryPath() and logAndValidateBinaryPath() in pkg/cli/mcp_helpers.go), which resolves the currently-running executable via os.Executable() + filepath.EvalSymlinks(). However, this helper is not reused by tests that need to invoke a freshly built gh-aw binary as a subprocess (e.g., for MCP server stdio integration checks). Instead, each test file reinvents binary discovery with a hardcoded relative path.

Metrics Collected:

Metric	Value	Status
Files with hardcoded "../../gh-aw" binary path	10 files	⚠️
Total duplicated skip-block occurrences	32	⚠️
Existing reusable helper in production code (GetBinaryPath)	Yes, unused by tests	⚠️
Dedicated test helper package (pkg/testutil)	Exists but lacks binary-path helper	⚠️
Total t.Skip(...) calls repo-wide	255	i️ (context)
"gh-aw binary not found" as single largest skip reason	35 occurrences	⚠️
Windows .exe variant handled consistently	Only in 1 of 10 files (mcp_server_json_integration_test.go)	⚠️

Findings

Strengths

• Production code already has a clean, tested GetBinaryPath() abstraction in pkg/cli/mcp_helpers.go that could be extended or mirrored for test use with minimal effort.
• The skip messages are consistently worded ("Run 'make build' first."), so consolidating them will not change developer-facing behavior — just the implementation.
• pkg/testutil already exists as the natural home for a shared cross-file test helper (currently holds only tempdir.go).

Areas for Improvement

• 🟠 High: 32 copies of the binary-discovery-and-skip pattern spread across 10 files (pkg/cli/mcp_server_compile_test.go alone has 8 copies at lines 20-23, 168-171, 271-274, 374-377, 485-488, 572-575, 693-696, plus one commented-out copy at 107-110). Any Makefile change to BINARY_NAME or build output location requires 32 manual edits with no single source of truth.
• 🟡 Medium: Only pkg/cli/mcp_server_json_integration_test.go (line 607-613) special-cases runtime.GOOS == "windows" to check for a .exe suffix. The other 9 files assume a Unix-style binary name unconditionally, meaning Windows CI runs of those suites (status_command_test.go, mcp_server_tools_test.go, mcp_server_operations_test.go, mcp_server_error_codes_test.go, mcp_server_fix_test.go, mcp_server_inspect_test.go) will silently skip every test on Windows rather than looking for gh-aw.exe.
• ⚪ Low: A stale commented-out duplicate of the skip block exists at pkg/cli/mcp_server_compile_test.go lines 107-110, adding noise without value.

Detailed Analysis

The pattern in question (representative example from pkg/cli/mcp_server_tools_test.go:17-21):

func TestMCPServer_ListTools(t *testing.T) {
	// Skip if the binary doesn't exist
	binaryPath := "../../gh-aw"
	if _, err := os.Stat(binaryPath); os.IsNotExist(err) {
		t.Skip("Skipping test: gh-aw binary not found. Run 'make build' first.")
	}
	...

is repeated verbatim (with only the enclosing function name differing) 32 times. A single shared helper such as:

// pkg/testutil/binary.go
func RequireGhAwBinary(t *testing.T) string {
	t.Helper()
	name := "gh-aw"
	if runtime.GOOS == "windows" {
		name += ".exe"
	}
	binaryPath := filepath.Join("..", "..", name)
	if _, err := os.Stat(binaryPath); os.IsNotExist(err) {
		t.Skipf("Skipping test: %s binary not found. Run 'make build' first.", name)
	}
	return binaryPath
}

would collapse each 3-4 line block into a single binaryPath := testutil.RequireGhAwBinary(t) call, uniformly fix the Windows .exe gap, and centralize any future path changes.

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Add a shared RequireGhAwBinary test helper to pkg/testutil

Priority: High
Estimated Effort: Small
Focus Area: Integration Test Binary-Path Duplication

Description: Create a new file pkg/testutil/binary.go with a RequireGhAwBinary(t *testing.T) string function that resolves the built gh-aw binary's relative path (honoring runtime.GOOS for the .exe suffix on Windows), calls t.Skipf(...) with the standard message if the binary is absent, and returns the resolved path for use in exec.Command(...). Add a small table-driven unit test in pkg/testutil/binary_test.go covering the exists/not-exists cases (create a temp file to simulate the "exists" branch; skip is inherently hard to assert directly, so assert on the returned path format and the non-skip branch).

Acceptance Criteria:

• pkg/testutil/binary.go exports RequireGhAwBinary(t *testing.T) string
• Function honors runtime.GOOS == "windows" to append .exe
• Function uses t.Helper() so failure/skip lines point at the caller
• Unit test added and passing (go test ./pkg/testutil/...)

Code Region: pkg/testutil/ (new file)

Add a new file pkg/testutil/binary.go exporting `func RequireGhAwBinary(t *testing.T) string` that:
1. Computes binaryPath as filepath.Join("..", "..", "gh-aw") with a ".exe" suffix added when runtime.GOOS == "windows".
2. Calls t.Helper() first.
3. Uses os.Stat to check existence; if os.IsNotExist(err), calls t.Skipf("Skipping test: %s binary not found. Run 'make build' first.", filepath.Base(binaryPath)) and returns "".
4. Returns the resolved binaryPath otherwise.
Add pkg/testutil/binary_test.go with a unit test that creates a temp file to simulate presence and asserts the returned path is non-empty, plus a test asserting correct suffix selection based on runtime.GOOS (using a build-tag-free approach, e.g. testing the string-building logic extracted into an unexported helper if needed for determinism across OSes).
Run `go test ./pkg/testutil/...` to confirm it passes, then `make fmt`.

---

Task 2: Migrate pkg/cli/mcp_server_compile_test.go and mcp_server_json_integration_test.go to the shared helper

Priority: High
Estimated Effort: Medium
Focus Area: Integration Test Binary-Path Duplication

Description: Replace all 8 duplicated skip blocks in pkg/cli/mcp_server_compile_test.go (lines ~20-23, 168-171, 271-274, 374-377, 485-488, 572-575, 693-696) and all 7 in pkg/cli/mcp_server_json_integration_test.go (including the Windows-specific block at line 607-613) with calls to testutil.RequireGhAwBinary(t). Also remove the stale commented-out duplicate block at lines 107-110 of mcp_server_compile_test.go.

Acceptance Criteria:

• All 8 skip blocks in mcp_server_compile_test.go replaced with binaryPath := testutil.RequireGhAwBinary(t)
• All 7 skip blocks in mcp_server_json_integration_test.go (including the .exe variant) replaced
• Stale commented-out block removed
• go build -tags integration ./pkg/cli/... compiles cleanly
• go vet ./pkg/cli/... passes

Code Region: pkg/cli/mcp_server_compile_test.go, pkg/cli/mcp_server_json_integration_test.go

In pkg/cli/mcp_server_compile_test.go and pkg/cli/mcp_server_json_integration_test.go, replace every occurrence of the pattern:
    // Skip if the binary doesn't exist
    binaryPath := "../../gh-aw"
    if _, err := os.Stat(binaryPath); os.IsNotExist(err) {
        t.Skip("Skipping test: gh-aw binary not found. Run 'make build' first.")
    }
(and the .exe variant in mcp_server_json_integration_test.go around line 607-613)
with:
    binaryPath := testutil.RequireGhAwBinary(t)
Import "github.com/github/gh-aw/pkg/testutil" as needed. Remove the now-unused os import if no longer referenced elsewhere in the file, and remove the stale commented-out duplicate block near line 107-110 in mcp_server_compile_test.go. Run `go build -tags integration ./pkg/cli/...` and `go vet ./pkg/cli/...` to confirm correctness, then `make fmt`.

---

Task 3: Migrate remaining MCP server test files to the shared helper

Priority: Medium
Estimated Effort: Medium
Focus Area: Integration Test Binary-Path Duplication

Description: Apply the same testutil.RequireGhAwBinary(t) migration to the remaining files: pkg/cli/mcp_server_tools_test.go (5 occurrences), pkg/cli/mcp_server_operations_test.go (4), pkg/cli/status_command_test.go (3), pkg/cli/mcp_server_error_codes_test.go (2), pkg/cli/mcp_server_fix_test.go (2), pkg/cli/mcp_server_inspect_test.go (2). This also fixes the Windows .exe blind spot for these six files, since the shared helper handles runtime.GOOS correctly.

Acceptance Criteria:

• All 18 remaining skip blocks replaced across the 6 listed files
• Windows binary discovery now works correctly for these previously Unix-only files
• go build -tags integration ./pkg/cli/... compiles cleanly
• make build && go test -tags integration -run 'TestMCPServer|TestStatus' ./pkg/cli/... passes locally (binary present)

Code Region: pkg/cli/mcp_server_tools_test.go, pkg/cli/mcp_server_operations_test.go, pkg/cli/status_command_test.go, pkg/cli/mcp_server_error_codes_test.go, pkg/cli/mcp_server_fix_test.go, pkg/cli/mcp_server_inspect_test.go

In pkg/cli/mcp_server_tools_test.go, pkg/cli/mcp_server_operations_test.go, pkg/cli/status_command_test.go, pkg/cli/mcp_server_error_codes_test.go, pkg/cli/mcp_server_fix_test.go, and pkg/cli/mcp_server_inspect_test.go, replace every occurrence of:
    // Skip if the binary doesn't exist
    binaryPath := "../../gh-aw"
    if _, err := os.Stat(binaryPath); os.IsNotExist(err) {
        t.Skip("Skipping test: gh-aw binary not found. Run 'make build' first.")
    }
with:
    binaryPath := testutil.RequireGhAwBinary(t)
Add the "github.com/github/gh-aw/pkg/testutil" import to each file and remove the os import if it becomes unused. Verify with `go build -tags integration ./pkg/cli/...` and, if a locally built binary is available via `make build`, run `go test -tags integration -run 'TestMCPServer|TestStatus' ./pkg/cli/...`. Finish with `make fmt`.

---

Task 4: Document the binary-discovery convention for integration tests

Priority: Low
Estimated Effort: Small
Focus Area: Documentation / Testing

Description: Add a short section to the developer-facing testing documentation (e.g., docs/src/content/docs/ testing guide or a relevant .github/skills/developer/SKILL.md reference) explaining that integration tests requiring the built gh-aw binary must use testutil.RequireGhAwBinary(t) rather than hardcoding relative paths, so the convention is discoverable for future test authors and does not regress.

Acceptance Criteria:

• A short paragraph documenting testutil.RequireGhAwBinary(t) usage is added to the appropriate developer docs location
• The doc references at least one migrated test file as an example
• Existing doc linting/build (if any, e.g. make recompile for workflow docs or the docs site build) still passes

Code Region: docs/src/content/docs/ (developer/testing guide) or .github/skills/developer/SKILL.md

Find the developer-facing testing documentation for gh-aw (check docs/src/content/docs/ for a contributing/testing guide, or .github/skills/developer/SKILL.md). Add a short note explaining that any `(go/redacted):build integration` test that needs to invoke the compiled gh-aw binary as a subprocess should call `testutil.RequireGhAwBinary(t)` from pkg/testutil instead of hardcoding a relative path like "../../gh-aw", since this helper correctly handles the Windows .exe suffix and centralizes future path changes. Reference pkg/cli/mcp_server_tools_test.go as a migrated example. Keep the addition concise (a few sentences plus a code snippet) and consistent with the existing doc's tone and formatting conventions.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-09-08	Slash-Command Trigger Centralization Gap	Custom	Y	Identified scattered slash-command parsing logic
2026-09-09	Framework-Job Timeout Coverage Gap	Custom	Y	Found jobs missing explicit timeout-minutes
2026-09-10	Security Scan Pipeline Reliability & Gosec Suppression-Requirement Debt	Standard (Security)	N	Flagged unjustified gosec suppressions
2026-09-11	Custom Go Linter CI Enablement Gap (Follow-up)	Reuse	N	Continued linter CI gating work
2026-09-14	Security Scan Pipeline Reliability (Follow-up)	Reuse	N	Further gosec suppression cleanup
2026-09-15	WASM Build-Variant Documentation & Stub-Coverage Debt	Custom	Y	Documented WASM stub gaps
2026-09-16	Skill Documentation Drift (Stale JS Location References)	Custom	Y	Fixed stale skill doc paths
2026-09-17	Release Documentation & Tooling Debt	Custom	Y	Addressed abandoned changeset/CHANGELOG pipeline
2026-09-18	Unjustified nolint Suppression Debt	Custom	Y	Audited nolint comments lacking justification
2026-09-21	Custom Go Linter CI Enablement Gap (Second Follow-up)	Reuse	N	Further linter CI enablement follow-up

---

🎯 Recommendations

Immediate Actions (This Week)

1. Implement testutil.RequireGhAwBinary(t) helper (Task 1) — Priority: High
2. Migrate the two most-duplicated files, mcp_server_compile_test.go and mcp_server_json_integration_test.go (Task 2) — Priority: High

Short-term Actions (This Month)

1. Migrate the remaining 6 MCP/status test files (Task 3) — Priority: Medium
2. Document the convention for future contributors (Task 4) — Priority: Low

Long-term Actions (This Quarter)

1. Consider a lint rule (custom pkg/linters analyzer) that flags hardcoded "../../gh-aw" string literals in test files to prevent regression — Priority: Low

---

📈 Success Metrics

• Duplicated binary-skip blocks: 32 → 0 (all routed through testutil.RequireGhAwBinary)
• Windows .exe-aware test files: 1 of 10 → 10 of 10
• Lines of duplicated test scaffolding removed: ~90 lines

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate this focus area in 4-6 weeks to confirm no new hardcoded binary-path duplication has crept back in

---

Generated by Repository Quality Improvement Agent
Next analysis: 2026-09-23 — Focus area selected by diversity algorithm

Generated by ⚡ Repository Quality Improvement Agent · copilot · auto · 50.7 AIC · ⌖ 11.3 AIC · ⊞ 9.8K · ◷

• expires on Sep 23, 2026, 5:03 AM UTC-08:00

[github-actions[bot]]

This discussion has been marked as outdated by Repository Quality Improvement Agent.

A newer discussion is available at Discussion #62961.