Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/cli/dev_install.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use clap::Args;

/// Build the current source tree and install it over the running binary.
///
/// Intended to be run from your *installed* `agentflare` inside a checkout:
/// it builds the checkout, verifies the binary, then swaps it into place using
/// the same MCP-safe replacement as `agentflare update`.
#[derive(Args)]
pub struct DevInstallArgs {
/// Build in debug mode instead of the default `--release`.
#[arg(long)]
pub debug: bool,
/// Build and verify, but report what would be installed without replacing.
#[arg(long)]
pub dry_run: bool,
}

impl DevInstallArgs {
pub fn run(self) {
crate::dev_install::run(!self.debug, self.dry_run);
}
}
3 changes: 3 additions & 0 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod channel;
mod claim;
mod coaching;
mod cost;
mod dev_install;
mod gateway;
mod handoff;
mod hook;
Expand Down Expand Up @@ -44,6 +45,7 @@ pub enum Commands {
Init(init::InitArgs),
Hook(hook::HookArgs),
Cost(cost::CostArgs),
DevInstall(dev_install::DevInstallArgs),
Coaching(coaching::CoachingArgs),
Gateway(gateway::GatewayArgs),
Mcp(mcp::McpArgs),
Expand Down Expand Up @@ -71,6 +73,7 @@ impl Commands {
Self::Init(cmd) => cmd.run(),
Self::Hook(cmd) => cmd.run(),
Self::Cost(cmd) => cmd.run(),
Self::DevInstall(cmd) => cmd.run(),
Self::Coaching(cmd) => cmd.run(),
Self::Gateway(cmd) => cmd.run(),
Self::Mcp(cmd) => cmd.run(),
Expand Down
132 changes: 132 additions & 0 deletions src/dev_install/cargo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
//! `cargo build` + built-artifact discovery for `dev-install`.

use std::io::Read;
use std::path::PathBuf;
use std::process::{Command, Stdio};

/// Build the `agentflare` binary from the current source tree and return the
/// path cargo actually wrote it to.
///
/// Reads the executable path from cargo's `compiler-artifact` JSON message
/// rather than reconstructing `target_directory/<profile>/agentflare`: that
/// reconstruction is wrong whenever `build.target` / `CARGO_BUILD_TARGET` adds a
/// `<triple>/` segment, or a custom profile changes the directory name. Human
/// progress and diagnostics still stream to stderr.
pub(crate) fn build_and_locate(release: bool) -> Result<PathBuf, String> {
// dev-install replaces the *running* binary, so the build must target this
// host. A configured cross target would produce a binary that can't run here
// (failing verification after a wasted build); reject it early with a clear
// message. A `.cargo/config` `build.target` is not caught here, but
// verify_runs() is the backstop that refuses to install a non-runnable binary.
if let Ok(t) = std::env::var("CARGO_BUILD_TARGET")
&& !t.is_empty()
&& t != crate::build_time::TARGET
{
return Err(format!(
"CARGO_BUILD_TARGET is `{t}`, but dev-install must build for the host target \
`{}` so the result can replace the running binary; unset CARGO_BUILD_TARGET",
crate::build_time::TARGET
));
}

let mut cmd = Command::new("cargo");
cmd.args([
"build",
"-p",
"agentflare",
"--bin",
"agentflare",
"--message-format",
"json-render-diagnostics",
]);
if release {
cmd.arg("--release");
}

let mut child = cmd
.stdout(Stdio::piped())
.stderr(Stdio::inherit())
.spawn()
.map_err(|e| format!("failed to run cargo build: {e}"))?;

// stderr is inherited (live progress); stdout is the JSON stream we parse.
// Only stdout is a pipe, so draining it fully cannot deadlock.
let mut json = String::new();
let read_result = child
.stdout
.take()
.expect("stdout was piped")
.read_to_string(&mut json);

// Always reap the child, even if reading its stdout failed, so cargo is
// never left running as an orphan.
let status = child.wait().map_err(|e| format!("waiting on cargo: {e}"))?;
read_result.map_err(|e| format!("reading cargo output: {e}"))?;
if !status.success() {
return Err("cargo build failed".to_string());
}

parse_executable_path(&json)
.ok_or_else(|| "cargo build did not report an agentflare executable".to_string())
}

/// Find the `agentflare` binary path in cargo's JSON build output. Pure, so it
/// is unit-testable without invoking cargo. Returns the last matching
/// `compiler-artifact` executable (there is normally exactly one).
fn parse_executable_path(build_json: &str) -> Option<PathBuf> {
let mut found = None;
for line in build_json.lines() {
let Ok(v) = serde_json::from_str::<serde_json::Value>(line) else {
continue;
};
if v.get("reason").and_then(serde_json::Value::as_str) != Some("compiler-artifact") {
continue;
}
let name = v
.get("target")
.and_then(|t| t.get("name"))
.and_then(serde_json::Value::as_str);
if name != Some("agentflare") {
continue;
}
if let Some(exe) = v.get("executable").and_then(serde_json::Value::as_str) {
found = Some(PathBuf::from(exe));
}
}
found
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn parse_executable_path_reads_the_agentflare_artifact_under_a_target_triple() {
// The path carries a `<triple>/` segment (build.target set) — exactly the
// case the reconstructed `target/<profile>/` lookup got wrong.
let json = concat!(
r#"{"reason":"compiler-artifact","target":{"name":"serde"},"executable":null}"#,
"\n",
r#"{"reason":"compiler-artifact","target":{"name":"agentflare"},"executable":"/repo/target/x86_64-unknown-linux-gnu/release/agentflare"}"#,
"\n",
r#"{"reason":"build-finished","success":true}"#,
"\n",
);
assert_eq!(
parse_executable_path(json),
Some(PathBuf::from(
"/repo/target/x86_64-unknown-linux-gnu/release/agentflare"
))
);
}

#[test]
fn parse_executable_path_none_when_no_agentflare_executable() {
let json = concat!(
r#"{"reason":"compiler-artifact","target":{"name":"agentflare"},"executable":null}"#,
"\n",
"not json\n",
);
assert_eq!(parse_executable_path(json), None);
}
}
149 changes: 149 additions & 0 deletions src/dev_install/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
//! `agentflare dev-install` — build the current source tree and atomically
//! install it over the running binary.
//!
//! Reuses the MCP-safe swap from [`crate::update::swap`] (item #122): the swap
//! never kills any process, so running `dev-install` from your installed
//! `agentflare` while an `agentflare mcp` server is live does not break the
//! server — it picks up the new binary on next launch.

mod cargo;

use std::path::Path;
use std::process::Command;
use std::time::{Duration, Instant};

/// How long to wait for the freshly built binary to answer `--version` before
/// declaring the build broken. `--version` returns immediately; this only
/// guards a pathological hang.
const VERIFY_TIMEOUT: Duration = Duration::from_secs(15);

/// Build (release unless `!release`), verify, and replace the running binary.
pub fn run(release: bool, dry_run: bool) {
println!(
"building agentflare ({})...",
if release { "release" } else { "debug" }
);
let built = match cargo::build_and_locate(release) {
Ok(p) if p.exists() => p,
Ok(p) => {
eprintln!(
"error: cargo reported {} but it does not exist",
p.display()
);
std::process::exit(1);
}
Err(e) => {
eprintln!("error: {e}");
std::process::exit(1);
}
};

// Verify the fresh build runs *before* replacing anything, so a broken
// build never overwrites a working install.
if let Err(e) = verify_runs(&built) {
eprintln!("error: built binary failed verification: {e}");
std::process::exit(1);
}

let target = match std::env::current_exe() {
Ok(p) => p,
Err(e) => {
eprintln!("error: cannot determine current binary path: {e}");
std::process::exit(1);
}
};

if same_file(&built, &target) {
eprintln!(
"refusing to install over the build output itself ({}).\n\
Run `dev-install` from your *installed* agentflare, not the freshly built binary.",
target.display()
);
std::process::exit(1);
}

if dry_run {
println!(
"dry-run: would install {} -> {}",
built.display(),
target.display()
);
return;
}

println!("installing {} -> {}", built.display(), target.display());
if let Err(e) = crate::update::swap::replace_binary(&built, &target) {
eprintln!("error installing binary: {e}");
std::process::exit(1);
}
println!("installed to {}", target.display());
println!("run `agentflare --version` to confirm");
}

/// Run `<binary> --version` and confirm it exits successfully within
/// [`VERIFY_TIMEOUT`].
fn verify_runs(binary: &Path) -> Result<(), String> {
let mut child = Command::new(binary)
.arg("--version")
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.spawn()
.map_err(|e| format!("failed to spawn --version: {e}"))?;

let deadline = Instant::now() + VERIFY_TIMEOUT;
loop {
match child.try_wait() {
Ok(Some(status)) if status.success() => return Ok(()),
Ok(Some(status)) => return Err(format!("--version exited with {status}")),
Ok(None) => {
if Instant::now() >= deadline {
let _ = child.kill();
return Err("--version timed out".to_string());
}
Comment on lines +99 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Reap the verification child after timing out.

kill() is followed by an immediate return, leaving the child unreaped. Wait after killing it before returning the timeout error.

Proposed fix
                 if Instant::now() >= deadline {
                     let _ = child.kill();
+                    let _ = child.wait();
                     return Err("--version timed out".to_string());
                 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if Instant::now() >= deadline {
let _ = child.kill();
return Err("--version timed out".to_string());
}
if Instant::now() >= deadline {
let _ = child.kill();
let _ = child.wait();
return Err("--version timed out".to_string());
}
🤖 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/dev_install/mod.rs` around lines 99 - 102, Update the timeout branch in
the verification child flow to wait for the process after calling child.kill()
and before returning the "--version timed out" error, ensuring the killed child
is reaped while preserving the existing timeout result.

std::thread::sleep(Duration::from_millis(50));
}
Err(e) => return Err(format!("waiting on --version: {e}")),
}
}
}

/// Whether two paths resolve to the same file. Canonicalizes both (following
/// symlinks); falls back to a raw comparison when a path can't be canonicalized
/// (e.g. the target doesn't exist yet).
fn same_file(a: &Path, b: &Path) -> bool {
match (std::fs::canonicalize(a), std::fs::canonicalize(b)) {
(Ok(ca), Ok(cb)) => ca == cb,
_ => a == b,
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn same_file_true_for_identical_path_false_for_distinct() {
let dir =
std::env::temp_dir().join(format!("agentflare-devinstall-same-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let f = dir.join("bin");
std::fs::write(&f, b"x").unwrap();
let other = dir.join("other");
std::fs::write(&other, b"y").unwrap();

assert!(same_file(&f, &f));
assert!(!same_file(&f, &other));

let _ = std::fs::remove_dir_all(&dir);
}

#[test]
fn verify_runs_errors_for_a_missing_binary() {
// The happy path is exercised by the real `dev-install` flow against a
// freshly built binary; here we pin down the guard that a non-runnable
// path is reported as an error rather than panicking.
let missing = std::env::temp_dir().join("agentflare-nonexistent-binary-xyz");
assert!(verify_runs(&missing).is_err());
}
}
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod compact;
mod components;
mod cost;
mod db;
mod dev_install;
mod dev_vars;
mod errors;
mod gateway_integrations;
Expand Down
Loading
Loading