Skip to content

Strip CLAUDECODE env var when spawning claude-code agent#273

Merged
wesm merged 7 commits intomainfrom
cc-nested-session-error
Feb 16, 2026
Merged

Strip CLAUDECODE env var when spawning claude-code agent#273
wesm merged 7 commits intomainfrom
cc-nested-session-error

Conversation

@wesm
Copy link
Collaborator

@wesm wesm commented Feb 16, 2026

Summary

  • Strip CLAUDECODE from the child process environment when spawning the claude-code agent, preventing Claude Code's nested-session guard from rejecting the spawn
  • Make filterEnv variadic to support stripping multiple env vars cleanly
  • Add test for multi-key filtering

Same class of fix as #267 (GIT_DIR stripping for daemon spawn). When the daemon starts from within a Claude Code session, or post-commit hooks fire during one, CLAUDECODE gets inherited by worker processes and Claude Code refuses to launch.

Closes #270

Test plan

  • go test ./internal/agent/ passes (including new TestFilterEnvMultipleKeys)
  • go vet ./... clean
  • Manual: start daemon from within Claude Code session, verify reviews succeed

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 16, 2026

roborev: Combined Review

Verdict: No Medium, High, or Critical issues found; this change appears safe and security-positive.

Findings (Medium+)

No findings at Medium severity or higher after deduplicating all agent outputs.


Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

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 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 16, 2026

roborev: Combined Review

Verdict: No Medium/High/Critical issues identified; changes look safe to merge.

Findings (Medium+)

No findings at Medium severity or above after deduplicating all review outputs.


Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

wesm and others added 2 commits February 16, 2026 08:47
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 16, 2026

roborev: Combined Review

Verdict: No Medium, High, or Critical issues found; PR is acceptable as-is.

Findings (Medium+)

None.

All reported concerns were Low severity (primarily around CI heuristic robustness in .github/workflows/ci.yml) and were omitted per requested threshold.


Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

wesm and others added 2 commits February 16, 2026 09:02
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 16, 2026

roborev: Combined Review

Verdict: One Medium issue should be fixed before merge.

Medium

  • Overbroad Windows CI failure suppression can mask real test/build failures
    Location: .github/workflows/ci.yml:44, .github/workflows/ci.yml:50
    Details: The workflow treats some non-zero go test exits as ignorable based on a broad "Access is denied" output match plus absence of ^FAIL\s. This can hide unrelated permission/build/runtime failures that do not emit a standard FAIL line.
    Recommended fix: Narrow suppression to the known cleanup-only pattern (for example, temp go-build .exe cleanup error signature), and otherwise fail closed. A stronger approach is parsing go test -json and allowing suppression only for explicitly identified cleanup events while requiring normal pass indicators.

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

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 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 16, 2026

roborev: Combined Review

Verdict: No Medium, High, or Critical issues identified across all reviews; the PR appears clean.

Consolidated Findings (Medium+)

No findings to report.

Reviewer Consensus

All four review outputs reported no issues, including both security-focused and general correctness/regression passes.


Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

@wesm wesm merged commit 76863c2 into main Feb 16, 2026
7 checks passed
@wesm wesm deleted the cc-nested-session-error branch February 16, 2026 15:44
hughdbrown pushed a commit to hughdbrown/roborev that referenced this pull request Feb 16, 2026
…#273)

## Summary

- Strip `CLAUDECODE` from the child process environment when spawning
the claude-code agent, preventing Claude Code's nested-session guard
from rejecting the spawn
- Make `filterEnv` variadic to support stripping multiple env vars
cleanly
- Add test for multi-key filtering

Same class of fix as roborev-dev#267 (`GIT_DIR` stripping for daemon spawn). When
the daemon starts from within a Claude Code session, or post-commit
hooks fire during one, `CLAUDECODE` gets inherited by worker processes
and Claude Code refuses to launch.

Closes roborev-dev#270

## Test plan

- [x] `go test ./internal/agent/` passes (including new
`TestFilterEnvMultipleKeys`)
- [x] `go vet ./...` clean
- [x] Manual: start daemon from within Claude Code session, verify
reviews succeed

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

claude-code agent fails with nested session error in devcontainers

1 participant

Comments