feat(cli): add tracevault agent-policies command#199
Conversation
Renders Markdown instructions from the active policies on the server so agents can self-discover what they need to do before pushing — no manual CLAUDE.md sync required. - New `tracevault agent-policies` CLI subcommand - New `agent_policies` MCP tool that shells out to the CLI for parity - README updated with the recommended minimal CLAUDE.md template - 13 unit tests covering all rendering branches (session/window scopes, block/warn/allow actions, must_succeed combinations, conditional file patterns, empty repos, validation_window_mode interactions) Closes #172.
| .filter(|o| o.status.success()) | ||
| .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()) | ||
| .as_deref() | ||
| .and_then(|p| p.rsplit('/').next()) |
There was a problem hiding this comment.
Repo name extraction splits only on '/'; Windows paths use '\', so repo_name becomes wrong and repo lookup can fail consistently on Windows.
| .and_then(|p| p.rsplit('/').next()) | |
| .and_then(|p| Path::new(p).file_name()?.to_str()) |
Details
✨ AI Reasoning
The code is trying to derive the repository name from the git top-level path. That derivation assumes POSIX path separators, but the command output is OS-dependent. On Windows, the separator is different, so the computed name becomes the full path string instead of the final directory name, which then breaks the server-side repo match step.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
|
|
||
| /// Pure-function renderer. Takes the raw API data and produces the Markdown | ||
| /// output. Lives apart from the network layer so it can be unit-tested. | ||
| fn render_instructions(policies: &[PolicyListItem], settings: &RepoSettings) -> String { |
There was a problem hiding this comment.
render_instructions has nested collection + guarded large rendering blocks; consider using early returns/guard clauses to simplify flow (collect -> early-return for empty, then render) to reduce nesting and improve readability.
Details
✨ AI Reasoning
Function collects policy-derived requirements into buckets then renders long Markdown sections guarded by boolean checks. The main rendering code is nested behind checks (has_session_section / has_window_section) and loops over the collected vectors — these guards and loops create multi-level nesting and make the function body harder to follow. Early returns / guard clauses could reduce indentation and separate data-collection from rendering more clearly, improving readability and maintainability.
🔧 How do I fix it?
Place parameter validation and guard clauses at the function start. Use early returns to reduce nesting levels and improve readability.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| .unwrap_or_else(|| "unknown".into()) | ||
| } | ||
|
|
||
| pub async fn run(project_root: &Path) -> Result<(), Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
Function run is too vague for its orchestration role; rename to a descriptive name like run_agent_policies_command or render_agent_policies to clarify its purpose.
| pub async fn run(project_root: &Path) -> Result<(), Box<dyn std::error::Error>> { | |
| pub async fn run_agent_policies_command(project_root: &Path) -> Result<(), Box<dyn std::error::Error>> { |
Details
✨ AI Reasoning
A newly added top-level command entrypoint is named with a single, generic verb. While the file provides module-level documentation describing its overall role, the function name itself ('run') does not convey what it runs (fetch+render agent policies). The function orchestrates multiple responsibilities (credential resolution, repo lookup, concurrent API calls, rendering and printing), so a more specific name would make intent clearer and reduce cognitive load when reading call-sites and command dispatch code.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| let (verb, succeed) = match self.action { | ||
| Action::Block => ("must be called", "must succeed"), | ||
| Action::Warn => ("should be called", "should succeed"), | ||
| Action::Allow => unreachable!(), |
There was a problem hiding this comment.
The match arm calling unreachable!() for Action::Allow in ToolRequirement::render_line can never be reached because the method returns earlier when action == Action::Allow; remove the unreachable arm.
| Action::Allow => unreachable!(), |
Details
✨ AI Reasoning
A guard at the start of render_line returns immediately when self.action == Action::Allow, so subsequent match over self.action can never reach the Action::Allow branch. This is dead/unreachable code introduced in the new renderer and should be removed to avoid confusion and untestable code paths.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
|
Closing — restarting with server-side rendering instead of CLI-side. New PR coming with proper endpoint + UI preview. |
Closes #172.
What
Adds
tracevault agent-policies— a CLI command (and matchingagent_policiesMCP tool) that fetches the active policies for the current repo and renders Markdown instructions describing what the agent needs to do before pushing.Why
CLAUDE.mdcurrently requires manual translation of Visdom Trace policies into agent instructions. When policies change in the dashboard,CLAUDE.mddrifts and needs manual updates. Withagent-policies,CLAUDE.mdstays a one-liner: "runtracevault agent-policiesand follow what it says." Output reflects the current policies automatically.Sample output
For this repo's current policies:
Behaviour rules
RequiredToolCallandConditionalToolCallconditions are rendered — other condition types (TokenBudget,TraceCompleteness, etc.) are server-evaluated without agent action and are skipped.action = block_push→ "must be called" / "must succeed";action = warn→ "should be called" / "should succeed";action = allow→ bare tool listing in the validation window section.validation_window_mode = 'disabled'.mcp__cargo__cargo_fmt) — the user is responsible for having the matching tools installed in their agent.Implementation
crates/tracevault-cli/src/commands/agent_policies.rs(CLI-side, not server-side — keeps server stateless and lets the rendering be agent-format-agnostic).list_policiesandget_repo_settings(the latter needed forvalidation_window_mode).tracevaultCLI rather than duplicating the rendering logic in TypeScript — one rendering implementation to maintain.Manual verification
Ran against the local instance for the tracevault repo — output matches expectations (see Sample output above).