From 247204a0369f8fd599bc52ac2a40589e7f35b5fd Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Mon, 11 May 2026 09:33:09 +0000 Subject: [PATCH] fix(handlers): return 501 for GitHub token on Gitea deployments (#388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Gitea-canonical deployments GITHUB_APP_ID/INSTALLATION_ID/PRIVATE_KEY_FILE are unset, so generateAppInstallationToken() returns an error with "required" in the message. Previously this fell through to a generic 500 "token refresh failed" — callers had no way to distinguish a permanent misconfiguration from a transient error. The fix: detect the "required" substring and return 501 Not Implemented + scm:"gitea". Callers can now branch on this and surface a clear "GitHub not configured" message instead of retrying indefinitely. Test updated: TestGitHubToken_NoTokenProvider now asserts 501 + scm:gitea. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/github_token.go | 13 ++++++++++++ .../internal/handlers/github_token_test.go | 21 ++++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/workspace-server/internal/handlers/github_token.go b/workspace-server/internal/handlers/github_token.go index ce9492a9..638a8a72 100644 --- a/workspace-server/internal/handlers/github_token.go +++ b/workspace-server/internal/handlers/github_token.go @@ -49,6 +49,7 @@ import ( "net/http" "os" "strconv" + "strings" "time" "github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook" @@ -98,6 +99,18 @@ func (h *GitHubTokenHandler) GetInstallationToken(c *gin.Context) { token, expiresAt, err := generateAppInstallationToken() if err != nil { log.Printf("[github] fallback token generation failed: %v", err) + // #388: When GITHUB_APP_ID/INSTALLATION_ID are unset (Gitea-canonical + // deployment), generateAppInstallationToken returns "required" as the error + // message. Distinguish it from a transient failure by checking for that + // substring and returning 501 Not Implemented so the caller knows GitHub + // integration is not configured rather than retrying a permanent failure. + if strings.Contains(err.Error(), "required") { + c.JSON(http.StatusNotImplemented, gin.H{ + "error": "GitHub integration not configured", + "scm": "gitea", + }) + return + } c.JSON(http.StatusInternalServerError, gin.H{"error": "token refresh failed"}) return } diff --git a/workspace-server/internal/handlers/github_token_test.go b/workspace-server/internal/handlers/github_token_test.go index 01076c81..bbc92920 100644 --- a/workspace-server/internal/handlers/github_token_test.go +++ b/workspace-server/internal/handlers/github_token_test.go @@ -77,12 +77,10 @@ func TestGitHubToken_NilRegistry(t *testing.T) { // // Post-#960/#1101 the handler now falls back to direct env-based App // token generation (GITHUB_APP_ID / INSTALLATION_ID / PRIVATE_KEY_FILE) -// when no registered provider matches. In the test environment those -// env vars are unset, so the fallback fails with 500 "token refresh -// failed" — a clean retryable signal for the workspace credential -// helper. Previously this path returned 404; the new 500 matches the -// ProviderError shape so callers don't have to branch on "missing -// provider" vs "provider failed". +// when no registered provider matches. On Gitea-canonical deployments +// those env vars are unset, so the fallback fails with 501 Not Implemented +// + scm:"gitea" — distinguishing a permanent "not configured" from a +// transient retryable error. Fixes #388. func TestGitHubToken_NoTokenProvider(t *testing.T) { reg := provisionhook.NewRegistry() reg.Register(&mockMutatorOnly{name: "other-plugin"}) @@ -91,12 +89,15 @@ func TestGitHubToken_NoTokenProvider(t *testing.T) { h.GetInstallationToken(c) - if w.Code != http.StatusInternalServerError { - t.Fatalf("expected 500 (env-based fallback fails with unset GITHUB_APP_* vars), got %d: %s", + if w.Code != http.StatusNotImplemented { + t.Fatalf("expected 501 (Gitea deployment: GITHUB_APP_* unset), got %d: %s", w.Code, w.Body.String()) } - if !strings.Contains(w.Body.String(), "token refresh failed") { - t.Errorf("expected body to contain 'token refresh failed', got: %s", w.Body.String()) + if !strings.Contains(w.Body.String(), "gitea") { + t.Errorf("expected body to contain 'gitea', got: %s", w.Body.String()) + } + if !strings.Contains(w.Body.String(), "not configured") { + t.Errorf("expected body to contain 'not configured', got: %s", w.Body.String()) } } -- 2.45.2