feat(repos): preserve SHA pinning during repos upgrade

[fullsend-ai-coder]

Summary

• Preserve SHA pinning in repos upgrade: when a workflow's current ref is a SHA, resolve the target version tag to its commit SHA via forge.Client.GetRef() and write @<sha> # <tag> format
• Tag-only repos continue to use tag-only format (@<tag>)
• Graceful error handling when tag resolution fails for SHA-pinned repos

Related Issue

Closes #5272

Changes

File	Change
internal/repos/upgrade.go	Add isSHARef() detection, resolve tags to SHAs via GetRef, preserve SHA-pinning convention
internal/repos/upgrade_test.go	Add tests for SHA-pinned upgrade, tag-only preservation, resolution errors, mixed pinning styles, isSHARef unit tests; update TestUpgrade_NonSemverCurrentRef for new behavior
docs/plans/repos-management.md	Update known limitation sections to reflect the fix

Testing

• go test ./internal/repos/ — all tests pass
• go vet ./... — clean
• scan-secrets --staged — no leaks
• New tests cover: SHA-pinned repo upgrade, tag-only repo stays tag-only, tag resolution failure, mixed pinning styles across repos, isSHARef edge cases

Checklist

• Commit message follows COMMITS.md conventions
• No secrets committed
• Tests added for new behavior
• Documentation updated

---

Closes #5272

Post-script verification

• Branch is not main/master (agent/5272-sha-pinning-preservation)
• Secret scan passed (gitleaks — b84696fd80a59eb24190aad8fdf6d42f6d8f39bc..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:49 PM UTC · Completed 11:02 PM UTC
Commit: ca8af5f · View workflow run →

[github-actions]

Site preview

Preview: https://388fdbef-site.fullsend-ai.workers.dev

Commit: cda2f9d953c32b22111b16b5a3bcb8360e36f07b

[codecov]

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/repos/upgrade.go	84.61%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Low

• [edge-case] internal/repos/upgrade.go:56 — isSHARef can produce false positives for tag names that happen to be 7–40 hex characters (e.g., deadbeef). The code comment acknowledges this limitation. In practice this is extremely unlikely since version tags follow the vX.Y.Z convention.

• [logic-error] internal/repos/upgrade.go:205 — In dry-run mode with a SHA-pinned current ref, replaceShimRef is called with the tag (targetRef) rather than a resolved SHA, so the changed check will always be true (SHA ≠ tag) and the no uses: lines matched skip is unreachable for SHA-pinned dry-runs. This is a deliberate design tradeoff — dry-run avoids API calls that could fail — and the message (SHA will be resolved at upgrade time) communicates the deferred resolution to the user.

---

Labels: PR modifies Go code in the repos upgrade package.

Previous run

Review

Findings

Low

• [edge-case] internal/repos/upgrade.go:56 — isSHARef can produce false positives for tag names that happen to be 7–40 hex characters (e.g., deadbeef). The code comment acknowledges this limitation. In practice this is extremely unlikely since version tags follow vX.Y.Z convention.

• [logic-error] internal/repos/upgrade.go:205 — In dry-run mode with a SHA-pinned current ref, the code does not call GetRef to resolve the target tag to a SHA. The dry-run path uses replaceShimRef(content, targetRef, "") to check replaceability using tag-only format and does not preview the SHA-pinned output format. This means dry-run output says "Would upgrade <sha> → <tag>" but the actual upgrade would produce @<resolved-sha> # <tag>.
  Remediation: Add a note in the dry-run message that SHA resolution is skipped, or resolve the SHA in dry-run too (at the cost of an API call).

• [code-organization] internal/repos/upgrade.go:39 — The new shimOwner and shimRepo constants extract the repo identity for the GetRef call, but the existing shimRefPattern regex still hardcodes fullsend-ai/fullsend inline. This creates a split where the same value is expressed two different ways.
  Remediation: Consider having shimRefPattern reference the constants for consistency, or inline the values at the GetRef call site to match the existing pattern.

Previous run (2)

Review

Findings

Low

• [edge-case] internal/repos/upgrade.go:56 — isSHARef can produce false positives for tag names that happen to be 7–40 hex characters (e.g., deadbeef). The code comment acknowledges this limitation. In practice this is extremely unlikely since version tags follow vX.Y.Z convention.

• [edge-case] internal/repos/upgrade.go:209 — For SHA-pinned repos, the GetRef API call happens before the DryRun check. In DryRun mode, if the target tag does not exist on the remote, the upgrade will fail with an error instead of reporting what would happen. This differs from the tag-only path where DryRun requires no API call beyond reading the workflow file.
  Remediation: Move the DryRun check before the isSHARef branch, or skip the GetRef call when DryRun is true and report the intended upgrade without resolving the SHA.

Previous run (3)

Review

Findings

Medium

• [stale-limitation] docs/cli/repos.md:352 — The Limitations section for repos upgrade still documents that SHA-pinned repos lose their pin during upgrade, but this PR implements SHA-pinning preservation. The docs/cli/repos.md file is not included in this PR's changed files.
  Remediation: Update or remove the "Tag-only pinning" limitation in docs/cli/repos.md. Replace the text to describe the new SHA-preservation behavior, or remove the limitation entirely.

Low

• [incomplete-scope-documentation] docs/plans/repos-management.md:235 — The updated plan documentation describes SHA pinning preservation but does not mention the failure mode: if GetRef fails (tag doesn't exist, API error), the upgrade fails for that repo entirely rather than falling back to tag-only. This is a change in error behavior from the prior code.
  Remediation: Add a note: "If tag-to-SHA resolution fails, the upgrade fails for that repo rather than falling back to tag-only format."

• [edge-case] internal/repos/upgrade.go:50 — isSHARef can produce false positives for tag names that happen to be 7–40 lowercase hex characters (e.g., deadbeef). If such a tag were used as a ref, the code would call GetRef to resolve the target tag to a SHA and write SHA-pinned format, changing the repo from tag-only to SHA-pinned unintentionally. In practice this is extremely unlikely since version tags follow vX.Y.Z convention.

• [naming-consistency] internal/repos/upgrade.go:46 — shaRefPattern only matches lowercase hex characters ([0-9a-f]), so an uppercase SHA ref in a workflow file would not be recognized as SHA-pinned. The code would fall through to the tag-only path, losing the SHA pin. GitHub normalizes SHAs to lowercase in API responses, so this is near-zero probability in practice.

[ggallen]

/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues.

[fullsend-ai-coder]

🤖 Finished Fix · ❌ Failure · Started 12:33 AM UTC · Completed 12:39 AM UTC
Commit: cc7a526 · View workflow run →

[ggallen]

/fs-fix rebase and fix all the issues, both in-line and in the sticky comment.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 4:20 PM UTC · Completed 4:27 PM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 4 review findings: updated stale docs/cli/repos.md limitation, added failure mode documentation to both plan doc locations, documented hex-only tag name edge case, and made SHA detection case-insensitive. Also rebased onto latest main. All tests pass.

Fixed (4):

1. stale-limitation: docs/cli/repos.md still documents SHA-pinned repos losing their pin (docs/cli/repos.md): Replaced stale 'Tag-only pinning' limitation with updated 'SHA pinning' section describing the new SHA-preservation behavior and its failure mode
2. incomplete-scope-documentation: docs/plans/repos-management.md missing failure mode for GetRef (docs/plans/repos-management.md): Added failure mode note to both plan doc locations (upgrade overview and implementation details): GetRef failure causes per-repo error rather than tag-only fallback
3. edge-case: isSHARef false positives for hex-only tag names (internal/repos/upgrade.go): Added code comment documenting the false-positive edge case for tag names that are 7-40 hex characters, noting this is extremely unlikely since version tags follow vX.Y.Z convention
4. naming-consistency: shaRefPattern only matches lowercase hex characters (internal/repos/upgrade.go): Made shaRefPattern case-insensitive with (?i) flag to handle uppercase SHA refs in workflow files. Updated test expectation for uppercase hex input accordingly

Tests: passed

_{Updated by fullsend fix agent}

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:59 PM UTC · Completed 5:17 PM UTC
Commit: 7ada4e0 · View workflow run →

[ggallen]

/fs-fix rebase and fix all the issues, both in-line and in the sticky comment. And don't forget to respond to and resolve in-line comments.

[fullsend-ai-coder]

🤖 Finished Fix · ❌ Failure · Started 5:20 PM UTC · Completed 5:26 PM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-coder]

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29852387088

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

[ggallen]

/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues. CRITICAL: DO NOT TRY TO SIGN THE COMMIT!

[fullsend-ai-coder]

🤖 Finished Fix · ❌ Failure · Started 5:39 PM UTC · Completed 5:46 PM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-coder]

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29853800295

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

[ggallen]

All review findings addressed:

• DryRun ordering (low, edge-case): Moved DryRun check before isSHARef branch so DryRun never calls GetRef. Added TestUpgrade_DryRunSHAPinnedSkipsGetRef test.
• isSHARef false positives (low, edge-case): Code comment documents the limitation — extremely unlikely since version tags follow vX.Y.Z.
• Case-insensitive SHA matching (low, naming-consistency): (?i) flag added to shaRefPattern.
• Failure mode documentation (low, incomplete-scope-documentation): Already documented in both docs/plans/repos-management.md and docs/cli/repos.md.

All tests pass. Coverage at 94.1% for the package, 95.5% for upgradeRepo.

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:47 PM UTC · Completed 8:01 PM UTC
Commit: 7ada4e0 · View workflow run →

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:16 PM UTC · Completed 8:30 PM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 8:53 PM UTC · Completed 9:23 PM UTC
Commit: cda2f9d · View workflow run →

[fullsend-ai-retro]

Retro: PR #5276 — feat(repos): preserve SHA pinning during repos upgrade

Timeline

Time (UTC)	Event	Outcome
Jul 17 22:33	Code agent creates PR from issue #5272	✅ Success
Jul 17 22:48–23:02	Auto-review finds 4 findings (1 medium, 3 low)	Valid, well-calibrated
Jul 18 00:33	Human triggers /fs-fix #1 — rebase + address review	❌ Post-fix Signed-off-by false positive
Jul 21 16:19	Human triggers /fs-fix #2 — rebase + fix all	✅ All 4 findings addressed
Jul 21 16:58–17:17	Manual /fs-review — finds 2 new low findings	APPROVED
Jul 21 17:18	Human triggers /fs-fix #3	❌ Post-fix Signed-off-by false positive
Jul 21 17:38	Human triggers /fs-fix #4 ("CRITICAL: DO NOT SIGN")	❌ Same false positive
Jul 21 19:35–19:37	Human manually pushes fixes + approves	Addresses DryRun msg + shimRefPattern
Jul 21 20:01	Manual /fs-review	APPROVED (1 new low finding)
Jul 21 20:04	Human addresses final finding	shimRefPattern refactored
Jul 21 20:30	Final /fs-review	✅ Clean approval
Jul 21 20:45	PR merged

What went well

• Review quality was high on the initial pass. The review agent found a genuine medium-severity stale-docs issue (the Limitations section still claimed SHA-pinned repos lose their pin — the exact behavior this PR was fixing) plus 3 valid low-severity findings. All were actionable.
• Fix agent addressed all 4 findings correctly on its successful run (attempt Add problem areas: Tekton pipeline review, migration path, multi-tenancy #2), including updating docs, adding code comments, and making the regex case-insensitive.
• Review agent tracked code-level resolutions well. Three of four initial findings were properly dropped in subsequent reviews once the code changed. A new valid finding (shimRefPattern constants) was identified in Review 3.

Key problems

1. Post-fix scan range bug caused 3 false-positive failures (evidence for #5419)

The post-fix.sh Signed-off-by check uses PRE_AGENT_HEAD..HEAD as its scan range. After a rebase, this range includes upstream human commits that legitimately carry Signed-off-by trailers. The agent's commits were clean in all 3 failures — the error message "agent commit contains a Signed-off-by trailer" was misleading. This caused the human to issue an instruction ("DO NOT TRY TO SIGN THE COMMIT!") addressing a problem that didn't exist on the agent side. The same root cause was independently identified on PR #5273 and filed as #5419. This PR provides additional evidence: 3 failed fix runs, 2 wasted human /fs-fix commands, and the human falling back to manual code changes.

2. Review agent re-raised acknowledged findings across iterations (evidence for #5265)

The isSHARef edge-case finding was raised in all 4 review iterations despite the fix agent adding a documenting code comment after Review 1 and the human replying "acknowledged, no code change needed." The review agent noted the code comment existed but still posted the finding as a new inline comment each time. This is the exact pattern described in #5265 — the review agent lacks author-response context to suppress explained-away findings.

3. Dispatch shim waste: 23 of 25 shim runs dispatched nothing (evidence for #2994)

The PR triggered 25 fullsend.yaml shim runs, but only 2 dispatched actual agent work (initial review + retro). The remaining 23 ran the routing logic and exited with "No stage matched." Key waste sources:

• 4 runs from the review bot's own review submissions (self-triggering, covered by Filter bot-submitted pull_request_review events from review dispatch #2994)
• 10 runs from human inline review comments (each comment fires a separate pull_request_review event, none match the routing gate)
• 4 runs from fix agent synchronize pushes (bot author fails is_event_actor_authorized)
• 3 runs from unrecognized label events
• 2 cancelled by concurrency controls

Existing issue coverage

All improvement opportunities identified in this retro are already covered by open issues:

• Post-script commit-range checks should be audited for rebase safety #5419 — Post-script commit-range rebase safety audit (exact root cause of fix failures)
• Review re-review anchoring should incorporate author response context to suppress explained-away findings #5265 — Re-review anchoring with author response context (review finding dedup)
• Filter bot-submitted pull_request_review events from review dispatch #2994 — Filter bot-submitted pull_request_review events (dispatch waste)
• Deduplicate review agent runs on the same commit SHA to reduce token waste #4681 / Prioritize same-commit review dedup — PR #944 shows 10x waste on a single SHA #5139 — Same-commit review dedup (Reviews 2 and 3 reviewed identical code)
• Skip re-review when PR diff is unchanged after rebase/reopen #1356 — Skip re-review when diff unchanged after rebase

No new proposals filed — existing issues comprehensively cover the findings. This retro's evidence is preserved here for reference.