feat(agent): integrate praxis-harness TUI + headless behind experimental flag - #65
feat(agent): integrate praxis-harness TUI + headless behind experimental flag#65vishnukv-facets wants to merge 4 commits into
Conversation
…tal flag Adds praxis-harness as a Go module dependency and exposes two new cobra commands gated behind PRAXIS_EXPERIMENTAL=1 or --experimental: - praxis chat: interactive Bubble Tea TUI coding agent (tui.Run) - praxis run: headless one-shot runner (native.Run) The bridge lives in internal/agent/ — the single import boundary between praxis-cli's cobra world and praxis-harness's SDK. It maps cobra flags to the harness's tui.Config (via ParseFlags) and native.Run (via argv), keeping the two repos decoupled. Auth is two-layer and stays separate: control-plane credentials (~/.praxis/credentials) for CLI gateway calls, LLM provider credentials (~/.praxis/agent/auth.json) for the agent. The TUI's /login handles provider auth. Changes: - go.mod: Go 1.25, praxis-harness dep (local replace for dev) - internal/agent/: bridge package (ChatOptions, HeadlessArgs, flag mapping) - cmd/chat.go: praxis chat cobra command with --experimental gate - cmd/run.go: praxis run cobra command with --experimental gate - .github/workflows: Go 1.25 in CI + release - 10 unit tests for the bridge (flag mapping, experimental gate, logo) Verified: go build, go vet, gofmt, 527 tests pass.
WalkthroughThe CLI adds an experimental ChangesExperimental agent integration
Sequence Diagram(s)sequenceDiagram
participant User
participant CobraCommand
participant AgentPackage
participant PraxisHarness
User->>CobraCommand: praxis chat --agents or praxis run
CobraCommand->>AgentPackage: Check enablement and pass options
AgentPackage->>PraxisHarness: Run TUI or headless execution
PraxisHarness-->>AgentPackage: Return error or exit code
AgentPackage-->>CobraCommand: Return execution result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
internal/agent/agent_test.go (1)
8-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a table-driven environment-gate test.
These near-duplicate cases should be represented as table rows for clarity and easier coverage of additional values such as
"0"and"false".As per coding guidelines: “Use table-driven tests as the default pattern.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/agent_test.go` around lines 8 - 58, Consolidate the near-duplicate environment-gate tests around Enabled and CheckEnabled into table-driven tests, using cases for unset, enabled values such as "1" and "true", and disabled values such as "0" and "false". Preserve the existing expected results and environment restoration for each case, while keeping the CheckEnabled error-versus-nil assertions explicit.Source: Coding guidelines
cmd/chat.go (1)
51-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated experimental-gate + signal-context boilerplate across
chatandrun. Both commands repeat the same "enable if flag set → CheckEnabled → print error → signal.NotifyContext + defer stop()" sequence; a shared helper would keep this consistent as more agent commands are added.
cmd/chat.go#L51-L60: extract this block into a shared helper (e.g.,agent.SetupExperimental(cmd, experimentalFlag) (context.Context, func(), error)) that both commands call.cmd/run.go#L54-L63: replace the identical block with a call to the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/chat.go` around lines 51 - 60, The experimental-agent setup and signal-context lifecycle are duplicated between the command blocks. In cmd/chat.go lines 51-60, add a shared agent.SetupExperimental helper that enables the agent when requested, checks availability, prints failures, and returns the context, cleanup function, and error; replace the inline setup with that helper. Apply the same replacement in cmd/run.go lines 54-63, preserving each command’s existing error-return behavior.cmd/run.go (1)
83-86: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid
os.Exithereos.Exit(exitCode)skips the deferredstop(). Return an exit-code error and translate it incmd.Execute/mainso cleanup still runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/run.go` around lines 83 - 86, Replace the os.Exit call in the headless execution path with an error carrying the returned exit code, allowing deferred stop() cleanup to run. Update cmd.Execute or main to recognize and translate this exit-code error into the process exit status while preserving successful execution behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/chat.go`:
- Around line 50-82: Add focused direct tests in cmd/chat_test.go for the chat
command’s RunE, covering experimental gating, agent.CheckEnabled behavior, and
complete flag-to-agent.ChatOptions mapping; add cmd/run_test.go for run’s RunE,
specifically verifying exit-code handling. Cover both successful and relevant
failure paths while isolating command dependencies.
In `@cmd/run.go`:
- Around line 24-30: Update the run command’s option definitions and
output-selection logic to add a unified --json flag and automatically select
parseable JSON output when os.Stdout is not a TTY. Integrate this behavior with
the existing runResultJSON and runUsageJSON paths so explicit flags remain
supported while machine consumers receive JSON by default.
- Around line 46-50: Update the run command help examples in the command
definition near the Examples block to use valid long-form flags with double
hyphens: replace each single-hyphen option for experimental, prompt,
prompt-file, model, and result-json while preserving the example arguments and
behavior.
In `@go.mod`:
- Line 90: Remove the absolute local replacement from go.mod for
github.com/Facets-cloud/praxis-harness. Use a published harness module version,
and place any local development override in a developer-only go.work file, or
use a portable relative replacement with the required CI checkout configuration.
In `@internal/agent/agent_test.go`:
- Around line 8-11: Update the environment setup and cleanup in
TestEnabledDefaultOff and the similarly structured test blocks to preserve
whether experimentalEnvVar was originally unset. Use t.Setenv for value-based
cases, and use os.LookupEnv when saving the prior state so cleanup calls
os.Unsetenv if it was absent, otherwise restores its original value.
- Around line 43-46: Strengthen the error assertion in the CheckEnabled test by
verifying the expected disabled-state error type or exact message, using
errors.Is, errors.As, or a precise message comparison. Keep the existing nil
check behavior while asserting the returned error matches the documented
contract.
- Around line 99-124: Strengthen the test’s ToNativeArgs assertions by
validating each flag’s associated value and the -no-mcp boolean semantics, not
merely flag presence. Check the expected prompt, model, cwd, and max-turns
values using the generated argument ordering or per-flag lookup, while
preserving coverage that -no-mcp is emitted with the correct behavior.
In `@internal/agent/agent.go`:
- Around line 18-20: Update the praxis-harness replace directive in go.mod to
use a repository-relative path or a published module version instead of the
developer-local absolute path, so the internal/agent package builds consistently
across environments.
---
Nitpick comments:
In `@cmd/chat.go`:
- Around line 51-60: The experimental-agent setup and signal-context lifecycle
are duplicated between the command blocks. In cmd/chat.go lines 51-60, add a
shared agent.SetupExperimental helper that enables the agent when requested,
checks availability, prints failures, and returns the context, cleanup function,
and error; replace the inline setup with that helper. Apply the same replacement
in cmd/run.go lines 54-63, preserving each command’s existing error-return
behavior.
In `@cmd/run.go`:
- Around line 83-86: Replace the os.Exit call in the headless execution path
with an error carrying the returned exit code, allowing deferred stop() cleanup
to run. Update cmd.Execute or main to recognize and translate this exit-code
error into the process exit status while preserving successful execution
behavior.
In `@internal/agent/agent_test.go`:
- Around line 8-58: Consolidate the near-duplicate environment-gate tests around
Enabled and CheckEnabled into table-driven tests, using cases for unset, enabled
values such as "1" and "true", and disabled values such as "0" and "false".
Preserve the existing expected results and environment restoration for each
case, while keeping the CheckEnabled error-versus-nil assertions explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d56328c4-846f-4d86-adc3-65001004b8af
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/release.ymlcmd/chat.gocmd/run.gogo.modinternal/agent/agent.gointernal/agent/agent_test.go
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if chatExperimental { | ||
| agent.Enable() | ||
| } | ||
| if err := agent.CheckEnabled(); err != nil { | ||
| render.PrintError(os.Stderr, false, err.Error(), "", 1) | ||
| return err | ||
| } | ||
|
|
||
| ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) | ||
| defer stop() | ||
|
|
||
| opts := agent.ChatOptions{ | ||
| Model: chatModel, | ||
| Thinking: chatThinking, | ||
| PermissionMode: chatPermission, | ||
| Cwd: chatCwd, | ||
| SessionID: chatSessionID, | ||
| SessionDir: chatSessionDir, | ||
| Resume: chatResume, | ||
| TeamName: chatTeamName, | ||
| SafeMode: chatSafe, | ||
| Ephemeral: chatEphemeral, | ||
| McpConfig: chatMcpConfig, | ||
| SettingsPath: chatSettings, | ||
| Profile: chatProfile, | ||
| FallbackModels: chatFallback, | ||
| Prompt: chatPrompt, | ||
| MaxTurns: chatMaxTurns, | ||
| } | ||
|
|
||
| return agent.RunChat(ctx, opts) | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no test files exist for the new commands
fd -g 'chat_test.go' -g 'run_test.go' cmdRepository: Facets-cloud/praxis-cli
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level cmd files:\n'
git ls-files cmd | sort
printf '\nTest files under cmd:\n'
git ls-files 'cmd/*_test.go' | sort
printf '\nRelevant command files:\n'
for f in cmd/chat.go cmd/run.go; do
if [ -f "$f" ]; then
printf '\n== %s ==\n' "$f"
wc -l "$f"
sed -n '1,220p' "$f"
fi
doneRepository: Facets-cloud/praxis-cli
Length of output: 9444
Add cmd/chat_test.go and cmd/run_test.go
These new cobra commands still lack direct tests for their RunE paths: experimental gating, flag→options mapping, and run’s exit-code handling. Add focused command tests before merge.
📍 Affects 2 files
cmd/chat.go#L50-L82(this comment)cmd/run.go#L53-L88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/chat.go` around lines 50 - 82, Add focused direct tests in
cmd/chat_test.go for the chat command’s RunE, covering experimental gating,
agent.CheckEnabled behavior, and complete flag-to-agent.ChatOptions mapping; add
cmd/run_test.go for run’s RunE, specifically verifying exit-code handling. Cover
both successful and relevant failure paths while isolating command dependencies.
Source: Coding guidelines
There was a problem hiding this comment.
Valid request. The bridge layer (internal/agent/) has 10 unit tests covering experimental gating, flag-to-options mapping, and argv generation. The RunE paths in cmd/chat.go/cmd/run.go directly call agent.RunChat/agent.RunHeadless, which launch the full TUI/agent loop — unit-testing those requires mocking tui.Run/native.Run, which are in the harness package (not stubbable without an interface). Deferring cmd/*_test.go to a follow-up PR.
There was a problem hiding this comment.
@vishnukv-facets, understood. The bridge tests provide useful coverage for the mapping and gate; direct cmd-level tests would need an injected runner interface/function seam rather than invoking the harness TUI/native loop.
The finding remains valid but can be deferred. Would you like me to open a GitHub follow-up issue for adding that seam and focused cmd/chat_test.go / cmd/run_test.go coverage?
✏️ Learnings added
Learnt from: vishnukv-facets
Repo: Facets-cloud/praxis-cli PR: 65
File: cmd/chat.go:50-82
Timestamp: 2026-07-29T18:16:10.497Z
Learning: In `praxis-cli`, `cmd/chat.go` and `cmd/run.go` directly invoke `internal/agent.RunChat` and `internal/agent.RunHeadless`, which enter the `praxis-harness` TUI/native agent loops. Command-level unit tests require an injectable runner seam because the harness runners are not currently stubbable. `internal/agent/agent_test.go` covers experimental gating, option-to-argument mapping, and argument generation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| runResultJSON bool | ||
| runUsageJSON bool | ||
| runNoMCP bool | ||
| runMcpConfig string | ||
| runSettings string | ||
| runMaxTurns int | ||
| runMaxTokenBudget int |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
No --json/TTY auto-detection for machine consumers, as required by guidelines.
run is explicitly designed for "scripting, CI, and automation pipelines" (Line 38) and exposes --result-json/--usage-json, but there's no unified --json flag and no logic to auto-emit JSON when os.Stdout isn't a TTY. AI hosts spawning praxis run as a subprocess without passing the JSON flags explicitly won't get parseable output.
As per coding guidelines, "If a command has parseable JSON output, support --json and auto-emit JSON when os.Stdout is not a TTY so AI hosts spawning praxis as a subprocess always get parseable output."
Also applies to: 91-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/run.go` around lines 24 - 30, Update the run command’s option definitions
and output-selection logic to add a unified --json flag and automatically select
parseable JSON output when os.Stdout is not a TTY. Integrate this behavior with
the existing runResultJSON and runUsageJSON paths so explicit flags remain
supported while machine consumers receive JSON by default.
Source: Coding guidelines
There was a problem hiding this comment.
Valid per the repo guidelines. --result-json already covers the explicit machine-consumer case. Adding a unified --json flag + non-TTY auto-detection is a good follow-up enhancement, but out of scope for this integration PR (the harness's native.Run owns the JSON encoding logic — wiring auto-detection through the cobra layer requires plumbing a TTY check into the bridge). Tracked for the next iteration.
There was a problem hiding this comment.
@vishnukv-facets, understood. The finding remains valid but is appropriately deferred from this integration PR given the harness-owned output path. Would you like me to open a GitHub follow-up issue for unified --json support and non-TTY JSON auto-selection?
Switches from SSH deploy key to a fine-grained PAT (CI_GITHUB_TOKEN secret) for private module access. Simpler setup — no SSH keys to manage. All three workflows (ci build, goreleaser-check, release) now: 1. Set GOPRIVATE + GONOSUMDB for Facets-cloud/* 2. Configure git insteadOf to embed the PAT in HTTPS URLs go.mod: replace directive removed; requires praxis-harness v0.1.0.
- Fix single-dash flag examples in help text to double-dash (--prompt etc.) - Use t.Setenv instead of manual os.Getenv/Setenv cleanup (preserves original unset state, auto-restores on test completion) - Assert errors.Is(err, ErrExperimentalDisabled) not just nil check - Strengthen HeadlessArgs.ToNativeArgs test to verify flag values not just presence - go.mod replace directive already removed in prior commit (now v0.1.0)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/agent/agent_test.go (1)
77-117: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the
NoMCP=falsebranch.The test only proves that
NoMCP=trueemits-no-mcp; an implementation that always appends the flag would still pass and disable MCP unexpectedly.Proposed test
+ for _, tt := range []struct { + name string + noMCP bool + wantFlag bool + }{ + {"enabled", true, true}, + {"disabled", false, false}, + } { + t.Run(tt.name, func(t *testing.T) { + args := (HeadlessArgs{NoMCP: tt.noMCP}).ToNativeArgs() + gotFlag := false + for _, arg := range args { + gotFlag = gotFlag || arg == "-no-mcp" + } + if gotFlag != tt.wantFlag { + t.Fatalf("ToNativeArgs() -no-mcp presence = %v, want %v; args = %v", gotFlag, tt.wantFlag, args) + } + }) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/agent_test.go` around lines 77 - 117, Extend TestHeadlessArgsToNativeArgs to also validate the NoMCP=false case: create or update HeadlessArgs with NoMCP disabled, call ToNativeArgs, and assert that -no-mcp is absent. Preserve the existing assertions proving the flag is emitted when NoMCP is true.Source: Coding guidelines
🧹 Nitpick comments (1)
internal/agent/agent_test.go (1)
8-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a table-driven enablement test.
These three cases duplicate the same setup and assertion; consolidate
"","1", and"true"into one table-driven test.Proposed refactor
-func TestEnabledDefaultOff(t *testing.T) { - t.Setenv(experimentalEnvVar, "") - if Enabled() { - t.Fatal("Enabled() = true, want false when PRAXIS_EXPERIMENTAL is unset") - } -} - -func TestEnabledEnvVar(t *testing.T) { - t.Setenv(experimentalEnvVar, "1") - if !Enabled() { - t.Fatal("Enabled() = false, want true when PRAXIS_EXPERIMENTAL=1") - } -} - -func TestEnabledEnvVarTrue(t *testing.T) { - t.Setenv(experimentalEnvVar, "true") - if !Enabled() { - t.Fatal("Enabled() = false, want true when PRAXIS_EXPERIMENTAL=true") - } +func TestEnabled(t *testing.T) { + tests := []struct { + name string + env string + want bool + }{ + {"default off", "", false}, + {"one enables", "1", true}, + {"true enables", "true", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(experimentalEnvVar, tt.env) + if got := Enabled(); got != tt.want { + t.Fatalf("Enabled() = %v, want %v", got, tt.want) + } + }) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/agent_test.go` around lines 8 - 27, Consolidate TestEnabledDefaultOff, TestEnabledEnvVar, and TestEnabledEnvVarTrue into one table-driven enablement test covering "", "1", and "true" with their expected results. Set experimentalEnvVar and assert Enabled() for each table entry, preserving the existing test behavior and failure reporting.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/agent/agent_test.go`:
- Around line 77-117: Extend TestHeadlessArgsToNativeArgs to also validate the
NoMCP=false case: create or update HeadlessArgs with NoMCP disabled, call
ToNativeArgs, and assert that -no-mcp is absent. Preserve the existing
assertions proving the flag is emitted when NoMCP is true.
---
Nitpick comments:
In `@internal/agent/agent_test.go`:
- Around line 8-27: Consolidate TestEnabledDefaultOff, TestEnabledEnvVar, and
TestEnabledEnvVarTrue into one table-driven enablement test covering "", "1",
and "true" with their expected results. Set experimentalEnvVar and assert
Enabled() for each table entry, preserving the existing test behavior and
failure reporting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 200b3c0d-1f92-4265-87b1-fc5ce02cf2aa
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/release.ymlcmd/run.gogo.modinternal/agent/agent_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/run.go
- .github/workflows/ci.yml
…harness v0.1.1 praxis-cli has its own 'praxis agents' command (lists installed agent files; called with --json by skills praxis installs into AI hosts), so the harness session dashboard cannot share that verb. Expose it as a start view of the agent command instead: 'praxis chat --agents'. internal/agent.ChatOptions.AgentsView emits a LEADING positional 'agents' into tui.ParseFlags, which only honors it as args[0] — any other position and the TUI silently opens a normal chat session. MarkFlagsMutuallyExclusive rejects --agents combined with --prompt / --resume / --session-id, since tui.loadDashboardApplication clears exactly those per row. Bump praxis-harness v0.1.0 -> v0.1.1 for the dashboard fixes that make this flag usable: multiline/ANSI/OSC session titles are flattened and sanitized at the label source, the dashboard body is clamped to its reserved height in physical lines (composer stays visible), and header counts share one partition with the group headers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/chat.go`:
- Around line 112-117: Replace the MarkFlagsMutuallyExclusive checks in the chat
command with conflicts gated on the chatAgents value, so only enabled agents
mode conflicts with prompt, resume, or session-id; explicitly passing
--agents=false must allow the single-session path. Add a regression test
covering --agents=false together with --prompt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fe04382-db5a-4d05-ab74-39053a4b9f67
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
cmd/chat.gocmd/chat_test.gogo.modinternal/agent/agent.gointernal/agent/agent_test.go
| // The dashboard owns session identity: it picks the row to open and clears the | ||
| // startup prompt and resume path (harness tui.loadDashboardApplication). Refuse | ||
| // the combinations it would silently drop rather than ignoring the user's input. | ||
| chatCmd.MarkFlagsMutuallyExclusive("agents", "prompt") | ||
| chatCmd.MarkFlagsMutuallyExclusive("agents", "resume") | ||
| chatCmd.MarkFlagsMutuallyExclusive("agents", "session-id") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== Files ==\n'
git ls-files cmd/chat.go cmd/chat_test.go
printf '\n== Relevant excerpt from cmd/chat.go ==\n'
cat -n cmd/chat.go | sed -n '90,150p'
printf '\n== Relevant excerpt from cmd/chat_test.go ==\n'
cat -n cmd/chat_test.go | sed -n '1,260p'
printf '\n== Search for --agents and mutual-exclusion usage ==\n'
rg -n "MarkFlagsMutuallyExclusive|agents=false|--agents|chatAgents|prompt" cmd/chat.go cmd/chat_test.goRepository: Facets-cloud/praxis-cli
Length of output: 8321
🌐 Web query:
Cobra v1.10.2 flag_groups.go Flag.Changed mutually exclusive bool flag false conflict
💡 Result:
In Cobra v1.10.2, the logic for mutually exclusive flags relies on tracking which flags have been set [1]. When using cmd.MarkFlagsMutuallyExclusive(...), Cobra internally monitors the Changed status of the flags within the group [1][2]. The Flag.Changed property (from the underlying pflag library) tracks whether a flag was explicitly set during parsing [3][4]. Once a flag's Changed status is true, it remains in that state for the duration of the command's execution [3][4]. If you are encountering conflicts where Cobra incorrectly reports a violation of mutual exclusivity during testing (even when flags are set in separate test runs), it is typically because the pflag.FlagSet is not being reset between tests [5]. Because Changed is true, the validation logic still perceives the flag as being "set" [5]. To resolve this conflict in your tests: 1. Ensure you are resetting the command's flag state between test cases [5]. 2. You can manually reset or re-initialize the FlagSet or the command object for each test to clear the Changed state [5]. 3. For programmatic testing, you can explicitly set the Changed field to false for flags if you need to reuse the same flag set, though re-instantiating the command/flagset is the recommended, cleaner approach [5]. Validation logic in flag_groups.go checks these Changed statuses to enforce exclusivity [1][2]. If more than one flag in a mutually exclusive group has Changed == true, Cobra will trigger an error [1][6][7].
Citations:
- 1: https://github.com/spf13/cobra/blob/v1.10.2/flag_groups.go
- 2: https://github.com/spf13/cobra/blob/main/flag_groups.go
- 3: Confusing semantics of Flag.Changed spf13/pflag#308
- 4: https://github.com/spf13/pflag/blob/master/flag.go
- 5: Issues with flags marked as mutually exclusive during testing spf13/cobra#2163
- 6: https://github.com/spf13/cobra/blob/b9ca5949e2f58373e8e4c3823c213401f7d9d0e3/flag_groups_test.go
- 7: https://github.com/andrewstellman/quality-playbook/blob/main/repos/docs_gathered/cobra/09-FLAG_GROUPS.md
Allow explicitly disabling --agents. MarkFlagsMutuallyExclusive treats an explicitly passed --agents=false as set, so praxis chat --agents=false --prompt ... still errors even though it should follow the single-session path. Gate the conflict on chatAgents instead and add a regression test for --agents=false + --prompt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/chat.go` around lines 112 - 117, Replace the MarkFlagsMutuallyExclusive
checks in the chat command with conflicts gated on the chatAgents value, so only
enabled agents mode conflicts with prompt, resume, or session-id; explicitly
passing --agents=false must allow the single-session path. Add a regression test
covering --agents=false together with --prompt.
Summary
Integrates the praxis-harness coding agent (TUI + headless runner) into praxis-cli as a Go module dependency. Both commands are gated behind
PRAXIS_EXPERIMENTAL=1or--experimentaland disabled by default.New commands
Without the flag:
Architecture
internal/agent/— the single import boundary between praxis-cli and praxis-harness. Maps cobra flags → harnesstui.Config/native.Runargv.~/.praxis/credentials(control-plane) +~/.praxis/agent/auth.json(LLM providers). The TUI's/loginhandles provider auth.go.modhas areplacedirective for local dev; CI will use the published tag once PR feat(login)!: replace localhost callback with server-mediated poll #5 merges.Changes
go.mod: Go 1.25.0, praxis-harness dep (localreplacefor dev)internal/agent/agent.go+agent_test.go: bridge packagecmd/chat.go:praxis chatwith--experimentalgatecmd/run.go:praxis runwith--experimentalgate.github/workflows/ci.yml+release.yml: Go 1.25Verification
go build✅go vet✅gofmt -lclean ✅go test ./...— 527 tests pass ✅praxis chatwithout flag → experimental gate error ✅praxis chat --help→ full help with flags ✅Summary by CodeRabbit
praxis chat --agentsto launch the session dashboard (instead of a single interactive session), with updated help text.praxis runfor headless prompt execution with extensive flag-based control (model, sessions, MCP, output JSON, and runtime limits).chat --agentsflag combinations.--agentsbehavior and argument mapping.