diff --git a/.githooks/pre-commit b/.githooks/pre-commit old mode 100644 new mode 100755 diff --git a/.githooks/pre-merge-commit b/.githooks/pre-merge-commit new file mode 100755 index 00000000..6a7467c3 --- /dev/null +++ b/.githooks/pre-merge-commit @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +# agentflare branch-protection guard (pre-merge-commit). +# +# `pre-commit` alone does NOT fire for a merge that creates a merge commit +# (`pre-merge-commit` is git's separate, dedicated hook for that -- see +# githooks(5)); without this, `git merge --no-ff ` while checked out +# on the default branch bypasses pre-commit's guard entirely, which is +# functionally the same as a direct commit to it. Delegates to pre-commit +# rather than duplicating its logic -- one source of truth for what "direct +# commit to the default branch" means, whether it's a plain commit or a +# merge commit. +exec "$(dirname "$0")/pre-commit" diff --git a/.githooks/pre-push b/.githooks/pre-push old mode 100644 new mode 100755 diff --git a/.githooks/prepare-commit-msg b/.githooks/prepare-commit-msg old mode 100644 new mode 100755 diff --git a/.githooks/reference-transaction b/.githooks/reference-transaction old mode 100644 new mode 100755 index 6016f7f7..aa348e6c --- a/.githooks/reference-transaction +++ b/.githooks/reference-transaction @@ -1,31 +1,116 @@ #!/usr/bin/env bash -# agentflare reference-transaction journal. +# agentflare reference-transaction journal + branch-protection backstop. # -# Backstop audit trail independent of the git-shim's own interception: this -# fires for EVERY ref move in this repo, whether git was invoked through -# the agentflare git shim, bare git, or any other path -- unlike the shim's -# own audit log (crates/flare-git-core/src/audit.rs), which only sees -# invocations that actually went through it. +# Two independent jobs, at two different transaction states: +# - "committed": audit-log every ref move (unchanged behavior). Fail-open: +# this half cannot block anything. +# - "prepared": DENY the transaction if it would advance +# refs/heads/ to a commit not already reachable from +# refs/remotes/origin/ -- i.e. block ANY operation +# (commit, merge, rebase, reset, cherry-pick, anything) that introduces +# work directly onto the default branch's local ref that the remote +# doesn't already have. A plain `git fetch` + fast-forward +# `git pull`/`git merge --ff-only` stays allowed, since the resulting +# oid is exactly what's already on origin -- 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 "new local work"). # -# Git invokes this hook once per state ("prepared", "committed", "aborted") -# with ref-update lines (` `) on stdin; only -# "committed" (the transaction that actually succeeded) is logged. +# This is the general backstop pre-commit/pre-merge-commit/pre-push can't +# be individually: those gate specific git verbs one at a time, so a verb +# nobody's added a hook for yet (reset --hard, rebase, cherry-pick, tag -f, +# branch -f) slips through. This hook fires for EVERY ref move regardless +# of which git command caused it. # -# Fail-open by design: if the agentflare binary isn't on PATH or errors, -# the underlying git operation is completely unaffected either way -- this -# hook cannot block anything, it only observes. +# Fail-open on ambiguity: if the default branch or origin's tracking ref +# can't be resolved, the update is allowed rather than blocked -- this +# check must never be able to brick git entirely. # # Installed into a project via `agentflare git install-hooks`, same as # pre-commit/pre-push/prepare-commit-msg. state="$1" -if [ "$state" != "committed" ]; then +resolve_default_branch() { + if ! git rev-parse --git-dir >/dev/null 2>&1; then + return 1 + fi + + if git symbolic-ref refs/remotes/origin/HEAD >/dev/null 2>&1; then + git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@' + return 0 + fi + + local cfg + cfg="$(git config --get init.defaultBranch 2>/dev/null || true)" + if [ -n "$cfg" ]; then + echo "$cfg" + return 0 + fi + + for cand in main master; do + if git show-ref --verify --quiet "refs/heads/$cand" 2>/dev/null; then + echo "$cand" + return 0 + fi + done + + echo "master" +} + +if [ "$state" = "committed" ]; then + if command -v agentflare >/dev/null 2>&1; then + agentflare git ref-transaction-log || true + fi exit 0 fi -if command -v agentflare >/dev/null 2>&1; then - agentflare git ref-transaction-log || true +if [ "$state" != "prepared" ]; then + exit 0 +fi + +default="$(resolve_default_branch)" || exit 0 +[ -n "$default" ] || exit 0 + +default_ref="refs/heads/$default" +origin_ref="refs/remotes/origin/$default" + +# Fail open if origin's tracking ref can't be resolved at all (no remote, +# never fetched) -- nothing to compare against, so nothing to safely block. +if ! git rev-parse --verify --quiet "$origin_ref" >/dev/null 2>&1; then + exit 0 fi +while IFS=' ' read -r old_oid new_oid refname; do + [ "$refname" = "$default_ref" ] || continue + + # Deletion (new_oid all-zeros) is never "catching up to remote". + if [[ "$new_oid" =~ ^0+$ ]]; then + echo "ERROR: refusing to delete the default branch ref '$default_ref'." >&2 + exit 1 + fi + + if git merge-base --is-ancestor "$new_oid" "$origin_ref" 2>/dev/null; then + continue + fi + + cat >&2 <- -b + # or, in-session: + git checkout -b + +Then commit there and open a PR. To override in an emergency (not recommended): + git -c core.hooksPath= +EOF + exit 1 +done + exit 0 diff --git a/Cargo.lock b/Cargo.lock index 2bd9e85f..257b035a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -118,6 +118,7 @@ dependencies = [ "thiserror", "tokio", "tokio-stream", + "toml", "ureq 2.12.1", "windows-sys 0.59.0", "zeroize", diff --git a/Cargo.toml b/Cargo.toml index 294a48e4..31b49e06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,7 @@ same-file = "1" zip = { version = "2", default-features = false, features = ["deflate"] } rusqlite = { version = "0.40", features = ["bundled"] } rusqlite_migration = "2" +toml = "0.8" rand = "0.8" aes-gcm = "0.10" pbkdf2 = { version = "0.12", features = ["simple"] } diff --git a/scripts/loc-gate.sh b/scripts/loc-gate.sh index 7b1777dc..17a611a0 100644 --- a/scripts/loc-gate.sh +++ b/scripts/loc-gate.sh @@ -8,6 +8,7 @@ FROZEN_LIMIT=2000 ALLOWLIST=( src/mcp_server.rs crates/agentflare-backend/src/item.rs + src/components.rs ) cd "$(dirname "$0")/.." diff --git a/src/cli/git.rs b/src/cli/git.rs index de2ea7e9..df2be1f1 100644 --- a/src/cli/git.rs +++ b/src/cli/git.rs @@ -1,7 +1,8 @@ //! `agentflare git` -- git-related CLI surface: installing the shared -//! branch-protection hooks (pre-commit / pre-push / prepare-commit-msg / -//! reference-transaction / post-commit) into a repo, installing/uninstalling the -//! flare-git-shim PATH shim, and the recovery-snapshot commands +//! branch-protection hooks (pre-commit / pre-merge-commit / pre-push / +//! prepare-commit-msg / reference-transaction / post-commit) into a repo, +//! installing/uninstalling the flare-git-shim PATH shim, and the +//! recovery-snapshot commands //! (`snapshot list/restore/prune`) that make `flare_git_core::snapshot`'s //! automatic pre-destructive snapshots actually usable. //! @@ -202,6 +203,11 @@ fn shared_hooks_dir() -> PathBuf { /// `~/.agentflare/githooks/` on first `install-hooks`, so the shared location /// is self-bootstrapping and survives repo checkouts. const PRE_COMMIT: &str = include_str!("../../.githooks/pre-commit"); +// `pre-commit` alone does not fire for a merge commit -- git only invokes it +// for a plain `git commit`. `pre-merge-commit` is git's separate hook for +// that (githooks(5)); ours just execs `pre-commit` so there's one source of +// truth for what "direct commit to the default branch" means. +const PRE_MERGE_COMMIT: &str = include_str!("../../.githooks/pre-merge-commit"); const PRE_PUSH: &str = include_str!("../../.githooks/pre-push"); const PREPARE_COMMIT_MSG: &str = include_str!("../../.githooks/prepare-commit-msg"); const REFERENCE_TRANSACTION: &str = include_str!("../../.githooks/reference-transaction"); @@ -210,6 +216,7 @@ const POST_COMMIT: &str = include_str!("../../.githooks/post-commit"); /// Every hook this command installs, in (filename, embedded template) pairs. const HOOKS: &[(&str, &str)] = &[ ("pre-commit", PRE_COMMIT), + ("pre-merge-commit", PRE_MERGE_COMMIT), ("pre-push", PRE_PUSH), ("prepare-commit-msg", PREPARE_COMMIT_MSG), ("reference-transaction", REFERENCE_TRANSACTION), @@ -368,7 +375,90 @@ pub(crate) fn ensure_on_path(_dir: &Path) -> Result { Ok(false) } +/// `true` on Unix when `path` has at least one executable bit set. Git +/// silently ignores a non-executable hook (just an advisory "hint", not an +/// error), so a content-correct-but-non-executable hook must NOT read as +/// installed -- confirmed live: this exact gap let a direct commit through +/// on `master` moments after this component's own commit landed, because +/// the merge hadn't yet brought in the executable-bit fix. +/// +/// Always `true` on non-Unix: there's no POSIX exec bit to check, and +/// `install_hooks_for` never attempts to set one there either (matching git +/// for Windows' own model, where hook "executability" isn't a filesystem +/// permission). +#[cfg(unix)] +fn is_executable(path: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + fs::metadata(path) + .map(|m| m.permissions().mode() & 0o111 != 0) + .unwrap_or(false) +} + +#[cfg(not(unix))] +fn is_executable(_path: &Path) -> bool { + true +} + +/// `true` when `repo_root`'s hooks are already current: `core.hooksPath` is +/// `.githooks` and every file in `HOOKS` exists there, executable, with +/// content matching the embedded template. Used by both the CLI command (to +/// skip a no-op re-copy) and the `init`/`doctor` "githooks" component (to +/// report satisfied without touching the filesystem). +pub(crate) fn hooks_installed_for(repo_root: &Path) -> bool { + let hooks_path = + flare_git_core::shell::run_in_opt(repo_root, &["config", "--get", "core.hooksPath"]); + if hooks_path.as_deref() != Some(".githooks") { + return false; + } + HOOKS.iter().all(|(name, template)| { + let dst = repo_root.join(".githooks").join(name); + fs::read(&dst).ok().as_deref() == Some(template.as_bytes()) && is_executable(&dst) + }) +} + +/// Writes the shared canonical templates (if missing), copies whichever of +/// `HOOKS` are missing or stale into `repo_root/.githooks/`, chmods +x +/// whichever aren't already executable (checked independently of content -- +/// a content-correct file can still have lost its executable bit), and +/// points `core.hooksPath` at it if it isn't already. Returns whether +/// anything actually changed. Shared by the interactive CLI command and the +/// `init`/`doctor` "githooks" component -- same logic, same source of +/// truth, so the two can never drift apart on what "installed" means. +pub(crate) fn install_hooks_for(repo_root: &Path) -> Result { + ensure_shared_templates().map_err(|e| format!("cannot write shared templates: {e}"))?; + + let local_dir = repo_root.join(".githooks"); + fs::create_dir_all(&local_dir).map_err(|e| format!("cannot create {local_dir:?}: {e}"))?; + + let mut changed = false; + 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; + } + + Ok(changed) +} + fn install_hooks(opts: InstallHooksArgs) { + let _ = opts; let repo_root = match std::env::current_dir() { Ok(d) => d, Err(e) => { @@ -387,54 +477,23 @@ fn install_hooks(opts: InstallHooksArgs) { return; } - if let Err(e) = ensure_shared_templates() { - crate::ui::error(&format!( - "agentflare git install-hooks: cannot write shared templates: {e}" - )); - return; - } - - let local_dir = repo_root.join(".githooks"); - if let Err(e) = fs::create_dir_all(&local_dir) { - crate::ui::error(&format!( - "agentflare git install-hooks: cannot create {local_dir:?}: {e}" - )); - return; - } - - let mut changed = false; - for (name, _) in HOOKS { - let src = shared_hooks_dir().join(name); - let dst = local_dir.join(name); - match fs::copy(&src, &dst) { - Ok(_) => { - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let _ = fs::set_permissions(&dst, fs::Permissions::from_mode(0o755)); - } + match install_hooks_for(&repo_root) { + Ok(changed) => { + for (name, _) in HOOKS { crate::ui::success(&format!(".githooks/{name}")); - changed = true; } - Err(e) => { - crate::ui::error(&format!("copying {name}: {e}")); - return; + crate::ui::success("core.hooksPath = .githooks"); + if changed { + println!( + "\nBranch-protection hooks installed. Direct commits/pushes to the \ + default branch are now blocked for every git client in this repo. \ + Commits are also stamped with provenance trailers, every ref \ + move is journaled to ~/.agentflare/audit/git-refs.jsonl, and \ + lean-ctx's code index refreshes in the background after each commit." + ); } } - } - - flare_git_core::shell::run_in(&repo_root, &["config", "core.hooksPath", ".githooks"]).ok(); - crate::ui::success("core.hooksPath = .githooks"); - - if changed { - println!( - "\nBranch-protection hooks installed. Direct commits/pushes to the \ - default branch are now blocked for every git client in this repo. \ - Commits are also stamped with provenance trailers, every ref \ - move is journaled to ~/.agentflare/audit/git-refs.jsonl, and \ - lean-ctx's code index refreshes in the background after each commit." - ); - let _ = opts; + Err(e) => crate::ui::error(&format!("agentflare git install-hooks: {e}")), } } @@ -1126,6 +1185,95 @@ mod tests { dir } + #[test] + fn install_hooks_for_writes_all_hooks_and_sets_core_hooks_path() { + let repo = init_repo(); + let changed = install_hooks_for(repo.path()).unwrap(); + assert!(changed); + for (name, template) in HOOKS { + let content = std::fs::read(repo.path().join(".githooks").join(name)).unwrap(); + assert_eq!( + content, + template.as_bytes(), + "{name} should match the embedded template" + ); + } + let hooks_path = + flare_git_core::shell::run_in_opt(repo.path(), &["config", "--get", "core.hooksPath"]); + assert_eq!(hooks_path.as_deref(), Some(".githooks")); + } + + #[test] + fn hooks_installed_for_reflects_install_state() { + let repo = init_repo(); + assert!(!hooks_installed_for(repo.path()), "nothing installed yet"); + install_hooks_for(repo.path()).unwrap(); + assert!( + hooks_installed_for(repo.path()), + "should report installed after install_hooks_for" + ); + } + + #[test] + fn install_hooks_for_is_idempotent() { + let repo = init_repo(); + assert!( + install_hooks_for(repo.path()).unwrap(), + "first install changes something" + ); + assert!( + !install_hooks_for(repo.path()).unwrap(), + "second install on an already-current repo must report no change" + ); + } + + #[test] + fn install_hooks_for_repairs_a_stale_hand_edited_hook() { + let repo = init_repo(); + install_hooks_for(repo.path()).unwrap(); + std::fs::write( + repo.path().join(".githooks").join("pre-commit"), + "tampered\n", + ) + .unwrap(); + + assert!( + !hooks_installed_for(repo.path()), + "tampered hook must not read as installed" + ); + let changed = install_hooks_for(repo.path()).unwrap(); + assert!(changed, "a stale hook must be rewritten"); + let content = std::fs::read(repo.path().join(".githooks").join("pre-commit")).unwrap(); + assert_eq!(content, PRE_COMMIT.as_bytes()); + } + + #[test] + #[cfg(unix)] + fn install_hooks_for_repairs_a_hook_that_lost_its_executable_bit() { + // Content-correct but not executable: git silently ignores the hook + // (an advisory hint, not an error) rather than running it -- so a + // check that only compares content would report "installed" on a + // hook that in practice never fires. Confirmed live: this exact gap + // let a direct commit through on master moments after this + // component's own fix commit landed, because the merge hadn't yet + // brought the executable-bit fix into the working tree. + use std::os::unix::fs::PermissionsExt; + let repo = init_repo(); + install_hooks_for(repo.path()).unwrap(); + let dst = repo.path().join(".githooks").join("pre-commit"); + std::fs::set_permissions(&dst, std::fs::Permissions::from_mode(0o644)).unwrap(); + + assert!( + !hooks_installed_for(repo.path()), + "a non-executable hook must not read as installed, even with correct content" + ); + let changed = install_hooks_for(repo.path()).unwrap(); + assert!(changed, "the lost executable bit must be restored"); + assert!(is_executable(&dst)); + // Content untouched -- only the mode needed fixing. + assert_eq!(std::fs::read(&dst).unwrap(), PRE_COMMIT.as_bytes()); + } + #[test] fn changed_paths_for_commit_includes_unstaged_modifications_not_just_staged() { // Regression for the CodeRabbit-flagged bypass on PR #303: `git diff --git a/src/cli/github_bridge.rs b/src/cli/github_bridge.rs new file mode 100644 index 00000000..cc0ec187 --- /dev/null +++ b/src/cli/github_bridge.rs @@ -0,0 +1,110 @@ +use clap::{Args, Subcommand}; + +/// Manage this repo's `.agentflare/config.toml` `[bridge]` overrides, used +/// by CLI/MCP call sites that publish or claim work through the GitHub +/// bridge (e.g. `handoff`'s `recipient="github"` path). Does NOT configure +/// the standalone daemon's claiming loop -- it has no reliable cwd, so it +/// only reads `AGENTFLARE_BRIDGE_ENABLED`/`AGENTFLARE_BRIDGE_REPO` env vars. +#[derive(Args)] +pub struct GithubBridgeArgs { + #[command(subcommand)] + pub command: GithubBridgeSubcommand, +} + +#[derive(Subcommand)] +pub enum GithubBridgeSubcommand { + /// Set repo/queue-label overrides for this repo. + Set { + /// Repo to publish/claim issues on (owner/repo). Defaults to this repo's origin remote when omitted. + #[arg(long)] + repo: Option, + /// Issue label marking the bridge's pull queue. + #[arg(long)] + queue_label: Option, + }, + /// Remove this repo's overrides, falling back to env vars / origin remote / defaults. + Unset, + /// Show the effective repo/queue-label for the current repo. + Status, +} + +impl GithubBridgeArgs { + pub fn run(self) { + match self.command { + GithubBridgeSubcommand::Set { repo, queue_label } => cmd_set(repo, queue_label), + GithubBridgeSubcommand::Unset => cmd_unset(), + GithubBridgeSubcommand::Status => cmd_status(), + } + } +} + +fn repo_root_or_exit() -> std::path::PathBuf { + let cwd = std::env::current_dir().unwrap_or_default(); + match flare_git_core::branch::repo_toplevel(&cwd) { + Some(root) => root, + None => { + eprintln!("error: not inside a git repository"); + std::process::exit(1); + } + } +} + +fn cmd_set(repo: Option, queue_label: Option) { + if repo.is_none() && queue_label.is_none() { + eprintln!("error: pass --repo and/or --queue-label"); + std::process::exit(1); + } + // Validate before persisting -- an unparseable `--repo` written as-is + // would only surface as a confusing failure the next time something + // resolves it, far from where the typo was made. + if let Some(r) = &repo + && crate::github::RepoId::parse(r).is_none() + { + eprintln!("error: --repo {r:?} is not a valid owner/repo"); + std::process::exit(1); + } + let root = repo_root_or_exit(); + match crate::github::bridge::config::write_project_bridge_settings( + &root, + repo.as_deref(), + queue_label.as_deref(), + ) { + Ok(path) => println!("wrote {}", path.display()), + Err(e) => { + eprintln!("error: {e}"); + std::process::exit(1); + } + } +} + +fn cmd_unset() { + let root = repo_root_or_exit(); + match crate::github::bridge::config::clear_project_bridge_settings(&root) { + Ok(path) => println!("cleared [bridge] overrides in {}", path.display()), + Err(e) => { + eprintln!("error: {e}"); + std::process::exit(1); + } + } +} + +fn cmd_status() { + let root = repo_root_or_exit(); + let repo = crate::github::bridge::config::resolve_project_repo(&root); + let queue_label = crate::github::bridge::config::resolve_project_queue_label(&root); + println!( + "repo: {}", + match repo { + Ok(Some(r)) => r.to_string(), + Ok(None) => "(none resolved)".to_string(), + Err(e) => format!("(error: {e})"), + } + ); + println!("queue_label: {queue_label}"); + println!(); + println!("note: this is what CLI/MCP calls (e.g. handoff) resolve from this repo."); + println!( + " the background daemon's claiming loop is unaffected -- it only reads \ + AGENTFLARE_BRIDGE_ENABLED/_REPO env vars, since it has no cwd to resolve this file from." + ); +} diff --git a/src/cli/memory.rs b/src/cli/memory.rs index 1c993c39..767e5fe2 100644 --- a/src/cli/memory.rs +++ b/src/cli/memory.rs @@ -44,7 +44,8 @@ pub enum MemoryCommands { batch: usize, }, /// Sync observations with a shared GitHub branch so other workstations - /// see the same facts. Requires AGENTFLARE_MEMORY_SYNC_REPO=owner/repo + /// see the same facts. Defaults to this repo's `origin` remote; set + /// AGENTFLARE_MEMORY_SYNC_REPO=owner/repo to point elsewhere /// (AGENTFLARE_MEMORY_SYNC_BRANCH/_PATH override the branch/file name). Sync, } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 324942d7..85d75dce 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -13,6 +13,7 @@ mod docs; mod doctor; mod gateway; pub(crate) mod git; +mod github_bridge; mod handoff; mod hook; mod init; @@ -69,6 +70,7 @@ pub enum Commands { Auth(auth::AuthArgs), Artifacts(artifacts::ArtifactsArgs), Handoff(handoff::HandoffArgs), + GithubBridge(github_bridge::GithubBridgeArgs), #[command(alias = "flare", visible_alias = "opt")] Optimize(optimize::OptimizeArgs), #[command(visible_alias = "logo")] @@ -107,6 +109,7 @@ impl Commands { Self::Auth(cmd) => cmd.run(), Self::Artifacts(cmd) => cmd.run(), Self::Handoff(cmd) => cmd.run(), + Self::GithubBridge(cmd) => cmd.run(), Self::Optimize(cmd) => cmd.run(), Self::About(cmd) => crate::about::run(cmd), Self::Channel(cmd) => cmd.run(), diff --git a/src/components.rs b/src/components.rs index 3f61fd3a..94786a6d 100644 --- a/src/components.rs +++ b/src/components.rs @@ -506,9 +506,15 @@ fn apply_coaching_defaults() -> String { } } -/// Fully-qualified flare-gateway tool names every core-module coaching rule -/// nudges toward. Kept allowlisted in `~/.claude/settings.json` so the -/// nudge doesn't cost a permission prompt on every call. +/// Fully-qualified flare-gateway tool names deemed safe to call unprompted. +/// Kept allowlisted in `~/.claude/settings.json` so calling them doesn't cost +/// a permission prompt every time. +/// +/// `handoff` is deliberately NOT here even though most of it is local +/// item/asset writes: its `recipient="github"` path publishes a real, +/// externally-visible GitHub issue, and an allowlisted tool call skips the +/// permission prompt that would otherwise let a human catch an unintended +/// external publish before it happens. const GATEWAY_PERMISSIONS_ALLOW: &[&str] = &[ "mcp__flare__docs", "mcp__flare__search", @@ -682,6 +688,32 @@ pub fn get_components(host: &str) -> Vec { check: Box::new(crate::shim_install::all_shims_present), apply: Box::new(crate::shim_install::install), }, + // Branch-protection git hooks (.githooks/, core.hooksPath): the + // PreToolUse guard in hook_redirect.rs only watches specific tool + // names, so a `git commit` via Bash -- or via any tool name it + // doesn't recognize (e.g. a gateway-routed ctx_patch call) -- slips + // past it entirely. A native git hook is the shell-agnostic + // enforcement boundary: it fires for every git client regardless of + // how the commit was invoked. Host-independent (a real git hook, + // not tied to any agent's own tool-call model), so this is not + // gated by `claude_code_only` the way `opencode-branch-guard` is. + Component { + id: "githooks", + needs_consent: true, + describe: "Branch-protection git hooks (.githooks/, core.hooksPath) — blocks direct commits/pushes to the default branch for every git client, not just tool calls this agent's PreToolUse hook watches".to_string(), + check: Box::new(|| match flare_git_core::branch::repo_toplevel(&cwd()) { + Some(root) => crate::cli::git::hooks_installed_for(&root), + None => true, + }), + apply: Box::new(|| match flare_git_core::branch::repo_toplevel(&cwd()) { + Some(root) => match crate::cli::git::install_hooks_for(&root) { + Ok(true) => "installed .githooks/* + core.hooksPath = .githooks".to_string(), + Ok(false) => "already up to date".to_string(), + Err(e) => format!("failed: {e}"), + }, + None => "not applicable outside a git repo".to_string(), + }), + }, // Claude Code's non-interactive Bash tool sources `~/.bashenv` via // BASH_ENV -- the lean-ctx function dispatcher (bash-level companion // to the PATH shims above) and the force-push/rm -rf DEBUG-trap @@ -1018,6 +1050,7 @@ mod tests { "rules", "mise", "shims", + "githooks", "claude-code-bashenv-guard", "opencode-branch-guard", "leanctx", @@ -1031,6 +1064,7 @@ mod tests { "rules", "mise", "shims", + "githooks", "claude-code-bashenv-guard", "opencode-branch-guard", "leanctx", @@ -1624,6 +1658,17 @@ mod tests { }); } + // No with_temp_cwd-based check/apply-cycle test here (unlike the other + // components above): the githooks component's closures resolve + // `flare_git_core::branch::repo_toplevel(&cwd())` fresh on every call, + // and under cargo test's parallel execution that raced with this + // process's real cwd and mutated *this actual checkout*'s + // core.hooksPath instead of the isolated tempdir (confirmed via + // .git/config's mtime). `hooks_installed_for`/`install_hooks_for` + // (`cli::git`'s tests) already cover the exact same logic these + // closures just delegate to, with explicit repo_root paths instead of + // ambient cwd -- no coverage lost by not re-testing it here too. + #[test] fn coaching_defaults_seed_all_default_rules_on_fresh_home() { crate::paths::test_support::with_temp_home(|| { diff --git a/src/github/bridge/config.rs b/src/github/bridge/config.rs index b5b6b0b9..4aac5e33 100644 --- a/src/github/bridge/config.rs +++ b/src/github/bridge/config.rs @@ -14,6 +14,242 @@ pub const MIN_INTERVAL_SECS: u64 = 15; const DEFAULT_MAX_CLAIMS: usize = 3; const DEFAULT_QUEUE_LABEL: &str = "agentflare"; +/// `repo`/`queue_label` overrides read from `.agentflare/config.toml`'s +/// `[bridge]` table. Deliberately narrower than `BridgeConfig`: the +/// standalone daemon (`runner::resolve_repo`) has no reliable cwd (neither +/// the launchd plist nor the systemd unit sets one), so it can't consume a +/// project-local file -- only CLI/MCP call sites that run from inside a +/// real repo (e.g. `handoff`'s `recipient="github"` path, `agentflare +/// github-bridge`) do. `enabled`/`interval_secs`/`max_claims` aren't read +/// from here for the same reason: nothing in this project-scoped path +/// claims issues, so those knobs would have no consumer. +#[derive(Debug, Default, Clone)] +struct ProjectBridgeSettings { + repo: Option, + queue_label: Option, +} + +fn bridge_table(doc: &toml::Value) -> Option<&toml::value::Table> { + doc.get("bridge")?.as_table() +} + +/// Project-local layer wins over user-home on a per-key basis (same +/// precedence flare_git_core's other config consumers use). A malformed +/// file falls back to defaults rather than failing the caller -- this is a +/// convenience override, not a hard requirement. +fn read_project_bridge_settings(repo_root: &Path) -> ProjectBridgeSettings { + let Ok(layers) = + flare_git_core::config_loader::locate_and_parse(repo_root, Some(&crate::paths::home())) + else { + return ProjectBridgeSettings::default(); + }; + let mut out = ProjectBridgeSettings::default(); + for doc in [ + layers.user_home.as_ref().map(|(_, v)| v), + layers.project_local.as_ref().map(|(_, v)| v), + ] + .into_iter() + .flatten() + { + let Some(bridge) = bridge_table(doc) else { + continue; + }; + if let Some(v) = bridge.get("repo").and_then(|v| v.as_str()) { + out.repo = Some(v.to_string()); + } + if let Some(v) = bridge.get("queue_label").and_then(|v| v.as_str()) { + out.queue_label = Some(v.to_string()); + } + } + out +} + +/// `AGENTFLARE_BRIDGE_REPO`, else `.agentflare/config.toml`'s +/// `[bridge].repo`, else `repo_root`'s `origin` remote. +/// +/// An explicit override (env var or project file) that fails to parse as +/// `owner/repo` is an `Err`, not a silent fall-through to `origin` — a typo'd +/// override that quietly published to the wrong repo is worse than a loud +/// failure. Only the absence of any override falls back to `origin`, which +/// is why that last step alone stays `Option`-shaped. +pub fn resolve_project_repo(repo_root: &Path) -> Result, String> { + if let Some(explicit) = std::env::var("AGENTFLARE_BRIDGE_REPO") + .ok() + .filter(|s| !s.trim().is_empty()) + { + return crate::github::RepoId::parse(explicit.trim()) + .map(Some) + .ok_or_else(|| { + format!("AGENTFLARE_BRIDGE_REPO={explicit:?} is not a valid owner/repo") + }); + } + if let Some(repo_str) = read_project_bridge_settings(repo_root).repo { + return crate::github::RepoId::parse(repo_str.trim()) + .map(Some) + .ok_or_else(|| { + format!( + "[bridge].repo = {repo_str:?} in .agentflare/config.toml is not a valid owner/repo" + ) + }); + } + Ok(crate::github::RepoId::resolve_from_remote(repo_root)) +} + +/// Same resolution the standalone daemon (`bridge::runner::resolve_repo`) +/// uses: `AGENTFLARE_BRIDGE_REPO`, else `repo_root`'s `origin` remote. +/// Deliberately excludes the project-local `.agentflare/config.toml` +/// override `resolve_project_repo` also consults -- the daemon has no +/// reliable cwd (see the module doc), so it can never read that file. +/// Exposed so a CLI/MCP call site that resolves via the project file can +/// tell whether a locally-running daemon would actually watch the same repo. +pub fn resolve_daemon_repo(repo_root: &Path) -> Option { + if let Some(explicit) = std::env::var("AGENTFLARE_BRIDGE_REPO") + .ok() + .filter(|s| !s.trim().is_empty()) + { + return crate::github::RepoId::parse(explicit.trim()); + } + crate::github::RepoId::resolve_from_remote(repo_root) +} + +/// Whether `AGENTFLARE_BRIDGE_ENABLED` would let a daemon started on this +/// workstation actually poll -- the same truthiness check `BridgeConfig` +/// applies, exposed standalone so a caller can decide whether comparing +/// against [`resolve_daemon_repo`] is even meaningful. +pub fn daemon_enabled() -> bool { + std::env::var("AGENTFLARE_BRIDGE_ENABLED").is_ok_and(|v| truthy(&v)) +} + +/// `AGENTFLARE_BRIDGE_QUEUE_LABEL`, else `.agentflare/config.toml`'s +/// `[bridge].queue_label`, else `DEFAULT_QUEUE_LABEL`. Every source is +/// trimmed and an empty/whitespace-only result is treated as absent, so a +/// stray blank value falls through to the next source instead of becoming +/// the effective (and unusable) label. +pub fn resolve_project_queue_label(repo_root: &Path) -> String { + std::env::var("AGENTFLARE_BRIDGE_QUEUE_LABEL") + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .or_else(|| { + read_project_bridge_settings(repo_root) + .queue_label + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + }) + .unwrap_or_else(|| DEFAULT_QUEUE_LABEL.to_string()) +} + +/// Opens (creating if absent) `.agentflare/config.toml.lock` next to the +/// config file and takes an exclusive advisory lock on it, blocking until +/// acquired. Held for the caller's whole read-modify-write section so two +/// concurrent `github-bridge set`/`unset` processes serialize instead of +/// racing to overwrite each other's change. The lock is released when the +/// returned file is dropped. +fn lock_project_config(repo_root: &Path) -> Result { + let dir = repo_root.join(".agentflare"); + std::fs::create_dir_all(&dir).map_err(|e| format!("{}: {e}", dir.display()))?; + let lock_path = dir.join("config.toml.lock"); + let file = std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .open(&lock_path) + .map_err(|e| format!("{}: {e}", lock_path.display()))?; + fs2::FileExt::lock_exclusive(&file).map_err(|e| format!("{}: {e}", lock_path.display()))?; + Ok(file) +} + +/// Writes `doc` to `path` via a same-directory temp file + rename, so a +/// process that dies mid-write leaves the original file intact rather than +/// truncated -- `rename` is atomic on both POSIX and Windows when source and +/// destination share a filesystem, which a sibling temp file guarantees. +fn atomic_write_toml(path: &Path, doc: &toml::Value) -> Result<(), String> { + let tmp_path = path.with_file_name(format!("config.toml.{}.tmp", std::process::id())); + std::fs::write( + &tmp_path, + toml::to_string_pretty(doc).map_err(|e| e.to_string())?, + ) + .map_err(|e| format!("{}: {e}", tmp_path.display()))?; + std::fs::rename(&tmp_path, path).map_err(|e| format!("{}: {e}", path.display())) +} + +/// Merges `repo`/`queue_label` into `.agentflare/config.toml`'s `[bridge]` +/// table (creating the file and directory if needed), leaving any other +/// top-level table (e.g. `[git_shim]`) untouched. Comments are not +/// preserved -- `toml::Value` isn't a comment-preserving representation, +/// same tradeoff `components::merge_json` already accepts for the JSON +/// config files agentflare merges elsewhere. +/// +/// The whole read-modify-write happens under [`lock_project_config`] and the +/// result lands via [`atomic_write_toml`], so two `github-bridge set` +/// processes racing on the same file serialize instead of one silently +/// clobbering the other's change, and a crash mid-write can't leave the file +/// truncated. +pub fn write_project_bridge_settings( + repo_root: &Path, + repo: Option<&str>, + queue_label: Option<&str>, +) -> Result { + let _lock = lock_project_config(repo_root)?; + let path = repo_root.join(".agentflare").join("config.toml"); + let mut doc: toml::Value = match std::fs::read_to_string(&path) { + Ok(s) => s.parse().map_err(|e| format!("{}: {e}", path.display()))?, + // Only an absent file means "start from an empty document" -- any + // other read failure (permissions, the path being a directory, a + // transient I/O error) must not be treated the same way, or this + // would silently overwrite an existing, merely-unreadable config + // file with one containing only the [bridge] table just written. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + toml::Value::Table(toml::value::Table::new()) + } + Err(e) => return Err(format!("{}: {e}", path.display())), + }; + let table = doc + .as_table_mut() + .ok_or_else(|| format!("{}: top-level value is not a table", path.display()))?; + let bridge = table + .entry("bridge") + .or_insert_with(|| toml::Value::Table(toml::value::Table::new())) + .as_table_mut() + .ok_or_else(|| format!("{}: [bridge] is not a table", path.display()))?; + if let Some(r) = repo { + bridge.insert("repo".to_string(), toml::Value::String(r.to_string())); + } + if let Some(l) = queue_label { + bridge.insert( + "queue_label".to_string(), + toml::Value::String(l.to_string()), + ); + } + atomic_write_toml(&path, &doc)?; + Ok(path) +} + +/// Removes the `[bridge]` table entirely from `.agentflare/config.toml`, +/// falling resolution back to env vars / origin remote / defaults. A noop +/// (not an error) when the file or table doesn't exist. Same locking + +/// atomic-replace treatment as [`write_project_bridge_settings`]. +pub fn clear_project_bridge_settings(repo_root: &Path) -> Result { + let _lock = lock_project_config(repo_root)?; + let path = repo_root.join(".agentflare").join("config.toml"); + // Only an absent file is a true noop -- any other read failure must + // surface as an error rather than falsely reporting "cleared" when + // nothing was actually read or changed. + let content = match std::fs::read_to_string(&path) { + Ok(s) => s, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(path), + Err(e) => return Err(format!("{}: {e}", path.display())), + }; + let mut doc: toml::Value = content + .parse() + .map_err(|e| format!("{}: {e}", path.display()))?; + if let Some(table) = doc.as_table_mut() { + table.remove("bridge"); + } + atomic_write_toml(&path, &doc)?; + Ok(path) +} + #[derive(Debug, Clone)] pub struct BridgeConfig { pub enabled: bool, @@ -26,6 +262,15 @@ pub struct BridgeConfig { pub max_claims: usize, pub ttl_secs: i64, pub queue_label: String, + /// Agent to dispatch for issues this instance claims, e.g. `claude-code` + /// -- must match an `agent_registry::Agent` id exactly + /// (`supervisor::resolve_confirmed_agent`), same as `handoff`'s + /// recipient. `None` (the default) means claimed items are never + /// labeled `ready-for-work`: nothing dispatches, same as before this + /// field existed. Deliberately not auto-detected -- which of several + /// installed agents should work claimed issues is a choice, not + /// something to guess. + pub work_agent: Option, pub instance_id: String, } @@ -256,6 +501,7 @@ impl BridgeConfig { get("AGENTFLARE_BRIDGE_INTERVAL_SECS").as_deref(), get("AGENTFLARE_BRIDGE_MAX_CLAIMS").as_deref(), get("AGENTFLARE_BRIDGE_QUEUE_LABEL").as_deref(), + get("AGENTFLARE_BRIDGE_WORK_AGENT").as_deref(), instance, ) } @@ -267,6 +513,7 @@ impl BridgeConfig { interval: Option<&str>, max_claims: Option<&str>, queue_label: Option<&str>, + work_agent: Option<&str>, instance_id: String, ) -> BridgeConfig { BridgeConfig { @@ -281,6 +528,10 @@ impl BridgeConfig { // Reuses the EXISTING claim TTL so marker liveness and the local // ledger expire on one schedule. ttl_secs: crate::claims::ttl_secs(), + work_agent: work_agent + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string), queue_label: queue_label .filter(|s| !s.is_empty()) .unwrap_or(DEFAULT_QUEUE_LABEL) @@ -296,12 +547,13 @@ mod tests { #[test] fn defaults_are_off_and_conservative() { - let c = BridgeConfig::from_values(None, None, None, None, "agent:1".to_string()); + let c = BridgeConfig::from_values(None, None, None, None, None, "agent:1".to_string()); assert!(!c.enabled, "bridge must be opt-in"); assert_eq!(c.interval_secs, 60); assert_eq!(c.max_claims, 3); assert_eq!(c.queue_label, "agentflare"); assert_eq!(c.instance_id, "agent:1"); + assert_eq!(c.work_agent, None, "no dispatch until opted in"); } #[test] @@ -311,26 +563,262 @@ mod tests { Some("15"), Some("7"), Some("queue"), + Some("claude-code"), "agent:1".to_string(), ); assert!(c.enabled); assert_eq!(c.interval_secs, 15); assert_eq!(c.max_claims, 7); assert_eq!(c.queue_label, "queue"); + assert_eq!(c.work_agent.as_deref(), Some("claude-code")); + } + + #[test] + fn work_agent_blank_or_whitespace_only_is_none() { + for v in ["", " "] { + let c = BridgeConfig::from_values(None, None, None, None, Some(v), "a".to_string()); + assert_eq!(c.work_agent, None, "{v:?} should not set a work agent"); + } } #[test] fn enabled_accepts_common_truthy_spellings() { for v in ["1", "true", "TRUE", "yes"] { - let c = BridgeConfig::from_values(Some(v), None, None, None, "a".to_string()); + let c = BridgeConfig::from_values(Some(v), None, None, None, None, "a".to_string()); assert!(c.enabled, "{v} should enable"); } for v in ["0", "false", "no", "", "banana"] { - let c = BridgeConfig::from_values(Some(v), None, None, None, "a".to_string()); + let c = BridgeConfig::from_values(Some(v), None, None, None, None, "a".to_string()); assert!(!c.enabled, "{v} should not enable"); } } + #[test] + fn resolve_project_repo_prefers_env_then_project_file_then_origin_remote() { + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_REPO"); + } + + let dir = tempfile::tempdir().unwrap(); + flare_git_core::shell::run_in(dir.path(), &["init", "-q"]).unwrap(); + flare_git_core::shell::run_in( + dir.path(), + &[ + "remote", + "add", + "origin", + "git@github.com:origin-owner/origin-repo.git", + ], + ) + .unwrap(); + + assert_eq!( + resolve_project_repo(dir.path()) + .unwrap() + .map(|r| r.to_string()), + Some("origin-owner/origin-repo".to_string()) + ); + + write_project_bridge_settings(dir.path(), Some("file-owner/file-repo"), None).unwrap(); + assert_eq!( + resolve_project_repo(dir.path()) + .unwrap() + .map(|r| r.to_string()), + Some("file-owner/file-repo".to_string()) + ); + + unsafe { + std::env::set_var("AGENTFLARE_BRIDGE_REPO", "env-owner/env-repo"); + } + assert_eq!( + resolve_project_repo(dir.path()) + .unwrap() + .map(|r| r.to_string()), + Some("env-owner/env-repo".to_string()) + ); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_REPO"); + } + } + + #[test] + fn resolve_project_repo_rejects_an_invalid_explicit_override_instead_of_falling_through() { + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_REPO"); + } + + let dir = tempfile::tempdir().unwrap(); + flare_git_core::shell::run_in(dir.path(), &["init", "-q"]).unwrap(); + flare_git_core::shell::run_in( + dir.path(), + &[ + "remote", + "add", + "origin", + "git@github.com:origin-owner/origin-repo.git", + ], + ) + .unwrap(); + + // A malformed project-file override must error, not silently fall + // through to origin — a typo should not go unnoticed and quietly + // publish somewhere else. + write_project_bridge_settings(dir.path(), Some("not-a-valid-repo"), None).unwrap(); + let err = resolve_project_repo(dir.path()).unwrap_err(); + assert!(err.contains("not-a-valid-repo"), "{err}"); + + clear_project_bridge_settings(dir.path()).unwrap(); + + // Same for an explicit env override. + unsafe { + std::env::set_var("AGENTFLARE_BRIDGE_REPO", "also-not-valid"); + } + let err = resolve_project_repo(dir.path()).unwrap_err(); + assert!(err.contains("also-not-valid"), "{err}"); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_REPO"); + } + } + + #[test] + fn resolve_project_queue_label_falls_back_through_env_file_default() { + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_QUEUE_LABEL"); + } + let dir = tempfile::tempdir().unwrap(); + + assert_eq!(resolve_project_queue_label(dir.path()), "agentflare"); + + write_project_bridge_settings(dir.path(), None, Some("custom-label")).unwrap(); + assert_eq!(resolve_project_queue_label(dir.path()), "custom-label"); + + unsafe { + std::env::set_var("AGENTFLARE_BRIDGE_QUEUE_LABEL", "env-label"); + } + assert_eq!(resolve_project_queue_label(dir.path()), "env-label"); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_QUEUE_LABEL"); + } + } + + #[test] + fn resolve_project_queue_label_trims_and_ignores_whitespace_only_overrides() { + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_QUEUE_LABEL"); + } + let dir = tempfile::tempdir().unwrap(); + + // A whitespace-only project-file value must not become the effective + // label -- fall through to the default instead. + write_project_bridge_settings(dir.path(), None, Some(" ")).unwrap(); + assert_eq!(resolve_project_queue_label(dir.path()), "agentflare"); + + // A padded value must come back trimmed. + write_project_bridge_settings(dir.path(), None, Some(" padded-label ")).unwrap(); + assert_eq!(resolve_project_queue_label(dir.path()), "padded-label"); + + unsafe { + std::env::set_var("AGENTFLARE_BRIDGE_QUEUE_LABEL", " "); + } + assert_eq!( + resolve_project_queue_label(dir.path()), + "padded-label", + "a whitespace-only env override must fall through to the project file" + ); + unsafe { + std::env::set_var("AGENTFLARE_BRIDGE_QUEUE_LABEL", " env-padded "); + } + assert_eq!(resolve_project_queue_label(dir.path()), "env-padded"); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_QUEUE_LABEL"); + } + } + + #[test] + fn write_project_bridge_settings_preserves_other_top_level_tables() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join(".agentflare")).unwrap(); + std::fs::write( + dir.path().join(".agentflare").join("config.toml"), + "[git_shim]\nextra_trust_root_paths = [\"x\"]\n", + ) + .unwrap(); + + write_project_bridge_settings(dir.path(), Some("o/r"), None).unwrap(); + + let content = + std::fs::read_to_string(dir.path().join(".agentflare").join("config.toml")).unwrap(); + let parsed: toml::Value = content.parse().unwrap(); + assert_eq!( + parsed + .get("git_shim") + .and_then(|g| g.get("extra_trust_root_paths")), + Some(&toml::Value::Array(vec![toml::Value::String("x".into())])) + ); + assert_eq!( + parsed + .get("bridge") + .and_then(|b| b.get("repo")) + .and_then(|v| v.as_str()), + Some("o/r") + ); + } + + #[test] + fn clear_project_bridge_settings_removes_only_the_bridge_table() { + let dir = tempfile::tempdir().unwrap(); + write_project_bridge_settings(dir.path(), Some("o/r"), Some("l")).unwrap(); + let mut content = + std::fs::read_to_string(dir.path().join(".agentflare").join("config.toml")).unwrap(); + content.push_str("\n[git_shim]\nextra_trust_root_paths = [\"x\"]\n"); + std::fs::write(dir.path().join(".agentflare").join("config.toml"), content).unwrap(); + + clear_project_bridge_settings(dir.path()).unwrap(); + + let content = + std::fs::read_to_string(dir.path().join(".agentflare").join("config.toml")).unwrap(); + let parsed: toml::Value = content.parse().unwrap(); + assert!(parsed.get("bridge").is_none()); + assert!(parsed.get("git_shim").is_some()); + } + + #[test] + fn write_project_bridge_settings_errors_rather_than_silently_starting_fresh_on_a_real_read_failure() + { + // A directory sitting where the config file is expected makes + // `read_to_string` fail with something other than `NotFound` on + // every platform -- must surface as an error, not be treated the + // same as "file absent" and have its (unreadable) content silently + // replaced by a document containing only the [bridge] table. + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join(".agentflare").join("config.toml")).unwrap(); + + let err = write_project_bridge_settings(dir.path(), Some("o/r"), None).unwrap_err(); + assert!(err.contains("config.toml"), "{err}"); + } + + #[test] + fn clear_project_bridge_settings_errors_rather_than_silently_reporting_cleared_on_a_real_read_failure() + { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join(".agentflare").join("config.toml")).unwrap(); + + let err = clear_project_bridge_settings(dir.path()).unwrap_err(); + assert!(err.contains("config.toml"), "{err}"); + } + #[test] fn garbage_numbers_fall_back_to_defaults_rather_than_panicking() { let c = BridgeConfig::from_values( @@ -338,6 +826,7 @@ mod tests { Some("not-a-number"), Some(""), None, + None, "a".to_string(), ); assert_eq!(c.interval_secs, 60); @@ -346,7 +835,7 @@ mod tests { #[test] fn interval_has_a_floor_so_a_typo_cannot_hammer_github() { - let c = BridgeConfig::from_values(Some("1"), Some("0"), None, None, "a".to_string()); + let c = BridgeConfig::from_values(Some("1"), Some("0"), None, None, None, "a".to_string()); assert_eq!(c.interval_secs, MIN_INTERVAL_SECS); } @@ -550,7 +1039,7 @@ mod tests { #[test] fn max_claims_zero_is_legal_drain_mode_not_a_floor_violation() { - let c = BridgeConfig::from_values(Some("1"), None, Some("0"), None, "a".to_string()); + let c = BridgeConfig::from_values(Some("1"), None, Some("0"), None, None, "a".to_string()); assert_eq!( c.max_claims, 0, "0 must pass through unfloored: it means drain mode (stop claiming \ diff --git a/src/github/bridge/handoff_payload.rs b/src/github/bridge/handoff_payload.rs new file mode 100644 index 00000000..20ccbbbd --- /dev/null +++ b/src/github/bridge/handoff_payload.rs @@ -0,0 +1,154 @@ +//! Embeds/recovers the structured handoff payload (`content`/`completed`/ +//! `remaining`/`thread_id`) and an idempotency key onto/from a GitHub issue +//! body published via `handoff`'s `recipient="github"` path +//! (`mcp_server::handoff::handoff_to_bridge_queue`), read back by the bridge +//! importer (`tick::record_claim`) when it turns a claimed issue into a +//! local item, and looked up again by `handoff_to_bridge_queue` itself +//! before publishing to avoid a duplicate on retry. +//! +//! Kept as a single hidden HTML comment appended after the human-readable +//! body, so the visible issue text stays exactly what the caller wrote -- +//! same rendering trick `bridge::marker` uses for claim state, but a +//! separate format: this is a one-shot descriptive payload, not the +//! append-only claim/heartbeat state machine `marker` models. +//! +//! The payload is base64-encoded before embedding, not embedded as raw +//! JSON: `content`/`completed`/`remaining` are caller-supplied text with no +//! constraint against containing ` -->` or even a full fake +//! `"; + +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct HandoffPayload { + /// Dedup key for idempotent publication: the handoff's `thread_id` when + /// given, else its `name`. Two publishes with the same key are the same + /// logical handoff -- a retry after a timeout, not a second one. + pub key: String, + pub content: String, + pub completed: String, + pub remaining: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub thread_id: Option, +} + +impl HandoffPayload { + /// Appends this payload as a hidden, base64-encoded marker after `body` + /// (the human-readable text shown on the issue). Always the LAST thing + /// in the returned string -- [`Self::extract`] relies on that to find + /// its own marker rather than an earlier one already present in `body`. + pub fn embed(&self, body: &str) -> String { + let json = serde_json::to_string(self).unwrap_or_default(); + let encoded = base64::engine::general_purpose::STANDARD.encode(json.as_bytes()); + format!("{body}\n\n{MARKER_PREFIX}{encoded}{MARKER_SUFFIX}") + } + + /// Recovers a payload previously written by [`Self::embed`], if `body` + /// ends with one. Tolerant of a missing or malformed marker (a + /// hand-edited issue, or one predating this format) -- returns `None` + /// rather than failing the caller. + /// + /// Anchored to the END of `body` on both sides: `strip_suffix` requires + /// the marker to be the very last thing present (true of every marker + /// this module writes), and `rfind` for the prefix then takes the LAST + /// match before that suffix -- so an earlier marker-shaped string + /// sitting in the human-visible text above it is not mistaken for the + /// real one. + pub fn extract(body: &str) -> Option { + let before_suffix = body.trim_end().strip_suffix(MARKER_SUFFIX)?; + let start = before_suffix.rfind(MARKER_PREFIX)? + MARKER_PREFIX.len(); + let decoded = base64::engine::general_purpose::STANDARD + .decode(&before_suffix[start..]) + .ok()?; + let json = String::from_utf8(decoded).ok()?; + serde_json::from_str(&json).ok() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn payload() -> HandoffPayload { + HandoffPayload { + key: "k".into(), + content: "c".into(), + completed: "done so far".into(), + remaining: "left to do".into(), + thread_id: Some("t".into()), + } + } + + #[test] + fn embed_then_extract_round_trips() { + let body = payload().embed("visible text"); + assert!(body.starts_with("visible text")); + assert_eq!(HandoffPayload::extract(&body), Some(payload())); + } + + #[test] + fn embed_without_a_thread_id_round_trips_to_none() { + let mut p = payload(); + p.thread_id = None; + let body = p.embed("text"); + assert_eq!(HandoffPayload::extract(&body).unwrap().thread_id, None); + } + + #[test] + fn extract_returns_none_for_a_body_with_no_marker() { + assert_eq!(HandoffPayload::extract("just some text"), None); + assert_eq!(HandoffPayload::extract(""), None); + } + + #[test] + fn extract_tolerates_a_hand_edited_or_pre_existing_issue() { + assert_eq!( + HandoffPayload::extract("some text\n\n"), + None + ); + } + + #[test] + fn a_field_containing_the_html_close_delimiter_still_round_trips() { + // A naive "search forward for ` -->`" extractor would stop at the + // delimiter INSIDE this field and truncate the marker; base64 + // encoding means the literal sequence can't appear in the marker at + // all, so this must round-trip exactly. + let mut p = payload(); + p.content = "before --> after, and before-->after too".into(); + let body = p.embed("visible text"); + assert_eq!(HandoffPayload::extract(&body), Some(p)); + } + + #[test] + fn a_field_containing_the_marker_prefix_still_round_trips() { + let mut p = payload(); + p.content = MARKER_PREFIX.to_string(); + let body = p.embed("visible text"); + assert_eq!(HandoffPayload::extract(&body), Some(p)); + } + + #[test] + fn a_valid_looking_marker_in_the_visible_body_is_not_mistaken_for_the_real_one() { + // The visible body already contains something that parses as a + // (different) marker -- extract must still recover the one `embed` + // actually appended at the end, not this earlier one. + let fake = HandoffPayload { + key: "fake".into(), + content: "fake".into(), + completed: "fake".into(), + remaining: "fake".into(), + thread_id: None, + }; + let visible = fake.embed("visible"); + let body = payload().embed(&visible); + assert_eq!(HandoffPayload::extract(&body), Some(payload())); + } +} diff --git a/src/github/bridge/mod.rs b/src/github/bridge/mod.rs index 4898e9ea..00aefe4c 100644 --- a/src/github/bridge/mod.rs +++ b/src/github/bridge/mod.rs @@ -3,8 +3,10 @@ pub mod claim; pub mod config; +pub mod handoff_payload; pub mod items; pub mod marker; +pub mod queue_status; pub mod runner; pub mod tick; diff --git a/src/github/bridge/queue_status.rs b/src/github/bridge/queue_status.rs new file mode 100644 index 00000000..fe409f8c --- /dev/null +++ b/src/github/bridge/queue_status.rs @@ -0,0 +1,218 @@ +//! Read-only queue-depth signal for the bridge's pull queue -- how many +//! labelled issues are open, how many are currently unclaimed, and how +//! stale the oldest unclaimed one is. Queryable from anywhere (no local +//! daemon state needed, only the GitHub API) since the point is to give an +//! agent something to check *before* deciding whether to route work onto +//! the queue (`handoff` `recipient="github"`) or keep it local: an empty or +//! fast-clearing queue suggests capacity exists somewhere; unclaimed issues +//! piling up suggests nothing is currently pulling from it. + +use crate::github::bridge::claim as claim_rules; +use crate::github::{Client, GitHubError, RepoId, issues}; + +#[derive(Debug, Clone, serde::Serialize)] +pub struct QueueStatus { + pub total_open: usize, + pub unclaimed: usize, + /// Seconds since the oldest unclaimed issue WITH A KNOWN `created_at` + /// was opened. `None` when no unclaimed issue has a known `created_at` + /// (including when `unclaimed` is 0) -- check + /// `unclaimed_with_unknown_age` before treating this as "no unclaimed + /// issues are old": a `None`/low value here can coexist with unclaimed + /// issues of truly unknown age. + pub oldest_unclaimed_age_secs: Option, + /// Unclaimed issues whose `created_at` was missing or unparseable, and + /// so could not factor into `oldest_unclaimed_age_secs` at all -- an + /// explicit signal that the "oldest" figure may be missing an even + /// older issue, rather than silently treating it as accurate. + pub unclaimed_with_unknown_age: usize, + /// Distinct claim owners currently holding at least one issue, with + /// their held count -- a rough proxy for how many workstations are + /// actively pulling from this queue right now. Sorted by owner name + /// for stable output. + pub claims_by_owner: Vec<(String, usize)>, +} + +fn parse_unix(ts: &Option) -> Option { + ts.as_deref() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok()) + .map(|dt| dt.timestamp()) +} + +/// One `list_comments` call per open (queue-labelled) issue -- NOT the +/// repo-wide `/issues/comments` endpoint, which has no per-issue or +/// per-label filter and would fetch comments on every issue and PR in the +/// repo just to find the handful belonging to this queue. For a busy repo +/// with far more total issue traffic than open queue depth (the common +/// case -- `max_claims` per instance is a handful, so the queue itself is +/// meant to stay small), that would cost far MORE data than the N calls +/// this makes, not less. Bounded by `open_issues.len()`, which the queue +/// label already keeps small by design. +pub fn queue_status( + client: &Client, + repo: &RepoId, + queue_label: &str, + now: i64, + ttl_secs: i64, +) -> Result { + let open_issues = issues::list_filtered(client, repo, "open", Some(queue_label), None)?; + let mut unclaimed = 0; + let mut unclaimed_with_unknown_age = 0; + let mut oldest_unclaimed_created_at: Option = None; + let mut claims_by_owner: std::collections::BTreeMap = Default::default(); + + for issue in &open_issues { + let comments: Vec<(u64, String)> = issues::list_comments(client, repo, issue.number, None)? + .into_iter() + .map(|c| (c.id, c.body)) + .collect(); + match claim_rules::resolve_holder(&comments, now, ttl_secs) { + Some(holder) => { + *claims_by_owner.entry(holder.marker.owner).or_insert(0) += 1; + } + None => { + unclaimed += 1; + match parse_unix(&issue.created_at) { + Some(created_at) => { + oldest_unclaimed_created_at = Some( + oldest_unclaimed_created_at.map_or(created_at, |c| c.min(created_at)), + ); + } + None => unclaimed_with_unknown_age += 1, + } + } + } + } + + Ok(QueueStatus { + total_open: open_issues.len(), + unclaimed, + oldest_unclaimed_age_secs: oldest_unclaimed_created_at.map(|c| (now - c).max(0)), + unclaimed_with_unknown_age, + claims_by_owner: claims_by_owner.into_iter().collect(), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::github::RepoId; + use crate::github::test_support::{MockResponse, MockServer}; + + fn repo() -> RepoId { + RepoId { + owner: "o".into(), + repo: "r".into(), + } + } + + fn issue_json(number: u64, created_at: &str) -> String { + format!( + r#"{{"number":{number},"html_url":"https://x/{number}","state":"open","title":"t{number}","created_at":"{created_at}"}}"# + ) + } + + fn claim_comment(owner: &str, ts: i64) -> String { + format!( + r#"{{"id":1,"user":{{"login":"bot"}},"body":"claiming\n\n"}}"# + ) + } + + #[test] + fn counts_unclaimed_and_tracks_the_oldest() { + let server = MockServer::start(vec![ + // issue_list + MockResponse::json( + 200, + &format!( + "[{},{}]", + issue_json(1, "2026-01-01T00:00:00Z"), + issue_json(2, "2026-01-02T00:00:00Z") + ), + ), + // comments for issue 1: none + MockResponse::json(200, "[]"), + // comments for issue 2: none + MockResponse::json(200, "[]"), + ]); + let client = server.client(None); + let now = chrono::DateTime::parse_from_rfc3339("2026-01-03T00:00:00Z") + .unwrap() + .timestamp(); + + let status = queue_status(&client, &repo(), "agentflare", now, 300).unwrap(); + assert_eq!(status.total_open, 2); + assert_eq!(status.unclaimed, 2); + // Oldest unclaimed is issue 1, created 2026-01-01 -- 2 days before `now`. + assert_eq!(status.oldest_unclaimed_age_secs, Some(2 * 24 * 60 * 60)); + assert_eq!(status.unclaimed_with_unknown_age, 0); + assert!(status.claims_by_owner.is_empty()); + } + + #[test] + fn counts_claimed_issues_by_owner() { + let now = 1_000_000i64; + let server = MockServer::start(vec![ + MockResponse::json(200, &format!("[{}]", issue_json(1, "2026-01-01T00:00:00Z"))), + MockResponse::json( + 200, + &format!("[{}]", claim_comment("workstation-a", now - 10)), + ), + ]); + let client = server.client(None); + + let status = queue_status(&client, &repo(), "agentflare", now, 300).unwrap(); + assert_eq!(status.total_open, 1); + assert_eq!(status.unclaimed, 0); + assert_eq!(status.oldest_unclaimed_age_secs, None); + assert_eq!(status.unclaimed_with_unknown_age, 0); + assert_eq!( + status.claims_by_owner, + vec![("workstation-a".to_string(), 1)] + ); + } + + #[test] + fn an_expired_claim_counts_as_unclaimed() { + let now = 1_000_000i64; + let server = MockServer::start(vec![ + MockResponse::json(200, &format!("[{}]", issue_json(1, "2026-01-01T00:00:00Z"))), + // Claim marker is way older than the ttl -- stale, must not count as held. + MockResponse::json( + 200, + &format!("[{}]", claim_comment("workstation-a", now - 10_000)), + ), + ]); + let client = server.client(None); + + let status = queue_status(&client, &repo(), "agentflare", now, 300).unwrap(); + assert_eq!(status.unclaimed, 1); + assert!(status.claims_by_owner.is_empty()); + } + + #[test] + fn an_unclaimed_issue_with_no_created_at_is_reported_as_unknown_age_not_ignored() { + let server = MockServer::start(vec![ + MockResponse::json( + 200, + r#"[{"number":1,"html_url":"u","state":"open","title":"t"}]"#, + ), + MockResponse::json(200, "[]"), + ]); + let client = server.client(None); + let now = chrono::DateTime::parse_from_rfc3339("2026-01-03T00:00:00Z") + .unwrap() + .timestamp(); + + let status = queue_status(&client, &repo(), "agentflare", now, 300).unwrap(); + assert_eq!(status.unclaimed, 1); + assert_eq!( + status.oldest_unclaimed_age_secs, None, + "no unclaimed issue has a known created_at" + ); + assert_eq!( + status.unclaimed_with_unknown_age, 1, + "the missing timestamp must be counted, not silently dropped" + ); + } +} diff --git a/src/github/bridge/runner.rs b/src/github/bridge/runner.rs index 3b94ab0c..4c2e4480 100644 --- a/src/github/bridge/runner.rs +++ b/src/github/bridge/runner.rs @@ -174,6 +174,7 @@ mod tests { None, None, None, + None, "a:1".to_string(), ); assert!(!should_run(&cfg)); @@ -186,6 +187,7 @@ mod tests { None, None, None, + None, "a:1".to_string(), ); assert!(should_run(&cfg)); diff --git a/src/github/bridge/tests/live_github.rs b/src/github/bridge/tests/live_github.rs index e35baaeb..ec73bc44 100644 --- a/src/github/bridge/tests/live_github.rs +++ b/src/github/bridge/tests/live_github.rs @@ -74,6 +74,7 @@ fn live(repo: RepoId, max_claims: usize) -> Live { None, Some(&max_claims.to_string()), None, + None, instance.clone(), ), project_id: project_id.clone(), diff --git a/src/github/bridge/tests/two_instance.rs b/src/github/bridge/tests/two_instance.rs index 132d1249..3971f94a 100644 --- a/src/github/bridge/tests/two_instance.rs +++ b/src/github/bridge/tests/two_instance.rs @@ -182,6 +182,7 @@ impl Instance { None, Some(&max_claims.to_string()), None, + None, owner.to_string(), ), project_id: project_id.clone(), diff --git a/src/github/bridge/tick.rs b/src/github/bridge/tick.rs index a7ad19b6..2e83c39a 100644 --- a/src/github/bridge/tick.rs +++ b/src/github/bridge/tick.rs @@ -347,26 +347,79 @@ fn record_claim( .map_err(|e| GitHubError::Parse(e.to_string()))?; existing } - None => agentflare_backend::item::create( - conn, - agentflare_backend::item::CreateItem { - project_id: ctx.project_id.clone(), - state_id, - name: issue.title.clone(), - description: issue.body.clone(), - priority: None, - parent_id: None, - assignee_agent: Some(ctx.config.instance_id.clone()), - sort_order: None, - external_source: Some(items::EXTERNAL_SOURCE.to_string()), - external_id: Some(issue.number.to_string()), - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .map_err(|e| GitHubError::Parse(e.to_string()))?, + None => { + // Labeling `ready-for-work` (when a work agent is configured + // and the project has that label at all -- skipped otherwise + // rather than creating it out of nowhere, same reasoning as + // `handoff_impl`) is what actually gets this claimed issue + // dispatched: `spawn_supervisor_discovery`'s own tick is what + // launches the agent from there, not this function. Without a + // work agent, `assignee_agent` stays the bridge's own instance + // id -- claimed but nothing picks it up, same as before this + // existed. + let ready_label_id = ctx.config.work_agent.as_ref().and_then(|_| { + agentflare_backend::label::list_by_project(conn, &ctx.project_id) + .ok() + .and_then(|labels| { + labels + .into_iter() + .find(|l| l.name == crate::supervisor::READY_LABEL) + }) + .map(|l| l.id) + }); + // `handoff`'s `recipient="github"` path embeds the full + // structured payload (content/completed/remaining/thread_id) as + // a hidden marker after the human-readable body -- recover it + // so a bridge-originated item carries the same fields a local + // handoff would, instead of only the rendered issue body with no + // metadata. An issue opened by hand (or from before this + // existed) has no marker; `handoff` returns `None` and this + // falls back to the old behavior unchanged. + let handoff = issue + .body + .as_deref() + .and_then(crate::github::bridge::handoff_payload::HandoffPayload::extract); + let description = handoff + .as_ref() + .map(|p| p.content.clone()) + .or_else(|| issue.body.clone()); + let metadata = handoff.as_ref().map(|p| { + let mut m = serde_json::json!({ + "completed": p.completed, + "remaining": p.remaining, + }); + if let Some(t) = &p.thread_id { + m["thread_id"] = serde_json::json!(t); + } + m.to_string() + }); + + agentflare_backend::item::create( + conn, + agentflare_backend::item::CreateItem { + project_id: ctx.project_id.clone(), + state_id, + name: issue.title.clone(), + description, + priority: None, + parent_id: None, + assignee_agent: Some( + ctx.config + .work_agent + .clone() + .unwrap_or_else(|| ctx.config.instance_id.clone()), + ), + sort_order: None, + external_source: Some(items::EXTERNAL_SOURCE.to_string()), + external_id: Some(issue.number.to_string()), + metadata, + label_ids: ready_label_id.into_iter().collect(), + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .map_err(|e| GitHubError::Parse(e.to_string()))? + } }; // Local ledger too, so this instance's OWN agents do not double-claim. @@ -783,6 +836,69 @@ mod tests { let sent: serde_json::Value = serde_json::from_str(&rewrite.body).unwrap(); let marker = Marker::parse(sent["body"].as_str().unwrap()).unwrap(); assert_eq!(marker.item, item.id); + + // Without a configured work agent, a claim is visible but nobody's + // been told to work it -- assignee stays the bridge's own instance + // id, not a real dispatchable agent name. + assert_eq!(item.assignee_agent.as_deref(), Some("me:1")); + } + + #[test] + fn winning_a_claim_with_a_work_agent_configured_dispatches_it() { + let server = MockServer::start(vec![ + MockResponse::json( + 200, + r#"[{"number":7,"html_url":"u","state":"open","title":"Do the thing","body":"","labels":[{"name":"agentflare"}]}]"#, + ), + MockResponse::json(200, "[]"), + MockResponse::json(201, r#"{"id":100}"#), + MockResponse::json( + 200, + &format!( + r#"[{{"id":100,"user":{{"login":"u"}},"body":{}}}]"#, + serde_json::to_string(&marker_body(Action::Claim, "me:1", NOW)).unwrap() + ), + ), + MockResponse::json(200, r#"{"id":100}"#), + MockResponse::json(200, "[]"), + ]); + let (conn, project_id) = test_db(); + let project = agentflare_backend::project::get(&conn, &project_id).unwrap(); + agentflare_backend::label::create( + &conn, + agentflare_backend::label::CreateLabel { + project_id: Some(project_id.clone()), + workspace_id: project.workspace_id, + name: crate::supervisor::READY_LABEL.to_string(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + + let mut ctx = ctx_with_project(test_ctx(&server, 3), project_id.clone()); + ctx.config.work_agent = Some("claude-code".to_string()); + let report = run_once(&ctx, &conn, NOW).unwrap(); + + assert_eq!(report.claimed, vec![7]); + let item = crate::github::bridge::items::find_by_issue(&conn, &project_id, 7).unwrap(); + assert_eq!( + item.assignee_agent.as_deref(), + Some("claude-code"), + "assignee must be the configured work agent, not the bridge's own instance id" + ); + let label_ids = agentflare_backend::item::list_labels(&conn, &item.id).unwrap(); + let label_names: Vec = label_ids + .iter() + .map(|id| agentflare_backend::label::get(&conn, id).unwrap().name) + .collect(); + assert!( + label_names.contains(&crate::supervisor::READY_LABEL.to_string()), + "expected ready-for-work label, got {label_names:?}" + ); } #[test] @@ -819,6 +935,54 @@ mod tests { let _ = server.requests(); } + #[test] + fn claiming_an_issue_published_by_handoff_recovers_its_full_payload() { + // `handoff`'s recipient="github" path embeds content/completed/ + // remaining/thread_id as a hidden marker after the visible body + // (`handoff_payload::HandoffPayload`); a bare `issue.body.clone()` + // would only ever see the visible half. + let payload = crate::github::bridge::handoff_payload::HandoffPayload { + key: "thread-1".into(), + content: "the full content".into(), + completed: "done so far".into(), + remaining: "left to do".into(), + thread_id: Some("thread-1".into()), + }; + let body = payload.embed("visible description"); + let issue_list = format!( + r#"[{{"number":7,"html_url":"u","state":"open","title":"t","body":{},"labels":[{{"name":"agentflare"}}]}}]"#, + serde_json::to_string(&body).unwrap() + ); + let server = MockServer::start(vec![ + MockResponse::json(200, &issue_list), + MockResponse::json(200, "[]"), + MockResponse::json(201, r#"{"id":100}"#), + MockResponse::json( + 200, + &format!( + r#"[{{"id":100,"user":{{"login":"u"}},"body":{}}}]"#, + serde_json::to_string(&marker_body(Action::Claim, "me:1", NOW)).unwrap() + ), + ), + MockResponse::json(200, r#"{"id":100}"#), // marker rewrite + MockResponse::json(200, "[]"), // claimed: label + ]); + let (conn, project_id) = test_db(); + let ctx = ctx_with_project(test_ctx(&server, 3), project_id.clone()); + let report = run_once(&ctx, &conn, NOW).unwrap(); + + assert_eq!(report.claimed, vec![7]); + let item = crate::github::bridge::items::find_by_issue(&conn, &project_id, 7).unwrap(); + assert_eq!( + item.description, "the full content", + "description must come from the embedded payload's content, not the visible body" + ); + let metadata: serde_json::Value = serde_json::from_str(&item.metadata).unwrap(); + assert_eq!(metadata["completed"], "done so far"); + assert_eq!(metadata["remaining"], "left to do"); + assert_eq!(metadata["thread_id"], "thread-1"); + } + #[test] fn losing_the_race_creates_no_local_item() { let server = MockServer::start(vec![ @@ -1597,6 +1761,7 @@ mod tests { None, Some(&max_claims.to_string()), None, + None, "me:1".to_string(), ), project_id: String::new(), diff --git a/src/github/contents.rs b/src/github/contents.rs index 97135a87..a13ea432 100644 --- a/src/github/contents.rs +++ b/src/github/contents.rs @@ -1,6 +1,8 @@ //! Git Contents/Refs API — just enough to read and write one file on a named -//! branch, creating that branch from the repo's default branch if it doesn't -//! exist yet. Backs `memory::sync`; not a general git client. +//! branch, creating that branch as an orphan commit (empty tree, no parent) +//! if it doesn't exist yet — the branch carries only what's written onto it +//! via `put_file`, not a snapshot of the whole repo. Backs `memory::sync`; +//! not a general git client. use crate::github::{Client, GitHubError, RepoId}; use base64::Engine as _; @@ -105,9 +107,12 @@ pub fn put_file( .ok_or_else(|| GitHubError::Parse("put response missing content.sha".to_string())) } -/// Makes sure `branch` exists, branching it off the repo's default branch if -/// not. The Contents API commits onto an existing branch ref only — it does -/// not create one implicitly. +/// Makes sure `branch` exists, creating it as an orphan branch (a root +/// commit over an empty tree, no parent) if not — so it starts out +/// containing nothing rather than a copy of the whole repo, since `put_file` +/// never removes what a branch inherited from wherever it was cut from. The +/// Contents API commits onto an existing branch ref only — it does not +/// create one implicitly. pub fn ensure_branch(client: &Client, repo: &RepoId, branch: &str) -> Result<(), GitHubError> { let ref_path = format!( "/repos/{}/{}/git/ref/heads/{}", @@ -118,28 +123,36 @@ pub fn ensure_branch(client: &Client, repo: &RepoId, branch: &str) -> Result<(), match client.request("GET", &ref_path, None) { Ok(_) => Ok(()), Err(GitHubError::NotFound) => { - let default_branch = super::repos::get_default_branch(client, repo)?; - let default_ref_path = format!( - "/repos/{}/{}/git/ref/heads/{}", - repo.owner, - repo.repo, - crate::github::encode_query(&default_branch) - ); - let default_ref = client.request("GET", &default_ref_path, None)?; - let sha = default_ref - .get("object") - .and_then(|o| o.get("sha")) + let tree_path = format!("/repos/{}/{}/git/trees", repo.owner, repo.repo); + let tree = + client.request("POST", &tree_path, Some(serde_json::json!({ "tree": [] })))?; + let tree_sha = tree + .get("sha") + .and_then(|s| s.as_str()) + .ok_or_else(|| GitHubError::Parse("tree response missing sha".to_string()))?; + + let commit_path = format!("/repos/{}/{}/git/commits", repo.owner, repo.repo); + let commit = client.request( + "POST", + &commit_path, + Some(serde_json::json!({ + "message": format!("chore: init {branch} (orphan)"), + "tree": tree_sha, + "parents": Vec::::new(), + })), + )?; + let commit_sha = commit + .get("sha") .and_then(|s| s.as_str()) - .ok_or_else(|| { - GitHubError::Parse("default branch ref missing object.sha".to_string()) - })?; + .ok_or_else(|| GitHubError::Parse("commit response missing sha".to_string()))?; + let create_path = format!("/repos/{}/{}/git/refs", repo.owner, repo.repo); let create_result = client.request( "POST", &create_path, Some(serde_json::json!({ "ref": format!("refs/heads/{branch}"), - "sha": sha, + "sha": commit_sha, })), ); // 422 here almost always means another sync run won the race and @@ -244,11 +257,11 @@ mod tests { } #[test] - fn ensure_branch_creates_from_default_branch_when_missing() { + fn ensure_branch_creates_an_orphan_commit_when_missing() { let server = MockServer::start(vec![ MockResponse::json(404, r#"{"message":"Not Found"}"#), - MockResponse::json(200, r#"{"default_branch":"main"}"#), - MockResponse::json(200, r#"{"object":{"sha":"tip123"}}"#), + MockResponse::json(201, r#"{"sha":"emptytree"}"#), + MockResponse::json(201, r#"{"sha":"orphancommit"}"#), MockResponse::json(201, r#"{"ref":"refs/heads/agentflare-memory"}"#), ]); let client = server.client(Some("tok")); @@ -256,13 +269,23 @@ mod tests { let reqs = server.requests(); assert_eq!(reqs[0].path, "/repos/o/r/git/ref/heads/agentflare-memory"); - assert_eq!(reqs[1].path, "/repos/o/r"); - assert_eq!(reqs[2].path, "/repos/o/r/git/ref/heads/main"); + + assert_eq!(reqs[1].method, "POST"); + assert_eq!(reqs[1].path, "/repos/o/r/git/trees"); + let tree_sent: serde_json::Value = serde_json::from_str(&reqs[1].body).unwrap(); + assert_eq!(tree_sent["tree"], serde_json::json!([])); + + assert_eq!(reqs[2].method, "POST"); + assert_eq!(reqs[2].path, "/repos/o/r/git/commits"); + let commit_sent: serde_json::Value = serde_json::from_str(&reqs[2].body).unwrap(); + assert_eq!(commit_sent["tree"], "emptytree"); + assert_eq!(commit_sent["parents"], serde_json::json!([])); + assert_eq!(reqs[3].method, "POST"); assert_eq!(reqs[3].path, "/repos/o/r/git/refs"); let sent: serde_json::Value = serde_json::from_str(&reqs[3].body).unwrap(); assert_eq!(sent["ref"], "refs/heads/agentflare-memory"); - assert_eq!(sent["sha"], "tip123"); + assert_eq!(sent["sha"], "orphancommit"); } #[test] @@ -285,8 +308,8 @@ mod tests { fn ensure_branch_treats_a_concurrent_create_422_as_success_when_the_branch_now_exists() { let server = MockServer::start(vec![ MockResponse::json(404, r#"{"message":"Not Found"}"#), - MockResponse::json(200, r#"{"default_branch":"main"}"#), - MockResponse::json(200, r#"{"object":{"sha":"tip123"}}"#), + MockResponse::json(201, r#"{"sha":"emptytree"}"#), + MockResponse::json(201, r#"{"sha":"orphancommit"}"#), MockResponse::json(422, r#"{"message":"Reference already exists"}"#), MockResponse::json(200, r#"{"object":{"sha":"tip123"}}"#), ]); @@ -304,8 +327,8 @@ mod tests { fn ensure_branch_propagates_a_422_when_the_branch_still_does_not_exist() { let server = MockServer::start(vec![ MockResponse::json(404, r#"{"message":"Not Found"}"#), - MockResponse::json(200, r#"{"default_branch":"main"}"#), - MockResponse::json(200, r#"{"object":{"sha":"tip123"}}"#), + MockResponse::json(201, r#"{"sha":"emptytree"}"#), + MockResponse::json(201, r#"{"sha":"orphancommit"}"#), MockResponse::json(422, r#"{"message":"Validation Failed"}"#), MockResponse::json(404, r#"{"message":"Not Found"}"#), ]); diff --git a/src/github/models.rs b/src/github/models.rs index b333d724..ff3cc938 100644 --- a/src/github/models.rs +++ b/src/github/models.rs @@ -65,6 +65,9 @@ pub struct Issue { #[serde(default)] #[allow(dead_code)] pub updated_at: Option, + #[serde(default)] + #[allow(dead_code)] + pub created_at: Option, } #[derive(Debug, Clone, Deserialize)] diff --git a/src/mcp_server/flare_git.rs b/src/mcp_server/flare_git.rs index eb302f6d..9567cf8a 100644 --- a/src/mcp_server/flare_git.rs +++ b/src/mcp_server/flare_git.rs @@ -23,6 +23,7 @@ impl AgentflareMcp { "issue_comment", "issue_close", "issue_label", + "bridge_queue_status", "release_list", "release_get", "release_latest", @@ -234,6 +235,43 @@ impl AgentflareMcp { issues::add_labels(&client, &repo, n, &labels).map_err(to_mcp_error)?; format!("Added {} label(s) to issue #{n}", labels.len()) } + "bridge_queue_status" => { + // Capacity signal for deciding whether to route new work + // onto the bridge queue (handoff recipient="github") or + // keep it local: an empty/fast-clearing queue suggests + // capacity exists somewhere; unclaimed issues piling up + // suggests nothing is currently pulling from it. Reads only + // -- no local daemon state needed, so this reflects reality + // across every workstation with the bridge enabled, not + // just this one. + let cwd = std::env::current_dir().unwrap_or_default(); + let queue_label = crate::github::bridge::config::resolve_project_queue_label(&cwd); + // An explicit `req.repo` always wins (`repo` above already + // reflects that). Otherwise resolve through the SAME chain + // `handoff`'s `recipient="github"` path uses + // (`resolve_project_repo`: env, then project-local + // `.agentflare/config.toml`, then origin) rather than the + // plain-origin resolution `repo` fell back to -- otherwise a + // project with a `[bridge].repo` override would have + // `handoff` publish to one repo and this status check read + // the queue of a different one. + let status_repo = if req.repo.is_some() { + repo.clone() + } else { + crate::github::bridge::config::resolve_project_repo(&cwd) + .map_err(|e| ErrorData::invalid_params(e, None))? + .unwrap_or_else(|| repo.clone()) + }; + let status = crate::github::bridge::queue_status::queue_status( + &client, + &status_repo, + &queue_label, + crate::claims::now(), + crate::claims::ttl_secs(), + ) + .map_err(to_mcp_error)?; + serde_json::to_string_pretty(&status).unwrap_or_default() + } "release_list" => { let rels = releases::list(&client, &repo).map_err(to_mcp_error)?; serde_json::to_string(&rels.iter().map(|r| &r.tag_name).collect::>()) diff --git a/src/mcp_server/handoff.rs b/src/mcp_server/handoff.rs index d48a3806..c9520078 100644 --- a/src/mcp_server/handoff.rs +++ b/src/mcp_server/handoff.rs @@ -44,6 +44,29 @@ impl AgentflareMcp { } let recipient = recipient.trim().to_string(); let name = name.trim().to_string(); + + // "github" is reserved: it means "any workstation," not a specific + // agent. Publishes as a labelled issue on the bridge's pull queue + // instead of a local item -- the already-running bridge tick loop + // (src/github/bridge/tick.rs) picks it up on whichever workstation + // has claim headroom next, no local item/asset created here at all. + if recipient.eq_ignore_ascii_case("github") { + if item_id.is_some() { + return Err(ErrorData::invalid_params( + "recipient=\"github\" publishes new work to the bridge queue -- it can't target an existing item_id", + None, + )); + } + return self.handoff_to_bridge_queue( + &name, + &content, + description.as_deref(), + &completed, + &remaining, + thread_id.as_deref(), + ); + } + let ext = match r#type.as_deref() { Some("html") => "html", Some("mermaid") | Some("diagram") => "mmd", @@ -289,6 +312,146 @@ impl AgentflareMcp { })? } + /// Publishes `name`/body as a GitHub issue labelled with the bridge's + /// queue label, on the repo resolved from `AGENTFLARE_BRIDGE_REPO`, else + /// this repo's `.agentflare/config.toml` `[bridge].repo` override, else + /// the workstation's `origin` remote (`bridge::config::resolve_project_repo` + /// -- same chain `agentflare github-bridge` and `bridge_queue_status` + /// resolve through). Deliberately thin: issue creation and the + /// claim/heartbeat/export lifecycle already live in `github::issues` and + /// `github::bridge::tick`; this just gets work onto the queue. + /// + /// Idempotent across retries while the previous attempt's issue is still + /// UNCLAIMED: the full structured payload (`content`, `completed`, + /// `remaining`, `thread_id`) and a dedup key (`thread_id`, else `name`) + /// are embedded as a hidden marker in the issue body + /// (`bridge::handoff_payload`) -- recovered by the bridge importer + /// (`tick::record_claim`) when the issue is claimed, and looked up here + /// first so a retry after a timeout reuses the existing issue instead of + /// publishing a duplicate. Once claimed, a matching key is NOT reused -- + /// a local item already exists carrying that payload, and nothing + /// re-reads the issue afterward, so reusing it would silently drop this + /// call's (possibly updated) `completed`/`remaining` instead of + /// publishing them as a fresh, distinct entry. The result's `reused` + /// field says which happened. + fn handoff_to_bridge_queue( + &self, + name: &str, + content: &str, + description: Option<&str>, + completed: &str, + remaining: &str, + thread_id: Option<&str>, + ) -> Result { + use crate::github::bridge::claim as claim_rules; + use crate::github::bridge::handoff_payload::HandoffPayload; + use crate::github::{Client, bridge::config, issues}; + + let repo_root = self.worktree_repo_root(); + let repo = config::resolve_project_repo(&repo_root) + .map_err(|e| ErrorData::invalid_params(e, None))? + .ok_or_else(|| { + ErrorData::invalid_params( + "recipient=\"github\" needs a GitHub `origin` remote in the current repo \ + (or a [bridge] repo override in .agentflare/config.toml)", + None, + ) + })?; + let client = Client::new().map_err(to_mcp_error)?; + let queue_label = config::resolve_project_queue_label(&repo_root); + + let key = thread_id.unwrap_or(name).to_string(); + let payload = HandoffPayload { + key: key.clone(), + content: content.to_string(), + completed: completed.to_string(), + remaining: remaining.to_string(), + thread_id: thread_id.map(str::to_string), + }; + + // A bare retry after e.g. a network timeout must reuse the issue + // this call already created rather than publish a second one -- + // `issues::create` has no idempotency of its own. But only while + // that issue is still UNCLAIMED: once the bridge (or anything else) + // has claimed it, a local item already exists carrying this exact + // payload, and nothing re-reads the issue afterward -- returning it + // again here would silently swallow this call's (possibly updated) + // completed/remaining instead of the bare retry this exists for. + // Same claim-liveness check `queue_status`/`tick` already use, so a + // stale (expired) claim is correctly treated as no claim at all. + let candidate = issues::list_filtered(&client, &repo, "open", Some(&queue_label), None) + .map_err(to_mcp_error)? + .into_iter() + .find(|issue| { + issue + .body + .as_deref() + .and_then(HandoffPayload::extract) + .is_some_and(|p| p.key == key) + }); + let reusable = match candidate { + Some(issue) => { + let comments: Vec<(u64, String)> = + issues::list_comments(&client, &repo, issue.number, None) + .map_err(to_mcp_error)? + .into_iter() + .map(|c| (c.id, c.body)) + .collect(); + let claimed = claim_rules::resolve_holder( + &comments, + crate::claims::now(), + crate::claims::ttl_secs(), + ) + .is_some(); + (!claimed).then_some(issue) + } + None => None, + }; + + let (issue, reused) = match reusable { + Some(issue) => (issue, true), + None => { + let body = payload.embed(description.unwrap_or(content)); + let issue = issues::create( + &client, + &repo, + name, + Some(&body), + std::slice::from_ref(&queue_label), + &[], + ) + .map_err(to_mcp_error)?; + (issue, false) + } + }; + + let mut result = serde_json::json!({ + "repo": repo.to_string(), + "issue_number": issue.number, + "issue_url": issue.html_url, + "queue_label": queue_label, + "recipient": "github", + "reused": reused, + }); + + // Report rather than reject: a project-local [bridge].repo override + // can legitimately target a repo another workstation's daemon + // watches, not this one -- see resolve_project_repo's module doc. + // But if THIS workstation's daemon is enabled and points somewhere + // else, say so -- nothing local will poll what was just published. + if config::daemon_enabled() + && let Some(daemon_repo) = config::resolve_daemon_repo(&repo_root) + && daemon_repo != repo + { + result["warning"] = serde_json::json!(format!( + "this workstation's bridge daemon is enabled but watches {daemon_repo} -- it \ + will not poll {repo}; relying on another workstation's daemon to pick this up" + )); + } + + Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) + } + /// Verified, not trusted: rejects a fabricated or typo'd continuation /// OID rather than recording it as-is. `oid` must exist in the repo as /// a commit (not just any object); when `branch` is given and exists, @@ -417,6 +580,55 @@ mod tests { .unwrap(); } + #[test] + fn recipient_github_rejects_an_item_id() { + // Credential-independent, like flare_git_impl's own + // unknown_action_is_rejected_before_repo_or_client_setup: this must + // fail on its own merits before ever resolving a repo or a client. + let (_tmp, mcp) = test_mcp(); + let req = HandoffRequest { + recipient: "github".to_string(), + item_id: Some("some-item".to_string()), + ..base_request() + }; + let err = mcp.handoff_impl(req).unwrap_err(); + assert!(err.to_string().contains("item_id"), "{err}"); + } + + #[test] + fn recipient_github_without_an_origin_remote_fails_clearly() { + // test_mcp()'s repo has no `origin` configured, so this exercises + // handoff_to_bridge_queue's repo resolution without hitting the + // network at all -- but only if AGENTFLARE_BRIDGE_REPO isn't + // inherited from the outer environment; resolve_project_repo checks + // it before `origin`, and a set value would let this reach + // Client::new()/issues::create instead, hitting the network and + // potentially creating a real issue. Cleared and restored under the + // shared lock other env-mutating tests in this crate already use. + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let prior = std::env::var("AGENTFLARE_BRIDGE_REPO").ok(); + unsafe { + std::env::remove_var("AGENTFLARE_BRIDGE_REPO"); + } + + let (_tmp, mcp) = test_mcp(); + let req = HandoffRequest { + recipient: "github".to_string(), + ..base_request() + }; + let err = mcp.handoff_impl(req).unwrap_err(); + assert!(err.to_string().contains("origin"), "{err}"); + + unsafe { + match &prior { + Some(v) => std::env::set_var("AGENTFLARE_BRIDGE_REPO", v), + None => std::env::remove_var("AGENTFLARE_BRIDGE_REPO"), + } + } + } + #[test] fn new_item_gets_labeled_ready_for_work_when_the_project_has_that_label() { let (_tmp, mcp) = test_mcp(); diff --git a/src/mcp_server/types.rs b/src/mcp_server/types.rs index 2295ddbf..f7b34807 100644 --- a/src/mcp_server/types.rs +++ b/src/mcp_server/types.rs @@ -469,7 +469,7 @@ pub(crate) struct FlareDocsRequest { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] pub(crate) struct GitHubRequest { #[schemars( - description = "Action: pr_create|pr_list|pr_get|pr_status|pr_wait|pr_merge|pr_comment|pr_request_review|issue_create|issue_list|issue_get|issue_comment|issue_close|issue_label|release_list|release_get|release_latest|release_create|run_list|run_get|run_rerun|workflow_dispatch" + description = "Action: pr_create|pr_list|pr_get|pr_status|pr_wait|pr_merge|pr_comment|pr_request_review|issue_create|issue_list|issue_get|issue_comment|issue_close|issue_label|bridge_queue_status|release_list|release_get|release_latest|release_create|run_list|run_get|run_rerun|workflow_dispatch" )] pub(crate) action: String, #[schemars(description = "owner/repo (default: resolved from the current repo's origin)")] diff --git a/src/memory/sync.rs b/src/memory/sync.rs index bd478090..5e95710c 100644 --- a/src/memory/sync.rs +++ b/src/memory/sync.rs @@ -24,23 +24,37 @@ pub struct MemorySyncConfig { } impl MemorySyncConfig { - /// `AGENTFLARE_MEMORY_SYNC_REPO` (required, `owner/repo`), - /// `AGENTFLARE_MEMORY_SYNC_BRANCH` (default `agentflare-memory`), - /// `AGENTFLARE_MEMORY_SYNC_PATH` (default `memory-sync.jsonl`). - /// - /// Env-driven and explicit-repo-only, same shape as `BridgeConfig` -- - /// but unlike the bridge, never derived from cwd's `origin` remote: - /// memory is global to the workstation, not scoped to whatever project - /// happens to be checked out where this command is run. + /// Repo defaults to the current directory's `origin` remote (same as + /// the GitHub bridge), so no per-machine setup is needed and each + /// project's observations land on its own branch of its own repo + /// rather than one global log shared across every project on this + /// workstation. Override with `AGENTFLARE_MEMORY_SYNC_REPO` + /// (`owner/repo`) to point somewhere else. + /// `AGENTFLARE_MEMORY_SYNC_BRANCH` (default `agentflare-memory`) and + /// `AGENTFLARE_MEMORY_SYNC_PATH` (default `memory-sync.jsonl`) are + /// still independently overridable. pub fn from_env() -> Result { - let repo_str = std::env::var("AGENTFLARE_MEMORY_SYNC_REPO").map_err(|_| { - "AGENTFLARE_MEMORY_SYNC_REPO is not set -- point it at an owner/repo you can \ - push to (a small private repo works fine)" - .to_string() - })?; - let repo = RepoId::parse(repo_str.trim()).ok_or_else(|| { - format!("AGENTFLARE_MEMORY_SYNC_REPO={repo_str:?} is not a GitHub owner/repo") - })?; + let cwd = std::env::current_dir() + .map_err(|e| format!("cannot read the working directory: {e}"))?; + Self::from_env_at(&cwd) + } + + /// Split out from `from_env` so repo resolution is testable without + /// mutating the process's working directory. + pub fn from_env_at(repo_root: &std::path::Path) -> Result { + let repo = match std::env::var("AGENTFLARE_MEMORY_SYNC_REPO") + .ok() + .filter(|s| !s.trim().is_empty()) + { + Some(repo_str) => RepoId::parse(repo_str.trim()).ok_or_else(|| { + format!("AGENTFLARE_MEMORY_SYNC_REPO={repo_str:?} is not a GitHub owner/repo") + })?, + None => RepoId::resolve_from_remote(repo_root).ok_or_else(|| { + "no GitHub `origin` remote here -- run this from a repo you can push to, \ + or set AGENTFLARE_MEMORY_SYNC_REPO=owner/repo" + .to_string() + })?, + }; let branch = std::env::var("AGENTFLARE_MEMORY_SYNC_BRANCH") .ok() .filter(|s| !s.trim().is_empty()) @@ -364,7 +378,34 @@ mod tests { } #[test] - fn from_env_requires_a_repo() { + fn from_env_at_falls_back_to_the_repo_roots_origin_remote() { + let _guard = agent_registry::detect::PATH_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let original = std::env::var_os("AGENTFLARE_MEMORY_SYNC_REPO"); + unsafe { + std::env::remove_var("AGENTFLARE_MEMORY_SYNC_REPO"); + } + + let dir = tempfile::tempdir().unwrap(); + flare_git_core::shell::run_in(dir.path(), &["init", "-q"]).unwrap(); + flare_git_core::shell::run_in( + dir.path(), + &["remote", "add", "origin", "git@github.com:o/r.git"], + ) + .unwrap(); + let config = MemorySyncConfig::from_env_at(dir.path()).unwrap(); + assert_eq!(config.repo.to_string(), "o/r"); + + if let Some(v) = original { + unsafe { + std::env::set_var("AGENTFLARE_MEMORY_SYNC_REPO", v); + } + } + } + + #[test] + fn from_env_at_requires_a_repo_when_theres_no_origin_remote() { let _guard = agent_registry::detect::PATH_LOCK .lock() .unwrap_or_else(|e| e.into_inner()); @@ -372,8 +413,11 @@ mod tests { unsafe { std::env::remove_var("AGENTFLARE_MEMORY_SYNC_REPO"); } - let err = MemorySyncConfig::from_env().unwrap_err(); + + let dir = tempfile::tempdir().unwrap(); + let err = MemorySyncConfig::from_env_at(dir.path()).unwrap_err(); assert!(err.contains("AGENTFLARE_MEMORY_SYNC_REPO")); + if let Some(v) = original { unsafe { std::env::set_var("AGENTFLARE_MEMORY_SYNC_REPO", v);