From b3e2e281910ca0cfc85bd94bc7d3cd86df9eda67 Mon Sep 17 00:00:00 2001 From: "linh.doan" Date: Wed, 9 Sep 2026 23:22:19 +0700 Subject: [PATCH] =?UTF-8?q?fix(config):=20inject=20the=20gh=20token=20reso?= =?UTF-8?q?lver=20seam=20=E2=80=94=20kills=20the=205s=20exec-timeout=20fla?= =?UTF-8?q?kes=20(#262)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ghAuthToken becomes a package-level seam (var ghAuthToken = ghAuthTokenExec); tests stub it instead of shimming PATH with a fake gh binary, so the 5s exec.CommandContext timeout can no longer fire under machine load (full-suite package parallelism was spawning the fake gh late). Trimming moves into resolveGithubToken; real-exec coverage retained for gh-absent and non-zero exit paths. --- docs/PRD.md | 20 ++++++----- internal/config/config.go | 11 +++--- internal/config/github_token_test.go | 51 ++++++++++++++++------------ 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index 47ca0fe..2fb070c 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -486,7 +486,10 @@ devagent run --ticket LINEAR-204 --worker claude-code # or opencode | both ### Environment variables (credentials only — FR-OPS-02) -`LINEAR_API_KEY`, `GITHUB_TOKEN` (scoped to contents:write + pull-requests:write on target repos), `DEVAGENT_HOME` (run state/logs). +`LINEAR_API_KEY`, `GITHUB_TOKEN` (scoped to contents:write + pull-requests:write +on target repos), `DEVAGENT_HOME` (run state/logs). `GITHUB_TOKEN` env wins; the +`gh auth token` keyring fallback (issue #234) resolves with a 5s timeout and +tests stub the resolver seam rather than exec gh (#262). ## 13. Integrations @@ -1469,11 +1472,10 @@ Master tracker with definition of done: [#207](https://github.com/FreePeak/devag --- -*Last updated: 2026-09-09 (TUI conventions-research polish wave landed: NO_COLOR/TERM=dumb -mono degradation, PAUSED aggregate + hero attention banner, `/` log search with n/N match -walking, selection-following viewports with hidden-count indicators, OSC-2 title, grouped -help, IUTF8, live version in the upgrade overlay — commit f3d21f9, research in -docs/research/tui-conventions.md. Supersedes the 09-09 all-four-open-issues stamp: #248 -worker observability (WatchdogSink default-wired, omp NDJSON events, stream metrics, -productive-wall-kill classification, PR #266); #206 scoreboard closed; #181 desktop app -v1 (PR #267); #146 TUI polish FR-TUI-P-01..12 (PR #268).)* \ No newline at end of file +*Last updated: 2026-09-09 (#262) — test hardening for the `gh auth token` +credential fallback: `ghAuthToken` is now an injectable seam +(`ghAuthTokenExec` keeps the real 5s-timeout exec; no production behavior +change), trimming moved into `resolveGithubToken`, and the previously +PATH-shim-based token tests stub the seam so the 5s exec timeout can no longer +flake under machine load; real-exec coverage retained for gh-absent and +non-zero-exit paths.* \ No newline at end of file diff --git a/internal/config/config.go b/internal/config/config.go index a339dce..3506848 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -662,20 +662,23 @@ func resolveGithubToken() string { return tok } githubTokenOnce.Do(func() { - githubTokenOnce.value = ghAuthToken() + githubTokenOnce.value = strings.TrimSpace(ghAuthToken()) }) return githubTokenOnce.value } -func ghAuthToken() string { +// ghAuthToken is the seam tests substitute to avoid a real exec; it defaults +// to the real `gh auth token` invocation (issue #262). +var ghAuthToken = ghAuthTokenExec + +func ghAuthTokenExec() string { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() out, err := exec.CommandContext(ctx, "gh", "auth", "token").Output() if err != nil { return "" } - tok := strings.TrimSpace(string(out)) - return tok + return string(out) } // CredentialStatus reports which credentials are present without ever diff --git a/internal/config/github_token_test.go b/internal/config/github_token_test.go index b3b5506..3537f19 100644 --- a/internal/config/github_token_test.go +++ b/internal/config/github_token_test.go @@ -29,6 +29,21 @@ func installFakeGh(t *testing.T, dir, token string, exit int) string { return counter } +// stubGhAuthToken replaces the gh resolver seam with a function returning +// token (or empty) and counting invocations in *int. Restores the real +// implementation on cleanup and resets the process-level cache (issue #262: +// no real exec under test, so the 5s timeout cannot flake under load). +func stubGhAuthToken(t *testing.T, token string, calls *int) { + t.Helper() + old := ghAuthToken + ghAuthToken = func() string { + *calls++ + return token + } + t.Cleanup(func() { ghAuthToken = old }) + resetGithubTokenCacheForTest() +} + // unsetGithubTokenForTest makes GITHUB_TOKEN genuinely absent (not just // empty) for the test and restores the original state afterwards. func unsetGithubTokenForTest(t *testing.T) { @@ -45,26 +60,22 @@ func unsetGithubTokenForTest(t *testing.T) { } func TestResolveGithubTokenEnvWins(t *testing.T) { - dir := t.TempDir() - counter := installFakeGh(t, dir, "gh-must-not-run", 0) - t.Setenv("PATH", dir) + calls := 0 + stubGhAuthToken(t, "gh-must-not-run", &calls) t.Setenv("GITHUB_TOKEN", "env-token") - resetGithubTokenCacheForTest() if got := LoadCredentials().GithubToken; got != "env-token" { t.Fatalf("GithubToken = %q, want env value", got) } - if _, err := os.Stat(counter); !os.IsNotExist(err) { - t.Fatalf("gh was invoked despite env GITHUB_TOKEN (counter: %v)", err) + if calls != 0 { + t.Fatalf("gh resolver invoked %d times despite env GITHUB_TOKEN, want 0", calls) } } func TestResolveGithubTokenGhFallback(t *testing.T) { - dir := t.TempDir() - installFakeGh(t, dir, " gh-keyring-token ", 0) // padded: resolution must trim - t.Setenv("PATH", dir) + calls := 0 + stubGhAuthToken(t, " gh-keyring-token ", &calls) // padded: resolution must trim unsetGithubTokenForTest(t) - resetGithubTokenCacheForTest() if got := LoadCredentials().GithubToken; got != "gh-keyring-token" { t.Fatalf("GithubToken = %q, want trimmed gh output", got) @@ -94,11 +105,9 @@ func TestResolveGithubTokenGhFailure(t *testing.T) { } func TestResolveGithubTokenEmptyEnvFallsThrough(t *testing.T) { - dir := t.TempDir() - installFakeGh(t, dir, "gh-empty-env", 0) - t.Setenv("PATH", dir) + calls := 0 + stubGhAuthToken(t, "gh-empty-env", &calls) t.Setenv("GITHUB_TOKEN", "") // empty counts as unset - resetGithubTokenCacheForTest() if got := LoadCredentials().GithubToken; got != "gh-empty-env" { t.Fatalf("GithubToken = %q, want gh fallback for empty env", got) @@ -106,11 +115,9 @@ func TestResolveGithubTokenEmptyEnvFallsThrough(t *testing.T) { } func TestResolveGithubTokenCachedPerProcess(t *testing.T) { - dir := t.TempDir() - counter := installFakeGh(t, dir, "gh-cached", 0) - t.Setenv("PATH", dir) + calls := 0 + stubGhAuthToken(t, "gh-cached", &calls) unsetGithubTokenForTest(t) - resetGithubTokenCacheForTest() if got := LoadCredentials().GithubToken; got != "gh-cached" { t.Fatalf("first call = %q, want gh token", got) @@ -118,8 +125,8 @@ func TestResolveGithubTokenCachedPerProcess(t *testing.T) { if got := LoadCredentials().GithubToken; got != "gh-cached" { t.Fatalf("second call = %q, want cached gh token", got) } - if data, err := os.ReadFile(counter); err != nil || string(data) != "x" { - t.Fatalf("gh invocation count wrong: data=%q err=%v, want exactly one exec", data, err) + if calls != 1 { + t.Fatalf("gh resolver invoked %d times, want exactly one exec", calls) } // A later-set env GITHUB_TOKEN still wins over the primed cache. @@ -127,7 +134,7 @@ func TestResolveGithubTokenCachedPerProcess(t *testing.T) { if got := LoadCredentials().GithubToken; got != "later-env" { t.Fatalf("GithubToken = %q, want later env value over cache", got) } - if data, err := os.ReadFile(counter); err != nil || string(data) != "x" { - t.Fatalf("env-wins check re-executed gh: data=%q err=%v", data, err) + if calls != 1 { + t.Fatalf("env-wins check re-executed gh: %d calls", calls) } }