recover local commits + add server-side groom/standup/health/plan actions - #214
Conversation
Document that ambient CARGO_TARGET_DIR env var outranks the per-worktree .cargo/config.toml (Cargo precedence CLI > env > config), so #133's ambient-env case remains open. Also warn on the re-claim fast path.
…init, components, MCP, and auth_runner
Key: rust (not rust-analyzer). Built-in server, no custom command/extensions needed. Settings go in initialization options.
Built-in servers need command when overridden in config
Adds item action=\groom\: returns a priority+recency-ranked shortlist with full description plus server-computed stale/unassigned/blocked_by/ depended_on_by_count/possible_duplicates/size/unestimated signals and a pull_next list, in one MCP round trip instead of list + N x get. - dependencies_for_items(): bulk dependency-edge query for the shortlist - UpdateItem/item(update) now accepts metadata, so size:S|M|L can be set on existing items (was create-only) - also fixes a latent param_idx bug in item::update where sort_order never advanced the placeholder index, silently reusing it for any field added after it - groom parses metadata.size instead of regexing description prose - /pm:groom, /pm:plan, and the read-recipe/rubric skill docs now call groom directly instead of the old list+get+hand-scoring path - benchmark test (ignored by default) comparing groom vs list+15xget
Dogfooding item(create) with metadata={"size":"S"} via a live MCP call
stored a JSON string containing JSON instead of the object itself, so
groom's parsed_size() silently reported these items as unestimated.
parsed_size() now unwraps one extra string-encoding layer before giving
up. Regression test reproduces the exact stored shape.
Extends item action="groom" with an optional `capacity` param that additionally buckets the shortlist into now/next/later/needs_estimation, reusing the rank/blocked_by/unestimated signals groom already computes. Omitted from the response when capacity is unset (backward compatible). Extracted priority_rank/parsed_size/dependency_signals/near_duplicates/ capacity_buckets into named module-level functions in item.rs - the handler was accreting cognitive complexity with every addition and this keeps it as an orchestration function. /pm:plan now calls groom(capacity=N) directly instead of re-bucketing groom's shortlist itself.
Adds item action="standup": returns done (completed within cutoff_hours, default 24)/in_progress (grouped by assignee, "unassigned" as its own group)/stuck (in-progress older than staleness_days, default 7) computed server-side from one state-filtered read, instead of the caller bucketing a flat list result by hand. /pm:standup and the read-recipe skill doc now call it directly.
Adds item action="health": trailing weekly velocity series (oldest to newest) with an up/down/flat trend, WIP list+count, stuck items (WIP older than staleness_days, default 7), and a bottlenecks field. Velocity is a live scan over list_by_project, not a precomputed/event- populated rollup table: events::emit (agentflare-backend/src/events.rs) turned out to be outbound webhook delivery only, not a persisted log, and there's no handoff-history table either - handoff is assign + asset version + comment, not a separate audit log. Building either is real new migration work; at this project's actual scale a live scan is sub-millisecond (see the groom benchmark), so that infrastructure would be speculative today. bottlenecks is therefore always empty, with bottleneck_note explaining why, matching the skill's own documented "if none available, print no handoff history" fallback. /pm:health now calls the action directly instead of hand-computing the weekly buckets.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds server-driven MCP grooming, standup, and health workflows; persists item sizing metadata; replaces ponytail integrations with optimize-code prompts and hooks; removes deprecated CLI modules; isolates Cargo targets per worktree; and updates tooling guidance and configuration. ChangesMCP item workflows
Optimize-code migration
Developer environment updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
getappz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
opencode.json (1)
3-10: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDrop the
rust-analyzer.prefixes ininitialization.
initializationis sent as rust-analyzerinitializationOptions, so these keys need to be nested ascargo.features,checkOnSave.enable,check.overrideCommand,diagnostics.enable, andinlayHints.enable. The current dotted keys will be treated as literal field names and won’t take effect.🤖 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 `@opencode.json` around lines 3 - 10, Update the rust initialization options to use nested rust-analyzer keys instead of dotted names: map cargo features to cargo.features, enable check-on-save via checkOnSave.enable, move clippy to check.overrideCommand, and retain diagnostics.enable and inlayHints.enable. Remove the rust-analyzer. prefixes and preserve the existing settings.src/mcp_server/item.rs (1)
275-284: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve existing metadata when adding a size.
item(update)stringifies the supplied object and the backend replaces the entire metadata value. Following the new sizing guidance can therefore erase unrelated fields such as handoff/thread metadata.
src/mcp_server/item.rs#L275-L284: merge object keys into existing metadata, or provide a dedicated size update operation.src/mcp_server.rs#L589-L591: document whether metadata is merged or replaced..claude/skills/pm/SKILL.md#L62-L66: do not recommend a partial object until preservation is guaranteed..claude/skills/pm/reference/rubric.md#L24-L40: require preservation/read-modify-write if replacement semantics remain.🤖 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 `@src/mcp_server/item.rs` around lines 275 - 284, Update the metadata handling in item update at src/mcp_server/item.rs#L275-L284 to preserve existing fields through a merge or dedicated size-update operation, rather than replacing metadata with the supplied partial object. Document the resulting merge/replacement semantics in src/mcp_server.rs#L589-L591. Update .claude/skills/pm/SKILL.md#L62-L66 to avoid recommending partial metadata objects until preservation is guaranteed, and update .claude/skills/pm/reference/rubric.md#L24-L40 to require preservation via read-modify-write when replacement semantics remain.src/components.rs (1)
542-564: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake
optimize-code-modeenforceultraon Cursor.
The component is marked satisfied on every non-Claude host, so Cursor never gets pinned even though optimize hooks are wired there.write_pinned_mode()also treats any existingdefaultModeas final, solite/fullwon't be upgraded. Compare the stored value againstultraand update it via the flare-code mode API; add coverage for Cursor and an existing non-ultra config.🤖 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 `@src/components.rs` around lines 542 - 564, Update the optimize-code-mode component’s check and apply flow so Cursor is evaluated rather than immediately treated as satisfied by the !claude_code_only branch. In write_pinned_mode, require the stored defaultMode to equal ultra; when it is lite, full, or missing, update it through the existing flare-code mode API. Add coverage verifying Cursor enforcement and upgrading an existing non-ultra configuration.
🤖 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 @.claude/skills/pm/reference/read-recipe.md:
- Around line 32-36: Update the “Detail fetch” guidance to remove “labels” from
the fields promised by item action="get"; keep the remaining documented Item
fields unchanged and do not alter backend serialization.
In `@src/init.rs`:
- Around line 604-607: Update the already_wired calculation in the hook-wiring
flow to validate every required integration entry independently: SessionStart,
SubagentStart, UserPromptSubmit, and statusLine. Only print the existing
“already wired” message when all required hooks are present; otherwise continue
with wiring the missing entries.
In `@src/mcp_server.rs`:
- Around line 619-623: Update the schema documentation for staleness_days in the
relevant configuration struct to describe every consuming action, including
groom’s default of 14 and standup and health’s default of 7; keep the existing
serde behavior unchanged.
In `@src/mcp_server/item.rs`:
- Around line 624-634: Update the dependency-signal computation around
dependency_signals to resolve dependency target states from the full
project/item set rather than shortlist, including completed and cancelled
targets. Also batch-load project-wide incoming dependency edges so
depended_on_by_count includes dependents outside shortlist, then compute fanin,
blocked_by, and counts from the expanded data.
- Around line 728-736: The completion-based metrics use updated_at, so editing
an already-completed item incorrectly changes its completion window. In
src/mcp_server/item.rs lines 728-736, update the done-item filter to compare
completed_at with done_cutoff; in src/mcp_server/item.rs lines 819-832, bucket
velocity using completed_at instead of updated_at.
- Around line 796-825: Cap user-controlled workflow cardinalities: in
item_health, enforce a documented maximum for window_weeks before constructing
the velocity range; in the groom handler, enforce a documented maximum for limit
to bound duplicate analysis and SQLite parameters; and update the window_weeks
schema definition in src/mcp_server.rs to advertise the supported range. Apply
changes at src/mcp_server/item.rs lines 796-825 and 581-586, and
src/mcp_server.rs lines 634-636.
In `@src/worktree.rs`:
- Around line 90-105: Update warn_if_ambient_target_dir to detect both
CARGO_TARGET_DIR and CARGO_BUILD_TARGET_DIR, warning when either environment
variable is set. Extend the existing coverage to verify warnings for each
variable independently, while preserving the current non-warning behavior when
neither is present.
- Around line 132-135: Update isolate_worktree_target_dir to account for both
Cargo config filenames, checking .cargo/config as well as config.toml before
writing. When an existing configuration has a shared or absolute target-dir,
detect the unisolated effective target directory and warn or fail instead of
silently returning; preserve intentional local overrides.
- Around line 563-575: Update the warn_if_ambient_target_dir_warns_when_set test
to avoid permanently mutating shared CARGO_TARGET_DIR state: save whether the
variable was originally set and its value, restore that exact state after
testing, and ensure cleanup also occurs if the test path fails. Prefer a test
helper or scoped restoration while preserving coverage for both set and unset
cases.
---
Outside diff comments:
In `@opencode.json`:
- Around line 3-10: Update the rust initialization options to use nested
rust-analyzer keys instead of dotted names: map cargo features to
cargo.features, enable check-on-save via checkOnSave.enable, move clippy to
check.overrideCommand, and retain diagnostics.enable and inlayHints.enable.
Remove the rust-analyzer. prefixes and preserve the existing settings.
In `@src/components.rs`:
- Around line 542-564: Update the optimize-code-mode component’s check and apply
flow so Cursor is evaluated rather than immediately treated as satisfied by the
!claude_code_only branch. In write_pinned_mode, require the stored defaultMode
to equal ultra; when it is lite, full, or missing, update it through the
existing flare-code mode API. Add coverage verifying Cursor enforcement and
upgrading an existing non-ultra configuration.
In `@src/mcp_server/item.rs`:
- Around line 275-284: Update the metadata handling in item update at
src/mcp_server/item.rs#L275-L284 to preserve existing fields through a merge or
dedicated size-update operation, rather than replacing metadata with the
supplied partial object. Document the resulting merge/replacement semantics in
src/mcp_server.rs#L589-L591. Update .claude/skills/pm/SKILL.md#L62-L66 to avoid
recommending partial metadata objects until preservation is guaranteed, and
update .claude/skills/pm/reference/rubric.md#L24-L40 to require preservation via
read-modify-write when replacement semantics remain.
🪄 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 Plus
Run ID: d37b1d8f-1135-49f7-8978-20f08f9fc4dc
📒 Files selected for processing (17)
.claude/skills/pm/SKILL.md.claude/skills/pm/reference/read-recipe.md.claude/skills/pm/reference/rubric.mdAGENTS.mdcrates/agentflare-backend/src/item.rsopencode.jsonsrc/auth_runner.rssrc/cli/caveman.rssrc/cli/mod.rssrc/cli/ponytail.rssrc/components.rssrc/init.rssrc/mcp_prompts.rssrc/mcp_server.rssrc/mcp_server/item.rssrc/optimize/code.rssrc/worktree.rs
💤 Files with no reviewable changes (3)
- src/cli/caveman.rs
- src/cli/ponytail.rs
- src/cli/mod.rs
| .map(|v| v.to_string().contains("optimize")) | ||
| .unwrap_or(false); | ||
| if already_wired { | ||
| println!(" skip ponytail hooks already wired in ~/.claude/settings.json"); | ||
| println!(" skip optimize code hooks already wired in ~/.claude/settings.json"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check all required hooks before declaring wiring complete.
The early return only checks whether SessionStart contains optimize. A partial configuration can therefore skip adding SubagentStart, UserPromptSubmit, or statusLine. Validate each required entry independently before reporting the integration as already wired.
🤖 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 `@src/init.rs` around lines 604 - 607, Update the already_wired calculation in
the hook-wiring flow to validate every required integration entry independently:
SessionStart, SubagentStart, UserPromptSubmit, and statusLine. Only print the
existing “already wired” message when all required hooks are present; otherwise
continue with wiring the missing entries.
| /// Warns (does not fail) when an ambient `CARGO_TARGET_DIR` is set in the | ||
| /// environment at claim time. A shared `CARGO_TARGET_DIR` across worktrees | ||
| /// is a silent correctness bug: Cargo's fingerprint hash omits the worktree | ||
| /// path, so two worktrees of the same repo reuse each other's stale local | ||
| /// crate artifacts (cargo #12516/#14053/#7740; OpenBlob #522). | ||
| /// | ||
| /// NOTE: this is a mitigation, NOT a fix for the ambient-env case of #133. | ||
| /// Per Cargo's precedence (CLI flag > env var > config file), an ambient | ||
| /// `CARGO_TARGET_DIR` *always* wins over the `.cargo/config.toml` that | ||
| /// `isolate_worktree_target_dir` writes — so when the env var is set, the | ||
| /// worktree's isolated `target/` is silently shadowed and the bug persists. | ||
| /// Nothing in code can force Cargo to prefer the config file over the env var; | ||
| /// the only safe remedies are unsetting the var or trusting CI. #133 therefore | ||
| /// remains OPEN for the ambient-env case. | ||
| fn warn_if_ambient_target_dir() { | ||
| if std::env::var_os("CARGO_TARGET_DIR").is_some() { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby context
sed -n '1,220p' src/worktree.rs
printf '\n---\n'
# Search for both target-dir env vars in the repository
rg -n "CARGO_(BUILD_)?TARGET_DIR|target-dir" .Repository: getappz/agentflare
Length of output: 11094
🌐 Web query:
Cargo CARGO_BUILD_TARGET_DIR target-dir precedence docs
💡 Result:
The precedence for specifying Cargo's target directory (the location for generated artifacts) follows a specific hierarchy. When multiple methods are used, the effective value is determined by the following order of precedence, from highest to lowest: 1. Command-line interface (CLI) options: Using the --target-dir flag directly with a Cargo command (e.g., cargo build --target-dir path) takes the highest precedence [1][2][3]. 2. CLI configuration: Values set using the --config flag (e.g., --config 'build.target-dir = "path"') [4]. 3. Environment variables: Setting CARGO_BUILD_TARGET_DIR or CARGO_TARGET_DIR (the latter being a common alias) [5][1][2]. 4. Configuration files: Values defined in.cargo/config.toml files (e.g., [build] target-dir = "path") [1][4]. 5. Default: If no other source is specified, Cargo defaults to a directory named target in the root of the workspace [1][2]. Note that while CARGO_BUILD_TARGET_DIR and CARGO_TARGET_DIR are both recognized, they refer to the same configuration value (build.target-dir) [5][1]. Additionally, recent updates ensure that configuration files explicitly loaded via the CLI take precedence over environment variables [4].
Citations:
- 1: https://doc.rust-lang.org/stable/cargo/reference/config.html
- 2: https://doc.rust-lang.org/nightly/cargo/commands/cargo-build.html
- 3: https://doc.rust-lang.org/stable/cargo/commands/cargo-build.html
- 4: Config file loaded via CLI takes priority over env vars rust-lang/cargo#11077
- 5: https://doc.rust-lang.org/stable/cargo/reference/environment-variables.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read Cargo documentation snippets relevant to env vars and precedence from the repo if present
rg -n "CARGO_BUILD_TARGET_DIR|CARGO_TARGET_DIR|target-dir|precedence" README* .cargo Cargo.toml src || trueRepository: getappz/agentflare
Length of output: 1978
Warn for both Cargo target-dir env vars. Cargo also honors CARGO_BUILD_TARGET_DIR for build.target-dir, so this warning can miss an ambient override and leave worktree isolation shadowed. Check both env vars and add coverage for each.
🤖 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 `@src/worktree.rs` around lines 90 - 105, Update warn_if_ambient_target_dir to
detect both CARGO_TARGET_DIR and CARGO_BUILD_TARGET_DIR, warning when either
environment variable is set. Extend the existing coverage to verify warnings for
each variable independently, while preserving the current non-warning behavior
when neither is present.
| let config_path = cargo_dir.join("config.toml"); | ||
| if config_path.exists() { | ||
| return; // don't clobber an intentional worktree-local override | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and the relevant regions.
sed -n '1,220p' src/worktree.rs
printf '\n---\n'
sed -n '520,580p' src/worktree.rsRepository: getappz/agentflare
Length of output: 11634
🌐 Web query:
Cargo config precedence .cargo/config .cargo/config.toml official docs
💡 Result:
Cargo configuration is defined across multiple files and environment variables, with a specific order of precedence. File Precedence and Merging Cargo searches for configuration files in the current directory and all parent directories, up to the filesystem root, followed by the global configuration in the CARGO_HOME directory [1][2]. If multiple configuration files define the same key, Cargo merges the values [1][3]: 1. Local configuration: Files found closer to the current working directory have higher precedence than those in parent directories [1][2]. 2. Global configuration: The file at $CARGO_HOME/config.toml (or the legacy.cargo/config) has the lowest precedence [1][2]. Config vs. Config.toml Cargo supports both.cargo/config.toml and the older.cargo/config (without an extension) [1][3]. If both files exist in the same directory, Cargo will ignore the.toml file and use the file without the extension [1][2]. The.toml extension is the modern, preferred format [1][3]. Overall Precedence Hierarchy When configuration is provided via multiple sources, the general order of precedence (from highest to lowest) is [4]: 1. Command-line overrides (e.g., --config) [4][3] 2. Environment variables [5][4] 3. Configuration files (merged from closest to the project root to global) [1][2] For keys containing numbers, strings, or booleans, the value from the higher-precedence file (closer to the current directory) overrides values from lower-precedence files [1][3]. For arrays, Cargo joins the values together, with higher-precedence items placed later in the resulting array [1][2].
Citations:
- 1: https://doc.rust-lang.org/stable/cargo/reference/config.html
- 2: https://doc.rust-lang.org/nightly/cargo/reference/config.html
- 3: https://dev-doc.rust-lang.org/cargo/reference/config.html
- 4: Environment variables override cargo --config <filename> in precedence rust-lang/cargo#10992
- 5: https://doc.rust-lang.org/cargo/reference/config.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs '\.cargo/config(\.toml)?|config\.toml|config\b' src testsRepository: getappz/agentflare
Length of output: 12021
Handle legacy Cargo config files and unisolated overrides
isolate_worktree_target_dir only checks .cargo/config.toml; if .cargo/config already exists, Cargo will prefer it and ignore the file written here. Preserving an existing config.toml with a shared or absolute target-dir also leaves the worktree unisolated without any signal. Check both names, or warn/fail when the effective target dir is not local.
🤖 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 `@src/worktree.rs` around lines 132 - 135, Update isolate_worktree_target_dir
to account for both Cargo config filenames, checking .cargo/config as well as
config.toml before writing. When an existing configuration has a shared or
absolute target-dir, detect the unisolated effective target directory and warn
or fail instead of silently returning; preserve intentional local overrides.
| #[test] | ||
| fn warn_if_ambient_target_dir_warns_when_set() { | ||
| // Just asserts the function runs without panicking whether or not the | ||
| // var is set; the warning is an ephemeral eprintln, not assertable here. | ||
| unsafe { | ||
| std::env::set_var("CARGO_TARGET_DIR", "/tmp/shared"); | ||
| } | ||
| warn_if_ambient_target_dir(); | ||
| unsafe { | ||
| std::env::remove_var("CARGO_TARGET_DIR"); | ||
| } | ||
| warn_if_ambient_target_dir(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of src/worktree.rs
python3 - <<'PY'
from pathlib import Path
p = Path('src/worktree.rs')
lines = p.read_text().splitlines()
start, end = 520, 620
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4}: {lines[i-1]}")
PYRepository: getappz/agentflare
Length of output: 4683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find test serialization markers and other environment-mutation patterns in the repo.
python3 - <<'PY'
from pathlib import Path
for path in Path('.').rglob('*.rs'):
try:
text = path.read_text()
except Exception:
continue
if 'serial_test' in text or 'set_var(' in text or 'remove_var(' in text:
print(f"\n== {path} ==")
for needle in ['serial_test', 'set_var(', 'remove_var(']:
idx = text.find(needle)
if idx != -1:
line = text[:idx].count('\n') + 1
print(f" first {needle!r} at line {line}")
PYRepository: getappz/agentflare
Length of output: 920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the test module setup around src/worktree.rs and the repo-wide test configuration.
python3 - <<'PY'
from pathlib import Path
# Relevant file slices
for path, start, end in [
('src/worktree.rs', 1, 120),
('Cargo.toml', 1, 260),
]:
p = Path(path)
if not p.exists():
continue
lines = p.read_text().splitlines()
print(f"\n== {path} ==")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4}: {lines[i-1]}")
# Search for explicit test serialization / single-threading config.
needles = [
'#[serial',
'serial_test',
'test-threads',
'RUST_TEST_THREADS',
'harness = false',
]
for path in Path('.').rglob('*'):
if not path.is_file():
continue
if path.suffix not in {'.toml', '.rs', '.md'}:
continue
try:
text = path.read_text()
except Exception:
continue
hits = [n for n in needles if n in text]
if hits:
print(f"\n== {path} ==")
for n in hits:
print(f" contains {n!r}")
PYRepository: getappz/agentflare
Length of output: 10216
Avoid mutating CARGO_TARGET_DIR in this test
set_var/remove_var can race with other tests, and this cleanup also drops any pre-existing value instead of restoring it. Pass the value through a test helper, or save and restore the original environment state around the test.
🤖 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 `@src/worktree.rs` around lines 563 - 575, Update the
warn_if_ambient_target_dir_warns_when_set test to avoid permanently mutating
shared CARGO_TARGET_DIR state: save whether the variable was originally set and
its value, restore that exact state after testing, and ensure cleanup also
occurs if the test path fails. Prefer a test helper or scoped restoration while
preserving coverage for both set and unset cases.
- cargo fmt --all: wraps a few lines the local dev-profile check didn't
flag (CI's fmt job uses --check with no width override)
- tests/caveman_cli.rs called the pre-rename `agentflare caveman compress`
subcommand, which no longer exists after the ponytail/caveman ->
optimize/flare-code rename; updated to `agentflare optimize output
compress` (FlareAction::Output { action: OutputAction::Compress },
src/cli/optimize.rs) and renamed the test function to match
Verified with the exact CI command set locally:
- cargo fmt --all --check: clean
- cargo clippy --locked --workspace --all-targets --all-features -- -D
warnings -A unsafe_code -A clippy::pedantic: clean
- cargo test --workspace: 543 passed, 1 ignored, 0 failed (plus all other
workspace crates' test suites, all passing)
- dependency_edges_for_items (was dependencies_for_items) now joins the dependency target's true state_group in SQL, instead of looking it up via a shortlist-scoped linear scan. Fixes a real bug: a completed dependency that fell outside the default state_group filter (e.g. "backlog,unstarted" excludes "completed") read back as "" from the old lookup and was treated as still-open, falsely blocking its dependent. - dependency_fanin_for_items counts dependents project-wide instead of only within the shortlist, fixing an undercounted depended_on_by_count when a dependent fell outside the shortlist/limit window. - standup's "done" filter and health's velocity bucketing now key off completed_at instead of updated_at. Editing an already-completed item (e.g. fixing a typo) bumps updated_at without re-completing it; using updated_at made old work spuriously reappear as "just done" or shift which week it counted toward. - window_weeks (health) and limit (groom) are now clamped (52, 200) instead of unbounded - an unbounded window_weeks drove a Vec allocation of that literal size while holding the backend DB lock. - Fixed two doc inaccuracies: read-recipe.md claimed item(get) returns labels (it doesn't - separate join table); staleness_days' schema description only mentioned groom's default, not standup/health's. 6 new regression tests, one per fix. cargo test --workspace: 548 passed, 1 ignored, 0 failed. cargo clippy --locked --workspace --all-targets --all-features: clean. cargo fmt --all --check: clean.
1 participant
Summary
Recovered (pre-existing on this branch):
New this round — server-side
itemMCP actions (groom/standup/health) +plan:item action="groom": priority+recency-ranked backlog shortlist withstale/unassigned/blocked_by/depended_on_by_count/possible_duplicates/size/unestimatedprecomputed, pluspull_next— replaces alist+ N×get+ hand-computed-flags round trip with one call.groom(capacity=N): addsnow/next/later/needs_estimationbuckets for/pm:plan, reusing groom's existing signals.item action="standup":done/in_progress(grouped by assignee)/stuckcomputed server-side.item action="health": weekly velocity + trend, WIP, stuck, and abottlenecksplaceholder (honestly empty — no persisted handoff-history log exists yet, documented rather than faked).item(update)now acceptsmetadata(was create-only), unblockingsizelabels on existing items; fixed a latentparam_idxbug initem::updatefound while extending it.groom'ssizeparsing, found live via dogfooding..claude/skills/pm/*updated so all four/pm:*workflows call the new actions directly instead of hand-computing flags/buckets.Test plan
cargo test -p agentflare-backend -p agentflare --bins --lib: 543 passed, 1 ignored (manual benchmark), 0 failedcargo clippy -p agentflare-backend -p agentflare --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic: cleangroom,groom(capacity=5),standup, andhealthlive end-to-end against the real project backlog after rebuild + reinstallSummary by CodeRabbit