fix: support gh account status on stderr (#2348) (#2379)

2026-08-07 18:13:31 +01:00
parent c97e85448b
commit 4f7298a5d1
5 changed files with 188 additions and 4 deletions
@@ -41,7 +41,10 @@ func (systemGHAccountRunner) Run(ctx context.Context, args ...string) (string, e
 		return cmd
 	})
 	if runErr == nil {
-		return stdout.String(), nil
+		if out := stdout.String(); strings.TrimSpace(out) != "" {
+			return out, nil
+		}
+		return stderr.String(), nil
 	}
 	if execCtxErr != nil && (errors.Is(execCtxErr, context.Canceled) || errors.Is(execCtxErr, context.DeadlineExceeded)) {
 		return "", execCtxErr
@@ -3,11 +3,28 @@ package github
 import (
 	"context"
 	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
 	"reflect"
+	"runtime"
 	"strings"
 	"testing"
 )

+func init() {
+	if os.Getenv("KANDEV_GH_ACCOUNT_TEST_HELPER") != "1" {
+		return
+	}
+	if len(os.Args) > 3 && os.Args[3] == "--json" {
+		fmt.Fprintln(os.Stderr, "unknown flag: --json")
+		os.Exit(1)
+	}
+	fmt.Fprintln(os.Stderr, "github.com")
+	fmt.Fprintln(os.Stderr, "  ✓ Logged in to github.com as alice (/home/alice/.config/gh/hosts.yml)")
+	os.Exit(0)
+}
+
 type fakeGHAccountRunner struct {
 	output string
 	err    error
@@ -38,6 +55,35 @@ func TestListGHAccountsParsesAllStoredLogins(t *testing.T) {
 	}
 }

+func TestSystemGHAccountRunnerUsesStderrWhenStdoutEmpty(t *testing.T) {
+	testBinary, err := os.Executable()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ghPath := filepath.Join(t.TempDir(), "gh")
+	if runtime.GOOS == "windows" {
+		ghPath += ".exe"
+	}
+	binary, err := os.ReadFile(testBinary)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(ghPath, binary, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", filepath.Dir(ghPath)+string(os.PathListSeparator)+os.Getenv("PATH"))
+	t.Setenv("KANDEV_GH_ACCOUNT_TEST_HELPER", "1")
+
+	accounts, err := listGHAccounts(context.Background(), systemGHAccountRunner{})
+	if err != nil {
+		t.Fatalf("listGHAccounts() error = %v", err)
+	}
+	want := []GHAccount{{Host: "github.com", Login: "alice", Active: true, State: "success"}}
+	if !reflect.DeepEqual(accounts, want) {
+		t.Fatalf("accounts = %#v, want %#v", accounts, want)
+	}
+}
+
 type legacyGHAccountRunner struct {
 	output string
 	args   [][]string
@@ -0,0 +1,61 @@
+---
+spec: docs/specs/integrations/github-authentication.md
+created: 2026-08-07
+status: done
+---
+
+# Implementation Plan: GitHub CLI account stderr fallback
+
+## Overview
+
+Update the host `gh` account command runner so a successful command with an empty
+stdout can return its stderr report to the existing legacy account parser. Keep
+non-empty stdout authoritative, preserving JSON account discovery and token
+resolution behavior. Add a regression test at the real subprocess boundary.
+
+## Backend
+
+### Successful command output selection
+
+- `apps/backend/internal/github/gh_accounts.go`, `systemGHAccountRunner.Run`: on
+  a successful `gh` command, return stdout when it contains non-whitespace; if
+  stdout is empty, return stderr. Preserve existing cancellation, timeout, and
+  non-zero exit handling.
+
+## Tests
+
+- **What:** A legacy `gh auth status` command that exits successfully and emits
+  its report only on stderr is parsed into the expected `GHAccount`.
+  **File:** `apps/backend/internal/github/gh_accounts_test.go`.
+  **How:** Use a temporary executable named `gh` on `PATH`, make structured
+  status fail with an unsupported-flag exit, emit legacy status on stderr, and
+  call `listGHAccounts` with `systemGHAccountRunner`.
+- **What:** Existing JSON, legacy stdout, token, and environment-sanitization
+  behaviors remain green. **File:** existing tests in
+  `apps/backend/internal/github/gh_accounts_test.go`. **How:** run the focused
+  package test selection and the package's full test suite.
+
+## Verification Results
+
+- RED: `rtk go test ./internal/github -run
+  TestSystemGHAccountRunnerUsesStderrWhenStdoutEmpty -count=1` failed with
+  `unsupported gh auth status output` before the production change.
+- GREEN: the same regression test passed after the fallback was added.
+- Focused: `rtk go test ./internal/github -run
+  'Test(ListGHAccounts|ResolveGHAccountToken|GitHubCLIEnvironment|SystemGHAccountRunner)' -count=1`
+  passed 12 tests.
+- Full package: `rtk go test ./internal/github -count=1` passed 1,092 tests.
+- `rtk git diff --check` passed.
+
+## Implementation Waves And Parallel Candidates
+
+Wave 1 (sequential):
+
+- [x] [task-01-gh-account-stderr](task-01-gh-account-stderr.md)
+
+No parallel-safe tasks: the production runner and its regression test share one
+behavioral boundary.
+
+## Open Questions
+
+None.
@@ -0,0 +1,72 @@
+---
+id: "01-gh-account-stderr"
+title: "Handle gh account status on stderr"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+spec: "../../specs/integrations/github-authentication.md"
+---
+
+# Task 01: Handle `gh` account status on stderr
+
+## Acceptance
+
+- A successful `systemGHAccountRunner.Run` returns non-empty stdout unchanged,
+  and returns stderr when stdout is empty.
+- `listGHAccounts` discovers a legacy account when the real `gh` command emits
+  its successful status report only on stderr.
+- Existing timeout, cancellation, non-zero exit, JSON, legacy stdout, and token
+  behaviors remain unchanged.
+
+## Verification
+
+- `cd apps/backend && go test ./internal/github -run 'Test(ListGHAccounts|ResolveGHAccountToken|GitHubCLIEnvironment|SystemGHAccountRunner)' -count=1`
+- `cd apps/backend && go test ./internal/github -count=1`
+
+## Files likely touched
+
+- `apps/backend/internal/github/gh_accounts.go`
+- `apps/backend/internal/github/gh_accounts_test.go`
+- `docs/specs/integrations/github-authentication.md`
+- `docs/plans/github-cli-account-stderr/plan.md`
+- `docs/plans/github-cli-account-stderr/task-01-gh-account-stderr.md`
+
+## Dependencies
+
+None.
+
+## Parallelism
+
+Sequential. The production runner and regression test share the same command
+output contract.
+
+## Inputs
+
+- The amended named-CLI behavior and scenario in
+  `docs/specs/integrations/github-authentication.md`.
+- The confirmed root cause in `systemGHAccountRunner.Run` and the existing
+  stderr-first pattern in `GHClient.RunAuthDiagnostics`.
+- Issue `kdlbs/kandev#2348`.
+
+## Output contract
+
+Report the files changed, exact test commands and outcomes, any remaining risk,
+and synchronized task/plan statuses in the primary session.
+
+## Results
+
+- RED: `rtk go test ./internal/github -run
+  TestSystemGHAccountRunnerUsesStderrWhenStdoutEmpty -count=1` failed as
+  expected with `unsupported gh auth status output` before the production
+  change.
+- GREEN: the same regression test passed after the fallback was added.
+- Focused task check: `rtk go test ./internal/github -run
+  'Test(ListGHAccounts|ResolveGHAccountToken|GitHubCLIEnvironment|SystemGHAccountRunner)' -count=1`
+  passed 12 tests.
+- Full task check: `rtk go test ./internal/github -count=1` passed 1,092 tests.
+- `rtk git diff --check` passed.
+- The helper executable is copied into `t.TempDir()` and removed by Go test
+  cleanup; no temporary files or processes remain.
+- Security/trust boundary: the runner still strips ambient `GH_TOKEN` and
+  `GITHUB_TOKEN`; the fallback only selects command output after a zero exit.
@@ -1,7 +1,7 @@
 ---
 status: building
 created: 2026-07-19
-amended: 2026-08-02
+amended: 2026-08-07
 owner: Kandev
 ---

@@ -40,6 +40,8 @@ automation under different GitHub Apps without operating separate Kandev deploym
 - PAT and named CLI automation act as the verified human account. A separate `My GitHub` connection
  is only offered when workspace automation uses a GitHub App, because App installations are not
  people and cannot provide authenticated-viewer semantics.
+- Named CLI account discovery accepts a successful `gh auth status` report from either stdout or
+  stderr, while preserving non-empty stdout as the authoritative command result.
 - User-triggered mutations prefer the workspace's verified personal connection, then a human
  PAT/CLI automation connection, then the App installation. The UI always identifies the effective
  actor and never labels an App mutation as human-attributed.
@@ -684,8 +686,8 @@ registration and never creates a global default.
  views the running session, **THEN** the Changes disclosure still shows its launch snapshot; a
  successful resume records and shows the newly resolved contract.
 - **GIVEN** an authenticated host GitHub CLI without structured status or named-token flags,
-  **WHEN** an operator selects its sole account, **THEN** Kandev discovers and validates that
-  account without requiring a CLI upgrade.
+  **WHEN** an operator selects its sole account and `gh auth status` reports successfully on
+  stderr, **THEN** Kandev discovers and validates that account without requiring a CLI upgrade.
 - **GIVEN** a valid migrated `legacy_shared` connection and a legacy shared managed checkout,
  **WHEN** a task needs a workspace-isolated checkout, **THEN** Kandev resolves the same automation
  identity's Git credential and clones into that workspace's managed root without persisting or