fix: let an explicit --agent-mode override agent detection - #41
Closed
Bradenream wants to merge 2 commits into
Closed
Bradenream wants to merge 2 commits into
Bradenream wants to merge 2 commits into
Conversation
Execute calls InitAgentMode before cobra parses flags, to keep the
explorer TUI away from agents. That call can only see the environment,
and its answer was final: InitAgentMode checked the flag only after a
CompareAndSwap that every later call failed. So --agent-mode=false
under Claude Code still printed JSON envelopes, --agent-mode in a plain
shell still printed human errors, and the flag's help ("Use
--agent-mode=false to disable") was untrue.
InitAgentMode now checks for an explicitly set --agent-mode before that
early return, so the call from PersistentPreRunE, the first to see the
parsed flags, applies it. The environment is still checked only once.
The explorer check is unchanged: it only runs when vf has no arguments,
so no flag can be present.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The behavior test can read Linux keyring credentials and become nondeterministic.
Review effort: Balanced
Findings: 1
What changed in this PR
Fixes --agent-mode so explicit values override environment detection.
Changes:
- Applies parsed flag values after initial environment detection.
- Adds Go and CLI behavior tests.
| File | Description |
|---|---|
internal/output/agentmode.go |
Prioritizes explicit agent-mode flags. |
internal/output/agentmode_test.go |
Tests override behavior. |
test/agent-mode-flag.test.ts |
Adds end-to-end coverage. |
Files not reviewed (1)
- internal/output/agentmode.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The no-token cases ran vf with an empty HOME, which hides the macOS Keychain, but on Linux vf reaches the keyring over the D-Bus session bus. A developer with a token saved there would have had it found, and the cases would have taken a different path. They now point DBUS_SESSION_BUS_ADDRESS at a socket that does not exist, as test/setup.ts does for the whole suite in #37. Copilot raised this in review.
effervescentia
approved these changes
Oct 2, 2026
Contributor
Merge activity
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--agent-modehad no effect.ExecutecallsInitAgentModebefore cobra parses flags, to keep the explorer TUI away from agents. That call can only see the environment, and its answer was final:InitAgentModechecked the flag only after aCompareAndSwapthat every later call failed. So:--agent-mode=falseunder Claude Code still printed JSON envelopes.--agent-modein a plain shell still printed human errors.InitAgentModenow checks for an explicitly set--agent-modebefore that early return, so the call fromPersistentPreRunE, the first to see the parsed flags, applies it. The environment is still checked only once. The explorer check is unchanged: it only runs when vf has no arguments, so no flag can be present.This PR and #38 can merge in either order. #38's stdin rule reads
output.IsAgentMode()each time instead of keeping a copy (addressing Copilot's review there), so it follows this flag too. That is checked with both applied:--agent-modeturns the no-wait rule on outside an agent, and--agent-mode=falseunderCLAUDECODE=1turns it off.Before and after
CLAUDECODE=1 vf workspace list --agent-mode=falsewith no tokenvf workspace list --agent-modewith no agent env and no tokenCLAUDECODE=1 vf agent update --llm gpt-4 --agent-mode=falseTest plan
gofmt,go vet ./...andgo test ./...pass;go.modis unchanged.internal/output/agentmode_test.goreplays Execute's early call and then PersistentPreRunE's call. It covers the flag in both directions and both no-flag cases; 2 cases fail on master.test/agent-mode-flag.test.ts: 4 cases; 3 fail on master.CLI behaviourshows master's 4 existing failures until fix: restore vf docs search, and make the test suite hermetic and green #37 merges.