From dcf77b758adc5d6260af5f55781745a2a758bcce Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:09:57 +0200 Subject: [PATCH 01/22] feat: separate agent identity from presentation agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- INVARIANTS.md | 1 + README.md | 27 +- crates/agent-spec/src/kdl_format.rs | 25 + crates/agent-spec/src/spec.rs | 52 +- crates/agent-spec/tests/discovery.rs | 142 +++ ...agent-identity-and-mutable-presentation.md | 71 ++ docs/vrs/02-agent-spec/requirements.md | 7 +- docs/vrs/02-agent-spec/spec.md | 28 + docs/vrs/requirements.md | 33 + docs/vrs/spec.md | 58 + flake.lock | 8 +- flake.nix | 2 +- src/agent_author.rs | 1025 +++++++++++++++++ src/agents.rs | 25 +- src/eval_run.rs | 2 + src/eval_spec.rs | 78 +- src/exec_backend.rs | 1 + src/lib.rs | 5 +- src/main.rs | 142 ++- src/message.rs | 66 +- src/reconcile.rs | 63 +- src/run.rs | 220 +++- tests/agent_presentation.rs | 358 ++++++ tests/exec_backend.rs | 1 + tests/hooks.rs | 4 +- tests/message_cli.rs | 81 +- tests/nomad_survival.rs | 172 +++ tests/reconcile.rs | 61 + tests/run.rs | 2 + tests/status_agents.rs | 19 +- 30 files changed, 2677 insertions(+), 102 deletions(-) create mode 100644 docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md create mode 100644 src/agent_author.rs create mode 100644 tests/agent_presentation.rs diff --git a/INVARIANTS.md b/INVARIANTS.md index 1cce36c3..450f6283 100644 --- a/INVARIANTS.md +++ b/INVARIANTS.md @@ -6,6 +6,7 @@ materialization, messaging, DING, or presence must preserve them. | Invariant | Guarantee | Proof | |---|---|---| | **R11 control-plane replacement safety** | Normal stop or SIGKILL of `st2 up`, atomic binary replacement, and control-plane restart leave live PTY and exec agents usable with the same PID and creation identity. The replacement adopts each survivor exactly once, launches only genuinely missing work, and never duplicates a survivor; only explicit teardown kills an agent. | `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::explicit_teardown_kills_exec_but_plain_stop_does_not`; `tests/nomad_survival.rs::explicit_teardown_kills_pty_but_plain_stop_does_not` | +| **Stable identity with live presentation** | The positional Agent Spec identity remains the only routing, authorization, task, and lifecycle key. Optional name and description are bounded declaration metadata. st2 projects them through one exact-ID atomic PTY metadata patch; changes and clears preserve PID and creation identity, unchanged projections emit nothing, and presentation failure never authorizes restart or teardown. | `crates/agent-spec/tests/discovery.rs::presentation_bounds_count_unicode_scalars_and_reject_noncanonical_values`; `tests/agent_presentation.rs::cli_sets_replaces_and_clears_fields_without_changing_identity_or_other_bytes`; `tests/agent_presentation.rs::concurrent_cli_writers_serialize_without_losing_either_field`; `tests/reconcile.rs::live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift`; `src/run.rs::metadata_patch_uses_exact_id_and_one_json_stdin_payload`; `tests/nomad_survival.rs::presentation_changes_patch_the_exact_live_pty_without_restarting_it` | | **R23 fail-closed diagnostic inventory** | `st2 tasks --json` joins a semantically stable desired-catalog observation to exact PTY and conservatively proven exec generations without writes or reconciliation. Missing roots are positively absent without creation; declaration drift, malformed, unreadable, duplicate, PID-reused, or incomplete evidence is indeterminate, makes the typed envelope incomplete, and exits non-zero. The observational boundary does not serialize catalog writers or authorize cutover. | `src/task_inventory.rs::stable_wire_shape_maps_pty_exec_explicit_default_and_ignores_foreign_host`; `src/task_inventory.rs::complete_missing_runtime_is_absent_but_incomplete_is_indeterminate`; `src/exec_backend.rs::legacy_plain_pid_is_observed_without_rewriting_it`; `src/exec_backend.rs::record_older_than_current_process_generation_cannot_be_promoted`; `tests/task_inventory_cli.rs` | | **Transport-decoupled lifecycle** | Each task is isolated from a supervisor/transport process-group or cgroup cascade. | `tests/transport_isolation.rs`; `tests/transport_isolation_macos.rs` | | **Clean exec teardown** | Killing an exec task reaps its whole process group. | `tests/exec_backend.rs::exec_kill_reaps_the_whole_process_group_not_just_the_leader` | diff --git a/README.md b/README.md index 48811878..80275757 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,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 @@ -163,6 +165,22 @@ 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 +`meta { managed-by "nix" }` targets. `ST_AGENT` callers may edit themselves or declared +descendants; an operator without `ST_AGENT` may use the same bounded authoring path. The sibling +`/name` convention is 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 @@ -317,9 +335,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 @@ -384,7 +403,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..a5012f0f --- /dev/null +++ b/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md @@ -0,0 +1,71 @@ +# 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. Catalog agents may edit themselves or +declared descendants; operators may use the same constrained path. Nix-owned +declarations remain writable only at their Nix source. st2 serializes these +edits with 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 compatibility marker because the new fields + are optional and additive. Adoption begins only after compatible PTY and st2 + binaries are deployed. +- 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..ce31c9f6 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 activation:** st2's exact-ID presentation interface and tests are + prepared, but the current pinned PTY does not yet provide the required atomic + metadata-patch API. Keep the dependency pin unchanged until that exact PTY + implementation is finalized and verified. ## 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..e0ee8f38 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,34 @@ 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, Nix-owned declarations, + stable-ID changes, and malformed or ambiguous targets fail closed. A catalog + agent may edit itself or a descendant reached through declared supervisor + edges; an operator outside a managed agent context may use the same operation. +- **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 075a460e..ef5430a1 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -60,6 +60,64 @@ 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 checks the caller's `ST_AGENT` + self/descendant relationship when present; +3. refuses `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. + +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 8182e262..92ccf493 100644 --- a/flake.lock +++ b/flake.lock @@ -41,17 +41,17 @@ ] }, "locked": { - "lastModified": 1785413661, - "narHash": "sha256-R4wNXc0qS3Dik284DQAfxdmKe5elmMLwqhse+glMSEI=", + "lastModified": 1785509299, + "narHash": "sha256-IHooeVERLkt4xYez4z+O6hVuljpS21CbztMySreZGJA=", "owner": "compoundingtech", "repo": "pty", - "rev": "afeb3b6234b7010b7db802fd029766ad17c14219", + "rev": "706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c", "type": "github" }, "original": { "owner": "compoundingtech", "repo": "pty", - "rev": "afeb3b6234b7010b7db802fd029766ad17c14219", + "rev": "706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c", "type": "github" } }, diff --git a/flake.nix b/flake.nix index ceb9b962..601f36b8 100644 --- a/flake.nix +++ b/flake.nix @@ -6,7 +6,7 @@ flake-utils.url = "github:numtide/flake-utils"; # Packaged PTY fleet-observation gate: the exact `pty list --json` producer revision with # ambiguity-safe PID reads, EPERM handling, and one fleet-wide socket fallback budget. - pty.url = "github:compoundingtech/pty/afeb3b6234b7010b7db802fd029766ad17c14219"; + pty.url = "github:compoundingtech/pty/706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c"; pty.inputs.nixpkgs.follows = "nixpkgs"; }; diff --git a/src/agent_author.rs b/src/agent_author.rs new file mode 100644 index 00000000..21b2c933 --- /dev/null +++ b/src/agent_author.rs @@ -0,0 +1,1025 @@ +//! 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, Nix-owned declarations, and unauthorized callers fail +//! closed. The lock serializes cooperating local st2 writers; it is not a cross-host lock service. + +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, + declaration: PathBuf, + retired: bool, +} + +/// Set or clear one presentation field for one stable Agent Spec identity. +/// +/// `actor` is the caller's `ST_AGENT` identity. An absent actor is the explicit operator path. A +/// catalog-managed caller may edit itself or a descendant reached through declared supervisor +/// edges; no presentation field expands that authority. +pub fn set_presentation( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + field: PresentationField, + requested: Option<&str>, +) -> Result { + let _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, + field, + requested.as_deref(), + || {}, + )?; + Ok(PresentationReceipt { + result, + identity: target.identity, + field, + value: requested, + retired: target.retired, + }) +} + +fn resolve_target( + specs: &[crate::AgentSpec], + selector: &str, + this_host: &str, +) -> Result { + let exact = specs + .iter() + .filter(|spec| spec.bus_id(this_host) == selector) + .collect::>(); + let matches = if exact.is_empty() { + specs + .iter() + .filter(|spec| spec.identity == selector) + .collect::>() + } else { + exact + }; + match matches.as_slice() { + [] => Err(AuthorError::new( + "target-not-found", + format!("no agent {selector:?} found in the selected catalog"), + )), + [spec] => Ok(AgentTarget { + identity: spec.bus_id(this_host), + declaration: spec.path.clone(), + retired: spec.retired, + }), + many => { + let mut candidates = many + .iter() + .map(|spec| format!("{} ({})", spec.bus_id(this_host), spec.path.display())) + .collect::>(); + candidates.sort(); + Err(AuthorError::new( + "target-ambiguous", + format!( + "agent selector {selector:?} is ambiguous: {}", + candidates.join(", ") + ), + )) + } + } +} + +fn authorize_actor( + specs: &[crate::AgentSpec], + target: &str, + this_host: &str, + actor: Option<&str>, +) -> Result<(), AuthorError> { + let Some(actor) = actor else { + return Ok(()); + }; + if actor == target { + return Ok(()); + } + let by_identity = specs + .iter() + .map(|spec| (spec.bus_id(this_host), spec)) + .collect::>(); + let mut current = target.to_owned(); + let mut visited = BTreeSet::new(); + while visited.insert(current.clone()) { + let Some(spec) = by_identity.get(¤t) else { + break; + }; + let Some(supervisor) = spec.supervisor.as_deref() else { + break; + }; + if supervisor == actor { + return Ok(()); + } + let same_host = format!("{}.{}", spec.resolved_host(this_host), supervisor); + let qualified = if by_identity.contains_key(supervisor) { + supervisor.to_owned() + } else if by_identity.contains_key(&same_host) { + same_host + } else { + supervisor.to_owned() + }; + if qualified == actor { + return Ok(()); + } + current = qualified; + } + Err(AuthorError::new( + "presentation-not-authorized", + format!("agent {actor:?} may edit only itself or a declared descendant, not {target:?}"), + )) +} + +#[cfg(test)] +fn edit_declaration_for_test( + path: &Path, + expected_identity: &str, + field: PresentationField, + requested: Option<&str>, + before_commit: impl FnOnce(), +) -> Result { + edit_declaration(path, expected_identity, field, requested, before_commit) +} + +fn edit_declaration( + path: &Path, + expected_identity: &str, + field: PresentationField, + requested: Option<&str>, + before_commit: impl FnOnce(), +) -> Result { + if path.extension().and_then(|value| value.to_str()) != Some("kdl") { + return Err(AuthorError::new( + "unsupported-declaration-format", + format!( + "presentation authoring requires canonical KDL, found {}", + path.display() + ), + )); + } + let metadata = fs::symlink_metadata(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + if !metadata.file_type().is_file() { + return Err(AuthorError::new( + "unsafe-declaration-path", + format!("refusing non-regular declaration path {}", path.display()), + )); + } + let original = fs::read(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + let original_version = SourceVersion::from_metadata(&metadata); + let text = std::str::from_utf8(&original).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("declaration {} is not UTF-8: {error}", path.display()), + ) + })?; + let document = KdlDocument::parse(text).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("parsing declaration {}: {error}", path.display()), + ) + })?; + let target = exact_agent_node(&document, expected_identity)?; + if is_nix_managed(target) { + return Err(AuthorError::new( + "nix-managed-declaration", + format!( + "agent {expected_identity:?} is Nix-owned; edit its Nix source instead of {}", + path.display() + ), + )); + } + let Some(replacement) = presentation_edit(text, target, field, requested)? else { + return Ok(AuthorOutcome::Unchanged); + }; + verify_candidate(&replacement, expected_identity, field, requested)?; + atomic_replace_checked( + path, + &original, + original_version, + replacement.as_bytes(), + metadata.permissions().mode() & 0o7777, + before_commit, + )?; + Ok(AuthorOutcome::Changed) +} + +fn exact_agent_node<'a>( + document: &'a KdlDocument, + expected_identity: &str, +) -> Result<&'a KdlNode, AuthorError> { + let (expected_host, expected_agent) = expected_identity.split_once('.').ok_or_else(|| { + AuthorError::new( + "invalid-target", + format!("expected host-qualified agent identity, found {expected_identity:?}"), + ) + })?; + let matches = document + .nodes() + .iter() + .filter(|node| { + node.name().value() == "agent" + && agent_identity_parts(node).is_some_and(|(host, identity)| { + identity == expected_agent + && host.as_deref().is_none_or(|host| host == expected_host) + }) + }) + .collect::>(); + match matches.as_slice() { + [target] => Ok(*target), + [] => Err(AuthorError::new( + "target-changed", + format!("declaration no longer contains explicit agent {expected_identity:?}"), + )), + _ => Err(AuthorError::new( + "target-ambiguous", + format!("declaration contains more than one agent {expected_identity:?}"), + )), + } +} + +fn agent_identity_parts(node: &KdlNode) -> Option<(Option, String)> { + let mut identity = node + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned); + let mut host = None; + for child in node.children()?.nodes() { + match child.name().value() { + "identity" => { + identity = child + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned) + .or(identity); + } + "host" => { + host = child + .get(0) + .and_then(|value| value.as_string()) + .map(str::to_owned); + } + _ => {} + } + } + Some((host, identity?)) +} + +fn is_nix_managed(node: &KdlNode) -> bool { + node.children().is_some_and(|children| { + children + .nodes() + .iter() + .filter(|child| child.name().value() == "meta") + .filter_map(KdlNode::children) + .flat_map(|meta| meta.nodes()) + .filter(|child| child.name().value() == "managed-by") + .any(|child| child.get(0).and_then(|value| value.as_string()) == Some("nix")) + }) +} + +fn presentation_edit( + text: &str, + target: &KdlNode, + field: PresentationField, + requested: Option<&str>, +) -> Result, AuthorError> { + let fields = target + .children() + .into_iter() + .flat_map(|children| children.nodes()) + .filter(|child| child.name().value() == field.as_str()) + .collect::>(); + match fields.as_slice() { + [] => match requested { + Some(value) => insert_field(text, target, field, value).map(Some), + None => Ok(None), + }, + [node] => match requested { + Some(value) => replace_field(text, node, field, value), + None => remove_field(text, node).map(Some), + }, + _ => Err(AuthorError::new( + "duplicate-presentation-field", + format!("target declares `{}` more than once", field.as_str()), + )), + } +} + +fn parse_field_value(node: &KdlNode, field: PresentationField) -> Result<&str, AuthorError> { + if node.children().is_some() || node.entries().len() != 1 || node.entries()[0].name().is_some() + { + return Err(AuthorError::new( + "malformed-presentation-field", + format!( + "`{}` must contain exactly one positional string", + field.as_str() + ), + )); + } + node.get(0) + .and_then(|value| value.as_string()) + .ok_or_else(|| { + AuthorError::new( + "malformed-presentation-field", + format!("`{}` must contain a string", field.as_str()), + ) + }) +} + +fn quoted(value: &str) -> Result { + serde_json::to_string(value).map_err(|error| { + AuthorError::new( + "unsafe-source-edit", + format!("encode presentation string for canonical KDL: {error}"), + ) + }) +} + +fn replace_field( + text: &str, + node: &KdlNode, + field: PresentationField, + value: &str, +) -> Result, AuthorError> { + if parse_field_value(node, field)? == value { + return Ok(None); + } + let entry = &node.entries()[0]; + let span = entry.span(); + let range = span.offset()..span.offset() + span.len(); + text.get(range.clone()).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "presentation value span falls outside the declaration", + ) + })?; + let mut replacement = text.to_owned(); + replacement.replace_range(range, "ed(value)?); + Ok(Some(replacement)) +} + +fn insert_field( + text: &str, + target: &KdlNode, + field: PresentationField, + value: &str, +) -> Result { + let span = target.span(); + let start = span.offset(); + let end = start + span.len(); + let source = text.get(start..end).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "agent span falls outside the declaration", + ) + })?; + let authored = format!("{} {}", field.as_str(), quoted(value)?); + let mut replacement = text.to_owned(); + if target.children().is_none() { + replacement.insert_str(end, &format!(" {{ {authored} }}")); + return Ok(replacement); + } + if !source.ends_with('}') { + return Err(AuthorError::new( + "unsafe-source-shape", + "agent child block does not end at a source-preserving insertion point", + )); + } + let close = source.len() - 1; + if let Some(newline) = source[..close].rfind('\n') { + let closing_indent = &source[newline + 1..close]; + if !closing_indent + .chars() + .all(|value| matches!(value, ' ' | '\t')) + { + return Err(AuthorError::new( + "unsafe-source-shape", + "cannot preserve a non-whitespace closing-brace prefix", + )); + } + let child_indent = target + .children() + .and_then(|children| children.nodes().first()) + .and_then(|child| line_indent(text, child.span().offset())) + .unwrap_or_else(|| format!("{closing_indent} ")); + replacement.insert_str(start + newline + 1, &format!("{child_indent}{authored}\n")); + return Ok(replacement); + } + let before_close = &source[..close]; + let trimmed = before_close.trim_end(); + let insertion = if trimmed.ends_with('{') { + format!(" {authored}") + } else if trimmed.ends_with(';') { + format!(" {authored};") + } else { + format!("; {authored}") + }; + replacement.insert_str(start + trimmed.len(), &insertion); + Ok(replacement) +} + +fn remove_field(text: &str, node: &KdlNode) -> Result { + let span = node.span(); + let start = span.offset(); + let end = start + span.len(); + text.get(start..end).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "presentation field span falls outside the declaration", + ) + })?; + let line_start = text[..start].rfind('\n').map_or(0, |newline| newline + 1); + let line_end = text[end..] + .find('\n') + .map_or(text.len(), |newline| end + newline); + if text[line_start..start] + .chars() + .all(|value| matches!(value, ' ' | '\t')) + && text[end..line_end] + .chars() + .all(|value| matches!(value, ' ' | '\t')) + { + let mut replacement = text.to_owned(); + let remove_end = usize::min(line_end + usize::from(line_end < text.len()), text.len()); + replacement.replace_range(line_start..remove_end, ""); + return Ok(replacement); + } + Err(AuthorError::new( + "unsafe-source-shape", + "clearing presentation metadata requires the field to occupy its own canonical KDL line", + )) +} + +fn line_indent(text: &str, offset: usize) -> Option { + let prefix = text.get(..offset)?; + let start = prefix.rfind('\n').map_or(0, |newline| newline + 1); + let indent = prefix.get(start..)?; + indent + .chars() + .all(|value| matches!(value, ' ' | '\t')) + .then(|| indent.to_owned()) +} + +fn verify_candidate( + candidate: &str, + expected_identity: &str, + field: PresentationField, + expected: Option<&str>, +) -> Result<(), AuthorError> { + let document = KdlDocument::parse(candidate).map_err(|error| { + AuthorError::new( + "unsafe-source-edit", + format!("presentation edit did not produce valid KDL: {error}"), + ) + })?; + let target = exact_agent_node(&document, expected_identity)?; + let fields = target + .children() + .into_iter() + .flat_map(|children| children.nodes()) + .filter(|child| child.name().value() == field.as_str()) + .collect::>(); + let observed = match fields.as_slice() { + [] => None, + [node] => Some(parse_field_value(node, field)?), + _ => { + return Err(AuthorError::new( + "unsafe-source-edit", + format!( + "presentation edit produced duplicate `{}` fields", + field.as_str() + ), + )); + } + }; + if observed != expected { + return Err(AuthorError::new( + "unsafe-source-edit", + format!( + "presentation edit did not produce the requested `{}`", + field.as_str() + ), + )); + } + Ok(()) +} + +fn atomic_replace_checked( + path: &Path, + original: &[u8], + original_version: SourceVersion, + replacement: &[u8], + mode: u32, + before_commit: impl FnOnce(), +) -> Result<(), AuthorError> { + let directory = path.parent().ok_or_else(|| { + AuthorError::new( + "invalid-target", + format!("declaration path {} has no parent", path.display()), + ) + })?; + let temporary = directory.join(format!( + ".agent.kdl.presentation-{}-{}", + std::process::id(), + TMP_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let write = (|| -> std::io::Result<()> { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(mode) + .open(&temporary)?; + file.write_all(replacement)?; + file.sync_all() + })(); + if let Err(error) = write { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "declaration-write-failed", + format!("staging declaration {}: {error}", path.display()), + )); + } + before_commit(); + let current = fs::symlink_metadata(path) + .ok() + .filter(|metadata| metadata.file_type().is_file()) + .map(|metadata| (SourceVersion::from_metadata(&metadata), fs::read(path).ok())); + if !matches!(current, Some((version, Some(bytes))) if version == original_version && bytes == original) + { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "source-changed", + format!( + "declaration {} changed while presentation was authored", + path.display() + ), + )); + } + if let Err(error) = fs::rename(&temporary, path) { + let _ = fs::remove_file(&temporary); + return Err(AuthorError::new( + "declaration-write-failed", + format!( + "atomically publishing declaration {}: {error}", + path.display() + ), + )); + } + fs::File::open(directory) + .and_then(|directory| directory.sync_all()) + .map_err(|error| { + AuthorError::new( + "declaration-write-failed", + format!( + "syncing declaration directory {}: {error}", + directory.display() + ), + ) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write(root: &Path, relative: &str, contents: &str) -> PathBuf { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, contents).unwrap(); + path + } + + fn declaration( + identity: &str, + host: &str, + supervisor: Option<&str>, + managed_by: &str, + ) -> String { + let supervisor = supervisor + .map(|value| format!(" supervisor {value:?}\n")) + .unwrap_or_default(); + format!( + "// keep this comment\nagent {identity:?} {{\n host {host:?}\n meta {{ managed-by {managed_by:?}; keep \"exact\" }}\n{supervisor} command \"sleep 60\"\n}}\n" + ) + } + + #[test] + fn source_preserving_set_replace_idempotent_and_clear() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + let before = fs::read_to_string(&path).unwrap(); + + let set = set_presentation( + root, + "h.worker", + "h", + None, + PresentationField::Name, + Some("Build owner"), + ) + .unwrap(); + assert_eq!(set.result, AuthorOutcome::Changed); + let after_set = fs::read_to_string(&path).unwrap(); + assert_eq!(after_set.matches("name \"Build owner\"").count(), 1); + assert_eq!(after_set.replace(" name \"Build owner\"\n", ""), before); + + assert_eq!( + set_presentation( + root, + "worker", + "h", + None, + PresentationField::Name, + Some("Build owner") + ) + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!( + set_presentation( + root, + "worker", + "h", + None, + PresentationField::Name, + Some("Release owner") + ) + .unwrap() + .result, + AuthorOutcome::Changed + ); + assert_eq!( + set_presentation(root, "worker", "h", None, PresentationField::Name, None) + .unwrap() + .result, + AuthorOutcome::Changed + ); + assert_eq!(fs::read_to_string(path).unwrap(), before); + } + + #[test] + fn self_and_supervisor_can_edit_but_sibling_and_nix_owner_cannot() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/root/agent.kdl", + &declaration("root", "h", None, "catalog"), + ); + write( + root, + "h/child/agent.kdl", + &declaration("child", "h", Some("root"), "catalog"), + ); + write( + root, + "h/sibling/agent.kdl", + &declaration("sibling", "h", Some("root"), "catalog"), + ); + write( + root, + "h/nix/agent.kdl", + &declaration("nix", "h", Some("root"), "nix"), + ); + + set_presentation( + root, + "h.child", + "h", + Some("h.child"), + PresentationField::Name, + Some("self"), + ) + .unwrap(); + set_presentation( + root, + "h.child", + "h", + Some("h.root"), + PresentationField::Description, + Some("supervised"), + ) + .unwrap(); + assert_eq!( + set_presentation( + root, + "h.sibling", + "h", + Some("h.child"), + PresentationField::Name, + Some("no") + ) + .unwrap_err() + .code(), + "presentation-not-authorized" + ); + assert_eq!( + set_presentation( + root, + "h.nix", + "h", + Some("h.root"), + PresentationField::Name, + Some("no") + ) + .unwrap_err() + .code(), + "nix-managed-declaration" + ); + } + + #[test] + fn stale_source_refuses_atomic_replace() { + let temporary = tempfile::tempdir().unwrap(); + let path = write( + temporary.path(), + "agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + let changed = declaration("worker", "h", None, "external"); + let error = edit_declaration_for_test( + &path, + "h.worker", + PresentationField::Name, + Some("Owner"), + || fs::write(&path, &changed).unwrap(), + ) + .unwrap_err(); + assert_eq!(error.code(), "source-changed"); + assert_eq!(fs::read_to_string(path).unwrap(), changed); + } + + #[test] + fn source_version_rejects_byte_identical_aba_rewrite() { + let temporary = tempfile::tempdir().unwrap(); + let original = declaration("worker", "h", None, "catalog"); + let path = write(temporary.path(), "agent.kdl", &original); + let error = edit_declaration_for_test( + &path, + "h.worker", + PresentationField::Name, + Some("Owner"), + || { + fs::write(&path, "temporary competing bytes").unwrap(); + fs::write(&path, &original).unwrap(); + }, + ) + .unwrap_err(); + assert_eq!(error.code(), "source-changed"); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } +} diff --git a/src/agents.rs b/src/agents.rs index 31d161c2..48e8e647 100644 --- a/src/agents.rs +++ b/src/agents.rs @@ -19,8 +19,10 @@ pub struct AgentRow { pub identity: String, /// Effective presence (derived: stale → `unknown`, etc.). pub status: State, - /// Optional display name (`/name`), else `None`. + /// Optional display name from the Agent Spec declaration. pub name: Option, + /// Optional enduring responsibility boundary from the Agent Spec declaration. + pub description: Option, /// Whether the declaration is explicitly retired. Presence remains a separate runtime signal. pub retired: bool, /// Typed Resource bindings declared directly by the agent. @@ -44,7 +46,8 @@ pub fn roster(catalog_root: &Path, this_host: &str) -> Vec { Some(AgentRow { identity: s.bus_id(this_host), status: status::read_state(&status::status_path(agent_dir)), - name: read_name(agent_dir), + name: s.name.clone(), + description: s.description.clone(), retired: s.retired, resources: s.resources.clone(), last_activity_ms: newest_mtime_ms(agent_dir), @@ -62,6 +65,7 @@ struct SummaryJson<'a> { identity: &'a str, status: &'a str, name: Option<&'a str>, + description: Option<&'a str>, retired: bool, resources: &'a [Resource], } @@ -72,6 +76,7 @@ struct EnrichedJson<'a> { identity: &'a str, status: &'a str, name: Option<&'a str>, + description: Option<&'a str>, retired: bool, resources: &'a [Resource], #[serde(rename = "lastActivity")] @@ -88,6 +93,7 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { identity: &r.identity, status: r.status.as_str(), name: r.name.as_deref(), + description: r.description.as_deref(), retired: r.retired, resources: &r.resources, last_activity: r.last_activity_ms, @@ -102,6 +108,7 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { identity: &r.identity, status: r.status.as_str(), name: r.name.as_deref(), + description: r.description.as_deref(), retired: r.retired, resources: &r.resources, }) @@ -110,13 +117,6 @@ pub fn to_json(rows: &[AgentRow], enrich: bool) -> String { } } -/// `/name` first line, if non-empty. -fn read_name(agent_dir: &Path) -> Option { - let raw = fs::read_to_string(agent_dir.join("name")).ok()?; - let first = raw.lines().next().unwrap_or("").trim(); - (!first.is_empty()).then(|| first.to_string()) -} - /// Count logically unread messages in the agent's `resources/inbox`. A same-filename archive receipt /// suppresses and cleans a raw inbox duplicate restored by eventually-consistent sync. fn inbox_count(agent_dir: &Path) -> usize { @@ -169,6 +169,7 @@ mod tests { identity: identity.to_string(), status, name: name.map(str::to_string), + description: None, retired, resources: Vec::new(), last_activity_ms: last, @@ -193,11 +194,11 @@ mod tests { assert_eq!( to_json(&rows, false), - r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"retired":false,"resources":[]},{"identity":"hetz.st2-claude","status":"busy","name":"owner","retired":true,"resources":[]}]"# + r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"description":null,"retired":false,"resources":[]},{"identity":"hetz.st2-claude","status":"busy","name":"owner","description":null,"retired":true,"resources":[]}]"# ); assert_eq!( to_json(&rows, true), - r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"retired":false,"resources":[],"lastActivity":1784653027733.6138,"inbox":1},{"identity":"hetz.st2-claude","status":"busy","name":"owner","retired":true,"resources":[],"lastActivity":null,"inbox":0}]"# + r#"[{"identity":"hetz.cos-claude","status":"available","name":null,"description":null,"retired":false,"resources":[],"lastActivity":1784653027733.6138,"inbox":1},{"identity":"hetz.st2-claude","status":"busy","name":"owner","description":null,"retired":true,"resources":[],"lastActivity":null,"inbox":0}]"# ); // Empty roster is `[]`, not `null`. assert_eq!(to_json(&[], true), "[]"); @@ -224,7 +225,7 @@ mod tests { assert_eq!( to_json(&[resource_row], false), - r#"[{"identity":"hetz.worker","status":"available","name":null,"retired":false,"resources":[{"name":"work","_tag":"vendor-specific-type","uri":"vendor+thing://authority/exact%20identity"}]}]"# + r#"[{"identity":"hetz.worker","status":"available","name":null,"description":null,"retired":false,"resources":[{"name":"work","_tag":"vendor-specific-type","uri":"vendor+thing://authority/exact%20identity"}]}]"# ); } } diff --git a/src/eval_run.rs b/src/eval_run.rs index 2f7689f4..c4f23874 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, 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..403447c8 100644 --- a/src/exec_backend.rs +++ b/src/exec_backend.rs @@ -938,6 +938,7 @@ mod generation_observation_tests { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } 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..75500605 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`. @@ -296,8 +300,9 @@ enum Command { /// running agent needs no flags. #[derive(Args)] struct MsgCtx { - /// Legacy catalog/bus root override. Prefer global `--catalog`; defaults to `$CATALOG`, then the - /// default st2 catalog. + /// Low-level bus root override. Declared identities use native resource boxes; any absent + /// identity is treated as an exact flat mailbox id. Prefer global `--catalog` for strict Agent + /// Spec routing. #[arg(long, conflicts_with = "catalog_path")] root: Option, /// The acting identity — who the message is `from` / whose inbox is "mine". Defaults to @@ -309,6 +314,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 +605,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 +1222,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 +1330,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, ); } @@ -1265,11 +1360,11 @@ fn ding_cmd( // id. So `st2 ding --identity mix.worker` pokes pty `mix.worker` (the redundant positional is now // optional). An explicit positional still overrides for the rare non-agent case. let session = session.unwrap_or_else(|| id.clone()); - // Flat-bus aware: a native catalog agent → its resources/inbox; a catalog-LESS bus (an eval's - // ST_ROOT) → the flat //inbox. Status lives beside it either way. + // Explicit-root transport still resolves a declared agent to its native resource inbox; only + // an absent identity uses the exact flat / mailbox. Status lives beside it either way. let agent_dir = message::resolve_agent_dir(&catalog_root, &id, &this_host) .unwrap_or_else(|| catalog_root.join(&id)); - let inbox = message::resolve_inbox(&catalog_root, &id, &this_host); + let inbox = message::resolve_inbox(&catalog_root, &id, &this_host, routing_mode(&ctx))?; let status_path = st2::status::status_path(&agent_dir); eprintln!( "st2 ding: watching {}'s inbox ({}) → poking pty '{session}'", @@ -1293,6 +1388,14 @@ fn resolve_ctx(ctx: &MsgCtx) -> Result<(PathBuf, String)> { Ok((root, host)) } +fn routing_mode(ctx: &MsgCtx) -> message::RoutingMode { + if ctx.root.is_some() { + message::RoutingMode::ExplicitRoot + } else { + message::RoutingMode::Catalog + } +} + /// The acting identity (`from` / whose inbox is "mine"): `--as`, else `$ST_AGENT`. fn acting_id(ctx: &MsgCtx) -> Result { ctx.as_id @@ -1343,9 +1446,10 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host); + let dir = message::resolve_inbox(&root, &to, &host, mode)?; let filename = message::send_to_inbox( &dir, &from, @@ -1364,8 +1468,9 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; - let my_inbox = message::resolve_inbox(&root, &from, &host); + let my_inbox = message::resolve_inbox(&root, &from, &host, mode)?; let original = message::read_msg(&my_inbox, &filename) .with_context(|| format!("no message '{filename}' in {}'s inbox", from))?; let to = original @@ -1374,7 +1479,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { .with_context(|| format!("message '{filename}' has no `from` to reply to"))?; let subject = subject.or_else(|| message::reply_subject(original.subject.as_deref())); let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host); + let dir = message::resolve_inbox(&root, &to, &host, mode)?; let sent = message::send_to_inbox( &dir, &from, @@ -1398,11 +1503,12 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let id = match identity { Some(id) => id, None => acting_id(&ctx)?, }; - let dir = message::resolve_list_box(&root, &id, &host, archive, orphan)?; + let dir = message::resolve_list_box(&root, &id, &host, archive, orphan, mode)?; let mut msgs = if archive { message::list_dir(&dir)? } else { @@ -1448,12 +1554,13 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; let dir = if archive { - message::resolve_archive(&root, &id, &host) + message::resolve_archive(&root, &id, &host, mode) } else { - message::resolve_inbox(&root, &id, &host) - }; + message::resolve_inbox(&root, &id, &host, mode) + }?; if raw { print!("{}", std::fs::read_to_string(dir.join(&filename))?); return Ok(()); @@ -1479,12 +1586,11 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { } MessageCmd::Archive { first, second, ctx } => { let (root, host) = resolve_ctx(&ctx)?; + let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; - message::archive_msg( - &message::resolve_inbox(&root, &id, &host), - &message::resolve_archive(&root, &id, &host), - &filename, - )?; + let inbox = message::resolve_inbox(&root, &id, &host, mode)?; + let archive = message::resolve_archive(&root, &id, &host, mode)?; + message::archive_msg(&inbox, &archive, &filename)?; println!("archived"); Ok(()) } diff --git a/src/message.rs b/src/message.rs index 3cb497f3..4d464899 100644 --- a/src/message.rs +++ b/src/message.rs @@ -314,38 +314,49 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf { agent_dir.join("resources").join("archive") } -/// The inbox dir for `id` under `root`: the NATIVE catalog inbox (`/resources/inbox`) if a -/// catalog agent is discoverable, else the flat bus inbox (`//inbox`). The -/// flat fallback lets `st2 ding`/`st2 message` operate on a catalog-LESS bus — e.g. an eval's ST_ROOT, -/// where agents are booted from a single spec (no on-disk `agent.kdl` to discover). `root` is whatever -/// `--root`/ST_ROOT names, so the layout follows the spec, never a hardcoded path. -pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> PathBuf { - match resolve_agent_dir(root, id, host) { - Some(dir) => inbox_dir(&dir), - None => root.join(id).join("inbox"), - } +/// How a message command selected its bus root. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum RoutingMode { + /// Catalog-derived routing accepts only declared Agent Spec identities. + Catalog, + /// Explicit `--root` is the low-level transport boundary: declared identities still use their + /// native resource boxes, while an absent identity names an exact flat mailbox. + ExplicitRoot, } -/// The archive dir for `id` under `root` — native catalog archive if discoverable, else the flat -/// `//archive` (companion to [`resolve_inbox`]). -pub fn resolve_archive(root: &Path, id: &str, host: &str) -> PathBuf { - match resolve_agent_dir(root, id, host) { - Some(dir) => archive_dir(&dir), - None => root.join(id).join("archive"), - } +/// Resolve the inbox for `id` under `root` according to the caller's routing boundary. +pub fn resolve_inbox( + root: &Path, + id: &str, + host: &str, + mode: RoutingMode, +) -> anyhow::Result { + resolve_list_box(root, id, host, false, false, mode) +} + +/// The archive companion to [`resolve_inbox`], with the same catalog-backed strictness and +/// explicit-root flat transport. +pub fn resolve_archive( + root: &Path, + id: &str, + host: &str, + mode: RoutingMode, +) -> anyhow::Result { + resolve_list_box(root, id, host, true, false, mode) } /// Resolve one box for `message ls`. /// -/// The permissive flat layout is automatic only when discovery proves that `root` is catalog-less. -/// Once any valid or malformed declaration makes it a catalog, an absent identity is an error. -/// `orphan` is the explicit recovery path for inspecting a raw flat box inside such a root. +/// Catalog-derived routing rejects absent identities. Explicit `--root` routing treats an absent +/// identity as an exact flat mailbox id; it never resolves presentation names. `orphan` is the +/// explicit recovery path for inspecting a raw flat box regardless of routing mode. pub fn resolve_list_box( root: &Path, id: &str, host: &str, archive: bool, orphan: bool, + mode: RoutingMode, ) -> anyhow::Result { let flat = || { root.join(id) @@ -368,7 +379,7 @@ pub fn resolve_list_box( }); } - if discovered.specs.is_empty() && discovered.errors.is_empty() { + if mode == RoutingMode::ExplicitRoot { return Ok(flat()); } anyhow::bail!("no agent '{id}' found in catalog {}", root.display()) @@ -649,16 +660,16 @@ mod tests { } #[test] - fn resolve_inbox_falls_back_to_the_flat_bus_when_catalog_less() { + fn explicit_root_resolves_declared_agents_then_falls_back_to_exact_flat_ids() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); // No catalog under root → the flat bus (//inbox|archive). assert_eq!( - resolve_inbox(root, "mix.sup", "h"), + resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), root.join("mix.sup").join("inbox") ); assert_eq!( - resolve_archive(root, "mix.sup", "h"), + resolve_archive(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), root.join("mix.sup").join("archive") ); // A discoverable native catalog agent → its resources/inbox. @@ -670,8 +681,13 @@ mod tests { ) .unwrap(); assert_eq!( - resolve_inbox(root, "mix.sup", "h"), + resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), ad.join("resources").join("inbox") ); + assert_eq!( + resolve_inbox(root, "requester", "h", RoutingMode::ExplicitRoot).unwrap(), + root.join("requester").join("inbox") + ); + assert!(resolve_inbox(root, "requester", "h", RoutingMode::Catalog).is_err()); } } diff --git a/src/reconcile.rs b/src/reconcile.rs index f69c3953..2c7b1206 100644 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -47,6 +47,47 @@ pub struct TaskTarget { pub env: BTreeMap, /// GC pin (task-level `keep`, or the agent-level `keep`). pub keep: bool, + /// Desired PTY-only presentation projected at spawn. Exec tasks carry `None`. + pub presentation: Option, +} + +/// Exact, non-lifecycle metadata desired for one managed PTY. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PtyPresentation { + /// Exact stable PTY task ID. Automation must never resolve this as a display alias. + pub pty_id: String, + /// `Some(value)` updates primary-agent display metadata; `Some(None)` clears it. `None` preserves + /// a secondary task's existing task-specific display convention. + pub display_name: Option>, + /// Complete st2-owned tag snapshot. `None` removes an optional owned key. + pub tags: BTreeMap>, +} + +pub const AGENT_PRESENTATION_SCHEMA_TAG: &str = "agent.presentation.schema"; +pub const AGENT_ACTOR_PATH_TAG: &str = "agent.actor.path"; +pub const AGENT_DESCRIPTION_TAG: &str = "agent.presentation.description"; + +fn pty_presentation( + spec: &AgentSpec, + task: &crate::spec::Task, + pty_id: &str, + bus_id: &str, +) -> Option { + if task.kind != TaskKind::Pty { + return None; + } + Some(PtyPresentation { + pty_id: pty_id.to_owned(), + display_name: (task.name == "agent").then(|| spec.name.clone()), + tags: BTreeMap::from([ + ( + AGENT_PRESENTATION_SCHEMA_TAG.to_owned(), + Some("1".to_owned()), + ), + (AGENT_ACTOR_PATH_TAG.to_owned(), Some(bus_id.to_owned())), + (AGENT_DESCRIPTION_TAG.to_owned(), spec.description.clone()), + ]), + }) } /// A resolved task launch accepted by the execution backends. @@ -89,6 +130,8 @@ pub struct ReconcilePlan<'a> { pub gc: Vec, /// Dead or absent `adopt-only` task ids held without reap or launch. pub held: Vec, + /// In-place presentation updates for healthy managed PTYs, independent of lifecycle actions. + pub presentation: Vec, } /// Resolve one exact local task selector (`host.agent.task` or explicit task id) without mutation. @@ -164,7 +207,7 @@ pub fn reconcile_selected<'a>( let target = TaskTarget { kind: task.kind, pty_id: runtime.clone(), - bus_id, + bus_id: bus_id.clone(), name: task.name.clone(), launch, cwd: task.cwd.clone(), @@ -172,9 +215,15 @@ pub fn reconcile_selected<'a>( tags: task.tags.clone(), env, keep: task.keep || owner.keep, + presentation: pty_presentation(owner, task, &runtime, &bus_id), }; match actual { - Some(s) if s.alive => plan.adopt.push(owner), + Some(s) if s.alive => { + if let Some(presentation) = target.presentation.clone() { + plan.presentation.push(presentation); + } + plan.adopt.push(owner); + } _ if task.lifecycle == TaskLifecycle::AdoptOnly => plan.held.push(runtime), Some(_) if target.keep => plan.adopt.push(owner), Some(_) => { @@ -280,10 +329,11 @@ pub fn reconcile<'a>( } else { env.remove("ST_SUPERVISOR"); } + let pty_id = resolve_task_id(&bus_id, &t.name, t.id.as_deref()); Some(( TaskTarget { kind: t.kind, - pty_id: resolve_task_id(&bus_id, &t.name, t.id.as_deref()), + pty_id: pty_id.clone(), bus_id: bus_id.clone(), name: t.name.clone(), launch, @@ -292,6 +342,7 @@ pub fn reconcile<'a>( tags: t.tags.clone(), env, keep: t.keep || spec.keep, + presentation: pty_presentation(spec, t, &pty_id, &bus_id), }, t.lifecycle, )) @@ -304,7 +355,11 @@ pub fn reconcile<'a>( let held_before = plan.held.len(); for (target, lifecycle) in targets { match session_state(&by_id, &target.pty_id) { - SessionState::Alive => {} + SessionState::Alive => { + if let Some(presentation) = target.presentation.clone() { + plan.presentation.push(presentation); + } + } SessionState::Dead | SessionState::Absent if lifecycle == TaskLifecycle::AdoptOnly => { diff --git a/src/run.rs b/src/run.rs index fb81a121..cdc42dde 100644 --- a/src/run.rs +++ b/src/run.rs @@ -14,7 +14,7 @@ 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::unix::fs::MetadataExt as _; use std::os::unix::process::CommandExt as _; use std::path::{Path, PathBuf}; @@ -23,12 +23,13 @@ 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, @@ -43,10 +44,22 @@ const PTY_DAEMON_SHUTDOWN_WAIT: Duration = Duration::from_secs(6); /// descendant that inherited stdout/stderr from blocking cleanup after the direct child times out. /// The child still gets a fresh process group so the common wrapper-and-descendants case is reaped. fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Result { + output_with_input_timeout(command, timeout, None) +} + +fn output_with_input_timeout( + command: &mut Command, + timeout: Duration, + input: Option<&[u8]>, +) -> anyhow::Result { let mut stdout = tempfile::tempfile()?; let mut stderr = tempfile::tempfile()?; command - .stdin(Stdio::null()) + .stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) .stdout(Stdio::from(stdout.try_clone()?)) .stderr(Stdio::from(stderr.try_clone()?)); unsafe { @@ -59,6 +72,13 @@ fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Resu }); } let mut child = command.spawn()?; + if let Some(input) = input { + child + .stdin + .take() + .context("metadata patch child has no piped stdin")? + .write_all(input)?; + } let pid = child.id() as i32; let deadline = Instant::now() + timeout; let status = loop { @@ -114,6 +134,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. @@ -160,6 +185,13 @@ struct PtyListEntry { created_at: Option, } +#[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, /// e.g. an eval run's short `/tmp/stev-` that dodges the 104-byte unix-socket-path limit that a /// deep `/pty` would blow — else what the catalog itself declares @@ -243,17 +275,40 @@ impl PtyCli { .arg("-d") // detached: leave it running in the background .arg("--force") // st2 itself may run inside a pty session; allow nesting .args(["--id", &target.pty_id]); - // Keep the adoption key task-specific, but make a differing human-facing label the owning - // agent's stable bus identity instead of pty's auto-derived `-sh` label. When the - // lifecycle id already IS that identity, suppress pty's automatic `-sh` alias: pty - // rejects displayName == id, and no displayName makes the UI fall back to the stable id. - if target.pty_id == target.bus_id { - cmd.arg("--no-display-name"); - } else { - cmd.args(["--name", &target.bus_id]); + match target + .presentation + .as_ref() + .map(|presentation| &presentation.display_name) + { + Some(Some(Some(name))) => { + cmd.args(["--name", name]); + } + Some(Some(None)) => { + cmd.arg("--no-display-name"); + } + // Secondary tasks retain the established task-specific presentation convention. + _ if target.pty_id == target.bus_id => { + cmd.arg("--no-display-name"); + } + _ => { + cmd.args(["--name", &target.bus_id]); + } } cmd.arg("--cwd").arg(&cwd); - for (k, v) in &target.tags { + let mut tags = target.tags.clone(); + if let Some(presentation) = &target.presentation { + for (key, value) in &presentation.tags { + match value { + Some(value) => { + tags.insert(key.clone(), value.clone()); + } + None => { + tags.remove(key); + } + } + } + } + for (k, v) in &tags { cmd.arg("--tag").arg(format!("{k}={}", self.expand(v))); } // Managed agent and DING sessions retain PTY exit evidence until the lifecycle owner records @@ -436,6 +491,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)) } @@ -523,6 +601,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") @@ -707,6 +789,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), @@ -850,6 +936,15 @@ pub fn execute( cap: &mut FlappingCap, report: &mut UpReport, ) { + // Presentation is an independent metadata path. A failure is visible and retried by the next + // reconcile pass, but never authorizes stop, reap, restart, or replacement. + for presentation in &plan.presentation { + if let Err(error) = runner.patch_presentation(presentation) { + report + .errors + .push(format!("metadata patch {}: {error}", presentation.pty_id)); + } + } // The corpses tied to a launch target (dead, non-keep, active ptys) are reaped inside the launch // loop so a parked flapper keeps its evidence. Everything else in `gc` (e.g. a retired agent's // dead sessions) is reaped here. @@ -1592,7 +1687,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 { @@ -1607,6 +1702,7 @@ mod tests { tags: BTreeMap::new(), env: BTreeMap::new(), keep: false, + presentation: None, } } @@ -1637,6 +1733,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, @@ -1726,6 +1824,8 @@ mod tests { fn spec_fixture() -> AgentSpec { AgentSpec { identity: "demo".into(), + name: None, + description: None, host: Some("hetz".into()), role: None, job_type: JobType::Service, @@ -1962,7 +2062,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(); @@ -1995,6 +2095,96 @@ mod tests { ); } + #[test] + fn build_run_command_projects_primary_name_and_owned_tags_at_spawn() { + let cli = PtyCli::default(); + let mut t = target("hetz.demo", "codex"); + t.bus_id = "hetz.demo".to_owned(); + t.tags.insert("unrelated".to_owned(), "preserved".to_owned()); + t.presentation = Some(PtyPresentation { + pty_id: "hetz.demo".to_owned(), + display_name: Some(Some("Build owner".to_owned())), + tags: BTreeMap::from([ + ( + "agent.presentation.schema".to_owned(), + Some("1".to_owned()), + ), + ( + "agent.actor.path".to_owned(), + Some("hetz.demo".to_owned()), + ), + ("agent.presentation.description".to_owned(), None), + ]), + }); + let cmd = cli.build_run_command(&t, Path::new("/cat/hetz/demo")); + let args = cmd + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + + let name = args.iter().position(|arg| arg == "--name").unwrap(); + assert_eq!(args[name + 1], "Build owner"); + let tags = args + .windows(2) + .filter(|pair| pair[0] == "--tag") + .map(|pair| pair[1].as_str()) + .collect::>(); + assert!(tags.contains("unrelated=preserved")); + assert!(tags.contains("agent.presentation.schema=1")); + assert!(tags.contains("agent.actor.path=hetz.demo")); + assert!( + !tags + .iter() + .any(|tag| tag.starts_with("agent.presentation.description=")) + ); + } + + #[test] + fn metadata_patch_uses_exact_id_and_one_json_stdin_payload() { + use std::os::unix::fs::PermissionsExt as _; + + let temporary = tempfile::tempdir().unwrap(); + let executable = temporary.path().join("pty-capture"); + std::fs::write( + &executable, + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$0.args\"\ncat > \"$0.stdin\"\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + let cli = PtyCli { + bin: executable.display().to_string(), + catalog_root: temporary.path().to_path_buf(), + }; + let presentation = PtyPresentation { + pty_id: "stable.agent.id".to_owned(), + display_name: Some(None), + tags: BTreeMap::from([ + ( + "agent.presentation.schema".to_owned(), + Some("1".to_owned()), + ), + ("agent.presentation.description".to_owned(), None), + ]), + }; + + cli.patch_presentation(&presentation).unwrap(); + + assert_eq!( + std::fs::read_to_string(executable.with_extension("args")).unwrap(), + "metadata\npatch\n--id\nstable.agent.id\n" + ); + let payload: serde_json::Value = serde_json::from_slice( + &std::fs::read(executable.with_extension("stdin")).unwrap(), + ) + .unwrap(); + assert_eq!(payload["displayName"], serde_json::Value::Null); + assert_eq!(payload["tags"]["agent.presentation.schema"], "1"); + assert_eq!( + payload["tags"]["agent.presentation.description"], + serde_json::Value::Null + ); + } + #[test] fn build_run_command_passes_direct_argv_without_a_shell() { let cli = PtyCli::new(PathBuf::from("/my/catalog")); diff --git a/tests/agent_presentation.rs b/tests/agent_presentation.rs new file mode 100644 index 00000000..5965dcc7 --- /dev/null +++ b/tests/agent_presentation.rs @@ -0,0 +1,358 @@ +use std::fs; +use std::fs::OpenOptions; +use std::os::fd::AsRawFd as _; +use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt 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_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/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..1b2eaa8a 100644 --- a/tests/message_cli.rs +++ b/tests/message_cli.rs @@ -1,8 +1,9 @@ //! CLI coverage for message-list filters and output modes. use std::fs; +use std::io::Write as _; use std::path::Path; -use std::process::Command; +use std::process::{Command, Stdio}; fn write_message(inbox: &Path, ts_ms: u64, suffix: &str, from: &str) { fs::create_dir_all(inbox).unwrap(); @@ -25,9 +26,14 @@ fn write_agent(root: &Path, identity: &str) { .unwrap(); } -fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { +fn list_identity_with_flag( + root: &Path, + identity: &str, + root_flag: &str, + extra: &[&str], +) -> std::process::Output { Command::new(env!("CARGO_BIN_EXE_st2")) - .args(["message", "ls", identity, "--root"]) + .args(["message", "ls", identity, root_flag]) .arg(root) .args(["--host", "h"]) .args(extra) @@ -35,6 +41,10 @@ fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::O .unwrap() } +fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { + list_identity_with_flag(root, identity, "--root", extra) +} + fn list(root: &Path, extra: &[&str]) -> std::process::Output { list_identity(root, "bob", extra) } @@ -147,7 +157,7 @@ fn an_unknown_catalog_identity_fails_before_every_output_mode_reads_a_box() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity(tmp.path(), "missing", &extra); + let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(), @@ -193,6 +203,67 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() { assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "1"); } +#[test] +fn routing_mode_separates_catalog_identities_from_explicit_root_transport() { + let tmp = tempfile::tempdir().unwrap(); + let catalog = tmp.path(); + write_agent(catalog, "worker"); + let declaration = catalog.join("h/worker/agent.kdl"); + fs::write( + &declaration, + fs::read_to_string(&declaration) + .unwrap() + .replace(" type \"service\"\n", " type \"service\"\n name \"Shared Worker\"\n"), + ) + .unwrap(); + + let send = |recipient: &str, root_flag: &str| { + let mut child = Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["message", "send", recipient, root_flag]) + .arg(catalog) + .args(["--host", "h", "--as", "h.sender"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child.stdin.take().unwrap().write_all(b"work\n").unwrap(); + child.wait_with_output().unwrap() + }; + + let refused = send("Shared Worker", "--catalog"); + assert!(!refused.status.success()); + assert!(String::from_utf8_lossy(&refused.stderr).contains("no agent 'Shared Worker' found")); + assert!(!catalog.join("Shared Worker").exists()); + + let declared = send("h.worker", "--root"); + assert!( + declared.status.success(), + "{}", + String::from_utf8_lossy(&declared.stderr) + ); + let listed = list_identity(catalog, "h.worker", &["--count"]); + assert!( + listed.status.success(), + "{}", + String::from_utf8_lossy(&listed.stderr) + ); + assert_eq!(String::from_utf8_lossy(&listed.stdout).trim(), "1"); + + let flat = send("requester", "--root"); + assert!( + flat.status.success(), + "{}", + String::from_utf8_lossy(&flat.stderr) + ); + assert_eq!( + fs::read_dir(catalog.join("requester/inbox")) + .unwrap() + .count(), + 1 + ); +} + #[test] fn orphan_mode_explicitly_reads_raw_flat_inbox_and_archive() { let tmp = tempfile::tempdir().unwrap(); @@ -255,7 +326,7 @@ fn malformed_catalog_declarations_disable_implicit_flat_fallback() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity(tmp.path(), "missing", &extra); + let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(), diff --git a/tests/nomad_survival.rs b/tests/nomad_survival.rs index 0682c2c0..309b5967 100644 --- a/tests/nomad_survival.rs +++ b/tests/nomad_survival.rs @@ -135,6 +135,19 @@ impl Fixture { std::fs::write(path, kdl).unwrap(); } + fn write_presented_compact_agent(&self, identity: &str, name: &str, description: &str) { + self.pty_sessions + .borrow_mut() + .push(format!("{HOST}.{identity}")); + let kdl = format!( + "agent \"{identity}\" {{\n identity \"{identity}\"\n host \"{HOST}\"\n \ + type \"service\"\n name {name:?}\n description {description:?}\n command \"{TASK_CMD}\"\n}}\n" + ); + let path = self.catalog.join(HOST).join(identity).join("agent.kdl"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, kdl).unwrap(); + } + /// Write a PTY task that records every st2-managed environment value and its cwd on each boot. /// The append-only snapshot lets a test compare the initial launch with a manual `pty restart`. fn write_restart_env_agent(&self, identity: &str) -> PathBuf { @@ -494,6 +507,165 @@ fn pty_sessions_use_unique_agent_identity_display_names_and_preserve_lifecycle() } } +#[test] +fn presentation_changes_patch_the_exact_live_pty_without_restarting_it() { + if !pty_gate("presentation_changes_patch_the_exact_live_pty_without_restarting_it") { + return; + } + let fx = Fixture::new(); + let identity = "presented"; + let session_id = format!("{HOST}.{identity}"); + fx.write_presented_compact_agent(identity, "Build owner", "Owns build delivery"); + + let launched = fx.up_once(); + assert!(launched.contains(&session_id), "output:\n{launched}"); + let pidfile = fx.pty_root.join(format!("{session_id}.pid")); + let initial_pid = read_pid(&pidfile).unwrap(); + let list_session = || { + let output = Command::new("pty") + .env("PTY_ROOT", &fx.pty_root) + .args(["list", "--json"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice::(&output.stdout) + .unwrap() + .as_array() + .unwrap() + .iter() + .find(|session| session["name"] == session_id) + .unwrap() + .clone() + }; + let event_count = || { + let output = Command::new("pty") + .env("PTY_ROOT", &fx.pty_root) + .args(["events", "--recent", "--json", &session_id]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout) + .matches("metadata_change") + .count() + }; + let initial = list_session(); + let created_at = initial["createdAt"].clone(); + assert_eq!(initial["displayName"], "Build owner"); + assert_eq!(initial["tags"]["agent.presentation.schema"], "1"); + assert_eq!(initial["tags"]["agent.actor.path"], session_id); + assert_eq!( + initial["tags"]["agent.presentation.description"], + "Owns build delivery" + ); + let initial_events = event_count(); + + for (command, value) in [ + ("rename", "Release owner"), + ("describe", "Owns release delivery"), + ] { + let output = fx + .st2() + .env_remove("ST_AGENT") + .args([ + "--catalog", + fx.catalog.to_str().unwrap(), + command, + &session_id, + value, + ]) + .args(["--host", HOST, "--json"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + let adopted = fx.up_once(); + assert!( + adopted.contains("adopted (1): presented"), + "output:\n{adopted}" + ); + let changed = list_session(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(changed["createdAt"], created_at); + assert_eq!(changed["displayName"], "Release owner"); + assert_eq!( + changed["tags"]["agent.presentation.description"], + "Owns release delivery" + ); + assert_eq!(event_count(), initial_events + 1); + + fx.up_once(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!( + event_count(), + initial_events + 1, + "unchanged projection emitted an event" + ); + + for command in ["rename", "describe"] { + let output = fx + .st2() + .env_remove("ST_AGENT") + .args([ + "--catalog", + fx.catalog.to_str().unwrap(), + command, + &session_id, + "--clear", + ]) + .args(["--host", HOST, "--json"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + fx.up_once(); + let cleared = list_session(); + assert_eq!(read_pid(&pidfile), Some(initial_pid)); + assert_eq!(cleared["createdAt"], created_at); + assert!(cleared.get("displayName").is_none()); + assert!( + cleared["tags"] + .get("agent.presentation.description") + .is_none() + ); + assert_eq!(event_count(), initial_events + 2); + + let declaration = fx.catalog.join(HOST).join(identity).join("agent.kdl"); + let source = std::fs::read_to_string(&declaration).unwrap(); + std::fs::write( + &declaration, + source.replace( + " type \"service\"\n", + " type \"service\"\n retired #true\n", + ), + ) + .unwrap(); + let retired = fx.up_once(); + assert!( + retired.contains(&format!("torn down (1): {session_id}")), + "output:\n{retired}" + ); + assert!( + poll_until(DEATH_TIMEOUT, || !read_alive(&pidfile)), + "the genuine retirement lifecycle change did not stop the PTY" + ); +} + #[test] fn manual_pty_restart_preserves_every_st2_managed_environment_and_config_value() { if !pty_gate("manual_pty_restart_preserves_every_st2_managed_environment_and_config_value") { diff --git a/tests/reconcile.rs b/tests/reconcile.rs index d4253755..f2a307bd 100644 --- a/tests/reconcile.rs +++ b/tests/reconcile.rs @@ -368,6 +368,8 @@ fn spec( ) -> AgentSpec { AgentSpec { identity: identity.to_string(), + name: None, + description: None, host: host.map(String::from), role: None, job_type, @@ -450,6 +452,65 @@ fn all_tasks_live_is_adopted() { assert_eq!(plan.adopt.len(), 1); } +#[test] +fn live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift() { + let mut owner = svc( + "worker", + Some(HOST), + vec![ + task( + TaskKind::Pty, + "agent", + Some("hetz.worker"), + Some("codex"), + ), + task( + TaskKind::Pty, + "shell", + Some("hetz.worker.shell"), + Some("sh"), + ), + ], + ); + owner.name = Some("Build owner".to_owned()); + owner.description = Some("Owns build delivery".to_owned()); + let specs = [owner]; + let plan = reconcile( + &specs, + &[live("hetz.worker"), live("hetz.worker.shell")], + HOST, + ); + + assert!(plan.launch.is_empty()); + assert!(plan.teardown.is_empty()); + assert!(plan.gc.is_empty()); + assert_eq!(plan.presentation.len(), 2); + let primary = plan + .presentation + .iter() + .find(|item| item.pty_id == "hetz.worker") + .unwrap(); + assert_eq!(primary.display_name, Some(Some("Build owner".to_owned()))); + assert_eq!( + primary.tags, + BTreeMap::from([ + ("agent.presentation.schema".to_owned(), Some("1".to_owned())), + ("agent.actor.path".to_owned(), Some("hetz.worker".to_owned())), + ( + "agent.presentation.description".to_owned(), + Some("Owns build delivery".to_owned()), + ), + ]) + ); + let secondary = plan + .presentation + .iter() + .find(|item| item.pty_id == "hetz.worker.shell") + .unwrap(); + assert_eq!(secondary.display_name, None); + assert_eq!(secondary.tags, primary.tags); +} + #[test] fn resource_only_changes_do_not_replace_or_relaunch_a_live_task() { let mut spec = svc( diff --git a/tests/run.rs b/tests/run.rs index a477a34e..297184a2 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -223,6 +223,8 @@ fn selected_one_shot_unknown_refuses_before_runner_list() { fn task_spec(identity: &str, host: Option<&str>, id: &str) -> AgentSpec { AgentSpec { identity: identity.into(), + name: None, + description: None, host: host.map(str::to_owned), role: None, job_type: JobType::Service, diff --git a/tests/status_agents.rs b/tests/status_agents.rs index 474facf4..2ef82661 100644 --- a/tests/status_agents.rs +++ b/tests/status_agents.rs @@ -1,6 +1,6 @@ //! M2.3 integration: presence status + the agent roster over a discovered catalog. Unit mechanics //! (state parse, staleness, atomic set/refresh) live in `src/status.rs`; this covers the composition -//! `st2 agents` relies on — enumerate specs, read each agent's `status`/`name`/inbox, project the +//! `st2 agents` relies on — enumerate specs, read each agent's status/presentation/inbox, project the //! roster, and derive `unknown` from staleness. use std::fs; @@ -26,6 +26,13 @@ fn agent_kdl(identity: &str, host: &str) -> String { ) } +fn presented_agent_kdl(identity: &str, host: &str) -> String { + agent_kdl(identity, host).replace( + " type \"service\"\n", + " type \"service\"\n name \"st2 owner\"\n description \"Own st2 delivery\"\n", + ) +} + fn retired_agent_kdl(identity: &str, host: &str) -> String { agent_kdl(identity, host).replace( " type \"service\"\n", @@ -42,7 +49,7 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { write( root, "hetz/st2-claude/agent.kdl", - &agent_kdl("st2-claude", "hetz"), + &presented_agent_kdl("st2-claude", "hetz"), ); write( root, @@ -55,15 +62,14 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { &agent_kdl("fabric-claude", "silber"), ); - // Presence: st2-claude busy, cos-claude available, fabric-claude unset (→ offline). A display name - // and an inbox message for st2-claude. + // Presence: st2-claude busy, cos-claude available, fabric-claude unset (→ offline). Presentation + // metadata and an inbox message belong to st2-claude's declaration. set_state(&status_path(&root.join("hetz/st2-claude")), State::Busy).unwrap(); set_state( &status_path(&root.join("hetz/cos-claude")), State::Available, ) .unwrap(); - write(root, "hetz/st2-claude/name", "st2 owner\n"); send_to_inbox( &st2::message::inbox_dir(&root.join("hetz/st2-claude")), "hetz.cos-claude", @@ -104,6 +110,7 @@ fn roster_projects_presence_name_and_enrich_across_the_catalog() { .unwrap(); assert_eq!(st2c.status, State::Busy); assert_eq!(st2c.name.as_deref(), Some("st2 owner")); + assert_eq!(st2c.description.as_deref(), Some("Own st2 delivery")); assert!(!st2c.retired); assert_eq!( st2c.inbox, 1, @@ -201,7 +208,7 @@ fn roster_json_and_human_output_distinguish_retirement_from_presence() { ); assert_eq!( String::from_utf8(human.stdout).unwrap(), - "h.live\tavailable\t\nh.retired\tbusy\t\t[retired]\n" + "h.live\tavailable\t\t\nh.retired\tbusy\t\t\t[retired]\n" ); } From 47047dedb16053d86675658bcd31ef76c014ca6b Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:16:38 +0200 Subject: [PATCH 02/22] fix: narrow presentation review scope agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- INVARIANTS.md | 1 - README.md | 8 +- ...agent-identity-and-mutable-presentation.md | 16 ++-- docs/vrs/02-agent-spec/spec.md | 8 +- docs/vrs/requirements.md | 11 ++- docs/vrs/spec.md | 14 +++- src/agent_author.rs | 32 +++++--- src/main.rs | 46 ++++------- src/message.rs | 66 ++++++--------- tests/message_cli.rs | 81 ++----------------- 10 files changed, 102 insertions(+), 181 deletions(-) diff --git a/INVARIANTS.md b/INVARIANTS.md index 450f6283..1cce36c3 100644 --- a/INVARIANTS.md +++ b/INVARIANTS.md @@ -6,7 +6,6 @@ materialization, messaging, DING, or presence must preserve them. | Invariant | Guarantee | Proof | |---|---|---| | **R11 control-plane replacement safety** | Normal stop or SIGKILL of `st2 up`, atomic binary replacement, and control-plane restart leave live PTY and exec agents usable with the same PID and creation identity. The replacement adopts each survivor exactly once, launches only genuinely missing work, and never duplicates a survivor; only explicit teardown kills an agent. | `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_exec_unchanged_without_duplicate`; `tests/nomad_survival.rs::normal_stop_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::forced_kill_and_binary_replacement_adopt_pty_unchanged_without_duplicate`; `tests/nomad_survival.rs::explicit_teardown_kills_exec_but_plain_stop_does_not`; `tests/nomad_survival.rs::explicit_teardown_kills_pty_but_plain_stop_does_not` | -| **Stable identity with live presentation** | The positional Agent Spec identity remains the only routing, authorization, task, and lifecycle key. Optional name and description are bounded declaration metadata. st2 projects them through one exact-ID atomic PTY metadata patch; changes and clears preserve PID and creation identity, unchanged projections emit nothing, and presentation failure never authorizes restart or teardown. | `crates/agent-spec/tests/discovery.rs::presentation_bounds_count_unicode_scalars_and_reject_noncanonical_values`; `tests/agent_presentation.rs::cli_sets_replaces_and_clears_fields_without_changing_identity_or_other_bytes`; `tests/agent_presentation.rs::concurrent_cli_writers_serialize_without_losing_either_field`; `tests/reconcile.rs::live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift`; `src/run.rs::metadata_patch_uses_exact_id_and_one_json_stdin_payload`; `tests/nomad_survival.rs::presentation_changes_patch_the_exact_live_pty_without_restarting_it` | | **R23 fail-closed diagnostic inventory** | `st2 tasks --json` joins a semantically stable desired-catalog observation to exact PTY and conservatively proven exec generations without writes or reconciliation. Missing roots are positively absent without creation; declaration drift, malformed, unreadable, duplicate, PID-reused, or incomplete evidence is indeterminate, makes the typed envelope incomplete, and exits non-zero. The observational boundary does not serialize catalog writers or authorize cutover. | `src/task_inventory.rs::stable_wire_shape_maps_pty_exec_explicit_default_and_ignores_foreign_host`; `src/task_inventory.rs::complete_missing_runtime_is_absent_but_incomplete_is_indeterminate`; `src/exec_backend.rs::legacy_plain_pid_is_observed_without_rewriting_it`; `src/exec_backend.rs::record_older_than_current_process_generation_cannot_be_promoted`; `tests/task_inventory_cli.rs` | | **Transport-decoupled lifecycle** | Each task is isolated from a supervisor/transport process-group or cgroup cascade. | `tests/transport_isolation.rs`; `tests/transport_isolation_macos.rs` | | **Clean exec teardown** | Killing an exec task reaps its whole process group. | `tests/exec_backend.rs::exec_kill_reaps_the_whole_process_group_not_just_the_leader` | diff --git a/README.md b/README.md index 80275757..dae0a79c 100644 --- a/README.md +++ b/README.md @@ -177,9 +177,11 @@ 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 -`meta { managed-by "nix" }` targets. `ST_AGENT` callers may edit themselves or declared -descendants; an operator without `ST_AGENT` may use the same bounded authoring path. The sibling -`/name` convention is hard-retired and ignored. +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 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 index a5012f0f..e8d3530f 100644 --- a/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md +++ b/docs/vrs/.decisions/0002-stable-agent-identity-and-mutable-presentation.md @@ -31,9 +31,12 @@ Spec declaration is their sole source of truth; a sibling `name` file is neither read nor written. Constrained KDL-only commands may mutate one presentation field without -publishing a second representation. Catalog agents may edit themselves or -declared descendants; operators may use the same constrained path. Nix-owned -declarations remain writable only at their Nix source. st2 serializes these +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 @@ -55,9 +58,10 @@ retries without restart or lifecycle accounting. disambiguation and automation. - The old equality between stable identity and every presentation surface is superseded, but stable routing semantics are preserved. -- Existing declarations require no compatibility marker because the new fields - are optional and additive. Adoption begins only after compatible PTY and st2 - binaries are deployed. +- 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. diff --git a/docs/vrs/02-agent-spec/spec.md b/docs/vrs/02-agent-spec/spec.md index ce31c9f6..eeb317a6 100644 --- a/docs/vrs/02-agent-spec/spec.md +++ b/docs/vrs/02-agent-spec/spec.md @@ -279,10 +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 activation:** st2's exact-ID presentation interface and tests are - prepared, but the current pinned PTY does not yet provide the required atomic - metadata-patch API. Keep the dependency pin unchanged until that exact PTY - implementation is finalized and verified. +- **G09, F17 release ordering:** source authoring requires Nix emitters to mark + generated declarations before the compatible st2 binary is activated. The + pinned PTY dependency provides the exact-ID atomic metadata-patch API, but + remains a draft merge dependency. ## Acceptance cases diff --git a/docs/vrs/requirements.md b/docs/vrs/requirements.md index e0ee8f38..3b7740a0 100644 --- a/docs/vrs/requirements.md +++ b/docs/vrs/requirements.md @@ -166,10 +166,13 @@ accepted. 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, Nix-owned declarations, - stable-ID changes, and malformed or ambiguous targets fail closed. A catalog - agent may edit itself or a descendant reached through declared supervisor - edges; an operator outside a managed agent context may use the same operation. + 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 diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index ef5430a1..1d08ba43 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -90,14 +90,20 @@ st2 neither reads, writes, migrates, nor interprets it. 1. acquires the persistent exclusive `/.st2/presentation-authoring.lock` before discovery; -2. resolves exactly one declaration and checks the caller's `ST_AGENT` - self/descendant relationship when present; -3. refuses `meta { managed-by "nix" }`, unsupported formats, malformed catalogs, - and ambiguous targets; +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 diff --git a/src/agent_author.rs b/src/agent_author.rs index 21b2c933..bcca9db7 100644 --- a/src/agent_author.rs +++ b/src/agent_author.rs @@ -2,8 +2,9 @@ //! //! 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, Nix-owned declarations, and unauthorized callers fail -//! closed. The lock serializes cooperating local st2 writers; it is not a cross-host lock service. +//! 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; @@ -30,9 +31,9 @@ struct PresentationAuthorLock { 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 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!( @@ -53,7 +54,10 @@ impl PresentationAuthorLock { 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::anyhow!( + "re-read catalog control path {}: {error}", + control.display() + ) })?; anyhow::ensure!( metadata.is_dir() && !metadata.file_type().is_symlink(), @@ -91,10 +95,12 @@ impl PresentationAuthorLock { .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()))?; + .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: {}", @@ -224,9 +230,9 @@ struct AgentTarget { /// Set or clear one presentation field for one stable Agent Spec identity. /// -/// `actor` is the caller's `ST_AGENT` identity. An absent actor is the explicit operator path. A -/// catalog-managed caller may edit itself or a descendant reached through declared supervisor -/// edges; no presentation field expands that authority. +/// `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, diff --git a/src/main.rs b/src/main.rs index 75500605..d9bf105d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -300,9 +300,8 @@ enum Command { /// running agent needs no flags. #[derive(Args)] struct MsgCtx { - /// Low-level bus root override. Declared identities use native resource boxes; any absent - /// identity is treated as an exact flat mailbox id. Prefer global `--catalog` for strict Agent - /// Spec routing. + /// Legacy catalog/bus root override. Prefer global `--catalog`; defaults to `$CATALOG`, then the + /// default st2 catalog. #[arg(long, conflicts_with = "catalog_path")] root: Option, /// The acting identity — who the message is `from` / whose inbox is "mine". Defaults to @@ -1360,11 +1359,11 @@ fn ding_cmd( // id. So `st2 ding --identity mix.worker` pokes pty `mix.worker` (the redundant positional is now // optional). An explicit positional still overrides for the rare non-agent case. let session = session.unwrap_or_else(|| id.clone()); - // Explicit-root transport still resolves a declared agent to its native resource inbox; only - // an absent identity uses the exact flat / mailbox. Status lives beside it either way. + // Flat-bus aware: a native catalog agent → its resources/inbox; a catalog-LESS bus (an eval's + // ST_ROOT) → the flat //inbox. Status lives beside it either way. 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, routing_mode(&ctx))?; + 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}'", @@ -1388,14 +1387,6 @@ fn resolve_ctx(ctx: &MsgCtx) -> Result<(PathBuf, String)> { Ok((root, host)) } -fn routing_mode(ctx: &MsgCtx) -> message::RoutingMode { - if ctx.root.is_some() { - message::RoutingMode::ExplicitRoot - } else { - message::RoutingMode::Catalog - } -} - /// The acting identity (`from` / whose inbox is "mine"): `--as`, else `$ST_AGENT`. fn acting_id(ctx: &MsgCtx) -> Result { ctx.as_id @@ -1446,10 +1437,9 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; - let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host, mode)?; + let dir = message::resolve_inbox(&root, &to, &host); let filename = message::send_to_inbox( &dir, &from, @@ -1468,9 +1458,8 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; - let mode = routing_mode(&ctx); let from = acting_id(&ctx)?; - let my_inbox = message::resolve_inbox(&root, &from, &host, mode)?; + 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 @@ -1479,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, mode)?; + let dir = message::resolve_inbox(&root, &to, &host); let sent = message::send_to_inbox( &dir, &from, @@ -1503,12 +1492,11 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; - let mode = routing_mode(&ctx); let id = match identity { Some(id) => id, None => acting_id(&ctx)?, }; - let dir = message::resolve_list_box(&root, &id, &host, archive, orphan, mode)?; + let dir = message::resolve_list_box(&root, &id, &host, archive, orphan)?; let mut msgs = if archive { message::list_dir(&dir)? } else { @@ -1554,13 +1542,12 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { ctx, } => { let (root, host) = resolve_ctx(&ctx)?; - let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; let dir = if archive { - message::resolve_archive(&root, &id, &host, mode) + message::resolve_archive(&root, &id, &host) } else { - message::resolve_inbox(&root, &id, &host, mode) - }?; + message::resolve_inbox(&root, &id, &host) + }; if raw { print!("{}", std::fs::read_to_string(dir.join(&filename))?); return Ok(()); @@ -1586,11 +1573,12 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { } MessageCmd::Archive { first, second, ctx } => { let (root, host) = resolve_ctx(&ctx)?; - let mode = routing_mode(&ctx); let (id, filename) = box_target(first, second, &ctx)?; - let inbox = message::resolve_inbox(&root, &id, &host, mode)?; - let archive = message::resolve_archive(&root, &id, &host, mode)?; - message::archive_msg(&inbox, &archive, &filename)?; + message::archive_msg( + &message::resolve_inbox(&root, &id, &host), + &message::resolve_archive(&root, &id, &host), + &filename, + )?; println!("archived"); Ok(()) } diff --git a/src/message.rs b/src/message.rs index 4d464899..3cb497f3 100644 --- a/src/message.rs +++ b/src/message.rs @@ -314,49 +314,38 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf { agent_dir.join("resources").join("archive") } -/// How a message command selected its bus root. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum RoutingMode { - /// Catalog-derived routing accepts only declared Agent Spec identities. - Catalog, - /// Explicit `--root` is the low-level transport boundary: declared identities still use their - /// native resource boxes, while an absent identity names an exact flat mailbox. - ExplicitRoot, -} - -/// Resolve the inbox for `id` under `root` according to the caller's routing boundary. -pub fn resolve_inbox( - root: &Path, - id: &str, - host: &str, - mode: RoutingMode, -) -> anyhow::Result { - resolve_list_box(root, id, host, false, false, mode) +/// The 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"), + } } -/// The archive companion to [`resolve_inbox`], with the same catalog-backed strictness and -/// explicit-root flat transport. -pub fn resolve_archive( - root: &Path, - id: &str, - host: &str, - mode: RoutingMode, -) -> anyhow::Result { - resolve_list_box(root, id, host, true, false, mode) +/// 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"), + } } /// Resolve one box for `message ls`. /// -/// Catalog-derived routing rejects absent identities. Explicit `--root` routing treats an absent -/// identity as an exact flat mailbox id; it never resolves presentation names. `orphan` is the -/// explicit recovery path for inspecting a raw flat box regardless of routing mode. +/// The permissive flat layout is automatic only when discovery proves that `root` is catalog-less. +/// Once any valid or malformed declaration makes it a catalog, an absent identity is an error. +/// `orphan` is the explicit recovery path for inspecting a raw flat box inside such a root. pub fn resolve_list_box( root: &Path, id: &str, host: &str, archive: bool, orphan: bool, - mode: RoutingMode, ) -> anyhow::Result { let flat = || { root.join(id) @@ -379,7 +368,7 @@ pub fn resolve_list_box( }); } - if mode == RoutingMode::ExplicitRoot { + if discovered.specs.is_empty() && discovered.errors.is_empty() { return Ok(flat()); } anyhow::bail!("no agent '{id}' found in catalog {}", root.display()) @@ -660,16 +649,16 @@ mod tests { } #[test] - fn explicit_root_resolves_declared_agents_then_falls_back_to_exact_flat_ids() { + fn resolve_inbox_falls_back_to_the_flat_bus_when_catalog_less() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); // No catalog under root → the flat bus (//inbox|archive). assert_eq!( - resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), + resolve_inbox(root, "mix.sup", "h"), root.join("mix.sup").join("inbox") ); assert_eq!( - resolve_archive(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), + resolve_archive(root, "mix.sup", "h"), root.join("mix.sup").join("archive") ); // A discoverable native catalog agent → its resources/inbox. @@ -681,13 +670,8 @@ mod tests { ) .unwrap(); assert_eq!( - resolve_inbox(root, "mix.sup", "h", RoutingMode::ExplicitRoot).unwrap(), + resolve_inbox(root, "mix.sup", "h"), ad.join("resources").join("inbox") ); - assert_eq!( - resolve_inbox(root, "requester", "h", RoutingMode::ExplicitRoot).unwrap(), - root.join("requester").join("inbox") - ); - assert!(resolve_inbox(root, "requester", "h", RoutingMode::Catalog).is_err()); } } diff --git a/tests/message_cli.rs b/tests/message_cli.rs index 1b2eaa8a..c8a5969e 100644 --- a/tests/message_cli.rs +++ b/tests/message_cli.rs @@ -1,9 +1,8 @@ //! 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, Stdio}; +use std::process::Command; fn write_message(inbox: &Path, ts_ms: u64, suffix: &str, from: &str) { fs::create_dir_all(inbox).unwrap(); @@ -26,14 +25,9 @@ fn write_agent(root: &Path, identity: &str) { .unwrap(); } -fn list_identity_with_flag( - root: &Path, - identity: &str, - root_flag: &str, - extra: &[&str], -) -> std::process::Output { +fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { Command::new(env!("CARGO_BIN_EXE_st2")) - .args(["message", "ls", identity, root_flag]) + .args(["message", "ls", identity, "--root"]) .arg(root) .args(["--host", "h"]) .args(extra) @@ -41,10 +35,6 @@ fn list_identity_with_flag( .unwrap() } -fn list_identity(root: &Path, identity: &str, extra: &[&str]) -> std::process::Output { - list_identity_with_flag(root, identity, "--root", extra) -} - fn list(root: &Path, extra: &[&str]) -> std::process::Output { list_identity(root, "bob", extra) } @@ -157,7 +147,7 @@ fn an_unknown_catalog_identity_fails_before_every_output_mode_reads_a_box() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); + let out = list_identity(tmp.path(), "missing", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(), @@ -203,67 +193,6 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() { assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "1"); } -#[test] -fn routing_mode_separates_catalog_identities_from_explicit_root_transport() { - let tmp = tempfile::tempdir().unwrap(); - let catalog = tmp.path(); - write_agent(catalog, "worker"); - let declaration = catalog.join("h/worker/agent.kdl"); - fs::write( - &declaration, - fs::read_to_string(&declaration) - .unwrap() - .replace(" type \"service\"\n", " type \"service\"\n name \"Shared Worker\"\n"), - ) - .unwrap(); - - let send = |recipient: &str, root_flag: &str| { - let mut child = Command::new(env!("CARGO_BIN_EXE_st2")) - .args(["message", "send", recipient, root_flag]) - .arg(catalog) - .args(["--host", "h", "--as", "h.sender"]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .unwrap(); - child.stdin.take().unwrap().write_all(b"work\n").unwrap(); - child.wait_with_output().unwrap() - }; - - let refused = send("Shared Worker", "--catalog"); - assert!(!refused.status.success()); - assert!(String::from_utf8_lossy(&refused.stderr).contains("no agent 'Shared Worker' found")); - assert!(!catalog.join("Shared Worker").exists()); - - let declared = send("h.worker", "--root"); - assert!( - declared.status.success(), - "{}", - String::from_utf8_lossy(&declared.stderr) - ); - let listed = list_identity(catalog, "h.worker", &["--count"]); - assert!( - listed.status.success(), - "{}", - String::from_utf8_lossy(&listed.stderr) - ); - assert_eq!(String::from_utf8_lossy(&listed.stdout).trim(), "1"); - - let flat = send("requester", "--root"); - assert!( - flat.status.success(), - "{}", - String::from_utf8_lossy(&flat.stderr) - ); - assert_eq!( - fs::read_dir(catalog.join("requester/inbox")) - .unwrap() - .count(), - 1 - ); -} - #[test] fn orphan_mode_explicitly_reads_raw_flat_inbox_and_archive() { let tmp = tempfile::tempdir().unwrap(); @@ -326,7 +255,7 @@ fn malformed_catalog_declarations_disable_implicit_flat_fallback() { vec!["--archive", "--json"], vec!["--archive", "--count"], ] { - let out = list_identity_with_flag(tmp.path(), "missing", "--catalog", &extra); + let out = list_identity(tmp.path(), "missing", &extra); assert!(!out.status.success(), "mode {extra:?} must fail"); assert!( out.stdout.is_empty(), From 68809389124be00fe085718523b15c522063a259 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:28:50 +0200 Subject: [PATCH 03/22] fix: preserve compact agent presentation sources agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/agent_author.rs | 71 +++++++++++++++++++------- tests/agent_presentation.rs | 99 ++++++++++++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 21 deletions(-) diff --git a/src/agent_author.rs b/src/agent_author.rs index bcca9db7..0a588df9 100644 --- a/src/agent_author.rs +++ b/src/agent_author.rs @@ -497,22 +497,24 @@ fn agent_identity_parts(node: &KdlNode) -> Option<(Option, String)> { .and_then(|value| value.as_string()) .map(str::to_owned); let mut host = None; - for child in node.children()?.nodes() { - match child.name().value() { - "identity" => { - identity = child - .get(0) - .and_then(|value| value.as_string()) - .map(str::to_owned) - .or(identity); - } - "host" => { - host = child - .get(0) - .and_then(|value| value.as_string()) - .map(str::to_owned); + 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?)) @@ -698,10 +700,40 @@ fn remove_field(text: &str, node: &KdlNode) -> Result { replacement.replace_range(line_start..remove_end, ""); return Ok(replacement); } - Err(AuthorError::new( - "unsafe-source-shape", - "clearing presentation metadata requires the field to occupy its own canonical KDL line", - )) + + 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 { @@ -783,6 +815,7 @@ fn atomic_replace_checked( .create_new(true) .mode(mode) .open(&temporary)?; + file.set_permissions(fs::Permissions::from_mode(mode))?; file.write_all(replacement)?; file.sync_all() })(); diff --git a/tests/agent_presentation.rs b/tests/agent_presentation.rs index 5965dcc7..6a6faf87 100644 --- a/tests/agent_presentation.rs +++ b/tests/agent_presentation.rs @@ -1,7 +1,8 @@ use std::fs; use std::fs::OpenOptions; use std::os::fd::AsRawFd as _; -use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt 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}; @@ -129,10 +130,104 @@ fn cli_sets_replaces_and_clears_fields_without_changing_identity_or_other_bytes( .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]["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(); From 2d2cd0dc16c79708c3778553bbb218e15189179b Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:40:19 +0200 Subject: [PATCH 04/22] build: repin reviewed pty metadata patch agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- flake.lock | 8 ++++---- flake.nix | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/flake.lock b/flake.lock index 92ccf493..3a23ab82 100644 --- a/flake.lock +++ b/flake.lock @@ -41,17 +41,17 @@ ] }, "locked": { - "lastModified": 1785509299, - "narHash": "sha256-IHooeVERLkt4xYez4z+O6hVuljpS21CbztMySreZGJA=", + "lastModified": 1785515623, + "narHash": "sha256-NXTWG/SIctaloeAxJP9GI+8ea2MWFs32bYlgT6WBpC4=", "owner": "compoundingtech", "repo": "pty", - "rev": "706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c", + "rev": "0deb3f0ad6eb07f30ad9dea6392b843d4495ad5f", "type": "github" }, "original": { "owner": "compoundingtech", "repo": "pty", - "rev": "706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c", + "rev": "0deb3f0ad6eb07f30ad9dea6392b843d4495ad5f", "type": "github" } }, diff --git a/flake.nix b/flake.nix index 601f36b8..a2845bcb 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 fleet-observation gate: the exact `pty list --json` producer revision with - # ambiguity-safe PID reads, EPERM handling, and one fleet-wide socket fallback budget. - pty.url = "github:compoundingtech/pty/706e4f5e4cd88fe24ce5b8afb7dbd507bd3e215c"; + # Packaged PTY dependency: the exact reviewed revision with atomic metadata patching and the + # fleet-observation guarantees required by st2 reconciliation. + pty.url = "github:compoundingtech/pty/0deb3f0ad6eb07f30ad9dea6392b843d4495ad5f"; pty.inputs.nixpkgs.follows = "nixpkgs"; }; From 79b0e483d1a2f324386ac57498ca45d78157f4c7 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:56:51 +0200 Subject: [PATCH 05/22] fix: fail closed on catalog presentation routes agent-session-id: dev3.cos-fr-relay agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/main.rs | 18 ++++++------ src/message.rs | 31 ++++++++------------ tests/message_cli.rs | 67 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/src/main.rs b/src/main.rs index d9bf105d..eecdfc01 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1363,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}'", @@ -1439,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, @@ -1459,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 @@ -1468,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, @@ -1547,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(()); @@ -1574,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..e67c4a10 100644 --- a/src/message.rs +++ b/src/message.rs @@ -314,25 +314,15 @@ 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; +/// once declarations or discovery errors make it a catalog, an absent identity fails closed. +pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> anyhow::Result { + resolve_list_box(root, id, host, false, false) } -/// 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 +644,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. @@ -670,8 +660,9 @@ mod tests { ) .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()); } } 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(); From 4061a1d9d5ae1add726768ab64b70cee9cadee87 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:00:11 +0200 Subject: [PATCH 06/22] fix: reap metadata child after stdin failure agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/src/run.rs b/src/run.rs index 25bb9dbc..a7476031 100644 --- a/src/run.rs +++ b/src/run.rs @@ -73,11 +73,24 @@ fn output_with_input_timeout( } let mut child = command.spawn()?; if let Some(input) = input { - child + let write = child .stdin .take() - .context("metadata patch child has no piped stdin")? - .write_all(input)?; + .context("metadata patch child has no piped stdin") + .and_then(|mut stdin| { + stdin + .write_all(input) + .context("write metadata patch payload") + }); + if let Err(error) = write { + let pid = child.id() as i32; + unsafe { + libc::kill(-pid, libc::SIGKILL); + } + let _ = child.kill(); + let _ = child.wait(); + return Err(error); + } } let pid = child.id() as i32; let deadline = Instant::now() + timeout; @@ -2200,6 +2213,41 @@ mod tests { ); } + #[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 build_run_command_passes_direct_argv_without_a_shell() { let cli = PtyCli::new(PathBuf::from("/my/catalog")); From 624550cbb369b68865f442d975f34891685db034 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:10:51 +0200 Subject: [PATCH 07/22] fix: bound metadata child reaping agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 70 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/src/run.rs b/src/run.rs index a7476031..e118ab5c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -18,7 +18,7 @@ use std::io::{Read as _, Seek as _, Write 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, Command, Output, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{Receiver, RecvTimeoutError, channel}; use std::time::{Duration, Instant}; @@ -47,6 +47,31 @@ fn output_with_timeout(command: &mut Command, timeout: Duration) -> anyhow::Resu 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 output_with_input_timeout( command: &mut Command, timeout: Duration, @@ -72,6 +97,8 @@ fn output_with_input_timeout( }); } let mut child = command.spawn()?; + let pid = child.id() as i32; + let deadline = Instant::now() + timeout; if let Some(input) = input { let write = child .stdin @@ -83,32 +110,16 @@ fn output_with_input_timeout( .context("write metadata patch payload") }); if let Err(error) = write { - let pid = child.id() as i32; - unsafe { - libc::kill(-pid, libc::SIGKILL); - } - let _ = child.kill(); - let _ = child.wait(); + terminate_and_reap_before(child, pid, deadline); return Err(error); } } - let pid = child.id() as i32; - let deadline = Instant::now() + timeout; 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)); @@ -2248,6 +2259,27 @@ mod tests { ); } + #[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")); From afb40bbe9bb0acabf8e4f2f30d05b527cd1e7652 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:32:01 +0200 Subject: [PATCH 08/22] fix: preserve dotted host identities agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/agent_author.rs | 75 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 10 deletions(-) diff --git a/src/agent_author.rs b/src/agent_author.rs index 0a588df9..d4128302 100644 --- a/src/agent_author.rs +++ b/src/agent_author.rs @@ -224,6 +224,8 @@ impl std::error::Error for AuthorError {} #[derive(Debug)] struct AgentTarget { identity: String, + source_host: String, + source_identity: String, declaration: PathBuf, retired: bool, } @@ -270,6 +272,8 @@ pub fn set_presentation( let result = edit_declaration( &target.declaration, &target.identity, + &target.source_host, + &target.source_identity, field, requested.as_deref(), || {}, @@ -307,6 +311,8 @@ fn resolve_target( )), [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, }), @@ -378,16 +384,28 @@ fn authorize_actor( 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, field, requested, before_commit) + 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(), @@ -432,7 +450,7 @@ fn edit_declaration( format!("parsing declaration {}: {error}", path.display()), ) })?; - let target = exact_agent_node(&document, expected_identity)?; + let target = exact_agent_node(&document, expected_identity, expected_host, expected_agent)?; if is_nix_managed(target) { return Err(AuthorError::new( "nix-managed-declaration", @@ -445,7 +463,14 @@ fn edit_declaration( let Some(replacement) = presentation_edit(text, target, field, requested)? else { return Ok(AuthorOutcome::Unchanged); }; - verify_candidate(&replacement, expected_identity, field, requested)?; + verify_candidate( + &replacement, + expected_identity, + expected_host, + expected_agent, + field, + requested, + )?; atomic_replace_checked( path, &original, @@ -460,13 +485,9 @@ fn edit_declaration( fn exact_agent_node<'a>( document: &'a KdlDocument, expected_identity: &str, + expected_host: &str, + expected_agent: &str, ) -> Result<&'a KdlNode, AuthorError> { - let (expected_host, expected_agent) = expected_identity.split_once('.').ok_or_else(|| { - AuthorError::new( - "invalid-target", - format!("expected host-qualified agent identity, found {expected_identity:?}"), - ) - })?; let matches = document .nodes() .iter() @@ -749,6 +770,8 @@ fn line_indent(text: &str, offset: usize) -> Option { fn verify_candidate( candidate: &str, expected_identity: &str, + expected_host: &str, + expected_agent: &str, field: PresentationField, expected: Option<&str>, ) -> Result<(), AuthorError> { @@ -758,7 +781,7 @@ fn verify_candidate( format!("presentation edit did not produce valid KDL: {error}"), ) })?; - let target = exact_agent_node(&document, expected_identity)?; + let target = exact_agent_node(&document, expected_identity, expected_host, expected_agent)?; let fields = target .children() .into_iter() @@ -950,6 +973,34 @@ mod tests { 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 self_and_supervisor_can_edit_but_sibling_and_nix_owner_cannot() { let temporary = tempfile::tempdir().unwrap(); @@ -1033,6 +1084,8 @@ mod tests { let error = edit_declaration_for_test( &path, "h.worker", + "h", + "worker", PresentationField::Name, Some("Owner"), || fs::write(&path, &changed).unwrap(), @@ -1050,6 +1103,8 @@ mod tests { let error = edit_declaration_for_test( &path, "h.worker", + "h", + "worker", PresentationField::Name, Some("Owner"), || { From d758dd15aaa4554cb9b514559858fcfb3d8f35df Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:33:02 +0200 Subject: [PATCH 09/22] fix: bound metadata stdin writes agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 68 insertions(+), 14 deletions(-) diff --git a/src/run.rs b/src/run.rs index e118ab5c..0d6ccad5 100644 --- a/src/run.rs +++ b/src/run.rs @@ -75,7 +75,7 @@ fn terminate_and_reap_before(mut child: Child, pid: i32, deadline: Instant) { fn output_with_input_timeout( command: &mut Command, timeout: Duration, - input: Option<&[u8]>, + input: Option>, ) -> anyhow::Result { let mut stdout = tempfile::tempfile()?; let mut stderr = tempfile::tempfile()?; @@ -100,18 +100,31 @@ fn output_with_input_timeout( let pid = child.id() as i32; let deadline = Instant::now() + timeout; if let Some(input) = input { - let write = child - .stdin - .take() - .context("metadata patch child has no piped stdin") - .and_then(|mut stdin| { - stdin - .write_all(input) - .context("write metadata patch payload") - }); - if let Err(error) = write { + let Some(mut stdin) = child.stdin.take() else { terminate_and_reap_before(child, pid, deadline); - return Err(error); + anyhow::bail!("metadata patch child has no piped stdin"); + }; + let (write_tx, write_rx) = channel(); + std::thread::spawn(move || { + let result = stdin + .write_all(&input) + .context("write metadata patch payload"); + let _ = write_tx.send(result); + }); + match write_rx.recv_timeout(deadline.saturating_duration_since(Instant::now())) { + Ok(Ok(())) => {} + Ok(Err(error)) => { + terminate_and_reap_before(child, pid, deadline); + return Err(error); + } + Err(RecvTimeoutError::Timeout) => { + terminate_and_reap_before(child, pid, deadline); + anyhow::bail!("timed out after {:.1}s", timeout.as_secs_f64()); + } + Err(RecvTimeoutError::Disconnected) => { + terminate_and_reap_before(child, pid, deadline); + anyhow::bail!("metadata patch stdin writer stopped before reporting its result"); + } } } let status = loop { @@ -532,7 +545,7 @@ impl PtyCli { .args(["metadata", "patch", "--id", &presentation.pty_id]) .env("PTY_ROOT", effective_pty_root(&self.catalog_root)), PTY_LIST_TIMEOUT, - Some(&payload), + Some(payload), ) .map_err(|error| anyhow::anyhow!("`pty metadata patch --id` failed: {error}"))?; if !out.status.success() { @@ -2241,7 +2254,7 @@ mod tests { let error = output_with_input_timeout( Command::new(&executable).env("PIDFILE", &pidfile), Duration::from_secs(1), - Some(&input), + Some(input), ) .unwrap_err(); let pid = std::fs::read_to_string(pidfile) @@ -2259,6 +2272,47 @@ mod tests { ); } + #[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)); + } + #[test] fn expired_cleanup_deadline_hands_reaping_off_without_blocking() { let child = Command::new("sleep").arg("60").spawn().unwrap(); From 47d205fce97a632117b7962ff99bbdb859646a33 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:50:16 +0200 Subject: [PATCH 10/22] fix: avoid retained metadata writers agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 104 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/src/run.rs b/src/run.rs index 0d6ccad5..93a16c9c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -15,10 +15,11 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::ffi::OsString; use std::fs::File; 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::{Child, 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}; @@ -72,6 +73,43 @@ fn terminate_and_reap_before(mut child: Child, pid: i32, deadline: Instant) { } } +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() { + 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, @@ -100,30 +138,19 @@ fn output_with_input_timeout( let pid = child.id() as i32; let deadline = Instant::now() + timeout; if let Some(input) = input { - let Some(mut stdin) = child.stdin.take() else { + let Some(stdin) = child.stdin.take() else { terminate_and_reap_before(child, pid, deadline); anyhow::bail!("metadata patch child has no piped stdin"); }; - let (write_tx, write_rx) = channel(); - std::thread::spawn(move || { - let result = stdin - .write_all(&input) - .context("write metadata patch payload"); - let _ = write_tx.send(result); - }); - match write_rx.recv_timeout(deadline.saturating_duration_since(Instant::now())) { - Ok(Ok(())) => {} - Ok(Err(error)) => { - terminate_and_reap_before(child, pid, deadline); - return Err(error); - } - Err(RecvTimeoutError::Timeout) => { + 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(RecvTimeoutError::Disconnected) => { + Err(error) => { terminate_and_reap_before(child, pid, deadline); - anyhow::bail!("metadata patch stdin writer stopped before reporting its result"); + return Err(error); } } } @@ -2313,6 +2340,47 @@ mod tests { assert!(!crate::host_lock::process_alive(pid)); } + #[cfg(target_os = "linux")] + #[test] + fn escaped_stdin_reader_does_not_retain_a_writer_thread() { + use std::os::unix::fs::PermissionsExt as _; + + let temporary = tempfile::tempdir().unwrap(); + let executable = temporary.path().join("escape-with-stdin"); + let escaped_pidfile = temporary.path().join("escaped.pid"); + std::fs::write( + &executable, + "#!/bin/sh\nexec 3<&0\nsetsid sh -c 'printf %s \"$$\" > \"$ESCAPED_PIDFILE\"; sleep 60' <&3 &\nsleep 60\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + output_with_input_timeout( + Command::new(&executable).env("ESCAPED_PIDFILE", &escaped_pidfile), + Duration::from_millis(100), + Some(vec![b'x'; 1024 * 1024]), + ) + .unwrap_err(); + let escaped_pid = std::fs::read_to_string(&escaped_pidfile) + .unwrap() + .parse::() + .unwrap(); + let pipe = std::fs::read_link(format!("/proc/{escaped_pid}/fd/0")).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(); + unsafe { + libc::kill(-escaped_pid, libc::SIGKILL); + } + + assert_eq!( + retained_writers, 0, + "escaped stdin reader retained a metadata writer file descriptor" + ); + } + #[test] fn expired_cleanup_deadline_hands_reaping_off_without_blocking() { let child = Command::new("sleep").arg("60").spawn().unwrap(); From d390e40c739bf0a0c57ff79cca69cee2a9ddc8d4 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:58:30 +0200 Subject: [PATCH 11/22] test: persist escaped pipe identity agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/run.rs b/src/run.rs index 93a16c9c..4f4fde35 100644 --- a/src/run.rs +++ b/src/run.rs @@ -2348,14 +2348,17 @@ mod tests { let temporary = tempfile::tempdir().unwrap(); let executable = temporary.path().join("escape-with-stdin"); let escaped_pidfile = temporary.path().join("escaped.pid"); + let pipefile = temporary.path().join("escaped.pipe"); std::fs::write( &executable, - "#!/bin/sh\nexec 3<&0\nsetsid sh -c 'printf %s \"$$\" > \"$ESCAPED_PIDFILE\"; sleep 60' <&3 &\nsleep 60\n", + "#!/bin/sh\nexec 3<&0\nsetsid sh -c 'printf %s \"$$\" > \"$ESCAPED_PIDFILE\"; readlink /proc/self/fd/0 > \"$PIPEFILE\"; sleep 60' <&3 &\nsleep 60\n", ) .unwrap(); std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); output_with_input_timeout( - Command::new(&executable).env("ESCAPED_PIDFILE", &escaped_pidfile), + Command::new(&executable) + .env("ESCAPED_PIDFILE", &escaped_pidfile) + .env("PIPEFILE", &pipefile), Duration::from_millis(100), Some(vec![b'x'; 1024 * 1024]), ) @@ -2364,12 +2367,12 @@ mod tests { .unwrap() .parse::() .unwrap(); - let pipe = std::fs::read_link(format!("/proc/{escaped_pid}/fd/0")).unwrap(); + let pipe = std::fs::read_to_string(&pipefile).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) + .filter(|target| target.to_string_lossy() == pipe.trim()) .count(); unsafe { libc::kill(-escaped_pid, libc::SIGKILL); From 4ecc13f8e6e873592b0cdecdab722161556cf4cd Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:05:31 +0200 Subject: [PATCH 12/22] test: await escaped reader readiness agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/run.rs b/src/run.rs index 4f4fde35..f4be168f 100644 --- a/src/run.rs +++ b/src/run.rs @@ -2363,6 +2363,12 @@ mod tests { Some(vec![b'x'; 1024 * 1024]), ) .unwrap_err(); + let escaped_ready_deadline = Instant::now() + Duration::from_secs(2); + while (!escaped_pidfile.exists() || !pipefile.exists()) + && Instant::now() < escaped_ready_deadline + { + std::thread::sleep(Duration::from_millis(10)); + } let escaped_pid = std::fs::read_to_string(&escaped_pidfile) .unwrap() .parse::() From cf8adbe531547eb0d72594a7a161a2c970cd73b9 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:13:24 +0200 Subject: [PATCH 13/22] test: model an undrained metadata pipe agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 56 +++++++++++++++++++----------------------------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/src/run.rs b/src/run.rs index f4be168f..29b87e04 100644 --- a/src/run.rs +++ b/src/run.rs @@ -2342,51 +2342,35 @@ mod tests { #[cfg(target_os = "linux")] #[test] - fn escaped_stdin_reader_does_not_retain_a_writer_thread() { - use std::os::unix::fs::PermissionsExt as _; + 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(); - let temporary = tempfile::tempdir().unwrap(); - let executable = temporary.path().join("escape-with-stdin"); - let escaped_pidfile = temporary.path().join("escaped.pid"); - let pipefile = temporary.path().join("escaped.pipe"); - std::fs::write( - &executable, - "#!/bin/sh\nexec 3<&0\nsetsid sh -c 'printf %s \"$$\" > \"$ESCAPED_PIDFILE\"; readlink /proc/self/fd/0 > \"$PIPEFILE\"; sleep 60' <&3 &\nsleep 60\n", - ) - .unwrap(); - std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); - output_with_input_timeout( - Command::new(&executable) - .env("ESCAPED_PIDFILE", &escaped_pidfile) - .env("PIPEFILE", &pipefile), - Duration::from_millis(100), - Some(vec![b'x'; 1024 * 1024]), - ) - .unwrap_err(); - let escaped_ready_deadline = Instant::now() + Duration::from_secs(2); - while (!escaped_pidfile.exists() || !pipefile.exists()) - && Instant::now() < escaped_ready_deadline - { - std::thread::sleep(Duration::from_millis(10)); - } - let escaped_pid = std::fs::read_to_string(&escaped_pidfile) + assert!( + !write_all_before( + ChildStdin::from(writer), + &vec![b'x'; 1024 * 1024], + Instant::now() + Duration::from_millis(100), + ) .unwrap() - .parse::() - .unwrap(); - let pipe = std::fs::read_to_string(&pipefile).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.to_string_lossy() == pipe.trim()) + .filter(|target| target == &pipe) .count(); - unsafe { - libc::kill(-escaped_pid, libc::SIGKILL); - } + assert!(started.elapsed() < Duration::from_secs(1)); assert_eq!( - retained_writers, 0, - "escaped stdin reader retained a metadata writer file descriptor" + retained_writers, 1, + "the undrained pipe retained a writer after the deadline" ); } From ffffb1f6dd014c703c00c3af887da40e372cea91 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:26:11 +0200 Subject: [PATCH 14/22] fix: enforce metadata write deadlines agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/run.rs b/src/run.rs index 29b87e04..74afe84b 100644 --- a/src/run.rs +++ b/src/run.rs @@ -87,6 +87,9 @@ fn write_all_before( 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)) @@ -2374,6 +2377,22 @@ mod tests { ); } + #[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(); From aad301e9a716f5956c3dc79b3dcf234bb3a965b5 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:39:32 +0200 Subject: [PATCH 15/22] fix: preserve literal presentation tags agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/run.rs b/src/run.rs index 74afe84b..ad0ae598 100644 --- a/src/run.rs +++ b/src/run.rs @@ -332,9 +332,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); @@ -362,7 +363,11 @@ impl PtyCli { } } cmd.arg("--cwd").arg(&cwd); - let mut tags = target.tags.clone(); + 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 { @@ -376,7 +381,7 @@ impl PtyCli { } } for (k, v) in &tags { - cmd.arg("--tag").arg(format!("{k}={}", self.expand(v))); + 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 @@ -2179,6 +2184,9 @@ 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(); @@ -2195,7 +2203,10 @@ mod tests { "agent.actor.path".to_owned(), Some("hetz.demo".to_owned()), ), - ("agent.presentation.description".to_owned(), None), + ( + "agent.presentation.description".to_owned(), + Some(format!("${key}")), + ), ]), }); let cmd = cli.build_run_command(&t, Path::new("/cat/hetz/demo")); @@ -2214,11 +2225,7 @@ mod tests { assert!(tags.contains("unrelated=preserved")); assert!(tags.contains("agent.presentation.schema=1")); assert!(tags.contains("agent.actor.path=hetz.demo")); - assert!( - !tags - .iter() - .any(|tag| tag.starts_with("agent.presentation.description=")) - ); + assert!(tags.contains("agent.presentation.description=$ST2_TEST_PRESENTATION_LITERAL_71c")); } #[test] From 9c4c18e619fa0474865a49484b3110d633ec5bf4 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:51:36 +0200 Subject: [PATCH 16/22] fix: preserve CRLF presentation sources agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/agent_author.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/agent_author.rs b/src/agent_author.rs index d4128302..3d2691fe 100644 --- a/src/agent_author.rs +++ b/src/agent_author.rs @@ -714,7 +714,7 @@ fn remove_field(text: &str, node: &KdlNode) -> Result { .all(|value| matches!(value, ' ' | '\t')) && text[end..line_end] .chars() - .all(|value| matches!(value, ' ' | '\t')) + .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()); @@ -1001,6 +1001,31 @@ mod tests { ); } + #[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(); From 391a684c261a81dfeec93cf741696fdf7500b9a0 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:06:06 +0200 Subject: [PATCH 17/22] fix: suppress lifecycle-equal display names agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/run.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/run.rs b/src/run.rs index ad0ae598..49790d43 100644 --- a/src/run.rs +++ b/src/run.rs @@ -348,6 +348,9 @@ impl PtyCli { .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]); } @@ -2540,6 +2543,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() From 222cab53134cfd326aa607d087ed47ebf1876670 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:24:15 +0200 Subject: [PATCH 18/22] fix(eval): preserve external requester routing agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/eval_run.rs | 10 ++++++++++ src/message.rs | 22 ++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index c4f23874..7e2cfbd7 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -1076,6 +1076,16 @@ 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" + ); + } + 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 { diff --git a/src/message.rs b/src/message.rs index e67c4a10..25eb2c94 100644 --- a/src/message.rs +++ b/src/message.rs @@ -314,10 +314,20 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf { agent_dir.join("resources").join("archive") } -/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus; -/// once declarations or discovery errors make it a catalog, an absent identity fails closed. +/// 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 { - resolve_list_box(root, id, host, false, false) + 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), + } + } + } } /// Archive companion to [`resolve_inbox`], with the same stable-ID and catalog-less boundaries. @@ -656,7 +666,7 @@ 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!( @@ -664,5 +674,9 @@ mod tests { 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); } } From 1cac690a1375559c3ca49f89b7632059c2300377 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:26:54 +0200 Subject: [PATCH 19/22] fix: keep lifecycle-equal names suppressed agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/reconcile.rs | 5 ++++- tests/nomad_survival.rs | 30 +++++++++++++++++++++++++++++- tests/reconcile.rs | 20 ++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/reconcile.rs b/src/reconcile.rs index 2c7b1206..c847db30 100644 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -78,7 +78,10 @@ fn pty_presentation( } Some(PtyPresentation { pty_id: pty_id.to_owned(), - display_name: (task.name == "agent").then(|| spec.name.clone()), + 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(), diff --git a/tests/nomad_survival.rs b/tests/nomad_survival.rs index 8be003ff..5958d0c1 100644 --- a/tests/nomad_survival.rs +++ b/tests/nomad_survival.rs @@ -757,6 +757,34 @@ fn presentation_changes_patch_the_exact_live_pty_without_restarting_it() { "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() @@ -787,7 +815,7 @@ fn presentation_changes_patch_the_exact_live_pty_without_restarting_it() { .get("agent.presentation.description") .is_none() ); - assert_eq!(event_count(), initial_events + 2); + 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(); diff --git a/tests/reconcile.rs b/tests/reconcile.rs index f2a307bd..55603052 100644 --- a/tests/reconcile.rs +++ b/tests/reconcile.rs @@ -511,6 +511,26 @@ fn live_pty_presentation_is_exact_id_metadata_and_not_lifecycle_drift() { 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 resource_only_changes_do_not_replace_or_relaunch_a_live_task() { let mut spec = svc( From 3d00dcd30f2c0c2d921b41af2151f1eace9bf060 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:36:01 +0200 Subject: [PATCH 20/22] fix: bound presentation reconciliation work agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/eval_run.rs | 9 +++--- src/exec_backend.rs | 1 + src/reconcile.rs | 45 +++++++++++++++++++++++++-- src/run.rs | 58 +++++++++++++++++++++++++++++------ tests/reconcile.rs | 57 ++++++++++++++++++++++++++++++++++ tests/run.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 227 insertions(+), 17 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index 7e2cfbd7..d6ec4b68 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -1574,6 +1574,7 @@ mod tests { pty_id: id.into(), alive: true, exit_code: None, + presentation: None, }) .collect(), killed: RefCell::new(Vec::new()), @@ -1729,14 +1730,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(()) } @@ -1754,7 +1755,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 { @@ -1777,7 +1778,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/exec_backend.rs b/src/exec_backend.rs index 403447c8..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) diff --git a/src/reconcile.rs b/src/reconcile.rs index c847db30..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 @@ -93,6 +102,21 @@ fn pty_presentation( }) } +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. #[derive(Debug, Clone, PartialEq, Eq)] pub enum TaskLaunch { @@ -222,7 +246,12 @@ pub fn reconcile_selected<'a>( }; match actual { Some(s) if s.alive => { - if let Some(presentation) = target.presentation.clone() { + 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); @@ -277,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 { @@ -359,7 +392,15 @@ pub fn reconcile<'a>( for (target, lifecycle) in targets { match session_state(&by_id, &target.pty_id) { SessionState::Alive => { - if let Some(presentation) = target.presentation.clone() { + 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); } } diff --git a/src/run.rs b/src/run.rs index 49790d43..644e64e0 100644 --- a/src/run.rs +++ b/src/run.rs @@ -40,6 +40,7 @@ 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. @@ -250,6 +251,10 @@ 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)] @@ -642,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()) } @@ -1013,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() } } @@ -1026,15 +1036,6 @@ pub fn execute( cap: &mut FlappingCap, report: &mut UpReport, ) { - // Presentation is an independent metadata path. A failure is visible and retried by the next - // reconcile pass, but never authorizes stop, reap, restart, or replacement. - for presentation in &plan.presentation { - if let Err(error) = runner.patch_presentation(presentation) { - report - .errors - .push(format!("metadata patch {}: {error}", presentation.pty_id)); - } - } // The corpses tied to a launch target (dead, non-keep, active ptys) are reaped inside the launch // loop so a parked flapper keeps its evidence. Everything else in `gc` (e.g. a retired agent's // dead sessions) is reaped here. @@ -1116,6 +1117,30 @@ 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; + // remaining drift is observed and retried on later passes. + for presentation in plan + .presentation + .iter() + .take(MAX_PRESENTATION_PATCHES_PER_PASS) + { + 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())); @@ -1908,6 +1933,7 @@ mod tests { pty_id: id.to_string(), alive, exit_code: None, + presentation: None, } } @@ -2938,7 +2964,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(); @@ -2963,6 +2989,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/reconcile.rs b/tests/reconcile.rs index 55603052..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", @@ -396,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 { @@ -403,6 +411,7 @@ fn dead(id: &str) -> Session { pty_id: id.to_string(), alive: false, exit_code: None, + presentation: None, } } @@ -531,6 +540,54 @@ fn lifecycle_equal_primary_name_is_cleared_during_live_reconciliation() { 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 297184a2..d65c4a5c 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}; @@ -453,6 +455,7 @@ struct FakeRunner { killed: RefCell>, reaped: RefCell>, removed: RefCell>, + patched: RefCell>, ops: RefCell>, } @@ -478,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()); @@ -506,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 { @@ -513,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(); + + execute( + &plan, + &runner, + &mut FlappingCap::default(), + &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()); +} + /// A v2 service job: a pty agent + an exec ding. const AGENT: &str = r#" identity = "demo" From 3187b79e08e916ce9ce1156a3a4c6091d5b405fb Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:52:23 +0200 Subject: [PATCH 21/22] fix: close presentation routing fairness gaps agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/eval_run.rs | 9 +++++++++ src/flapping.rs | 10 ++++++++++ src/run.rs | 14 +++++++++----- tests/eval_run_e2e.rs | 8 ++++++++ tests/run.rs | 12 ++++++------ 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index d6ec4b68..484ce3b7 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -1082,6 +1082,15 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos "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"))?; } 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/run.rs b/src/run.rs index 644e64e0..860d4cab 100644 --- a/src/run.rs +++ b/src/run.rs @@ -1119,12 +1119,16 @@ 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; - // remaining drift is observed and retried on later passes. - for presentation in plan + // the persistent cursor rotates remaining drift through later passes without starvation. + let presentation_count = plan .presentation - .iter() - .take(MAX_PRESENTATION_PATCHES_PER_PASS) - { + .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 diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 04bc0681..09e9aa8b 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -802,6 +802,14 @@ 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" }"#, + )], + ), ]; for (expected, target, declarations) in cases { let tmp = tempfile::tempdir().unwrap(); diff --git a/tests/run.rs b/tests/run.rs index d65c4a5c..650f03c6 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -566,13 +566,9 @@ fn lifecycle_work_precedes_a_bounded_presentation_batch() { }; let runner = FakeRunner::default(); let mut report = UpReport::default(); + let mut cap = FlappingCap::default(); - execute( - &plan, - &runner, - &mut FlappingCap::default(), - &mut report, - ); + execute(&plan, &runner, &mut cap, &mut report); assert_eq!( &runner.ops.borrow()[..2], @@ -586,6 +582,10 @@ fn lifecycle_work_precedes_a_bounded_presentation_batch() { .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. From 70729a34823487e31adfeaee78581e729aa5389c Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:08:39 +0200 Subject: [PATCH 22/22] fix: reject ambiguous canonical routes agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty --- src/eval_run.rs | 14 ++++++++++++-- tests/eval_run_e2e.rs | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index 484ce3b7..c29e7190 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -258,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)); diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 09e9aa8b..5dc150ed 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -810,6 +810,20 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { 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();