Improve actionpins spec coverage for fallback, mapping, and context propagation

[Copilot]

pkg/actionpins/spec_test.go was missing coverage for several high-value resolution paths in pkg/actionpins/actionpins.go, especially around SHA passthrough, mapping failures, and fallback behavior. This change tightens the spec tests around those branches while simplifying a few low-signal cases.

• Resolution edge cases

  • add coverage for unknown full-SHA inputs returning the formatted passthrough form
  • add coverage for invalid PinContext.Mappings entries being ignored without altering resolution
  • add coverage for ResolveLatestActionPin falling back when ResolveActionPin errors, including the no-fallback empty case

• Resolver context propagation

  • add a resolver test that verifies PinContext.Ctx is forwarded unchanged, including context values and pre-cancelled state

• Test structure and readability

  • refactor ResolveActionPin enforce-pinned cases into a table-driven test
  • replace the manual completion channel in the concurrency test with sync.WaitGroup
  • upgrade selected assert.NotEmpty checks to require.NotEmpty where follow-on assertions depend on a non-empty result

• Signal-to-noise cleanup

  • remove struct-field-only tests for ActionPin, ActionYAMLInput, ContainerPin, and ResolutionFailure

Example of the newly covered passthrough behavior:

unknownFullSHA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

result, err := actionpins.ResolveActionPin("actions/checkout", unknownFullSHA, nil)
// result == "actions/checkout@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

[Copilot]

Pull request overview

This pull request strengthens pkg/actionpins spec coverage by adding targeted tests for previously untested resolution branches in ResolveActionPin/ResolveLatestActionPin, including full-SHA passthrough formatting, mapping validation behavior, fallback behavior on resolution errors, and propagation of caller-provided contexts into dynamic resolution.

Changes:

• Add spec tests for unknown full-SHA passthrough formatting, invalid mapping entries being ignored, and ResolveLatestActionPin embedded-fallback behavior when ResolveActionPin errors (including the no-fallback case).
• Add a resolver-context propagation test to verify PinContext.Ctx (including cancellation and values) is forwarded unchanged into SHAResolver.ResolveSHA.
• Refactor/clean up tests for clarity (table-driven enforce-pinned cases, WaitGroup in concurrency test, and use require.NotEmpty where subsequent assertions depend on it), and remove low-signal struct-field-only tests.

Show a summary per file

File	Description
pkg/actionpins/spec_test.go	Adds higher-value spec coverage for action pin resolution edge cases (SHA passthrough, mapping validation, fallback behavior, and context propagation) and refactors existing tests for readability.

Review details

Tip

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

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Low

[github-actions]

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

[github-actions]

⚠️ PR Code Quality Reviewer failed during code quality review.

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 88/100 — Excellent

Analyzed 2 modified test functions: 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)

Metric	Value
Analyzed	2 (Go: 2, JS: 0)
✅ Design	2 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	2 (100%)
Duplicate clusters	0
Inflation	NO (1.62:1, acceptable)
🚨 Violations	0

Test	File	Classification	Issues
TestSpec_PublicAPI_ResolveLatestActionPin_FallbackOnEnforceError	pkg/actionpins/spec_test.go:372	Design (refactored into 2 subtests)	None — expands edge case coverage
TestSpec_PublicAPI_ResolveActionPin_UsesProvidedContext	pkg/actionpins/spec_test.go:657	Design (enhanced)	None — clarifies context semantics

✅ Changes Breakdown

Constants Extracted (3 additions)

• testContextKey type for context key identity
• testContextPropagationKey constant — improves test clarity
• testResolvedSHA constant — reduces inline duplication

Tests Modified (2 functions)

1. ResolveLatestActionPin_FallbackOnEnforceError (line 372–397)

   • Expansion: Split into 2 subtests
   • Subtest 1 (line 373–389): Validates fallback to embedded pin when resolver fails — Design test ✅
     • Setup: Dynamic resolver with error, SkipHardcodedFallback=true
     • Assertions: Verifies correct fallback format (2 assertions with context)
     • Edge case: Resolver error handling
   • Subtest 2 (line 391–397): Original simple case, unknown repo returns empty — Design test ✅
     • Assertions: 1 empty check

2. UsesProvidedContext (line 657–677)

   • Enhancement: Clarifies context propagation semantics
   • Uses extracted constants (testResolvedSHA, testContextPropagationKey)
   • Added comment (line 661–663): Explains pre-cancellation for testing identity
   • Changed require.Equal → assert.Same (line 671) — semantically correct for identity checks
   • Assertions: 5 total, all with descriptive messages
   • Edge case: Canceled context with values intact (design-critical)

Test Quality Indicators

• ✅ Build tag present (//go:build !integration line 1)
• ✅ No forbidden mocking (testSHAResolver is a simple fake)
• ✅ All assertions have descriptive failure context
• ✅ 100% edge case coverage in modified tests
• ✅ Constants reduce duplication and improve maintainability

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). No violations detected.

Scoring basis:

• Design tests: 2/2 (100%) → +40 points
• Edge/error coverage: 2/2 (100%) → +30 points
• No duplicates → +20 points
• Test inflation 1.62:1 (acceptable) → +10 points
• No violations → No deductions

This PR improves test quality through strategic refactoring: expanding fallback error handling, clarifying context semantics with better naming, and reducing duplication through extracted constants. All changes are design-focused and add meaningful coverage to behavioral contracts.

Warning

Firewall blocked 1 domain

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

• awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🧪 Test quality analysis by Test Quality Sentinel · 18.8 AIC · ⌖ 15.6 AIC · ⊞ 6.8K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 88/100. 0% implementation tests (threshold: 30%). All changes are design-focused with excellent edge-case coverage and no violations.

[github-actions]

Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (pkg/ directory, 175 new lines) but does not have a linked Architecture Decision Record (ADR).

Draft ADR committed: docs/adr/43871-actionpins-spec-tests-verify-behavioral-contracts.md — review and complete it before merging.

This PR cannot merge until an ADR is linked in the PR body.

What to do next

1. Review the draft ADR committed to your branch — it was generated from the PR diff
2. Complete the missing sections — add context the AI could not infer, refine the decision rationale, and list real alternatives you considered
3. Commit the finalized ADR to docs/adr/ on your branch
4. Reference the ADR in this PR body by adding a line such as:

   ADR: ADR-43871: actionpins Spec Tests Verify Behavioral Contracts

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

Why ADRs Matter

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.

Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

• Context — What is the problem? What forces are at play?
• Decision — What did you decide? Why?
• Alternatives Considered — What else could have been done?
• Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 43871-actionpins-spec-tests-verify-behavioral-contracts.md for PR #43871).

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · 55.8 AIC · ⌖ 10.1 AIC · ⊞ 8.5K · ◷
_{Comment /review to run again}

[gh-aw-bot]

@copilot please run the pr-finisher skill, verify the branch is up to date, rerun any needed checks, and finish the PR for maintainer review.

Generated by 👨‍🍳 PR Sous Chef · 9.53 AIC · ⌖ 5.42 AIC · ⊞ 4.7K · ◷
_{Comment /souschef to run again}

[github-actions]

🤖 PR Triage — Run §28834001067

Field	Value
Category	test
Risk	🟢 Low
Priority Score	62 / 100
Recommended Action	auto_merge

Score Breakdown:

• Impact: 28 / 50 — test/spec coverage improvement in actionpins, 2 files only
• Urgency: 16 / 30 — fresh PR (< 8h), CI approved by bot
• Quality: 18 / 20 — APPROVED by github-actions[bot], descriptive body, ADR doc added

CI: github-actions[bot] APPROVED at 2026-07-07T00:32Z.

Rationale: Pure test + ADR doc — no production code changes. Low risk. Recommend auto-merge after human spot-check.

Labels applied: pr-type:test · pr-risk:low · pr-priority:medium · pr-action:auto_merge · pr-agent:copilot

Generated by 🔧 PR Triage Agent · 96.3 AIC · ⌖ 8.74 AIC · ⊞ 5.5K · ◷

[Copilot]

@copilot please run the pr-finisher skill, verify the branch is up to date, rerun any needed checks, and finish the PR for maintainer review.

...

PR is ready for maintainer review. Here's the status summary:

• ✅ Reviews — No unresolved review threads. github-actions[bot] approved (Test Quality Sentinel: 88/100). copilot-pull-request-reviewer commented with no blocking issues.
• ✅ Checks (local) — make fmt, make lint (0 errors, pre-existing warnings only), make test-unit (all pass), and make agent-report-progress all pass cleanly.
• ❓ Checks (CI) — Stale after agent push; needs human re-trigger. Prior failures on 70dd8bd5 (PR Code Quality Reviewer, Matt Pocock Skills Reviewer, Impeccable Skills Reviewer) were infra-level (blocked network requests within the reviewer agents, no code feedback emitted). Latest HEAD (8d381f10) shows action_required on CWI/CGO/Smoke CI — standard GitHub approval gate for bot-pushed commits, not a code failure.
• ✅ Mergeable — PR is open, not draft, not behind base (main SHA matches the branch's base).

Actions taken: Verified branch state, confirmed no unresolved threads, validated all local make targets pass, inspected CI failures at log level (infra flakes, no actionable code changes needed).

Hand-off: A maintainer needs to approve and re-trigger the CWI/CGO/Smoke CI workflow runs (close/reopen or trigger via workflow_dispatch) before merging.

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.82.3