diff --git a/agentctl/CHANGELOG.md b/agentctl/CHANGELOG.md index db1b530f..d0e41e99 100644 --- a/agentctl/CHANGELOG.md +++ b/agentctl/CHANGELOG.md @@ -14,7 +14,7 @@ agents. The Rust workspace version is a build detail and is not tracked here. ### Added -- `agentctl setup` takes you from nothing to a running agent: from a standard agent agentctl carries, or from your team's agent on disk or on GitHub. It fills in recommended answers and shows them before it creates anything, and asks each question only if you choose to change something: a name, a size that fits your machine, which harnesses the agent has and the default one, the folders on your machine to share with the agent, a GitHub token and any other values the manifest declares. It signs you in to Claude Code, Codex and GitHub in a short-lived sandbox, so this machine needs none of them, or uses a harness login agentctl holds already, or the GitHub account another agent uses, if you say so, writes `~/agents//`, creates the agent, sets up SSH and opens it in VS Code or Zed. Run again, it offers the agents in `~/agents` first, to connect, sign in or replace; replacing asks first, and removes the old folder and agent before the new one takes its place. When a standard agent's image cannot be pulled, such as before it is published, setup offers to build it on your machine from the Dockerfile agentctl carries, and `--build-image` always does. You choose by number or with the arrow keys and go back with Esc. Every question has a flag for scripts, setup ends by printing the command that repeats it without questions, and the installers start it after a first install unless `AGENT_SETUP=0`. +- `agentctl setup` takes you from nothing to a running agent: from a standard agent agentctl carries, or from your team's agent on disk or on GitHub. It fills in recommended answers and shows them before it creates anything, and asks each question only if you choose to change something: a name, a size that fits your machine, which harnesses the agent has and the default one, the folders on your machine to share with the agent, a GitHub token and any other values the manifest declares. It signs you in to Claude Code, Codex and GitHub in a short-lived sandbox, so this machine needs none of them, or uses a harness login agentctl holds already, or the GitHub account another agent uses, if you say so, writes `~/agents//`, creates the agent, sets up SSH and opens it in VS Code or Zed. Run again, it offers the agents in `~/agents` first, to connect, sign in, update from where they came from, keeping their disk and sessions where the change allows, or replace; replacing asks first, and removes the old folder and agent before the new one takes its place. When a standard agent's image cannot be pulled, such as before it is published, setup offers to build it on your machine from the Dockerfile agentctl carries, and `--build-image` always does. You choose by number or with the arrow keys and go back with Esc. Every question has a flag for scripts, setup ends by printing the command that repeats it without questions, and the installers start it after a first install unless `AGENT_SETUP=0`. - `optional: true` in `spec.instructions` and `spec.skills` leaves out a source that does not exist, so a committed manifest can name your own files, such as `~/agent-instructions/everywhere.md`. `agentctl describe` names what was left out, and the agent gets it once it exists. ([#331](https://github.com/digdir/digdir-agents/issues/331)) - Your terminal tab is named after the agent and session you are attached to, such as `builder/b1`, and reads `agentctl` again when you return to `agentctl tui`. ([#343](https://github.com/digdir/digdir-agents/pull/343)) - `agentctl get agents --watch -o json` prints the agents as one JSON line, then again each time they change, so a tool can follow them without polling. It exits once an `agentd` upgrade makes it incompatible. `get sessions --watch` works the same way. ([#349](https://github.com/digdir/digdir-agents/issues/349)) diff --git a/agentctl/README.md b/agentctl/README.md index 90c73988..c3f76b62 100644 --- a/agentctl/README.md +++ b/agentctl/README.md @@ -59,8 +59,15 @@ it later. setup offers the account another agent in `~/agents` uses (`--github existing`), after checking its token still works. - **Agents you set up before** are offered first when you run setup again: connect a running one to an editor, sign - in to its harnesses, or replace it with a new definition, which setup asks about first. For a stopped agent or one - that is not ready, setup names the `agentctl` command that helps. + in to its harnesses, update it, or replace it with a new definition, which setup asks about first. For a stopped + agent or one that is not ready, setup names the `agentctl` command that helps. +- **Updating** fetches an agent's definition again from where setup got it, such as after its repository improved it, + and applies it with the agent's disk, sessions and `.env` kept (`--existing --action update`). Your answers + for its name, size, harnesses and shared folders carry over, so what you left out stays out while what the source + adds comes in. The review lists the files that change first, and anything else you edited in its `agent.yaml` gives + way to the source. An existing agent cannot change some fields, such as its image or the list of skills it reads, so + an update that changes one creates the agent again, deleting its disk. Setup asks first, and a script passes + `--replace`. - **Editors:** setup opens the agent in VS Code or Zed. Other apps that work over SSH, such as JetBrains Gateway and the Claude desktop app's Code tab, connect to the SSH host `agentctl-` that setup names, in the folder it names. The Claude desktop app copies its own Claude sign-in into the agent, while sessions agentctl starts never let diff --git a/agentctl/src/bin/agentctl/setup/create.rs b/agentctl/src/bin/agentctl/setup/create.rs index 9a415ffd..d6e1e814 100644 --- a/agentctl/src/bin/agentctl/setup/create.rs +++ b/agentctl/src/bin/agentctl/setup/create.rs @@ -5,7 +5,8 @@ use super::answers::{Answers, Choice}; use super::sign_in::login_command; use super::sizing::Whole; use super::{ - BrowserSignIn, Draft, SetupArguments, bundle, command_line, env_file, existing, github, ide, image, repeat, staging, + BrowserSignIn, Draft, SetupArguments, bundle, command_line, env_file, existing, github, ide, image, repeat, + staging, update, }; use crate::{CommandError, CommandResult}; use agent::local::home::ControlPlaneHome; @@ -251,19 +252,33 @@ pub(super) fn write_folder(draft: &mut Draft, folder: &Path) -> CommandResult(path) - })(); - if written.is_err() { - std::fs::remove_dir_all(folder).ok(); + match write_contents(draft, folder) { + Ok(agent) => { + draft.agent = agent; + Ok(folder.join(manifest::MANIFEST_FILE)) + } + Err(error) => { + std::fs::remove_dir_all(folder).ok(); + Err(error.into()) + } + } +} + +/// Writes into `folder` what makes up the agent: its manifest, the files it reads, its `.env` and +/// where it came from. Returns the manifest written, whose paths point at the copies. +pub(super) fn write_contents(draft: &Draft, folder: &Path) -> Result { + let mut agent = draft.agent.clone(); + bundle_into(&mut agent, draft, folder)?; + std::fs::write(folder.join(manifest::MANIFEST_FILE), manifest_text(&agent, draft)?)?; + env_file::write(folder, &draft.access.values)?; + if let Some(recorded) = &draft.source.recorded { + let record = update::Record { + source: recorded.clone(), + left_out: draft.left_out.clone(), + }; + update::record(folder, &record)?; } - Ok(written?) + Ok(agent) } /// Copies what `agent` reads into `folder`, and the sources of its image when it is built here. diff --git a/agentctl/src/bin/agentctl/setup/existing.rs b/agentctl/src/bin/agentctl/setup/existing.rs index 8322c14d..19d5f32e 100644 --- a/agentctl/src/bin/agentctl/setup/existing.rs +++ b/agentctl/src/bin/agentctl/setup/existing.rs @@ -1,5 +1,5 @@ //! The agents setup made before, in `~/agents//`: finding them with their state, carrying -//! on with one, and replacing one with a new definition. +//! on with one, updating one from where it came from, and replacing one with a new definition. //! //! Replacing is done so that a failure or a change of mind leaves the old definition as it was: //! the new folder is written in full first, and only then is the old agent deleted and its folder @@ -33,6 +33,8 @@ pub(super) struct Existing { pub(super) name: String, /// What setup made it from, when setup made it. pub(super) origin: Option, + /// Where setup can fetch it again, when setup recorded that. + pub(super) recorded: Option, pub(super) state: State, } @@ -114,6 +116,7 @@ pub(super) async fn scan(client: Option<&Client>, root: &Path, agents: &[Agent]) } }; found.push(Existing { + recorded: super::update::recorded(&folder), folder, name, origin, @@ -152,6 +155,8 @@ pub(crate) enum ExistingAction { Connect, /// Sign in to its harnesses. SignIn, + /// Update it from the definition it came from, keeping its disk and sessions. + Update, /// Replace it with a new agent definition. Replace, } @@ -168,6 +173,7 @@ pub(super) enum Proceed { pub(super) struct Context<'a> { pub(super) arguments: &'a SetupArguments, pub(super) answers: &'a Answers, + pub(super) catalog: &'a agent::catalog::Catalog, pub(super) client: &'a Client, pub(super) home: &'a agent::local::home::ControlPlaneHome, pub(super) staging: &'a super::staging::Staging, @@ -204,7 +210,7 @@ pub(super) async fn proceed(context: &Context<'_>, existing: Existing) -> Comman } _ => false, }; - let offered = actions(&existing.state, sign_in); + let offered = actions(&existing.state, sign_in, existing.recorded.is_some()); let action = match context.arguments.action { Some(action) if offered.iter().any(|(offered, _)| *offered == Some(action)) => action, Some(action) => { @@ -232,7 +238,8 @@ pub(super) async fn proceed(context: &Context<'_>, existing: Existing) -> Comman action } }; - if action != ExistingAction::Replace { + // Updating and replacing ask more before they do anything. + if !matches!(action, ExistingAction::Update | ExistingAction::Replace) { context.answers.no_way_back(); } let name = existing.name.as_str(); @@ -259,6 +266,7 @@ pub(super) async fn proceed(context: &Context<'_>, existing: Existing) -> Comman .await?; println!("Signed in; agent/{name} picks the logins up as it converges."); } + ExistingAction::Update => Box::pin(super::update::run(context, &existing)).await?, ExistingAction::Replace => return Ok(Proceed::Replace(existing)), } Ok(Proceed::Done) @@ -285,8 +293,9 @@ fn remedy(existing: &Existing, cli: &str) -> Option { } /// What setup offers for an agent in `state`: connecting a running one, signing in when -/// `sign_in` allows it, replacing it, and leaving it as it is. -fn actions(state: &State, sign_in: bool) -> Vec<(Option, &'static str)> { +/// `sign_in` allows it, updating it when it was `recorded` where it came from, replacing it, and +/// leaving it as it is. +fn actions(state: &State, sign_in: bool, recorded: bool) -> Vec<(Option, &'static str)> { let mut actions = Vec::new(); if *state == State::Running { actions.push((Some(ExistingAction::Connect), "Connect it to an editor on this machine")); @@ -294,6 +303,18 @@ fn actions(state: &State, sign_in: bool) -> Vec<(Option, &'stati if sign_in { actions.push((Some(ExistingAction::SignIn), "Sign in to its harnesses")); } + // An update keeps what setup's questions decided, so it reads them from a manifest that + // resolves, and applies it to the agent of this folder. + let updatable = matches!( + state, + State::Running | State::Stopped | State::NotReady | State::NotCreated + ); + if recorded && updatable { + actions.push(( + Some(ExistingAction::Update), + "Update it from where it came from, keeping its disk and sessions", + )); + } actions.push((Some(ExistingAction::Replace), "Replace it with a new agent definition")); actions.push((None, "Nothing, for now")); actions @@ -335,15 +356,20 @@ pub(super) async fn swap( } wait_until_deleted(client, name, timeout).await?; } - std::fs::rename(&replaced.folder, backup).map_err(|error| { - CommandError::Message(format!("could not move {} aside: {error}", replaced.folder.display())) - })?; - if let Err(error) = move_folder(staged, &replaced.folder) { - std::fs::remove_dir_all(&replaced.folder).ok(); - std::fs::rename(backup, &replaced.folder).ok(); + exchange(&replaced.folder, staged, backup) +} + +/// Moves `folder` aside to `backup` and the folder written at `staged` into its place. Should the +/// move fail, the old folder is put back. +pub(super) fn exchange(folder: &Path, staged: &Path, backup: &Path) -> CommandResult<()> { + std::fs::rename(folder, backup) + .map_err(|error| CommandError::Message(format!("could not move {} aside: {error}", folder.display())))?; + if let Err(error) = move_folder(staged, folder) { + std::fs::remove_dir_all(folder).ok(); + std::fs::rename(backup, folder).ok(); return Err(CommandError::Message(format!( "could not put the new definition in {}: {error}; the old one is back", - replaced.folder.display() + folder.display() ))); } Ok(()) @@ -397,22 +423,33 @@ mod tests { use super::{ExistingAction, State, actions, move_folder, scan}; #[test] - fn a_running_agent_can_be_connected_and_any_replaced() { - let offered = |state: &State, sign_in| -> Vec<_> { - actions(state, sign_in).into_iter().map(|(action, _)| action).collect() + fn a_running_agent_can_be_connected_one_with_a_known_source_updated_and_any_replaced() { + let offered = |state: &State, sign_in, recorded| -> Vec<_> { + actions(state, sign_in, recorded) + .into_iter() + .map(|(action, _)| action) + .collect() }; assert_eq!( - offered(&State::Running, true), + offered(&State::Running, true, true), [ Some(ExistingAction::Connect), Some(ExistingAction::SignIn), + Some(ExistingAction::Update), Some(ExistingAction::Replace), None ] ); - assert_eq!(offered(&State::Stopped, false), [Some(ExistingAction::Replace), None]); assert_eq!( - offered(&State::Invalid("bad".into()), false), + offered(&State::Stopped, false, false), + [Some(ExistingAction::Replace), None] + ); + assert_eq!( + offered(&State::NotCreated, false, true), + [Some(ExistingAction::Update), Some(ExistingAction::Replace), None] + ); + assert_eq!( + offered(&State::Invalid("bad".into()), false, true), [Some(ExistingAction::Replace), None] ); } diff --git a/agentctl/src/bin/agentctl/setup/mod.rs b/agentctl/src/bin/agentctl/setup/mod.rs index 53068bbf..88814318 100644 --- a/agentctl/src/bin/agentctl/setup/mod.rs +++ b/agentctl/src/bin/agentctl/setup/mod.rs @@ -30,6 +30,7 @@ pub(crate) mod sign_in; mod sizing; mod source; mod staging; +mod update; mod values; use std::{ @@ -221,6 +222,8 @@ struct Draft { access: Access, /// The published image the agent's image is built here in place of. built_here: Option, + /// What its source defines that setup left out, so an update leaves it out again. + left_out: update::LeftOut, } impl Draft { @@ -233,6 +236,7 @@ impl Draft { replace: None, access: Access::default(), built_here: None, + left_out: update::LeftOut::default(), } } } @@ -243,6 +247,8 @@ struct Source { directory: PathBuf, /// What the agent came from, named in the written manifest. origin: String, + /// Where to fetch it again to update the agent from it. + recorded: Option, /// The repository the manifest is in, whose checkout its relative bind mounts mount. repository: Option, /// Whether it is a standard agent, which a mounted folder's own agents may replace. @@ -332,6 +338,7 @@ async fn run_once( let context = existing::Context { arguments, answers, + catalog, client, home, staging: &staging, diff --git a/agentctl/src/bin/agentctl/setup/mounts.rs b/agentctl/src/bin/agentctl/setup/mounts.rs index 1960ed36..7a556c1f 100644 --- a/agentctl/src/bin/agentctl/setup/mounts.rs +++ b/agentctl/src/bin/agentctl/setup/mounts.rs @@ -382,8 +382,8 @@ fn listed( } /// Applies `shares` to `agent`: its repository's mounts share the checkout chosen, or are left out -/// without one, and the mounts left out are removed. -fn apply(agent: &mut Agent, repository: Option<&RepositoryShares>, shares: &Shares) { +/// without one, and the mounts left out are removed. Returns where the agent would have seen those. +fn apply(agent: &mut Agent, repository: Option<&RepositoryShares>, shares: &Shares) -> Vec { let mut removed = shares.left_out.clone(); for mount in repository .map(|repository| repository.found.as_slice()) @@ -400,9 +400,15 @@ fn apply(agent: &mut Agent, repository: Option<&RepositoryShares>, shares: &Shar } removed.sort_unstable(); removed.dedup(); - for index in removed.into_iter().rev() { - agent.spec.sandbox.mounts.remove(index); - } + let mut targets: Vec = removed + .into_iter() + .rev() + .map(|index| match agent.spec.sandbox.mounts.remove(index) { + MountSpec::Bind { target, .. } | MountSpec::Tmpfs { target, .. } => target.as_str().to_owned(), + }) + .collect(); + targets.reverse(); + targets } /// The folder `within` of `checkout`. Joining an empty path would add a trailing separator. @@ -458,7 +464,7 @@ pub(super) fn choose_shared( { return Ok(Some((manifest, folder.clone()))); } - apply(&mut draft.agent, repository.as_ref(), &shares); + draft.left_out.shared = apply(&mut draft.agent, repository.as_ref(), &shares); if let Some(folder) = &added { mount_unless_mounted(draft, folder)?; } diff --git a/agentctl/src/bin/agentctl/setup/shape.rs b/agentctl/src/bin/agentctl/setup/shape.rs index 9036fb90..c756ec4e 100644 --- a/agentctl/src/bin/agentctl/setup/shape.rs +++ b/agentctl/src/bin/agentctl/setup/shape.rs @@ -202,6 +202,11 @@ pub(super) fn choose_harnesses(arguments: &SetupArguments, answers: &Answers, dr "--harness {harness} must be one of the --harnesses the agent has" ))); } + draft.left_out.harnesses = declared + .iter() + .copied() + .filter(|harness| !kept.contains(harness)) + .collect(); let harnesses = &mut draft.agent.spec.harnesses; harnesses.retain(|harness| kept.contains(&harness.kind)); let current = harnesses.iter().position(|harness| harness.default).unwrap_or_default(); diff --git a/agentctl/src/bin/agentctl/setup/source.rs b/agentctl/src/bin/agentctl/setup/source.rs index 770d796a..ae9863c7 100644 --- a/agentctl/src/bin/agentctl/setup/source.rs +++ b/agentctl/src/bin/agentctl/setup/source.rs @@ -4,6 +4,7 @@ use super::answers::{Answers, Choice}; use super::sizing::Whole; +use super::update::Recorded; use super::{Draft, SetupArguments, Source, SourceChoice, fetch, image, mounts}; use crate::{CommandError, CommandResult}; use agent::catalog::Catalog; @@ -19,7 +20,7 @@ pub(super) async fn choose_draft( source: SourceChoice, ) -> CommandResult { match source { - SourceChoice::Standard => standard_draft(arguments, answers, catalog, staging), + SourceChoice::Standard => standard_draft(arguments.archetype.as_deref(), answers, catalog, staging), SourceChoice::Team => team_source(arguments, answers, staging).await, SourceChoice::Existing => Err(CommandError::Message( "an existing agent is carried on with, not set up again".into(), @@ -41,7 +42,7 @@ pub(super) async fn team_source(arguments: &SetupArguments, answers: &Answers, s check_team_source, )?, }; - match team_draft_from(arguments, answers, staging, &from).await { + match team_draft_from(arguments.variant.as_ref(), answers, staging, &from).await { Err(error) if arguments.from.is_none() && answers.interactive() && !answers.went_back() => { // What went wrong with the address, without the prefix a manifest's own errors carry. match error { @@ -82,18 +83,19 @@ pub(super) fn check_team_source(answer: &str) -> Result<(), String> { } /// The team's agent at `from`, a GitHub address or a path, its manifest chosen when it holds -/// several. +/// several, or the one of `variant`. pub(super) async fn team_draft_from( - arguments: &SetupArguments, + variant: Option<&agent::AgentVariantName>, answers: &Answers, staging: &Path, from: &str, ) -> CommandResult { if let Some(mut github) = fetch::GithubSource::parse(from) { let folder = github.fetch(staging).await?; - let manifest = choose_manifest(answers, arguments.variant.as_ref(), &folder)?; + let manifest = choose_manifest(answers, variant, &folder)?; let within = manifest.strip_prefix(staging.join("checkout")).unwrap_or(&manifest); - record_team_source(answers, &github.address_of(within), &manifest); + let address = github.address_of(within); + record_team_source(answers, &address, &manifest); let origin = format!( "github.com/{}/{}, {}", github.owner, @@ -105,25 +107,26 @@ pub(super) async fn team_draft_from( &staging.join("checkout"), &github.repository, )?); + draft.source.recorded = Some(Recorded::Team { + from: address, + variant: manifest::variant_from_filename(&manifest).map(|variant| variant.as_str().to_owned()), + }); return Ok(draft); } - let manifest = choose_manifest( - answers, - arguments.variant.as_ref(), - &manifest::expand_host_path(Path::new(from))?, - )?; + let manifest = choose_manifest(answers, variant, &manifest::expand_host_path(Path::new(from))?)?; answers.record("--from", &[&manifest.display().to_string()]); answers.forget("--variant"); team_draft(&manifest, &manifest.display().to_string()) } +/// The standard agent `id`, else the one the person picks from the catalog. pub(super) fn standard_draft( - arguments: &SetupArguments, + id: Option<&str>, answers: &Answers, catalog: &Catalog, staging: &Path, ) -> CommandResult { - let archetype = if let Some(id) = &arguments.archetype { + let archetype = if let Some(id) = id { catalog.get(id).ok_or_else(|| { Error::Invalid(format!( "no standard agent {id:?}; see `agentctl setup --list-archetypes`" @@ -152,6 +155,9 @@ pub(super) fn standard_draft( let directory = parent(&manifest_path)?; let source = Source { origin: format!("the standard agent {}", archetype.id), + recorded: Some(Recorded::Standard { + id: archetype.id.clone(), + }), repository: None, from_catalog: true, image_sources: Some(directory.clone()).filter(|directory| image::buildable(directory)), @@ -233,6 +239,13 @@ pub(super) fn team_draft(manifest_path: &Path, origin: &str) -> CommandResult/` from where setup made it from, such as a team's +//! repository or a folder of definitions where the agent improved its own. Replacing an agent +//! deletes it with its disk, so an update instead writes the folder again from the source, +//! keeps what setup's questions decided, and applies it as `agentctl apply` does. + +use std::{ + collections::BTreeSet, + path::{Path, PathBuf}, +}; + +use agent::{AccessSpec, Agent, Error, Harness, MountSpec, control_plane::ApplyRequest, manifest}; +use sandbox::{RootFilesystem, SandboxPath, image::ImageSource}; +use serde::{Deserialize, Serialize}; + +use super::answers::Choice; +use super::create::{OnFailure, create, write_contents}; +use super::existing::{Context, Existing, State, exchange, move_folder, swap}; +use super::mounts::{check_mounts, protected_folders, refuse_staged_mounts}; +use super::source::{standard_draft, team_draft_from}; +use super::{Draft, command_line, docker, edit, github, image, sign_in, values}; +use crate::{CommandError, CommandResult}; + +/// The file in an agent's folder that records where setup made it from. +const SOURCE_FILE: &str = ".setup-source.yaml"; + +/// Where an agent's definition came from, so setup can fetch it again. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(tag = "type", rename_all = "camelCase", deny_unknown_fields)] +pub(super) enum Recorded { + /// A standard agent, from the catalog of the agentctl that updates it. + Standard { id: String }, + /// A team's agent: a manifest on this machine, or the GitHub address of the folder holding it + /// with its variant. + Team { + from: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + variant: Option, + }, +} + +/// What setup left out of the agent its source defines, so an update leaves it out again while +/// taking what the source adds. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(super) struct LeftOut { + /// The harnesses the agent does not have. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub(super) harnesses: Vec, + /// The folders it does not share, by where the agent would see them. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub(super) shared: Vec, +} + +impl LeftOut { + const fn is_empty(&self) -> bool { + self.harnesses.is_empty() && self.shared.is_empty() + } +} + +/// What setup records in an agent's folder: where the agent came from and what it left out. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(super) struct Record { + pub(super) source: Recorded, + #[serde(default, skip_serializing_if = "LeftOut::is_empty")] + pub(super) left_out: LeftOut, +} + +/// Records in `folder` where its agent came from and what setup left out. +pub(super) fn record(folder: &Path, record: &Record) -> Result<(), Error> { + let yaml = serde_yaml_ng::to_string(record).map_err(Error::Yaml)?; + std::fs::write( + folder.join(SOURCE_FILE), + format!("# Where `agentctl setup` made this agent from, to update it from there.\n{yaml}"), + )?; + Ok(()) +} + +/// Where the agent in `folder` came from, and what setup left out, when setup recorded it. +pub(super) fn recorded(folder: &Path) -> Option { + let text = std::fs::read_to_string(folder.join(SOURCE_FILE)).ok()?; + serde_yaml_ng::from_str(&text).ok() +} + +/// Updates `existing` from where it came from, after showing what changes. +pub(super) async fn run(context: &Context<'_>, existing: &Existing) -> CommandResult<()> { + let Some(Record { + source: recorded, + left_out, + }) = &existing.recorded + else { + return Err(CommandError::Message(format!( + "setup did not record where {} came from; replace it instead", + existing.name + ))); + }; + let answers = context.answers; + let staging = context.staging.path(); + let current = manifest::resolve(&existing.folder.join(manifest::MANIFEST_FILE))?.agent; + let mut draft = match recorded { + Recorded::Standard { id } => standard_draft(Some(id), answers, context.catalog, staging)?, + Recorded::Team { from, variant } => { + let variant = variant + .as_deref() + .map(crate::parse_variant_name) + .transpose() + .map_err(CommandError::Message)?; + team_draft_from(variant.as_ref(), answers, staging, from).await? + } + }; + keep_answers(¤t, left_out, &existing.folder, &mut draft); + refuse_staged_mounts(&draft, staging)?; + let protected = protected_folders(existing.folder.parent().unwrap_or(&existing.folder), context.home); + check_mounts(&draft, &protected)?; + draft.access.known_values = + agent::environment::read_or_empty(&existing.folder.join(agent::control_plane::ENV_FILE)).await?; + values::collect(answers, None, &mut draft).await?; + + let staged = staging.join("update"); + let agent = loop { + if staged.exists() { + std::fs::remove_dir_all(&staged).map_err(Error::from)?; + } + std::fs::create_dir_all(&staged).map_err(Error::from)?; + let agent = write_contents(&draft, &staged)?; + let changes = changes(&existing.folder, &staged)?; + if changes.is_empty() { + println!("{} is up to date with {}.", existing.name, draft.source.origin); + return Ok(()); + } + let recreate = fixed_fields_changed(context, existing, &agent).await?; + match review(context, existing, &draft, &agent, &changes, &recreate)? { + Review::Update => break (agent, !recreate.is_empty()), + Review::Edit => { + edit::manifest(answers, &mut draft)?; + check_mounts(&draft, &protected)?; + } + Review::Stop => { + println!("Nothing was changed."); + return Ok(()); + } + } + }; + let (agent, recreate) = agent; + answers.no_way_back(); + if matches!(agent.spec.sandbox.image, ImageSource::Build { .. }) { + docker::ensure().await?; + } + if recreate { + return create_again(context, existing, &agent, &staged).await; + } + apply(context, existing, &agent, &staged).await +} + +/// The fields agentd cannot change on the agent of this folder that `agent` changes, so updating +/// to it takes creating the agent again. +async fn fixed_fields_changed( + context: &Context<'_>, + existing: &Existing, + agent: &Agent, +) -> CommandResult> { + let Some(name) = existing.agent_here() else { + return Ok(Vec::new()); + }; + let current = context.client.get(name).await?; + // agentd holds bind mount sources resolved, as it resolves those it is given. + let mut desired = agent.spec.clone(); + for mount in &mut desired.sandbox.mounts { + if let MountSpec::Bind { source, .. } = mount + && let Ok(resolved) = std::fs::canonicalize(&*source) + { + *source = resolved; + } + } + Ok(current.spec.immutable_changes(&desired)) +} + +/// Carries over to `draft`, made from the source again, what setup's questions decided for the +/// `current` agent in `folder`: its name, its size, its harnesses and the default one, its mounts, +/// GitHub access and an image built here. What setup `left_out` stays out, while what the source +/// adds since comes in. Whatever else was changed by hand in `folder` gives way to the source, which +/// the review shows. +fn keep_answers(current: &Agent, left_out: &LeftOut, folder: &Path, draft: &mut Draft) { + draft.agent.metadata.name.clone_from(¤t.metadata.name); + let spec = &mut draft.agent.spec; + let resources = &mut spec.sandbox.resources; + resources.cpu = current.spec.sandbox.resources.cpu; + resources.memory = current.spec.sandbox.resources.memory; + resources.root_filesystem = RootFilesystem::new( + current.spec.sandbox.resources.root_filesystem.capacity(), + resources.root_filesystem.mode(), + ); + + // A source that keeps only harnesses setup left out keeps them, as an agent needs one. + if spec + .harnesses + .iter() + .any(|harness| !left_out.harnesses.contains(&harness.kind)) + { + spec.harnesses + .retain(|harness| !left_out.harnesses.contains(&harness.kind)); + } + draft.left_out = left_out.clone(); + if let Some(default) = current.spec.default_harness().map(|harness| harness.kind) + && spec.harnesses.iter().any(|harness| harness.kind == default) + { + for harness in &mut spec.harnesses { + harness.default = harness.kind == default; + harness.optional &= harness.kind != default; + } + } + + // A mount of the current agent stands in for the source's at the same place, since setup made + // its source absolute or chose the checkout behind it; the folder setup mounted is kept too. + spec.sandbox + .mounts + .retain(|mount| !left_out.shared.iter().any(|shared| shared == target(mount).as_str())); + let mut kept = current.spec.sandbox.mounts.clone(); + for mount in &mut spec.sandbox.mounts { + if let Some(index) = kept.iter().position(|current| target(current) == target(mount)) { + *mount = kept.remove(index); + } + } + spec.sandbox.mounts.extend(kept); + + if current + .spec + .secrets + .iter() + .any(|secret| secret.environment == github::TOKEN_VARIABLE) + { + github::declare(spec); + } + if current + .spec + .environment + .iter() + .any(|variable| variable.name == github::GIT_USER_NAME) + { + github::declare_identity(spec); + } + if !spec.access.contains(&AccessSpec::Ssh {}) { + spec.access.push(AccessSpec::Ssh {}); + } + + let built_here = folder.join(image::SOURCES).is_dir(); + if let (true, Some(_), ImageSource::Reference { reference }) = + (built_here, &draft.source.image_sources, &spec.sandbox.image) + { + draft.built_here = Some(reference.clone()); + } +} + +const fn target(mount: &MountSpec) -> &SandboxPath { + match mount { + MountSpec::Bind { target, .. } | MountSpec::Tmpfs { target, .. } => target, + } +} + +/// The files that differ between the folder `before` and the one `after`, each marked as added +/// (`+`), removed (`-`) or changed (`~`). The `.env` is left out, since an update keeps its values. +fn changes(before: &Path, after: &Path) -> Result, Error> { + let old = files(before)?; + let new = files(after)?; + let mut changes = Vec::new(); + for file in old.union(&new) { + let change = match (old.contains(file), new.contains(file)) { + (false, _) => '+', + (_, false) => '-', + _ if std::fs::read(before.join(file))? != std::fs::read(after.join(file))? => '~', + _ => continue, + }; + let file: Vec<_> = file + .components() + .map(|part| part.as_os_str().to_string_lossy()) + .collect(); + changes.push(format!("{change} {}", file.join("/"))); + } + Ok(changes) +} + +/// The files below `root`, relative to it, but not its `.env`. +fn files(root: &Path) -> Result, Error> { + let mut found = BTreeSet::new(); + let mut folders = vec![root.to_path_buf()]; + while let Some(folder) = folders.pop() { + for entry in std::fs::read_dir(&folder)? { + let path = entry?.path(); + if path.is_dir() { + folders.push(path); + } else if let Ok(relative) = path.strip_prefix(root) + && relative != Path::new(agent::control_plane::ENV_FILE) + { + found.insert(relative.to_path_buf()); + } + } + } + Ok(found) +} + +enum Review { + Update, + /// Edit its manifest first, then review the changes again. + Edit, + Stop, +} + +fn review( + context: &Context<'_>, + existing: &Existing, + draft: &Draft, + agent: &Agent, + changes: &[String], + recreate: &[&str], +) -> CommandResult { + let mut lines = vec![format!( + "Update {} from {}:", + existing.folder.display(), + draft.source.origin + )]; + lines.extend(changes.iter().map(|change| format!(" {change}"))); + if changes.iter().any(|change| change.ends_with(manifest::MANIFEST_FILE)) { + lines.push( + "Changes made to agent.yaml by hand, other than its name, size, harnesses and shared folders, give \ + way to the source." + .to_owned(), + ); + } + let name = &agent.metadata.name; + let (question, yes, default) = if recreate.is_empty() { + let question = if existing.agent_here().is_some() { + format!("Update agent/{name}? It keeps its disk and sessions.") + } else { + format!("Update the folder and create agent/{name}?") + }; + (question, "Yes, update it", 0) + } else { + // Like replacing, this deletes the agent's disk, so neither `--yes` nor a script does it + // without `--replace`. + if !context.answers.interactive() && !context.arguments.replace { + return Err(CommandError::Message(format!( + "the update changes {}, which agent/{name} cannot change without being created again, its disk \ + and sessions deleted; pass --replace to do so", + recreate.join(", ") + ))); + } + let question = format!( + "An existing agent cannot change {}, so the update creates agent/{name} again, and its disk and \ + sessions are deleted. Do it?", + recreate.join(", ") + ); + let default = if context.arguments.replace { 0 } else { 2 }; + (question, "Yes, create it again, deleting its disk", default) + }; + lines.push(question); + Ok(context.answers.choose( + "--yes", + &lines.join("\n"), + vec![ + Choice::new(Review::Update, yes), + Choice::new(Review::Edit, "Edit its agent.yaml first, then decide"), + Choice::new(Review::Stop, "No, change nothing"), + ], + default, + )?) +} + +/// Deletes the agent of this folder, puts the folder written at `staged` in its place and creates +/// the agent from it, as replacing it does. +async fn create_again(context: &Context<'_>, existing: &Existing, agent: &Agent, staged: &Path) -> CommandResult<()> { + let backup = context.staging.path().join("updated"); + let timeout = context.arguments.timeout; + swap(context.client, existing, staged, &backup, timeout).await?; + let folder = &existing.folder; + let cli = command_line(context.home); + let name = create( + context.answers, + context.client, + &cli, + agent, + folder, + &folder.join(manifest::MANIFEST_FILE), + timeout, + OnFailure::Restore(backup), + ) + .await?; + println!("agent/{name} is created again from the update and ready."); + Ok(()) +} + +/// Puts the folder written at `staged` in place of the agent's own and applies it. Should applying +/// fail, the folder from before comes back. +async fn apply(context: &Context<'_>, existing: &Existing, agent: &Agent, staged: &Path) -> CommandResult<()> { + let folder = &existing.folder; + let backup = context.staging.path().join("updated"); + exchange(folder, staged, &backup)?; + let applied = context + .client + .apply(ApplyRequest { + source_directory: folder.clone(), + manifest_path: Some(folder.join(manifest::MANIFEST_FILE)), + env_file: None, + create_only: false, + agent: agent.clone(), + }) + .await; + if let Err(error) = applied { + std::fs::remove_dir_all(folder).ok(); + if move_folder(&backup, folder).is_ok() { + eprintln!("The agent was not updated; {} is as it was.", folder.display()); + } + return Err(error.into()); + } + println!("Updated {}", folder.display()); + let name = &agent.metadata.name; + let cli = command_line(context.home); + if existing.state == State::Stopped { + println!("agent/{name} stays stopped; `{cli} start agent/{name}` starts it as updated."); + } else { + println!("agent/{name} is updating"); + if let Err(error) = crate::wait_until_converged(context.client, name, context.arguments.timeout).await { + eprintln!("\nagent/{name} did not become ready: {error}"); + return Err(CommandError::Message(format!( + "agent/{name} is not ready; `{cli} describe agent/{name}` shows why" + ))); + } + println!("agent/{name} is updated and ready."); + } + let missing = sign_in::logins(context.client, agent).await?.missing; + if !missing.is_empty() { + let names: Vec<_> = missing.iter().map(|harness| harness.display_name()).collect(); + println!( + "It has no login for {} yet; `{cli} setup --existing {name} --action sign-in` signs in.", + names.join(" or ") + ); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::expect_used)] + + use agent::{Harness, MountSpec, manifest}; + + use super::{LeftOut, Record, Recorded, changes, keep_answers, record, recorded}; + use crate::setup::source::team_draft; + + const MANIFEST: &str = "apiVersion: agents.platform/v1alpha1\nkind: Agent\nmetadata: {name: worker}\nspec:\n \ + sandbox:\n image: {type: reference, reference: example.com/agent:latest}\n \ + platform: {os: linux}\n resources: {cpu: '2', memory: 4Gi, rootFilesystem: \ + {capacity: 20Gi, mode: layered}}\n mounts:\n - {type: bind, source: ../repo, \ + target: /mnt/repo, readOnly: false}\n home: {source: home}\n harnesses:\n - {type: \ + claudeCode, auth: mediated, default: true}\n - {type: codex, auth: mediated, \ + optional: true}\n network: {mode: mediated, allow: all}\n"; + + #[test] + fn an_update_keeps_what_setup_asked_and_takes_the_rest_from_the_source() { + let root = tempfile::tempdir().expect("a root"); + let source = root.path().join("source"); + std::fs::create_dir_all(source.join("home")).expect("a home"); + std::fs::write(source.join("agent.yaml"), MANIFEST).expect("a manifest"); + let current = MANIFEST + .replace("name: worker", "name: mine") + .replace("cpu: '2', memory: 4Gi", "cpu: '6', memory: 12Gi") + .replace("capacity: 20Gi", "capacity: 90Gi") + .replace("source: ../repo", "source: /work/repo") + .replace( + "target: /mnt/repo, readOnly: false}\n", + "target: /mnt/repo, readOnly: false}\n - {type: bind, source: /work/notes, target: \ + /mnt/notes, readOnly: false}\n", + ) + .replace( + "claudeCode, auth: mediated, default: true", + "claudeCode, auth: mediated", + ) + .replace( + "codex, auth: mediated, optional: true", + "codex, auth: mediated, default: true", + ) + .replace( + " network:", + " secrets:\n - {environment: GITHUB_TOKEN, allowedHosts: [github.com]}\n network:", + ); + let current = manifest::decode(current.as_bytes()).expect("the current agent"); + let mut draft = team_draft(&source.join("agent.yaml"), "a test").expect("a draft"); + + keep_answers(¤t, &LeftOut::default(), root.path(), &mut draft); + + let spec = &draft.agent.spec; + assert_eq!(draft.agent.metadata.name, "mine"); + assert_eq!(spec.sandbox.resources.cpu, current.spec.sandbox.resources.cpu); + assert_eq!(spec.sandbox.resources.memory, current.spec.sandbox.resources.memory); + assert_eq!( + spec.sandbox.resources.root_filesystem.capacity(), + current.spec.sandbox.resources.root_filesystem.capacity() + ); + assert_eq!(spec.default_harness().map(|harness| harness.kind), Some(Harness::Codex)); + let sources: Vec<_> = spec + .sandbox + .mounts + .iter() + .filter_map(|mount| match mount { + MountSpec::Bind { source, .. } => Some(source.display().to_string()), + MountSpec::Tmpfs { .. } => None, + }) + .collect(); + assert_eq!(sources, ["/work/repo", "/work/notes"]); + assert!(spec.secrets.iter().any(|secret| secret.environment == "GITHUB_TOKEN")); + assert!(spec.environment.iter().any(|variable| variable.name == "GIT_USER_NAME")); + assert_eq!(draft.built_here, None); + } + + #[test] + fn what_setup_left_out_stays_out_while_what_the_source_adds_comes_in() { + let root = tempfile::tempdir().expect("a root"); + let source = root.path().join("source"); + std::fs::create_dir_all(source.join("home")).expect("a home"); + let added = MANIFEST.replace("optional: true}\n", "optional: true}\n - {type: pi}\n"); + std::fs::write(source.join("agent.yaml"), added).expect("a manifest"); + let current = MANIFEST + .replace( + " mounts:\n - {type: bind, source: ../repo, target: /mnt/repo, readOnly: false}\n", + "", + ) + .replace("\n - {type: codex, auth: mediated, optional: true}", ""); + let current = manifest::decode(current.as_bytes()).expect("the current agent"); + let left_out = LeftOut { + harnesses: vec![Harness::Codex], + shared: vec!["/mnt/repo".into()], + }; + let mut draft = team_draft(&source.join("agent.yaml"), "a test").expect("a draft"); + + keep_answers(¤t, &left_out, root.path(), &mut draft); + + let spec = &draft.agent.spec; + let harnesses: Vec<_> = spec.harnesses.iter().map(|harness| harness.kind).collect(); + assert_eq!(harnesses, [Harness::ClaudeCode, Harness::Pi]); + assert_eq!( + spec.default_harness().map(|harness| harness.kind), + Some(Harness::ClaudeCode) + ); + assert!(spec.sandbox.mounts.is_empty(), "{:?}", spec.sandbox.mounts); + assert_eq!(draft.left_out, left_out, "an update records what stays out"); + } + + #[test] + fn where_an_agent_came_from_reads_back_as_recorded() { + let folder = tempfile::tempdir().expect("a folder"); + for source in [ + Recorded::Standard { + id: "dotnet-backend-developer".into(), + }, + Recorded::Team { + from: "https://github.com/Altinn/altinn-studio//agents/minimal@main".into(), + variant: Some("nested".into()), + }, + ] { + let record_of = |left_out| Record { + source: source.clone(), + left_out, + }; + for left_out in [ + LeftOut::default(), + LeftOut { + harnesses: vec![Harness::Codex], + shared: vec!["/mnt/repo".into()], + }, + ] { + record(folder.path(), &record_of(left_out.clone())).expect("recorded"); + assert_eq!(recorded(folder.path()), Some(record_of(left_out))); + } + } + } + + #[test] + fn the_changes_name_each_file_added_removed_or_changed_but_not_the_values() { + let root = tempfile::tempdir().expect("a root"); + let (before, after) = (root.path().join("before"), root.path().join("after")); + for folder in [&before, &after] { + std::fs::create_dir_all(folder.join("skills/a")).expect("folders"); + std::fs::write(folder.join("agent.yaml"), "same").expect("a manifest"); + } + std::fs::write(before.join(".env"), "A=1").expect("values"); + std::fs::write(after.join(".env"), "A=2").expect("values"); + std::fs::write(before.join("instructions.md"), "old").expect("instructions"); + std::fs::write(after.join("instructions.md"), "new").expect("instructions"); + std::fs::write(before.join("skills/a/SKILL.md"), "gone").expect("a skill"); + std::fs::write(after.join("skills/a/notes.md"), "added").expect("a note"); + + assert_eq!( + changes(&before, &after).expect("compared"), + ["~ instructions.md", "- skills/a/SKILL.md", "+ skills/a/notes.md"] + ); + assert_eq!(changes(&before, &before).expect("compared"), Vec::::new()); + } +} diff --git a/agentctl/src/control_plane/service.rs b/agentctl/src/control_plane/service.rs index e5905fdf..9446d3b9 100644 --- a/agentctl/src/control_plane/service.rs +++ b/agentctl/src/control_plane/service.rs @@ -584,65 +584,12 @@ fn applied_run_state(requested: Option, current: Option<&AgentR } fn validate_immutable_fields(current: &AgentRecord, desired: &Agent) -> Result<(), Error> { - if current.agent.spec.sandbox.image != desired.spec.sandbox.image { - return Err(Error::Immutable("spec.sandbox.image")); - } - if current.agent.spec.sandbox.platform != desired.spec.sandbox.platform { - return Err(Error::Immutable("spec.sandbox.platform")); - } - if current.agent.spec.sandbox.init_system != desired.spec.sandbox.init_system { - return Err(Error::Immutable("spec.sandbox.initSystem")); - } - if current.agent.spec.sandbox.resources.root_filesystem.mode() - != desired.spec.sandbox.resources.root_filesystem.mode() - { - return Err(Error::Immutable("spec.sandbox.resources.rootFilesystem.mode")); - } - if current.agent.spec.sandbox.mounts != desired.spec.sandbox.mounts { - return Err(Error::Immutable("spec.sandbox.mounts")); - } - if current.agent.spec.home != desired.spec.home { - return Err(Error::Immutable("spec.home")); - } - if current.agent.spec.instructions != desired.spec.instructions { - return Err(Error::Immutable("spec.instructions")); - } - if current.agent.spec.skills != desired.spec.skills { - return Err(Error::Immutable("spec.skills")); - } - let current_kinds = current - .agent - .spec - .harnesses - .iter() - .map(|harness| harness.kind) - .collect::>(); - let desired_kinds = desired - .spec - .harnesses - .iter() - .map(|harness| harness.kind) - .collect::>(); - if current_kinds != desired_kinds { - return Err(Error::Immutable("spec.harnesses.type")); - } - let current_auth = current + current .agent .spec - .harnesses - .iter() - .map(|harness| (harness.kind, harness.auth)) - .collect::>(); - let desired_auth = desired - .spec - .harnesses - .iter() - .map(|harness| (harness.kind, harness.auth)) - .collect::>(); - if current_auth != desired_auth { - return Err(Error::Immutable("spec.harnesses.auth")); - } - Ok(()) + .immutable_changes(&desired.spec) + .first() + .map_or(Ok(()), |field| Err(Error::Immutable(field))) } async fn resolve_mount_sources(agent: &mut Agent, source_directory: &std::path::Path) -> Result<(), Error> { diff --git a/agentctl/src/manifest.rs b/agentctl/src/manifest.rs index 92c76df7..421e5b68 100644 --- a/agentctl/src/manifest.rs +++ b/agentctl/src/manifest.rs @@ -521,6 +521,44 @@ impl Spec { } } + /// The fields an existing agent cannot change, in manifest notation, that `desired` changes. + /// Each shapes what the agent was created from, such as its image or the files it reads, so + /// changing one takes creating the agent again. + #[must_use] + pub fn immutable_changes(&self, desired: &Self) -> Vec<&'static str> { + let kinds = |spec: &Self| -> std::collections::BTreeSet<_> { + spec.harnesses.iter().map(|harness| harness.kind).collect() + }; + let auth = |spec: &Self| -> std::collections::BTreeSet<_> { + spec.harnesses + .iter() + .map(|harness| (harness.kind, harness.auth)) + .collect() + }; + let (current, desired_sandbox) = (&self.sandbox, &desired.sandbox); + [ + ("spec.sandbox.image", current.image != desired_sandbox.image), + ("spec.sandbox.platform", current.platform != desired_sandbox.platform), + ( + "spec.sandbox.initSystem", + current.init_system != desired_sandbox.init_system, + ), + ( + "spec.sandbox.resources.rootFilesystem.mode", + current.resources.root_filesystem.mode() != desired_sandbox.resources.root_filesystem.mode(), + ), + ("spec.sandbox.mounts", current.mounts != desired_sandbox.mounts), + ("spec.home", self.home != desired.home), + ("spec.instructions", self.instructions != desired.instructions), + ("spec.skills", self.skills != desired.skills), + ("spec.harnesses.type", kinds(self) != kinds(desired)), + ("spec.harnesses.auth", auth(self) != auth(desired)), + ] + .into_iter() + .filter_map(|(field, changed)| changed.then_some(field)) + .collect() + } + /// Returns whether the agent declares SSH access. #[must_use] pub fn ssh_access(&self) -> bool {