feat(claims): leased work-claim ledger so agents don't grab the same issue/PR - #141
Conversation
…issue/PR
A claim = an owner holding a target (issue#N/pr#M) in a repo, stored in a new
SQLite ledger at ~/.agentflare/agentflare.db. Acquire and stale-steal are one
atomic UPSERT (ON CONFLICT ... WHERE done OR heartbeat<now-ttl OR owner=me), so
two agents can never both own a target and a crashed agent's claim is reclaimed
after the TTL. owner = <agent>:<instance> (handoff's agent chain + session/pid);
repo normalized from the origin remote to owner/name.
Surface: MCP tools claim_acquire/claim_heartbeat/claim_release/claim_list on the
flare server, plus `agentflare claim {acquire|heartbeat|release|done|list}`.
Adds src/db.rs (shared agentflare.db opener; gateway secrets fold in later, #138).
Closes #139
getappz
added
enhancement
|
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 SQLite-backed work-claim ledger with TTL-based ownership, repository and agent identity helpers, persistent database access, CLI commands, MCP tools, and a 25-minute CI build timeout. ChangesWork-claim ledger
CI timeout
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant ClaimInterface
participant Database
participant ClaimsLedger
Agent->>ClaimInterface: request claim operation
ClaimInterface->>Database: open and migrate ledger
Database-->>ClaimInterface: SQLite connection
ClaimInterface->>ClaimsLedger: acquire, heartbeat, release, done, or list
ClaimsLedger-->>ClaimInterface: operation result
ClaimsLedger-->>ClaimInterface: operation result
ClaimInterface-->>Agent: CLI output or MCP JSON
Possibly related PRs
🚥 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: 3
🧹 Nitpick comments (1)
src/claims.rs (1)
53-91: 🩺 Stability & Availability | 🔵 TrivialSet a busy timeout in the shared opener Add
busy_timeoutinsrc/db.rs; this ledger is used by parallel agents, so contending writers can fail fast withSQLITE_BUSY. If the database is shared across processes, enable WAL there too.🤖 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/claims.rs` around lines 53 - 91, Add SQLite concurrency settings in the shared database opener in db.rs: configure a busy timeout so parallel writers wait instead of failing immediately with SQLITE_BUSY, and enable WAL mode when the database is shared across processes. Keep claims::acquire unchanged and apply these settings centrally wherever the shared connection is opened.
🤖 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/cli/claim.rs`:
- Around line 67-70: Update the ClaimAction::Acquire flow and the corresponding
MCP acquire path to resolve the normalized provenance repository and commit
together: only retain git_commit() when that repository matches the selected
repository, and otherwise pass no commit provenance. Ensure --repo overrides
never attach HEAD from the current checkout.
In `@src/db.rs`:
- Around line 18-21: Update the database setup around Connection::open to create
the parent directory with 0700 permissions, propagate any create_dir_all error
instead of discarding it, and apply 0600 permissions to the database file after
opening it. Preserve the existing connection error propagation and use
platform-appropriate permission APIs.
In `@src/mcp_server.rs`:
- Around line 726-738: Expose a new claim_done MCP tool alongside claim_release
and the other claim tools, accepting the claim target and optional repository
context through ClaimTargetRequest. Resolve the claim context and current owner
via the existing helpers, invoke the claims completion operation while
preserving the audit row, map failures to ErrorData::internal_error, and return
a JSON response indicating whether completion succeeded along with the repo and
target.
---
Nitpick comments:
In `@src/claims.rs`:
- Around line 53-91: Add SQLite concurrency settings in the shared database
opener in db.rs: configure a busy timeout so parallel writers wait instead of
failing immediately with SQLITE_BUSY, and enable WAL mode when the database is
shared across processes. Keep claims::acquire unchanged and apply these settings
centrally wherever the shared connection is opened.
🪄 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: 7d900965-fe1b-4162-8f8a-53240424b953
📒 Files selected for processing (6)
src/claims.rssrc/cli/claim.rssrc/cli/mod.rssrc/db.rssrc/main.rssrc/mcp_server.rs
| if let Some(parent) = path.parent() { | ||
| let _ = std::fs::create_dir_all(parent); | ||
| } | ||
| let conn = Connection::open(path)?; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## src/db.rs\n'
wc -l src/db.rs
cat -n src/db.rs | sed -n '1,220p'
printf '\n## Search for related filesystem/setup code\n'
rg -n "create_dir_all|chmod|set_permissions|PermissionsExt|0600|0700|agentflare|Connection::open|open_with_flags" src . -g '!target' -g '!node_modules'Repository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Call sites for src/db.rs::open()\n'
rg -n "db::open\(\)|agentflare_db_path\(\)" src
printf '\n## Relevant DB setup helpers\n'
cat -n src/auth_db.rs | sed -n '1,140p'
printf '\n---\n'
cat -n src/gateway_secrets.rs | sed -n '1,120p'Repository: getappz/agentflare
Length of output: 11473
🌐 Web query:
SQLite new database file permissions umask Unix create database default 0644
💡 Result:
When SQLite creates a new database file on Unix-like systems, it uses a default permission mode of 0644 [1][2]. This default is governed by the SQLITE_DEFAULT_FILE_PERMISSIONS compile-time macro, which is set to 0644 by default [1][2]. Crucially, this mode is modified by the process's umask when the database file is initially created [1][3]. For example, if your system umask is 0022, the resulting file permissions will be 0644 (0644 & ~0022 = 0644) [4]. If you need to ensure different permissions (such as 0660 or 0600) for your database file and its associated auxiliary files (like -wal and -shm), there are three primary strategies: 1. Pre-create the file: Create the database file manually before initializing the SQLite connection [3][4]. Since the file already exists, SQLite will open the existing file rather than creating a new one with its own default permissions [5]. 2. Adjust permissions after creation: Use system utilities or programming interfaces (e.g., chmod) to update the file permissions after SQLite has created the file [6]. 3. Recompile SQLite: You can change the default by defining SQLITE_DEFAULT_FILE_PERMISSIONS at compile time (e.g., -DSQLITE_DEFAULT_FILE_PERMISSIONS=0600) [7][8]. Note that for auxiliary files like -journal, -wal, and -shm, SQLite intentionally ignores the system umask to ensure these files maintain the same permissions as the main database file, which is critical for consistent access in environments where multiple processes need to recover or use the database [9][1]. Attempting to modify permissions using umask within the same process is generally discouraged because it affects the entire process, not just the thread handling the database [6].
Citations:
- 1: https://github.com/sqlite/sqlite/blob/397a3c4a/src/os_unix.c
- 2: https://github.com/sqlite/sqlite/blob/da67225f04ad0470b54433fee31baf266e6630ef/src/os_unix.c
- 3: https://www.mail-archive.com/sqlite-users@mailinglists.sqlite.org/msg22296.html
- 4: https://sqlite-users.sqlite.narkive.com/qfeW1sZx/about-default-file-permission-of-sqlite-database-file
- 5: https://sqlite-users.sqlite.narkive.com/8HoxFb5I/sqlite-sqlite3-open-exclusive
- 6: https://www.sqlite.org/src/info/6c4c2b7dba
- 7: https://stackoverflow.com/questions/13701803/set-group-writeable-when-opening-db-with-sqlite-open-create
- 8: Create database file as read-write only for the owner? rusqlite/rusqlite#1743
- 9: https://stackoverflow.com/questions/12610702/can-i-get-a-callback-do-i-know-when-sqlite-has-created-write-ahead-log-files
Restrict ~/.agentflare/agentflare.db permissions.
Connection::open(path) will create the DB with SQLite’s default Unix mode (0644, masked by umask), so this ledger can be left readable to other local users. Create ~/.agentflare with 0700, chmod the DB to 0600, and stop discarding create_dir_all errors so setup failures surface early.
🤖 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/db.rs` around lines 18 - 21, Update the database setup around
Connection::open to create the parent directory with 0700 permissions, propagate
any create_dir_all error instead of discarding it, and apply 0600 permissions to
the database file after opening it. Preserve the existing connection error
propagation and use platform-appropriate permission APIs.
…atabase is locked' Review follow-up on #141. A leased ledger is written by many agent processes at once; with bundled SQLite's default busy_timeout of 0, a contended acquire returned SQLITE_BUSY immediately and surfaced as an error instead of serializing behind the current writer. Set a 5s busy_timeout (writers wait their turn) and WAL (list reads proceed during a write). Also document that CLI ownership continuity across separate invocations needs AGENTFLARE_SESSION (the pid fallback differs per process; a long-lived MCP server is unaffected).
CodeRabbit follow-up on #141: - Restrict agentflare.db to 0600 and ~/.agentflare to 0700 (was SQLite's umask-masked 0644) — matters once #138 folds gateway secrets in. - Don't attach the current checkout's HEAD as claim provenance when --repo / repo= names a different repository (CLI + MCP). - Add the missing claim_done MCP tool so agents can mark a claim complete (keeping its audit row) not just release it.
|
All three review comments addressed in
(The busy_timeout+WAL nitpick was already handled earlier in @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,60p' src/db.rs
echo "---claim.rs---"
sed -n '1,160p' src/cli/claim.rs
echo "---mcp_server.rs claim section---"
rg -n "claim_" src/mcp_server.rs | head -50Length of output: 8768 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '685,800p' src/mcp_server.rsLength of output: 6281 All three fixes verified in the current diff — permissions, provenance-on-override, and the new
Nice work — triggering a full review now to confirm nothing else needs attention. ✅ Action performedReview finished.
|
# Conflicts: # src/main.rs
…pinning a runner for 6h
roundtrip_active_mode and clear_nonexistent_is_noop both read/write the same process-global flag/session files. Cargo runs them on parallel threads, so clear_nonexistent_is_noop's clear_active() could delete the flag file roundtrip_active_mode depends on — a race that passed on Linux but panicked on macOS and hung on Windows (blocking CI). Serialize both behind a Mutex.
1 participant
What
Step 1 of the GitHub-across-agents roadmap: a leased work-claim ledger so
parallel AI agents don't both grab the same GitHub issue/PR. A claim = "owner
holds target (issue#N / pr#M) in repo".
How
~/.agentflare/agentflare.db(SQLite, rusqlite) with aclaimstablePRIMARY KEY(repo, target).src/db.rsis the shared opener;gateway secrets fold in later (refactor(db): consolidate source-of-truth DBs into ~/.agentflare/agentflare.db #138). The rebuildable caches under
~/.local/share/agentflare/stay separate.INSERT ... ON CONFLICT(repo,target) DO UPDATE ... WHERE status='done' OR heartbeat_at < now-ttl OR owner=me. Two agents can never both own a target;a live claim by someone else changes 0 rows → reported as held. (A filesystem
create_newlock can't make the steal atomic — this is why SQLite.)heartbeat_at; past the TTL (default 30 min,AGENTFLARE_CLAIM_TTL_SECS) another agent may steal it, so a crashed agentdoesn't wedge a target. Re-acquiring your own claim refreshes it.
owner = <agent>:<instance>: handoff's chain(
AGENTFLARE_AGENT→agent_detector::agent_name→cli) + instance(
AGENTFLARE_SESSIONelse pid).reponormalized from the origin remote toowner/name(handles https, ssh-alias,.git).Surface
claim_acquire,claim_heartbeat,claim_release,claim_list.agentflare claim {acquire|heartbeat|release|done|list} <target> [--repo].Reference
Models Beads's SQLite-backed
claim/close model, minus the full issue-tracker surface (deps, JSONL git-sync)
— we're a lease over existing GitHub issues/PRs, not a tracker.
Tests
8 ledger unit tests (in-memory db): free→held, own re-acquire refreshes,
stale-steal vs fresh-blocked, done re-acquirable, owner-scoped
heartbeat/release/done, list hides stale/done, repo scoping, repo
normalization. Full suite: 268 passed, 0 failed.
Also driven end-to-end via the CLI (two agents, a real repo): acquire →
block-other → list → heartbeat → owner-scoped release denial → TTL steal after
a real time gap → ownership transfers.
Closes #139 · related #138 (db consolidation), #136/#137 (step 0)
Summary by CodeRabbit
claimCLI with subcommands to acquire, heartbeat (refresh), release, mark done, and list claims, including repo auto-detection and status/staleness indicators.