Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 20 additions & 23 deletions crates/agentflare-backend/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,20 +439,25 @@ pub fn claim(
/// (contrast with the old `claim_done`, which did both atomically): the
/// `"done"` MCP arm calls this, then runs `worktree::push_and_open_pr`
/// (which needs the lease to still look held so a concurrent `claim()` on
/// this same item can't grab it while the PR is still being opened — see
/// item #37), and only releases the lease itself afterward via
/// `crate::claim::done`. Returns `false` (no-op, no state change) if
/// `owner` doesn't currently hold the claim.
/// the same item between mark_completed and the deferred release below is
/// still correctly rejected), and only *after* publish releases the lease
/// via `claim::done`. Returns `Ok(true)` when the item was actually moved
/// to completed, `Ok(false)` when the caller doesn't own the claim.
pub fn mark_completed(conn: &Connection, item_id: &str, owner: &str) -> Result<bool> {
// One transaction start to finish so the ownership check can't go stale
// between the guard and the write — without this, a concurrent
// release()+claim() by a different owner could slip in between the
// check and update_state below, completing the item out from under its
// new owner.
let tx = conn.unchecked_transaction()?;
if !crate::claim::is_owner(&tx, item_id, owner)? {
tx.commit()?;
return Ok(false);
}
let item = get(&tx, item_id)?;
let completed_state = crate::state::first_in_group(&tx, &item.project_id, "completed")?;
update_state(&tx, item_id, &completed_state.id)?;
tx.commit()?;
// Keep the claim lease held for the MCP caller's deferred release.
Ok(true)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down Expand Up @@ -1039,7 +1044,7 @@ mod tests {
}

#[test]
fn mark_completed_moves_to_completed_state_and_a_later_release_makes_it_reclaimable() {
fn mark_completed_moves_to_completed_state_and_lease_stays_held() {
let conn = db::open_in_memory().unwrap();
let (pid, sid) = seed_project(&conn, "");
let item = make_item(&conn, &pid, &sid);
Expand All @@ -1049,31 +1054,25 @@ mod tests {
assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed"));
assert!(done_item.completed_at.is_some());

// The lease is still held by "agent:1" at this point (item #37: the
// lease must outlive the state transition) — a concurrent claimer
// must be rejected...
let blocked = claim(&conn, &item.id, "agent:2", 1150, TTL).unwrap();
assert!(matches!(
blocked,
crate::claim::Acquire::Held { ref owner, .. } if owner == "agent:1"
));
// Lease is still held — concurrent claim must be rejected.
match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() {
crate::claim::Acquire::Held { .. } => {}
other => panic!("expected Held after mark_completed, got {other:?}"),
}

// ...only after the lease is actually released does the item become
// reclaimable by anyone.
assert!(crate::claim::done(&conn, &item.id, "agent:1", 1100).unwrap());
let outcome = claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap();
// Release the lease, now re-acquirable.
assert!(crate::claim::done(&conn, &item.id, "agent:1", 1300).unwrap());
let outcome = claim(&conn, &item.id, "agent:2", 1400, TTL).unwrap();
assert_eq!(outcome, crate::claim::Acquire::Acquired);
}

#[test]
fn mark_completed_is_a_noop_when_owner_does_not_hold_the_claim() {
fn mark_completed_noop_for_non_owner() {
let conn = db::open_in_memory().unwrap();
let (pid, sid) = seed_project(&conn, "");
let item = make_item(&conn, &pid, &sid);
claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap();
assert!(!mark_completed(&conn, &item.id, "agent:2").unwrap());
let unchanged = get(&conn, &item.id).unwrap();
assert_ne!(unchanged.state_id, state_in_group(&conn, &pid, "completed"));
}

#[test]
Expand All @@ -1086,10 +1085,8 @@ mod tests {
assert!(!crate::claim::heartbeat(&conn, &item.id, "agent:2", 1100).unwrap());
assert!(!crate::claim::release(&conn, &item.id, "agent:2").unwrap());
assert!(!crate::claim::done(&conn, &item.id, "agent:2", 1100).unwrap());
assert!(!mark_completed(&conn, &item.id, "agent:2").unwrap());

assert!(crate::claim::heartbeat(&conn, &item.id, "agent:1", 1100).unwrap());
assert!(mark_completed(&conn, &item.id, "agent:1").unwrap());
assert!(crate::claim::done(&conn, &item.id, "agent:1", 1200).unwrap());
}
}
12 changes: 6 additions & 6 deletions crates/agentflare-db-kit/src/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,15 @@ impl ClaimLedger {
Ok(conn.execute(&sql, params.as_slice())? > 0)
}

/// True if `owner` currently holds the claim record for this key
/// (regardless of status) — lets a caller gate a follow-up action on
/// still owning the lease without mutating anything.
/// Ownership check without mutation — used by the `mark_completed` +
/// deferred-release split to verify the caller still holds the claim
/// before advancing the item's state.
pub fn is_owner(&self, conn: &Connection, key: &[&str], owner: &str) -> rusqlite::Result<bool> {
let owner_p = key.len() + 1;
let sql = format!(
"SELECT 1 FROM {t} WHERE {pred} AND owner = ?{owner_p} LIMIT 1",
"SELECT 1 FROM {t} WHERE {pred} AND owner = ?{owner_p} AND status = 'claimed'",
t = self.table,
pred = self.where_pred()
pred = self.where_pred(),
owner_p = key.len() + 1
);
let mut params = self.key_params(key);
params.push(&owner);
Expand Down
2 changes: 1 addition & 1 deletion src/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ pub fn get_components(host: &str) -> Vec<Component> {
// (`gateway_integrations::LEANCTX`) and, for claude-code, strip
// whatever native entry the upstream onboarder already created so
// the same ~80 ctx_* tools aren't declared twice.
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew), registered behind the agentflare gateway (tool_search/tool_execute), not the host's native tool list".to_string(),
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew), registered behind the agentflare gateway (the `tool` action-dispatch), not the host's native tool list".to_string(),
check: Box::new(|| {
crate::tool_install::installed(&crate::tool_install::LEAN_CTX)
&& crate::gateway_integrations::already_registered("leanctx")
Expand Down
12 changes: 6 additions & 6 deletions src/gateway_integrations.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// During `init`, detect project context (e.g. a GitHub remote) and, with the
// user's OK, register the matching MCP server BEHIND agentflare's own gateway
// (`~/.agentflare/gateway.toml`) — so its tools stay reachable through
// `tool_search`/`tool_execute` instead of bloating the host's always-on
// tool list. Adding another gateway-fronted MCP later is one more entry in
// (`~/.agentflare/gateway.toml`) — so its tools stay reachable through the
// `tool` action-dispatch (search/execute) instead of bloating the host's
// always-on tool list. Adding another gateway-fronted MCP later is one more entry in
// `INTEGRATIONS`; the plumbing (detect → consent → idempotent append) is shared.
use crate::paths::home;
use std::fs;
Expand All @@ -29,7 +29,7 @@ pub const INTEGRATIONS: &[GatewayIntegration] = &[GITHUB, LEANCTX];
const GITHUB: GatewayIntegration = GatewayIntegration {
name: "github",
detect: git_remote_is_github,
prompt: "⚑ GitHub repo detected. github-mcp-server can sit behind the agentflare gateway\n (its tools stay under tool_search/tool_execute, not the host's tool list).",
prompt: "⚑ GitHub repo detected. github-mcp-server can sit behind the agentflare gateway\n (its tools stay under the `tool` action-dispatch, not the host's tool list).",
// Remote HTTP backend — zero-install (no docker/binary). The gateway
// sends `auth_header` verbatim, so the stored secret is the full header
// value (`Bearer <token>`), see `post_note`.
Expand All @@ -54,7 +54,7 @@ fn github_post_note() -> Vec<String> {
pub const LEANCTX: GatewayIntegration = GatewayIntegration {
name: "leanctx",
detect: leanctx_installed,
prompt: "⚑ lean-ctx detected. Its ~80 ctx_* tools can sit behind the agentflare gateway\n (reachable via tool_search/tool_execute) instead of bloating the host's tool list.",
prompt: "⚑ lean-ctx detected. Its ~80 ctx_* tools can sit behind the agentflare gateway\n (reachable via the `tool` action-dispatch) instead of bloating the host's tool list.",
// Local stdio backend — same binary lean-ctx's own installer already put
// on PATH; the gateway just spawns it instead of the host declaring it
// natively. No auth needed (local process).
Expand All @@ -68,7 +68,7 @@ fn leanctx_installed() -> bool {

fn leanctx_post_note() -> Vec<String> {
vec![
" next its ctx_* tools are now reached via tool_search/tool_execute, not called natively"
" next its ctx_* tools are now reached via the `tool` action-dispatch, not called natively"
.to_string(),
]
}
Expand Down
Loading
Loading