Repository navigation
fix(handoff): address PR #365 review findings - #366
Merged
Merged
Conversation
…oad off the DB lock
- cargo fmt (item.rs claim closure, types.rs schemars doc attr)
- item::claim: canonicalize both sides of the assignee/owner comparison so
an alias owner (claude:1) isn't wrongly BlockedByAssignee against its own
canonical handoff assignee (claude-code)
- verify_continuation_commit: resolve the target branch under the backend
DB lock, then run all git subprocess checks after releasing it, matching
the existing item_claim split; validate oid is a plain hex id and force
commit-type resolution (oid^{commit}) so a non-hex/flag-like value or a
blob/tree/tag can't pass as a continuation commit
- handoff_impl: reject empty completed/remaining instead of silently
accepting an empty structured payload
- mcp_prompts: document completed/remaining as required handoff fields
so generated requests don't fail deserialization
|
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:
Comment |
getappz
added a commit
that referenced
this pull request
Jul 29, 2026
…icate-item reuse, assignee freeze (#365) * feat(handoff): verified continuation commit, structured payload, duplicate-item reuse, assignee freeze Item #236 (handoff hardening, QuorumGit adoption): - HandoffRequest gains last_commit: verified via `git cat-file -e` and, when the item's task/<seq> branch already exists, `git merge-base --is-ancestor` -- a fabricated or unreachable OID is rejected, not trusted. - HandoffRequest gains required completed/remaining and optional blockers fields, stored in the asset's metadata. - A handoff without item_id now reuses an existing open item assigned to the recipient (matched by name or thread_id) instead of blindly creating a duplicate; genuinely new work still auto-creates. - agentflare_backend::item::claim gains a BlockedByAssignee outcome: a freshly handed-off, never-claimed item can only be claimed by its assignee until accepted, excluding completed/cancelled items. Deferred: shim-side branch-push denial for open handoffs (spec's own review already downgraded this to defense-in-depth, covered in large part by #234's scope enforcement and the opencode branch-guard plugin). * fix(handoff): fmt, canonicalize claim owner, validate commit oid/payload off the DB lock (#366) - cargo fmt (item.rs claim closure, types.rs schemars doc attr) - item::claim: canonicalize both sides of the assignee/owner comparison so an alias owner (claude:1) isn't wrongly BlockedByAssignee against its own canonical handoff assignee (claude-code) - verify_continuation_commit: resolve the target branch under the backend DB lock, then run all git subprocess checks after releasing it, matching the existing item_claim split; validate oid is a plain hex id and force commit-type resolution (oid^{commit}) so a non-hex/flag-like value or a blob/tree/tag can't pass as a continuation commit - handoff_impl: reject empty completed/remaining instead of silently accepting an empty structured payload - mcp_prompts: document completed/remaining as required handoff fields so generated requests don't fail deserialization * fix(test): stop with_temp_home_clears_the_override_env_var_after_returning from racing concurrent with_temp_home callers The assertion read AGENTFLARE_HOME_OVERRIDE outside GLOBAL_STATE_LOCK, the lock that guards every set/remove of it. A concurrent thread's own correctly-scoped with_temp_home call could be transiently holding the var set at the exact moment this test read it, flaking the check on a var this test was never entitled to observe in the first place. Fix: acquire the same lock before reading.
getappz
added a commit
that referenced
this pull request
Oct 16, 2026
…366 PR-A) (#912) Add `compile_workflow_in(json, send, cwd)`; `compile_workflow` delegates with `None`. A compiled cwd is stored on both executors: `CommandExecutor` sets `current_dir` on the child, and `PromptExecutor` carries it as `StepInvocation::cwd`. `agent_send_hook` now honors that cwd, resolving personas from `<cwd>/.claude/agents` and launching via `run_headless_in`; `None` keeps the ambient-cwd behavior, and `app_send_hook` is unchanged. No process-wide chdir anywhere. Tests T33–T35 from the workflow-schedules spec (rev 4 §8, §12 PR-A). No scheduler code in this change. Agentflare-Agent: claude-code Agentflare-Branch: task/368-pr-a-flare-workflow-cwd-plumbing-compile Agentflare-Item: 368 Agentflare-Session: 24cb6990-ba3e-4e85-86e1-5488b3f19121
getappz
added a commit
that referenced
this pull request
Oct 16, 2026
… schedule store/tick, daemon ticker (#366 Phase 1, PR 2 of 3) (#913) * feat(schedule): durable workflow scheduling, atomic start-once, daemon ticker (item #369 PR-B) Schedules/schedule_runs/workflow_start_receipts migration; SQLite-backed start_workflow_once_with_params + create_start_once under BEGIN IMMEDIATE; generic terminal-outcome callback; cancel_if_orphaned with disposition; src/schedule store/tick/tests; spawn_schedule_ticker booted beside the binary staleness watchdog. Spec artifact MICPSjcR3V83J2v_AXHkm rev 4. Agentflare-Agent: opencode Agentflare-Branch: task/369-pr-b-durable-workflow-scheduling-migrati Agentflare-Item: 369 * fix(schedule): portable print-cwd in T36, correct cancel_if_orphaned doc (item #369 PR-B) T36 spawned `pwd` directly; on Windows it resolves to Git coreutils pwd.exe, whose POSIX output breaks canonicalize comparison. Use cmd /c cd on Windows, pwd elsewhere. Doc: terminal callback is NOT invoked by cancel_if_orphaned; caller maps disposition. Agentflare-Agent: opencode Agentflare-Branch: task/369-pr-b-durable-workflow-scheduling-migrati Agentflare-Item: 369
getappz
added a commit
that referenced
this pull request
Oct 17, 2026
getappz
added a commit
that referenced
this pull request
Oct 24, 2026
* docs: add PR lifecycle workflow design spec for item #395 Carve docs/superpowers out of /docs gitignore so shared specs can be versioned like existing superpowers plans. Documents replacing the supervisor review sweep with flare-workflow pr-reconcile/pr-lifecycle. Co-authored-by: Cursor <cursoragent@cursor.com> Agentflare-Agent: cursor Agentflare-Branch: task/395-design-spec-pr-lifecycle-workflow-replac Agentflare-Item: 395 * docs: rev 2 PR lifecycle spec — schedule engine gap and reviewer pass Clarify #366 start_key_for semantics, JSON-vs-Rust schedule fire path, expanded pr-agent audit, and explicit design-spec acceptance criteria. Co-authored-by: Cursor <cursoragent@cursor.com> Agentflare-Agent: cursor Agentflare-Branch: task/395-design-spec-pr-lifecycle-workflow-replac Agentflare-Item: 395 * docs: rev 3 PR lifecycle spec — item-work pipeline and design-spec evidence Clarify how SDD design-spec items differ from review-only tasks, what counts as completion evidence versus cargo test on an unchanged tree, and how #395 relates to the proposed pr-reconcile lifecycle workflows. Co-authored-by: Cursor <cursoragent@cursor.com> Agentflare-Agent: cursor Agentflare-Branch: task/395-design-spec-pr-lifecycle-workflow-replac Agentflare-Item: 395 * wip(sdd-loop): task 0 checkpoint Agentflare-Branch: task/395-design-spec-pr-lifecycle-workflow-replac Agentflare-Item: 395 * Verifying the analyst's report against the repo: reading the spec, git state, and key codebase references. REVIEW_ISSUES: - **Stale verification claim:** Rev 5 is **committed** at `HEAD` (`71cf49f8`, +43 lines to the spec). Working tree is clean; there is no uncommitted rev 5 delta. The “commit before `item done`” note applies only if that was written before the checkpoint commit. - **Branch scope not surfaced:** `git diff master` is **not** limited to design docs. Besides `.gitignore` and the spec, **19 Rust paths** differ (~93 insertions, **~1,889 deletions**), including large removals under `src/review/diff.rs`, `src/memory/mcp.rs`, `src/mcp_server/item_merged_pr.rs`, and supervisor/worktree tests. That contradicts item #395 (“design-spec only, no application code”) and is a merge risk; the analysis should call this out explicitly for operators. - **Embedded spec verification vs current HEAD:** The rev 5 verification block still records `HEAD=83f1d90a` and **497** lines; the file is now **500** lines at `71cf49f8`. Harmless as history, but the analyst run should report **current** `wc -l` and `HEAD` when citing verification. - **Line count:** Spec is **500** lines, not “~501” (minor). - **Branch sync:** Branch is **behind `origin/master` by 4** commits (and ahead by 5). Not mentioned; relevant before PR/merge. - **Technical summary:** Substance on `pr-reconcile` / `pr-lifecycle`, `spawn_schedule_ticker` + JSON-only schedule fire (`resolve_named_definition` in `schedule/tick.rs`), `start_workflow_once`, cutover flag, pr-agent stance, item-work vs PR lifecycle, and parity rows for all `PrCiStatus` variants matches the spec and current code anchors. No correction needed there beyond the branch hygiene gap above.The `mbx build -p agentflare` check never finished: it was killed after about two minutes (exit **137**, lean-ctx output cap), so we don’t have a compile pass/fail for this branch. That doesn’t change the review outcome. The branch still has large Rust diffs vs `master` (~1.9k lines removed across 19 files) on top of the design spec, which is the main hygiene issue called out in **REVIEW_ISSUES**. Agentflare-Branch: task/395-design-spec-pr-lifecycle-workflow-replac Agentflare-Item: 395
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.
Summary
Fixes for the review findings on #365 (item #236 handoff hardening):
cargo fmt --checkfailures (item.rsclaim closure,types.rsschemars attribute) that were failing thefmt/CI Greenchecks on feat(handoff): verified continuation commit, structured payload, duplicate-item reuse, assignee freeze #365.crates/agentflare-backend/src/item.rs):agent_part()compared the raw owner id against the already-canonicalizedassignee_agent. An alias owner likeclaude:1didn't match the stored canonicalclaude-code, so the handoff's own assignee could get wronglyBlockedByAssigneeon their own item. Now canonicalizes both sides.src/mcp_server/handoff.rs):verify_continuation_commitran up to three git subprocess calls insidewith_backend_db, which holds a shared mutex — the same pattern the codebase already avoids initem_claim(DB resolve under lock, then blocking work after). Restructured to resolve the target branch under the lock and run all git checks after releasing it.last_commitvalidation gap (security/correctness): a leading-inoidwould be parsed by git as an option rather than a rev, and plaincat-file -eaccepts blobs/trees/tags, not just commits. Now rejects non-hex input up front and forces commit-type resolution viaoid^{commit}.completed/remainingare documented as required but weren't checked for emptiness, so""satisfied the contract silently. Added the sametrim().is_empty()guard used for the other required fields./handoffprompt didn't mentioncompleted/remainingas required fields, so agent-generated requests built from the prompt could fail to deserialize.Also added two regression tests (owner-alias claim, and updated existing handoff tests to supply
completed/remainingnow that they're enforced as non-empty).Test plan
cargo fmt --check— cleancargo clippy --workspace --all-features -- -D warnings— cleancargo test --workspace --all-features— all green (886 passed in the main crate, 60 in agentflare-backend, no failures workspace-wide)