From d93b06005db6e7846df96754b8ac984ccb8fd192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Wed, 29 Jul 2026 23:47:56 +0100 Subject: [PATCH] refactor(backend): drop dead req param from resolveGHToken (#2013) PR #1810 retired the host-global gh token detection that injected GITHUB_TOKEN into the request env, leaving both remote executors with a `_ = req` placeholder to silence the unused parameter. resolveGHToken is now just a filter over the selected credential IDs, so the parameter is gone from the Sprites and SSH flavors alike. Test asserts the pass-through selection value, not just length. Co-Authored-By: Claude Opus 5 (1M context) --- .../lifecycle/executor_sprites_credentials.go | 5 ++-- .../lifecycle/executor_ssh_credentials.go | 5 ++-- .../executor_ssh_credentials_test.go | 23 +++++++------------ 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go index 4a19ffd47c..6183706399 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go @@ -42,7 +42,7 @@ func (r *SpritesExecutor) uploadCredentials( // Ignore the retired host-global gh token method in stale profiles. GitHub // automation credentials are supplied by the workspace credential broker. - selectedMethodIDs = r.resolveGHToken(selectedMethodIDs, req) + selectedMethodIDs = r.resolveGHToken(selectedMethodIDs) if len(selectedMethodIDs) == 0 { return nil @@ -122,8 +122,7 @@ func (r *SpritesExecutor) runAuthSetupScripts( // resolveGHToken filters the retired host-global gh token method from stale // profiles. Explicit secret-backed GITHUB_TOKEN values are resolved separately. -func (r *SpritesExecutor) resolveGHToken(credentialIDs []string, req *ExecutorCreateRequest) []string { - _ = req +func (r *SpritesExecutor) resolveGHToken(credentialIDs []string) []string { return removeID(credentialIDs, "gh_cli_token") } diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go index 168c67ffb6..e12a3ee15c 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go @@ -46,7 +46,7 @@ func (r *SSHExecutor) uploadCredentials( return fmt.Errorf("failed to parse remote_credentials: %w", err) } - selectedMethodIDs = r.resolveGHToken(selectedMethodIDs, req) + selectedMethodIDs = r.resolveGHToken(selectedMethodIDs) fileMethods := selectFileMethods(catalog, selectedMethodIDs, r.logger) if len(fileMethods) == 0 { @@ -133,8 +133,7 @@ func (r *SSHExecutor) runOneAuthSetupScript( // resolveGHToken filters the retired host-global gh token method from stale // profiles. Explicit secret-backed GITHUB_TOKEN values are resolved separately. -func (r *SSHExecutor) resolveGHToken(ids []string, req *ExecutorCreateRequest) []string { - _ = req +func (r *SSHExecutor) resolveGHToken(ids []string) []string { return removeID(ids, "gh_cli_token") } diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go index d784c199bc..cd537e21c1 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go @@ -2,8 +2,6 @@ package lifecycle import ( "context" - "os" - "path/filepath" "strings" "testing" @@ -11,30 +9,25 @@ import ( ) func TestRemoteExecutorsIgnoreLegacyHostGHTokenSelection(t *testing.T) { - binDir := t.TempDir() - ghPath := filepath.Join(binDir, "gh") - if err := os.WriteFile(ghPath, []byte("#!/bin/sh\nprintf host-global-token\n"), 0o755); err != nil { - t.Fatalf("write fake gh: %v", err) - } - t.Setenv("PATH", binDir) - + // resolveGHToken is a pure filter: it can no longer reach the request, so + // a stale gh_cli_token selection is dropped rather than turned into a + // host-global GITHUB_TOKEN injection. tests := []struct { name string - resolve func([]string, *ExecutorCreateRequest) []string + resolve func([]string) []string }{ {name: "ssh", resolve: (&SSHExecutor{logger: newTestLogger()}).resolveGHToken}, {name: "sprites", resolve: (&SpritesExecutor{logger: newTestLogger()}).resolveGHToken}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := &ExecutorCreateRequest{Env: map[string]string{}} - remaining := tt.resolve([]string{"gh_cli_token", "agent:codex:files:0"}, req) - if got := req.Env["GITHUB_TOKEN"]; got != "" { - t.Fatalf("GITHUB_TOKEN = %q, want no host-global token injection", got) - } + remaining := tt.resolve([]string{"gh_cli_token", "agent:codex:files:0"}) if len(remaining) != 1 || remaining[0] != "agent:codex:files:0" { t.Fatalf("remaining methods = %v, want stale gh_cli_token filtered", remaining) } + if got := tt.resolve([]string{"agent:codex:files:0"}); len(got) != 1 || got[0] != "agent:codex:files:0" { + t.Fatalf("resolve without gh_cli_token = %v, want untouched selection", got) + } }) } }