feat(mcp): vox_tool_search — progressive tool disclosure - #263
Merged
Conversation
…-core slice + vox-langtool Hardening pass from the install/upgrade/release audit (docs/plans/INSTALL-RELEASE-AUDIT.md): - Phase 0.1: standardize Rust on 1.96.0 across rust-toolchain.toml(existing), contracts/toolchain SSoT, Cargo workspace MSRV, and both CI Dockerfiles; the ci.yml Toolchain SSoT Drift Guard now passes. - Phase 0.2: add .github/workflows/version-tag-guard.yml asserting the workspace version matches the pushed v* tag before release artifacts build. - Phase 0.3: .actrc cross-platform artifact path (./.act-artifacts, git-ignored) + document that act cannot run windows/macos jobs. - Phase 4.1: gate vox-db/vox-gamify/vox-repository behind a default-on `db` feature in vox-cli-core; cfg-gate benchmark_telemetry/gamify_shim/ workflow_journal_codex and the vox_db test in scientia. Backward compatible (consumers keep db via default features). Both check lanes verified. - Phase 4.2: new crates/vox-langtool — minimal DB-free CLI (check/fmt/run/build) wrapping the language core; opts out of `db` via a direct path dep so the workspace default and other consumers are untouched. cargo tree clean of vox-db/orchestrator/search/populi/gamify/lsp/cli; 10/10 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…elease-binaries - Phase 4.1b: make vox-db/vox-gamify optional behind a default-on `db` feature in vox-lsp; cfg-gate cached_project_db, the Ludus telemetry block, and the now db-only Arc/OnceLock imports + err_n/warn_n counts in main.rs. Both lanes verified: `cargo check -p vox-lsp` (db on) and `--no-default-features` (vox-db/ vox-gamify absent from cargo tree). Lib validation path was always db-free, so consumers are unaffected. Enables embedding the LSP in a minimal toolchain. - release-binaries.yml: add an SPDX SBOM step (anchore/sbom-action) to the publish job, guarded with continue-on-error + fail_on_unmatched_files:false so it can never block a release. CI-unverified (runs only on a v* tag). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements rank_tools keyword ranking over TOOL_REGISTRY (name-segment exact 8 / name substring 4 / description 1; score desc, name asc, limit) plus the vox_tool_search handler returning name+description+input_schema per hit. Adds tool.search to the operations catalog (regenerated tool-registry.canonical.yaml via operations-sync), dispatch arm, strict derived input schema, and handler tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request coordinates six major initiatives: a Rust 1.96 version bump across build infrastructure, GitHub Actions enhancements for release automation and tag validation, database-feature gating for minimal deployments (vox-cli-core and vox-lsp), a new DB-free vox-langtool CLI with check/fmt/run/build subcommands, MCP progressive tool search with keyword ranking, and substantial MENS training improvements including BF16 activation dtypes, explicit gradient clipping, VRAM sizing precedence ordering, per-node GPU inventory, a federated cohort planner, and a resilient launcher script. ChangesMulti-track infrastructure, CLI modularity, tool discovery, MENS improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ox_tool_search operations-sync capability, gui-surface-registry (classify repl as tier none), capability-sync model manifest, regenerate plugin catalog docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… docs with branch binary The pre-push hook runs the branch's own vox-cli; prior artifacts were written by a newer installed binary and differed in format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
patches/qlora-rs-1.0.5/src/training.rs (1)
929-935:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
training_step_lmdoes not apply gradient clipping.
backward_stepandtraining_stepwere updated to explicitly callbackward(), applyclip_grad_norm, thenstep(). However,training_step_lm(line 931) still usesoptimizer.backward_step(&scaled_loss)?which bundles backward+step without the clipping hook.If gradient clipping is important for training stability (which it typically is for LM fine-tuning), this path should be updated to match the others.
Proposed fix
if let Some(ref mut optimizer) = self.optimizer { if self.accumulation_step >= accum_steps { - optimizer.backward_step(&scaled_loss)?; + let mut grads = scaled_loss.backward()?; + if let Some(max_norm) = self.config.adapter_config.max_grad_norm { + let vars = self.varmap.all_vars(); + clip_grad_norm(&mut grads, &vars, max_norm)?; + } + optimizer.step(&grads)?; self.accumulation_step = 0; } else { let _ = scaled_loss.backward(); }🤖 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 `@patches/qlora-rs-1.0.5/src/training.rs` around lines 929 - 935, The LM training path in training_step_lm still calls optimizer.backward_step(&scaled_loss)? which performs backward+step without applying gradient clipping; update the block handling Some(ref mut optimizer) so it instead calls scaled_loss.backward() (or optimizer.backward(&scaled_loss) if available), then call clip_grad_norm on self.model parameters with the configured max_grad_norm, and finally call optimizer.step() and optimizer.zero_grad() (or optimizer.post_step() if API differs), preserving accumulation logic (use self.accumulation_step and accum_steps as currently used) and ensure self.accumulation_step is reset after stepping; replace the optimizer.backward_step call with these explicit backward, clip_grad_norm, step, zero_grad calls to match training_step and backward_step behavior.
🧹 Nitpick comments (1)
docs/plans/INSTALL-RELEASE-AUDIT.md (1)
286-293: ⚡ Quick winKeep install/release automation in
.vox.These phases still propose new
.sh/.ps1glue, but repo policy says project automation must live in.voxfiles and run viavox run. Recast the bootstrap and release steps as thin.voxlaunchers or existing.voxwrappers instead.As per coding guidelines, all project automation must use
.voxfiles executed viavox runinstead of creating new.ps1,.sh, or.pyscripts.Also applies to: 321-325
🤖 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 `@docs/plans/INSTALL-RELEASE-AUDIT.md` around lines 286 - 293, The docs propose adding new shell/PowerShell scripts and changes in install.rs (notably the install precondition at install.rs:102-107) but repo policy mandates automation live as .vox tasks run via `vox run`; update the plan to replace `scripts/install.sh` and `scripts/install.ps1` with thin `.vox` launchers or existing `.vox` wrappers that invoke the POSIX/PowerShell steps, and change the `voxup`/install.rs notes (including the fetch/verify/link flow and removal of the "binary must already exist" precondition) to describe implementing those steps inside .vox tasks (and calling the Rust install code from a `.vox` task) so all bootstrap/release automation is executed via `vox run` with idempotent PATH edits described.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.
Inline comments:
In @.github/workflows/version-tag-guard.yml:
- Line 30: Replace the runner selection for this workflow: change the runs-on
value currently set to "ubuntu-latest" to the required self-hosted matrix form
runs-on: [self-hosted, linux, x64] so the job uses the repository's Basic Linux
self-hosted runner; update the runs-on entry in the workflow (the runs-on key)
accordingly.
- Line 32: Pin the third‑party GitHub Actions to immutable SHAs and harden
checkout credentials: replace the floating uses: actions/checkout@v4 entry with
a pinned full commit SHA (uses: actions/checkout@<full-sha>) and add a with:
persist-credentials: false block to the checkout step (unless the workflow
actually needs authenticated git operations), and likewise pin
anchore/sbom-action (uses: anchore/sbom-action@<full-sha>) in
release-binaries.yml; ensure you update the uses strings to full commit SHAs and
add persist-credentials: false to the checkout steps to prevent credential
leakage.
In `@crates/vox-cli-core/src/lib.rs`:
- Around line 18-22: The module declaration for scientia is currently
unconditional and must be feature-gated to match the DB-gated surface; change
the unguarded "pub mod scientia;" to a DB-gated declaration (e.g. #[cfg(feature
= "db")] pub mod scientia;) or alternatively move any non-DB code out of
scientia into a separate always-on module and keep DB-only wiring in a
#[cfg(feature = "db")] scientia module so building with --no-default-features
succeeds; look for the existing module symbols "scientia", "gamify_shim", and
"workflow_journal_codex" to align gating consistently.
In `@crates/vox-langtool/src/commands/build.rs`:
- Around line 7-47: Add a same-file test module for the new public entrypoint
`pub fn run` by adding a `#[cfg(test)] mod tests` block in this file that
contains at least one `#[test]` (or `#[tokio::test]` if async) which exercises
`run`; create a temporary directory and a temporary input file (with content
that makes `crate::is_script_like` predictable), call `run(&path, &out_dir)`,
assert it returns Ok(()), and check expected output files exist and contain the
generated content (and also add a negative test that feeds a file producing
frontend errors and asserts `run` returns an Err). Ensure tests reference the
function name `run`, the `PipelineOptions` behavior indirectly via input
contents, and clean up temporary files.
In `@crates/vox-langtool/src/commands/check.rs`:
- Around line 8-40: Add a same-file test module to satisfy the
skeleton/untested-pub-api rule: create a #[cfg(test)] mod tests in
crates/vox-langtool/src/commands/check.rs that includes a #[test] (or
#[tokio::test]) which writes a minimal valid source to a temp file (using
tempfile::NamedTempFile or std::fs::write to a temp path), calls the public
run(&Path) function, and asserts the Result is Ok (or checks the expected
error/warning counts). Ensure the test imports run and any needed helpers and
cleans up the temp file so the test is self-contained.
In `@crates/vox-langtool/src/commands/fmt.rs`:
- Around line 30-37: The non-Unix branch currently removes the destination file
before attempting std::fs::rename, which risks permanently deleting the user's
original if rename fails; in the cfg(not(unix)) block (the code using path, tmp,
std::fs::remove_file and std::fs::rename) remove the pre-rename remove_file call
and simply call std::fs::rename(&tmp, path) directly (or implement a safe
backup-and-restore strategy) so the original file is not deleted unless the
rename succeeds.
In `@crates/vox-langtool/src/commands/run.rs`:
- Around line 10-58: Add a same-file test block for the new public function run
to satisfy the crate's testing policy: create a #[cfg(test)] mod tests with a
#[test] that writes a minimal Vox source to a temporary file, calls the public
function run(file_path, &[]), and asserts it returns Ok(()) (optionally verify
expected stdout or side-effects); reference the function signature pub fn
run(file: &Path, _args: &[String]) -> Result<()> and ensure the test imports any
needed items (Path/TempDir/tempfile) so the test compiles and lives in the same
source file as run.
In `@crates/vox-langtool/src/lib.rs`:
- Around line 5-18: is_script_like currently just substring-matches decorators
and misclassifies real module syntax and things inside comments/strings; update
is_script_like to first strip/ignore comments and string literals, then check
for module-level syntax patterns instead of raw substrings: detect component
declarations (e.g. regex like \bcomponent\s+\w+\s*\() and workflow blocks (e.g.
\bworkflow\s*\{), while still checking for true decorators like
`@page/`@query/@mutation/@server/@table; replace the app_markers-only approach in
is_script_like with this comment/string-stripping + token/regex checks so
comments or string contents no longer flip the result and real module forms are
classified correctly.
In `@crates/vox-langtool/tests/integration.rs`:
- Around line 107-115: The test run_caps_directive_exits_ok currently only
prints and doesn't verify that the parsed caps are enforced or stored; update
the test or fixture so it exercises capability-gated behavior: modify
caps_directive.vox to perform an operation that requires one of the declared
caps (e.g., attempt a network or filesystem access) and assert that
vox_langtool::commands::run::run either succeeds when the declared cap is
present or fails when it is absent, or alternatively extend the test to inspect
the interpreter state after run (via the API that exposes parsed capabilities)
to assert the parsed caps are present; locate the test function
run_caps_directive_exits_ok and the fixture caps_directive.vox and change the
fixture or add assertions accordingly so the test actually validates capability
parsing/enforcement.
- Around line 38-95: Replace the fragile/skipping assertions in tests
fmt_check_on_already_formatted_exits_ok, fmt_rewrites_unformatted_file, and
fmt_check_on_unformatted_fails so they exercise the formatter deterministically:
use a known-good canonical fixture (e.g. formatted.vox) instead of
fixture("hello.vox"), assert explicitly that vox_compiler::fmt::try_format(...)
returns Ok and equals the fixture before calling
vox_langtool::commands::fmt::run(..., true) in
fmt_check_on_already_formatted_exits_ok; in fmt_rewrites_unformatted_file write
a non-canonical snippet, call run(&path, false), then read the file back and
assert its contents equal vox_compiler::fmt::try_format(unformatted).unwrap();
and in fmt_check_on_unformatted_fails assert try_format(unformatted) is Ok and
different from the original then assert run(&path, true) returns Err so no
behavior is silently skipped.
In `@crates/vox-plugin-mens-candle-cuda/src/candle_qlora_train/mod.rs`:
- Around line 42-51: Replace direct std::env::var_os reads with
vox_secrets::resolve_secret(...) so env access follows project policy: in
activation_compute_dtype(), call vox_secrets::resolve_secret("VOX_MENS_ACT_F32")
and check the returned Option (is_some()) instead of std::env::var_os,
preserving the same F32/BF16 branch behavior; do the same for the
VOX_MENS_NO_WEIGHT_CACHE usage (replace
std::env::var_os("VOX_MENS_NO_WEIGHT_CACHE") with
vox_secrets::resolve_secret("VOX_MENS_NO_WEIGHT_CACHE") and adapt the
conditional). Also, if these VOX_* variables are new, add entries for
VOX_MENS_ACT_F32 and VOX_MENS_NO_WEIGHT_CACHE to
contracts/config/env-vars.v1.yaml. Ensure the code compiles by importing
vox_secrets and handling any Result/Option types the resolver returns.
In `@crates/vox-populi/src/mens/cohort/planner.rs`:
- Around line 152-153: exclusion_reason currently calls
memory_budget::plan(node.vram_gib, target_params_b) and ignores the model
family, causing mismatch with the trainer which picks plan_qwen25coder,
plan_qwen35, or plan based on target_model; update exclusion_reason (and the
similar logic around the other occurrence) to accept/inspect target_model (or
accept a planner function) and call the same model-family selection used in
crates/vox-ml-cli/src/commands/mens/populi/train_arm.rs so it uses
plan_qwen25coder, plan_qwen35, or generic plan consistently with training;
ensure the function signature(s) and call sites (where exclusion_reason and the
second block are invoked) are updated to pass target_model (or the selected
planner) so cohort admission matches the trainer’s VRAM planner.
In
`@docs/src/architecture/mens-training-pipeline-audit-and-improvement-plan-2026-06-07.md`:
- Around line 1-5: Add the required frontmatter keys to the two architecture
Markdown files by inserting a YAML frontmatter block that includes at minimum
status (string) and training_eligible (boolean) at the top of each document
(e.g., for the "MENS training pipeline — audit + improvement scoping" doc add
status: "<appropriate-status>" and training_eligible: true/false); also remove
the hand-added last_updated field from docs/src/reference/mens-training.md so
the pipeline can derive it from git. Ensure the frontmatter is valid YAML,
placed between --- lines at the very top, and do not add any manual last_updated
entries.
In `@scripts/mens/train_resilient.vox`:
- Around line 10-24: The run_train function currently ignores forwarded CLI
flags so the model parameter remains empty; update run_train to parse and accept
forwarded CLI args (or use the program's argv) so a passed "--model <name>" is
detected and set into the model variable before building args; specifically,
modify logic around the model parameter in run_train (and the code paths
covering lines 46-61) to prefer any "--model" flag from the wrapper CLI/argv and
only fall back to the auto-scaler default when none is provided, then push that
model into the args array as currently done.
---
Outside diff comments:
In `@patches/qlora-rs-1.0.5/src/training.rs`:
- Around line 929-935: The LM training path in training_step_lm still calls
optimizer.backward_step(&scaled_loss)? which performs backward+step without
applying gradient clipping; update the block handling Some(ref mut optimizer) so
it instead calls scaled_loss.backward() (or optimizer.backward(&scaled_loss) if
available), then call clip_grad_norm on self.model parameters with the
configured max_grad_norm, and finally call optimizer.step() and
optimizer.zero_grad() (or optimizer.post_step() if API differs), preserving
accumulation logic (use self.accumulation_step and accum_steps as currently
used) and ensure self.accumulation_step is reset after stepping; replace the
optimizer.backward_step call with these explicit backward, clip_grad_norm, step,
zero_grad calls to match training_step and backward_step behavior.
---
Nitpick comments:
In `@docs/plans/INSTALL-RELEASE-AUDIT.md`:
- Around line 286-293: The docs propose adding new shell/PowerShell scripts and
changes in install.rs (notably the install precondition at install.rs:102-107)
but repo policy mandates automation live as .vox tasks run via `vox run`; update
the plan to replace `scripts/install.sh` and `scripts/install.ps1` with thin
`.vox` launchers or existing `.vox` wrappers that invoke the POSIX/PowerShell
steps, and change the `voxup`/install.rs notes (including the fetch/verify/link
flow and removal of the "binary must already exist" precondition) to describe
implementing those steps inside .vox tasks (and calling the Rust install code
from a `.vox` task) so all bootstrap/release automation is executed via `vox
run` with idempotent PATH edits described.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8df805d8-baf2-4c45-89b7-91749bc23cce
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockcrates/vox-gui/ui/src/generated/surfaceRegistry.generated.tsis excluded by!**/generated/**
📒 Files selected for processing (61)
.actrc.github/workflows/release-binaries.yml.github/workflows/version-tag-guard.yml.gitignoreCargo.tomlDockerfileDockerfile.ci-runnerREADME.mdcontracts/capability/capability-registry.yamlcontracts/capability/model-manifest.generated.jsoncontracts/gui/surface-registry.v1.yamlcontracts/mcp/http-read-role-governance.yamlcontracts/mcp/tool-registry.canonical.yamlcontracts/operations/catalog.v1.yamlcontracts/reports/gui-surface-coverage.v1.jsoncontracts/reports/gui-surface-registry.v1.jsoncontracts/reports/operations-catalog-inventory.v1.jsoncontracts/toolchain/workspace-toolchain.v1.yamlcrates/vox-cli-core/Cargo.tomlcrates/vox-cli-core/src/lib.rscrates/vox-cli-core/src/scientia.rscrates/vox-langtool/Cargo.tomlcrates/vox-langtool/src/commands/build.rscrates/vox-langtool/src/commands/check.rscrates/vox-langtool/src/commands/fmt.rscrates/vox-langtool/src/commands/mod.rscrates/vox-langtool/src/commands/run.rscrates/vox-langtool/src/lib.rscrates/vox-langtool/src/main.rscrates/vox-langtool/tests/fixtures/caps_directive.voxcrates/vox-langtool/tests/fixtures/formatted.voxcrates/vox-langtool/tests/fixtures/hello.voxcrates/vox-langtool/tests/fixtures/type_error.voxcrates/vox-langtool/tests/integration.rscrates/vox-lsp/Cargo.tomlcrates/vox-lsp/src/main.rscrates/vox-ml-cli/Cargo.tomlcrates/vox-ml-cli/src/commands/mens/populi/gpu_tests_body.rscrates/vox-ml-cli/src/commands/mens/populi/train_arm.rscrates/vox-orchestrator-mcp/src/dispatch.rscrates/vox-orchestrator-mcp/src/input_schemas.rscrates/vox-orchestrator-mcp/src/lib.rscrates/vox-orchestrator-mcp/src/params.rscrates/vox-orchestrator-mcp/src/tool_search.rscrates/vox-plugin-mens-candle-cuda/src/candle_qlora_train/mod.rscrates/vox-plugin-mens-candle-cuda/src/model.rscrates/vox-populi-types/src/node_record.rscrates/vox-populi/src/lib.rscrates/vox-populi/src/mens/cohort/mod.rscrates/vox-populi/src/mens/cohort/planner.rscrates/vox-populi/src/mens/mod.rscrates/vox-populi/src/transport/handlers/nodes.rsdocs/plans/INSTALL-RELEASE-AUDIT.mddocs/src/architecture/mens-training-pipeline-audit-and-improvement-plan-2026-06-07.mddocs/src/architecture/qwen-3.7-profile-and-mens-4b-feasibility-2026-06-07.mddocs/src/reference/mens-training.mddocs/superpowers/plans/2026-06-07-mens-4b-single-gpu-and-heterogeneous-mesh.mdinfra/ci-runner/Dockerfilepatches/qlora-rs-1.0.5/src/qlora.rspatches/qlora-rs-1.0.5/src/training.rsscripts/mens/train_resilient.vox
|
|
||
| jobs: | ||
| assert-version-matches-tag: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Use the required self-hosted label set for this Basic Linux workflow.
This job currently uses ubuntu-latest; repository policy requires runs-on: [self-hosted, linux, x64] for Basic Linux workflows unless it is one of the documented exceptions.
As per coding guidelines, “Use runs-on: [self-hosted, linux, x64] for Basic Linux GitHub Actions workflows.”
🤖 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 @.github/workflows/version-tag-guard.yml at line 30, Replace the runner
selection for this workflow: change the runs-on value currently set to
"ubuntu-latest" to the required self-hosted matrix form runs-on: [self-hosted,
linux, x64] so the job uses the repository's Basic Linux self-hosted runner;
update the runs-on entry in the workflow (the runs-on key) accordingly.
Source: Coding guidelines
| assert-version-matches-tag: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print relevant sections for the referenced workflows
files=(
".github/workflows/version-tag-guard.yml"
".github/workflows/release-binaries.yml"
)
for f in "${files[@]}"; do
if [ ! -f "$f" ]; then
echo "MISSING FILE: $f" >&2
continue
fi
echo "==== $f ===="
# Print around likely mentioned lines; also include any uses: lines
nl -ba "$f" | sed -n '1,220p' | sed -n '1p; /uses:/p;'
echo "---- context around line 32 (best effort) ----"
nl -ba "$f" | sed -n '20,50p'
echo "---- context around line 135 (best effort) ----"
nl -ba "$f" | sed -n '120,160p'
done
# Global search for unpinned uses: within these two workflows
echo "==== ACTION USES (all) in version-tag-guard.yml ===="
rg -n "^\s*-\s*uses:\s*[^#\s]+" .github/workflows/version-tag-guard.yml || true
echo "==== ACTION USES (all) in release-binaries.yml ===="
rg -n "^\s*-\s*uses:\s*[^#\s]+" .github/workflows/release-binaries.yml || true
# Check for checkout persistence setting
echo "==== checkout credential persistence settings ===="
rg -n "persist-credentials|persist_credentials" .github/workflows/version-tag-guard.yml .github/workflows/release-binaries.yml || trueRepository: vox-foundation/vox
Length of output: 174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
".github/workflows/version-tag-guard.yml"
".github/workflows/release-binaries.yml"
)
awk_print_range() {
local f="$1" start="$2" end="$3"
awk -v s="$start" -v e="$end" '{if(NR>=s && NR<=e) printf "%5d %s\n", NR, $0}' "$f"
}
for f in "${files[@]}"; do
echo "==== $f (exists? $(test -f "$f" && echo yes || echo no)) ===="
if [ ! -f "$f" ]; then
continue
fi
echo "---- context around line 32 ----"
awk_print_range "$f" 20 50
echo "---- context around line 135 ----"
awk_print_range "$f" 120 160
echo "---- all uses: lines in file ----"
rg -n "^[[:space:]]*-[[:space:]]uses:" "$f" || true
echo "---- uses: lines with version-like refs (not SHAs or v tags) ----"
rg -n "^[[:space:]]*-[[:space:]]uses:[[:space:]]*[^#[:space:]]+" "$f" || true
echo "---- checkout credential persistence settings (if any) ----"
rg -n "persist-credentials|persist_credentials" "$f" || true
echo
doneRepository: vox-foundation/vox
Length of output: 4767
🌐 Web query:
GitHub actions/checkout persist-credentials default value and security recommendation to set it to false
💡 Result:
The persist-credentials input for the GitHub actions/checkout action has a default value of true [1][2][3]. When set to true, the action configures the local git environment with the provided authentication token (commonly the GITHUB_TOKEN), which allows subsequent git commands (such as fetch, pull, or push) to run authenticated without additional configuration [1][3][4]. Security Recommendation It is widely recommended to set persist-credentials to false unless your workflow explicitly requires authenticated git operations (e.g., pushing commits back to the repository) [5]. Setting it to false is a security best practice for the following reasons: 1. Risk of Credential Exposure: By default, the authentication token is stored in the local git configuration of the checked-out repository [1][5][6]. If the workflow subsequently archives the repository directory as an artifact or if a malicious or compromised third-party action runs in the same environment, the token may be exposed or misused [5][6][7]. 2. Principle of Least Privilege: Disabling this option limits the window of opportunity for token misuse by ensuring that sensitive credentials are not unnecessarily persisted in the runner's file system [5]. To implement this security hardening, update your workflow step as follows: - uses: actions/checkout@v4 with: persist-credentials: false If you later determine that you need to perform authenticated git actions, you can manually authenticate using the token within the specific step, or re-enable the persistence only for the workflows that strictly require it [5].
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: https://github.com/actions/checkout/blob/72f2cec99f417b1a1c5e2e88945068983b7965f9/action.yml
- 3: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 4: actions/checkout@c170eef
- 5: https://actsense.dev/vulnerabilities/unsafe_checkout/
- 6: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 7: https://earthly.dev/lunar/guardrails/github-actions/checkout-no-persist-credentials/
Pin third-party GitHub Actions to immutable SHAs + harden checkout credentials.
.github/workflows/version-tag-guard.ymlusesactions/checkout@v4(line 32): pin to a full commit SHA and setwith: persist-credentials: falseunless authenticated git operations are required..github/workflows/release-binaries.ymlusesactions/checkout@v6(line 32) andanchore/sbom-action@v0(line 135): pin both to full commit SHAs.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 32-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 32-32: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/version-tag-guard.yml at line 32, Pin the third‑party
GitHub Actions to immutable SHAs and harden checkout credentials: replace the
floating uses: actions/checkout@v4 entry with a pinned full commit SHA (uses:
actions/checkout@<full-sha>) and add a with: persist-credentials: false block to
the checkout step (unless the workflow actually needs authenticated git
operations), and likewise pin anchore/sbom-action (uses:
anchore/sbom-action@<full-sha>) in release-binaries.yml; ensure you update the
uses strings to full commit SHAs and add persist-credentials: false to the
checkout steps to prevent credential leakage.
Source: Linters/SAST tools
Comment on lines
+18
to
22
| #[cfg(feature = "db")] | ||
| pub mod gamify_shim; | ||
| pub mod scientia; | ||
| #[cfg(feature = "db")] | ||
| pub mod workflow_journal_codex; |
There was a problem hiding this comment.
scientia still escapes the new db feature gate.
vox-cli-core is now advertised as buildable with --no-default-features, but pub mod scientia; remains unconditional between two DB-gated exports. If scientia.rs still carries the DB/repository wiring described in the PR objective, the DB-free slice will fail to compile. Gate scientia here as well, or split any non-DB pieces into a separate always-on module. Based on PR objectives, scientia was intended to be part of the DB-gated surface.
🤖 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 `@crates/vox-cli-core/src/lib.rs` around lines 18 - 22, The module declaration
for scientia is currently unconditional and must be feature-gated to match the
DB-gated surface; change the unguarded "pub mod scientia;" to a DB-gated
declaration (e.g. #[cfg(feature = "db")] pub mod scientia;) or alternatively
move any non-DB code out of scientia into a separate always-on module and keep
DB-only wiring in a #[cfg(feature = "db")] scientia module so building with
--no-default-features succeeds; look for the existing module symbols "scientia",
"gamify_shim", and "workflow_journal_codex" to align gating consistently.
Comment on lines
+7
to
+47
| pub fn run(file: &Path, out_dir: &Path) -> Result<()> { | ||
| let source = std::fs::read_to_string(file) | ||
| .with_context(|| format!("Failed to read {}", file.display()))?; | ||
|
|
||
| let options = PipelineOptions { | ||
| script_mode: crate::is_script_like(&source), | ||
| ..PipelineOptions::default() | ||
| }; | ||
|
|
||
| let result = run_frontend_str_with_options(&source, &file.to_string_lossy(), &options)?; | ||
|
|
||
| if result.has_errors() { | ||
| for diag in &result.diagnostics { | ||
| eprintln!("{:?}: {}", diag.severity, diag.message); | ||
| } | ||
| anyhow::bail!("Build failed with {} error(s)", result.error_count()); | ||
| } | ||
|
|
||
| let package_name = file.file_stem().and_then(|s| s.to_str()).ok_or_else(|| { | ||
| anyhow::anyhow!("Cannot derive package name from path: {}", file.display()) | ||
| })?; | ||
|
|
||
| let codegen_out = vox_codegen::codegen_rust::generate_script(&result.hir, package_name, None) | ||
| .map_err(|e| anyhow::anyhow!("Codegen failed: {e}"))?; | ||
|
|
||
| std::fs::create_dir_all(out_dir) | ||
| .with_context(|| format!("Failed to create out-dir {}", out_dir.display()))?; | ||
|
|
||
| for (filename, content) in &codegen_out.files { | ||
| let path = out_dir.join(filename); | ||
| if let Some(parent) = path.parent() { | ||
| std::fs::create_dir_all(parent)?; | ||
| } | ||
| std::fs::write(&path, content) | ||
| .with_context(|| format!("Failed to write {}", path.display()))?; | ||
| println!(" wrote {}", path.display()); | ||
| } | ||
|
|
||
| println!("Build complete -> {}", out_dir.display()); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Add same-file tests for the new public run entrypoint.
This file is over the 30-nonblank-line exemption and introduces a new pub fn run, but it has no local #[test], #[tokio::test], or #[cfg(test)] mod tests block. That trips the repo’s skeleton/untested-pub-api rule for crates/*/src/**. As per coding guidelines, Every new pub fn in crates/*/src/** (excluding main.rs, bin/, tests/, and files under 30 non-blank lines) requires at least one #[test], #[tokio::test], or #[cfg(test)] mod tests block in the same file before the commit lands.
🤖 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 `@crates/vox-langtool/src/commands/build.rs` around lines 7 - 47, Add a
same-file test module for the new public entrypoint `pub fn run` by adding a
`#[cfg(test)] mod tests` block in this file that contains at least one `#[test]`
(or `#[tokio::test]` if async) which exercises `run`; create a temporary
directory and a temporary input file (with content that makes
`crate::is_script_like` predictable), call `run(&path, &out_dir)`, assert it
returns Ok(()), and check expected output files exist and contain the generated
content (and also add a negative test that feeds a file producing frontend
errors and asserts `run` returns an Err). Ensure tests reference the function
name `run`, the `PipelineOptions` behavior indirectly via input contents, and
clean up temporary files.
Source: Coding guidelines
Comment on lines
+8
to
+40
| pub fn run(file: &Path) -> Result<()> { | ||
| let source = std::fs::read_to_string(file) | ||
| .map_err(|e| anyhow::anyhow!("Failed to read {}: {}", file.display(), e))?; | ||
|
|
||
| let options = PipelineOptions { | ||
| script_mode: crate::is_script_like(&source), | ||
| ..PipelineOptions::default() | ||
| }; | ||
|
|
||
| let result = run_frontend_str_with_options(&source, &file.to_string_lossy(), &options)?; | ||
|
|
||
| for diag in &result.diagnostics { | ||
| let level = match diag.severity { | ||
| TypeckSeverity::Error => "error", | ||
| TypeckSeverity::Warning => "warning", | ||
| }; | ||
| eprintln!("{}: {}", level, diag.message); | ||
| } | ||
|
|
||
| let error_count = result.error_count(); | ||
| let warning_count = result.warning_count(); | ||
|
|
||
| if result.has_errors() { | ||
| anyhow::bail!( | ||
| "Check failed with {} error(s) and {} warning(s)", | ||
| error_count, | ||
| warning_count | ||
| ); | ||
| } | ||
|
|
||
| println!("Check passed with {} warning(s)", warning_count); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Add same-file tests for the new public vox-langtool command entrypoints.
crates/vox-langtool/src/commands/check.rs and crates/vox-langtool/src/commands/fmt.rs both introduce a new pub fn run(...) in crates/*/src/** without a same-file test block, which violates the repo’s skeleton/untested-pub-api rule and will block merge.
As per coding guidelines: "Every new pub fn in crates/*/src/** (excluding main.rs, bin/, tests/, and files under 30 non-blank lines) requires at least one #[test], #[tokio::test], or #[cfg(test)] mod tests block in the same file before the commit lands; detected by the skeleton/untested-pub-api detector."
🤖 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 `@crates/vox-langtool/src/commands/check.rs` around lines 8 - 40, Add a
same-file test module to satisfy the skeleton/untested-pub-api rule: create a
#[cfg(test)] mod tests in crates/vox-langtool/src/commands/check.rs that
includes a #[test] (or #[tokio::test]) which writes a minimal valid source to a
temp file (using tempfile::NamedTempFile or std::fs::write to a temp path),
calls the public run(&Path) function, and asserts the Result is Ok (or checks
the expected error/warning counts). Ensure the test imports run and any needed
helpers and cleans up the temp file so the test is self-contained.
Source: Coding guidelines
Comment on lines
+107
to
+115
| fn run_caps_directive_exits_ok() { | ||
| // caps_directive.vox has `// vox:caps net fs` on the first line. | ||
| // The directive must be parsed and set on the interpreter without breaking execution. | ||
| let result = vox_langtool::commands::run::run(&fixture("caps_directive.vox"), &[]); | ||
| assert!( | ||
| result.is_ok(), | ||
| "run caps_directive.vox failed (caps parsing broke execution): {:?}", | ||
| result | ||
| ); |
There was a problem hiding this comment.
The caps test never exercises capability-gated behavior.
caps_directive.vox only prints, so this test still passes if // vox:caps ... parsing is ignored entirely. To protect the new feature, make the fixture perform an operation that requires one of the declared caps, or add a narrower test that asserts the parsed caps reach the interpreter state.
🤖 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 `@crates/vox-langtool/tests/integration.rs` around lines 107 - 115, The test
run_caps_directive_exits_ok currently only prints and doesn't verify that the
parsed caps are enforced or stored; update the test or fixture so it exercises
capability-gated behavior: modify caps_directive.vox to perform an operation
that requires one of the declared caps (e.g., attempt a network or filesystem
access) and assert that vox_langtool::commands::run::run either succeeds when
the declared cap is present or fails when it is absent, or alternatively extend
the test to inspect the interpreter state after run (via the API that exposes
parsed capabilities) to assert the parsed caps are present; locate the test
function run_caps_directive_exits_ok and the fixture caps_directive.vox and
change the fixture or add assertions accordingly so the test actually validates
capability parsing/enforcement.
Comment on lines
+42
to
+51
| fn activation_compute_dtype(device: &Device) -> DType { | ||
| if std::env::var_os("VOX_MENS_ACT_F32").is_some() { | ||
| return DType::F32; | ||
| } | ||
| if device.is_cuda() { | ||
| DType::BF16 | ||
| } else { | ||
| DType::F32 | ||
| } | ||
| } |
There was a problem hiding this comment.
Direct std::env::var_os reads in crates/vox-plugin-mens-candle-cuda/src/candle_qlora_train/mod.rs.
Both VOX_MENS_ACT_F32 (line 43) and VOX_MENS_NO_WEIGHT_CACHE (line 514) are read directly via std::env::var_os. Per coding guidelines, consumer code must use vox_secrets::resolve_secret(...) for environment variable access. Additionally, if these are new VOX_* variables, they must be registered in contracts/config/env-vars.v1.yaml.
🤖 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 `@crates/vox-plugin-mens-candle-cuda/src/candle_qlora_train/mod.rs` around
lines 42 - 51, Replace direct std::env::var_os reads with
vox_secrets::resolve_secret(...) so env access follows project policy: in
activation_compute_dtype(), call vox_secrets::resolve_secret("VOX_MENS_ACT_F32")
and check the returned Option (is_some()) instead of std::env::var_os,
preserving the same F32/BF16 branch behavior; do the same for the
VOX_MENS_NO_WEIGHT_CACHE usage (replace
std::env::var_os("VOX_MENS_NO_WEIGHT_CACHE") with
vox_secrets::resolve_secret("VOX_MENS_NO_WEIGHT_CACHE") and adapt the
conditional). Also, if these VOX_* variables are new, add entries for
VOX_MENS_ACT_F32 and VOX_MENS_NO_WEIGHT_CACHE to
contracts/config/env-vars.v1.yaml. Ensure the code compiles by importing
vox_secrets and handling any Result/Option types the resolver returns.
Source: Coding guidelines
Comment on lines
+152
to
+153
| for node in nodes { | ||
| let reason = exclusion_reason(node, target_params_b); |
There was a problem hiding this comment.
Keep cohort admission aligned with the trainer’s family-specific VRAM planner.
exclusion_reason() only uses memory_budget::plan(node.vram_gib, target_params_b) and ignores target_model, but crates/vox-ml-cli/src/commands/mens/populi/train_arm.rs selects plan_qwen25coder, plan_qwen35, or generic plan depending on the model family. That means the cohort planner can admit or exclude nodes differently from the real training path, which skews the recommendation and can still send an “eligible” node into an OOM rung.
Suggested fix
- let reason = exclusion_reason(node, target_params_b);
+ let reason = exclusion_reason(node, target_model, target_params_b);
@@
-fn exclusion_reason(node: &CohortNode, target_params_b: f64) -> Option<ExclusionReason> {
+fn exclusion_reason(
+ node: &CohortNode,
+ target_model: &str,
+ target_params_b: f64,
+) -> Option<ExclusionReason> {
if node.quarantined {
return Some(ExclusionReason::Quarantined);
}
@@
- if memory_budget::plan(node.vram_gib, target_params_b).over_budget {
+ let over_budget = if memory_budget::is_qwen25coder(target_model) {
+ memory_budget::plan_qwen25coder(node.vram_gib, target_params_b).over_budget
+ } else if memory_budget::is_qwen35(target_model) {
+ memory_budget::plan_qwen35(node.vram_gib, target_params_b).over_budget
+ } else {
+ memory_budget::plan(node.vram_gib, target_params_b).over_budget
+ };
+ if over_budget {
return Some(ExclusionReason::OverVramBudget);
}
None
}Also applies to: 215-229
🤖 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 `@crates/vox-populi/src/mens/cohort/planner.rs` around lines 152 - 153,
exclusion_reason currently calls memory_budget::plan(node.vram_gib,
target_params_b) and ignores the model family, causing mismatch with the trainer
which picks plan_qwen25coder, plan_qwen35, or plan based on target_model; update
exclusion_reason (and the similar logic around the other occurrence) to
accept/inspect target_model (or accept a planner function) and call the same
model-family selection used in
crates/vox-ml-cli/src/commands/mens/populi/train_arm.rs so it uses
plan_qwen25coder, plan_qwen35, or generic plan consistently with training;
ensure the function signature(s) and call sites (where exclusion_reason and the
second block are invoked) are updated to pass target_model (or the selected
planner) so cohort admission matches the trainer’s VRAM planner.
Comment on lines
+1
to
+5
| --- | ||
| title: "MENS training pipeline — audit + improvement scoping (2026-06-07)" | ||
| description: "Read-only audit of the MENS QLoRA trainer: checkpoint/resume reality, the crash-at-checkpoint diagnosis, a ranked single-GPU VRAM/throughput backlog, and the verdict on mesh/Populi LAN-distributed training — with scoped plans for auto-recovery, a VRAM bundle, and federated-LoRA-over-LAN." | ||
| category: "Architecture SSOTs" | ||
| --- |
There was a problem hiding this comment.
Restore the docs frontmatter contract.
The two architecture docs are missing the required status and training_eligible frontmatter, and docs/src/reference/mens-training.md also hand-adds last_updated, which the docs pipeline derives from git. Please add the required fields and remove last_updated from the reference doc.
As per coding guidelines, docs/src Markdown files must include frontmatter; based on learnings, last_updated must never be hand-added because vox-doc-pipeline derives it from git.
🤖 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
`@docs/src/architecture/mens-training-pipeline-audit-and-improvement-plan-2026-06-07.md`
around lines 1 - 5, Add the required frontmatter keys to the two architecture
Markdown files by inserting a YAML frontmatter block that includes at minimum
status (string) and training_eligible (boolean) at the top of each document
(e.g., for the "MENS training pipeline — audit + improvement scoping" doc add
status: "<appropriate-status>" and training_eligible: true/false); also remove
the hand-added last_updated field from docs/src/reference/mens-training.md so
the pipeline can derive it from git. Ensure the frontmatter is valid YAML,
placed between --- lines at the very top, and do not add any manual last_updated
entries.
Sources: Coding guidelines, Learnings
Comment on lines
+10
to
+24
| fn run_train(model: str, out: str, seq: int) -> int { | ||
| let mut args = ["mens", "train", "--resume", out] | ||
| // Mid-epoch checkpoints every 200 steps so a crash loses at most ~200 steps | ||
| // (the CLI default is none = epoch-boundary only). Retention is capped by | ||
| // VOX_MENS_KEEP_CHECKPOINTS (default 3), so disk stays bounded. | ||
| args.push("--checkpoint-every") | ||
| args.push("200") | ||
| if model != "" { | ||
| args.push("--model") | ||
| args.push(model) | ||
| } | ||
| if seq > 0 { | ||
| args.push("--seq-len") | ||
| args.push(str(seq)) | ||
| } |
There was a problem hiding this comment.
Accept the starting model from the wrapper CLI instead of hard-coding rung 0.
This script never reads forwarded args, so vox run scripts/mens/train_resilient.vox -- --model Qwen/Qwen3.5-4B-Base still starts with model == "" and lets the default auto-scaler choose the model. That blocks the documented Phase 2 flow from actually exercising the requested 4B starting rung before fallback.
Also applies to: 46-61
🤖 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 `@scripts/mens/train_resilient.vox` around lines 10 - 24, The run_train
function currently ignores forwarded CLI flags so the model parameter remains
empty; update run_train to parse and accept forwarded CLI args (or use the
program's argv) so a passed "--model <name>" is detected and set into the model
variable before building args; specifically, modify logic around the model
parameter in run_train (and the code paths covering lines 46-61) to prefer any
"--model" flag from the wrapper CLI/argv and only fall back to the auto-scaler
default when none is provided, then push that model into the args array as
currently done.
…-driver + serde_yaml deps)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-search # Conflicts: # contracts/reports/operations-catalog-inventory.v1.json # crates/vox-cli-core/Cargo.toml
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
1 participant
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
vox_tool_search: keyword search overTOOL_REGISTRYso agents can discover tools on demand instead of pre-loading every schema into context (mirrors Claude Code's MCP tool search).rank_tools(query, limit)incrates/vox-orchestrator-mcp/src/tool_search.rs: per-term scoring — exact name-segment 8, name substring 4, description 1; filters score 0, sorts score desc then name asc, truncates to limit.{query, total, tools: [{name, description, input_schema}]}using the canonicalinput_schemas::tool_input_schema.tool.searchadded tocontracts/operations/catalog.v1.yaml;contracts/mcp/tool-registry.canonical.yamlregenerated viavox ci operations-sync --target mcp --write(not hand-edited).queryrequired,limit1–100,deny_unknown_fields).Test plan
cargo test -p vox-orchestrator-mcp— all green (166 lib tests incl. 8 tool_search tests; registry/dispatch coverage testsyaml_registry_tools_have_dispatch_match_armsandevery_registry_tool_has_static_dispatchpass with the new row).cargo clippy -p vox-orchestrator-mcp --all-targets -- -D warnings— clean.cargo fmt -p vox-orchestrator-mcp— applied.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
vox-langtoolDB-free language tool with check, format, run, and build commandsBug Fixes
Chores