Conversation
Split single-file src/update.rs into src/update/{mod,github,swap}.rs.
swap.rs is the reusable MCP-safe binary-replacement primitive:
- never signals/kills any process, so a live `agentflare mcp` server keeps
running its loaded image and picks up the new binary on next launch
- Windows: rename-aside + copy, with rollback on failure and a deferred
.bat fallback when the exe is hard-locked against renaming
- Unix: same-fs staged atomic rename
- find_killable_pids() reports (never kills) other running instances
github.rs isolates release discovery/download/checksum, with pure
parse_checksum/verify_checksum helpers now unit-tested.
Prepares update::swap::replace_binary for reuse by dev-install (#127).
…nary New `agentflare dev-install` subcommand: cargo build (release by default), resolve the built binary via `cargo metadata` (honors CARGO_TARGET_DIR and workspace shared targets), verify it answers `--version` under a timeout, then swap it over the running binary using the MCP-safe update::swap::replace_binary primitive (#122) — so a live `agentflare mcp` server is never disturbed. - refuses to overwrite the build output itself (guards against running from the freshly built binary instead of the installed one) - --debug builds debug; --dry-run reports the swap without performing it - pure cargo-metadata target-dir parsing + same-file guard are unit-tested
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds GitHub-based self-updates with checksum verification, cross-platform binary replacement, and a ChangesSelf-update and development installation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Agentflare
participant UpdateRoutine
participant GitHub
participant BinarySwap
Agentflare->>UpdateRoutine: run(version, check_only, quiet)
UpdateRoutine->>GitHub: fetch release metadata, asset, and checksum
GitHub-->>UpdateRoutine: release data and asset bytes
UpdateRoutine->>UpdateRoutine: verify checksum and extract binary
UpdateRoutine->>BinarySwap: replace_binary(new_binary, current_executable)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/update/mod.rs (1)
127-156: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExtracted temp directory is never cleaned up.
extract_binarycreatesagentflare-update-<pid>under the OS temp dir and unpacks the full release archive into it, but nothing removes it afterswap::replace_binarysucceeds (line 65 inrun). Every successful (and failed) update leaves the full extracted payload behind, growing unbounded over repeated updates.♻️ Proposed cleanup after a successful swap
if let Err(e) = swap::replace_binary(&new_binary, ¤t) { eprintln!("error replacing binary: {e}"); std::process::exit(1); } + if let Some(tmpdir) = new_binary.parent() { + let _ = std::fs::remove_dir_all(tmpdir); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/update/mod.rs` around lines 127 - 156, Ensure the temporary directory created by extract_binary is removed after the update completes, including after swap::replace_binary succeeds and when later update steps fail. Preserve the extracted binary’s use through replacement, then clean up the corresponding agentflare-update-<pid> directory in the run flow or via a guaranteed cleanup mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dev_install/cargo.rs`:
- Around line 28-44: The built_binary_path function must account for Cargo
target-specific output directories. Use Cargo metadata’s reported
target/executable path when available, or resolve the configured target triple
and insert it between target_directory and profile_dir(release), while
preserving the existing host-target path behavior.
In `@src/update/github.rs`:
- Around line 53-59: Update gh_get to configure an explicit per-request timeout
on the ureq request before call(), using the project’s established timeout value
if one exists. Keep the timeout applied to all callers, including
latest_version, download_asset, and expected_checksum, while preserving the
existing error mapping.
In `@src/update/swap.rs`:
- Around line 39-57: Update staging_path and its use in unix_replace so the
staged filename is unique per process, incorporating the current process ID
while retaining the target-directory location and filename-based staging
behavior. Preserve same-filesystem atomic rename and cleanup-on-error semantics,
and match the existing process-scoped naming convention used by other temporary
artifacts.
- Around line 59-86: Update the copy-error branch in windows_replace so the
rollback rename result is checked instead of discarded. If restoring old to
target fails, return an error that distinctly reports both the copy failure and
rollback failure, including the old path so the stranded binary can be
recovered; preserve the existing copy-error response when rollback succeeds.
---
Nitpick comments:
In `@src/update/mod.rs`:
- Around line 127-156: Ensure the temporary directory created by extract_binary
is removed after the update completes, including after swap::replace_binary
succeeds and when later update steps fail. Preserve the extracted binary’s use
through replacement, then clean up the corresponding agentflare-update-<pid>
directory in the run flow or via a guaranteed cleanup mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 416b3eb2-20aa-49dc-a508-0c18191a907a
📒 Files selected for processing (9)
src/cli/dev_install.rssrc/cli/mod.rssrc/dev_install/cargo.rssrc/dev_install/mod.rssrc/main.rssrc/update.rssrc/update/github.rssrc/update/mod.rssrc/update/swap.rs
💤 Files with no reviewable changes (1)
- src/update.rs
…ofile> CodeRabbit (PR #206): built_binary_path assumed target_directory/<profile>/, which is wrong when build.target / CARGO_BUILD_TARGET adds a <triple>/ segment (or a custom profile renames the dir) -- the lookup would miss the binary. Read the executable path from cargo's compiler-artifact JSON message instead of reconstructing it, so any target/profile layout is handled. build_and_locate() replaces the target-dir/profile-dir reconstruction and its parser.
… net timeout PR #206 CodeRabbit findings on the #122 swap/github code: - swap.rs: surface Windows rollback failure (and the recoverable old-binary path) instead of swallowing it, honoring the "never leave without a binary" invariant. - swap.rs: pid-scope the Unix staging filename so concurrent swaps against the same target cannot clobber each other's staged file. - github.rs: give the release HTTP client connect + per-read timeouts so a hung network cannot block an update indefinitely (no overall timeout, to avoid capping large asset downloads).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dev_install/cargo.rs`:
- Around line 38-44: Update the child-process handling around the stdout read
and `child.wait()` flow so a `read_to_string` failure terminates Cargo and waits
for it before returning the error; alternatively, use `wait_with_output()` to
manage collection and reaping together while preserving the existing error
message.
- Around line 16-28: Update the Cargo command construction in verify_runs to
handle configured non-host targets: either force the build target to the current
host or validate build.target/CARGO_BUILD_TARGET and return a clear error before
building. Preserve the existing release and binary arguments while ensuring
dev-install never attempts to execute a binary built for a different target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e38ed2c-cf6f-4e9e-9d2d-e7dc49bfca61
📒 Files selected for processing (4)
src/dev_install/cargo.rssrc/dev_install/mod.rssrc/update/github.rssrc/update/swap.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/dev_install/mod.rs
- src/update/swap.rs
- src/update/github.rs
…ead error CodeRabbit (PR #206) on build_and_locate: - Reject a non-host CARGO_BUILD_TARGET early with a clear message: dev-install replaces the running binary, so a cross target would build something that cannot run here. verify_runs() remains the backstop for a .cargo/config build.target. - Always child.wait() even when reading cargo stdout fails, so a failed read never orphans the cargo process.
This was referenced Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bundles two stacked, dependency-linked changes (one CI run).
#122 — MCP-safe self-upgrade primitive
Refactors single-file
src/update.rsinto a module dirsrc/update/{mod,github,swap}.rs.swap::replace_binaryis the reusable MCP-safe binary-replacement primitive: it only touches the file on disk and never signals/kills any process, so a liveagentflare mcpstdio server keeps running its already-loaded image and picks up the new binary on next launch..batfallback when the exe is hard-locked against renaming.find_killable_pids()reports other running instances (never kills them).github.rsisolates release discovery/download/checksum, with pure unit-testedparse_checksum/verify_checksum.#127 —
agentflare dev-installBuild the current source tree and atomically install it over the running binary, reusing
update::swap::replace_binary.cargo build(release by default;--debugfor debug)cargo metadata(honorsCARGO_TARGET_DIR/ shared workspace targets)--versionunder a timeout before swapping--dry-runreports without swappingVerification
cargo fmt --check: cleancargo clippy -p agentflare --no-deps -D warnings(allowing the crate''s pre-existing intentionalunsafe_codewarnings): cleancargo test -p agentflare update:: dev_install::: 14 passingagentflare dev-install --dry-runbuilds the workspace (release), resolves the artifact through a sharedCARGO_TARGET_DIR, verifies--version, and reports the swap — exit 0Summary by CodeRabbit
dev-installto build and install the current version, with debug and dry-run options.dev-installvalidates the built binary (--versionwith a timeout) and prevents installing over the freshly built output itself.