handoff: assign items + attach versioned assets instead of raw artifacts - #169
Merged
Merged
Conversation
…ifact_list artifact_list has no project scoping (filters only by session_id/recipient/ thread_id), so /handoff inbox returned artifacts across every project the agent has ever touched. item(action=list) already scopes to this repo's linked project. Repoint the inbox grammar there, filtering to items whose assignee_agent matches the caller (or is unassigned); pull full content via artifact_get/asset only when needed. thread <id> stays on artifact_list since a thread id is already a precise, non-leaky lookup.
…versioned asset Previously fn handoff published straight to the flat-file ArtifactStore, never touching an item — so a completed handoff (e.g. opencode's MCP host wiring report) could sit unreflected in any tracked item. Now handoff: - assigns an existing item (item_id) or creates one in the repo's linked project, with assignee_agent set to the recipient - attaches the content to that item as an asset (entity_type=item_attachment) instead of publishing an artifact - re-attaching under the same item_id/filename becomes the next asset version rather than a duplicate row — assets gained a version column (migration 0003), computed in asset::create from existing rows sharing (entity_type, entity_id, filename); no base_version conflict-check needed since items already serialize access via claim/heartbeat mcp_prompts.rs's /handoff grammar is rewritten to match (send assigns+ attaches, inbox already used item list, thread now walks item metadata + assets instead of artifact_list). The artifact/HTTP-serving crate and its publish/list/get/delete tools are untouched and still fully functional — just marked deprecated for the handoff use case in the /artifact prompt text, per instruction to retire rather than remove them. This branch is rebased onto drop-backend-prefix (not master): master doesn't yet have the consolidated item/asset/label/project tools this change depends on.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server.rs`:
- Around line 1132-1163: Update the asset creation flow around
Self::slugify(&name) so the generated filename remains stable for the same
item.id across handoffs, ensuring versioning continues its existing chain when
the handoff name changes. Preserve the current extension and storage-path
behavior while deriving the stable filename from item.id rather than the mutable
name.
🪄 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: addb354f-09dd-444a-a6fa-5357336905bc
📒 Files selected for processing (5)
crates/agentflare-backend/src/asset.rscrates/agentflare-backend/src/db.rscrates/agentflare-backend/src/migrations/0003_asset_versioning.sqlsrc/mcp_prompts.rssrc/mcp_server.rs
CodeRabbit (PR #169): versioning is computed from (entity_type, entity_id, filename), and filename was derived from slugify(&name). Replying to the same item_id with a different brief (the common case — a reply's brief differs from the original ask) silently started a new filename chain at version 1 instead of continuing it, contradicting HandoffRequest's own doc comment. Filename is now derived from item.id, which is stable across every call on the same item regardless of what name/brief is passed.
4 tasks
This was referenced Jul 18, 2026
getappz
added a commit
that referenced
this pull request
Jul 18, 2026
) Splits the file that grew to 3.6x over its own LOC-gate frozen ceiling into 13 sibling files under src/mcp_server/, following the delegation pattern item.rs already established (each MCP tool method's body moves to a pub fn in its own file; the #[tool_router]-tagged impl block in mcp_server.rs keeps only thin one-line dispatchers, so the router macro's exactly_one_tool_router_block_exists invariant still holds). - types.rs: request/response structs + small shared helpers (856 lines) - artifact.rs, asset.rs, claim.rs, comment.rs, flare_git.rs, handoff.rs, memory_tool.rs, review.rs: one file per tool's method body - tests/: the former ~3580-line monolithic test module, split by domain into mod.rs (shared harness + misc tests), item_tests.rs, asset_tests.rs, artifact_tests.rs, action_tests.rs -- all under the 1500-line limit for new files Widely-shared helpers used across many tools (with_backend_db, resolve_project, claim_db, resolve_repo_or_err, run_git, etc.) stay in mcp_server.rs itself rather than being assigned to one tool's file. Verified: all 619 tests pass (confirmed identical test-name set before and after via diff, after catching and fixing one dropped #[test] attribute at a file-split boundary); cargo clippy with CI's exact flags clean (aside from the pre-existing Windows-only agent_launch.rs import, tracked as #169); scripts/loc-gate.sh passes clean, no allowlist bump needed; .githooks/pre-commit's staged-file LOC check now passes without --no-verify. mcp_server.rs remains on the LOC gate's frozen allowlist at 1521 lines (down from 7249) -- under the 2000 ceiling but not yet under the 1500 line ordinarily required to drop off the allowlist entirely; the remaining content is mostly cross-tool shared infrastructure that doesn't cleanly belong to any single tool's file.
4 tasks
getappz
added a commit
that referenced
this pull request
Jul 18, 2026
…tion to nanoid (#257) * item/claim: accept numeric sequence_id or #-prefixed id; switch id generation to nanoid item/claim MCP tools (get, update, update_state, delete, claim, heartbeat, release, done, add_label, remove_label; claim's target param) now accept either a UUID or a numeric sequence_id (bare or #-prefixed), resolved via agentflare_backend::item::resolve_id scoped to the repo's linked project. Not-found numeric ids return the same not-found shape as an unmatched UUID. Closes #184. Also switches db_kit::ids::new_id() from uuid::Uuid::now_v7() to nanoid::nanoid!(), updating every caller across agentflare-artifacts and agentflare-backend (asset/comment/label/project/state/webhook/workspace). cargo build --workspace --all-features, cargo test --workspace (630+ passing across the bin plus every crate), cargo fmt --check, and cargo clippy --workspace --all-features -D warnings all clean (the one remaining clippy hit is the pre-existing Windows-only agent_launch.rs test import, tracked separately as item #169, unrelated to this change). * item/claim: wire item_get through sequence_id resolution, support #-prefix, fix flaky nanoid test Review of task/184 (item #186) found the PR didn't actually cover its own motivating case: item_get -- the exact call from #184's bug report (item(get, id="178")) -- was never wired to resolve_item_id, and resolve_id's numeric parse didn't strip a leading '#', so #-prefixed sequence ids silently fell through to the UUID passthrough branch instead of resolving, despite both the commit message and the tool's own schema description claiming that support. - item_get now resolves through resolve_item_id, closing the original gap - resolve_id strips a leading '#' before parsing as numeric - Added the tests #184 explicitly required and that were missing: bare numeric, #-prefixed numeric, not-found numeric, project-scoped lookup, end-to-end via the item MCP tool - Fixed a flaky test: handoff_tool_requires_recipient_and_assigns_item asserted a filename via item_id.to_lowercase(), which doesn't match production's actual AgentflareMcp::slugify() transform -- slugify also collapses '_' to '-', which to_lowercase() doesn't, so the assertion failed whenever a randomly-generated nanoid id happened to contain an underscore. Now asserts against the real transform. cargo test --workspace (632 passing, 0 failed), cargo fmt --check, and cargo clippy --workspace --all-features -D warnings all clean.
3 tasks
This was referenced Jul 19, 2026
Merged
4 tasks
getappz
pushed a commit
that referenced
this pull request
Aug 21, 2026
PR #581's review (comment on item #169) found 2 critical bugs and 3 moderate issues in the combined verification-before-completion / finishing-a-development-branch gate. This fixes all five: 1. is_verification_command did a blunt substring match, so a command that merely mentioned a marker (grep -rn "cargo test" src/, echo "remember to run npm test") was recorded as real passing evidence. Now splits into shell statements, strips quoted substrings, and skips non-executing first words (echo/printf/grep/rg/...) before checking for a marker. 2. Recorded verification evidence was never invalidated when a mutating tool ran afterward, so cargo test (pass) -> edit -> item done could sail through on stale evidence. post_tool_use now clears a session's last_verification whenever a MUTATING_TOOLS call succeeds. 3. shows_finishing_branch_menu fired purely off the request action, never checking whether item done/check_merge actually took effect (done:true / promoted:true in the response). A no-op done or a check_merge whose PR isn't merged yet no longer shows the menu. 4. Dropped "status" from the exit-code field-name fallback list -- generic enough to collide with an unrelated field on some tool shapes (e.g. an HTTP status code). 5. Scoped the PostToolUse hook's matcher to the Bash-family/item/ mutating-tool union instead of firing unmatched on every tool call. Split the PostToolUse-hook logic out of hook.rs into a new hook_completion_gate module (hook.rs now re-exports post_tool_use) to stay under the repo's 1500-line LOC gate. Also reverted an unrelated, non-compiling uncommitted diff to src/cli/work.rs that was sitting in this worktree from an unrelated, apparently crashed session (dead duplicate-PR-check and workflow-store smoke-test code, never wired up anywhere) -- unblocks the build, not part of this item's scope. Agentflare-Agent: claude-code Agentflare-Branch: task/169-implement-combined-completion-gate-verif Agentflare-Item: 169
getappz
pushed a commit
that referenced
this pull request
Aug 21, 2026
PR #581's review (comment on item #169) found 2 critical bugs and 3 moderate issues in the combined verification-before-completion / finishing-a-development-branch gate. This fixes all five: 1. is_verification_command did a blunt substring match, so a command that merely mentioned a marker (grep -rn "cargo test" src/, echo "remember to run npm test") was recorded as real passing evidence. Now splits into shell statements, strips quoted substrings, and skips non-executing first words (echo/printf/grep/rg/...) before checking for a marker. 2. Recorded verification evidence was never invalidated when a mutating tool ran afterward, so cargo test (pass) -> edit -> item done could sail through on stale evidence. post_tool_use now clears a session's last_verification whenever a MUTATING_TOOLS call succeeds. 3. shows_finishing_branch_menu fired purely off the request action, never checking whether item done/check_merge actually took effect (done:true / promoted:true in the response). A no-op done or a check_merge whose PR isn't merged yet no longer shows the menu. 4. Dropped "status" from the exit-code field-name fallback list -- generic enough to collide with an unrelated field on some tool shapes (e.g. an HTTP status code). 5. Scoped the PostToolUse hook's matcher to the Bash-family/item/ mutating-tool union instead of firing unmatched on every tool call. Split the PostToolUse-hook logic out of hook.rs into a new hook_completion_gate module (hook.rs now re-exports post_tool_use) to stay under the repo's 1500-line LOC gate. Also reverted an unrelated, non-compiling uncommitted diff to src/cli/work.rs that was sitting in this worktree from an unrelated, apparently crashed session (dead duplicate-PR-check and workflow-store smoke-test code, never wired up anywhere) -- unblocks the build, not part of this item's scope. Agentflare-Agent: claude-code Agentflare-Branch: task/169-implement-combined-completion-gate-verif Agentflare-Item: 169
getappz
added a commit
that referenced
this pull request
Aug 22, 2026
…ion + finishing-a-development-branch) (#581) * Auto-committed by item done: uncommitted changes at completion Agentflare-Agent: claude-code_2-1-238_agent Agentflare-Branch: task/169-implement-combined-completion-gate-verif Agentflare-Item: 169-implement-combined-completion-gate-verif * fix(hook): address code review findings on the item #169 completion gate PR #581's review (comment on item #169) found 2 critical bugs and 3 moderate issues in the combined verification-before-completion / finishing-a-development-branch gate. This fixes all five: 1. is_verification_command did a blunt substring match, so a command that merely mentioned a marker (grep -rn "cargo test" src/, echo "remember to run npm test") was recorded as real passing evidence. Now splits into shell statements, strips quoted substrings, and skips non-executing first words (echo/printf/grep/rg/...) before checking for a marker. 2. Recorded verification evidence was never invalidated when a mutating tool ran afterward, so cargo test (pass) -> edit -> item done could sail through on stale evidence. post_tool_use now clears a session's last_verification whenever a MUTATING_TOOLS call succeeds. 3. shows_finishing_branch_menu fired purely off the request action, never checking whether item done/check_merge actually took effect (done:true / promoted:true in the response). A no-op done or a check_merge whose PR isn't merged yet no longer shows the menu. 4. Dropped "status" from the exit-code field-name fallback list -- generic enough to collide with an unrelated field on some tool shapes (e.g. an HTTP status code). 5. Scoped the PostToolUse hook's matcher to the Bash-family/item/ mutating-tool union instead of firing unmatched on every tool call. Split the PostToolUse-hook logic out of hook.rs into a new hook_completion_gate module (hook.rs now re-exports post_tool_use) to stay under the repo's 1500-line LOC gate. Also reverted an unrelated, non-compiling uncommitted diff to src/cli/work.rs that was sitting in this worktree from an unrelated, apparently crashed session (dead duplicate-PR-check and workflow-store smoke-test code, never wired up anywhere) -- unblocks the build, not part of this item's scope. Agentflare-Agent: claude-code Agentflare-Branch: task/169-implement-combined-completion-gate-verif Agentflare-Item: 169 * style: cargo fmt src/optimize/runtime.rs Agentflare-Agent: claude-code Agentflare-Branch: task/169-implement-combined-completion-gate-verif Agentflare-Item: 169 --------- Co-authored-by: shiva <shiva@gosysinfo.tech>
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
1 participant
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.
Stacked on drop-backend-prefix
This branch depends on the consolidated
item/asset/label/projecttools introduced in #drop-backend-prefix, which hasn't merged to master yet. Merge that first, then this.What changed
Problem:
/handoff inboxlisted viaartifact_list, which has no project scoping (only session_id/recipient/thread_id filters) — it returned artifacts across every project the agent had ever touched. Separately,fn handoff's send path only published to the flat-file artifact store and never touched an item, so a completed handoff (e.g. an "impl complete, ready for review" report) could sit with no tracked item behind it at all.Fix, two commits:
inboxnow callsitem(action=list)(already scoped to the repo's linked project) instead of rawartifact_list.handoff's send path now assigns an existing item (item_id) or creates one in the linked project (assignee_agent = recipient), and attaches the content to it as an asset (entity_type=item_attachment) instead of publishing a flat-file artifact. Re-attaching under the same item/filename becomes the next asset version rather than a duplicate —assetsgained aversioncolumn (migration0003), computed from existing rows sharing(entity_type, entity_id, filename). No conflict-check needed since items already serialize access via claim/heartbeat.mcp_prompts.rs's/handoffgrammar text is rewritten to match.thread <id>now walks item metadata + assets instead ofartifact_list(thread_id=...).What's intentionally untouched
The
agentflare-artifactscrate, its HTTP server,src/artifacts.rs,src/cli/handoff.rs, and theartifact_publish/list/get/deletetools are all left as-is and fully functional — retired from the handoff path, not removed. The/artifactprompt's help text gets a one-line deprecation note pointing to/handofffor agent-to-agent work; it's still fine for standalone shareable pages.Verification
cargo fmt --checkcleancargo clippy --workspace --all-targets -- -D warnings -A unsafe_code -A clippy::pedanticcleanSummary by CodeRabbit
New Features
Documentation