Skip to content

feat: agent-policies — server-rendered policy instructions for agents#200

Merged
hashedone merged 6 commits into
mainfrom
feat/agent-policies
May 26, 2026
Merged

feat: agent-policies — server-rendered policy instructions for agents#200
hashedone merged 6 commits into
mainfrom
feat/agent-policies

Conversation

@hashedone
Copy link
Copy Markdown
Contributor

Closes #172.

What

Adds three coordinated surfaces, all backed by a single server-side renderer in tracevault-core:

  1. CLI: tracevault agent-policies — prints rendered Markdown to stdout.
  2. MCP: agent_policies tool — returns the same Markdown to the calling agent.
  3. Dashboard: "Preview" button on each repo page, with Copy and Refresh actions.

Why

CLAUDE.md previously required manual translation of Visdom Trace policies into agent instructions. When policies changed in the dashboard, CLAUDE.md drifted. Now CLAUDE.md is a one-liner:

Before starting any session, run tracevault agent-policies (or call the agent_policies MCP tool) and follow the instructions it returns.

The dashboard preview lets repo owners see exactly what their agents will be told.

Architecture

  • tracevault-core::agent_policies — pure renderer fn render_markdown(&[PolicyRule], &ValidationWindowMode) -> String. No I/O, no async. 12 unit tests cover session/window scopes, block/warn/allow actions, must_succeed combinations, conditional file patterns, empty repos, validation_window_mode interactions, scope=Both, and non-actionable conditions.
  • New endpoint GET /api/v1/orgs/{slug}/repos/{repo_id}/policies/agent-instructions returns { format: "markdown", content: "..." }. Lives in api::policies::get_agent_instructions. Parses DB rows into core types and calls the renderer.
  • CLI command is now a thin fetch+print. No rendering logic on the client.
  • MCP tool fetches via HTTP directly — no shell-out to the CLI, no need for tracevault to be on PATH. Requires repo_id in .tracevault/config.toml (already there post-tracevault init).
  • UI: new AgentInstructionsPreview.svelte component rendered on the repo detail page right after the Policies section.

Behaviour rules

  • Only RequiredToolCall and ConditionalToolCall are rendered — other condition types (TokenBudget, etc.) are server-evaluated without agent action and skipped.
  • Disabled policies are ignored.
  • action = BlockPush → "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 false claims about push blocking — language reflects the actual per-policy action and window mode.
  • Empty repo → terse "No active policies for this repository." message.
  • Tool names are emitted literally (e.g. mcp__cargo__cargo_fmt).
  • Fails fast with clear messages if not logged in / not initialised / repo not found.

Sample output

For the local fake-project repo's 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_check` — must be called and must succeed
- `Read` — should be called at least once

### 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__cargo__cargo_fmt` — must be called and must succeed
- `Read` — should be called at least once

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

Verification

  • cargo fmt + cargo check + tests: 24 + 24 + 89 + 128 pass.
  • pnpm run check: 0 errors, 0 warnings.
  • End-to-end manually verified against local server: endpoint returns 200, CLI prints expected output, UI Preview button reveals matching Markdown.

Replaces

Closed #199 (CLI-side rendering) in favour of this server-side approach.

Adds `tracevault agent-policies` CLI, `agent_policies` MCP tool, and a
dashboard preview, all backed by a single server-side renderer in
tracevault-core. Agents fetch Markdown instructions describing which
tools to call before pushing, which must succeed, what file patterns
trigger conditional tools, and how the validation window works. When
policies change, the rendered output updates automatically — no manual
CLAUDE.md sync required.

Architecture:
- `tracevault-core::agent_policies::render_markdown` — pure Markdown
  renderer over (Vec<PolicyRule>, ValidationWindowMode). 12 unit tests
  cover all rendering branches.
- New endpoint `GET /api/v1/orgs/{slug}/repos/{repo_id}/policies/agent-instructions`
  returns `{ format, content }`.
- CLI command is a thin fetch+print.
- MCP tool fetches via HTTP — no shell-out to CLI required, but does
  need `repo_id` from .tracevault/config.toml.
- UI: 'Preview' button on each repo page with Copy/Refresh actions.

Closes #172.
try {
await navigator.clipboard.writeText(content);
} catch (err) {
console.warn('Clipboard write failed:', err);
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.

console.warn('Clipboard write failed:', err) is ad-hoc debug output; remove or replace with structured logging / user-visible error handling.

Suggested change
console.warn('Clipboard write failed:', err);
// Silently handle clipboard errors
Details

✨ AI Reasoning
​A new ad-hoc logging call (console.warn) was added inside the copyToClipboard error handler. Ad-hoc console output is commonly used for debugging but may be undesired in production UI code. This can leak implementation details to the browser console and is better replaced by a user-facing notification or routed through a structured logging/telemetry system. The change introduces the statement that wasn't present before and therefore worsens the code quality by leaving a debug artifact.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Ok(Json(policies))
}

/// Response body for the agent-instructions endpoint.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

New agent-instructions endpoint and response struct enlarge this already large controller; consider moving AgentInstructionsResponse and get_agent_instructions into a smaller focused module.

Details

✨ AI Reasoning
​The diff adds a new public response struct AgentInstructionsResponse and the get_agent_instructions handler (lines ~99-164). This pushes the module past 800 lines and introduces a distinct responsibility (server-side agent-instruction rendering orchestration) alongside existing policy CRUD, checks, and evaluation logic. Large controller files that bundle multiple API surfaces and data-shaping logic become harder to navigate, test, and maintain. The problematic change is the addition of a new API endpoint and related data conversion code inside the same large file rather than colocating it with smaller, cohesive modules.

🔧 How do I fix it?
Split large files into smaller, focused modules. Each file should have a single responsibility.

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 {
ActionTag::Block => ("must be called", "must succeed"),
ActionTag::Warn => ("should be called", "should succeed"),
ActionTag::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 ActionTag::Allow match arm is unreachable due to the preceding if-return guard; remove the unreachable!() arm to avoid dead code and potential panic.

Suggested change
ActionTag::Allow => unreachable!(),
Details

✨ AI Reasoning
​A match arm contains an unreachable!() call but the code already returns when self.action == ActionTag::Allow. The ActionTag::Allow arm of the match can never be reached at runtime given the preceding if-return guard, so that match arm is dead/unnecessary and may confuse readers or trigger a panic if code is refactored incorrectly.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment on lines +17 to +23
async function toggle() {
if (open) {
open = false;
return;
}
open = true;
if (content !== null) return; // cached
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In toggle(), check for cached content before setting open=true to avoid unnecessary state change and flatten control flow; invert the guard so the cache path returns early.

Show fix
Suggested change
async function toggle() {
if (open) {
open = false;
return;
}
open = true;
if (content !== null) return; // cached
async function toggle() {
if (content !== null) {
open = true;
return; // cached
}
open = true;
Details

✨ AI Reasoning
​toggle() currently sets open = true before checking whether content is already cached. That orders a side-effect (showing the UI) before a lightweight guard check (content !== null). Moving the cache guard earlier would avoid changing component state unnecessarily and makes the success path flatter and clearer.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

- Extract agent-instructions endpoint into its own api/agent_instructions.rs
  module so api/policies.rs doesn't keep growing
- Remove unreachable!() match arm in core renderer by returning early in
  the match's Allow arm
- Replace ad-hoc console.warn for clipboard failures with user-visible
  copyStatus state (Copy → Copied / Copy failed)
- Reshape toggle() so the cached path returns early instead of falling
  through to the fetch branch
- Extract shared fetchContent() helper to avoid duplication between
  toggle and refresh paths
// the whole render.
let rules: Vec<tracevault_core::policy::PolicyRule> = rows
.into_iter()
.filter_map(|r| {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Anonymous closure passed to filter_map performs multi-step parsing and mapping. Extract into a named helper (e.g., map_row_to_policy_rule) to clarify intent and simplify testing.

Details

✨ AI Reasoning
​An anonymous closure passed to filter_map performs multiple distinct operations: deserializing condition, action, scope, severity, and constructing a PolicyRule. This bundles parsing/validation/mapping into an unnamed lambda, which hides business logic and would be clearer as a named helper function. Extracting it improves testability and readability without large refactor.

🔧 How do I fix it?
Extract complex anonymous functions into named functions with descriptive names, or add explanatory comments for their purpose.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

<div class="flex gap-2">
{#if open && content}
<Button variant="outline" size="sm" onclick={copyToClipboard}>
{copyStatus === 'copied' ? 'Copied' : copyStatus === 'failed' ? 'Copy failed' : 'Copy'}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nested ternary at the button label makes intent hard to read; extract into a small computed variable or function that returns the label text.

Details

✨ AI Reasoning
​The Svelte template uses a nested ternary to select copy button text based on copyStatus. Nested ternaries are hard to parse in templates and reduce readability; a small helper function or computed variable would be clearer.

🔧 How do I fix it?
Break long lines to enhance clarity. Aim for a maximum of 80-120 characters per line, depending on the context and coding standards.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment on lines +46 to +69
let rules: Vec<tracevault_core::policy::PolicyRule> = rows
.into_iter()
.filter_map(|r| {
let condition: tracevault_core::policy::PolicyCondition =
serde_json::from_value(r.condition).ok()?;
let action: tracevault_core::policy::PolicyAction =
serde_json::from_value(serde_json::Value::String(r.action)).ok()?;
let scope: tracevault_core::policy::PolicyScope =
serde_json::from_value(serde_json::Value::String(r.scope)).ok()?;
let severity: tracevault_core::policy::PolicySeverity =
serde_json::from_value(serde_json::Value::String(r.severity)).ok()?;
Some(tracevault_core::policy::PolicyRule {
id: r.id,
org_id: Some(r.org_id.to_string()),
name: r.name,
description: r.description,
condition,
action,
severity,
enabled: r.enabled,
scope,
})
})
.collect();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This will fix the Don't overuse undocumented anonymous functions issue detected on line: 48.

Show Fix

Aikido AutoFix Patch Suggestion - low confidence
This patch mitigates anonymous closure complexity by extracting the filter_map callback into a named helper function map_row_to_policy_rule that explicitly describes the row-to-rule parsing and mapping logic.

Suggested change
let rules: Vec<tracevault_core::policy::PolicyRule> = rows
.into_iter()
.filter_map(|r| {
let condition: tracevault_core::policy::PolicyCondition =
serde_json::from_value(r.condition).ok()?;
let action: tracevault_core::policy::PolicyAction =
serde_json::from_value(serde_json::Value::String(r.action)).ok()?;
let scope: tracevault_core::policy::PolicyScope =
serde_json::from_value(serde_json::Value::String(r.scope)).ok()?;
let severity: tracevault_core::policy::PolicySeverity =
serde_json::from_value(serde_json::Value::String(r.severity)).ok()?;
Some(tracevault_core::policy::PolicyRule {
id: r.id,
org_id: Some(r.org_id.to_string()),
name: r.name,
description: r.description,
condition,
action,
severity,
enabled: r.enabled,
scope,
})
})
.collect();
fn map_row_to_policy_rule(
r: crate::repo::policies::PolicyRow,
) -> Option<tracevault_core::policy::PolicyRule> {
let condition: tracevault_core::policy::PolicyCondition =
serde_json::from_value(r.condition).ok()?;
let action: tracevault_core::policy::PolicyAction =
serde_json::from_value(serde_json::Value::String(r.action)).ok()?;
let scope: tracevault_core::policy::PolicyScope =
serde_json::from_value(serde_json::Value::String(r.scope)).ok()?;
let severity: tracevault_core::policy::PolicySeverity =
serde_json::from_value(serde_json::Value::String(r.severity)).ok()?;
Some(tracevault_core::policy::PolicyRule {
id: r.id,
org_id: Some(r.org_id.to_string()),
name: r.name,
description: r.description,
condition,
action,
severity,
enabled: r.enabled,
scope,
})
}
let rules: Vec<tracevault_core::policy::PolicyRule> =
rows.into_iter().filter_map(map_row_to_policy_rule).collect();

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

hashedone added 3 commits May 26, 2026 13:07
PolicyAction already represents what ActionTag was duplicating. Adding
PartialEq + Eq + Copy (free derives on a unit-variant enum) lets the
renderer match and compare on PolicyAction directly without a parallel
local enum.
…on_*

'has_window' was ambiguous (what window?). The renamed identifiers
make it clear these track the validation window section.
- Pull the PolicyRow -> PolicyRule conversion out of the filter_map
  closure into a named map_row_to_policy_rule helper for clarity
- Replace nested ternary in Svelte template with a $derived copyLabel
Comment on lines +18 to +20
const copyLabel = $derived(
copyStatus === 'copied' ? 'Copied' : copyStatus === 'failed' ? 'Copy failed' : 'Copy'
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The $derived initializer uses a nested ternary (copyStatus === 'copied' ? ... : copyStatus === 'failed' ? ... : ...). Replace with a small helper or explicit if/else for clarity.

Show fix
Suggested change
const copyLabel = $derived(
copyStatus === 'copied' ? 'Copied' : copyStatus === 'failed' ? 'Copy failed' : 'Copy'
);
function getCopyLabel(status: 'idle' | 'copied' | 'failed'): string {
if (status === 'copied') return 'Copied';
if (status === 'failed') return 'Copy failed';
return 'Copy';
}
const copyLabel = $derived(getCopyLabel(copyStatus));
Details

✨ AI Reasoning
​A single expression composes multiple conditional branches using nested ternary operators. Nested ternaries increase cognitive load and make the expression harder to read and maintain. Extracting into a small function or using a clearer conditional structure will improve readability.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@hashedone hashedone merged commit ac06872 into main May 26, 2026
1 check passed
hashedone added a commit that referenced this pull request May 26, 2026
- Extract agent-instructions endpoint into its own api/agent_instructions.rs
  module so api/policies.rs doesn't keep growing
- Remove unreachable!() match arm in core renderer by returning early in
  the match's Allow arm
- Replace ad-hoc console.warn for clipboard failures with user-visible
  copyStatus state (Copy → Copied / Copy failed)
- Reshape toggle() so the cached path returns early instead of falling
  through to the fetch branch
- Extract shared fetchContent() helper to avoid duplication between
  toggle and refresh paths
@github-actions github-actions Bot mentioned this pull request May 26, 2026
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