Repository navigation
feat: migrate artifact storage onto agentflare-store documents+blobs + vent: origin routing, throttle, batched filing, judge-prompt hook - #331
Merged
Conversation
|
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 Claude failure judging, origin-based vent routing with throttling, pending vent batching and GitHub filing through CLI and MCP, global vent paths, tag preservation during consolidation, and associated tests. ChangesVent pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PostToolUseFailure
participant append_routed
participant VentLog
PostToolUseFailure->>append_routed: failure message
append_routed->>append_routed: classify origin and check throttle window
append_routed->>VentLog: write VentLine or return suppressed status
sequenceDiagram
participant VentClient
participant pending_batch
participant create_github_issue
participant mark_filed
VentClient->>pending_batch: request pending core vents
VentClient->>create_github_issue: submit title and body
create_github_issue-->>VentClient: issue URL
VentClient->>mark_filed: record topic keys and timestamp
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@claude ready for review — 9 TDD tasks implementing vent pipeline redesign (origin classification, throttle, batched filing, judge-prompt hook). 48 tests pass, cargo check clean. |
![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 (1)
src/vent/consolidate.rs (1)
79-107: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMerge tags across vents and consolidation runs.
g.tags = l.tags.clone()makes the last line in a topic replace earlier tags. The backend contract incrates/agentflare-backend/src/vent.rsLines 28-93 also replacesvents.tagson update, so a later untagged batch can erase tags persisted by an earlier run. Accumulate/dedupe tags and merge them with existing stored tags; extend the test to cover multiple same-topic lines and a later untagged consolidation.Also applies to: 306-334
🤖 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/vent/consolidate.rs` around lines 79 - 107, Update the consolidation logic around the Group accumulation and agentflare_backend::vent::upsert call to union and deduplicate tags from all same-topic lines, rather than replacing them with the latest line’s tags. Merge this accumulated set with tags already stored for the topic before upsert so a later untagged consolidation preserves existing tags. Extend the relevant test to cover multiple same-topic lines and a subsequent untagged consolidation.
🤖 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 `@src/mcp_server.rs`:
- Around line 1206-1213: The GitHub filing flow is not retry-safe when remote
issue creation succeeds before mark_filed. In src/mcp_server.rs lines 1206-1213,
update the filing operation to persist and reconcile a durable pending record,
returning or recovering its existing issue before creating another; in
src/cli/vent.rs lines 87-94, use the same durable filing/reconciliation flow
before retrying gh issue create. Ensure retries for the same topics do not
create duplicate issues.
- Around line 1205-1208: Update create_github_issue and its call from the
title/body match to enforce a finite deadline for the gh subprocess, terminating
the child process when the timeout expires and propagating a clear internal
error through the existing map_err path. Preserve successful issue creation and
the current error context for ordinary command failures.
In `@src/mcp_server/types.rs`:
- Around line 329-334: Update VentFileRequest and its handling in the mcp_server
request flow to distinguish three states: no title and no body remains a valid
preview request, both fields remain valid for file creation, and exactly one
field must be rejected as invalid parameters. Ensure supplied title or body
values are never silently ignored.
In `@src/vent/capture.rs`:
- Around line 80-85: Serialize the throttle check and subsequent append in the
capture flow around recent_count_for_topic and append, using a cross-process
lock associated with log_path. Acquire the lock before reading the recent count,
hold it through the limit check and append, and release it afterward so
concurrent callers cannot bypass the per-topic limit.
- Around line 57-61: Update the timestamp predicate in the capture filter to
require the parsed UTC timestamp is less than or equal to now before checking
whether its age is within THROTTLE_WINDOW_SECS. Preserve the existing false
result for unparseable timestamps and exclude future-dated entries.
In `@src/vent/file.rs`:
- Around line 95-100: Update create_github_issue so the gh child process has a
finite execution timeout instead of waiting indefinitely in Command::output().
Use a cancellable process flow that terminates and reaps the child when the
timeout expires, while preserving the existing stdout result and propagating
launch or execution errors.
- Around line 46-48: Update the pending_batch flow and its callers in vent.rs
and mcp_server.rs to atomically claim topics before issue creation, using a
cross-process lock or durable in-flight claim that spans reading pending topics,
creating the issue, and marking them filed. Ensure concurrent invocations cannot
select the same topics and that stale reads cannot overwrite filed entries.
- Around line 29-40: Update load_filed to return and propagate file-read and
JSON-parse errors instead of defaulting to an empty BTreeMap, and update its
callers accordingly. In save_filed, serialize the map and persist it through a
temporary file followed by an atomic rename, preserving the existing
parent-directory creation and error propagation.
- Around line 20-26: Update read_all_lines to stop silently dropping malformed
JSONL records: replace filter_map(...ok()) with explicit per-line parsing that
reports the line number and returns or propagates a parse error, or logs each
rejected record if the function must retain its Vec<VentLine> return type.
Preserve successful record collection and existing file-read behavior.
---
Outside diff comments:
In `@src/vent/consolidate.rs`:
- Around line 79-107: Update the consolidation logic around the Group
accumulation and agentflare_backend::vent::upsert call to union and deduplicate
tags from all same-topic lines, rather than replacing them with the latest
line’s tags. Merge this accumulated set with tags already stored for the topic
before upsert so a later untagged consolidation preserves existing tags. Extend
the relevant test to cover multiple same-topic lines and a subsequent untagged
consolidation.
🪄 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: c682b90a-6fa2-458a-a1b4-1db876365ca2
📒 Files selected for processing (11)
src/cli/vent.rssrc/init.rssrc/mcp_server.rssrc/mcp_server/types.rssrc/rule_text.rssrc/vent/capture.rssrc/vent/classify.rssrc/vent/consolidate.rssrc/vent/file.rssrc/vent/mod.rssrc/vent/paths.rs
Comment on lines
+1205
to
+1208
| (Some(title), Some(body)) => { | ||
| let url = crate::vent::file::create_github_issue(&title, &body).map_err(|e| { | ||
| ErrorData::internal_error(format!("gh issue create failed: {e}"), None) | ||
| })?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound the gh subprocess execution.
create_github_issue synchronously waits for gh with no deadline. A stalled network or credential flow can block this MCP request indefinitely; run it with a timeout and terminate it on expiry.
🤖 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.rs` around lines 1205 - 1208, Update create_github_issue and
its call from the title/body match to enforce a finite deadline for the gh
subprocess, terminating the child process when the timeout expires and
propagating a clear internal error through the existing map_err path. Preserve
successful issue creation and the current error context for ordinary command
failures.
Comment on lines
+329
to
+334
| #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] | ||
| pub(crate) struct VentFileRequest { | ||
| #[serde(default)] | ||
| pub(crate) title: Option<String>, | ||
| #[serde(default)] | ||
| pub(crate) body: Option<String>, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject half-specified file requests.
With both fields independently optional, src/mcp_server.rs Lines 1197-1219 treats {title: ...} or {body: ...} as a preview request and silently ignores the supplied value. Keep the no-fields preview case, but reject XOR input as invalid parameters.
🤖 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/types.rs` around lines 329 - 334, Update VentFileRequest and
its handling in the mcp_server request flow to distinguish three states: no
title and no body remains a valid preview request, both fields remain valid for
file creation, and exactly one field must be rejected as invalid parameters.
Ensure supplied title or body values are never silently ignored.
Comment on lines
+80
to
+85
| let topic_key = crate::vent::classify::topic_key(message); | ||
| let now = chrono::Utc::now(); | ||
| if recent_count_for_topic(&log_path, &topic_key, now) >= THROTTLE_MAX_PER_WINDOW { | ||
| return Ok((crate::vent::event_id(message), true)); | ||
| } | ||
| let id = append(&log_path, session, severity, tags, message)?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make throttle check and append atomic.
Concurrent callers can both observe zero recent entries, then both append. This breaks the one-vent-per-topic limit and can inflate downstream pending batches. Serialize the read/check/append critical section with a cross-process lock for the destination log.
🤖 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/vent/capture.rs` around lines 80 - 85, Serialize the throttle check and
subsequent append in the capture flow around recent_count_for_topic and append,
using a cross-process lock associated with log_path. Acquire the lock before
reading the recent count, hold it through the limit check and append, and
release it afterward so concurrent callers cannot bypass the per-topic limit.
Comment on lines
+20
to
+26
| fn read_all_lines(log_path: &Path) -> Vec<VentLine> { | ||
| let Ok(text) = std::fs::read_to_string(log_path) else { | ||
| return Vec::new(); | ||
| }; | ||
| text.lines() | ||
| .filter_map(|l| serde_json::from_str::<VentLine>(l).ok()) | ||
| .collect() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Do not silently discard malformed vent records.
filter_map(...ok()) makes truncated or malformed JSONL entries disappear without diagnostics, so the filing pipeline can undercount vents and never explain why a vent was not filed. Return a parse error with the line number, or at minimum log rejected records.
🤖 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/vent/file.rs` around lines 20 - 26, Update read_all_lines to stop
silently dropping malformed JSONL records: replace filter_map(...ok()) with
explicit per-line parsing that reports the line number and returns or propagates
a parse error, or logs each rejected record if the function must retain its
Vec<VentLine> return type. Preserve successful record collection and existing
file-read behavior.
Comment on lines
+29
to
+40
| fn load_filed(filed_path: &Path) -> BTreeMap<String, FiledEntry> { | ||
| let Ok(text) = std::fs::read_to_string(filed_path) else { | ||
| return BTreeMap::new(); | ||
| }; | ||
| serde_json::from_str(&text).unwrap_or_default() | ||
| } | ||
|
|
||
| fn save_filed(filed_path: &Path, filed: &BTreeMap<String, FiledEntry>) -> std::io::Result<()> { | ||
| if let Some(parent) = filed_path.parent() { | ||
| std::fs::create_dir_all(parent)?; | ||
| } | ||
| std::fs::write(filed_path, serde_json::to_string_pretty(filed)?) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fail closed and write filed state atomically.
load_filed treats corrupted or truncated JSON as an empty map, while save_filed overwrites the file in place. A crash during persistence can make every previously filed topic appear pending, causing duplicate GitHub issues. Propagate parse errors and use temp-file-plus-rename or equivalent atomic persistence.
🤖 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/vent/file.rs` around lines 29 - 40, Update load_filed to return and
propagate file-read and JSON-parse errors instead of defaulting to an empty
BTreeMap, and update its callers accordingly. In save_filed, serialize the map
and persist it through a temporary file followed by an atomic rename, preserving
the existing parent-directory creation and error propagation.
Comment on lines
+46
to
+48
| pub fn pending_batch(log_path: &Path, filed_path: &Path) -> Vec<PendingGroup> { | ||
| let lines = read_all_lines(log_path); | ||
| let filed = load_filed(filed_path); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Atomically claim topics before creating an issue.
src/cli/vent.rs Lines 36-141 and src/mcp_server.rs Lines 1197-1219 both read the pending batch, create an issue, and then mark topics filed. Concurrent invocations can therefore create duplicate issues, and stale reads can overwrite each other’s filed entries. Add a cross-process lock or durable in-flight claim spanning the read/create/mark sequence.
Also applies to: 74-90
🤖 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/vent/file.rs` around lines 46 - 48, Update the pending_batch flow and its
callers in vent.rs and mcp_server.rs to atomically claim topics before issue
creation, using a cross-process lock or durable in-flight claim that spans
reading pending topics, creating the issue, and marking them filed. Ensure
concurrent invocations cannot select the same topics and that stale reads cannot
overwrite filed entries.
… args append_routed's throttled path called event_id(message) again to build its return value, but event_id() salts with the current timestamp, so the id returned for a suppressed vent never matched any logged entry. It now returns the real id of the matching recent log line instead. Also stop silently listing pending vents when only one of --title/--body (or MCP title/body) is given to vent file -- treat that as an error instead of quietly ignoring the partial input.
pending_batch excluded any topic_key ever recorded in filed_path, so a bug that regresses (or was never actually fixed) after its issue is filed would never resurface for re-filing. Add a 30-day TTL (FILED_TTL_SECS) so a filed topic becomes eligible again once enough time has passed, based purely on the existing filed_at timestamp -- no extra gh calls needed.
- recent_matches_for_topic: exclude future-dated log entries instead of letting a negative age satisfy "< THROTTLE_WINDOW_SECS" by sign alone. - save_filed: write via temp file + rename so a killed process can't leave filed.json truncated/corrupt. - create_github_issue: bound the gh subprocess with a 30s timeout (reusing ipc::process::run_with_timeout) so a hung gh can't block the calling MCP tool call or CLI invocation forever. - CLI and MCP vent-file paths: surface the created issue_url even when mark_filed fails afterward, instead of only reporting the write error -- losing the URL there was the only way an operator could avoid filing a duplicate issue on retry.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/vent/file.rs`:
- Around line 43-49: The filing workflow must serialize the
load→select→create-issue→mark sequence across CLI and MCP processes to prevent
duplicate issues and lost filed entries. Add a cross-process lock or durable
in-flight claim around the shared workflow, including state recording, and
update save_filed to use a unique temporary path per write before renaming.
- Around line 116-138: Update the issue-creation flow around run_with_timeout to
spawn gh as a Child instead of calling blocking Command::output(). Ensure the
timeout path terminates the child and waits for it to be reaped before returning
the timed-out error, preventing retries from racing with a still-running gh
process. Preserve collection and propagation of the child’s output and existing
success/error behavior for completed commands.
- Around line 78-81: Update the filtering logic around the filed.get lookup so
future-dated filed_at values are treated as expired/untrusted rather than
producing a negative age; retain the existing FILED_TTL_SECS expiration behavior
for timestamps at or before now. Add a regression test covering a future
filed_at entry and verify the topic is not suppressed.
🪄 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: 2a435664-b689-4b1b-837d-ed804b3db44a
📒 Files selected for processing (9)
src/cli/vent.rssrc/init.rssrc/ipc/process.rssrc/mcp_server.rssrc/vent/capture.rssrc/vent/classify.rssrc/vent/consolidate.rssrc/vent/file.rssrc/vent/paths.rs
💤 Files with no reviewable changes (1)
- src/ipc/process.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- src/vent/classify.rs
- src/vent/consolidate.rs
- src/vent/paths.rs
- src/cli/vent.rs
- src/init.rs
- src/mcp_server.rs
- src/vent/capture.rs
Comment on lines
+43
to
+49
| fn save_filed(filed_path: &Path, filed: &BTreeMap<String, FiledEntry>) -> std::io::Result<()> { | ||
| if let Some(parent) = filed_path.parent() { | ||
| std::fs::create_dir_all(parent)?; | ||
| } | ||
| let tmp_path = filed_path.with_extension("json.tmp"); | ||
| std::fs::write(&tmp_path, serde_json::to_string_pretty(filed)?)?; | ||
| std::fs::rename(&tmp_path, filed_path) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize filing claims and filed-state updates.
The fixed *.json.tmp path races between CLI and MCP invocations, and rename does not serialize the load→select→create-issue→mark flow. Concurrent calls can overwrite filed entries or create duplicate issues for the same topics. Use a cross-process lock or durable in-flight claim spanning batch selection through state recording; use unique temp files for persistence.
🤖 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/vent/file.rs` around lines 43 - 49, The filing workflow must serialize
the load→select→create-issue→mark sequence across CLI and MCP processes to
prevent duplicate issues and lost filed entries. Add a cross-process lock or
durable in-flight claim around the shared workflow, including state recording,
and update save_filed to use a unique temporary path per write before renaming.
Comment on lines
+78
to
+81
| .filter(|g| match filed.get(&g.topic_key) { | ||
| Some(entry) => now - entry.filed_at >= FILED_TTL_SECS, | ||
| None => true, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Ignore future-dated filed entries.
A future filed_at makes now - filed_at negative, suppressing the topic until 30 days after that future timestamp. Treat future entries as expired/untrusted and add a regression test.
Proposed fix
- Some(entry) => now - entry.filed_at >= FILED_TTL_SECS,
+ Some(entry) if entry.filed_at <= now => {
+ now.saturating_sub(entry.filed_at) >= FILED_TTL_SECS
+ }
+ Some(_) => true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .filter(|g| match filed.get(&g.topic_key) { | |
| Some(entry) => now - entry.filed_at >= FILED_TTL_SECS, | |
| None => true, | |
| }) | |
| .filter(|g| match filed.get(&g.topic_key) { | |
| Some(entry) if entry.filed_at <= now => { | |
| now.saturating_sub(entry.filed_at) >= FILED_TTL_SECS | |
| } | |
| Some(_) => true, | |
| None => true, | |
| }) |
🤖 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/vent/file.rs` around lines 78 - 81, Update the filtering logic around the
filed.get lookup so future-dated filed_at values are treated as
expired/untrusted rather than producing a negative age; retain the existing
FILED_TTL_SECS expiration behavior for timestamps at or before now. Add a
regression test covering a future filed_at entry and verify the topic is not
suppressed.
Comment on lines
+116
to
+138
| let output = crate::ipc::process::run_with_timeout( | ||
| move || { | ||
| std::process::Command::new("gh") | ||
| .args([ | ||
| "issue", | ||
| "create", | ||
| "--repo", | ||
| "getappz/agentflare", | ||
| "--title", | ||
| &title, | ||
| "--body", | ||
| &body, | ||
| ]) | ||
| .output() | ||
| }, | ||
| GH_ISSUE_CREATE_TIMEOUT, | ||
| ) | ||
| .map_err(|_| { | ||
| std::io::Error::new( | ||
| std::io::ErrorKind::TimedOut, | ||
| format!("gh issue create did not finish within {GH_ISSUE_CREATE_TIMEOUT:?}"), | ||
| ) | ||
| })??; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Terminate and reap gh on timeout.
run_with_timeout returns after 30 seconds but leaves its worker thread—and its blocking Command::output() child—running. gh may create an issue after the caller receives a timeout and retries. Spawn Child directly, then kill and wait for it when the deadline expires.
🤖 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/vent/file.rs` around lines 116 - 138, Update the issue-creation flow
around run_with_timeout to spawn gh as a Child instead of calling blocking
Command::output(). Ensure the timeout path terminates the child and waits for it
to be reaped before returning the timed-out error, preventing retries from
racing with a still-running gh process. Preserve collection and propagation of
the child’s output and existing success/error behavior for completed commands.
…y-throttle # Conflicts: # src/ipc/process.rs
getappz
changed the title
getappz
added a commit
that referenced
this pull request
Jul 26, 2026
* store: migrate artifact storage onto agentflare-store documents+blobs ArtifactStore gains with_store(Store) constructor backed by documents + blobs (content-addressed, gzip, deduped). Metadata serialized to documents.metadata JSON. Version history via doc_history. FTS5 search replaces flat-file scan (fixes #180). Dashboard serves artifacts under /artifacts/ via axum routes (index, artifact page, version page, versions JSON, SSE live). No more random-port URL — uses dashboard port. Integration: ensure_artifact_server, CLI handoff, standalone serve all default to store backend, fall back to flat-file. 32 tests pass (26 original + 6 new store-backed). * hook_redirect: block agent shell commands from deleting agentflare db files opencode ran an rm mid-migration and silently wiped store.db's metadata, recovered by hand from a pre-migration flat-file backup that happened to still exist. Extend the shared PreToolUse classifier to deny destructive shell commands targeting agentflare's own db files or the data dir itself, for Bash and PowerShell tool calls. * store: fix doc_history skip check for blob-backed callers, clippy nits DocUpsertOpts gained a track_history field upstream since this branch's artifact-store migration was written; doc_upsert_with_opts's history-skip check compared old_content != content, but blob-backed callers (artifacts) always pass an empty content string and store the real payload via blob_hash, so the check was always a no-op false and history rows never got recorded. Compare blob_hash too. Also fixes two clippy findings (io_other_error, needless_borrow) surfaced by -D warnings. * coaching: embedded agent-managed rules with tier/sync, materialization, staleness, CLI RuleTier (Builtin/Override), sync fields on CoachingRule, parse/write. apply_rule/remove_rule carry tier+sync, sync_targets_for_host query. rule_targets merges coaching rules per host (per-file or joined). Snapshot previous body on builtin overwrite; is_stale_rule checks it. sync_now/unsync_host for immediate materialization. CLI: --tier/--sync flags, sync subcommand. Tested: 112 pass across coaching/components/init/hook. * coaching: prune stale coaching-rule entries from opencode.jsonc wire_opencode_instructions's doc comment promised removing entries for rules no longer synced, but only the hardcoded legacy engram.md path was ever pruned -- unsync_host deleting a coaching rule's file left a dangling reference to it in opencode.jsonc forever. Retain only array entries under our rules_dir whose filename is still expected. * task/366: wip config_loader scaffold * flare-git-core: add config_loader for ~/.agentflare/config.toml * flare-git-core: add policy_config to merge git_shim config.toml layers * flare-git-core: thread ResolvedGitShimPolicy through classify_pure and resolve_trust_root_touch * flare-git-core: end-to-end test for config.toml relaxing git-shim policy * fmt: apply cargo fmt and sync Cargo.lock for new deps CI's fmt job was failing on unformatted coaching/dashboard/mcp_server changes, and clippy's --locked check was failing because Cargo.lock hadn't been regenerated after adding toml/thiserror/agentflare-store/ blake3 as dependencies. * flare-git-core: box toml::de::Error in LoaderError to fix clippy::result_large_err toml::de::Error is >128 bytes, so embedding it directly in LoaderError tripped clippy::result_large_err (denied via -D warnings) on every function returning Result<_, LoaderError>. * paths: fix test-isolation race in with_temp_home/with_temp_cwd Root cause of the intermittent build (windows-latest) CI failures in state::tests::* and vent::capture::tests::*: with_temp_home/with_temp_cwd reused a single fixed directory name across every call. A mutex serialized the env-var mutation itself, but under cargo test's default parallel runner and heavy concurrent filesystem load elsewhere in the 811-test suite, a previous call's directory could still be non-empty (or its file handles not yet released) by the time the next call reused the same path, leaking persisted state (SQLite store contents, vent log entries) from one test into an unrelated one. Fixed by giving each call a uniquely-named tempfile::tempdir() instead of a shared fixed name, so no two calls can ever collide on the same directory regardless of timing. Also made both helpers panic-safe via Drop guards, so a failing assertion inside the wrapped closure can no longer leave AGENTFLARE_HOME_OVERRIDE (or the cwd) permanently altered for whatever the test binary runs next. Verified: 7 consecutive clean cargo test --workspace / -p agentflare runs (0 failures) after the fix, versus 100% reproducible failure before it. Added two regression tests exercising with_temp_home under real thread contention.
7 tasks done
5 tasks done
6 tasks done
getappz
added a commit
that referenced
this pull request
Aug 3, 2026
- Dedupe agent-name resolution: export agent_registry::agent_by_name (canonicalize + registry lookup) as the one place every agent-name string resolves through, and use it in router.rs's TOML parsing and work.rs's --agent/assignee_agent paths. Previously only the assignee_agent path canonicalized, so '--agent claude' and 'default = \claude\' in config.toml silently failed where assignee_agent=\claude\ worked. - Remove the work.rs .expect() after claiming: resolve_agent is now called once, post-claim, for both the explicit and auto-routed paths, instead of a separate pre-claim lookup whose result was assumed (rather than proven) to still hold after the claim. - load_router_config now only treats ErrorKind::NotFound as 'no config yet'; any other read error (permissions, etc.) warns instead of silently degrading to the empty config. - RuleMatch's kind/size comparison is now case-insensitive, matching the labels comparison it sits next to instead of diverging from it. - resolve_agent now returns (Agent, reason) and agentflare work prints the reason alongside the agent name, so an auto-routed pick is self-explaining instead of just a bare id. - Doc fixes: router.rs's module comment no longer references a TaskRouter type that was never actually created; route()'s doc now states explicitly (with the rationale) that explicit assignment is the one path that bypasses the installed check; WorkArgs's --agent doc no longer claims config.toml rules are gated on #331 landing (the loader shipped in this same PR). - Adds a double-encoded-metadata test for parsed_size (parsed_kind already had one) and a test proving explicit assignment survives route() even when the assigned agent isn't in installed. Addresses all 3 actionable comments and the RouteDecision.reason nitpick from CodeRabbit's review; the detect_all_with cold-start-cost observation is left as-is per its own 'measure before changing' caveat, and dashboard display of the reason stays out of scope.
getappz
added a commit
that referenced
this pull request
Aug 3, 2026
…tegration, config.toml loader (#380) * feat(agent-registry): add task-attribute agent router Adds the missing fourth piece from item #419: given a claimed task (labels/kind/size) and the set of installed agents, decide which one should run it. RouteDecision always carries a reason. Precedence: explicit assignment > first matching rule with an installed preference > default (only if installed) > None (caller falls back, e.g. cede). Named TaskRouter/TaskContext rather than reusing optimize::runtime's Router/RouteContext, which is a different, prompt-text-based nudge for per-call model routing -- kept as is to avoid an unrelated rename. Scoped to the decision function only. Loading rules from ~/.agentflare/config.toml is left to #331 (unified config, still in progress) so this doesn't duplicate that surface ahead of time. * feat(work): auto-route to an agent when --agent is omitted Wires agent_registry::route into 'agentflare work' as its first real consumer. --agent still wins when given (fails fast before claiming, same as before). Omit it and the item is claimed first, then routed from its own attributes via a TaskContext (labels, metadata.size, and critically assignee_agent) against the installed-agent set from detect_all_with. RouterConfig::default() has no rules yet, so today this only resolves when the item itself already names an assignee -- work.rs previously ignored that field entirely and always required a manual --agent. Rule- based routing from ~/.agentflare/config.toml activates automatically once #331 lands, with no further change here. Widens mcp_server::item::parsed_size to pub(crate) to reuse the existing (and already metadata-double-encoding-safe) size parser instead of duplicating it. * feat(router): parse the [router] table from ~/.agentflare/config.toml Adds agent_registry::parse_router_config: reads a config.toml's [router] table (default agent + [[router.rule]] entries with a labels/kind/size 'when' and a use preference list, single string or array) into a RouterConfig. Every other top-level table in the file is ignored -- this only ever reads its own corner, so it composes cleanly once #331 (unified config) actually owns the file. An agent name that doesn't resolve (typo, or unknown to this build's registry) is dropped from wherever it appeared rather than failing the whole parse -- route() already treats a missing preference as 'try the next one', so a bad name behaves like one that's merely not installed. Only malformed TOML is a hard Err. Wires it into 'agentflare work': load_router_config() reads the file (missing file -> the empty config, parse error -> a warning + the empty config, never a crash) and resolve_agent() now takes it instead of a hardcoded RouterConfig::default(). Auto-routing (--agent omitted) can now match a labels/kind/size rule, not just an item's own assignee_agent. Adds the toml crate (0.8, already resolved transitively in the lockfile) as agent-registry's first non-clap/serde dependency -- TOML parsing is exactly the kind of already-solved-by-a-library problem worth reaching for one over a hand-rolled subset parser for. * feat(router): wire TaskContext.kind from item metadata Adds mcp_server::item::parsed_kind, same metadata-blob convention as the existing parsed_size ({\kind\: \locate\}) but free-form rather than a fixed S/M/L set, since task shape isn't a small enumerable list. Factors the shared double-encoding-safe JSON unwrap the two now have in common out into parsed_metadata so the workaround lives in exactly one place. resolve_agent's TaskContext.kind was hardcoded to None (no item field mapped to it yet) -- now reads parsed_kind, so a '[[router.rule]] when.kind = \...\' rule can actually match once something sets that metadata key. Nothing writes it yet (no kind UI/tool surface exists), same bootstrapping state size was in before groom's UI grew a way to set it. 7 new tests (5 for parsed_kind/parsed_size directly, 2 for a kind-matching rule through resolve_agent); moved the new test module to the end of item.rs after clippy flagged items-after-test-module from the pre-existing top-of-file placement. * style: cargo fmt * fix(router): address CodeRabbit review on PR #380 - Dedupe agent-name resolution: export agent_registry::agent_by_name (canonicalize + registry lookup) as the one place every agent-name string resolves through, and use it in router.rs's TOML parsing and work.rs's --agent/assignee_agent paths. Previously only the assignee_agent path canonicalized, so '--agent claude' and 'default = \claude\' in config.toml silently failed where assignee_agent=\claude\ worked. - Remove the work.rs .expect() after claiming: resolve_agent is now called once, post-claim, for both the explicit and auto-routed paths, instead of a separate pre-claim lookup whose result was assumed (rather than proven) to still hold after the claim. - load_router_config now only treats ErrorKind::NotFound as 'no config yet'; any other read error (permissions, etc.) warns instead of silently degrading to the empty config. - RuleMatch's kind/size comparison is now case-insensitive, matching the labels comparison it sits next to instead of diverging from it. - resolve_agent now returns (Agent, reason) and agentflare work prints the reason alongside the agent name, so an auto-routed pick is self-explaining instead of just a bare id. - Doc fixes: router.rs's module comment no longer references a TaskRouter type that was never actually created; route()'s doc now states explicitly (with the rationale) that explicit assignment is the one path that bypasses the installed check; WorkArgs's --agent doc no longer claims config.toml rules are gated on #331 landing (the loader shipped in this same PR). - Adds a double-encoded-metadata test for parsed_size (parsed_kind already had one) and a test proving explicit assignment survives route() even when the assigned agent isn't in installed. Addresses all 3 actionable comments and the RouteDecision.reason nitpick from CodeRabbit's review; the detect_all_with cold-start-cost observation is left as-is per its own 'measure before changing' caveat, and dashboard display of the reason stays out of scope.
7 tasks done
getappz
added a commit
that referenced
this pull request
Aug 18, 2026
…550) * fix(worktree): prune stale registration before reusing an existing branch create_worktree() only checked worktree_already_checked_out(), which requires the on-disk directory to still exist. If a worktree dir was removed out-of-band (crash, manual cleanup) while git's own .git/worktrees/<slug>/ registration survived, the branch stayed git-internally tied to the missing path, so git worktree add refused to reuse it, and the failed add itself regenerated a fresh broken registration -- every dispatch attempt reproduced the same failure forever (confirmed live on item #331: three consecutive dispatches, three identical failures). Prune before reusing an existing branch so a stale registration never survives to block it. Also surface the real worktree_error in execute_work_impl instead of a hardcoded generic message, since the underlying git failure was already captured but silently dropped. Agentflare-Agent: claude-code_2-1-234_agent Agentflare-Branch: fix/511-worktree-prune-stale-registration * fix(work-item-pipeline): recover instead of panicking on non-object item metadata persist_run_id blindly cloned the item's existing metadata Value and assigned a key into it via IndexMut. When metadata was corrupted into a double-JSON-encoded string (confirmed live on item #331 -- the literal string "{\"size\": \"M\"}" stored instead of the object {"size": "M"}), parsing it produced Value::String(...), and IndexMut panics assigning a key into anything that isn't already an object -- crashing the whole job with "cannot access key workflow_run_id in JSON string" on every single dispatch attempt. Coerce to an empty object when the existing value isn't already one, instead of trusting its shape. Regression test verified red (exact panic reproduced) without the fix, green with it. Agentflare-Agent: claude-code_2-1-234_agent Agentflare-Branch: fix/511-worktree-prune-stale-registration
This was referenced Aug 18, 2026
getappz
added a commit
that referenced
this pull request
Aug 19, 2026
…m's actual branch differently across call sites — `push_branch` used `resolve_worktree_branch` (checks what's really on disk), while `branch_diverged`/`is_pr_merged`/`relabel_pr_completed`/`pr_ci_status` all used the naive `task_branch_name` guess. When those two disagree (a renamed branch, or a worktree predating the current slugged-naming scheme), `branch_diverged` checks a branch that doesn't match what was actually pushed, finds \"nothing diverged\", and `item_done` returns `Ok` with no push, no PR, no comment, no state change — reproduced live on item #331. (#558) Fix: added `flare_git_core::worktree::resolve_item_task_branch(item, repo_root)` as the single source of truth for \"what branch is this item's worktree actually on\" (reads the real checkout, falls back to `task_branch_name` only if no worktree exists yet), and switched every one of those call sites to use it instead of recomputing the guess independently. Also fixed a related issue this WIP surfaced: `run_or_resume_with_sender`'s periodic heartbeat call didn't carry the owner override across the async workflow-engine thread boundary, so `item_heartbeat` calls made from deep in the loop could silently fail owner checks — same class of bug as the earlier `finalize`-step owner-identity fix (PR #531). Verified: `cargo build --lib` and `cargo build -p flare-git-core --lib` clean; `cargo fmt --check` clean; `cargo clippy --tests -- -A unsafe_code -A clippy::pedantic -D warnings` clean on both `agentflare` and `flare-git-core`; `flare-git-core::worktree::tests` (41/41), `work_item_pipeline::*` (54/54, 2 pre-existing ignores unrelated), and the new `item_pr_failure_tests` integration tests (2/2) all pass.
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
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.
Bundles two changes on vent-origin-classify-throttle:
feat: migrate artifact storage onto agentflare-store documents+blobs
vent: origin routing, throttle, batched filing, judge-prompt hook
(addresses the previous commits on this branch)