Fix lint-go and test failures in actionpins and cli specs

[Copilot]

• Investigate failing CI run 24730058083 and identify root causes in lint-go and test jobs
• Reproduce failures locally with make agent-finish and targeted go test to confirm breakage
• Update pkg/actionpins/spec_test.go to use the current public API (GetActionPinsByRepo) instead of removed GetActionPins
• Fix related lint findings in updated tests (intrange, testifylint expected/actual ordering)
• Run targeted tests for modified packages (go test ./pkg/actionpins, go test -run TestSpec_Types_ShellType ./pkg/cli)
• Run make agent-finish after changes (blocked only by pre-existing repo-level gosec findings unrelated to this change)
• Run parallel validation and review findings
• Commit/push changes and ensure PR is created/updated

[Copilot]

Pull request overview

Fixes CI failures in Go linting and tests by updating spec tests to match current public APIs and lint expectations.

Changes:

• Update pkg/actionpins/spec_test.go to use GetActionPinsByRepo (and adjust the concurrency/thread-safety spec test accordingly).
• Adjust pkg/cli/spec_test.go assertions to satisfy testifylint’s expected/actual ordering and remove trailing whitespace.

Show a summary per file

File	Description
pkg/cli/spec_test.go	Reorders assert.Equal arguments to match linter conventions; minor whitespace cleanup.
pkg/actionpins/spec_test.go	Migrates removed API usage to GetActionPinsByRepo and updates the concurrent access test accordingly.

Copilot's findings

Tip

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

• Files reviewed: 2/2 changed files
• Comments generated: 0

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 85/100

✅ Excellent test quality

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

---

Test Classification Details

View Test Classifications (2 tests)

Test	File	Classification	Issues Detected
TestSpec_ThreadSafety_ConcurrentGetActionPinsByRepo	pkg/actionpins/spec_test.go:261	✅ Design	None — well-structured concurrent safety test with descriptive assertions
TestSpec_Types_ShellType	pkg/cli/spec_test.go:854	✅ Design	None — correctly fixed expected/actual argument order in testify calls

---

Change Summary

This PR is a targeted fix to two tests that were failing lint/test checks:

✅ TestSpec_ThreadSafety_ConcurrentGetActionPinsByRepo (pkg/actionpins/spec_test.go)

What changed: Updated from removed GetActionPins() API → GetActionPinsByRepo(repo). Added assert.NotEmpty on results[0] (previously only non-zero indices were checked), switched to range-over-int loop style.
Classification: Design test — verifies the sync.Once initialization guarantee: concurrent callers must always see a consistent, non-empty result from the same known repo. Covers a real concurrency edge case.
Assertion messages: All assertions include descriptive messages ✅
Build tag: //go:build !integration on line 1 ✅

✅ TestSpec_Types_ShellType (pkg/cli/spec_test.go)

What changed: Fixed testify assert.Equal argument order from (t, actual, expected, msg) → (t, expected, actual, msg). This matters for failure message readability (want X, got Y vs the reverse).
Classification: Design test — verifies the documented string values of ShellType constants. These string values are part of the behavioral contract (shell detection and dispatch logic depends on them matching exactly).
Assertion messages: All 5 assertions have descriptive messages ✅
Build tag: //go:build !integration on line 1 ✅

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). Both modified tests are behavioral design tests with correct assertions. No coding-guideline violations detected.

---

📖 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.

---

Language Support

Tests analyzed:

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

References: §24731692742

🧪 Test quality analysis by Test Quality Sentinel · ● 2.2M · ◷

[github-actions]

✅ Test Quality Sentinel: 85/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). Both modified tests enforce behavioral contracts with correct assertions and no coding-guideline violations.