diff --git a/README.md b/README.md index e4634491..c07c0767 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,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 @@ -170,6 +172,24 @@ 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 serialize local writers through the persistent +private `.st2/presentation-authoring.lock`. They refuse TOML, JSON, and +explicitly `meta { managed-by "nix" }` targets. Nix generators must emit that marker before the +compatible st2 binary is activated. In the trusted single-operator fleet, caller-supplied +`ST_AGENT` limits an invocation to itself or declared descendants; it is a guardrail rather than +authentication, and absence selects the operator path. The sibling `/name` convention +is hard-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 @@ -324,9 +344,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 @@ -391,7 +412,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 compile-agent (experimental) 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..ee974abb 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")] @@ -600,6 +612,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 +682,8 @@ impl RawSpec { Ok(AgentSpec { identity, + name: self.name, + description: self.description, host, role: self.role, job_type, @@ -679,6 +699,36 @@ 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(|character| { + character.is_control() || matches!(character, '\u{2028}' | '\u{2029}') + }), + "agent presentation `{field}` must be one printable line without control characters or Unicode line separators" + ); + 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..abcaeee4 100644 --- a/crates/agent-spec/tests/discovery.rs +++ b/crates/agent-spec/tests/discovery.rs @@ -306,6 +306,148 @@ 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 = 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 = 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 (field, max_chars) in [ + ("name", AGENT_NAME_MAX_CHARS), + ("description", AGENT_DESCRIPTION_MAX_CHARS), + ] { + assert!(validate_presentation(field, Some(r"slash/name\path"), max_chars).is_ok()); + for invalid in [ + "", + " leading", + "trailing ", + "two\nlines", + "control\u{7f}", + "line\u{2028}separator", + "paragraph\u{2029}separator", + ] { + assert!( + validate_presentation(field, Some(invalid), max_chars).is_err(), + "accepted {field} {invalid:?}" + ); + } + } +} + +#[test] +fn presentation_parser_rejects_unicode_line_and_paragraph_separators() { + for field in ["name", "description"] { + for separator in ['\u{2028}', '\u{2029}'] { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "agents/h/worker/agent.kdl", + &format!( + "agent \"worker\" {{\n host \"h\"\n type \"service\"\n {field} \"left{separator}right\"\n pty \"agent\" {{ command \"true\" }}\n}}\n" + ), + ); + let found = discover(tmp.path()); + assert!( + found.specs.is_empty(), + "accepted {field} U+{:04X}", + separator as u32 + ); + assert_eq!(found.errors.len(), 1, "{field}: {:?}", found.errors); + } + } +} + #[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..e8d3530f --- /dev/null +++ b/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md @@ -0,0 +1,75 @@ +# Stable agent identity is separate from mutable presentation + +Status: draft + +Requirements change authorized by Johannes on 2026-07-31. + +Merge and acceptance 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 is a second source of truth and does not +compose with canonical Agent Spec authoring. + +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. Within the trusted-fleet model, +caller-supplied `ST_AGENT` limits an invocation to itself or declared +descendants; this is an operational guardrail, not an authenticated capability, +and absence selects the operator path. Declarations explicitly marked Nix-owned +remain writable only at their Nix source, and Nix emitters must publish that +marker before authoring is activated. st2 serializes these +edits with the private persistent `.st2/presentation-authoring.lock`, preserves +unrelated source bytes, detects stale source, and atomically replaces the +declaration. The lock covers cooperating local st2 writers in one POSIX +filesystem/kernel lock domain; it does not claim exclusion across independently +synchronized hosts or direct external writers. + +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 presentation compatibility marker because + the new fields are optional and additive. Adoption begins only after + compatible PTY and st2 binaries are deployed; Nix-generated declarations + first add their ownership marker. +- This decision remains draft and is not accepted or mergeable until Nathan + approves it. + +## 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/02-agent-spec/requirements.md b/docs/vrs/02-agent-spec/requirements.md index 1f848a41..24f8a663 100644 --- a/docs/vrs/02-agent-spec/requirements.md +++ b/docs/vrs/02-agent-spec/requirements.md @@ -13,7 +13,8 @@ the Agent Spec. General field-change behavior that the canonical specification does not yet define is proposed until a matching evals specification and proof change adopts it. -This work supports root [R01, R06, R11, and R13 through R19](../requirements.md). +This work supports root [R01, R06, R11, R13 through R19, and R24 through +R26](../requirements.md). It applies to every st2 tool that runs an agent or test. A valid local catalog and host-local runtime state are sufficient. It requires no compare-and-swap (CAS), lock service, cross-host call, or external registry. @@ -27,7 +28,7 @@ Field lookup: [F01](./spec.md#f01), [F02](./spec.md#f02), [F06](./spec.md#f06), [F07](./spec.md#f07), [F08](./spec.md#f08), [F09](./spec.md#f09), [F10](./spec.md#f10), [F11](./spec.md#f11), [F12](./spec.md#f12), [F13](./spec.md#f13), [F14](./spec.md#f14), -[F15](./spec.md#f15), and [F16](./spec.md#f16). +[F15](./spec.md#f15), [F16](./spec.md#f16), and [F17](./spec.md#f17). ## Shared invariants @@ -59,6 +60,8 @@ Field lookup: [F01](./spec.md#f01), [F02](./spec.md#f02), inputs form a versioned launch fingerprint. A healthy fingerprint mismatch is visible as `drifted` or `unknown`, but it does not authorize replacement. Unrelated work receives no action. + Presentation changes use exact-ID metadata projection only; they do not alter + launch fingerprints or authorize lifecycle work. - **SPEC-R04 Change membership and lifecycle only for exact IDs.** Add only a missing ID. Remove only an exactly attributed old ID. Retirement stops the diff --git a/docs/vrs/02-agent-spec/spec.md b/docs/vrs/02-agent-spec/spec.md index 2f8febe3..4e2b0407 100644 --- a/docs/vrs/02-agent-spec/spec.md +++ b/docs/vrs/02-agent-spec/spec.md @@ -201,6 +201,26 @@ source: [`RawSpec` and `AgentSpec`](../../../crates/agent-spec/src/spec.rs). Evidence: [validation](../../../src/validate.rs) and [reconciliation](../../../src/reconcile.rs). +

F17 Agent name and description

+ +Update observable declaration and runtime presentation metadata only. Neither +field participates in identity, routing, selection, authorization, state paths, +launch fingerprints, workspaces, inbox events, DING, or lifecycle. The roster +reads the declaration directly; sibling `name` files are ignored. + +For a healthy managed PTY, patch the exact runtime task ID in place. Every owned +PTY receives the versioned stable-actor and optional-description tag snapshot; +only the primary task named `agent` maps optional name to native display +metadata. Clearing removes only the corresponding st2-owned value. Preserve +unrelated tags and secondary display conventions. An unchanged projection is a +no-op. Failure reports and retries without stop, reap, restart, replacement, or +flapping accounting. Absent work receives the same projection at spawn. + +Authoring: canonical Agent Spec presentation fields after the matching evals +change lands. st2 source: [`AgentSpec`](../../../crates/agent-spec/src/spec.rs), +[roster](../../../src/agents.rs), and [reconciliation](../../../src/reconcile.rs). +Evidence: parser, roster, exact-ID metadata, and no-restart presentation tests. + Catalog and PTY roots are host runtime inputs, not Agent Spec fields. Their migration contract is outside this VRS. See [#85](https://github.com/compoundingtech/st2/issues/85). @@ -259,6 +279,10 @@ replacement of drifted work. and [runner](../../../src/run.rs). - **G08, moved intent:** parser, status, and executor support are absent; syntax is unspecified. [Parser](../../../crates/agent-spec/src/kdl_format.rs) +- **G09, F17 release ordering:** source authoring requires Nix emitters to mark + generated declarations before the compatible st2 binary is activated. The + pinned merged PTY dependency provides the exact-ID atomic metadata-patch API; + compatible st2 and Nix provenance adoption must still deploy as one gated cohort. ## Acceptance cases @@ -274,6 +298,10 @@ replacement of drifted work. child removal. Legacy or partial proof holds or refuses. - Matching fingerprints adopt. Mismatches drift. Explicit replacement proves the exact incarnation through fence, quiesce, materialize, and boot. +- Name and description changes update roster and exact PTY metadata while task + ID, PID, creation identity, and generation remain unchanged. Repeating the + desired projection emits no metadata event; clearing removes only owned + presentation values. A genuine retirement still follows ordinary teardown. - Host projections converge after overlap, absence, and reconnection without a shared receipt. - Moved intent rejects cycles, conflicts, and host changes. It removes before diff --git a/docs/vrs/requirements.md b/docs/vrs/requirements.md index 1ee19d05..3b7740a0 100644 --- a/docs/vrs/requirements.md +++ b/docs/vrs/requirements.md @@ -121,6 +121,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 @@ -146,3 +148,37 @@ accepted. incomplete; the external backend may already have recreated a concurrently removed registry. This diagnostic boundary is not transactionally serialized with catalog or runtime writers and is not control-plane cutover authority. +- **R24 Stable identity and bounded presentation:** The positional Agent Spec + identity and its host-qualified bus identity remain the sole stable keys for + routing, ownership, adoption, lifecycle, and automation. Agent Specs may + declare optional, non-empty `name` and `description` strings in canonical KDL + and the readable TOML/JSON forms. `name` is a non-unique mutable human label, + limited to 160 Unicode scalars; `description` is an enduring responsibility + boundary, limited to 1,000. Both are single-line: Cc control characters and + U+2028/U+2029 are invalid. Omission means absence. Presentation is never an + alias, and the declaration is its sole source of truth; a sibling `name` file + is ignored without migration or compatibility behavior. +- **R25 Constrained presentation authoring:** `st2 rename` and `st2 describe` + set or clear only their corresponding direct field in one canonical KDL + declaration selected by stable identity. They preserve unrelated source + bytes, serialize cooperating local writers through the persistent private + `.st2/presentation-authoring.lock`, reject a stale source before atomic + replacement, fsync the result, and return classified receipts. The lock inode + is never removed or stale-recovered and defines one local POSIX + filesystem/kernel exclusion domain; it is not cross-host coordination or OS + isolation from direct external writers. TOML, JSON, declarations explicitly + marked Nix-owned, stable-ID changes, and malformed or ambiguous targets fail + closed. Nix emitters must publish that marker before authoring is activated. + In the trusted-fleet model, caller-supplied `ST_AGENT` provides a guardrail, + not authentication: a catalog agent may edit itself or a descendant reached + through declared supervisor edges, while its absence selects the operator + path. +- **R26 Live PTY presentation projection:** For every healthy managed PTY, st2 + reconciles a versioned owned tag snapshot containing the stable actor identity + plus optional description through one exact task-ID metadata patch. The + primary `agent` task additionally maps optional name to native PTY display + metadata; secondary PTYs preserve their task-specific display convention. + Projection preserves unrelated tags, removes absent owned values, reports and + retries failure, and is idempotent. It never uses display-name resolution or + enters launch, teardown, garbage collection, replacement, or flapping + accounting. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index a915577c..38636fca 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -60,6 +60,70 @@ 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, R24-R26) + +The positional value in `agent ""` remains the stable Agent Spec ID. +The supported child/TOML/JSON `identity` spelling and roster JSON `identity` +field remain unchanged. Host qualification produces the existing +`.` bus ID. Only that stable identity controls routing, +selection, authorization, state paths, task identity, adoption, and lifecycle. +There is no display-name resolver, stable-ID alias, or stable-ID rename command. + +```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, Cc-control, U+2028/U+2029, or over-limit values are +invalid; slash and backslash remain ordinary printable characters. The Agent +Spec declaration is the sole source of truth. `/name` is hard-retired: +st2 neither reads, writes, migrates, nor interprets it. + +`st2 rename` and `st2 describe` accept one stable selector and either a value or +`--clear`. They edit canonical KDL only. The operation: + +1. acquires the persistent exclusive + `/.st2/presentation-authoring.lock` before discovery; +2. resolves exactly one declaration and applies the caller-supplied `ST_AGENT` + self/descendant guardrail when present; +3. refuses declarations explicitly marked `meta { managed-by "nix" }`, + unsupported formats, malformed catalogs, and ambiguous targets; +4. applies one span-bounded edit, reparses and validates the candidate; +5. fsyncs a same-directory temporary, rechecks the original inode/version and + bytes, atomically renames it, then fsyncs the declaration directory. + +`ST_AGENT` is a runner-provided convention in this trusted single-operator fleet, +not an authenticated capability: a same-UID caller can alter or remove it, and +its absence selects the operator path. Nix generators must emit the ownership +marker before activating a binary with authoring commands; st2 cannot infer an +unmarked generator from KDL bytes. + +The lock file is a persistent real inode and is never removed or stale-recovered. +It serializes cooperating st2 presentation writers in one local POSIX +filesystem/kernel lock domain. Direct same-UID writes and independently +synchronized hosts do not participate; the source recheck detects observed +interference but is not a distributed CAS or lock service. The classified +refusal codes are an operational trusted-fleet boundary, not adversarial OS +isolation. + +For each healthy managed PTY, reconciliation uses one atomic exact-task-ID +`pty metadata patch --id ` request. Every PTY receives the versioned +st2-owned tags `agent.presentation.schema=1`, +`agent.actor.path=.`, and the optional +`agent.presentation.description`. The primary task named `agent` additionally +maps `name` to native `displayName`; secondary tasks retain their task-specific +display convention. Name is not duplicated in tags. Clearing removes only the +owned native value or tag, and unrelated PTY metadata is preserved. Repeating +the same projection is a no-op. Failure is reported and retried by the ordinary +loop, never converted into launch, teardown, garbage collection, replacement, +or flapping authority. + ## Resource bindings (R20-R21) An agent may directly declare zero or more generic Resource bindings: diff --git a/flake.lock b/flake.lock index 89c059da..00c0bb3f 100644 --- a/flake.lock +++ b/flake.lock @@ -41,17 +41,17 @@ ] }, "locked": { - "lastModified": 1785501505, - "narHash": "sha256-LCNdog5kz8qcxQCMomOdDjzmLBtgOr8z9YwHH72UxDY=", + "lastModified": 1785518728, + "narHash": "sha256-NXTWG/SIctaloeAxJP9GI+8ea2MWFs32bYlgT6WBpC4=", "owner": "compoundingtech", "repo": "pty", - "rev": "d5fabc3917407aeb937a012bd97679c303e18033", + "rev": "504ac7332895fe1fa3767b530dcd99f091f56cda", "type": "github" }, "original": { "owner": "compoundingtech", "repo": "pty", - "rev": "d5fabc3917407aeb937a012bd97679c303e18033", + "rev": "504ac7332895fe1fa3767b530dcd99f091f56cda", "type": "github" } }, diff --git a/flake.nix b/flake.nix index e30a1d65..a47c9a73 100644 --- a/flake.nix +++ b/flake.nix @@ -4,9 +4,9 @@ inputs = { nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; flake-utils.url = "github:numtide/flake-utils"; - # Packaged PTY contract: the exact fleet-observation producer revision, including persisted - # environment removals used by managed agent restarts. - pty.url = "github:compoundingtech/pty/d5fabc3917407aeb937a012bd97679c303e18033"; + # Packaged PTY dependency: the merged revision with atomic metadata patching and the + # fleet-observation guarantees required by st2 reconciliation. + pty.url = "github:compoundingtech/pty/504ac7332895fe1fa3767b530dcd99f091f56cda"; pty.inputs.nixpkgs.follows = "nixpkgs"; }; diff --git a/src/agent_author.rs b/src/agent_author.rs new file mode 100644 index 00000000..3d2691fe --- /dev/null +++ b/src/agent_author.rs @@ -0,0 +1,1144 @@ +//! Constrained, source-preserving authoring of Agent Spec presentation metadata. +//! +//! Presentation is declaration state, not runtime identity. Every edit holds a private persistent +//! catalog-wide presentation lock, rechecks the original bytes, and atomically replaces exactly one +//! canonical KDL declaration. TOML, JSON, declarations marked Nix-owned, and callers outside the +//! supplied actor relationship fail closed. `ST_AGENT` is a trusted-fleet guardrail rather than +//! authentication. The lock serializes cooperating local st2 writers; it is not a cross-host lock. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt; +use std::fs::{self, File, OpenOptions}; +use std::io::Write as _; +use std::os::fd::AsRawFd 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); +const PRESENTATION_LOCK_FILE: &str = "presentation-authoring.lock"; + +/// Persistent local lock for the two presentation-authoring commands. Keeping one inode is +/// essential: unlinking a lock file while a process holds it would split the exclusion domain. +#[derive(Debug)] +struct PresentationAuthorLock { + file: File, +} + +impl PresentationAuthorLock { + fn exclusive(catalog_root: &Path) -> anyhow::Result { + let catalog = catalog_root.canonicalize().map_err(|error| { + anyhow::anyhow!("canonicalize catalog {}: {error}", catalog_root.display()) + })?; + let metadata = fs::symlink_metadata(&catalog) + .map_err(|error| anyhow::anyhow!("read catalog {}: {error}", catalog.display()))?; + anyhow::ensure!( + metadata.is_dir() && !metadata.file_type().is_symlink(), + "catalog root is not a real directory: {}", + catalog.display() + ); + + let control = catalog.join(".st2"); + match fs::symlink_metadata(&control) { + Ok(metadata) => anyhow::ensure!( + metadata.is_dir() && !metadata.file_type().is_symlink(), + "catalog control path is not a real directory: {}", + control.display() + ), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + match fs::create_dir(&control) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + let metadata = fs::symlink_metadata(&control).map_err(|error| { + anyhow::anyhow!( + "re-read catalog control path {}: {error}", + control.display() + ) + })?; + anyhow::ensure!( + metadata.is_dir() && !metadata.file_type().is_symlink(), + "catalog control path is not a real directory: {}", + control.display() + ); + } + Err(error) => { + return Err(anyhow::anyhow!( + "create catalog control path {}: {error}", + control.display() + )); + } + } + } + Err(error) => { + return Err(anyhow::anyhow!( + "read catalog control path {}: {error}", + control.display() + )); + } + } + // Persist a newly created control-directory entry before the lock can guard a declaration + // replacement. Repeating this for an existing directory is harmless and closes a prior + // creator's crash between mkdir and parent fsync. + File::open(&catalog) + .and_then(|directory| directory.sync_all()) + .map_err(|error| anyhow::anyhow!("sync catalog {}: {error}", catalog.display()))?; + + let path = control.join(PRESENTATION_LOCK_FILE); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .mode(0o600) + .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW) + .open(&path) + .map_err(|error| { + anyhow::anyhow!("open presentation lock {}: {error}", path.display()) + })?; + let metadata = file.metadata().map_err(|error| { + anyhow::anyhow!("inspect presentation lock {}: {error}", path.display()) + })?; + anyhow::ensure!( + metadata.is_file(), + "presentation lock is not a regular file: {}", + path.display() + ); + let result = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) }; + if result != 0 { + return Err(anyhow::anyhow!( + "lock presentation lock {}: {}", + path.display(), + std::io::Error::last_os_error() + )); + } + Ok(Self { file }) + } +} + +impl Drop for PresentationAuthorLock { + fn drop(&mut self) { + unsafe { + libc::flock(self.file.as_raw_fd(), libc::LOCK_UN); + } + } +} + +#[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, + source_host: String, + source_identity: String, + declaration: PathBuf, + retired: bool, +} + +/// Set or clear one presentation field for one stable Agent Spec identity. +/// +/// `actor` is the caller-supplied `ST_AGENT` identity. An absent actor is the explicit operator +/// path. Within the trusted-fleet model, the guardrail limits a catalog-managed caller to itself or +/// a descendant reached through declared supervisor edges; no presentation field expands it. +pub fn set_presentation( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + field: PresentationField, + requested: Option<&str>, +) -> Result { + let _presentation_lock = PresentationAuthorLock::exclusive(catalog_root).map_err(|error| { + AuthorError::new( + "presentation-lock-failed", + format!("acquire presentation-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, + &target.source_host, + &target.source_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), + source_host: spec.resolved_host(this_host).to_owned(), + source_identity: spec.identity.clone(), + 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, + expected_host: &str, + expected_agent: &str, + field: PresentationField, + requested: Option<&str>, + before_commit: impl FnOnce(), +) -> Result { + edit_declaration( + path, + expected_identity, + expected_host, + expected_agent, + field, + requested, + before_commit, + ) +} + +fn edit_declaration( + path: &Path, + expected_identity: &str, + expected_host: &str, + expected_agent: &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, expected_host, expected_agent)?; + 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, + expected_host, + expected_agent, + 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, + expected_host: &str, + expected_agent: &str, +) -> Result<&'a KdlNode, AuthorError> { + 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; + if let Some(children) = node.children() { + for child in 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' | '\r')) + { + 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); + } + + let after = &text[end..line_end]; + let after_indent = after.len() - after.trim_start_matches([' ', '\t']).len(); + let after_content = end + after_indent; + if text[after_content..line_end].starts_with(';') { + let mut remove_end = after_content + 1; + while remove_end < line_end + && text.as_bytes()[remove_end].is_ascii_whitespace() + && text.as_bytes()[remove_end] != b'\n' + && text.as_bytes()[remove_end] != b'\r' + { + remove_end += 1; + } + let mut replacement = text.to_owned(); + replacement.replace_range(start..remove_end, ""); + return Ok(replacement); + } + + let before = &text[line_start..start]; + let before_content = line_start + before.trim_end_matches([' ', '\t']).len(); + let preceding = text[..before_content].chars().next_back(); + let remove_start = match preceding { + Some(';') => before_content - 1, + Some('{') => start, + _ => { + return Err(AuthorError::new( + "unsafe-source-shape", + "compact presentation metadata has no adjacent KDL separator", + )); + } + }; + let mut replacement = text.to_owned(); + replacement.replace_range(remove_start..end, ""); + Ok(replacement) +} + +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, + expected_host: &str, + expected_agent: &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, expected_host, expected_agent)?; + 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.set_permissions(fs::Permissions::from_mode(mode))?; + 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 source_preserving_edit_accepts_a_dotted_host_identity() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "us.east/worker/agent.kdl", + &declaration("worker", "us.east", None, "catalog"), + ); + + let receipt = set_presentation( + root, + "us.east.worker", + "elsewhere", + None, + PresentationField::Name, + Some("Build owner"), + ) + .unwrap(); + + assert_eq!(receipt.identity, "us.east.worker"); + assert!( + fs::read_to_string(path) + .unwrap() + .contains("name \"Build owner\"") + ); + } + + #[test] + fn source_preserving_clear_accepts_a_crlf_dedicated_field() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let original = declaration("worker", "h", None, "catalog").replace('\n', "\r\n"); + let with_name = original.replace( + " command \"sleep 60\"", + " name \"Build owner\"\r\n command \"sleep 60\"", + ); + let path = write(root, "h/worker/agent.kdl", &with_name); + + let receipt = set_presentation( + root, + "h.worker", + "h", + None, + PresentationField::Name, + None, + ) + .unwrap(); + + assert_eq!(receipt.result, AuthorOutcome::Changed); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } + + #[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", + "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", + "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/eval_run.rs b/src/eval_run.rs index 2f7689f4..c29e7190 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -90,6 +90,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, @@ -256,8 +258,18 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result { + entry.insert(route.clone()); + } + std::collections::btree_map::Entry::Occupied(_) => { + anyhow::bail!( + "canonical-agents found duplicate canonical route spelling `{spelling}`" + ); + } + } + } } runtime_tasks.sort_by(|left, right| left.runtime_id.cmp(&right.runtime_id)); @@ -1074,6 +1086,25 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos None }; + if let Some(routes) = canonical_routes.as_ref() { + if routes.contains_key(&requester) { + anyhow::bail!( + "canonical-agents requester `{requester}` must be external to the admitted Agent Specs" + ); + } + if let Some(owner) = specs + .iter() + .find(|spec| spec.name.as_deref() == Some(requester.as_str())) + { + anyhow::bail!( + "canonical-agents requester `{requester}` matches the presentation name of admitted Agent Spec `{}`", + owner.bus_id(host) + ); + } + std::fs::create_dir_all(bus.join(&requester).join("inbox")) + .with_context(|| format!("provisioning external requester `{requester}` inbox"))?; + } + eval_log!("== boot team ({} agents) ==", specs.len()); let boot = boot_team(&specs, host, catalog)?; if eval.canonical_agents { @@ -1562,6 +1593,7 @@ mod tests { pty_id: id.into(), alive: true, exit_code: None, + presentation: None, }) .collect(), killed: RefCell::new(Vec::new()), @@ -1717,14 +1749,14 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } #[test] fn reap_race_errors_converge_only_after_empty_list() { - let runner = RaceRunner { lists: RefCell::new(vec![vec![Session { pty_id: "x".into(), alive: true, exit_code: None }], vec![]]), ops: RefCell::new(Vec::new()) }; + let runner = RaceRunner { lists: RefCell::new(vec![vec![Session { pty_id: "x".into(), alive: true, exit_code: None, presentation: None }], vec![]]), ops: RefCell::new(Vec::new()) }; assert!(reap_all_eval_sessions_with_runner(&runner, "test").is_ok()); assert_eq!(runner.ops.borrow().len(), 2); } struct PersistentRunner { lists: RefCell, ops: RefCell } impl Runner for PersistentRunner { - fn list_sessions(&self) -> anyhow::Result> { *self.lists.borrow_mut() += 1; Ok(vec![Session { pty_id: "stuck".into(), alive: true, exit_code: None }]) } + fn list_sessions(&self) -> anyhow::Result> { *self.lists.borrow_mut() += 1; Ok(vec![Session { pty_id: "stuck".into(), alive: true, exit_code: None, presentation: None }]) } fn spawn(&self, _: &TaskTarget, _: &Path) -> anyhow::Result<()> { Ok(()) } fn kill(&self, _: &str) -> anyhow::Result<()> { *self.ops.borrow_mut() += 1; Ok(()) } fn remove(&self, _: &str) -> anyhow::Result<()> { *self.ops.borrow_mut() += 1; Ok(()) } @@ -1742,7 +1774,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } #[test] fn cleanup_guard_reaps_on_unwind_without_double_panic() { use std::rc::Rc; - let lists = Rc::new(RefCell::new(vec![vec![Session { pty_id: "panic".into(), alive: true, exit_code: None }], vec![]])); + let lists = Rc::new(RefCell::new(vec![vec![Session { pty_id: "panic".into(), alive: true, exit_code: None, presentation: None }], vec![]])); let ops = Rc::new(RefCell::new(Vec::new())); struct Shared { lists: Rc>>>, ops: Rc>> } impl Runner for Shared { @@ -1765,7 +1797,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } fn cleanup_guard_catalog_lifetime_matrix() { for keep in [false, true] { let dir = tempfile::tempdir().unwrap(); let catalog = dir.path().join("catalog"); std::fs::create_dir_all(&catalog).unwrap(); - let runner = RaceRunner { lists: RefCell::new(vec![vec![Session { pty_id: "x".into(), alive: true, exit_code: None }], vec![]]), ops: RefCell::new(Vec::new()) }; + let runner = RaceRunner { lists: RefCell::new(vec![vec![Session { pty_id: "x".into(), alive: true, exit_code: None, presentation: None }], vec![]]), ops: RefCell::new(Vec::new()) }; { let _guard = EvalCleanupGuard { runner, catalog: catalog.clone(), host: "test".into(), keep }; } assert_eq!(catalog.exists(), keep); } diff --git a/src/eval_spec.rs b/src/eval_spec.rs index 3873075b..3f720364 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. @@ -188,6 +193,28 @@ fn child_arg(node: &KdlNode, name: &str) -> Option { .and_then(arg) } +fn parse_agent_presentation( + node: &KdlNode, + agent_id: &str, + field: &str, + destination: &mut Option, +) -> anyhow::Result<()> { + anyhow::ensure!( + destination.is_none(), + "agent '{agent_id}' declares `{field}` more than once" + ); + anyhow::ensure!( + node.children().is_none() + && node.entries().len() == 1 + && node.entries()[0].name().is_none(), + "agent '{agent_id}' `{field}` must contain exactly one positional string" + ); + let value = arg(node) + .ok_or_else(|| anyhow::anyhow!("agent '{agent_id}' `{field}` must contain a string"))?; + *destination = Some(value); + Ok(()) +} + /// Parse a flat `run "label" { … }` node into one [`RunStep`], appended in order. The `run` node IS /// one step: its arg is the label, its body holds the command directly. Multiple `run "label"` nodes in /// the eval block run in file order. (The old nested `run { step … }` wrapper was retired once every @@ -389,6 +416,8 @@ fn parse_agent(node: &KdlNode, prefix: &str, parent_env: &BTreeMap {} + "name" => parse_agent_presentation(c, &id, "name", &mut display_name)?, + "description" => { + parse_agent_presentation(c, &id, "description", &mut description)? + } "workspace" => workspace = arg(c), "supervisor" => supervisor = arg(c), "command" => command = arg(c), @@ -456,13 +489,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, @@ -758,6 +799,39 @@ eval { }); } + #[test] + fn compact_agents_lower_bounded_presentation_without_changing_identity() { + let spec = parse_spec( + r#"agent "worker" { + name "Build owner" + description "Own build delivery" + command "true" +}"#, + ) + .unwrap(); + let agent = &spec.agents[0]; + assert_eq!(agent.id, "worker"); + assert_eq!(agent.name.as_deref(), Some("Build owner")); + assert_eq!(agent.description.as_deref(), Some("Own build delivery")); + + for field in ["name", "description"] { + for separator in ['\u{2028}', '\u{2029}'] { + let source = format!( + "agent \"worker\" {{ {field} \"left{separator}right\"; command \"true\" }}" + ); + assert!(parse_spec(&source).is_err(), "accepted {field} U+{:04X}", separator as u32); + } + let duplicate = format!( + "agent \"worker\" {{ {field} \"one\"; {field} \"two\"; command \"true\" }}" + ); + assert!(parse_spec(&duplicate).is_err(), "accepted duplicate {field}"); + + let malformed = + format!("agent \"worker\" {{ {field} 1; command \"true\" }}"); + assert!(parse_spec(&malformed).is_err(), "accepted malformed {field}"); + } + } + #[test] fn verbose_full_label_exec_still_parses_without_double_prefix() { // Back-compat: the older fully-qualified exec label resolves to the same id (no double-prefix). diff --git a/src/exec_backend.rs b/src/exec_backend.rs index 90de502c..545003fa 100644 --- a/src/exec_backend.rs +++ b/src/exec_backend.rs @@ -211,6 +211,7 @@ impl ExecBackend { pty_id: id.to_string(), alive, exit_code: None, + presentation: None, }); } Ok(out) @@ -938,6 +939,7 @@ mod generation_observation_tests { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } diff --git a/src/flapping.rs b/src/flapping.rs index fa873c6f..92fd231d 100644 --- a/src/flapping.rs +++ b/src/flapping.rs @@ -32,6 +32,7 @@ pub struct FlappingCap { launches: HashMap>, last_launch: HashMap, parked: HashSet, + presentation_batch_cursor: usize, } impl FlappingCap { @@ -49,6 +50,15 @@ impl FlappingCap { self.parked.iter() } + pub(crate) fn presentation_batch_start(&mut self, total: usize, batch: usize) -> usize { + if total == 0 { + return 0; + } + let start = self.presentation_batch_cursor % total; + self.presentation_batch_cursor = (start + batch.min(total)) % total; + start + } + /// Decide whether `id` may be (re)launched at `now` under `policy`. On `Allow` the caller should /// spawn and then call [`record`](Self::record). pub fn decide(&mut self, id: &str, now: Instant, policy: &Restart) -> RestartDecision { diff --git a/src/lib.rs b/src/lib.rs index 7aca9184..73f733b6 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 agents; pub mod catalog; pub mod compile_agent; @@ -44,7 +45,9 @@ pub use exec_backend::ExecBackend; pub use expand::{expand_env, expand_vars}; pub use flapping::FlappingCap; pub use host_lock::HostLock; -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_once, up_once_selected, up_once_selected_specs, up_once_specs, diff --git a/src/main.rs b/src/main.rs index 1dc3f7c5..eecdfc01 100644 --- a/src/main.rs +++ b/src/main.rs @@ -120,6 +120,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), /// EXPERIMENTAL: generate one compact agent declaration plus catalog-owned templates. Hand-authored /// KDL is canonical. Inspect the full generated KDL and every workspace `render {}` target before /// materialization. The workspace remains untouched until `st2 up --materialize-only` or `st2 up`. @@ -309,6 +313,28 @@ struct MsgCtx { host: Option, } +#[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 ServiceCmd { /// Write the `st2.service` systemd-user unit, enable it (start on boot), and start it now. @@ -578,6 +604,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::Agents { catalog, status, @@ -1191,6 +1221,69 @@ fn report_check(problems: &mut usize, ok: bool, label: &str, detail: &str) { } } +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 status_cmd(identity: Option, set: Option, ctx: MsgCtx) -> Result<()> { let (root, host) = resolve_ctx(&ctx)?; let id = match identity { @@ -1236,10 +1329,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, ); } @@ -1269,7 +1363,7 @@ fn ding_cmd( // ST_ROOT) → the flat //inbox. 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)?; let status_path = st2::status::status_path(&agent_dir); eprintln!( "st2 ding: watching {}'s inbox ({}) → poking pty '{session}'", @@ -1345,7 +1439,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { let (root, host) = resolve_ctx(&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)?; let filename = message::send_to_inbox( &dir, &from, @@ -1365,7 +1459,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { } => { let (root, host) = resolve_ctx(&ctx)?; let from = acting_id(&ctx)?; - let my_inbox = message::resolve_inbox(&root, &from, &host); + let my_inbox = message::resolve_inbox(&root, &from, &host)?; let original = message::read_msg(&my_inbox, &filename) .with_context(|| format!("no message '{filename}' in {}'s inbox", from))?; let to = original @@ -1374,7 +1468,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)?; let sent = message::send_to_inbox( &dir, &from, @@ -1453,7 +1547,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { message::resolve_archive(&root, &id, &host) } else { message::resolve_inbox(&root, &id, &host) - }; + }?; if raw { print!("{}", std::fs::read_to_string(dir.join(&filename))?); return Ok(()); @@ -1480,11 +1574,9 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { MessageCmd::Archive { first, second, ctx } => { let (root, host) = resolve_ctx(&ctx)?; let (id, filename) = box_target(first, second, &ctx)?; - message::archive_msg( - &message::resolve_inbox(&root, &id, &host), - &message::resolve_archive(&root, &id, &host), - &filename, - )?; + let inbox = message::resolve_inbox(&root, &id, &host)?; + let archive = message::resolve_archive(&root, &id, &host)?; + message::archive_msg(&inbox, &archive, &filename)?; println!("archived"); Ok(()) } diff --git a/src/message.rs b/src/message.rs index 3cb497f3..25eb2c94 100644 --- a/src/message.rs +++ b/src/message.rs @@ -314,25 +314,25 @@ 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) -> PathBuf { - match resolve_agent_dir(root, id, host) { - Some(dir) => inbox_dir(&dir), - None => root.join(id).join("inbox"), +/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus. +/// Inside a catalog, an absent identity fails closed unless a real flat inbox was explicitly +/// provisioned, as eval does for its external requester. +pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> anyhow::Result { + match resolve_list_box(root, id, host, false, false) { + Ok(inbox) => Ok(inbox), + Err(error) => { + let flat = root.join(id).join("inbox"); + match fs::symlink_metadata(&flat) { + Ok(metadata) if metadata.file_type().is_dir() => Ok(flat), + _ => Err(error), + } + } } } -/// 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) -> PathBuf { - match resolve_agent_dir(root, id, host) { - Some(dir) => archive_dir(&dir), - None => root.join(id).join("archive"), - } +/// Archive companion to [`resolve_inbox`], with the same stable-ID and catalog-less boundaries. +pub fn resolve_archive(root: &Path, id: &str, host: &str) -> anyhow::Result { + resolve_list_box(root, id, host, true, false) } /// Resolve one box for `message ls`. @@ -654,11 +654,11 @@ mod tests { let root = tmp.path(); // No catalog under root → the flat bus (//inbox|archive). assert_eq!( - resolve_inbox(root, "mix.sup", "h"), + resolve_inbox(root, "mix.sup", "h").unwrap(), root.join("mix.sup").join("inbox") ); assert_eq!( - resolve_archive(root, "mix.sup", "h"), + resolve_archive(root, "mix.sup", "h").unwrap(), root.join("mix.sup").join("archive") ); // A discoverable native catalog agent → its resources/inbox. @@ -666,12 +666,17 @@ mod tests { std::fs::create_dir_all(&ad).unwrap(); std::fs::write( ad.join("agent.kdl"), - "agent \"mix.sup\" {\n identity \"mix.sup\"\n host \"h\"\n type \"service\"\n pty \"agent\" { command \"x\" }\n}\n", + "agent \"mix.sup\" {\n identity \"mix.sup\"\n name \"Shared Worker\"\n host \"h\"\n type \"service\"\n pty \"agent\" { command \"x\" }\n}\n", ) .unwrap(); assert_eq!( - resolve_inbox(root, "mix.sup", "h"), + resolve_inbox(root, "mix.sup", "h").unwrap(), ad.join("resources").join("inbox") ); + assert!(resolve_inbox(root, "Shared Worker", "h").is_err()); + + let requester = root.join("requester").join("inbox"); + std::fs::create_dir_all(&requester).unwrap(); + assert_eq!(resolve_inbox(root, "requester", "h").unwrap(), requester); } } diff --git a/src/reconcile.rs b/src/reconcile.rs index f69c3953..832ca8b1 100644 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -23,6 +23,15 @@ pub struct Session { /// The process exit code once exited (`None` while running, or if killed/vanished with no code). /// Reconcile ignores this; it exists only for crash-vs-clean-exit detection (the crash-ding). pub exit_code: Option, + /// PTY presentation observed in the same authoritative inventory snapshot. Exec sessions and + /// older/partial observations leave this unknown so reconciliation repairs them fail-closed. + pub presentation: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ObservedPtyPresentation { + pub display_name: Option, + pub tags: BTreeMap, } /// A concrete task st2 should spawn — everything a backend needs, resolved from the spec. Produced @@ -47,6 +56,65 @@ 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(|| match spec.name.as_ref() { + Some(name) if name == pty_id => None, + _ => 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()), + ]), + }) +} + +fn presentation_matches( + desired: &PtyPresentation, + observed: &ObservedPtyPresentation, +) -> bool { + let display_name_matches = desired + .display_name + .as_ref() + .is_none_or(|display_name| display_name == &observed.display_name); + display_name_matches + && desired + .tags + .iter() + .all(|(key, value)| observed.tags.get(key) == value.as_ref()) } /// A resolved task launch accepted by the execution backends. @@ -89,6 +157,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 +234,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 +242,20 @@ 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() + && !s + .presentation + .as_ref() + .is_some_and(|observed| presentation_matches(&presentation, observed)) + { + 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(_) => { @@ -225,6 +306,10 @@ pub fn reconcile<'a>( .iter() .map(|s| (s.pty_id.as_str(), s.alive)) .collect(); + let sessions_by_id: HashMap<&str, &Session> = sessions + .iter() + .map(|session| (session.pty_id.as_str(), session)) + .collect(); let mut plan = ReconcilePlan::default(); for spec in specs { @@ -280,10 +365,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 +378,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 +391,19 @@ 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 => { + let actual = sessions_by_id + .get(target.pty_id.as_str()) + .expect("alive state has a session"); + if let Some(presentation) = target.presentation.clone() + && !actual + .presentation + .as_ref() + .is_some_and(|observed| presentation_matches(&presentation, observed)) + { + plan.presentation.push(presentation); + } + } SessionState::Dead | SessionState::Absent if lifecycle == TaskLifecycle::AdoptOnly => { diff --git a/src/run.rs b/src/run.rs index ec50f54a..860d4cab 100644 --- a/src/run.rs +++ b/src/run.rs @@ -14,21 +14,23 @@ use std::cell::RefCell; use std::collections::{BTreeMap, HashMap, HashSet}; use std::ffi::OsString; use std::fs::File; -use std::io::{Read as _, Seek as _}; +use std::io::{Read as _, Seek as _, Write as _}; +use std::os::fd::AsRawFd as _; use std::os::unix::fs::MetadataExt as _; use std::os::unix::process::CommandExt as _; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Child, ChildStdin, Command, Output, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{Receiver, RecvTimeoutError, channel}; use std::time::{Duration, Instant}; -use serde::Deserialize; +use anyhow::Context as _; +use serde::{Deserialize, Serialize}; use crate::exec_backend::ExecBackend; use crate::flapping::FlappingCap; 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, @@ -38,15 +40,93 @@ use agent_spec::spec::TaskKind; // This is an outer containment bound for a wedged runtime, not a fleet-scalability mechanism. const PTY_LIST_TIMEOUT: Duration = Duration::from_secs(2); const PTY_DAEMON_SHUTDOWN_WAIT: Duration = Duration::from_secs(6); +const MAX_PRESENTATION_PATCHES_PER_PASS: usize = 8; /// Run a non-interactive child with bounded output capture. Regular temporary files keep an escaped /// 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 terminate_and_reap_before(mut child: Child, pid: i32, deadline: Instant) { + unsafe { + libc::kill(-pid, libc::SIGKILL); + } + let _ = child.kill(); + loop { + match child.try_wait() { + Ok(Some(_)) => return, + Ok(None) if Instant::now() < deadline => { + std::thread::sleep( + deadline + .saturating_duration_since(Instant::now()) + .min(Duration::from_millis(20)), + ); + } + Ok(None) | Err(_) => { + std::thread::spawn(move || { + let _ = child.wait(); + }); + return; + } + } + } +} + +fn write_all_before( + mut stdin: ChildStdin, + mut input: &[u8], + deadline: Instant, +) -> anyhow::Result { + let fd = stdin.as_raw_fd(); + let flags = unsafe { libc::fcntl(fd, libc::F_GETFL) }; + if flags == -1 { + return Err(std::io::Error::last_os_error()).context("read metadata stdin flags"); + } + if unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) } == -1 { + return Err(std::io::Error::last_os_error()).context("make metadata stdin nonblocking"); + } + while !input.is_empty() { + if Instant::now() >= deadline { + return Ok(false); + } + match stdin.write(input) { + Ok(0) => { + return Err(std::io::Error::from(std::io::ErrorKind::WriteZero)) + .context("write metadata patch payload"); + } + Ok(written) => input = &input[written..], + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => {} + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + if Instant::now() >= deadline { + return Ok(false); + } + std::thread::sleep( + deadline + .saturating_duration_since(Instant::now()) + .min(Duration::from_millis(20)), + ); + } + Err(error) => return Err(error).context("write metadata patch payload"), + } + } + Ok(true) +} + +fn output_with_input_timeout( + command: &mut Command, + timeout: Duration, + input: Option>, +) -> 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 { @@ -61,21 +141,29 @@ fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Resu let mut child = command.spawn()?; let pid = child.id() as i32; let deadline = Instant::now() + timeout; + if let Some(input) = input { + let Some(stdin) = child.stdin.take() else { + terminate_and_reap_before(child, pid, deadline); + anyhow::bail!("metadata patch child has no piped stdin"); + }; + match write_all_before(stdin, &input, deadline) { + Ok(true) => {} + Ok(false) => { + terminate_and_reap_before(child, pid, deadline); + anyhow::bail!("timed out after {:.1}s", timeout.as_secs_f64()); + } + Err(error) => { + terminate_and_reap_before(child, pid, deadline); + return Err(error); + } + } + } let status = loop { if let Some(status) = child.try_wait()? { break status; } if Instant::now() >= deadline { - unsafe { - libc::kill(-pid, libc::SIGKILL); - } - let _ = child.kill(); - // Never turn the timeout into another unbounded wait. Reap asynchronously so a - // long-running supervisor does not accumulate zombies, while a wedged runtime cannot - // hold up this failed probe or a short-lived doctor process. - std::thread::spawn(move || { - let _ = child.wait(); - }); + terminate_and_reap_before(child, pid, deadline); anyhow::bail!("timed out after {:.1}s", timeout.as_secs_f64()); } std::thread::sleep(Duration::from_millis(20)); @@ -114,6 +202,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. @@ -158,6 +251,17 @@ struct PtyListEntry { /// PTY-owned generation creation time. #[serde(rename = "createdAt", default)] created_at: Option, + #[serde(rename = "displayName", default)] + display_name: Option, + #[serde(default)] + tags: BTreeMap, +} + +#[derive(Serialize)] +struct PtyMetadataPatch<'a> { + #[serde(rename = "displayName", skip_serializing_if = "Option::is_none")] + display_name: Option<&'a Option>, + tags: &'a BTreeMap>, } /// The `PTY_ROOT` st2 uses for a pty op. An EXPORTED ambient `PTY_ROOT` WINS — a decoupled partition, @@ -233,9 +337,10 @@ impl PtyCli { /// Build (but do not run) the `pty run` invocation for `target`. Split out so the exact argv + /// env can be unit-tested without spawning anything. /// - /// `$VAR`s are expanded here for everything that does NOT pass through a shell — env values, tag - /// values, `cwd`, and direct argv — because `pty` passes them through verbatim. Shell source is - /// left unexpanded: `sh -c` expands it at spawn from the same env (which includes `$CATALOG`). + /// `$VAR`s are expanded here for task-authored values that do NOT pass through a shell — env, + /// tags, `cwd`, and direct argv — because `pty` passes them through verbatim. The st2-owned + /// presentation snapshot remains literal so initial spawn and later metadata patches agree. + /// Shell source is left unexpanded: `sh -c` expands it at spawn from the same env. fn build_run_command(&self, target: &TaskTarget, spec_dir: &Path) -> Command { let cwd = self.resolve_cwd(target, spec_dir); let mut cmd = Command::new(&self.bin); @@ -243,18 +348,48 @@ 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))) if name == &target.pty_id => { + cmd.arg("--no-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 { - cmd.arg("--tag").arg(format!("{k}={}", self.expand(v))); + let mut tags = target + .tags + .iter() + .map(|(key, value)| (key.clone(), self.expand(value))) + .collect::>(); + 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}={v}")); } // Managed agent and DING sessions retain PTY exit evidence until the lifecycle owner records // the receipt and explicitly removes the generation. This prevents face607 clean-exit reaping @@ -443,6 +578,29 @@ impl PtyCli { } } + 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(&self) -> anyhow::Result> { self.list_entries_at(&effective_pty_root(&self.catalog_root)) } @@ -489,6 +647,10 @@ impl Runner for PtyCli { pty_id: e.name, alive: e.status == "running", exit_code: e.exit_code, + presentation: Some(crate::reconcile::ObservedPtyPresentation { + display_name: e.display_name, + tags: e.tags, + }), }) .collect()) } @@ -538,6 +700,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") @@ -722,6 +888,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), @@ -852,6 +1022,7 @@ impl UpReport { || !self.torn_down.is_empty() || !self.gc.is_empty() || !self.flapping.is_empty() + || !self.warnings.is_empty() || !self.errors.is_empty() } } @@ -946,6 +1117,34 @@ pub fn execute( } } + // Presentation never delays lifecycle convergence. Drift repair is bounded to eight sequential + // children, keeping its worst-case 2s-per-child containment below the 30s supervisor cadence; + // the persistent cursor rotates remaining drift through later passes without starvation. + let presentation_count = plan + .presentation + .len() + .min(MAX_PRESENTATION_PATCHES_PER_PASS); + let presentation_start = + cap.presentation_batch_start(plan.presentation.len(), presentation_count); + for offset in 0..presentation_count { + let presentation = + &plan.presentation[(presentation_start + offset) % plan.presentation.len()]; + if let Err(error) = runner.patch_presentation(presentation) { + report + .errors + .push(format!("metadata patch {}: {error}", presentation.pty_id)); + } + } + let deferred_presentation = plan + .presentation + .len() + .saturating_sub(MAX_PRESENTATION_PATCHES_PER_PASS); + if deferred_presentation > 0 { + report.warnings.push(format!( + "deferred {deferred_presentation} presentation patches after bounded batch of {MAX_PRESENTATION_PATCHES_PER_PASS}" + )); + } + report .adopted .extend(plan.adopt.iter().map(|s| s.identity.clone())); @@ -1607,7 +1806,7 @@ mod tests { use super::*; use agent_spec::spec::{AgentSpec, JobType, Task, TaskKind, TaskLifecycle}; use std::cell::Cell; - use std::collections::BTreeMap; + use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; fn target(id: &str, cmd: &str) -> TaskTarget { @@ -1622,6 +1821,7 @@ mod tests { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } @@ -1652,6 +1852,8 @@ mod tests { fn selected_codex_gate_suppresses_launch_on_stale_hooks() { let spec = AgentSpec { identity: "codex".into(), + name: None, + description: None, host: None, role: None, job_type: JobType::Service, @@ -1735,12 +1937,15 @@ mod tests { pty_id: id.to_string(), alive, exit_code: None, + presentation: None, } } fn spec_fixture() -> AgentSpec { AgentSpec { identity: "demo".into(), + name: None, + description: None, host: Some("hetz".into()), role: None, job_type: JobType::Service, @@ -1977,7 +2182,7 @@ mod tests { } /// 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(); @@ -2010,6 +2215,245 @@ mod tests { ); } + #[test] + fn build_run_command_projects_primary_name_and_owned_tags_at_spawn() { + let key = "ST2_TEST_PRESENTATION_LITERAL_71c"; + unsafe { std::env::set_var(key, "expanded") } + + 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(), + Some(format!("${key}")), + ), + ]), + }); + 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.contains("agent.presentation.description=$ST2_TEST_PRESENTATION_LITERAL_71c")); + } + + #[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 input_write_failure_terminates_and_reaps_the_child() { + use std::os::unix::fs::PermissionsExt as _; + + let temporary = tempfile::tempdir().unwrap(); + let executable = temporary.path().join("close-stdin"); + let pidfile = temporary.path().join("child.pid"); + std::fs::write( + &executable, + "#!/bin/sh\nprintf '%s' \"$$\" > \"$PIDFILE\"\nexec 0<&-\nsleep 60\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + let input = vec![b'x'; 1024 * 1024]; + let error = output_with_input_timeout( + Command::new(&executable).env("PIDFILE", &pidfile), + Duration::from_secs(1), + Some(input), + ) + .unwrap_err(); + let pid = std::fs::read_to_string(pidfile) + .unwrap() + .parse::() + .unwrap(); + + assert!( + format!("{error:#}").contains("Broken pipe"), + "unexpected write error: {error:#}" + ); + assert!( + !crate::host_lock::process_alive(pid), + "failed metadata child {pid} was not terminated and reaped" + ); + } + + #[test] + fn input_write_obeys_the_child_deadline() { + use std::os::unix::fs::PermissionsExt as _; + + let temporary = tempfile::tempdir().unwrap(); + let executable = temporary.path().join("ignore-stdin"); + let pidfile = temporary.path().join("child.pid"); + std::fs::write( + &executable, + "#!/bin/sh\nprintf '%s' \"$$\" > \"$PIDFILE\"\nsleep 60\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + let input = vec![b'x'; 1024 * 1024]; + let started = Instant::now(); + let error = output_with_input_timeout( + Command::new(&executable).env("PIDFILE", &pidfile), + Duration::from_millis(100), + Some(input), + ) + .unwrap_err(); + let pid = std::fs::read_to_string(pidfile) + .unwrap() + .parse::() + .unwrap(); + + assert!( + format!("{error:#}").contains("timed out"), + "unexpected write error: {error:#}" + ); + assert!( + started.elapsed() < Duration::from_secs(1), + "blocked stdin write ignored the child deadline" + ); + let reap_deadline = Instant::now() + Duration::from_secs(1); + while crate::host_lock::process_alive(pid) && Instant::now() < reap_deadline { + std::thread::sleep(Duration::from_millis(10)); + } + assert!(!crate::host_lock::process_alive(pid)); + } + + #[cfg(target_os = "linux")] + #[test] + fn undrained_reader_does_not_retain_the_nonblocking_writer() { + use std::os::fd::{FromRawFd as _, OwnedFd}; + + let mut pipe_fds = [0; 2]; + assert_eq!(unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }, 0); + let reader = unsafe { OwnedFd::from_raw_fd(pipe_fds[0]) }; + let writer = unsafe { OwnedFd::from_raw_fd(pipe_fds[1]) }; + let pipe = std::fs::read_link(format!("/proc/self/fd/{}", reader.as_raw_fd())).unwrap(); + let started = Instant::now(); + + assert!( + !write_all_before( + ChildStdin::from(writer), + &vec![b'x'; 1024 * 1024], + Instant::now() + Duration::from_millis(100), + ) + .unwrap() + ); + let retained_writers = std::fs::read_dir("/proc/self/fd") + .unwrap() + .filter_map(Result::ok) + .filter_map(|entry| std::fs::read_link(entry.path()).ok()) + .filter(|target| target == &pipe) + .count(); + + assert!(started.elapsed() < Duration::from_secs(1)); + assert_eq!( + retained_writers, 1, + "the undrained pipe retained a writer after the deadline" + ); + } + + #[cfg(target_os = "linux")] + #[test] + fn expired_write_deadline_prevents_further_progress() { + use std::os::fd::{FromRawFd as _, OwnedFd}; + + let mut pipe_fds = [0; 2]; + assert_eq!(unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }, 0); + let _reader = unsafe { OwnedFd::from_raw_fd(pipe_fds[0]) }; + let writer = unsafe { OwnedFd::from_raw_fd(pipe_fds[1]) }; + + assert!( + !write_all_before(ChildStdin::from(writer), b"x", Instant::now()).unwrap(), + "an expired child deadline still allowed stdin progress" + ); + } + + #[test] + fn expired_cleanup_deadline_hands_reaping_off_without_blocking() { + let child = Command::new("sleep").arg("60").spawn().unwrap(); + let pid = child.id() as i32; + let started = Instant::now(); + terminate_and_reap_before(child, pid, Instant::now()); + + assert!( + started.elapsed() < Duration::from_secs(1), + "expired cleanup deadline blocked the caller" + ); + let reap_deadline = Instant::now() + Duration::from_secs(1); + while crate::host_lock::process_alive(pid) && Instant::now() < reap_deadline { + std::thread::sleep(Duration::from_millis(10)); + } + assert!( + !crate::host_lock::process_alive(pid), + "background reaper did not collect child {pid}" + ); + } + #[test] fn build_run_command_passes_direct_argv_without_a_shell() { let cli = PtyCli::new(PathBuf::from("/my/catalog")); @@ -2129,6 +2573,11 @@ mod tests { let cli = PtyCli::default(); let mut t = target("hetz.demo", "exec codex 'boot'"); t.bus_id = t.pty_id.clone(); + t.presentation = Some(PtyPresentation { + pty_id: t.pty_id.clone(), + display_name: Some(Some(t.pty_id.clone())), + tags: BTreeMap::new(), + }); let cmd = cli.build_run_command(&t, Path::new("/cat/hetz/demo")); let args: Vec = cmd .get_args() @@ -2519,7 +2968,7 @@ mod tests { std::fs::write( &fake, r#"#!/bin/sh -printf '%s\n' '[{"name":"h.live","status":"running","pid":41,"createdAt":"2026-07-31T10:00:00.000Z"},{"name":"h.exit","status":"exited","exitCode":0,"pid":42,"createdAt":"2026-07-31T09:00:00.000Z"},{"name":"h.gone","status":"vanished","pid":43,"createdAt":"2026-07-31T08:00:00.000Z"}]' +printf '%s\n' '[{"name":"h.live","status":"running","pid":41,"createdAt":"2026-07-31T10:00:00.000Z","displayName":"Build owner","tags":{"agent.presentation.schema":"1","unrelated":"preserved"}},{"name":"h.exit","status":"exited","exitCode":0,"pid":42,"createdAt":"2026-07-31T09:00:00.000Z"},{"name":"h.gone","status":"vanished","pid":43,"createdAt":"2026-07-31T08:00:00.000Z"}]' "#, ) .unwrap(); @@ -2544,6 +2993,18 @@ printf '%s\n' '[{"name":"h.live","status":"running","pid":41,"createdAt":"2026-0 assert!(generation.generation_id().starts_with("sha256:")); assert_eq!(first.observations[1].state, ObservedState::Exited); assert_eq!(first.observations[2].state, ObservedState::Vanished); + + let sessions = cli.list_sessions().unwrap(); + let presentation = sessions[0].presentation.as_ref().unwrap(); + assert_eq!(presentation.display_name.as_deref(), Some("Build owner")); + assert_eq!( + presentation.tags.get("agent.presentation.schema").map(String::as_str), + Some("1") + ); + assert_eq!( + presentation.tags.get("unrelated").map(String::as_str), + Some("preserved") + ); } #[test] diff --git a/tests/agent_presentation.rs b/tests/agent_presentation.rs new file mode 100644 index 00000000..6a6faf87 --- /dev/null +++ b/tests/agent_presentation.rs @@ -0,0 +1,453 @@ +use std::fs; +use std::fs::OpenOptions; +use std::os::fd::AsRawFd as _; +use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt as _, PermissionsExt as _}; +use std::os::unix::process::CommandExt as _; +use std::path::Path; +use std::process::{Command, Stdio}; + +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 + ); + let cleared_roster = Command::new(env!("CARGO_BIN_EXE_st2")) + .args([ + "--catalog", + root.to_str().unwrap(), + "agents", + "--host", + "h", + "--json", + ]) + .output() + .unwrap(); + assert!(cleared_roster.status.success()); + let rows: serde_json::Value = serde_json::from_slice(&cleared_roster.stdout).unwrap(); + assert!( + rows[0]["name"].is_null(), + "retired sibling name file was consulted" + ); + assert!(rows[0]["description"].is_null()); +} + +#[test] +fn cli_authors_positional_identity_without_an_existing_child_block() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write(root, "h/worker/agent.kdl", "agent \"worker\"\n"); + + let output = run( + root, + "rename", + &["h.worker", "Owner", "--host", "h", "--json"], + None, + ); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + fs::read_to_string(root.join("h/worker/agent.kdl")).unwrap(), + "agent \"worker\" { name \"Owner\" }\n" + ); +} + +#[test] +fn cli_clears_a_presentation_field_from_compact_kdl() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + "agent \"worker\" { host \"h\"; name \"Owner\"; command \"x\" }\n", + ); + + let output = run( + root, + "rename", + &["h.worker", "--clear", "--host", "h", "--json"], + None, + ); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let authored = fs::read_to_string(root.join("h/worker/agent.kdl")).unwrap(); + assert_eq!(authored, "agent \"worker\" { host \"h\"; command \"x\" }\n"); + let found = st2::discover(root); + assert!(found.errors.is_empty(), "{:?}", found.errors); + assert_eq!(found.specs[0].name, None); +} + +#[test] +fn cli_preserves_declaration_mode_under_a_restrictive_umask() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = root.join("h/worker/agent.kdl"); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", None, "catalog"), + ); + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + + let mut process = Command::new(env!("CARGO_BIN_EXE_st2")); + process + .args([ + "--catalog", + root.to_str().unwrap(), + "rename", + "h.worker", + "Owner", + "--host", + "h", + "--json", + ]) + .env_remove("ST_AGENT"); + unsafe { + process.pre_exec(|| { + libc::umask(0o077); + Ok(()) + }); + } + let output = process.output().unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(fs::metadata(path).unwrap().mode() & 0o777, 0o644); +} + +#[test] +fn cli_rejects_unicode_line_and_paragraph_separators_for_both_fields() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let initial = declaration("worker", None, "catalog"); + write(root, "h/worker/agent.kdl", &initial); + + for command in ["rename", "describe"] { + for separator in ['\u{2028}', '\u{2029}'] { + let value = format!("left{separator}right"); + let output = run( + root, + command, + &["h.worker", &value, "--host", "h", "--json"], + None, + ); + assert!( + !output.status.success(), + "accepted {command} U+{:04X}", + separator as u32 + ); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["result"], "error"); + assert_eq!(receipt["code"], "invalid-presentation"); + } + } + + 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) + ); +} + +#[test] +fn concurrent_cli_writers_serialize_without_losing_either_field() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", None, "catalog"), + ); + fs::create_dir(root.join(".st2")).unwrap(); + let lock_path = root.join(".st2/presentation-authoring.lock"); + let lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .mode(0o600) + .open(&lock_path) + .unwrap(); + assert_eq!(unsafe { libc::flock(lock.as_raw_fd(), libc::LOCK_EX) }, 0); + let inode = fs::metadata(&lock_path).unwrap().ino(); + + let spawn = |command: &str, value: &str| { + Command::new(env!("CARGO_BIN_EXE_st2")) + .args([ + "--catalog", + root.to_str().unwrap(), + command, + "h.worker", + value, + "--host", + "h", + "--json", + ]) + .env_remove("ST_AGENT") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap() + }; + let rename = spawn("rename", "Build owner"); + let describe = spawn("describe", "Own build delivery"); + assert_eq!(unsafe { libc::flock(lock.as_raw_fd(), libc::LOCK_UN) }, 0); + + for output in [ + rename.wait_with_output().unwrap(), + describe.wait_with_output().unwrap(), + ] { + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + let found = st2::discover(root); + assert!(found.errors.is_empty(), "{:?}", found.errors); + assert_eq!(found.specs[0].name.as_deref(), Some("Build owner")); + assert_eq!( + found.specs[0].description.as_deref(), + Some("Own build delivery") + ); + assert_eq!(fs::metadata(lock_path).unwrap().ino(), inode); +} + +#[test] +fn presentation_lock_refuses_a_symlinked_control_directory() { + use std::os::unix::fs::symlink; + + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path().join("catalog"); + let outside = temporary.path().join("outside"); + fs::create_dir(&root).unwrap(); + fs::create_dir(&outside).unwrap(); + write( + &root, + "h/worker/agent.kdl", + &declaration("worker", None, "catalog"), + ); + symlink(&outside, root.join(".st2")).unwrap(); + + let output = run( + &root, + "rename", + &["h.worker", "Owner", "--host", "h", "--json"], + None, + ); + assert!(!output.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["code"], "presentation-lock-failed"); + assert!(!outside.join("presentation-authoring.lock").exists()); +} + +#[test] +fn presentation_lock_refuses_a_symlinked_lock_file() { + use std::os::unix::fs::symlink; + + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path().join("catalog"); + let outside = temporary.path().join("outside-lock"); + fs::create_dir(&root).unwrap(); + fs::create_dir(root.join(".st2")).unwrap(); + fs::write(&outside, "unchanged").unwrap(); + write( + &root, + "h/worker/agent.kdl", + &declaration("worker", None, "catalog"), + ); + symlink(&outside, root.join(".st2/presentation-authoring.lock")).unwrap(); + + let output = run( + &root, + "describe", + &["h.worker", "Owner", "--host", "h", "--json"], + None, + ); + assert!(!output.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["code"], "presentation-lock-failed"); + assert_eq!(fs::read_to_string(outside).unwrap(), "unchanged"); +} diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 04bc0681..5dc150ed 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -802,6 +802,28 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent" { id ""; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, )], ), + ( + "presentation name", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; name "requester"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "duplicate canonical route", + "worker", + vec![ + ( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + ), + ( + "qualified", + r#"agent "evalhost.worker" { identity "evalhost.worker"; host "evalhost"; argv "sh" "-c" "sleep 60" }"#, + ), + ], + ), ]; for (expected, target, declarations) in cases { let tmp = tempfile::tempdir().unwrap(); diff --git a/tests/exec_backend.rs b/tests/exec_backend.rs index b4e469df..2c0adbc6 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..6f874820 100644 --- a/tests/hooks.rs +++ b/tests/hooks.rs @@ -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 reconcile metadata but must not run, kill, or remove" ); assert!(!hooks_root.exists()); } diff --git a/tests/message_cli.rs b/tests/message_cli.rs index c8a5969e..dde32fb5 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(); @@ -193,6 +194,70 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() { assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "1"); } +#[test] +fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bus() { + let send = |root: &Path, recipient: &str, root_flag: &str| { + let mut child = Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["message", "send", recipient, root_flag]) + .arg(root) + .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 catalog = tempfile::tempdir().unwrap(); + write_agent(catalog.path(), "worker"); + let declaration = catalog.path().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 display = send(catalog.path(), "Shared Worker", "--catalog"); + assert!(!display.status.success()); + assert!( + String::from_utf8_lossy(&display.stderr) + .contains("no agent 'Shared Worker' found in catalog") + ); + assert!(!catalog.path().join("Shared Worker").exists()); + + let stable = send(catalog.path(), "h.worker", "--catalog"); + assert!( + stable.status.success(), + "{}", + String::from_utf8_lossy(&stable.stderr) + ); + assert_eq!( + fs::read_dir(catalog.path().join("h/worker/resources/inbox")) + .unwrap() + .count(), + 1 + ); + + let flat = tempfile::tempdir().unwrap(); + let raw = send(flat.path(), "requester", "--root"); + assert!( + raw.status.success(), + "{}", + String::from_utf8_lossy(&raw.stderr) + ); + assert_eq!( + fs::read_dir(flat.path().join("requester/inbox")) + .unwrap() + .count(), + 1 + ); +} + #[test] fn orphan_mode_explicitly_reads_raw_flat_inbox_and_archive() { let tmp = tempfile::tempdir().unwrap(); diff --git a/tests/nomad_survival.rs b/tests/nomad_survival.rs index de752a4d..5958d0c1 100644 --- a/tests/nomad_survival.rs +++ b/tests/nomad_survival.rs @@ -158,6 +158,19 @@ impl Fixture { snapshot } + 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 { @@ -638,6 +651,193 @@ fn assert_managed_agent_color_contract(suffix: &str) { ); } +#[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" + ); + + let output = fx + .st2() + .env_remove("ST_AGENT") + .args([ + "--catalog", + fx.catalog.to_str().unwrap(), + "rename", + &session_id, + &session_id, + ]) + .args(["--host", HOST, "--json"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + fx.up_once(); + let lifecycle_equal = list_session(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(lifecycle_equal["createdAt"], created_at); + assert!(lifecycle_equal.get("displayName").is_none()); + assert_eq!(event_count(), initial_events + 2); + + fx.up_once(); + assert_eq!(event_count(), initial_events + 2); + + 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 + 3); + + 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..2668927b 100644 --- a/tests/reconcile.rs +++ b/tests/reconcile.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use st2::reconcile::reconcile_selected; use st2::reconcile::resolve_task; +use st2::reconcile::ObservedPtyPresentation; use st2::spec::{AgentSpec, JobType, Resource, Task, TaskKind, TaskLifecycle}; use st2::{Session, reconcile}; @@ -152,6 +153,7 @@ fn selected_reconcile_freezes_dead_keep_and_retired_task_keep() { pty_id: "host.a.x".into(), alive: false, exit_code: Some(7), + presentation: None, }], "host", "host.a.x", @@ -175,6 +177,7 @@ fn selected_reconcile_freezes_dead_keep_and_retired_task_keep() { pty_id: "host.b.x".into(), alive: false, exit_code: Some(7), + presentation: None, }], "host", "host.b.x", @@ -194,6 +197,7 @@ fn selected_reconcile_holds_adopt_only_dead_or_absent_without_mutation() { pty_id: "host.a.agent".into(), alive: false, exit_code: Some(1), + presentation: None, }], vec![], ] { @@ -247,16 +251,19 @@ fn selected_dead_non_keep_gc_and_relaunch_only_selected() { pty_id: "host.a.x".into(), alive: false, exit_code: Some(1), + presentation: None, }, Session { pty_id: "host.a.y".into(), alive: false, exit_code: Some(1), + presentation: None, }, Session { pty_id: "host.b.z".into(), alive: false, exit_code: Some(1), + presentation: None, }, ], "host", @@ -368,6 +375,8 @@ fn spec( ) -> AgentSpec { AgentSpec { identity: identity.to_string(), + name: None, + description: None, host: host.map(String::from), role: None, job_type, @@ -394,6 +403,7 @@ fn live(id: &str) -> Session { pty_id: id.to_string(), alive: true, exit_code: None, + presentation: None, } } fn dead(id: &str) -> Session { @@ -401,6 +411,7 @@ fn dead(id: &str) -> Session { pty_id: id.to_string(), alive: false, exit_code: None, + presentation: None, } } @@ -450,6 +461,133 @@ 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 lifecycle_equal_primary_name_is_cleared_during_live_reconciliation() { + let mut owner = svc( + "worker", + Some(HOST), + vec![task( + TaskKind::Pty, + "agent", + Some("hetz.worker"), + Some("codex"), + )], + ); + owner.name = Some("hetz.worker".to_owned()); + + let specs = [owner]; + let plan = reconcile(&specs, &[live("hetz.worker")], HOST); + assert_eq!(plan.presentation.len(), 1); + assert_eq!(plan.presentation[0].display_name, Some(None)); +} + +#[test] +fn live_pty_presentation_only_queues_observed_drift() { + let mut owner = svc( + "worker", + Some(HOST), + vec![task( + TaskKind::Pty, + "agent", + Some("hetz.worker"), + Some("codex"), + )], + ); + owner.name = Some("Build owner".to_owned()); + owner.description = Some("Owns build delivery".to_owned()); + let specs = [owner]; + let exact_tags = BTreeMap::from([ + ("agent.presentation.schema".to_owned(), "1".to_owned()), + ("agent.actor.path".to_owned(), "hetz.worker".to_owned()), + ( + "agent.presentation.description".to_owned(), + "Owns build delivery".to_owned(), + ), + ("unrelated".to_owned(), "preserved".to_owned()), + ]); + let exact = Session { + pty_id: "hetz.worker".to_owned(), + alive: true, + exit_code: None, + presentation: Some(ObservedPtyPresentation { + display_name: Some("Build owner".to_owned()), + tags: exact_tags.clone(), + }), + }; + assert!(reconcile(&specs, &[exact], HOST).presentation.is_empty()); + + let drifted = Session { + pty_id: "hetz.worker".to_owned(), + alive: true, + exit_code: None, + presentation: Some(ObservedPtyPresentation { + display_name: Some("Old owner".to_owned()), + tags: exact_tags, + }), + }; + assert_eq!(reconcile(&specs, &[drifted], HOST).presentation.len(), 1); + assert_eq!(reconcile(&specs, &[live("hetz.worker")], HOST).presentation.len(), 1); +} + #[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 a477a34e..650f03c6 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -6,7 +6,9 @@ use std::fs; use std::path::Path; use st2::message; -use st2::reconcile::{Session, TaskTarget}; +use st2::reconcile::{ + Launch, PtyPresentation, ReconcilePlan, Session, TaskLaunch, TaskTarget, Teardown, +}; use st2::run::Runner; use st2::run::{CrashLoop, surface_crash_loop, up_once_selected, up_once_selected_specs}; use st2::spec::{AgentSpec, JobType, Task, TaskKind, TaskLifecycle}; @@ -223,6 +225,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, @@ -451,6 +455,7 @@ struct FakeRunner { killed: RefCell>, reaped: RefCell>, removed: RefCell>, + patched: RefCell>, ops: RefCell>, } @@ -476,9 +481,17 @@ impl Runner for FakeRunner { Ok(()) } fn kill(&self, pty_id: &str) -> anyhow::Result<()> { + self.ops.borrow_mut().push(format!("kill:{pty_id}")); self.killed.borrow_mut().push(pty_id.to_string()); Ok(()) } + fn patch_presentation(&self, presentation: &PtyPresentation) -> anyhow::Result<()> { + self.ops + .borrow_mut() + .push(format!("patch:{}", presentation.pty_id)); + self.patched.borrow_mut().push(presentation.pty_id.clone()); + Ok(()) + } fn reap_for_restart(&self, pty_id: &str) -> anyhow::Result<()> { self.ops.borrow_mut().push(format!("reap:{pty_id}")); self.reaped.borrow_mut().push(pty_id.to_string()); @@ -504,6 +517,7 @@ fn live(id: &str) -> Session { pty_id: id.to_string(), alive: true, exit_code: None, + presentation: None, } } fn dead(id: &str) -> Session { @@ -511,9 +525,69 @@ fn dead(id: &str) -> Session { pty_id: id.to_string(), alive: false, exit_code: None, + presentation: None, } } +#[test] +fn lifecycle_work_precedes_a_bounded_presentation_batch() { + let spec = task_spec("owner", None, "host.owner.work"); + let target = TaskTarget { + kind: TaskKind::Exec, + pty_id: "host.owner.work".to_owned(), + bus_id: "host.owner".to_owned(), + name: "work".to_owned(), + launch: TaskLaunch::Shell("true".to_owned()), + cwd: None, + workspace: None, + tags: BTreeMap::new(), + env: BTreeMap::new(), + keep: false, + presentation: None, + }; + let presentation = (0..10) + .map(|index| PtyPresentation { + pty_id: format!("host.presented.{index}"), + display_name: Some(Some(format!("Presented {index}"))), + tags: BTreeMap::new(), + }) + .collect(); + let plan = ReconcilePlan { + launch: vec![Launch { + spec: &spec, + tasks: vec![target], + }], + teardown: vec![Teardown { + spec: &spec, + pty_ids: vec!["host.retired.work".to_owned()], + }], + presentation, + ..ReconcilePlan::default() + }; + let runner = FakeRunner::default(); + let mut report = UpReport::default(); + let mut cap = FlappingCap::default(); + + execute(&plan, &runner, &mut cap, &mut report); + + assert_eq!( + &runner.ops.borrow()[..2], + ["spawn:host.owner.work", "kill:host.retired.work"] + ); + assert_eq!(runner.patched.borrow().len(), 8); + assert!( + report + .warnings + .iter() + .any(|warning| warning.contains("deferred 2 presentation patches")) + ); + assert!(report.is_noteworthy()); + + execute(&plan, &runner, &mut cap, &mut UpReport::default()); + let patched = runner.patched.borrow(); + assert!((0..10).all(|index| patched.contains(&format!("host.presented.{index}")))); +} + /// A v2 service job: a pty agent + an exec ding. const AGENT: &str = r#" identity = "demo" 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" ); }