fix(#779): recover from stale repo names in token requests

[fullsend-ai-coder]

Summary

When config.yaml contains repos that were deleted, transferred, or renamed from the org, GitHub's POST /app/installations/{id}/access_tokens rejects the entire batch with a 422 — blocking reconciliation for ALL repos, not just the stale ones. This caused a real incident affecting ~129 repos.

This PR adds retry-with-recovery to CreateInstallationToken: on a 422 with a repositories validation error, the mint parses the invalid repo names from GitHub's error response, filters them out, and retries with only the valid repos. Invalid repos are surfaced in the response as invalid_repos so callers can detect and report stale config entries.

Changes

• internal/mintcore/github.go: Refactored CreateInstallationToken into a two-layer design: requestInstallationToken handles a single API call, while CreateInstallationToken orchestrates retry logic. On 422, parses GitHub's validation error to identify invalid repos, filters them out, and retries. Added InvalidRepos field to GrantedScope, githubValidationError types for parsing, and parseInvalidRepos helper.
• internal/mintcore/handler.go: Added invalid_repos field to mintResponse and warning logging when repos are dropped.
• internal/mintcore/github_test.go: Added tests for retry with invalid repos, all-repos-invalid error, non-repo 422 errors, 422 without repos, and parseInvalidRepos parsing.
• internal/mintcore/handler_test.go: Added end-to-end handler test for the full invalid-repo recovery flow.
• Embedded GCF copies: Synced github.go.embed and handler.go.embed.

Testing

• All existing mintcore tests pass (95 tests, race detection enabled)
• cmd/mint tests pass
• go vet ./... passes
• Embed sync test (TestEmbeddedMintSource_MatchesOriginal) passes
• New unit tests cover: mixed valid/invalid repos → retry succeeds, all repos invalid → clear error, non-repo 422 → error with diagnostics, 422 without repos → no retry, parseInvalidRepos edge cases
• New handler integration test covers the full flow through ServeHTTP

---

Closes #779

Post-script verification

• Branch is not main/master (agent/779-mint-stale-repo-recovery)
• Secret scan passed (gitleaks — ba77dbdbbd17e9bb18c47bf6efa023c98b697158..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 3:02 AM UTC · Completed 3:13 AM UTC
Commit: 2ff60ad · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [defense-in-depth] internal/mintcore/github.go:542 — parseInvalidRepos validates extracted repo names against RepoNamePattern but does not apply the strings.Contains(repo, "..") check that the handler applies to user-supplied repo names (handler.go:208). RepoNamePattern accepts names like ..traversal. While the practical impact is near-zero — these names come from GitHub's API response and are only used for set-difference filtering against already-validated repos — adding the .. check aligns with the handler's defense-in-depth approach.

• [architectural-conflict] internal/mintcore/github.go:476 — The retry logic parses GitHub's 422 error response using structured JSON field matching (e.Field == "repositories"). If GitHub changes the error format, parseInvalidRepos returns nil and the 422 is treated as a non-retryable error with the full response body included in the error message for diagnostics. This is a safe degradation mode, but there is no dedicated log line when parsing fails on a 422-with-repos — consider adding one for observability.

• [coherence-with-issue] internal/mintcore/handler.go:665 — invalid_repos is exposed in the mint API response, providing callers with visibility into which repos were dropped. Issue mint: undefined recovery behavior when a config.yaml repo no longer exists in the org #779 asks whether config.yaml should be pruned of stale entries, but this PR addresses only mint-side recovery. Consider filing a follow-up issue for client-side cleanup using this response field.

Previous run

Review

Findings

Medium

• [error-handling] internal/mintcore/github.go:474 — The Value field in githubValidationErrorItem is typed as []string, but GitHub's validation error format can return value as various types depending on the error context. If GitHub returns "value": "some-repo" (a plain string) instead of "value": ["some-repo"], json.Unmarshal in parseInvalidRepos will return an error and the function will return nil, causing the 422 to be treated as a generic non-repo error rather than triggering recovery. The failure mode is safe (returns error, no data loss), but the recovery feature would be silently inoperative for that error shape.
  Remediation: Consider using json.RawMessage for the Value field and handling both []string and string forms in parseInvalidRepos.

Low

• [unsanitized-external-input] internal/mintcore/github.go:517 — parseInvalidRepos extracts repo name strings from GitHub's 422 error body without validating them against RepoNamePattern. These flow into log.Printf calls and the HTTP response JSON. The actual risk is low since input repos are pre-validated at the handler level (handler.go:211) and GitHub's response would echo the same names back, but as a defense-in-depth measure, validating the error-recovery path would close the gap.

• [architectural-inconsistency] internal/mintcore/github.go — The mint generally uses fail-closed semantics for validation (role, provenance, org). This PR introduces recovery semantics for repo validation specifically — dropping invalid repos and succeeding with a narrower token scope. The token scope is strictly narrowed (never widened) and the all-invalid case correctly errors out, so this is not a security concern. The asymmetry in validation behavior is worth documenting in a code comment.

• [incomplete-observability] internal/mintcore/handler.go:264 — Invalid repos are logged server-side (WARNING level) and returned in the invalid_repos API response field — a strict improvement over the pre-PR behavior (complete failure). Consider coordinating with reconcile-repos.sh / repo-maintenance.yml to surface this field in workflow outputs so operators are notified of config drift.

---

Labels: PR is a bug fix (fix prefix, linked to bug issue #779).

Previous run (2)

Review

Findings

Medium

• [intent-authorization] — Issue mint: undefined recovery behavior when a config.yaml repo no longer exists in the org #779 presents multiple options without prescribing a single solution (client-side validation, mint-side drop-and-retry, config pruning, fail-loudly-per-repo). The PR implements the mint-side drop-and-retry approach without documented design rationale for why this option was chosen over alternatives.
  Remediation: Add a brief rationale to the PR description explaining why mint-side drop-and-retry was chosen — e.g., the chicken-and-egg problem with client-side validation (no token to check repo existence), and interaction with fix(harness): exit gracefully when all repos are disabled fullsend-ai/fullsend#1833's unenrollment requirement.

Low

• [edge-case] internal/mintcore/github.go:506 — parseInvalidRepos assumes GitHub's 422 error item Value field is always []string. If GitHub changes the shape, json.Unmarshal would silently leave Value as nil, causing the retry feature to silently degrade to the generic 422 error path without observable signal. Consider logging when a field:"repositories" error entry is found but Value is empty.

• [edge-case] internal/mintcore/github.go:500 — If GitHub returns invalid repo names in the Value field that are not present in the original repos slice (e.g., due to name normalization), the case-insensitive filtering would be a no-op and the retry would fail with the same 422. Unlikely given GitHub echoes back the submitted names, but the error message in that case ("retry after dropping invalid repos") would be confusing.

• [scope-creep] internal/mintcore/github.go — The design rationale for choosing silent drop-and-continue over alternatives mentioned in mint: undefined recovery behavior when a config.yaml repo no longer exists in the org #779 (fail loudly per-repo, config pruning) could be better documented. The invalid_repos field in the response provides caller visibility, but no client-side guidance exists for acting on it.

• [architectural-coherence] internal/mintcore/github.go — The retry logic intentionally creates an asymmetry: zero repos in the request yields an installation-wide token, while all-invalid repos yields a hard error. This is a correct security-conscious design (preventing unintended privilege escalation to installation-wide scope), but the distinction is worth documenting in the function's doc comment.

• [architectural-coherence] internal/mintcore/github.go — The two-layer design (requestInstallationToken as single-call primitive + CreateInstallationToken as retry orchestrator) is well-structured. The CreateInstallationToken doc comment already describes the retry behavior — minor naming observation only.

[guyoron1]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:46 AM UTC · Completed 7:00 AM UTC
Commit: 2ff60ad · View workflow run →

[guyoron1]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 7:29 AM UTC · Completed 7:37 AM UTC
Commit: 2ff60ad · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 3 of 4 review findings: hardened Value field parsing to handle both []string and string forms via json.RawMessage, added RepoNamePattern validation as defense-in-depth, and documented the intentional asymmetry in recovery vs fail-closed validation. Disagreed with 1 finding (coordinating with operational tooling is out of scope for this PR). All changes synced to embedded GCF copy.

Fixed (3):

1. Value field typed as []string but GitHub may return plain string (internal/mintcore/github.go): Changed githubValidationErrorItem.Value from []string to json.RawMessage. Updated parseInvalidRepos to try unmarshalling as []string first, then fall back to a single string. Added test case covering the plain-string value form.
2. parseInvalidRepos does not validate repo names against RepoNamePattern (internal/mintcore/github.go): Added RepoNamePattern validation for each repo name extracted from the GitHub error response. Names that don't match a valid repo pattern are silently dropped. Added test case verifying that invalid patterns like '../traversal' and empty strings are filtered out.
3. Recovery semantics asymmetry not documented (internal/mintcore/github.go): Added design-rationale comment to CreateInstallationToken explaining why recovery semantics (drop invalid repos, succeed with narrower scope) are intentionally asymmetric with fail-closed validation used elsewhere in the mint.

Disagreed (1):

1. Coordinate with reconcile-repos.sh / repo-maintenance.yml to surface invalid_repos: This is an operational tooling concern outside the scope of this PR. The PR already surfaces invalid repos in both server-side logs (WARNING level) and the API response's invalid_repos field. Coordinating with external scripts belongs in a follow-up issue.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:38 AM UTC · Completed 7:51 AM UTC
Commit: 895df3e · View workflow run →

[fullsend-ai-review]

Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

• internal/mintcore/github.go:542: [low] defense-in-depth

parseInvalidRepos validates extracted repo names against RepoNamePattern but does not apply the strings.Contains(repo, "..") check that the handler applies to user-supplied repo names (handler.go:208). RepoNamePattern accepts names like '..traversal'. While the practical impact is near-zero (names from GitHub's API, only used for set-difference filtering), adding the check aligns with the handler's defense-in-depth approach.

Suggested fix: Add strings.Contains(n, "..") rejection in parseInvalidRepos alongside the RepoNamePattern check.

• internal/mintcore/github.go:476: [low] architectural-conflict

The retry logic parses GitHub's 422 error response using structured JSON field matching (e.Field == 'repositories'). If GitHub changes the error format, parseInvalidRepos returns nil and the 422 is treated as a non-retryable error with the response body in the error message. Consider adding a log line when parseInvalidRepos returns nil on a 422-with-repos for observability.

Suggested fix: Add log.Printf when parseInvalidRepos returns empty results on a 422 response with repos.

• internal/mintcore/handler.go (file-level): Line 665 · [low] coherence-with-issue

invalid_repos is exposed in the mint API response but no client-side follow-up is planned to act on it for config.yaml cleanup. Issue #779 asks about config pruning, but this PR addresses only mint-side recovery.

Suggested fix: File a follow-up issue for client-side cleanup of stale repos using the invalid_repos response field.

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the stale label to reset the inactivity timer.