From 2542b215c87133b68b975174fa4cf821c9191120 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 08:23:25 -0600 Subject: [PATCH 1/7] Strip CLAUDECODE env var when spawning claude-code agent (#270) When the daemon is started from within a Claude Code session (or hooks fire during one), the CLAUDECODE environment variable gets inherited by worker processes. Claude Code's nested-session guard then rejects the spawn with "cannot be launched inside another Claude Code session". Strip CLAUDECODE alongside ANTHROPIC_API_KEY when building the child process environment. Make filterEnv variadic to support multiple keys cleanly. Closes #270 Co-Authored-By: Claude Opus 4.6 --- internal/agent/claude.go | 23 ++++++++++++++--------- internal/agent/claude_test.go | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/internal/agent/claude.go b/internal/agent/claude.go index 6e17d57e..223f39ed 100644 --- a/internal/agent/claude.go +++ b/internal/agent/claude.go @@ -136,14 +136,13 @@ func (a *ClaudeAgent) Review(ctx context.Context, repoPath, commitSHA, prompt st cmd.Dir = repoPath cmd.WaitDelay = 5 * time.Second - // Handle API key: use configured key if set, otherwise filter out env var - // to ensure Claude uses subscription auth instead of unexpected API charges + // Strip CLAUDECODE to prevent nested-session detection (#270), + // and handle API key (configured key or subscription auth). + stripKeys := []string{"ANTHROPIC_API_KEY", "CLAUDECODE"} if apiKey := AnthropicAPIKey(); apiKey != "" { - // Use explicitly configured API key from roborev config - cmd.Env = append(filterEnv(os.Environ(), "ANTHROPIC_API_KEY"), "ANTHROPIC_API_KEY="+apiKey) + cmd.Env = append(filterEnv(os.Environ(), stripKeys...), "ANTHROPIC_API_KEY="+apiKey) } else { - // Clear env var so Claude uses subscription auth - cmd.Env = filterEnv(os.Environ(), "ANTHROPIC_API_KEY") + cmd.Env = filterEnv(os.Environ(), stripKeys...) } // Suppress sounds from Claude Code (notification/completion sounds) cmd.Env = append(cmd.Env, "CLAUDE_NO_SOUND=1") @@ -253,11 +252,17 @@ func (a *ClaudeAgent) parseStreamJSON(r io.Reader, output io.Writer) (string, er } // filterEnv returns a copy of env with the specified key removed -func filterEnv(env []string, key string) []string { - prefix := key + "=" +func filterEnv(env []string, keys ...string) []string { result := make([]string, 0, len(env)) for _, e := range env { - if !strings.HasPrefix(e, prefix) { + strip := false + for _, key := range keys { + if strings.HasPrefix(e, key+"=") { + strip = true + break + } + } + if !strip { result = append(result, e) } } diff --git a/internal/agent/claude_test.go b/internal/agent/claude_test.go index e9cea81d..80817f15 100644 --- a/internal/agent/claude_test.go +++ b/internal/agent/claude_test.go @@ -239,3 +239,26 @@ func TestFilterEnv(t *testing.T) { t.Fatalf("missing expected env vars in filtered result: %v", filtered) } } + +func TestFilterEnvMultipleKeys(t *testing.T) { + env := []string{ + "PATH=/usr/bin", + "ANTHROPIC_API_KEY=secret", + "CLAUDECODE=1", + "HOME=/home/test", + } + + filtered := filterEnv(env, "ANTHROPIC_API_KEY", "CLAUDECODE") + + if len(filtered) != 2 { + t.Fatalf("expected 2 env vars, got %d: %v", len(filtered), filtered) + } + for _, e := range filtered { + if strings.HasPrefix(e, "ANTHROPIC_API_KEY=") { + t.Fatal("ANTHROPIC_API_KEY should be stripped") + } + if strings.HasPrefix(e, "CLAUDECODE=") { + t.Fatal("CLAUDECODE should be stripped") + } + } +} From eb7834009d44f55b51bf1ac2f75d0d6d5c689024 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 08:30:06 -0600 Subject: [PATCH 2/7] Use display width instead of byte length for help line wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The arrow characters (↑↓←→) are 3 bytes in UTF-8 but 1 terminal column wide. Using len() overestimated help line wrapping at widths 91-93, costing a visible row unnecessarily. Switch to runewidth.StringWidth() in both queue and review views. Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/roborev/tui.go b/cmd/roborev/tui.go index f6f85227..1d7b7db0 100644 --- a/cmd/roborev/tui.go +++ b/cmd/roborev/tui.go @@ -1631,8 +1631,8 @@ func queueHelpLines(width int) int { if width <= 0 { return 2 } - h1 := (len(queueHelpLine1) + width - 1) / width - h2 := (len(queueHelpLine2) + width - 1) / width + h1 := (runewidth.StringWidth(queueHelpLine1) + width - 1) / width + h2 := (runewidth.StringWidth(queueHelpLine2) + width - 1) / width return h1 + h2 } @@ -2620,8 +2620,8 @@ func (m tuiModel) renderReviewView() string { const helpLine2 = "↑/↓: scroll | ←/→: prev/next | ?: commands | esc: back" helpLines := 2 if m.width > 0 { - h1 := (len(helpLine1) + m.width - 1) / m.width - h2 := (len(helpLine2) + m.width - 1) / m.width + h1 := (runewidth.StringWidth(helpLine1) + m.width - 1) / m.width + h2 := (runewidth.StringWidth(helpLine2) + m.width - 1) / m.width helpLines = h1 + h2 } From 7a1c97f894b6b8162b6d6dc48dd062285e44ad6f Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 08:47:31 -0600 Subject: [PATCH 3/7] Ignore Windows temp file cleanup error in CI go test on Windows can exit 1 when it fails to remove temp .exe files due to OS file locking ("Access is denied"), even when all tests pass. Check for actual FAIL lines in test output instead of relying on the exit code alone. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/ci.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23ee6426..96021377 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,22 @@ jobs: - name: Test (Windows) if: matrix.os == 'windows-latest' - run: go test -tags integration ./... + shell: pwsh + run: | + # go test on Windows can exit 1 due to temp .exe cleanup + # ("Access is denied") even when all tests pass. Capture + # output and verify test results independently. + $output = go test -tags integration ./... 2>&1 | Tee-Object -Variable testOut + $exitCode = $LASTEXITCODE + if ($exitCode -ne 0) { + $failed = $testOut | Select-String "^FAIL\s" + if ($failed) { + Write-Host "Tests failed:" + Write-Host ($failed -join "`n") + exit 1 + } + Write-Host "All tests passed (ignoring temp file cleanup error)" + } - name: Build with CGO disabled run: go build ./... From 9a31db5ecc636bb6abf314e484bd43954769299b Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 08:48:59 -0600 Subject: [PATCH 4/7] Tighten Windows CI error suppression to cleanup errors only Only suppress non-zero exit when the output contains "Access is denied" and no FAIL lines. Propagate all other non-zero exits (toolchain errors, module failures, etc.) as real failures. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/ci.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 96021377..e239ab37 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,9 +36,9 @@ jobs: shell: pwsh run: | # go test on Windows can exit 1 due to temp .exe cleanup - # ("Access is denied") even when all tests pass. Capture - # output and verify test results independently. - $output = go test -tags integration ./... 2>&1 | Tee-Object -Variable testOut + # ("Access is denied") even when all tests pass. Only + # suppress that specific error; propagate all others. + go test -tags integration ./... 2>&1 | Tee-Object -Variable testOut $exitCode = $LASTEXITCODE if ($exitCode -ne 0) { $failed = $testOut | Select-String "^FAIL\s" @@ -47,6 +47,11 @@ jobs: Write-Host ($failed -join "`n") exit 1 } + $cleanup = $testOut | Select-String "Access is denied" + if (-not $cleanup) { + Write-Host "go test failed with unknown error" + exit $exitCode + } Write-Host "All tests passed (ignoring temp file cleanup error)" } From b6a4a9d7216d90a8c17027e173510b96e1e40591 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 09:02:59 -0600 Subject: [PATCH 5/7] Use exact key match in filterEnv instead of prefix match Split on first = and compare key portion exactly, preventing false matches when one key is a prefix of another (e.g. CLAUDE would have also stripped CLAUDECODE, CLAUDE_NO_SOUND). Co-Authored-By: Claude Opus 4.6 --- internal/agent/claude.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/agent/claude.go b/internal/agent/claude.go index 223f39ed..ed11053d 100644 --- a/internal/agent/claude.go +++ b/internal/agent/claude.go @@ -255,9 +255,10 @@ func (a *ClaudeAgent) parseStreamJSON(r io.Reader, output io.Writer) (string, er func filterEnv(env []string, keys ...string) []string { result := make([]string, 0, len(env)) for _, e := range env { + k, _, _ := strings.Cut(e, "=") strip := false for _, key := range keys { - if strings.HasPrefix(e, key+"=") { + if k == key { strip = true break } From f44cf65445a84a0a735a5c02290884f55bc13900 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 09:05:25 -0600 Subject: [PATCH 6/7] Add regression test for filterEnv exact key matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verify that stripping "CLAUDE" does not also strip "CLAUDECODE" or "CLAUDE_NO_SOUND" — the prefix-collision scenario that motivated the strings.Cut change. Co-Authored-By: Claude Opus 4.6 --- internal/agent/claude_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/internal/agent/claude_test.go b/internal/agent/claude_test.go index 80817f15..78d2127b 100644 --- a/internal/agent/claude_test.go +++ b/internal/agent/claude_test.go @@ -262,3 +262,24 @@ func TestFilterEnvMultipleKeys(t *testing.T) { } } } + +func TestFilterEnvExactMatchOnly(t *testing.T) { + env := []string{ + "CLAUDE=1", + "CLAUDECODE=1", + "CLAUDE_NO_SOUND=1", + "PATH=/usr/bin", + } + + filtered := filterEnv(env, "CLAUDE") + + if len(filtered) != 3 { + t.Fatalf("expected 3 env vars, got %d: %v", len(filtered), filtered) + } + for _, e := range filtered { + k, _, _ := strings.Cut(e, "=") + if k == "CLAUDE" { + t.Fatal("CLAUDE should be stripped") + } + } +} From 5af919c7523ba07233a4ba85f8774be1239b7b0c Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 16 Feb 2026 09:24:57 -0600 Subject: [PATCH 7/7] Fix Windows CI exit code and tighten cleanup suppression pattern The PowerShell script was missing `exit 0` after suppressing the temp .exe cleanup error, so the non-zero exit code from `go test` propagated through. Also narrowed the match from "Access is denied" to the specific "go: remove ...\.exe: Access is denied" pattern to avoid masking unrelated permission failures. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e239ab37..85ce63c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,8 +36,8 @@ jobs: shell: pwsh run: | # go test on Windows can exit 1 due to temp .exe cleanup - # ("Access is denied") even when all tests pass. Only - # suppress that specific error; propagate all others. + # ("go: remove ...\.exe: Access is denied") even when all + # tests pass. Only suppress that specific pattern. go test -tags integration ./... 2>&1 | Tee-Object -Variable testOut $exitCode = $LASTEXITCODE if ($exitCode -ne 0) { @@ -47,12 +47,13 @@ jobs: Write-Host ($failed -join "`n") exit 1 } - $cleanup = $testOut | Select-String "Access is denied" + $cleanup = $testOut | Select-String "go: remove .+\.exe: Access is denied" if (-not $cleanup) { Write-Host "go test failed with unknown error" exit $exitCode } - Write-Host "All tests passed (ignoring temp file cleanup error)" + Write-Host "All tests passed (ignoring temp .exe cleanup error)" + exit 0 } - name: Build with CGO disabled