ci: run tests with nextest, add cargo-hakari workspace-hack - #399
Conversation
Switches CI's cargo test --workspace to cargo nextest run --workspace (doctests still covered by a separate cargo test --doc step, since nextest doesn't run them). Wiring this up surfaced a real bug: the root Cargo.toml's unconstrained `agent-detector = "0.2.1"` dependency pulled in the process-tree feature by default, and Cargo's feature unification silently overrode flare-git-core's explicit default-features = false opt-out whenever the whole workspace built together. With process-tree active, parent-process matching flags "claude" as an ancestor of every process in a Claude-Code session, which defeated the git shim's env-based agent detection regardless of what env vars a test stripped -- confirmed this also broke plain `cargo test --workspace` before nextest was ever involved. Fixed by aligning the root crate's declaration with flare-git-core's. Also adds cargo-hakari (agentflare-workspace-hack) to collapse per-crate feature-set duplication across the 18-crate workspace, with a CI job that fails if the generated crate drifts out of sync.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a hakari-managed workspace-hack crate, connects it to workspace packages, and updates CI to validate dependency generation and run tests with Cargo Nextest and separate doctest execution. ChangesWorkspace dependency consolidation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant CargoNextest
participant CargoDocTests
participant CargoHakari
participant CiGreen
GitHubActions->>CargoNextest: Run workspace tests
GitHubActions->>CargoDocTests: Run workspace doctests
GitHubActions->>CargoHakari: Generate and verify workspace dependencies
CargoNextest-->>CiGreen: Report test result
CargoDocTests-->>CiGreen: Report doctest result
CargoHakari-->>CiGreen: Report hakari result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.config/nextest.toml:
- Around line 1-5: Update the [profile.default] configuration to set fail-fast =
false alongside slow-timeout, ensuring timeout failures do not stop cargo
nextest run from scheduling the remaining tests.
🪄 Autofix
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: 51e2ef57-0cc9-4f2f-95f1-6691cc0ae9d5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.config/hakari.toml.config/nextest.toml.github/workflows/ci.ymlCargo.tomlagentflare-workspace-hack/.gitattributesagentflare-workspace-hack/Cargo.tomlagentflare-workspace-hack/build.rsagentflare-workspace-hack/src/lib.rscrates/agent-registry/Cargo.tomlcrates/agentflare-artifacts/Cargo.tomlcrates/agentflare-backend/Cargo.tomlcrates/agentflare-db-kit/Cargo.tomlcrates/agentflare-jobs/Cargo.tomlcrates/agentflare-shim/Cargo.tomlcrates/agentflare-store/Cargo.tomlcrates/flare-code/Cargo.tomlcrates/flare-docs/Cargo.tomlcrates/flare-git-core/Cargo.tomlcrates/flare-git-shim/Cargo.tomlcrates/flare-output/Cargo.tomlcrates/flare-proxy/Cargo.tomlcrates/flare-search-kit/Cargo.tomlcrates/flare-vault/Cargo.tomlcrates/gateway-registry/Cargo.tomlcrates/skill-registry/Cargo.toml
| [profile.default] | ||
| # A single hung test shouldn't be able to pin a runner for the job's full | ||
| # 25-minute timeout (see ci.yml) — kill it well before that and let the rest | ||
| # of the suite keep going. | ||
| slow-timeout = { period = "60s", terminate-after = 5 } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- tracked nextest and CI files ---'
git ls-files | rg '(^|/)(nextest\.toml|.*ci.*\.ya?ml|.*workflow.*\.ya?ml)$|(^|/)Cargo\.toml$' | head -200
printf '%s\n' '--- nextest configuration references ---'
rg -n -C 4 'slow-timeout|terminate-after|fail-fast|nextest run|cargo nextest' .config .github . 2>/dev/null | head -300Repository: getappz/agentflare
Length of output: 3258
🌐 Web query:
cargo nextest configuration reference default profile fail-fast timeout terminate-after behavior
💡 Result:
In cargo-nextest, fail-fast and timeout configurations are managed via profiles (such as [profile.default]) in the.config/nextest.toml repository configuration file [1][2]. Fail-Fast Behavior The fail-fast setting determines how nextest behaves when tests fail [3]. - Default: Enabled (true), which stops scheduling new tests after the first failure [1][3]. - Configuration: It can be set as a boolean or an object [1][3]. - fail-fast = true (default): Stops after the first failure, waiting for already running tests to finish naturally [3]. - fail-fast = { max-fail = N }: Stops the run after N failures [3]. - fail-fast = false (or max-fail = "all"): Continues running all tests regardless of failures [3]. - Termination Modes: When max-fail is reached, you can control how currently running tests are handled [3]: - terminate = "wait" (default): Allows running tests to complete naturally [3]. - terminate = "immediate": Sends termination signals to running tests immediately (respecting the grace period) [3]. Timeout and Terminate-After Behavior The slow-timeout setting identifies tests taking longer than expected [1][4]. - Configuration: Defined as a string (e.g., "60s") or an object [1]. - Object Parameters [1]: - period: The time duration after which a test is marked slow [1]. - terminate-after: The number of 'period' durations to wait before terminating the test [1]. By default, this is not set (tests are not terminated) [1]. - grace-period: The time to wait for a graceful shutdown before force-terminating a test (default: 10s) [1]. - on-timeout: Defines the result if a test times out; defaults to "fail", but can be set to "pass" (e.g., for certain fuzz tests) [1][4]. When a test is terminated (either via fail-fast's "immediate" mode or exceeding a slow-timeout), nextest uses its grace period logic to attempt a clean shutdown before force-killing the test process [1][3]. These settings can be applied globally in a profile or via per-test overrides using the [[profile.default.overrides]] section [4][5].
Citations:
- 1: https://nexte.st/docs/configuration/reference/
- 2: https://nexte.st/docs/configuration/
- 3: https://nexte.st/docs/running/
- 4: https://nexte.st/docs/features/slow-tests/
- 5: https://nexte.st/docs/configuration/per-test-overrides/
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
p = Path(".config/nextest.toml")
text = p.read_text()
print("--- .config/nextest.toml ---")
print(text)
section = re.search(r"(?ms)^\[profile\.default\]\s*(.*?)(?=^\[|\Z)", text)
if not section:
raise SystemExit("profile.default not found")
body = section.group(1)
print("--- parsed profile.default keys ---")
for line in body.splitlines():
line = line.split("#", 1)[0].strip()
if line and "=" in line:
print(line.split("=", 1)[0].strip())
print("--- fail-fast declaration count ---")
print(len(re.findall(r"(?m)^\s*fail-fast\s*=", body)))
PYRepository: getappz/agentflare
Length of output: 524
Set fail-fast = false in the default profile.
terminate-after causes a test to fail. With the default fail-fast = true, cargo nextest run stops scheduling new tests after the first timeout. This prevents the full suite from running.
🤖 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 @.config/nextest.toml around lines 1 - 5, Update the [profile.default]
configuration to set fail-fast = false alongside slow-timeout, ensuring timeout
failures do not stop cargo nextest run from scheduling the remaining tests.
cargo-deny failed CI: agentflare-workspace-hack (generated by cargo hakari init) had no license field, and cargo-deny's licenses check denies unlicensed crates by default. Adds license = "Apache-2.0", matching the rest of the workspace. zizmor also failed with impostor-commit on all three dtolnay/rust-toolchain@4cda84d... pins (ci.yml's clippy/fmt jobs, ppa-publish.yml): that crate rebases its "stable" branch to track new Rust releases, which orphans previously-pinned SHAs from the branch's history even though the commit itself is genuine and signed by dtolnay. Refreshed all three pins to the current stable tip (4360b52568e2003a75bf9bc1d59f33a8e3fc893c, 2026-08-05, verified).
…sses with_temp_config_dir used a fixed, hardcoded directory path guarded by a process-local Mutex (ENV_TEST_LOCK). That serialized the three tests sharing it (defaults_to_full, reads_env_var, roundtrip_default_mode) fine under cargo test, since all tests in a binary share one process -- but nextest runs each test as its own process, so the mutex no longer protects anything and sibling processes race on the same directory. Surfaced as a hard failure on Windows CI (concurrent delete/write on the same path errors outright instead of just racing): roundtrip_default_mode panicked restoring the prior mode after another process's cleanup deleted the directory out from under it. Keys the directory by process id so each nextest process gets its own.
Same root cause as the config.rs fix: flag_path()/session_path() resolved straight to dirs::state_dir() with no test override, and the tests guarded that shared, real, on-disk path with a process-local Mutex. That only serializes tests within one process (cargo test's default) -- nextest runs each test as its own process, so sibling processes raced on the same real state files. Adds FLARE_CODE_STATE_DIR_OVERRIDE (mirroring config_dir()'s FLARE_CODE_CONFIG_DIR_OVERRIDE) and keys the test temp dir by process id, same fix as with_temp_config_dir. Also means these tests no longer touch the real state directory at all. Audited the rest of the workspace for the same shape (a #[cfg(test)] Mutex guarding a fixed shared path): agent-registry's PATH_LOCK and flare-vault's HOME_OVERRIDE_LOCK already key their temp paths by process id or a random tempdir, so neither races.
WorkerPool::shutdown() called Queue::wake_workers() exactly once, which sets a single boolean flag and notify_all()s the condvar. With N idle workers, whichever one reaches the notify check first consumes the flag; any other worker still on its way there (e.g. mid-dequeue) gets no signal and falls through to wait_for_work's full fallback timeout instead of noticing `running` immediately. Reproduced consistently on CI's more heavily loaded runners (nextest running the whole workspace concurrently makes the race window much easier to hit than on an idle local machine) as shutdown_returns_promptly_... consistently taking ~1s instead of the <300ms the test expects. Retries wake_workers() while polling JoinHandle::is_finished(), so a worker that hasn't reached the notify check yet still gets caught -- a no-op for a worker mid-job (nothing parked on the condvar to wake), bounded by the retry cap, and the trailing join still waits out real work regardless. Queue::wait_for_work/wake_workers themselves are unchanged -- this only closes the gap on the shutdown path.
…orkspace-hack (#410) * fix(agent-detector): restore process-tree feature dropped by hakari workspace-hack cargo-hakari's workspace-hack (#399) changed agent-detector to default-features=false without re-adding process-tree explicitly, silently disabling tier-1 (parent process-tree walk) identity detection for every session. Every session now falls straight to tier-2 (generic AI_AGENT/AGENT env vars), which is fragile — any harness/job-runner setting a bare AGENT=<slot-index> collides with it. Concretely broke opencode's item claim: its job runner sets AGENT=1, which misresolved its identity to \1\ instead of \opencode\, tripping item::claim()'s assignee-freeze check. * chore: regenerate workspace-hack for process-tree feature deps cargo hakari generate to add objc2-core-foundation and windows crate entries picked up by re-enabling agent-detector's process-tree feature.
1 participant
Summary
cargo test --workspacetocargo nextest run --workspace(+ a separatecargo test --doc --workspace, since nextest doesn't run doctests), backed by.config/nextest.toml.Cargo.toml's unconstrainedagent-detector = "0.2.1"pulled in theprocess-treefeature by default, and Cargo's feature unification silently overrodeflare-git-core's explicitdefault-features = falseopt-out whenever the whole workspace built together. Withprocess-treeactive, parent-process matching flagsclaudeas an ancestor of every process in a Claude-Code session, defeating the git shim's env-based agent detection regardless of what env vars a test stripped. Confirmed this also broke plaincargo test --workspacebefore nextest was involved. Fixed by aligning the root crate's declaration withflare-git-core's.cargo-hakari(agentflare-workspace-hack) to collapse per-crate feature-set duplication across the 18-crate workspace, with a newhakariCI job that fails if the generated crate drifts out of sync.Test plan
cargo nextest run --workspace— 1961 tests pass (previously 3 failed deterministically before theagent-detectorfix)cargo test --workspace --test shim_test— confirmed the same 3 failures reproduce under plaincargo test, ruling out a nextest-specific causecargo test --doc --workspace— clean (0 doctests today)cargo clippy --workspace --all-targets --all-features— cleancargo fmt --check— cleancargo hakari generate --diff,cargo hakari manage-deps --dry-run,cargo hakari verify— all cleanSummary by CodeRabbit
Tests
Chores