From 4a4ab9b09e750898511e6653bc3ea0fd35fe7daa Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:32:04 +0200 Subject: [PATCH 1/2] feat: separate agent identity from presentation agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- INVARIANTS.md | 1 + README.md | 26 +- crates/agent-spec/src/kdl_format.rs | 25 + crates/agent-spec/src/spec.rs | 53 +- crates/agent-spec/tests/discovery.rs | 107 ++ ...agent-identity-and-mutable-presentation.md | 66 ++ docs/vrs/requirements.md | 37 + docs/vrs/spec.md | 48 + flake.lock | 8 +- flake.nix | 5 +- src/agent_author.rs | 923 ++++++++++++++++++ src/agents.rs | 25 +- src/cutover_admission.rs | 2 + src/ding_reconcile.rs | 1 + src/eval_run.rs | 2 + src/eval_spec.rs | 21 +- src/exec_backend.rs | 1 + src/lib.rs | 5 +- src/main.rs | 96 +- src/reconcile.rs | 63 +- src/run.rs | 211 +++- tests/agent_presentation.rs | 186 ++++ tests/exec_backend.rs | 1 + tests/hooks.rs | 6 +- tests/nomad_survival.rs | 130 +++ tests/reconcile.rs | 61 ++ tests/run.rs | 2 + tests/status_agents.rs | 19 +- 28 files changed, 2075 insertions(+), 56 deletions(-) create mode 100644 docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md create mode 100644 src/agent_author.rs create mode 100644 tests/agent_presentation.rs diff --git a/INVARIANTS.md b/INVARIANTS.md index e0bc96d4..54dd43a1 100644 --- a/INVARIANTS.md +++ b/INVARIANTS.md @@ -6,6 +6,7 @@ materialization, messaging, DING, or presence must preserve them. | Invariant | Guarantee | Proof | |---|---|---| | **R11 control-plane replacement safety** | Normal stop or SIGKILL of `st2 up`, atomic binary replacement, and control-plane restart leave live PTY and exec agents usable with the same PID and creation identity. The replacement adopts each survivor exactly once, launches only genuinely missing work, and never duplicates a survivor; only explicit teardown kills an agent. | `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::explicit_teardown_kills_exec_but_plain_stop_does_not`; `tests/nomad_survival.rs::explicit_teardown_kills_pty_but_plain_stop_does_not` | +| **Stable identity with live presentation** | The positional Agent Spec identity remains the only routing, authorization, task, and lifecycle key. Optional name and description are bounded declaration metadata. st2 projects them through one exact-ID atomic PTY metadata patch; changes and clears preserve PID and creation identity, unchanged projections emit nothing, and presentation failure never authorizes restart or teardown. | `crates/agent-spec/tests/discovery.rs::presentation_bounds_count_unicode_scalars_and_reject_noncanonical_values`; `tests/agent_presentation.rs::cli_sets_replaces_and_clears_fields_without_changing_identity_or_other_bytes`; `tests/reconcile.rs::live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift`; `src/run.rs::metadata_patch_uses_exact_id_and_one_json_stdin_payload`; `tests/nomad_survival.rs::presentation_changes_patch_the_exact_live_pty_without_restarting_it` | | **R23 fail-closed adoption inventory** | `st2 tasks --json` joins the shared-locked desired catalog to exact PTY and exec generations without reconciliation. Missing roots are positively absent without creation; malformed, unreadable, duplicate, PID-reused, or incomplete evidence is indeterminate, makes the typed envelope incomplete, and exits non-zero. Strict exec generations are durably published while legacy PID evidence is observed read-only. | `src/task_inventory.rs::stable_wire_shape_maps_pty_exec_explicit_default_and_ignores_foreign_host`; `src/task_inventory.rs::observer_cannot_claim_complete_running_without_generation_evidence`; `src/task_inventory.rs::retired_task_desires_absence_without_erasing_observed_generation`; `src/exec_backend.rs::spawn_atomically_publishes_strict_generation_json`; `src/exec_backend.rs::start_token_mismatch_cannot_report_running_or_signal_reused_pid`; `tests/task_inventory_cli.rs` | | **Transport-decoupled lifecycle** | Each task is isolated from a supervisor/transport process-group or cgroup cascade. | `tests/transport_isolation.rs`; `tests/transport_isolation_macos.rs` | | **Clean exec teardown** | Killing an exec task reaps its whole process group. | `tests/exec_backend.rs::exec_kill_reaps_the_whole_process_group_not_just_the_leader` | diff --git a/README.md b/README.md index 09f5b0e6..27730041 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,8 @@ agent "" { // Optional metadata: // role "worker" // supervisor "" + // name "Release worker" + // description "Owns release preparation and verification." env { ST_AGENT "." } argv "codex" "--dangerously-bypass-approvals-and-sandbox" "--dangerously-bypass-hook-trust" "" ding @@ -186,6 +188,21 @@ access, readiness, or lifecycle policy, and URI possession conveys no authority. declaration edits do not stop, replace, or relaunch a live task. Resource types and resolvers remain opaque to st2; catalog readers use the public `agent-spec` crate to inspect the typed bindings. +The positional agent value is the stable automation identity. Optional `name` and `description` +fields are presentation only; they never route messages, select tasks, or rename durable +state. Mutate a catalog-owned KDL declaration through the constrained commands: + +```sh +st2 rename "Release worker" +st2 describe "Owns release preparation and verification." +st2 rename --clear +``` + +These commands preserve unrelated KDL bytes and refuse TOML, JSON, and +`meta { managed-by "nix" }` targets. `ST_AGENT` callers may edit themselves or declared +descendants; an operator without `ST_AGENT` may use the same bounded authoring path. The sibling +`/name` convention is retired and ignored. + `argv` launches its first value directly with the remaining values as arguments. It resolves a bare program such as `codex` through the task environment's `PATH`, preserves argument boundaries, and does not introduce a shell. Use `command #"..."#` instead when the task intentionally needs shell @@ -340,9 +357,10 @@ st2 context read --full ``` The roster includes retired declarations instead of silently conflating them with runtime -presence. Both JSON shapes contain `retired` and the declaration's ordered `resources` descriptors; -`--enrich` additionally supplies `lastActivity` and `inbox`. Human output leaves active rows -unchanged and appends `[retired]` to a retired row. +presence. Both JSON shapes keep stable `identity` separate from optional `name` and `description`, +and contain `retired` plus the declaration's ordered `resources` descriptors. `--enrich` +additionally supplies `lastActivity` and `inbox`. Human output prints the same presentation fields +as separate columns and appends `[retired]` to a retired row. For a catalog-backed agent, every native bus operation resolves the same agent directory used by the roster: presence is `/status`, while unread messages, archive receipts, context, and @@ -407,7 +425,7 @@ st2 service uninstall ```text ls, up, down, validate, doctor -message, ding, agents, status, context, resource +message, ding, agents, status, context, resource, rename, describe env, pty, shell, pretrust hooks, service, eval agent digest, agent publish diff --git a/crates/agent-spec/src/kdl_format.rs b/crates/agent-spec/src/kdl_format.rs index a14a3979..c86c052f 100644 --- a/crates/agent-spec/src/kdl_format.rs +++ b/crates/agent-spec/src/kdl_format.rs @@ -76,6 +76,8 @@ fn agent_node_to_raw(node: &KdlNode) -> anyhow::Result { for child in children.nodes() { match child.name().value() { "identity" => raw.identity = arg_string(child).or(raw.identity), + "name" => parse_presentation(child, "name", &mut raw.name)?, + "description" => parse_presentation(child, "description", &mut raw.description)?, "host" => raw.host = arg_string(child), "role" => raw.role = arg_string(child), "type" => raw.job_type = arg_string(child), @@ -116,6 +118,29 @@ fn agent_node_to_raw(node: &KdlNode) -> anyhow::Result { Ok(raw) } +fn parse_presentation( + node: &KdlNode, + field: &str, + destination: &mut Option, +) -> anyhow::Result<()> { + anyhow::ensure!( + destination.is_none(), + "agent declares `{field}` more than once" + ); + anyhow::ensure!( + node.children().is_none() + && node.entries().len() == 1 + && node.entries()[0].name().is_none(), + "agent `{field}` must contain exactly one positional string" + ); + let value = node + .get(0) + .and_then(|value| value.as_string()) + .ok_or_else(|| anyhow::anyhow!("agent `{field}` must contain a string"))?; + *destination = Some(value.to_owned()); + Ok(()) +} + fn resource_node_to_raw(node: &KdlNode) -> anyhow::Result<(String, RawResource)> { if node.children().is_some() { anyhow::bail!("resource binding cannot have children"); diff --git a/crates/agent-spec/src/spec.rs b/crates/agent-spec/src/spec.rs index f47d2d16..84be334f 100644 --- a/crates/agent-spec/src/spec.rs +++ b/crates/agent-spec/src/spec.rs @@ -3,7 +3,8 @@ //! A job reads like a Nomad job: the *agent* is the job, its **tasks** are `pty{}` (interactive — //! allocates a terminal, an agent harness) and `exec{}` (a plain process — the ding, daemons, a //! stage's script; must NOT allocate a terminal, R09). st2 reads only the runner-normative subset: -//! `identity`, `host`, `role` (metadata only), `type`, `workspace`, `retired`, `keep`, `supervisor`, +//! `identity`, presentation (`name`, `description`), `host`, `role` (metadata only), +//! `type`, `workspace`, `retired`, `keep`, `supervisor`, //! `restart{}`, task lifecycle, Resource bindings (declaration metadata), and the tasks. Everything render-only //! (`harness`, `model`, `persona`, `permissions`, `transport`, `strategy`, `meta{}`) is baked into //! the tasks/commands by the render layer and ignored here. @@ -19,11 +20,20 @@ use std::time::Duration; use serde::de::{self, MapAccess, Visitor}; use serde::{Deserialize, Serialize}; +/// Maximum Unicode scalar count for an agent's human-facing label. +pub const AGENT_NAME_MAX_CHARS: usize = 160; +/// Maximum Unicode scalar count for an agent's enduring responsibility description. +pub const AGENT_DESCRIPTION_MAX_CHARS: usize = 1_000; + /// A rendered agent job, lowered to the shared declaration fields st2 and other readers inspect. #[derive(Debug, Clone, PartialEq, Eq)] pub struct AgentSpec { /// Unique id; the bus id is `.`. pub identity: String, + /// Optional mutable human-facing label. Never used as an automation selector. + pub name: Option, + /// Optional enduring responsibility boundary. Never used for lifecycle decisions. + pub description: Option, /// Which machine runs this agent. `None` → resolved to the path's host / this machine. pub host: Option, /// Optional declared persona role. Preserved as metadata and ignored for execution. @@ -264,6 +274,8 @@ pub fn parse_duration(s: &str) -> Result { #[derive(Debug, Default, Deserialize)] pub(crate) struct RawSpec { pub identity: Option, + pub name: Option, + pub description: Option, pub host: Option, pub role: Option, #[serde(rename = "type")] @@ -553,8 +565,7 @@ fn validate_uri_component(value: &str, extra: &[u8]) -> Result<(), &'static str> } else if byte.is_ascii_alphanumeric() || matches!( byte, - b'-' - | b'.' + b'-' | b'.' | b'_' | b'~' | b'!' @@ -600,6 +611,12 @@ impl RawSpec { host: Option, path: PathBuf, ) -> anyhow::Result { + validate_presentation("name", self.name.as_deref(), AGENT_NAME_MAX_CHARS)?; + validate_presentation( + "description", + self.description.as_deref(), + AGENT_DESCRIPTION_MAX_CHARS, + )?; validate_launch( &identity, self.command.as_ref(), @@ -664,6 +681,8 @@ impl RawSpec { Ok(AgentSpec { identity, + name: self.name, + description: self.description, host, role: self.role, job_type, @@ -679,6 +698,34 @@ impl RawSpec { } } +/// Validate one optional presentation field at the shared parse/authoring boundary. +pub fn validate_presentation( + field: &str, + value: Option<&str>, + max_chars: usize, +) -> anyhow::Result<()> { + let Some(value) = value else { + return Ok(()); + }; + anyhow::ensure!( + !value.is_empty(), + "agent presentation `{field}` cannot be empty; omit it to clear it" + ); + anyhow::ensure!( + value.trim() == value, + "agent presentation `{field}` cannot begin or end with whitespace" + ); + anyhow::ensure!( + !value.chars().any(char::is_control), + "agent presentation `{field}` must be one printable line without control characters" + ); + anyhow::ensure!( + value.chars().count() <= max_chars, + "agent presentation `{field}` exceeds the {max_chars}-character limit" + ); + Ok(()) +} + impl RawTask { pub(crate) fn lower( self, diff --git a/crates/agent-spec/tests/discovery.rs b/crates/agent-spec/tests/discovery.rs index e7113abd..116d3ff1 100644 --- a/crates/agent-spec/tests/discovery.rs +++ b/crates/agent-spec/tests/discovery.rs @@ -306,6 +306,113 @@ argv = ["claude", "--resume", "session id"] ); } +#[test] +fn presentation_metadata_lowers_from_kdl_toml_and_json_without_changing_identity() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "agents/h/kdl/agent.kdl", + r#"agent "kdl" { + host "h" + name "Display label" + description "Enduring responsibility" + command "true" +}"#, + ); + write( + tmp.path(), + "agents/h/toml/agent.toml", + r#"identity = "toml" +host = "h" +name = "Display label" +description = "Enduring responsibility" +command = "true" +"#, + ); + write( + tmp.path(), + "agents/h/json/agent.json", + r#"{"identity":"json","host":"h","name":"Display label","description":"Enduring responsibility","command":"true"}"#, + ); + + let found = agent_spec::discover(tmp.path()); + assert!(found.errors.is_empty(), "{:?}", found.errors); + for identity in ["kdl", "toml", "json"] { + let spec = find(&found.specs, identity); + assert_eq!(spec.identity, identity); + assert_eq!(spec.name.as_deref(), Some("Display label")); + assert_eq!(spec.description.as_deref(), Some("Enduring responsibility")); + } +} + +#[test] +fn malformed_or_duplicate_kdl_presentation_is_rejected() { + for (case, body) in [ + ("duplicate", "name \"one\"; name \"two\""), + ("wrong-type", "description 42"), + ("children", "description { nested \"no\" }"), + ] { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + &format!("agents/h/{case}/agent.kdl"), + &format!("agent {case:?} {{ host \"h\"; {body}; command \"true\" }}"), + ); + let found = agent_spec::discover(tmp.path()); + assert!(found.specs.is_empty(), "{case}: {:?}", found.specs); + assert_eq!(found.errors.len(), 1, "{case}: {:?}", found.errors); + assert!( + found.errors[0].message.contains("must contain") + || found.errors[0].message.contains("more than once"), + "{case}: {}", + found.errors[0].message + ); + } +} + +#[test] +fn presentation_bounds_count_unicode_scalars_and_reject_noncanonical_values() { + use agent_spec::spec::{ + AGENT_DESCRIPTION_MAX_CHARS, AGENT_NAME_MAX_CHARS, validate_presentation, + }; + + let name_at_limit = "é".repeat(AGENT_NAME_MAX_CHARS); + let description_at_limit = "界".repeat(AGENT_DESCRIPTION_MAX_CHARS); + assert!( + validate_presentation("name", Some(&name_at_limit), AGENT_NAME_MAX_CHARS).is_ok() + ); + assert!( + validate_presentation( + "description", + Some(&description_at_limit), + AGENT_DESCRIPTION_MAX_CHARS, + ) + .is_ok() + ); + assert!( + validate_presentation( + "name", + Some(&format!("{name_at_limit}x")), + AGENT_NAME_MAX_CHARS, + ) + .is_err() + ); + assert!( + validate_presentation( + "description", + Some(&format!("{description_at_limit}x")), + AGENT_DESCRIPTION_MAX_CHARS, + ) + .is_err() + ); + for invalid in ["", " leading", "trailing ", "two\nlines", "control\u{7f}"] { + assert!( + validate_presentation("name", Some(invalid), AGENT_NAME_MAX_CHARS).is_err(), + "accepted {invalid:?}" + ); + } +} + #[test] fn named_resource_bindings_are_typed_uri_identities_and_order_independent() { let tmp = tempfile::tempdir().unwrap(); diff --git a/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md b/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md new file mode 100644 index 00000000..1189895d --- /dev/null +++ b/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md @@ -0,0 +1,66 @@ +# Stable agent identity is separate from mutable presentation + +Status: draft + +Approval required: Nathan + +## Context + +Agent Spec currently overloads one identity string as both an automation key +and the only human recognition surface. Operators cannot improve a label +without changing routing, durable paths, task identity, and process lifecycle. +An experimental sibling `name` file introduced a second source of truth and did +not compose with transactional Agent Spec publication. + +The fleet requires a stable automation identity and presentation that can +change while the running process, PTY generation, bus, and durable state remain +continuous. The change must not introduce a stable-ID alias, dual parser, or +long-lived migration branch. + +## Decision + +The existing positional Agent Spec identity remains the sole stable automation +ID. Its grammar and the established `identity` JSON/TOML/roster spelling remain +unchanged. There is no stable-ID rename operation. + +Agent Spec adds direct optional `name` and `description` fields. They are +non-authoritative, non-unique presentation. Omission means absence. The Agent +Spec declaration is their sole source of truth; a sibling `name` file is neither +read nor written. + +Constrained KDL-only commands may mutate one presentation field without +publishing a second representation. Catalog agents may edit themselves or +declared descendants; operators may use the same constrained path. Nix-owned +declarations remain writable only at their Nix source. st2 serializes these +edits with its catalog lock, preserves unrelated source bytes, detects stale +source, and atomically replaces the declaration. + +Healthy runtime reconciliation uses the atomic exact-ID-only `pty metadata +patch --id ` operation. It projects name to native PTY `displayName` +and a versioned st2-owned tag snapshot containing stable actor identity plus +optional description. Name is not duplicated in tags. One real patch emits one +coherent `metadata_change` event; an unchanged patch emits none. Automation +never uses human display-name resolution. Presentation drift degrades and +retries without restart or lifecycle accounting. + +## Consequences + +- Human labels can improve without breaking routing or continuity. +- Duplicate or absent names are valid; stable IDs remain visible for exact + disambiguation and automation. +- The old equality between stable identity and every presentation surface is + superseded, but stable routing semantics are preserved. +- Existing declarations require no compatibility marker because the new fields + are optional and additive. Adoption begins only after compatible PTY and st2 + binaries are deployed. +- This decision is not accepted until Nathan approves the protected requirement + changes. + +## Evidence required for acceptance + +- parser and roster tests across KDL, TOML, and JSON; +- source-preservation, authority, Nix refusal, and stale-writer tests; +- exact-ID PTY projection tests for set, clear, idempotence, and partial failure; +- a live no-restart test preserving stable task ID, PID, creation identity, and + generation across presentation changes; +- a genuine lifecycle-change control that still performs ordinary replacement. diff --git a/docs/vrs/requirements.md b/docs/vrs/requirements.md index c688fef2..d66cccf5 100644 --- a/docs/vrs/requirements.md +++ b/docs/vrs/requirements.md @@ -43,6 +43,9 @@ accepted. lifecycle meaning; discovery excludes `.git` and `.st2` control directories at any depth, the catalog-root `pty` runtime directory, and state namespaces directly owned by a declaration. + The positional agent identity remains the stable automation ID. Optional + `name` and `description` fields are presentation only, may be + absent or non-unique, and never become routing aliases. - **R03 Host-pinned placement:** Every runnable agent or task resolves to its declared host; host-local roots own reconciliation. @@ -79,6 +82,9 @@ accepted. control plane adopts those existing processes by stable identity and starts only genuinely missing work. Stopping an agent is a separate, explicit lifecycle action. + Changing or clearing presentation metadata is not a lifecycle action and + must preserve the PTY, PID, task generation, inbox, context, Resources, and + supervisor binding. ### Must externalize agent state and scope @@ -86,6 +92,8 @@ accepted. presence, declared activity status, current plan, and current plan step without PTY or transcript inspection. Presence and activity status are distinct, and stale state is identifiable. + Machine and human rosters expose stable identity separately from optional + presentation metadata and never read a sibling display-name file. - **R09 State continuity:** An agent's current work and durable decisions can survive process replacement without depending on its transcript. - **R10 Agent-only identity:** st2 models agents. Non-agent identities are @@ -94,6 +102,8 @@ accepted. - **R13 Shortest-path reconciliation:** An event is evidence, not permission to run the world. st2 classifies source, path, kind, and affected identity, then takes the shortest correct path from observed state to desired state. + Presentation-only declaration changes take a metadata-reconciliation path + outside launch, replacement, garbage collection, and flapping accounting. - **R14 Explicit filesystem-event contracts:** Every watcher is deny-by-default with exact roots, paths, mutation kinds, semantic meaning, debounce policy, and consumer. Reads, opens, unknown paths, and runtime output never trigger @@ -114,6 +124,8 @@ accepted. gates, PTY inspection, and plan execution are limited to the selected owner/task; unrelated diagnostics remain visible while unrelated workspaces, tasks, and live PTY PID/generation stay unchanged. + Stable IDs alone select and authorize automation; presentation values never + resolve a message, Resource, status, lifecycle, or authoring target. - **R20 Portable Resource bindings:** An agent may directly carry zero or more order-independent Resource bindings. Each binding has a non-empty, agent-local unique name and preserves a non-empty, opaque type discriminator and an RFC @@ -303,3 +315,28 @@ accepted. entering successor supervision. It never reopens transaction or predecessor authority. A mismatched history fails closed rather than beginning a new cutover. + +- **R26 Bounded presentation contract:** Agent Specs may declare optional, + non-empty, bounded `name` and `description` strings in canonical KDL and the + readable TOML/JSON forms. `name` is a mutable human label and `description` + is an enduring responsibility boundary. Omission means absence; there is no + fallback or second source of truth. +- **R27 Constrained presentation authoring:** `st2 rename` and `st2 describe` + set or clear only their corresponding direct field in one + canonical KDL declaration. The commands preserve unrelated source bytes, + serialize through the catalog authoring lock, reject a stale source before + atomic replacement, and return classified receipts. TOML, JSON, Nix-owned + declarations, stable-ID changes, and malformed or ambiguous targets fail + closed. A catalog agent may edit itself or a descendant reached through + declared supervisor edges; an operator outside a managed agent context may + use the same constrained operation. This is an operational admission + boundary inside the trusted private fleet, not OS security isolation. +- **R28 Live PTY presentation projection:** For a healthy exact stable task, + st2 reconciles the owning agent's optional name to native PTY display + metadata and publishes a versioned st2-owned tag snapshot containing stable + actor identity plus optional description. It preserves unrelated + tags, removes absent owned values, retries visible partial failure, and is + idempotent. Projection uses exact PTY task IDs only; it never falls back to a + human display-name resolver and never restarts the task. Secondary tasks + preserve their task-specific identity and presentation unless an explicit + deterministic projection says otherwise. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index a708d87d..26c9e3a1 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -60,6 +60,54 @@ materialization, frozen routing after declaration removal, singleton completion, custom task-ID supervision/logging/teardown, and the no-opt-in legacy control in `tests/eval_run_e2e.rs`. +## Stable identity and mutable presentation (R02, R08, R11, R13, R19, R26-R28) + +The positional value in `agent ""` remains the stable Agent Spec ID. +The supported child/TOML/JSON `identity` spelling and the roster JSON +`identity` field remain unchanged. Host qualification produces the existing +`.` bus ID. Only that stable identity controls routing, +selectors, supervisor edges, task IDs, durable paths, Resources, +authorization, and lifecycle reconciliation. + +An Agent Spec may additionally carry two direct optional strings: + +```kdl +agent "worker" { + host "host" + name "Release worker" + description "Owns release preparation and verification." +} +``` + +`name` is a non-unique human label and `description` is the enduring +responsibility boundary. Omission is the only cleared representation. Name is +limited to 160 Unicode scalars and description to 1,000. Explicit empty, +surrounding-whitespace, multiline, control-character, or over-limit values are +invalid. The declaration is the +sole source of truth; `/name` is not read, projected, or maintained. + +`st2 rename` and `st2 describe` accept one stable selector +and either a value or `--clear`. They edit canonical KDL only. Under the +persistent exclusive catalog-authoring lock, st2 resolves one declaration, +checks the caller's `ST_AGENT` self/descendant relationship when present, +refuses `meta { managed-by "nix" }`, applies one span-bounded edit, reparses the +candidate, rechecks the original source bytes, and atomically replaces and +fsyncs the declaration. TOML/JSON authoring and stable-ID rename do not exist. +The classified refusal codes are an operational trusted-fleet boundary; they +do not claim adversarial OS isolation from another process that can write the +catalog directly. + +For each healthy managed primary agent PTY, presentation reconciliation uses +the exact stable PTY task ID. `name` maps only to native `displayName`; it is +not duplicated into a tag. st2 owns a versioned tag snapshot containing +`agent.actor.path=.` plus optional description, +while every unrelated tag is preserved. Clearing a field clears only its +native or owned projection. Repeated reconciliation is a no-op. A failed +metadata projection is reported as degraded and retried by the ordinary loop; +it never enters launch, teardown, garbage collection, or flapping accounting. +Secondary PTYs retain their task-specific display identity unless a separate +deterministic task convention explicitly defines a presentation. + ## Resource bindings (R20-R21) An agent may directly declare zero or more generic Resource bindings: diff --git a/flake.lock b/flake.lock index 8182e262..ea0ff748 100644 --- a/flake.lock +++ b/flake.lock @@ -41,17 +41,17 @@ ] }, "locked": { - "lastModified": 1785413661, - "narHash": "sha256-R4wNXc0qS3Dik284DQAfxdmKe5elmMLwqhse+glMSEI=", + "lastModified": 1785496230, + "narHash": "sha256-AiJzsN12YQP64W2Sq+iePVtJqq1wPrWhTR2K60W5l9c=", "owner": "compoundingtech", "repo": "pty", - "rev": "afeb3b6234b7010b7db802fd029766ad17c14219", + "rev": "4c67fb51a161b66e949b5be905ca9164f7a04c99", "type": "github" }, "original": { "owner": "compoundingtech", "repo": "pty", - "rev": "afeb3b6234b7010b7db802fd029766ad17c14219", + "rev": "4c67fb51a161b66e949b5be905ca9164f7a04c99", "type": "github" } }, diff --git a/flake.nix b/flake.nix index ceb9b962..3b10d4a9 100644 --- a/flake.nix +++ b/flake.nix @@ -4,9 +4,8 @@ inputs = { nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; flake-utils.url = "github:numtide/flake-utils"; - # Packaged PTY fleet-observation gate: the exact `pty list --json` producer revision with - # ambiguity-safe PID reads, EPERM handling, and one fleet-wide socket fallback budget. - pty.url = "github:compoundingtech/pty/afeb3b6234b7010b7db802fd029766ad17c14219"; + # Exact PTY producer revision for ambiguity-safe lookup and atomic exact-ID metadata patches. + pty.url = "github:compoundingtech/pty/4c67fb51a161b66e949b5be905ca9164f7a04c99"; pty.inputs.nixpkgs.follows = "nixpkgs"; }; diff --git a/src/agent_author.rs b/src/agent_author.rs new file mode 100644 index 00000000..627afd55 --- /dev/null +++ b/src/agent_author.rs @@ -0,0 +1,923 @@ +//! Constrained, source-preserving authoring of Agent Spec presentation metadata. +//! +//! Presentation is declaration state, not runtime identity. Every edit holds the shared catalog +//! writer lock, rechecks the original bytes, and atomically replaces exactly one canonical KDL +//! declaration. TOML, JSON, Nix-owned declarations, and unauthorized agent callers fail closed. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt; +use std::fs::{self, OpenOptions}; +use std::io::Write as _; +use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt as _, PermissionsExt as _}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +use agent_spec::spec::{AGENT_DESCRIPTION_MAX_CHARS, AGENT_NAME_MAX_CHARS, validate_presentation}; +use kdl::{KdlDocument, KdlNode}; +use serde::Serialize; + +static TMP_COUNTER: AtomicU64 = AtomicU64::new(0); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct SourceVersion { + device: u64, + inode: u64, + length: u64, + modified_seconds: i64, + modified_nanoseconds: i64, + changed_seconds: i64, + changed_nanoseconds: i64, +} + +impl SourceVersion { + fn from_metadata(metadata: &fs::Metadata) -> Self { + Self { + device: metadata.dev(), + inode: metadata.ino(), + length: metadata.len(), + modified_seconds: metadata.mtime(), + modified_nanoseconds: metadata.mtime_nsec(), + changed_seconds: metadata.ctime(), + changed_nanoseconds: metadata.ctime_nsec(), + } + } +} + +/// A mutable presentation field with no routing or lifecycle authority. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum PresentationField { + Name, + Description, +} + +impl PresentationField { + pub fn as_str(self) -> &'static str { + match self { + Self::Name => "name", + Self::Description => "description", + } + } + + fn max_chars(self) -> usize { + match self { + Self::Name => AGENT_NAME_MAX_CHARS, + Self::Description => AGENT_DESCRIPTION_MAX_CHARS, + } + } +} + +/// Whether a request changed declaration bytes. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum AuthorOutcome { + Changed, + Unchanged, +} + +/// Stable machine-readable receipt from one presentation edit. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct PresentationReceipt { + pub result: AuthorOutcome, + pub identity: String, + pub field: PresentationField, + pub value: Option, + pub retired: bool, +} + +/// A classified authoring refusal. `code` is stable for machine consumers. +#[derive(Debug)] +pub struct AuthorError { + code: &'static str, + message: String, +} + +impl AuthorError { + fn new(code: &'static str, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + pub fn code(&self) -> &'static str { + self.code + } +} + +impl fmt::Display for AuthorError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(&self.message) + } +} + +impl std::error::Error for AuthorError {} + +#[derive(Debug)] +struct AgentTarget { + identity: String, + declaration: PathBuf, + retired: bool, +} + +/// Set or clear one presentation field for one stable Agent Spec identity. +/// +/// `actor` is the caller's `ST_AGENT` identity. An absent actor is the explicit operator path. A +/// catalog-managed caller may edit itself or a descendant reached through declared supervisor +/// edges; no presentation field expands that authority. +pub fn set_presentation( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + field: PresentationField, + requested: Option<&str>, +) -> Result { + let _catalog_lock = crate::CatalogLock::exclusive(catalog_root).map_err(|error| { + AuthorError::new( + "catalog-lock-failed", + format!("acquire exclusive catalog-authoring lock: {error:#}"), + ) + })?; + let found = crate::discover(catalog_root); + if let Some(error) = found.errors.first() { + return Err(AuthorError::new( + "catalog-malformed", + format!( + "cannot prove an exact presentation target while {} is malformed: {}", + error.path.display(), + error.message + ), + )); + } + let target = resolve_target(&found.specs, selector, this_host)?; + authorize_actor(&found.specs, &target.identity, this_host, actor)?; + let requested = requested + .map(|value| { + validate_presentation(field.as_str(), Some(value), field.max_chars()) + .map(|()| value.to_owned()) + .map_err(|error| AuthorError::new("invalid-presentation", error.to_string())) + }) + .transpose()?; + let result = edit_declaration( + &target.declaration, + &target.identity, + field, + requested.as_deref(), + || {}, + )?; + Ok(PresentationReceipt { + result, + identity: target.identity, + field, + value: requested, + retired: target.retired, + }) +} + +fn resolve_target( + specs: &[crate::AgentSpec], + selector: &str, + this_host: &str, +) -> Result { + let exact = specs + .iter() + .filter(|spec| spec.bus_id(this_host) == selector) + .collect::>(); + let matches = if exact.is_empty() { + specs + .iter() + .filter(|spec| spec.identity == selector) + .collect::>() + } else { + exact + }; + match matches.as_slice() { + [] => Err(AuthorError::new( + "target-not-found", + format!("no agent {selector:?} found in the selected catalog"), + )), + [spec] => Ok(AgentTarget { + identity: spec.bus_id(this_host), + declaration: spec.path.clone(), + retired: spec.retired, + }), + many => { + let mut candidates = many + .iter() + .map(|spec| format!("{} ({})", spec.bus_id(this_host), spec.path.display())) + .collect::>(); + candidates.sort(); + Err(AuthorError::new( + "target-ambiguous", + format!( + "agent selector {selector:?} is ambiguous: {}", + candidates.join(", ") + ), + )) + } + } +} + +fn authorize_actor( + specs: &[crate::AgentSpec], + target: &str, + this_host: &str, + actor: Option<&str>, +) -> Result<(), AuthorError> { + let Some(actor) = actor else { + return Ok(()); + }; + if actor == target { + return Ok(()); + } + let by_identity = specs + .iter() + .map(|spec| (spec.bus_id(this_host), spec)) + .collect::>(); + let mut current = target.to_owned(); + let mut visited = BTreeSet::new(); + while visited.insert(current.clone()) { + let Some(spec) = by_identity.get(¤t) else { + break; + }; + let Some(supervisor) = spec.supervisor.as_deref() else { + break; + }; + if supervisor == actor { + return Ok(()); + } + let same_host = format!("{}.{}", spec.resolved_host(this_host), supervisor); + let qualified = if by_identity.contains_key(supervisor) { + supervisor.to_owned() + } else if by_identity.contains_key(&same_host) { + same_host + } else { + supervisor.to_owned() + }; + if qualified == actor { + return Ok(()); + } + current = qualified; + } + Err(AuthorError::new( + "presentation-not-authorized", + format!("agent {actor:?} may edit only itself or a declared descendant, not {target:?}"), + )) +} + +#[cfg(test)] +fn edit_declaration_for_test( + path: &Path, + expected_identity: &str, + field: PresentationField, + requested: Option<&str>, + before_commit: impl FnOnce(), +) -> Result { + edit_declaration(path, expected_identity, field, requested, before_commit) +} + +fn edit_declaration( + path: &Path, + expected_identity: &str, + field: PresentationField, + requested: Option<&str>, + before_commit: impl FnOnce(), +) -> Result { + if path.extension().and_then(|value| value.to_str()) != Some("kdl") { + return Err(AuthorError::new( + "unsupported-declaration-format", + format!( + "presentation authoring requires canonical KDL, found {}", + path.display() + ), + )); + } + let metadata = fs::symlink_metadata(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + if !metadata.file_type().is_file() { + return Err(AuthorError::new( + "unsafe-declaration-path", + format!("refusing non-regular declaration path {}", path.display()), + )); + } + let original = fs::read(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + let original_version = SourceVersion::from_metadata(&metadata); + let text = std::str::from_utf8(&original).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("declaration {} is not UTF-8: {error}", path.display()), + ) + })?; + let document = KdlDocument::parse(text).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("parsing declaration {}: {error}", path.display()), + ) + })?; + let target = exact_agent_node(&document, expected_identity)?; + if is_nix_managed(target) { + return Err(AuthorError::new( + "nix-managed-declaration", + format!( + "agent {expected_identity:?} is Nix-owned; edit its Nix source instead of {}", + path.display() + ), + )); + } + let Some(replacement) = presentation_edit(text, target, field, requested)? else { + return Ok(AuthorOutcome::Unchanged); + }; + verify_candidate(&replacement, expected_identity, field, requested)?; + atomic_replace_checked( + path, + &original, + original_version, + replacement.as_bytes(), + metadata.permissions().mode() & 0o7777, + before_commit, + )?; + Ok(AuthorOutcome::Changed) +} + +fn exact_agent_node<'a>( + document: &'a KdlDocument, + expected_identity: &str, +) -> Result<&'a KdlNode, AuthorError> { + let (expected_host, expected_agent) = expected_identity.split_once('.').ok_or_else(|| { + AuthorError::new( + "invalid-target", + format!("expected host-qualified agent identity, found {expected_identity:?}"), + ) + })?; + let matches = document + .nodes() + .iter() + .filter(|node| { + node.name().value() == "agent" + && agent_identity_parts(node).is_some_and(|(host, identity)| { + identity == expected_agent + && host.as_deref().is_none_or(|host| host == expected_host) + }) + }) + .collect::>(); + match matches.as_slice() { + [target] => Ok(*target), + [] => Err(AuthorError::new( + "target-changed", + format!("declaration no longer contains explicit agent {expected_identity:?}"), + )), + _ => Err(AuthorError::new( + "target-ambiguous", + format!("declaration contains more than one agent {expected_identity:?}"), + )), + } +} + +fn agent_identity_parts(node: &KdlNode) -> Option<(Option, String)> { + let mut identity = node + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned); + let mut host = None; + for child in node.children()?.nodes() { + match child.name().value() { + "identity" => { + identity = child + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned) + .or(identity); + } + "host" => { + host = child + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned); + } + _ => {} + } + } + Some((host, identity?)) +} + +fn is_nix_managed(node: &KdlNode) -> bool { + node.children().is_some_and(|children| { + children + .nodes() + .iter() + .filter(|child| child.name().value() == "meta") + .filter_map(KdlNode::children) + .flat_map(|meta| meta.nodes()) + .filter(|child| child.name().value() == "managed-by") + .any(|child| child.get(0).and_then(|value| value.as_string()) == Some("nix")) + }) +} + +fn presentation_edit( + text: &str, + target: &KdlNode, + field: PresentationField, + requested: Option<&str>, +) -> Result, AuthorError> { + let fields = target + .children() + .into_iter() + .flat_map(|children| children.nodes()) + .filter(|child| child.name().value() == field.as_str()) + .collect::>(); + match fields.as_slice() { + [] => match requested { + Some(value) => insert_field(text, target, field, value).map(Some), + None => Ok(None), + }, + [node] => match requested { + Some(value) => replace_field(text, node, field, value), + None => remove_field(text, node).map(Some), + }, + _ => Err(AuthorError::new( + "duplicate-presentation-field", + format!("target declares `{}` more than once", field.as_str()), + )), + } +} + +fn parse_field_value(node: &KdlNode, field: PresentationField) -> Result<&str, AuthorError> { + if node.children().is_some() || node.entries().len() != 1 || node.entries()[0].name().is_some() + { + return Err(AuthorError::new( + "malformed-presentation-field", + format!( + "`{}` must contain exactly one positional string", + field.as_str() + ), + )); + } + node.get(0) + .and_then(|value| value.as_string()) + .ok_or_else(|| { + AuthorError::new( + "malformed-presentation-field", + format!("`{}` must contain a string", field.as_str()), + ) + }) +} + +fn quoted(value: &str) -> Result { + serde_json::to_string(value).map_err(|error| { + AuthorError::new( + "unsafe-source-edit", + format!("encode presentation string for canonical KDL: {error}"), + ) + }) +} + +fn replace_field( + text: &str, + node: &KdlNode, + field: PresentationField, + value: &str, +) -> Result, AuthorError> { + if parse_field_value(node, field)? == value { + return Ok(None); + } + let entry = &node.entries()[0]; + let span = entry.span(); + let range = span.offset()..span.offset() + span.len(); + text.get(range.clone()).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "presentation value span falls outside the declaration", + ) + })?; + let mut replacement = text.to_owned(); + replacement.replace_range(range, "ed(value)?); + Ok(Some(replacement)) +} + +fn insert_field( + text: &str, + target: &KdlNode, + field: PresentationField, + value: &str, +) -> Result { + let span = target.span(); + let start = span.offset(); + let end = start + span.len(); + let source = text.get(start..end).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "agent span falls outside the declaration", + ) + })?; + let authored = format!("{} {}", field.as_str(), quoted(value)?); + let mut replacement = text.to_owned(); + if target.children().is_none() { + replacement.insert_str(end, &format!(" {{ {authored} }}")); + return Ok(replacement); + } + if !source.ends_with('}') { + return Err(AuthorError::new( + "unsafe-source-shape", + "agent child block does not end at a source-preserving insertion point", + )); + } + let close = source.len() - 1; + if let Some(newline) = source[..close].rfind('\n') { + let closing_indent = &source[newline + 1..close]; + if !closing_indent + .chars() + .all(|value| matches!(value, ' ' | '\t')) + { + return Err(AuthorError::new( + "unsafe-source-shape", + "cannot preserve a non-whitespace closing-brace prefix", + )); + } + let child_indent = target + .children() + .and_then(|children| children.nodes().first()) + .and_then(|child| line_indent(text, child.span().offset())) + .unwrap_or_else(|| format!("{closing_indent} ")); + replacement.insert_str(start + newline + 1, &format!("{child_indent}{authored}\n")); + return Ok(replacement); + } + let before_close = &source[..close]; + let trimmed = before_close.trim_end(); + let insertion = if trimmed.ends_with('{') { + format!(" {authored}") + } else if trimmed.ends_with(';') { + format!(" {authored};") + } else { + format!("; {authored}") + }; + replacement.insert_str(start + trimmed.len(), &insertion); + Ok(replacement) +} + +fn remove_field(text: &str, node: &KdlNode) -> Result { + let span = node.span(); + let start = span.offset(); + let end = start + span.len(); + text.get(start..end).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "presentation field span falls outside the declaration", + ) + })?; + let line_start = text[..start].rfind('\n').map_or(0, |newline| newline + 1); + let line_end = text[end..] + .find('\n') + .map_or(text.len(), |newline| end + newline); + if text[line_start..start] + .chars() + .all(|value| matches!(value, ' ' | '\t')) + && text[end..line_end] + .chars() + .all(|value| matches!(value, ' ' | '\t')) + { + let mut replacement = text.to_owned(); + let remove_end = usize::min(line_end + usize::from(line_end < text.len()), text.len()); + replacement.replace_range(line_start..remove_end, ""); + return Ok(replacement); + } + Err(AuthorError::new( + "unsafe-source-shape", + "clearing presentation metadata requires the field to occupy its own canonical KDL line", + )) +} + +fn line_indent(text: &str, offset: usize) -> Option { + let prefix = text.get(..offset)?; + let start = prefix.rfind('\n').map_or(0, |newline| newline + 1); + let indent = prefix.get(start..)?; + indent + .chars() + .all(|value| matches!(value, ' ' | '\t')) + .then(|| indent.to_owned()) +} + +fn verify_candidate( + candidate: &str, + expected_identity: &str, + field: PresentationField, + expected: Option<&str>, +) -> Result<(), AuthorError> { + let document = KdlDocument::parse(candidate).map_err(|error| { + AuthorError::new( + "unsafe-source-edit", + format!("presentation edit did not produce valid KDL: {error}"), + ) + })?; + let target = exact_agent_node(&document, expected_identity)?; + let fields = target + .children() + .into_iter() + .flat_map(|children| children.nodes()) + .filter(|child| child.name().value() == field.as_str()) + .collect::>(); + let observed = match fields.as_slice() { + [] => None, + [node] => Some(parse_field_value(node, field)?), + _ => { + return Err(AuthorError::new( + "unsafe-source-edit", + format!( + "presentation edit produced duplicate `{}` fields", + field.as_str() + ), + )); + } + }; + if observed != expected { + return Err(AuthorError::new( + "unsafe-source-edit", + format!( + "presentation edit did not produce the requested `{}`", + field.as_str() + ), + )); + } + Ok(()) +} + +fn atomic_replace_checked( + path: &Path, + original: &[u8], + original_version: SourceVersion, + replacement: &[u8], + mode: u32, + before_commit: impl FnOnce(), +) -> Result<(), AuthorError> { + let directory = path.parent().ok_or_else(|| { + AuthorError::new( + "invalid-target", + format!("declaration path {} has no parent", path.display()), + ) + })?; + let temporary = directory.join(format!( + ".agent.kdl.presentation-{}-{}", + std::process::id(), + TMP_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let write = (|| -> std::io::Result<()> { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(mode) + .open(&temporary)?; + file.write_all(replacement)?; + file.sync_all() + })(); + if let Err(error) = write { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "declaration-write-failed", + format!("staging declaration {}: {error}", path.display()), + )); + } + before_commit(); + let current = fs::symlink_metadata(path) + .ok() + .filter(|metadata| metadata.file_type().is_file()) + .map(|metadata| (SourceVersion::from_metadata(&metadata), fs::read(path).ok())); + if !matches!(current, Some((version, Some(bytes))) if version == original_version && bytes == original) + { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "source-changed", + format!( + "declaration {} changed while presentation was authored", + path.display() + ), + )); + } + if let Err(error) = fs::rename(&temporary, path) { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "declaration-write-failed", + format!( + "atomically publishing declaration {}: {error}", + path.display() + ), + )); + } + fs::File::open(directory) + .and_then(|directory| directory.sync_all()) + .map_err(|error| { + AuthorError::new( + "declaration-write-failed", + format!( + "syncing declaration directory {}: {error}", + directory.display() + ), + ) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write(root: &Path, relative: &str, contents: &str) -> PathBuf { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, contents).unwrap(); + path + } + + fn declaration( + identity: &str, + host: &str, + supervisor: Option<&str>, + managed_by: &str, + ) -> String { + let supervisor = supervisor + .map(|value| format!(" supervisor {value:?}\n")) + .unwrap_or_default(); + format!( + "// keep this comment\nagent {identity:?} {{\n host {host:?}\n meta {{ managed-by {managed_by:?}; keep \"exact\" }}\n{supervisor} command \"sleep 60\"\n}}\n" + ) + } + + #[test] + fn source_preserving_set_replace_idempotent_and_clear() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + let before = fs::read_to_string(&path).unwrap(); + + let set = set_presentation( + root, + "h.worker", + "h", + None, + PresentationField::Name, + Some("Build owner"), + ) + .unwrap(); + assert_eq!(set.result, AuthorOutcome::Changed); + let after_set = fs::read_to_string(&path).unwrap(); + assert_eq!(after_set.matches("name \"Build owner\"").count(), 1); + assert_eq!(after_set.replace(" name \"Build owner\"\n", ""), before); + + assert_eq!( + set_presentation( + root, + "worker", + "h", + None, + PresentationField::Name, + Some("Build owner") + ) + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!( + set_presentation( + root, + "worker", + "h", + None, + PresentationField::Name, + Some("Release owner") + ) + .unwrap() + .result, + AuthorOutcome::Changed + ); + assert_eq!( + set_presentation(root, "worker", "h", None, PresentationField::Name, None) + .unwrap() + .result, + AuthorOutcome::Changed + ); + assert_eq!(fs::read_to_string(path).unwrap(), before); + } + + #[test] + fn self_and_supervisor_can_edit_but_sibling_and_nix_owner_cannot() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/root/agent.kdl", + &declaration("root", "h", None, "catalog"), + ); + write( + root, + "h/child/agent.kdl", + &declaration("child", "h", Some("root"), "catalog"), + ); + write( + root, + "h/sibling/agent.kdl", + &declaration("sibling", "h", Some("root"), "catalog"), + ); + write( + root, + "h/nix/agent.kdl", + &declaration("nix", "h", Some("root"), "nix"), + ); + + set_presentation( + root, + "h.child", + "h", + Some("h.child"), + PresentationField::Name, + Some("self"), + ) + .unwrap(); + set_presentation( + root, + "h.child", + "h", + Some("h.root"), + PresentationField::Description, + Some("supervised"), + ) + .unwrap(); + assert_eq!( + set_presentation( + root, + "h.sibling", + "h", + Some("h.child"), + PresentationField::Name, + Some("no") + ) + .unwrap_err() + .code(), + "presentation-not-authorized" + ); + assert_eq!( + set_presentation( + root, + "h.nix", + "h", + Some("h.root"), + PresentationField::Name, + Some("no") + ) + .unwrap_err() + .code(), + "nix-managed-declaration" + ); + } + + #[test] + fn stale_source_refuses_atomic_replace() { + let temporary = tempfile::tempdir().unwrap(); + let path = write( + temporary.path(), + "agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + let changed = declaration("worker", "h", None, "external"); + let error = edit_declaration_for_test( + &path, + "h.worker", + PresentationField::Name, + Some("Owner"), + || fs::write(&path, &changed).unwrap(), + ) + .unwrap_err(); + assert_eq!(error.code(), "source-changed"); + assert_eq!(fs::read_to_string(path).unwrap(), changed); + } + + #[test] + fn source_version_rejects_byte_identical_aba_rewrite() { + let temporary = tempfile::tempdir().unwrap(); + let original = declaration("worker", "h", None, "catalog"); + let path = write(temporary.path(), "agent.kdl", &original); + let error = edit_declaration_for_test( + &path, + "h.worker", + PresentationField::Name, + Some("Owner"), + || { + fs::write(&path, "temporary competing bytes").unwrap(); + fs::write(&path, &original).unwrap(); + }, + ) + .unwrap_err(); + assert_eq!(error.code(), "source-changed"); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } +} diff --git a/src/agents.rs b/src/agents.rs index 31d161c2..48e8e647 100644 --- a/src/agents.rs +++ b/src/agents.rs @@ -19,8 +19,10 @@ pub struct AgentRow { pub identity: String, /// Effective presence (derived: stale → `unknown`, etc.). pub status: State, - /// Optional display name (`/name`), else `None`. + /// Optional display name from the Agent Spec declaration. pub name: Option, + /// Optional enduring responsibility boundary from the Agent Spec declaration. + pub description: Option, /// Whether the declaration is explicitly retired. Presence remains a separate runtime signal. pub retired: bool, /// Typed Resource bindings declared directly by the agent. @@ -44,7 +46,8 @@ pub fn roster(catalog_root: &Path, this_host: &str) -> Vec { Some(AgentRow { identity: s.bus_id(this_host), status: status::read_state(&status::status_path(agent_dir)), - name: read_name(agent_dir), + name: s.name.clone(), + description: s.description.clone(), retired: s.retired, resources: s.resources.clone(), last_activity_ms: newest_mtime_ms(agent_dir), @@ -62,6 +65,7 @@ struct SummaryJson<'a> { identity: &'a str, status: &'a str, name: Option<&'a str>, + description: Option<&'a str>, retired: bool, resources: &'a [Resource], } @@ -72,6 +76,7 @@ struct EnrichedJson<'a> { identity: &'a str, status: &'a str, name: Option<&'a str>, + description: Option<&'a str>, retired: bool, resources: &'a [Resource], #[serde(rename = "lastActivity")] @@ -88,6 +93,7 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { identity: &r.identity, status: r.status.as_str(), name: r.name.as_deref(), + description: r.description.as_deref(), retired: r.retired, resources: &r.resources, last_activity: r.last_activity_ms, @@ -102,6 +108,7 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { identity: &r.identity, status: r.status.as_str(), name: r.name.as_deref(), + description: r.description.as_deref(), retired: r.retired, resources: &r.resources, }) @@ -110,13 +117,6 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { } } -/// `/name` first line, if non-empty. -fn read_name(agent_dir: &Path) -> Option { - let raw = fs::read_to_string(agent_dir.join("name")).ok()?; - let first = raw.lines().next().unwrap_or("").trim(); - (!first.is_empty()).then(|| first.to_string()) -} - /// Count logically unread messages in the agent's `resources/inbox`. A same-filename archive receipt /// suppresses and cleans a raw inbox duplicate restored by eventually-consistent sync. fn inbox_count(agent_dir: &Path) -> usize { @@ -169,6 +169,7 @@ mod tests { identity: identity.to_string(), status, name: name.map(str::to_string), + description: None, retired, resources: Vec::new(), last_activity_ms: last, @@ -193,11 +194,11 @@ mod tests { assert_eq!( to_json(&rows, false), - r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"retired":false,"resources":[]},{"identity":"hetz.st2-claude","status":"busy","name":"owner","retired":true,"resources":[]}]"# + r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"description":null,"retired":false,"resources":[]},{"identity":"hetz.st2-claude","status":"busy","name":"owner","description":null,"retired":true,"resources":[]}]"# ); assert_eq!( to_json(&rows, true), - r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"retired":false,"resources":[],"lastActivity":1784653027733.6138,"inbox":1},{"identity":"hetz.st2-claude","status":"busy","name":"owner","retired":true,"resources":[],"lastActivity":null,"inbox":0}]"# + r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"description":null,"retired":false,"resources":[],"lastActivity":1784653027733.6138,"inbox":1},{"identity":"hetz.st2-claude","status":"busy","name":"owner","description":null,"retired":true,"resources":[],"lastActivity":null,"inbox":0}]"# ); // Empty roster is `[]`, not `null`. assert_eq!(to_json(&[], true), "[]"); @@ -224,7 +225,7 @@ mod tests { assert_eq!( to_json(&[resource_row], false), - r#"[{"identity":"hetz.worker","status":"available","name":null,"retired":false,"resources":[{"name":"work","_tag":"vendor-specific-type","uri":"vendor+thing://authority/exact%20identity"}]}]"# + r#"[{"identity":"hetz.worker","status":"available","name":null,"description":null,"retired":false,"resources":[{"name":"work","_tag":"vendor-specific-type","uri":"vendor+thing://authority/exact%20identity"}]}]"# ); } } diff --git a/src/cutover_admission.rs b/src/cutover_admission.rs index 79d751ef..47063e54 100644 --- a/src/cutover_admission.rs +++ b/src/cutover_admission.rs @@ -3689,6 +3689,8 @@ mod tests { }; let spec = agent_spec::spec::AgentSpec { identity: "worker".to_owned(), + name: None, + description: None, host: Some("test-host".to_owned()), role: None, job_type: agent_spec::spec::JobType::Service, diff --git a/src/ding_reconcile.rs b/src/ding_reconcile.rs index 9d5348bc..3c08b046 100644 --- a/src/ding_reconcile.rs +++ b/src/ding_reconcile.rs @@ -212,6 +212,7 @@ impl DingExecBackend for SystemDingExecBackend { tags: BTreeMap::new(), env: desired.canonical_env.clone(), keep: false, + presentation: None, }; let generation = self .inner diff --git a/src/eval_run.rs b/src/eval_run.rs index e2259b0f..0fadd3e6 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -98,6 +98,8 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec } AgentSpec { identity: a.id.clone(), + name: a.name.clone(), + description: a.description.clone(), host: Some(host.to_string()), role: None, job_type: JobType::Service, diff --git a/src/eval_spec.rs b/src/eval_spec.rs index 3873075b..e47ccb2f 100644 --- a/src/eval_spec.rs +++ b/src/eval_spec.rs @@ -15,7 +15,10 @@ use std::time::Duration; use kdl::{KdlDocument, KdlNode, KdlValue}; -use agent_spec::spec::{Restart, RestartMode, parse_duration}; +use agent_spec::spec::{ + AGENT_DESCRIPTION_MAX_CHARS, AGENT_NAME_MAX_CHARS, Restart, RestartMode, parse_duration, + validate_presentation, +}; /// A parsed st2 spec: a base team (`st2 up` boots this) plus an optional `eval` (`st2 eval` runs it). #[derive(Debug, Clone, PartialEq, Eq)] @@ -38,6 +41,8 @@ pub struct Spec { #[derive(Debug, Clone, PartialEq, Eq)] pub struct SpecAgent { pub id: String, + pub name: Option, + pub description: Option, pub workspace: Option, /// This agent's supervisor (the id its crash escalates to). The chain of `supervisor` fields is /// walked to the root (the root's is `None` — that is the cos) for crash-ding escalation. @@ -389,6 +394,8 @@ fn parse_agent(node: &KdlNode, prefix: &str, parent_env: &BTreeMap {} + "name" => display_name = arg(c), + "description" => description = arg(c), "workspace" => workspace = arg(c), "supervisor" => supervisor = arg(c), "command" => command = arg(c), @@ -456,13 +465,21 @@ fn parse_agent(node: &KdlNode, prefix: &str, parent_env: &BTreeMap anyhow::bail!( - "agent '{id}': unexpected node '{other}' (expected workspace|supervisor|env|command|ding|exec)" + "agent '{id}': unexpected node '{other}' (expected name|description|workspace|supervisor|env|command|ding|exec)" ), } } + validate_presentation("name", display_name.as_deref(), AGENT_NAME_MAX_CHARS)?; + validate_presentation( + "description", + description.as_deref(), + AGENT_DESCRIPTION_MAX_CHARS, + )?; let env = cascade(parent_env, &agent_env); return Ok(SpecAgent { id: id.clone(), + name: display_name, + description, workspace, supervisor, env, diff --git a/src/exec_backend.rs b/src/exec_backend.rs index a321f984..e88dfb51 100644 --- a/src/exec_backend.rs +++ b/src/exec_backend.rs @@ -1274,6 +1274,7 @@ mod generation_tests { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } diff --git a/src/lib.rs b/src/lib.rs index 10b9a9a5..4a17a8f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ //! declared task running and delivers native messages. Harness-specific behavior stays explicit in //! each declaration's command, environment, hooks, and workspace materialization block. +pub mod agent_author; pub mod agent_publish; pub mod agents; pub mod catalog; @@ -51,7 +52,9 @@ pub use exec_backend::ExecBackend; pub use expand::{expand_env, expand_vars}; pub use flapping::FlappingCap; pub use host_lock::{HostLock, HostOwnership}; -pub use reconcile::{Launch, ReconcilePlan, Session, TaskLaunch, TaskTarget, Teardown, reconcile}; +pub use reconcile::{ + Launch, PtyPresentation, ReconcilePlan, Session, TaskLaunch, TaskTarget, Teardown, reconcile, +}; pub use run::{ PtyCli, Runner, SystemRunner, UpReport, detect_host, down, down_specs, exec_state_dir, execute, up_loop, up_loop_specs, up_loop_with_ownership, up_once, up_once_selected, diff --git a/src/main.rs b/src/main.rs index d5f47759..0278e5f5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -206,6 +206,10 @@ enum Command { #[command(flatten)] ctx: MsgCtx, }, + /// Set or clear an agent's human-facing name without changing stable identity. + Rename(PresentationArgs), + /// Set or clear an agent's enduring responsibility description. + Describe(PresentationArgs), /// Transactionally publish one canonical Agent Spec into the live catalog. #[command(subcommand)] Agent(AgentCmd), @@ -420,6 +424,28 @@ enum AgentCmd { }, } +#[derive(Args)] +struct PresentationArgs { + /// Exact bus identity, or a bare stable identity only when unique in the selected catalog. + identity: String, + /// Presentation text. Use --clear to remove the field. + #[arg( + value_name = "TEXT", + required_unless_present = "clear", + conflicts_with = "clear" + )] + value: Option, + /// Remove the optional field. + #[arg(long)] + clear: bool, + /// Emit a stable JSON receipt or classified refusal. + #[arg(long)] + json: bool, + /// Host used only to resolve declarations whose host is omitted. + #[arg(long)] + host: Option, +} + #[derive(Subcommand)] enum ExecCmd { /// Prepare or apply an immutable exact-retirement capability. @@ -835,6 +861,10 @@ fn main() -> Result<()> { interval, } => ding_cmd(session, identity, root, host, interval), Command::Status { identity, set, ctx } => status_cmd(identity, set, ctx), + Command::Rename(args) => presentation_cmd(st2::agent_author::PresentationField::Name, args), + Command::Describe(args) => { + presentation_cmd(st2::agent_author::PresentationField::Description, args) + } Command::Cutover(cmd) => cutover_cmd(cmd, catalog_path.as_deref()), Command::Agent(AgentCmd::Publish { spec, @@ -1131,6 +1161,69 @@ fn main() -> Result<()> { } } +fn presentation_cmd( + field: st2::agent_author::PresentationField, + args: PresentationArgs, +) -> Result<()> { + let PresentationArgs { + identity, + value, + clear, + json, + host, + } = args; + let root = catalog_arg(None)?; + let host = host.unwrap_or_else(detect_host); + let actor = std::env::var("ST_AGENT") + .ok() + .filter(|value| !value.is_empty()); + let requested = if clear { None } else { value.as_deref() }; + match st2::agent_author::set_presentation( + &root, + &identity, + &host, + actor.as_deref(), + field, + requested, + ) { + Ok(receipt) => { + if json { + println!("{}", serde_json::to_string(&receipt)?); + } else { + let state = match (receipt.result, receipt.value.as_deref()) { + (st2::agent_author::AuthorOutcome::Changed, Some(value)) => { + format!("set to {value:?}") + } + (st2::agent_author::AuthorOutcome::Changed, None) => "cleared".to_owned(), + (st2::agent_author::AuthorOutcome::Unchanged, Some(value)) => { + format!("already {value:?}") + } + (st2::agent_author::AuthorOutcome::Unchanged, None) => { + "already clear".to_owned() + } + }; + println!("{} {}: {state}", receipt.identity, field.as_str()); + } + Ok(()) + } + Err(error) => { + if json { + println!( + "{}", + serde_json::json!({ + "result": "error", + "code": error.code(), + "identity": identity, + "field": field, + "error": error.to_string(), + }) + ); + } + Err(error.into()) + } + } +} + fn require_exec_retirement_catalog( catalog: Option, json: bool, @@ -1992,10 +2085,11 @@ fn agents_cmd( for r in &rows { let retired = if r.retired { "\t[retired]" } else { "" }; println!( - "{}\t{}\t{}{}", + "{}\t{}\t{}\t{}{}", r.identity, r.status.as_str(), r.name.as_deref().unwrap_or(""), + r.description.as_deref().unwrap_or(""), retired, ); } diff --git a/src/reconcile.rs b/src/reconcile.rs index f69c3953..2c7b1206 100644 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -47,6 +47,47 @@ pub struct TaskTarget { pub env: BTreeMap, /// GC pin (task-level `keep`, or the agent-level `keep`). pub keep: bool, + /// Desired PTY-only presentation projected at spawn. Exec tasks carry `None`. + pub presentation: Option, +} + +/// Exact, non-lifecycle metadata desired for one managed PTY. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PtyPresentation { + /// Exact stable PTY task ID. Automation must never resolve this as a display alias. + pub pty_id: String, + /// `Some(value)` updates primary-agent display metadata; `Some(None)` clears it. `None` preserves + /// a secondary task's existing task-specific display convention. + pub display_name: Option>, + /// Complete st2-owned tag snapshot. `None` removes an optional owned key. + pub tags: BTreeMap>, +} + +pub const AGENT_PRESENTATION_SCHEMA_TAG: &str = "agent.presentation.schema"; +pub const AGENT_ACTOR_PATH_TAG: &str = "agent.actor.path"; +pub const AGENT_DESCRIPTION_TAG: &str = "agent.presentation.description"; + +fn pty_presentation( + spec: &AgentSpec, + task: &crate::spec::Task, + pty_id: &str, + bus_id: &str, +) -> Option { + if task.kind != TaskKind::Pty { + return None; + } + Some(PtyPresentation { + pty_id: pty_id.to_owned(), + display_name: (task.name == "agent").then(|| spec.name.clone()), + tags: BTreeMap::from([ + ( + AGENT_PRESENTATION_SCHEMA_TAG.to_owned(), + Some("1".to_owned()), + ), + (AGENT_ACTOR_PATH_TAG.to_owned(), Some(bus_id.to_owned())), + (AGENT_DESCRIPTION_TAG.to_owned(), spec.description.clone()), + ]), + }) } /// A resolved task launch accepted by the execution backends. @@ -89,6 +130,8 @@ pub struct ReconcilePlan<'a> { pub gc: Vec, /// Dead or absent `adopt-only` task ids held without reap or launch. pub held: Vec, + /// In-place presentation updates for healthy managed PTYs, independent of lifecycle actions. + pub presentation: Vec, } /// Resolve one exact local task selector (`host.agent.task` or explicit task id) without mutation. @@ -164,7 +207,7 @@ pub fn reconcile_selected<'a>( let target = TaskTarget { kind: task.kind, pty_id: runtime.clone(), - bus_id, + bus_id: bus_id.clone(), name: task.name.clone(), launch, cwd: task.cwd.clone(), @@ -172,9 +215,15 @@ pub fn reconcile_selected<'a>( tags: task.tags.clone(), env, keep: task.keep || owner.keep, + presentation: pty_presentation(owner, task, &runtime, &bus_id), }; match actual { - Some(s) if s.alive => plan.adopt.push(owner), + Some(s) if s.alive => { + if let Some(presentation) = target.presentation.clone() { + plan.presentation.push(presentation); + } + plan.adopt.push(owner); + } _ if task.lifecycle == TaskLifecycle::AdoptOnly => plan.held.push(runtime), Some(_) if target.keep => plan.adopt.push(owner), Some(_) => { @@ -280,10 +329,11 @@ pub fn reconcile<'a>( } else { env.remove("ST_SUPERVISOR"); } + let pty_id = resolve_task_id(&bus_id, &t.name, t.id.as_deref()); Some(( TaskTarget { kind: t.kind, - pty_id: resolve_task_id(&bus_id, &t.name, t.id.as_deref()), + pty_id: pty_id.clone(), bus_id: bus_id.clone(), name: t.name.clone(), launch, @@ -292,6 +342,7 @@ pub fn reconcile<'a>( tags: t.tags.clone(), env, keep: t.keep || spec.keep, + presentation: pty_presentation(spec, t, &pty_id, &bus_id), }, t.lifecycle, )) @@ -304,7 +355,11 @@ pub fn reconcile<'a>( let held_before = plan.held.len(); for (target, lifecycle) in targets { match session_state(&by_id, &target.pty_id) { - SessionState::Alive => {} + SessionState::Alive => { + if let Some(presentation) = target.presentation.clone() { + plan.presentation.push(presentation); + } + } SessionState::Dead | SessionState::Absent if lifecycle == TaskLifecycle::AdoptOnly => { diff --git a/src/run.rs b/src/run.rs index 34c99bcf..fa868729 100644 --- a/src/run.rs +++ b/src/run.rs @@ -13,7 +13,7 @@ use std::cell::RefCell; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::ffi::OsString; -use std::io::{Read as _, Seek as _}; +use std::io::{Read as _, Seek as _, Write as _}; use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt as _}; use std::os::unix::process::CommandExt as _; use std::path::{Path, PathBuf}; @@ -31,7 +31,7 @@ use crate::exec_backend::ExecBackend; use crate::flapping::FlappingCap; use crate::host_lock::HostOwnership; use crate::message; -use crate::reconcile::{ReconcilePlan, Session, TaskLaunch, TaskTarget}; +use crate::reconcile::{PtyPresentation, ReconcilePlan, Session, TaskLaunch, TaskTarget}; use crate::task_inventory::{ DesiredRuntime, ObservationBatch, ObservedState, RuntimeGeneration, RuntimeObservation, RuntimeObserver, generation_id, @@ -47,10 +47,22 @@ const OBSERVED_PROCESS_PID_TAG: &str = "st2.observation.process.pid"; /// descendant that inherited stdout/stderr from blocking cleanup after the direct child times out. /// The child still gets a fresh process group so the common wrapper-and-descendants case is reaped. fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Result { + output_with_input_timeout(command, timeout, None) +} + +fn output_with_input_timeout( + command: &mut Command, + timeout: Duration, + input: Option<&[u8]>, +) -> anyhow::Result { let mut stdout = tempfile::tempfile()?; let mut stderr = tempfile::tempfile()?; command - .stdin(Stdio::null()) + .stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) .stdout(Stdio::from(stdout.try_clone()?)) .stderr(Stdio::from(stderr.try_clone()?)); unsafe { @@ -63,6 +75,13 @@ fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Resu }); } let mut child = command.spawn()?; + if let Some(input) = input { + child + .stdin + .take() + .context("metadata patch child has no piped stdin")? + .write_all(input)?; + } let pid = child.id() as i32; let deadline = Instant::now() + timeout; let status = loop { @@ -118,6 +137,11 @@ pub trait Runner { /// Spawn `target` in the background from its explicit launch. `spec_dir` is the spec file's /// directory — part of the cwd fallback chain (task.cwd → workspace → spec dir). fn spawn(&self, target: &TaskTarget, spec_dir: &Path) -> anyhow::Result<()>; + /// Atomically reconcile display metadata and the complete st2-owned tag snapshot for one exact + /// existing PTY ID. The default is a no-op for non-PTY test/backends. + fn patch_presentation(&self, _presentation: &PtyPresentation) -> anyhow::Result<()> { + Ok(()) + } /// SIGTERM a running session. fn kill(&self, pty_id: &str) -> anyhow::Result<()>; /// Reap an exited session before restarting it. Backends may preserve bounded diagnostics here. @@ -188,6 +212,13 @@ struct PtyListEntry { tags: BTreeMap, } +#[derive(Serialize)] +struct PtyMetadataPatch<'a> { + #[serde(rename = "displayName", skip_serializing_if = "Option::is_none")] + display_name: Option<&'a Option>, + tags: &'a BTreeMap>, +} + #[derive(Debug, Deserialize)] struct PtyStatsEntry { name: String, @@ -291,17 +322,40 @@ impl PtyCli { .arg("-d") // detached: leave it running in the background .arg("--force") // st2 itself may run inside a pty session; allow nesting .args(["--id", &target.pty_id]); - // Keep the adoption key task-specific, but make a differing human-facing label the owning - // agent's stable bus identity instead of pty's auto-derived `-sh` label. When the - // lifecycle id already IS that identity, suppress pty's automatic `-sh` alias: pty - // rejects displayName == id, and no displayName makes the UI fall back to the stable id. - if target.pty_id == target.bus_id { - cmd.arg("--no-display-name"); - } else { - cmd.args(["--name", &target.bus_id]); + match target + .presentation + .as_ref() + .map(|presentation| &presentation.display_name) + { + Some(Some(Some(name))) => { + cmd.args(["--name", name]); + } + Some(Some(None)) => { + cmd.arg("--no-display-name"); + } + // Secondary tasks retain the established task-specific presentation convention. + _ if target.pty_id == target.bus_id => { + cmd.arg("--no-display-name"); + } + _ => { + cmd.args(["--name", &target.bus_id]); + } } cmd.arg("--cwd").arg(&cwd); - for (k, v) in &target.tags { + let mut tags = target.tags.clone(); + if let Some(presentation) = &target.presentation { + for (key, value) in &presentation.tags { + match value { + Some(value) => { + tags.insert(key.clone(), value.clone()); + } + None => { + tags.remove(key); + } + } + } + } + for (k, v) in &tags { cmd.arg("--tag").arg(format!("{k}={}", self.expand(v))); } // Managed agent and DING sessions retain PTY exit evidence until the lifecycle owner records @@ -463,6 +517,29 @@ impl PtyCli { self.list_entries_at(&effective_pty_root(&self.catalog_root)) } + fn patch_presentation(&self, presentation: &PtyPresentation) -> anyhow::Result<()> { + let payload = serde_json::to_vec(&PtyMetadataPatch { + display_name: presentation.display_name.as_ref(), + tags: &presentation.tags, + })?; + let out = output_with_input_timeout( + Command::new(&self.bin) + .args(["metadata", "patch", "--id", &presentation.pty_id]) + .env("PTY_ROOT", effective_pty_root(&self.catalog_root)), + PTY_LIST_TIMEOUT, + Some(&payload), + ) + .map_err(|error| anyhow::anyhow!("`pty metadata patch --id` failed: {error}"))?; + if !out.status.success() { + anyhow::bail!( + "`pty metadata patch --id {}` failed: {}", + presentation.pty_id, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(()) + } + fn list_entries_at(&self, root: &Path) -> anyhow::Result> { let out = output_with_timeout( Command::new(&self.bin) @@ -608,6 +685,10 @@ impl Runner for PtyCli { anyhow::bail!("spawning pty '{}' failed: {last_err}", target.pty_id); } + fn patch_presentation(&self, presentation: &PtyPresentation) -> anyhow::Result<()> { + PtyCli::patch_presentation(self, presentation) + } + fn kill(&self, pty_id: &str) -> anyhow::Result<()> { let out = Command::new(&self.bin) .arg("kill") @@ -796,6 +877,10 @@ impl Runner for SystemRunner { } } + fn patch_presentation(&self, presentation: &PtyPresentation) -> anyhow::Result<()> { + self.pty.patch_presentation(presentation) + } + fn kill(&self, pty_id: &str) -> anyhow::Result<()> { match self.index.borrow().get(pty_id) { Some(TaskKind::Exec) => self.exec.kill(pty_id), @@ -962,6 +1047,15 @@ pub fn execute( )); return; } + // Presentation is an independent metadata path. A failure is visible and retried by the next + // reconcile pass, but never authorizes stop, reap, restart, or replacement. + for presentation in &plan.presentation { + if let Err(error) = runner.patch_presentation(presentation) { + report + .errors + .push(format!("metadata patch {}: {error}", presentation.pty_id)); + } + } // The corpses tied to a launch target (dead, non-keep, active ptys) are reaped inside the launch // loop so a parked flapper keeps its evidence. Everything else in `gc` (e.g. a retired agent's // dead sessions) is reaped here. @@ -3691,6 +3785,7 @@ printf '%s\n' '[{"name":"custom-provider-runtime","process":{"alive":true,"pid": tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } @@ -3724,6 +3819,8 @@ printf '%s\n' '[{"name":"custom-provider-runtime","process":{"alive":true,"pid": let admission = RuntimeMutationAdmission::ordinary(&ownership).unwrap(); let spec = AgentSpec { identity: "codex".into(), + name: None, + description: None, host: None, role: None, job_type: JobType::Service, @@ -3816,6 +3913,8 @@ printf '%s\n' '[{"name":"custom-provider-runtime","process":{"alive":true,"pid": fn spec_fixture() -> AgentSpec { AgentSpec { identity: "demo".into(), + name: None, + description: None, host: Some("hetz".into()), role: None, job_type: JobType::Service, @@ -4041,7 +4140,7 @@ printf '%s\n' '[{"name":"custom-provider-runtime","process":{"alive":true,"pid": } /// The built `pty run` argv runs the command verbatim under `sh -c`, detached, with the pinned id - /// and the owning agent's bus identity as its human-facing name. + /// and the established fallback presentation when no Agent Spec name is projected. #[test] fn build_run_command_wraps_command_in_sh_c() { let cli = PtyCli::default(); @@ -4074,6 +4173,92 @@ printf '%s\n' '[{"name":"custom-provider-runtime","process":{"alive":true,"pid": ); } + #[test] + fn build_run_command_projects_primary_name_and_owned_tags_at_spawn() { + let cli = PtyCli::default(); + let mut t = target("hetz.demo", "codex"); + t.bus_id = "hetz.demo".to_owned(); + t.tags.insert("unrelated".to_owned(), "preserved".to_owned()); + t.presentation = Some(PtyPresentation { + pty_id: "hetz.demo".to_owned(), + display_name: Some(Some("Build owner".to_owned())), + tags: BTreeMap::from([ + ( + "agent.presentation.schema".to_owned(), + Some("1".to_owned()), + ), + ( + "agent.actor.path".to_owned(), + Some("hetz.demo".to_owned()), + ), + ("agent.presentation.description".to_owned(), None), + ]), + }); + let cmd = cli.build_run_command(&t, Path::new("/cat/hetz/demo")); + let args = cmd + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + + let name = args.iter().position(|arg| arg == "--name").unwrap(); + assert_eq!(args[name + 1], "Build owner"); + let tags = args + .windows(2) + .filter(|pair| pair[0] == "--tag") + .map(|pair| pair[1].as_str()) + .collect::>(); + assert!(tags.contains("unrelated=preserved")); + assert!(tags.contains("agent.presentation.schema=1")); + assert!(tags.contains("agent.actor.path=hetz.demo")); + assert!(!tags.iter().any(|tag| tag.starts_with("agent.presentation.description="))); + } + + #[test] + fn metadata_patch_uses_exact_id_and_one_json_stdin_payload() { + use std::os::unix::fs::PermissionsExt as _; + + let temporary = tempfile::tempdir().unwrap(); + let executable = temporary.path().join("pty-capture"); + std::fs::write( + &executable, + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$0.args\"\ncat > \"$0.stdin\"\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + let cli = PtyCli { + bin: executable.display().to_string(), + catalog_root: temporary.path().to_path_buf(), + }; + let presentation = PtyPresentation { + pty_id: "stable.agent.id".to_owned(), + display_name: Some(None), + tags: BTreeMap::from([ + ( + "agent.presentation.schema".to_owned(), + Some("1".to_owned()), + ), + ("agent.presentation.description".to_owned(), None), + ]), + }; + + cli.patch_presentation(&presentation).unwrap(); + + assert_eq!( + std::fs::read_to_string(executable.with_extension("args")).unwrap(), + "metadata\npatch\n--id\nstable.agent.id\n" + ); + let payload: serde_json::Value = serde_json::from_slice( + &std::fs::read(executable.with_extension("stdin")).unwrap(), + ) + .unwrap(); + assert_eq!(payload["displayName"], serde_json::Value::Null); + assert_eq!(payload["tags"]["agent.presentation.schema"], "1"); + assert_eq!( + payload["tags"]["agent.presentation.description"], + serde_json::Value::Null + ); + } + #[test] fn build_run_command_passes_direct_argv_without_a_shell() { let cli = PtyCli::new(PathBuf::from("/my/catalog")); diff --git a/tests/agent_presentation.rs b/tests/agent_presentation.rs new file mode 100644 index 00000000..58949bc2 --- /dev/null +++ b/tests/agent_presentation.rs @@ -0,0 +1,186 @@ +use std::fs; +use std::path::Path; +use std::process::Command; + +fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); +} + +fn declaration(identity: &str, supervisor: Option<&str>, managed_by: &str) -> String { + let supervisor = supervisor + .map(|value| format!(" supervisor {value:?}\n")) + .unwrap_or_default(); + format!( + "// unrelated comment\nagent {identity:?} {{\n host \"h\"\n meta {{ managed-by {managed_by:?}; keep \"unchanged\" }}\n{supervisor} command \"sleep 300\"\n}}\n" + ) +} + +fn run(root: &Path, command: &str, args: &[&str], actor: Option<&str>) -> std::process::Output { + let mut process = Command::new(env!("CARGO_BIN_EXE_st2")); + process + .args(["--catalog", root.to_str().unwrap(), command]) + .args(args) + .env_remove("ST_AGENT"); + if let Some(actor) = actor { + process.env("ST_AGENT", actor); + } + process.output().unwrap() +} + +#[test] +fn cli_sets_replaces_and_clears_fields_without_changing_identity_or_other_bytes() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let initial = declaration("worker", None, "catalog"); + write(root, "h/worker/agent.kdl", &initial); + write(root, "h/worker/name", "obsolete sibling authority\n"); + + for (command, field, value) in [ + ("rename", "name", "Build owner"), + ("describe", "description", "Own build delivery"), + ] { + let output = run( + root, + command, + &["h.worker", value, "--host", "h", "--json"], + None, + ); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["result"], "changed"); + assert_eq!(receipt["identity"], "h.worker"); + assert_eq!(receipt["field"], field); + assert_eq!(receipt["value"], value); + } + + let found = st2::discover(root); + assert!(found.errors.is_empty(), "{:?}", found.errors); + let spec = &found.specs[0]; + assert_eq!(spec.identity, "worker"); + assert_eq!(spec.name.as_deref(), Some("Build owner")); + assert_eq!(spec.description.as_deref(), Some("Own build delivery")); + assert_eq!( + fs::read_to_string(root.join("h/worker/name")).unwrap(), + "obsolete sibling authority\n", + "the retired sibling source is ignored, not rewritten or consulted" + ); + + let roster = Command::new(env!("CARGO_BIN_EXE_st2")) + .args([ + "--catalog", + root.to_str().unwrap(), + "agents", + "--host", + "h", + "--json", + ]) + .output() + .unwrap(); + assert!(roster.status.success()); + let rows: serde_json::Value = serde_json::from_slice(&roster.stdout).unwrap(); + assert_eq!(rows[0]["identity"], "h.worker"); + assert_eq!(rows[0]["name"], "Build owner"); + assert_eq!(rows[0]["description"], "Own build delivery"); + + let repeat = run( + root, + "rename", + &["worker", "Build owner", "--host", "h", "--json"], + None, + ); + assert!(repeat.status.success()); + assert_eq!( + serde_json::from_slice::(&repeat.stdout).unwrap()["result"], + "unchanged" + ); + + for command in ["rename", "describe"] { + let clear = run( + root, + command, + &["h.worker", "--clear", "--host", "h", "--json"], + None, + ); + assert!(clear.status.success()); + } + assert_eq!( + fs::read_to_string(root.join("h/worker/agent.kdl")).unwrap(), + initial + ); +} + +#[test] +fn cli_enforces_agent_authority_and_nix_and_format_refusals() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/root/agent.kdl", + &declaration("root", None, "catalog"), + ); + write( + root, + "h/child/agent.kdl", + &declaration("child", Some("root"), "catalog"), + ); + write( + root, + "h/sibling/agent.kdl", + &declaration("sibling", Some("root"), "catalog"), + ); + write( + root, + "h/nix/agent.kdl", + &declaration("nix", Some("root"), "nix"), + ); + write( + root, + "h/json/agent.json", + r#"{"identity":"json","host":"h","command":"sleep 300"}"#, + ); + + for (command, target, actor, code) in [ + ( + "rename", + "h.sibling", + Some("h.child"), + "presentation-not-authorized", + ), + ( + "describe", + "h.nix", + Some("h.root"), + "nix-managed-declaration", + ), + ("describe", "h.json", None, "unsupported-declaration-format"), + ] { + let output = run( + root, + command, + &[target, "refused", "--host", "h", "--json"], + actor, + ); + assert!(!output.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["result"], "error"); + assert_eq!(receipt["code"], code); + } + + let allowed = run( + root, + "describe", + &["h.child", "Owned by root", "--host", "h", "--json"], + Some("h.root"), + ); + assert!( + allowed.status.success(), + "{}", + String::from_utf8_lossy(&allowed.stderr) + ); +} diff --git a/tests/exec_backend.rs b/tests/exec_backend.rs index 09b43fad..cc37b283 100644 --- a/tests/exec_backend.rs +++ b/tests/exec_backend.rs @@ -24,6 +24,7 @@ fn exec_target(id: &str, command: &str) -> TaskTarget { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } diff --git a/tests/hooks.rs b/tests/hooks.rs index 06f27381..a2017d94 100644 --- a/tests/hooks.rs +++ b/tests/hooks.rs @@ -490,7 +490,7 @@ fn up_once_never_mutates_the_ambient_codex_config_before_account_selection() { } #[test] -fn missing_hooks_do_not_rewrite_or_stop_an_already_live_codex_agent() { +fn missing_hooks_defer_workspace_but_reconcile_live_presentation_without_lifecycle_mutation() { let tmp = tempfile::tempdir().unwrap(); let catalog = tmp.path().join("catalog"); let workspace = tmp.path().join("workspace"); @@ -542,8 +542,8 @@ fn missing_hooks_do_not_rewrite_or_stop_an_already_live_codex_agent() { let pty_actions = fs::read_to_string(&pty_log).unwrap_or_default(); assert_eq!( pty_actions.lines().collect::>(), - ["list --json"], - "the existing Codex session must be adopted without run, kill, or remove" + ["list --json", "metadata patch --id h.worker"], + "the existing Codex session may receive exact-ID metadata but no run, kill, or remove" ); assert!(!hooks_root.exists()); } diff --git a/tests/nomad_survival.rs b/tests/nomad_survival.rs index a19f85c7..ec95b0b2 100644 --- a/tests/nomad_survival.rs +++ b/tests/nomad_survival.rs @@ -134,6 +134,19 @@ impl Fixture { std::fs::write(path, kdl).unwrap(); } + fn write_presented_compact_agent(&self, identity: &str, name: &str, description: &str) { + self.pty_sessions + .borrow_mut() + .push(format!("{HOST}.{identity}")); + let kdl = format!( + "agent \"{identity}\" {{\n identity \"{identity}\"\n host \"{HOST}\"\n \ + type \"service\"\n name {name:?}\n description {description:?}\n command \"{TASK_CMD}\"\n}}\n" + ); + let path = self.catalog.join(HOST).join(identity).join("agent.kdl"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, kdl).unwrap(); + } + /// Write a PTY task that records every st2-managed environment value and its cwd on each boot. /// The append-only snapshot lets a test compare the initial launch with a manual `pty restart`. fn write_restart_env_agent(&self, identity: &str) -> PathBuf { @@ -493,6 +506,123 @@ fn pty_sessions_use_unique_agent_identity_display_names_and_preserve_lifecycle() } } +#[test] +fn presentation_changes_patch_the_exact_live_pty_without_restarting_it() { + if !pty_gate("presentation_changes_patch_the_exact_live_pty_without_restarting_it") { + return; + } + let fx = Fixture::new(); + let identity = "presented"; + let session_id = format!("{HOST}.{identity}"); + fx.write_presented_compact_agent(identity, "Build owner", "Owns build delivery"); + + let launched = fx.up_once(); + assert!(launched.contains(&session_id), "output:\n{launched}"); + let pidfile = fx.pty_root.join(format!("{session_id}.pid")); + let initial_pid = read_pid(&pidfile).unwrap(); + let list_session = || { + let output = Command::new("pty") + .env("PTY_ROOT", &fx.pty_root) + .args(["list", "--json"]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + serde_json::from_slice::(&output.stdout).unwrap() + .as_array() + .unwrap() + .iter() + .find(|session| session["name"] == session_id) + .unwrap() + .clone() + }; + let event_count = || { + let output = Command::new("pty") + .env("PTY_ROOT", &fx.pty_root) + .args(["events", "--recent", "--json", &session_id]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + String::from_utf8_lossy(&output.stdout) + .matches("metadata_change") + .count() + }; + let initial = list_session(); + let created_at = initial["createdAt"].clone(); + assert_eq!(initial["displayName"], "Build owner"); + assert_eq!(initial["tags"]["agent.presentation.schema"], "1"); + assert_eq!(initial["tags"]["agent.actor.path"], session_id); + assert_eq!( + initial["tags"]["agent.presentation.description"], + "Owns build delivery" + ); + let initial_events = event_count(); + + for (command, value) in [ + ("rename", "Release owner"), + ("describe", "Owns release delivery"), + ] { + let output = fx + .st2() + .env_remove("ST_AGENT") + .args(["--catalog", fx.catalog.to_str().unwrap(), command, &session_id, value]) + .args(["--host", HOST, "--json"]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + } + let adopted = fx.up_once(); + assert!(adopted.contains("adopted (1): presented"), "output:\n{adopted}"); + let changed = list_session(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(changed["createdAt"], created_at); + assert_eq!(changed["displayName"], "Release owner"); + assert_eq!( + changed["tags"]["agent.presentation.description"], + "Owns release delivery" + ); + assert_eq!(event_count(), initial_events + 1); + + fx.up_once(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(event_count(), initial_events + 1, "unchanged projection emitted an event"); + + for command in ["rename", "describe"] { + let output = fx + .st2() + .env_remove("ST_AGENT") + .args(["--catalog", fx.catalog.to_str().unwrap(), command, &session_id, "--clear"]) + .args(["--host", HOST, "--json"]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + } + fx.up_once(); + let cleared = list_session(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(cleared["createdAt"], created_at); + assert!(cleared.get("displayName").is_none()); + assert!( + cleared["tags"] + .get("agent.presentation.description") + .is_none() + ); + assert_eq!(event_count(), initial_events + 2); + + let declaration = fx.catalog.join(HOST).join(identity).join("agent.kdl"); + let source = std::fs::read_to_string(&declaration).unwrap(); + std::fs::write( + &declaration, + source.replace(" type \"service\"\n", " type \"service\"\n retired #true\n"), + ) + .unwrap(); + let retired = fx.up_once(); + assert!(retired.contains(&format!("torn down (1): {session_id}")), "output:\n{retired}"); + assert!( + poll_until(DEATH_TIMEOUT, || !read_alive(&pidfile)), + "the genuine retirement lifecycle change did not stop the PTY" + ); +} + #[test] fn manual_pty_restart_preserves_every_st2_managed_environment_and_config_value() { if !pty_gate("manual_pty_restart_preserves_every_st2_managed_environment_and_config_value") { diff --git a/tests/reconcile.rs b/tests/reconcile.rs index d4253755..f2a307bd 100644 --- a/tests/reconcile.rs +++ b/tests/reconcile.rs @@ -368,6 +368,8 @@ fn spec( ) -> AgentSpec { AgentSpec { identity: identity.to_string(), + name: None, + description: None, host: host.map(String::from), role: None, job_type, @@ -450,6 +452,65 @@ fn all_tasks_live_is_adopted() { assert_eq!(plan.adopt.len(), 1); } +#[test] +fn live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift() { + let mut owner = svc( + "worker", + Some(HOST), + vec![ + task( + TaskKind::Pty, + "agent", + Some("hetz.worker"), + Some("codex"), + ), + task( + TaskKind::Pty, + "shell", + Some("hetz.worker.shell"), + Some("sh"), + ), + ], + ); + owner.name = Some("Build owner".to_owned()); + owner.description = Some("Owns build delivery".to_owned()); + let specs = [owner]; + let plan = reconcile( + &specs, + &[live("hetz.worker"), live("hetz.worker.shell")], + HOST, + ); + + assert!(plan.launch.is_empty()); + assert!(plan.teardown.is_empty()); + assert!(plan.gc.is_empty()); + assert_eq!(plan.presentation.len(), 2); + let primary = plan + .presentation + .iter() + .find(|item| item.pty_id == "hetz.worker") + .unwrap(); + assert_eq!(primary.display_name, Some(Some("Build owner".to_owned()))); + assert_eq!( + primary.tags, + BTreeMap::from([ + ("agent.presentation.schema".to_owned(), Some("1".to_owned())), + ("agent.actor.path".to_owned(), Some("hetz.worker".to_owned())), + ( + "agent.presentation.description".to_owned(), + Some("Owns build delivery".to_owned()), + ), + ]) + ); + let secondary = plan + .presentation + .iter() + .find(|item| item.pty_id == "hetz.worker.shell") + .unwrap(); + assert_eq!(secondary.display_name, None); + assert_eq!(secondary.tags, primary.tags); +} + #[test] fn resource_only_changes_do_not_replace_or_relaunch_a_live_task() { let mut spec = svc( diff --git a/tests/run.rs b/tests/run.rs index 877755a6..91768a62 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -218,6 +218,8 @@ fn selected_one_shot_unknown_refuses_before_runner_list() { fn task_spec(identity: &str, host: Option<&str>, id: &str) -> AgentSpec { AgentSpec { identity: identity.into(), + name: None, + description: None, host: host.map(str::to_owned), role: None, job_type: JobType::Service, diff --git a/tests/status_agents.rs b/tests/status_agents.rs index 474facf4..2ef82661 100644 --- a/tests/status_agents.rs +++ b/tests/status_agents.rs @@ -1,6 +1,6 @@ //! M2.3 integration: presence status + the agent roster over a discovered catalog. Unit mechanics //! (state parse, staleness, atomic set/refresh) live in `src/status.rs`; this covers the composition -//! `st2 agents` relies on — enumerate specs, read each agent's `status`/`name`/inbox, project the +//! `st2 agents` relies on — enumerate specs, read each agent's status/presentation/inbox, project the //! roster, and derive `unknown` from staleness. use std::fs; @@ -26,6 +26,13 @@ fn agent_kdl(identity: &str, host: &str) -> String { ) } +fn presented_agent_kdl(identity: &str, host: &str) -> String { + agent_kdl(identity, host).replace( + " type \"service\"\n", + " type \"service\"\n name \"st2 owner\"\n description \"Own st2 delivery\"\n", + ) +} + fn retired_agent_kdl(identity: &str, host: &str) -> String { agent_kdl(identity, host).replace( " type \"service\"\n", @@ -42,7 +49,7 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { write( root, "hetz/st2-claude/agent.kdl", - &agent_kdl("st2-claude", "hetz"), + &presented_agent_kdl("st2-claude", "hetz"), ); write( root, @@ -55,15 +62,14 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { &agent_kdl("fabric-claude", "silber"), ); - // Presence: st2-claude busy, cos-claude available, fabric-claude unset (→ offline). A display name - // and an inbox message for st2-claude. + // Presence: st2-claude busy, cos-claude available, fabric-claude unset (→ offline). Presentation + // metadata and an inbox message belong to st2-claude's declaration. set_state(&status_path(&root.join("hetz/st2-claude")), State::Busy).unwrap(); set_state( &status_path(&root.join("hetz/cos-claude")), State::Available, ) .unwrap(); - write(root, "hetz/st2-claude/name", "st2 owner\n"); send_to_inbox( &st2::message::inbox_dir(&root.join("hetz/st2-claude")), "hetz.cos-claude", @@ -104,6 +110,7 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { .unwrap(); assert_eq!(st2c.status, State::Busy); assert_eq!(st2c.name.as_deref(), Some("st2 owner")); + assert_eq!(st2c.description.as_deref(), Some("Own st2 delivery")); assert!(!st2c.retired); assert_eq!( st2c.inbox, 1, @@ -201,7 +208,7 @@ fn roster_json_and_human_output_distinguish_retirement_from_presence() { ); assert_eq!( String::from_utf8(human.stdout).unwrap(), - "h.live\tavailable\t\nh.retired\tbusy\t\t[retired]\n" + "h.live\tavailable\t\t\nh.retired\tbusy\t\t\t[retired]\n" ); } From acb00164d8e1d0b08e70c8cc7fb932aee214f555 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:59:15 +0200 Subject: [PATCH 2/2] fix: enforce stable message routing identities agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- README.md | 10 +++--- src/main.rs | 40 ++++++++++++++++-------- src/message.rs | 72 +++++++++++++++++++++++++------------------- tests/message_cli.rs | 64 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 133 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 27730041..ea9cb808 100644 --- a/README.md +++ b/README.md @@ -364,10 +364,12 @@ as separate columns and appends `[retired]` to a retired row. For a catalog-backed agent, every native bus operation resolves the same agent directory used by the roster: presence is `/status`, while unread messages, archive receipts, context, and -links live under `/resources/`. The flat `/` layout remains only as the -intentional catalog-less fallback used by isolated folder evals. In a catalog-backed root, -`st2 message ls` rejects an absent identity; recovery inspection of a deliberately orphaned flat -box must be explicit with `st2 message ls --orphan` (and optionally `--archive`). +links live under `/resources/`. Catalog-derived routing, including global `--catalog`, +rejects an absent identity. Explicit `--root` is the low-level transport boundary used by isolated +folder evals: declared identities still resolve to their native resource boxes, while an absent +identity names an exact flat `/` mailbox. It never resolves a display name. +Recovery inspection of a deliberately orphaned flat box can also be requested with +`st2 message ls --orphan` (and optionally `--archive`). Adopters should cut directly to the native layout. Before launching a migrated identity, install and verify hooks, validate and materialize its hand-authored declaration, stop any predecessor transport, diff --git a/src/main.rs b/src/main.rs index 0278e5f5..50096909 100644 --- a/src/main.rs +++ b/src/main.rs @@ -544,8 +544,9 @@ enum CatalogCmd { /// running agent needs no flags. #[derive(Args)] struct MsgCtx { - /// Legacy catalog/bus root override. Prefer global `--catalog`; defaults to `$CATALOG`, then the - /// default st2 catalog. + /// Low-level bus root override. Declared identities use native resource boxes; any absent + /// identity is treated as an exact flat mailbox id. Prefer global `--catalog` for strict Agent + /// Spec routing. #[arg(long, conflicts_with = "catalog_path")] root: Option, /// The acting identity — who the message is `from` / whose inbox is "mine". Defaults to @@ -2115,11 +2116,11 @@ fn ding_cmd( // id. So `st2 ding --identity mix.worker` pokes pty `mix.worker` (the redundant positional is now // optional). An explicit positional still overrides for the rare non-agent case. let session = session.unwrap_or_else(|| id.clone()); - // Flat-bus aware: a native catalog agent → its resources/inbox; a catalog-LESS bus (an eval's - // ST_ROOT) → the flat //inbox. Status lives beside it either way. + // Explicit-root transport still resolves a declared agent to its native resource inbox; only + // an absent identity uses the exact flat / mailbox. Status lives beside it either way. let agent_dir = message::resolve_agent_dir(&catalog_root, &id, &this_host) .unwrap_or_else(|| catalog_root.join(&id)); - let inbox = message::resolve_inbox(&catalog_root, &id, &this_host)?; + let inbox = message::resolve_inbox(&catalog_root, &id, &this_host, routing_mode(&ctx))?; let status_path = st2::status::status_path(&agent_dir); eprintln!( "st2 ding: watching {}'s inbox ({}) → poking pty '{session}'", @@ -2143,6 +2144,14 @@ fn resolve_ctx(ctx: &MsgCtx) -> Result<(PathBuf, String)> { Ok((root, host)) } +fn routing_mode(ctx: &MsgCtx) -> message::RoutingMode { + if ctx.root.is_some() { + message::RoutingMode::ExplicitRoot + } else { + message::RoutingMode::Catalog + } +} + /// The acting identity (`from` / whose inbox is "mine"): `--as`, else `$ST_AGENT`. fn acting_id(ctx: &MsgCtx) -> Result { ctx.as_id @@ -2193,9 +2202,10 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host)?; + let dir = message::resolve_inbox(&root, &to, &host, mode)?; let filename = message::send_to_inbox( &dir, &from, @@ -2214,8 +2224,9 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; - let my_inbox = message::resolve_inbox(&root, &from, &host)?; + let my_inbox = message::resolve_inbox(&root, &from, &host, mode)?; let original = message::read_msg(&my_inbox, &filename) .with_context(|| format!("no message '{filename}' in {}'s inbox", from))?; let to = original @@ -2224,7 +2235,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { .with_context(|| format!("message '{filename}' has no `from` to reply to"))?; let subject = subject.or_else(|| message::reply_subject(original.subject.as_deref())); let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host)?; + let dir = message::resolve_inbox(&root, &to, &host, mode)?; let sent = message::send_to_inbox( &dir, &from, @@ -2248,11 +2259,12 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let id = match identity { Some(id) => id, None => acting_id(&ctx)?, }; - let dir = message::resolve_list_box(&root, &id, &host, archive, orphan)?; + let dir = message::resolve_list_box(&root, &id, &host, archive, orphan, mode)?; let mut msgs = if archive { message::list_dir(&dir)? } else { @@ -2298,11 +2310,12 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; let dir = if archive { - message::resolve_archive(&root, &id, &host) + message::resolve_archive(&root, &id, &host, mode) } else { - message::resolve_inbox(&root, &id, &host) + message::resolve_inbox(&root, &id, &host, mode) }?; if raw { print!("{}", std::fs::read_to_string(dir.join(&filename))?); @@ -2329,9 +2342,10 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { } MessageCmd::Archive { first, second, ctx } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; - let inbox = message::resolve_inbox(&root, &id, &host)?; - let archive = message::resolve_archive(&root, &id, &host)?; + let inbox = message::resolve_inbox(&root, &id, &host, mode)?; + let archive = message::resolve_archive(&root, &id, &host, mode)?; message::archive_msg(&inbox, &archive, &filename)?; println!("archived"); Ok(()) diff --git a/src/message.rs b/src/message.rs index 0c453fb7..4c59ea2a 100644 --- a/src/message.rs +++ b/src/message.rs @@ -316,44 +316,49 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf { agent_dir.join("resources").join("archive") } -/// The inbox dir for `id` under `root`: the NATIVE catalog inbox (`/resources/inbox`) if a -/// catalog agent is discoverable, else the flat bus inbox (`//inbox`). The -/// flat fallback lets `st2 ding`/`st2 message` operate on a catalog-LESS bus — e.g. an eval's ST_ROOT, -/// where agents are booted from a single spec (no on-disk `agent.kdl` to discover). `root` is whatever -/// `--root`/ST_ROOT names, so the layout follows the spec, never a hardcoded path. -pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> anyhow::Result { - match resolve_agent_dir(root, id, host) { - Some(dir) => Ok(inbox_dir(&dir)), - None if apply_incomplete(root) => { - anyhow::bail!("agent '{id}' is not addressable while catalog apply is incomplete") - } - None => Ok(root.join(id).join("inbox")), - } +/// How a message command selected its bus root. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum RoutingMode { + /// Catalog-derived routing accepts only declared Agent Spec identities. + Catalog, + /// Explicit `--root` is the low-level transport boundary: declared identities still use their + /// native resource boxes, while an absent identity names an exact flat mailbox. + ExplicitRoot, } -/// The archive dir for `id` under `root` — native catalog archive if discoverable, else the flat -/// `//archive` (companion to [`resolve_inbox`]). -pub fn resolve_archive(root: &Path, id: &str, host: &str) -> anyhow::Result { - match resolve_agent_dir(root, id, host) { - Some(dir) => Ok(archive_dir(&dir)), - None if apply_incomplete(root) => { - anyhow::bail!("agent '{id}' is not addressable while catalog apply is incomplete") - } - None => Ok(root.join(id).join("archive")), - } +/// Resolve the inbox for `id` under `root` according to the caller's routing boundary. +pub fn resolve_inbox( + root: &Path, + id: &str, + host: &str, + mode: RoutingMode, +) -> anyhow::Result { + resolve_list_box(root, id, host, false, false, mode) +} + +/// The archive companion to [`resolve_inbox`], with the same catalog-backed strictness and +/// explicit-root flat transport. +pub fn resolve_archive( + root: &Path, + id: &str, + host: &str, + mode: RoutingMode, +) -> anyhow::Result { + resolve_list_box(root, id, host, true, false, mode) } /// Resolve one box for `message ls`. /// -/// The permissive flat layout is automatic only when discovery proves that `root` is catalog-less. -/// Once any valid or malformed declaration makes it a catalog, an absent identity is an error. -/// `orphan` is the explicit recovery path for inspecting a raw flat box inside such a root. +/// Catalog-derived routing rejects absent identities. Explicit `--root` routing treats an absent +/// identity as an exact flat mailbox id; it never resolves presentation names. `orphan` is the +/// explicit recovery path for inspecting a raw flat box regardless of routing mode. pub fn resolve_list_box( root: &Path, id: &str, host: &str, archive: bool, orphan: bool, + mode: RoutingMode, ) -> anyhow::Result { let flat = || { root.join(id) @@ -389,7 +394,7 @@ pub fn resolve_list_box( }); } - if discovered.specs.is_empty() && discovered.errors.is_empty() { + if mode == RoutingMode::ExplicitRoot { return Ok(flat()); } anyhow::bail!("no agent '{id}' found in catalog {}", root.display()) @@ -739,16 +744,16 @@ mod tests { } #[test] - fn resolve_inbox_falls_back_to_the_flat_bus_when_catalog_less() { + fn explicit_root_resolves_declared_agents_then_falls_back_to_exact_flat_ids() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); // No catalog under root → the flat bus (//inbox|archive). assert_eq!( - resolve_inbox(root, "mix.sup", "h").unwrap(), + resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), root.join("mix.sup").join("inbox") ); assert_eq!( - resolve_archive(root, "mix.sup", "h").unwrap(), + resolve_archive(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), root.join("mix.sup").join("archive") ); // A discoverable native catalog agent → its resources/inbox. @@ -760,8 +765,13 @@ mod tests { ) .unwrap(); assert_eq!( - resolve_inbox(root, "mix.sup", "h").unwrap(), + resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), ad.join("resources").join("inbox") ); + assert_eq!( + resolve_inbox(root, "requester", "h", RoutingMode::ExplicitRoot).unwrap(), + root.join("requester").join("inbox") + ); + assert!(resolve_inbox(root, "requester", "h", RoutingMode::Catalog).is_err()); } } diff --git a/tests/message_cli.rs b/tests/message_cli.rs index c8a5969e..13aa8ceb 100644 --- a/tests/message_cli.rs +++ b/tests/message_cli.rs @@ -1,8 +1,9 @@ //! CLI coverage for message-list filters and output modes. use std::fs; +use std::io::Write as _; use std::path::Path; -use std::process::Command; +use std::process::{Command, Stdio}; fn write_message(inbox: &Path, ts_ms: u64, suffix: &str, from: &str) { fs::create_dir_all(inbox).unwrap(); @@ -25,9 +26,14 @@ fn write_agent(root: &Path, identity: &str) { .unwrap(); } -fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { +fn list_identity_with_flag( + root: &Path, + identity: &str, + root_flag: &str, + extra: &[&str], +) -> std::process::Output { Command::new(env!("CARGO_BIN_EXE_st2")) - .args(["message", "ls", identity, "--root"]) + .args(["message", "ls", identity, root_flag]) .arg(root) .args(["--host", "h"]) .args(extra) @@ -35,6 +41,10 @@ fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::O .unwrap() } +fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { + list_identity_with_flag(root, identity, "--root", extra) +} + fn list(root: &Path, extra: &[&str]) -> std::process::Output { list_identity(root, "bob", extra) } @@ -147,7 +157,7 @@ fn an_unknown_catalog_identity_fails_before_every_output_mode_reads_a_box() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity(tmp.path(), "missing", &extra); + let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(), @@ -193,6 +203,50 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() { assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "1"); } +#[test] +fn routing_mode_separates_catalog_identities_from_explicit_root_transport() { + let tmp = tempfile::tempdir().unwrap(); + let catalog = tmp.path(); + write_agent(catalog, "worker"); + let declaration = catalog.join("h/worker/agent.kdl"); + fs::write( + &declaration, + fs::read_to_string(&declaration) + .unwrap() + .replace(" type \"service\"\n", " type \"service\"\n name \"Shared Worker\"\n"), + ) + .unwrap(); + + let send = |recipient: &str, root_flag: &str| { + let mut child = Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["message", "send", recipient, root_flag]) + .arg(catalog) + .args(["--host", "h", "--as", "h.sender"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child.stdin.take().unwrap().write_all(b"work\n").unwrap(); + child.wait_with_output().unwrap() + }; + + let refused = send("Shared Worker", "--catalog"); + assert!(!refused.status.success()); + assert!(String::from_utf8_lossy(&refused.stderr).contains("no agent 'Shared Worker' found")); + assert!(!catalog.join("Shared Worker").exists()); + + let declared = send("h.worker", "--root"); + assert!(declared.status.success(), "{}", String::from_utf8_lossy(&declared.stderr)); + let listed = list_identity(catalog, "h.worker", &["--count"]); + assert!(listed.status.success(), "{}", String::from_utf8_lossy(&listed.stderr)); + assert_eq!(String::from_utf8_lossy(&listed.stdout).trim(), "1"); + + let flat = send("requester", "--root"); + assert!(flat.status.success(), "{}", String::from_utf8_lossy(&flat.stderr)); + assert_eq!(fs::read_dir(catalog.join("requester/inbox")).unwrap().count(), 1); +} + #[test] fn orphan_mode_explicitly_reads_raw_flat_inbox_and_archive() { let tmp = tempfile::tempdir().unwrap(); @@ -255,7 +309,7 @@ fn malformed_catalog_declarations_disable_implicit_flat_fallback() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity(tmp.path(), "missing", &extra); + let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(),