feat: GitHub bridge dogfooding — memory sync fix, handoff→queue publish, capacity signal, claim dispatch - #412
Merged
Merged
Conversation
added 5 commits
August 8, 2026 18:35
… an orphan MemorySyncConfig::from_env() derived the sync repo from AGENTFLARE_MEMORY_SYNC_REPO only, requiring manual setup on every workstation; it now falls back to the current repo's origin remote (same resolution the GitHub bridge already uses), so each project gets its own branch without configuration. The env var still overrides when set. ensure_branch() branched a new sync branch off the default branch's HEAD, so it silently carried a full (and ever-staler) copy of the repo tree alongside memory-sync.jsonl. It now creates an orphan commit (empty tree, no parent) instead, so the branch holds only the synced memory data.
handoff assigns work locally today -- recipient="github" instead publishes it as a labelled issue on the bridge's pull queue (github::bridge), so any workstation running the bridge can claim and work it, not just this one. Reuses the existing issue-creation and claim/heartbeat/export machinery in github::bridge::tick unchanged. mcp__flare__handoff is now in the gateway's auto-allowed tool list (agentflare init syncs this into ~/.claude/settings.json), since it only ever writes to this workstation's own item tracker or, now, publishes a new issue -- not worth a permission prompt per call. Repo/queue-label resolution for the bridge-publish path (and a new `agentflare github-bridge set/unset/status` CLI) can be overridden per-project via .agentflare/config.toml's [bridge] table, reusing flare_git_core's existing project+home-layered TOML config loader instead of introducing a new config file. This is scoped to CLI/MCP call sites only -- the standalone daemon has no reliable cwd, so its own claiming loop still resolves purely from AGENTFLARE_BRIDGE_ENABLED/_REPO env vars.
There was no way to know whether routing work onto the bridge queue (handoff recipient="github") made sense at a given moment -- claim headroom is private per-daemon state, not observable across workstations. Add a read-only queue_status() that lists open queue-labelled issues via the GitHub API and classifies each as claimed (via its live claim marker, same TTL/parsing github::bridge already uses) or unclaimed, reporting total open, unclaimed count, oldest-unclaimed age, and per-owner claim counts. Exposed as a new flare_git MCP action (bridge_queue_status) rather than a new tool, since flare_git already owns GitHub repo/client resolution. An empty or fast-clearing queue is a proxy for capacity existing somewhere; unclaimed issues piling up means nothing is currently pulling from it -- a signal for deciding whether to queue new work or keep it local. Also adds Issue.created_at (previously only updated_at was deserialized), needed to measure unclaimed age.
…gent configured Claiming an issue only ever created a local item marked assignee = the bridge's own instance id (e.g. flared:51bb8de6c33b) -- not a real agent name, so supervisor::resolve_confirmed_agent always rejected it and nothing ever ran the work. Confirmed live: issue #409 sat claimed and "In Progress" for good. BridgeConfig gains work_agent (AGENTFLARE_BRIDGE_WORK_AGENT), None by default so nothing changes until explicitly opted in. When set, record_claim assigns the item to that agent instead of the instance id and labels it ready-for-work (same label handoff's own new-item path already uses), so the supervisor's already-running discovery loop (spawn_supervisor_discovery, ticking independently in the same daemon process) picks it up and launches it -- no new dispatch path, just closing the gap that kept claimed items invisible to the one that already exists. The other half -- updating the issue once the task is done -- was already built and tested (export_if_dirty/close_if_still_open post a Completed comment and close the issue once completed_at is set); nothing needed there.
|
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 repository-local GitHub bridge configuration, GitHub handoff and queue-status MCP actions, structured issue payloads, work-agent assignment, orphan branch creation, origin-based memory repository resolution, and managed branch-protection hooks. ChangesGitHub bridge workflow
Repository integration
Managed Git hooks
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant handoff_impl
participant GitHubAPI
participant BridgeDaemon
MCPClient->>handoff_impl: handoff recipient="github"
handoff_impl->>GitHubAPI: search or create labelled issue with payload
GitHubAPI-->>handoff_impl: issue metadata
handoff_impl-->>MCPClient: handoff result
BridgeDaemon->>GitHubAPI: read queued issue and comments
GitHubAPI-->>BridgeDaemon: issue and handoff payload
BridgeDaemon->>BridgeDaemon: create item and assign work agent
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/github/bridge/tick.rs (1)
344-395: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftApply dispatch settings when re-adopting an existing item.
The existing-item branch only restores the state. If an item was created without
work_agent, then ceded, and later reclaimed withwork_agentconfigured, it remains assigned to the bridge instance and has noready-for-worklabel.Update the existing item assignment and add the ready label when
ctx.config.work_agentis set. Add a regression test that reclaims a previously ceded non-dispatched item.🤖 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/github/bridge/tick.rs` around lines 344 - 395, Update the existing-item branch of the item creation flow to apply dispatch settings whenever ctx.config.work_agent is configured: assign the item to the configured work agent and add the existing project’s READY_LABEL, while preserving state restoration and current behavior without a work agent. Add a regression test covering reclamation of a previously ceded, non-dispatched item.
🤖 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/components.rs`:
- Around line 509-519: Update GATEWAY_PERMISSIONS_ALLOW to remove
mcp__flare__handoff from the unconditional allowlist, ensuring handoffs
targeting recipient="github" require explicit approval while preserving the
existing allowlist entries for docs, search, and tool.
In `@src/github/bridge/config.rs`:
- Around line 69-81: Update resolve_project_repo in src/github/bridge/config.rs
(lines 69-81) to return an error when AGENTFLARE_BRIDGE_REPO or [bridge].repo is
present but RepoId::parse fails; only absent overrides should fall back to
resolving from the remote origin. In src/cli/github_bridge.rs (lines 52-62),
validate --repo with RepoId::parse before persisting it and reject invalid
values.
- Around line 86-92: Update resolve_project_queue_label to trim configured
values before filtering and returning them, so whitespace-only environment or
project settings fall back to DEFAULT_QUEUE_LABEL and padded values are
normalized. Add tests covering whitespace-only and padded values from both the
environment and project configuration.
- Around line 105-128: Update the configuration write flow around the
read-modify-write sequence to acquire an exclusive lock before reading and
retain it through the final replacement, preventing concurrent github-bridge set
processes from overwriting changes. Serialize the merged document to a temporary
file in the same directory, then atomically replace config.toml with the
temporary file instead of calling std::fs::write directly; preserve the existing
directory creation and error propagation behavior.
In `@src/github/bridge/queue_status.rs`:
- Around line 17-19: Update QueueStatus’s oldest_unclaimed_age_secs contract and
calculation to distinguish zero unclaimed issues from unclaimed issues whose
Issue.created_at is absent or invalid. Prefer an explicit unknown state in the
result and propagate it through the logic around the queue-status calculation;
update the documentation and add coverage for an unclaimed issue without
created_at.
- Around line 45-49: Replace the per-issue issues::list_comments call inside the
open_issues loop with a batched comment query or a cached claim-resolution
mechanism with an explicit freshness limit. Reuse the resulting comment data
while processing each issue, preserving the existing (comment ID, body) mapping
and claim-resolution behavior without issuing one request per issue.
In `@src/mcp_server/flare_git.rs`:
- Around line 247-255: Update the repository selection before the queue_status
call in the handoff/bridge status flow: when req.repo is provided, preserve it;
otherwise resolve the project root and call resolve_project_repo, using its
result instead of the repository currently derived from origin. Keep
resolve_project_queue_label and the existing queue_status arguments unchanged.
In `@src/mcp_server/handoff.rs`:
- Around line 308-313: Update the documentation comment for
handoff_to_bridge_queue to describe repository resolution precedence accurately:
use AGENTFLARE_BRIDGE_REPO first, then project-local [bridge].repo, and fall
back to the workstation’s origin remote.
- Around line 496-509: Make
recipient_github_without_an_origin_remote_fails_clearly hermetic by acquiring
the shared test environment lock, temporarily clearing AGENTFLARE_BRIDGE_REPO
before calling mcp.handoff_impl, and restoring its original value afterward,
including on failure. Preserve the existing origin-error assertion and avoid any
network or external issue creation.
- Around line 333-341: Update the handoff publication flow around issues::create
to be idempotent across retries: preserve thread_id when constructing the
handoff, derive a stable handoff marker, and look up an existing issue by that
marker before performing the external create. Return or reuse the existing issue
when found, and only call issues::create for genuinely new handoffs.
- Around line 314-338: The handoff_to_bridge_queue flow currently sends only
description or content, losing the structured handoff payload required by
handoff_impl. Update handoff_to_bridge_queue and its callers to serialize and
include content, completed, remaining, and all required handoff metadata in the
GitHub issue body, or explicitly establish and test a reduced recipient="github"
contract that matches what the bridge importer can recover.
- Around line 322-338: The handoff path resolves project-specific repo and queue
settings via resolve_project_repo and resolve_project_queue_label, but the
daemon may use different bridge configuration. Align handoff_to_bridge_queue
with the daemon’s effective AGENTFLARE_BRIDGE_REPO and
AGENTFLARE_BRIDGE_QUEUE_LABEL settings, or explicitly reject and report
unsupported [bridge].repo and queue_label overrides before creating the issue.
---
Outside diff comments:
In `@src/github/bridge/tick.rs`:
- Around line 344-395: Update the existing-item branch of the item creation flow
to apply dispatch settings whenever ctx.config.work_agent is configured: assign
the item to the configured work agent and add the existing project’s
READY_LABEL, while preserving state restoration and current behavior without a
work agent. Add a regression test covering reclamation of a previously ceded,
non-dispatched item.
🪄 Autofix
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
Run ID: f8aec42f-5be0-4cb2-9ab0-a41a5e9f6f92
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
Cargo.tomlsrc/cli/github_bridge.rssrc/cli/memory.rssrc/cli/mod.rssrc/components.rssrc/github/bridge/config.rssrc/github/bridge/mod.rssrc/github/bridge/queue_status.rssrc/github/bridge/runner.rssrc/github/bridge/tests/live_github.rssrc/github/bridge/tests/two_instance.rssrc/github/bridge/tick.rssrc/github/contents.rssrc/github/models.rssrc/mcp_server/flare_git.rssrc/mcp_server/handoff.rssrc/mcp_server/types.rssrc/memory/sync.rs
- cargo fmt (queue_status.rs, config.rs, contents.rs) to unbreak the fmt CI check - security: drop mcp__flare__handoff from the always-allowed gateway tool list so recipient="github" publishes require an explicit permission prompt - resolve_project_repo now errors on an invalid explicit AGENTFLARE_BRIDGE_REPO or [bridge].repo override instead of silently falling through to origin; `github-bridge set --repo` validates before persisting - resolve_project_queue_label trims and treats whitespace-only overrides as absent - write/clear_project_bridge_settings take an exclusive lock and write via a temp-file + rename so concurrent `github-bridge set` calls can't corrupt .agentflare/config.toml - bridge_queue_status (flare_git.rs) now resolves the repo the same way handoff's recipient="github" path does, instead of plain origin - queue_status batches comment fetches into one repo-wide listing instead of one API call per open issue, and reports unclaimed issues with no parseable created_at as unclaimed_with_unknown_age instead of silently ignoring them - handoff_to_bridge_queue embeds the full structured payload (content, completed, remaining, thread_id) and a dedup key in the issue body (bridge::handoff_payload), checks for a matching open issue before publishing so a retry can't double-publish, recovers the payload into the bridge-claimed item's description/metadata (tick.rs) instead of dropping it, and warns when this workstation's own bridge daemon won't watch the resolved repo - made a handoff test hermetic against an inherited AGENTFLARE_BRIDGE_REPO
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/github/bridge/config.rs (1)
194-197: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate non-
NotFoundconfiguration read errors.Both paths treat every read failure as if
.agentflare/config.tomldoes not exist. This can overwrite existing configuration duringsetor falsely report success duringunset.
src/github/bridge/config.rs#L194-L197: Create an empty TOML document only when the error kind isNotFound. Return every other read error.src/github/bridge/config.rs#L226-L228: Return success only when the error kind isNotFound. Return every other read error.🤖 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/github/bridge/config.rs` around lines 194 - 197, Update the configuration read error handling in src/github/bridge/config.rs:194-197 so the set path creates an empty TOML document only for ErrorKind::NotFound and propagates all other errors; update src/github/bridge/config.rs:226-228 so the unset path returns success only for NotFound and propagates other errors. Apply these changes around the existing config read logic without altering successful reads.
🤖 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/github/bridge/handoff_payload.rs`:
- Around line 34-48: Update HandoffPayload::embed and HandoffPayload::extract so
serialized payload data cannot contain marker delimiters and extraction targets
the generated marker rather than an earlier visible occurrence. Preserve
tolerant None behavior for missing or malformed markers, and add tests covering
“ -->”, MARKER_PREFIX text, and a valid marker appearing in the visible body.
In `@src/github/bridge/queue_status.rs`:
- Line 57: Update the comment retrieval around issues::list_all_comments to
restrict processing to queued issue numbers: either fetch comments individually
for each queued issue or cache the full paginated history and filter it before
claim resolution. Preserve all history needed by claim_rules::resolve_holder,
and add a regression test covering a claim marker found on page 2.
---
Outside diff comments:
In `@src/github/bridge/config.rs`:
- Around line 194-197: Update the configuration read error handling in
src/github/bridge/config.rs:194-197 so the set path creates an empty TOML
document only for ErrorKind::NotFound and propagates all other errors; update
src/github/bridge/config.rs:226-228 so the unset path returns success only for
NotFound and propagates other errors. Apply these changes around the existing
config read logic without altering successful reads.
🪄 Autofix
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
Run ID: d2c59851-d40e-4dd1-82fa-4d12bfd66632
📒 Files selected for processing (12)
src/cli/github_bridge.rssrc/components.rssrc/github/bridge/config.rssrc/github/bridge/handoff_payload.rssrc/github/bridge/mod.rssrc/github/bridge/queue_status.rssrc/github/bridge/tick.rssrc/github/contents.rssrc/github/issues.rssrc/github/models.rssrc/mcp_server/flare_git.rssrc/mcp_server/handoff.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/mcp_server/flare_git.rs
- src/cli/github_bridge.rs
- src/github/contents.rs
- src/mcp_server/handoff.rs
- handoff_payload: base64-encode the embedded JSON so a caller-supplied content/completed/remaining field containing " -->" or the literal marker prefix can no longer corrupt marker parsing; extract() anchors to the end of the body so it recovers the marker embed() actually appended rather than an earlier marker-shaped string in the visible text - queue_status: revert the repo-wide /issues/comments batching -- that endpoint has no per-issue or per-label filter, so for a repo with far more total issue/PR traffic than open queue depth it fetches strictly more data than the per-issue calls it replaced. Back to one list_comments call per open (queue-labelled) issue, bounded by queue depth by design. Kept the unclaimed_with_unknown_age tracking, which is unaffected. - removed list_all_comments/Comment::issue_url/issue_number, which only existed to support the reverted batching and would otherwise be dead code in the production binary
…ating them as absent
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/handoff.rs`:
- Around line 365-401: The existing-issue lookup in the handoff flow must not
reuse an issue that has already been claimed outside a bounded retry window,
since its local item will not receive a later payload. Update the logic around
HandoffPayload::extract, record_claim, and the issue selection to permit reuse
only for an unclaimed issue or a clearly bounded recent retry; otherwise create
a new issue. Add a boolean reused field to the result JSON indicating whether an
existing issue was reused.
🪄 Autofix
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
Run ID: 1a6e9562-023b-4b07-bfe4-6b5074342181
📒 Files selected for processing (10)
src/cli/github_bridge.rssrc/components.rssrc/github/bridge/config.rssrc/github/bridge/handoff_payload.rssrc/github/bridge/mod.rssrc/github/bridge/queue_status.rssrc/github/bridge/tick.rssrc/github/contents.rssrc/mcp_server/flare_git.rssrc/mcp_server/handoff.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components.rs
- src/github/bridge/mod.rs
- src/mcp_server/flare_git.rs
- src/github/contents.rs
- src/github/bridge/tick.rs
- src/cli/github_bridge.rs
- src/github/bridge/config.rs
.githooks/pre-commit already existed and correctly blocks direct commits to the default branch -- its own header comment even names the exact gap this closes (a git commit issued through a Bash/shell tool, or any tool name hook_redirect.rs's PreToolUse guard doesn't recognize, slips past it entirely). But nothing ever installed it: components.rs's get_components() -- the one list both init and doctor walk -- had no githooks entry, so neither could detect or fix a repo that was missing it. Confirmed live this session: this repo's own core.hooksPath was unset, ~/.agentflare/githooks/ didn't exist, and the tracked .githooks/* scripts weren't even executable -- so every commit landed straight on master. Extracts install_hooks()'s copy-templates-and-set-core.hooksPath logic (previously CLI-only, print-and-return) into hooks_installed_for/ install_hooks_for so a Component's check/apply closures can call it directly instead of duplicating it -- same source of truth for the interactive `agentflare git install-hooks` command and the new automatic component. src/components.rs was already 1731 lines (over the 1500-line LOC gate) before this change -- now enforced for the first time on this repo since core.hooksPath was never active before. Allowlisted alongside mcp_server.rs/item.rs rather than force through with --no-verify or rush a risky full-file split under time pressure. .githooks/pre-commit, pre-push, prepare-commit-msg, and reference-transaction gain the executable bit -- they were tracked as regular files, so even a correctly-configured core.hooksPath couldn't have run them. Agentflare-Agent: claude-code_2-1-226_agent Agentflare-Branch: feat/init-installs-githooks
hooks_installed_for only compared file content against the embedded template -- a hook with correct content but a lost executable bit (fresh clone on a filesystem/tool that doesn't preserve it, an accidental chmod -x, ...) read as "installed" even though git silently ignores a non-executable hook (an advisory hint, not an error) rather than running it. install_hooks_for had the matching gap: it only chmodded inside the content-mismatch branch, so re-running it against a content-correct-but-non-executable hook was also a no-op. Confirmed live: a direct commit briefly succeeded on master right after the githooks component's own commit landed, because the merge hadn't yet brought the executable-bit fix into the working tree -- the exact failure mode this closes. Agentflare-Agent: claude-code_2-1-226_agent Agentflare-Branch: fix/githooks-perm-check
Two more branch-protection gaps closed, both found live this session: 1. pre-commit alone does not fire for a merge commit -- git only invokes it for a plain `git commit`; pre-merge-commit is its separate hook for that (githooks(5)). Confirmed live: three local `git merge --no-ff ... master` calls this session all went through completely unguarded, even after pre-commit became active, because this hook didn't exist. It's a one-line wrapper that execs pre-commit -- one source of truth for what "direct commit to the default branch" means, plain or merge. 2. reference-transaction was audit-only by design (documented as "this hook cannot block anything"). Per-verb hooks (pre-commit, pre-merge-commit, pre-push) only cover the verbs someone thought to add a hook for -- reset --hard, rebase, cherry-pick, tag -f, branch -f all still slip straight through. reference-transaction fires for EVERY ref move regardless of which git command caused it, so it's the actual general backstop instead of chasing verbs one at a time. Scoped narrowly to keep the blast radius down (a bug here could affect every git operation in the repo, not just commits to master): only denies an update to refs/heads/<default-branch> itself when the new commit isn't already reachable from refs/remotes/origin/<default-branch>. A fast-forward sync from origin is explicitly allowed -- the check is "is this oid already on the remote", not "is this a fast-forward" (a fresh local commit is ALSO a fast-forward from its own parent, so that alone can't distinguish syncing from origin from introducing new local work). Fails open if origin's tracking ref can't be resolved. Verified in an isolated origin+clone fixture before touching this repo's live hooks: direct commit on master denied, feature branch commits unaffected, fast-forward sync from origin allowed, git reset --hard introducing unpushed work denied (the verb-independence claim), no-remote repo fails open, and the documented emergency override (git -c core.hooksPath=) works. Agentflare-Agent: claude-code_2-1-226_agent Agentflare-Branch: feat/pre-merge-commit-hook
The retry-dedup lookup in handoff_to_bridge_queue matched on the embedded payload key alone, so calling it again with the same thread_id/name after the matching issue had already been claimed (a local item created from it) returned that stale issue instead of publishing a fresh one -- silently dropping this call's completed/remaining update, since nothing re-reads the issue body after claim time. Now checks claim liveness (same claim_rules::resolve_holder used by queue_status/tick) before reusing: a still-unclaimed matching issue is reused as before (the actual retry-after-timeout case), a claimed one is left alone and a new issue is published instead. The result JSON gains a `reused` boolean so callers can tell which happened.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/cli/git.rs`:
- Around line 434-454: Update the hook installation flow around the HOOKS loop
and core.hooksPath configuration to avoid overwriting existing same-named hooks
or silently replacing a non-.githooks path. Detect existing custom hooks and
alternate hooks paths, then either migrate or wrap them so their behavior
remains available, or abort with an explicit migration requirement; add a
regression test proving an existing hook still runs after installation.
🪄 Autofix
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
Run ID: 241e1841-27b3-42d3-9f04-7e35b788790c
📒 Files selected for processing (9)
.githooks/pre-commit.githooks/pre-merge-commit.githooks/pre-push.githooks/prepare-commit-msg.githooks/reference-transactionscripts/loc-gate.shsrc/cli/git.rssrc/components.rssrc/mcp_server/handoff.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp_server/handoff.rs
Comment on lines
+434
to
+454
| for (name, template) in HOOKS { | ||
| let dst = local_dir.join(name); | ||
| if fs::read(&dst).ok().as_deref() != Some(template.as_bytes()) { | ||
| fs::write(&dst, template).map_err(|e| format!("writing {name}: {e}"))?; | ||
| changed = true; | ||
| } | ||
| #[cfg(unix)] | ||
| if !is_executable(&dst) { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| fs::set_permissions(&dst, fs::Permissions::from_mode(0o755)) | ||
| .map_err(|e| format!("chmod +x {name}: {e}"))?; | ||
| changed = true; | ||
| } | ||
| } | ||
|
|
||
| let current_hooks_path = | ||
| flare_git_core::shell::run_in_opt(repo_root, &["config", "--get", "core.hooksPath"]); | ||
| if current_hooks_path.as_deref() != Some(".githooks") { | ||
| flare_git_core::shell::run_in(repo_root, &["config", "core.hooksPath", ".githooks"]) | ||
| .map_err(|e| format!("git config core.hooksPath: {e}"))?; | ||
| changed = true; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not silently replace existing repository hooks.
Line 436 overwrites same-named hooks in .githooks. Line 452 replaces any other core.hooksPath. Git uses one hooks path, so existing hooks from another manager stop running after installation. This can disable repository checks and policy hooks.
If another hooks path or custom hook exists, preserve its behavior through a migration or wrapper. Otherwise, stop with an explicit migration requirement. Add a regression test with an existing hook that must still run after installation.
🤖 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/cli/git.rs` around lines 434 - 454, Update the hook installation flow
around the HOOKS loop and core.hooksPath configuration to avoid overwriting
existing same-named hooks or silently replacing a non-.githooks path. Detect
existing custom hooks and alternate hooks paths, then either migrate or wrap
them so their behavior remains available, or abort with an explicit migration
requirement; add a regression test proving an existing hook still runs after
installation.
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.
Summary
MemorySyncConfignow defaults the sync repo to the current repo'soriginremote instead of requiringAGENTFLARE_MEMORY_SYNC_REPO;ensure_branchcreates new sync branches as clean orphan commits instead of copying the whole repo tree onto them.recipient="github"publishes work as a labelled issue on the bridge's pull queue instead of a local item, so any workstation running the bridge can claim it..agentflare/config.toml's[bridge]table (repo/queue_label overrides) and a newagentflare github-bridge set/unset/statusCLI back this for CLI/MCP call sites.mcp__flare__handoffis now in the gateway's auto-allowed tool list.bridge_queue_statusMCP action (flare_git) reports queue depth, unclaimed count, oldest-unclaimed age, and per-owner claim counts — a capacity signal for deciding whether to route work onto the bridge queue or keep it local.record_claimnow dispatches a real agent whenAGENTFLARE_BRIDGE_WORK_AGENTis configured — assigns the claimed item to that agent and labels itready-for-workso the already-running supervisor discovery loop picks it up, instead of leaving claimed items assigned to the bridge's own instance id (whichresolve_confirmed_agentalways rejected, so nothing ever dispatched).Verification
cargo test --bin agentflare: all affected modules pass (memory::sync, github::bridge::*, github::contents, mcp_server::handoff, mcp_server::flare_git, components) — 100+ tests across this diff, 0 failures.cargo clippy --all-targets: clean on all touched files.getappz/agentflareitself: created a real issue, watched the bridge claim it (~10s), label itready-for-work, the supervisor dispatch a realagentflare workjob in an isolated worktree, the agent complete the task and open a PR, the item get marked done, and the bridge export completion back onto the issue (comment + auto-close) on the next tick — full loop, unattended.Summary by CodeRabbit
New Features
originby default.Bug Fixes