Skip to content

fix(#6196): authenticate lookupAppID with env token when available - #6199

Merged
ralphbean merged 1 commit into
mainfrom
agent/6196-mint-lookup-auth-token
Aug 13, 2026
Merged

fix(#6196): authenticate lookupAppID with env token when available#6199
ralphbean merged 1 commit into
mainfrom
agent/6196-mint-lookup-auth-token

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

lookupAppID in internal/cli/mint.go always made unauthenticated GET /apps/{slug} requests, sharing GitHub's 60-requests/hour IP-based rate limit with all other unauthenticated calls from the same runner IP. In CI, where Actions runners share IP ranges, this limit is routinely exhausted across concurrent jobs, causing intermittent failures in the behaviour test suite's mint deploy --pem-dir bootstrap step.

This PR makes lookupAppID check for GH_TOKEN then GITHUB_TOKEN in the environment (matching the resolution order used by resolveToken() and other CLI commands). When a token is present, the request is authenticated at 5,000 requests/hour. When no token is set, the request remains unauthenticated, preserving the local/first-time bootstrap experience.

Changes

  • Modified lookupAppID to read GH_TOKEN/GITHUB_TOKEN from the environment and set an Authorization: Bearer header when available
  • Updated the rate-limit error message: suggests setting a token when unauthenticated, omits that guidance when already authenticated
  • Updated the doc comment to describe the new behavior
  • Added tests for: GH_TOKEN authentication, GITHUB_TOKEN fallback, GH_TOKEN precedence over GITHUB_TOKEN, authenticated rate-limit error message

Testing

  • All new and existing TestLookupAppID_* tests pass
  • go vet passes
  • Patch coverage at 86.7% (above 80% threshold)
  • Pre-existing failures in TestStartFetchService_* and TestPostScriptEnv_* confirmed on main — not caused by this change

Closes #6196

Post-script verification

  • Branch is not main/master (agent/6196-mint-lookup-auth-token)
  • Secret scan passed (gitleaks — 7cf869c963af69d3edd39dff53600155cb2e5244..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

lookupAppID always made unauthenticated GET /apps/{slug} requests,
subject to GitHub's shared 60-requests/hour IP-based rate limit.
On GitHub Actions runners that share IP ranges, this limit is
routinely exhausted across concurrent CI jobs, causing the
behaviour test suite's mint deploy PEM bootstrap to fail
intermittently.

Check GH_TOKEN then GITHUB_TOKEN from the environment (matching the
resolution order used elsewhere in the CLI). When a token is
present, attach it as a Bearer header, lifting the limit to
5,000/hour. When no token is set, the request remains
unauthenticated, preserving the local/first-time bootstrap path.

The rate-limit error message is updated to suggest setting a token
when the request was unauthenticated, and to omit that guidance
when a token was already present.

Closes #6196
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 13, 2026 18:05
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 13, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:06 PM UTC · Completed 6:19 PM UTC

Commit: 8e148ff · View workflow run →

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [edge-case] internal/cli/mint.go:191 — When a token is present, HTTP 403 is assumed to mean rate limiting, but GitHub also returns 403 for abuse-detection triggers, IP blocks, or insufficient token scopes. The error message says "rate limit exceeded" which could mislead if the real cause is different. The pre-existing code had the same ambiguity for the unauthenticated path; this PR actually improves the situation by differentiating error messages between authenticated and unauthenticated cases.
    Remediation: Consider inspecting the X-RateLimit-Remaining response header or the response body to distinguish rate-limit 403 from other 403 causes, or soften the error text to "request forbidden (possible rate limit)".

  • [test-adequacy] internal/cli/mint_test.go:2305TestLookupAppID_EscapesSlug, TestLookupAppID_NotFound, and TestLookupAppID_ServerError do not clear GH_TOKEN/GITHUB_TOKEN via t.Setenv, unlike the other lookupAppID tests updated in this PR. If these tests run in a CI environment where GH_TOKEN is set, the requests will carry an Authorization header, which is inconsistent with the test's implicit expectation of unauthenticated behavior. This doesn't cause test failures today since those code paths don't branch on token presence.
    Remediation: Add t.Setenv("GH_TOKEN", "") and t.Setenv("GITHUB_TOKEN", "") to these three tests for consistency.

  • [code-reuse] internal/cli/mint.go:171 — The token-resolution pattern (check GH_TOKEN, then GITHUB_TOKEN) is inlined rather than shared with resolveToken() in admin.go. However, resolveToken() has different semantics: it requires a token (returns error on empty) and includes a gh auth token fallback, neither of which is wanted here. With only two true duplicates of the exact two-env-var pattern, extracting a shared helper would be premature.


Labels: PR modifies internal/cli/mint.go — the mint CLI component

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • internal/cli/mint.go:191: [low] edge-case

When a token is present, HTTP 403 is assumed to mean rate limiting, but GitHub also returns 403 for abuse-detection triggers, IP blocks, or insufficient token scopes. The pre-existing code had the same ambiguity; this PR improves the situation by differentiating error messages.

Suggested fix: Consider inspecting the X-RateLimit-Remaining response header to distinguish rate-limit 403 from other 403 causes.

  • internal/cli/mint_test.go (file-level): Line 2305 · [low] test-adequacy

TestLookupAppID_EscapesSlug, TestLookupAppID_NotFound, and TestLookupAppID_ServerError do not clear GH_TOKEN/GITHUB_TOKEN via t.Setenv, unlike the other lookupAppID tests updated in this PR.

Suggested fix: Add t.Setenv("GH_TOKEN", "") and t.Setenv("GITHUB_TOKEN", "") to these three tests for consistency.

  • internal/cli/mint.go:171: [low] code-reuse

The token-resolution pattern is inlined rather than shared with resolveToken() in admin.go, but resolveToken() has different semantics (requires a token, includes gh-auth fallback). With only two true duplicates, a shared helper would be premature.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/mint Token mint and cross-boundary credentials labels Aug 13, 2026
@ralphbean
ralphbean added this pull request to the merge queue Aug 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 13, 2026
@ralphbean
ralphbean added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit fcd4702 Aug 13, 2026
30 checks passed
@ralphbean
ralphbean deleted the agent/6196-mint-lookup-auth-token branch August 13, 2026 22:17
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 13, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 10:19 PM UTC · Completed 10:35 PM UTC

Commit: 8e148ff · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6199 — authenticate lookupAppID with env token

Workflow quality: Excellent. This was a clean end-to-end agent workflow with zero rework cycles. Issue #6196 (flaky behaviour tests hitting GitHub's unauthenticated 60/hr rate limit) was filed at 17:46 UTC. The triage agent classified it in 4 minutes, the code agent produced a correct single-commit fix with 86.7% patch coverage in 15 minutes, the review agent approved with 3 low-severity findings in 14 minutes, and the human reviewer (ralphbean) approved without comments ~3 hours later. The PR merged at 22:17 UTC.

One issue delayed merge by ~48 minutes: the first merge queue attempt was ejected when the Fork PR kill switch blocks all harnesses behaviour test failed — not from a code defect, but because the after-scenario cleanup hook's call to fullsend inference status hit a transient GCP IAM Credentials 503 ("upstream connect error or disconnect/reset before headers"). ralphbean manually re-queued and it merged cleanly on the second attempt.

Existing issues with new supporting evidence

  • #6039 (review inline comment 422s): The review agent's 3 inline findings all received GitHub 422 errors and had to be posted in the review body instead. This PR is another data point for that issue.
  • #1557 / #5817 (duplicate dispatch): Two review dispatches fired within 1 second (one from opened, one from labeled: ready-for-review). The concurrency group correctly cancelled the duplicate — the mechanism works, but the wasted runner allocation adds to the evidence in Review-fix race condition causes duplicate review cycles and wasted tokens #1557.
  • #5774 (after-scenario hook flakiness): This PR hit a different after-scenario hook failure mode (GCP IAM 503 vs. GitHub 422 Tree SHA), reinforcing the pattern that these cleanup hooks are a recurring source of merge queue ejections.
  • #2837 (diagnostic comment on merge queue ejection): ralphbean had to manually discover the ejection and re-queue — automated diagnostics would have helped.

Autonomy readiness

The review agent's findings fully covered the human review — ralphbean approved without additional comments. This adds another data point to the 81 existing autonomy-readiness tracking issues (nearest precedents: #5353, #3021). For this class of change (small, well-scoped Go bug fix with comprehensive tests, single package, bot-authored), the review agent's approval was sufficient. No new tracking issue needed — the evidence base is well-established.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/mint Token mint and cross-boundary credentials ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

behaviour test flaky: mint deploy PEM bootstrap hits GitHub's unauthenticated app rate limit (60/hr)

1 participant