test: tighten single-role GH_AW_REQUIRED_ROLES assertion (fixes #26799)

[Copilot]

Summary

• addresses review feedback from issue Likely false-positive match on single-string role test assert #26799
• makes the single-role string test assert the exact compiled YAML fragment: GH_AW_REQUIRED_ROLES: "write"
• adds a negative assertion to ensure the default role list is not used for single-string role input

Validation

• targeted tests passed:
  • go test -v -run 'TestRoleMembershipSupportsSingleRoleString|TestRoleMembershipUsesGitHubToken|TestRoleMembershipTokenWithBots' ./pkg/workflow/
• make agent-finish was run and fails due a pre-existing timeout in pkg/cli tests (panic: test timed out after 10m0s), reproducible before and after this test-only change
• parallel validation completed:
  • Code Review: no comments
  • CodeQL: no alerts reported (analysis skipped for large DB)

[Copilot]

Pull request overview

Updates a workflow compiler test to avoid false-positive matching when validating single-string on.roles configuration, per issue #26799.

Changes:

• Tighten the single-role assertion to match the exact compiled YAML fragment GH_AW_REQUIRED_ROLES: "write".
• Add a negative assertion to ensure the compiler does not fall back to the default role list when a single-string role is provided.

Show a summary per file

File	Description
pkg/workflow/role_checks_test.go	Makes single-role test assertions more specific and adds a guard against default-role fallback.

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: 0

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	0

---

Test Classification Details

Test	File	Classification	Issues Detected
TestRoleMembershipSupportsSingleRoleString	pkg/workflow/role_checks_test.go:151	✅ Design	None

---

Analysis

This PR fixes a weak assertion in TestRoleMembershipSupportsSingleRoleString. The original assertion assert.Contains(t, compiledStr, "write", ...) was too broad — it would pass even if the compiled workflow incorrectly expanded the single role write into the default list admin,maintainer,write. The fix makes two targeted improvements:

1. Tightened positive assertion: Replaced the vague "write" substring check with GH_AW_REQUIRED_ROLES: "write", pinning the exact environment variable and value that must appear.
2. Added regression guard: Added assert.NotContains for GH_AW_REQUIRED_ROLES: "admin,maintainer,write" — this directly encodes the invariant that a single-role string must not fall back to the default role list.

Both new assertions carry descriptive messages ✅. The test compiles a real workflow and inspects the output (no mocks) ✅. The t.Fatalf calls cover error paths ✅. Build tag //go:build !integration is present on line 1 ✅.

What design invariant does this test enforce? A behavioral contract: when roles: is specified as a scalar string (roles: write), the compiled GH_AW_REQUIRED_ROLES env var must equal exactly "write" — not the default multi-role fallback.

What would break if deleted? A regression where a single-role string is silently coerced into the default role list would go undetected, allowing unauthorized actors to trigger role-gated workflows.

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 1 test — unit (//go:build !integration)
• 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

---

Verdict

✅ Check passed. 0% of new/modified tests are implementation tests (threshold: 30%). The changes demonstrate high-quality behavioral testing with precise assertions and a regression guard.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24548231227

🧪 Test quality analysis by Test Quality Sentinel · ● 601.7K · ◷

[github-actions]

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of modified tests are implementation tests (threshold: 30%). The PR tightens a vague substring assertion into a precise GH_AW_REQUIRED_ROLES: "write" check and adds a NotContains regression guard, directly encoding the single-role behavioral contract.