Skip to content

feat(cli): add tracevault agent-policies command#199

Closed
hashedone wants to merge 1 commit into
mainfrom
feat/agent-policies-cli
Closed

feat(cli): add tracevault agent-policies command#199
hashedone wants to merge 1 commit into
mainfrom
feat/agent-policies-cli

Conversation

@hashedone
Copy link
Copy Markdown
Contributor

Closes #172.

What

Adds tracevault agent-policies — a CLI command (and matching agent_policies MCP 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.md currently requires manual translation of Visdom Trace policies into agent instructions. When policies change in the dashboard, CLAUDE.md drifts and needs manual updates. With agent-policies, CLAUDE.md stays a one-liner: "run tracevault agent-policies and follow what it says." Output reflects the current policies automatically.

Sample output

For this repo's current policies:

## Visdom Trace — agent policy instructions

These instructions reflect the active policies for this repository. They take precedence over any manual instructions elsewhere.

### Before push
The following pre-push checks apply to this repository:
- `mcp__cargo__cargo_fmt` — should be called at least once
- `mcp__cargo__cargo_check` — must be called and must succeed
- `mcp__cargo__cargo_audit` — when files matching `Cargo.lock` are changed, should be called and should succeed

### Validation window (pre-push gating)
A validation window restricts which tools can be called before push, gating the push on a clean validation run. Before pushing, open a validation window:

    tracevault validation-start

The window stays open until you push, or until you open a new window. Opening a new window invalidates the prior one.

Required tools (must be called inside the window):
- `mcp__review__agent_review` — when files matching `**/*.rs` are changed, should be called

Allowed tools (may be called freely inside the window):
- `Read`

If you need to call additional tools after opening the window, open a new window afterwards and rerun all required tools.

Behaviour rules

  • Only RequiredToolCall and ConditionalToolCall conditions are rendered — other condition types (TokenBudget, TraceCompleteness, etc.) are server-evaluated without agent action and are skipped.
  • Disabled policies are ignored.
  • 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 section is hidden entirely when validation_window_mode = 'disabled'.
  • No statements about "push will be blocked" are emitted — those depend on per-policy action + window mode and would be false for some setups, so we don't claim them.
  • Empty repo (no actionable policies) → terse "No active policies for this repository." message.
  • Tool names are emitted literally (e.g. mcp__cargo__cargo_fmt) — the user is responsible for having the matching tools installed in their agent.
  • Fails fast with clear messages if not logged in / not initialised / repo not found on server.

Implementation

  • Rendering lives in crates/tracevault-cli/src/commands/agent_policies.rs (CLI-side, not server-side — keeps server stateless and lets the rendering be agent-format-agnostic).
  • API client gets two new methods: list_policies and get_repo_settings (the latter needed for validation_window_mode).
  • MCP tool shells out to the installed tracevault CLI rather than duplicating the rendering logic in TypeScript — one rendering implementation to maintain.
  • 13 unit tests cover all rendering branches: session/window scopes, block/warn/allow actions, must_succeed combinations, conditional file patterns, empty repos, validation_window_mode interactions, scope=both, unknown condition types.

Manual verification

Ran against the local instance for the tracevault repo — output matches expectations (see Sample output above).

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())
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repo name extraction splits only on '/'; Windows paths use '\', so repo_name becomes wrong and repo lookup can fail consistently on Windows.

Suggested change
.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 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>> {
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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!(),
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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

@hashedone
Copy link
Copy Markdown
Contributor Author

Closing — restarting with server-side rendering instead of CLI-side. New PR coming with proper endpoint + UI preview.

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.

feat(cli): tracevault claude-policies — generate CLAUDE.md instructions from active repo policies

1 participant