diff --git a/README.md b/README.md index 61801ae2..5f773f8a 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,8 @@ The envelope is `name` + `uri` + a required human-facing `reason`, plus an optio access, readiness, or lifecycle policy, and URI possession conveys no authority. A Resource URI may be referenced by any number of agent declarations. Resource-only declaration edits do not stop, replace, or relaunch a live task. Resource profiles and resolvers remain opaque to st2; catalog -readers use the public `agent-spec` crate to inspect the bindings. +readers use the public `agent-spec` crate to inspect the bindings, and `st2 resource ls|read` +projects them for one agent. 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. @@ -270,6 +271,9 @@ Mutate a catalog-owned KDL declaration through the constrained commands: st2 rename "Release worker" st2 describe "Owns release preparation and verification." st2 rename --clear +st2 resource add --uri --reason "" +st2 resource remove +st2 resource rename ``` These commands preserve unrelated KDL bytes and serialize local writers through the persistent @@ -468,8 +472,8 @@ so external harness hooks can read its current name and description without pars on a duplicate state file. For a catalog-backed agent, every native bus operation resolves the same agent directory used by -the roster: presence is `/status`, while unread messages, archive receipts, context, and -links live under `/resources/`. The flat `/` layout remains only as the +the roster: presence is `/status`, while unread messages, archive receipts, and context +live under `/resources/`. The flat `/` layout remains only as the intentional catalog-less fallback used by isolated folder evals. In a catalog-backed root, `st2 message ls` rejects an absent identity; recovery inspection of a deliberately orphaned flat box must be explicit with `st2 message ls --orphan` (and optionally `--archive`). diff --git a/docs/vrs/.decisions/0011-the-linked-record-plane-is-retired.md b/docs/vrs/.decisions/0011-the-linked-record-plane-is-retired.md new file mode 100644 index 00000000..9cdb397f --- /dev/null +++ b/docs/vrs/.decisions/0011-the-linked-record-plane-is-retired.md @@ -0,0 +1,98 @@ +# The linked-record plane is retired and `resource` names one concept + +Status: accepted + +Design decision made by Johannes on 2026-08-27 (interview over the +07-resource measurements and +[dotfiles#2071](https://github.com/schickling/dotfiles/pull/2071)). Merge and +acceptance approval required: upstream maintainers. + +## Context + +Two durable edges were both called *resource*: Agent Spec Resource bindings, and +the link records written by `st2 resource add`. An agent reading one while +reasoning about the other measured the wrong store — a declaration with five +bindings and no link records reported `# 0 resources`, and a declaration with +two bindings and fourteen link records reported fourteen rows containing neither +binding. Both surfaces answered correctly; neither said which question it had +answered. + +The measurements (07-resource `.experiments/2026-08-27-resource-read-surfaces.md`, +one live catalog of 655 declarations) established that the planes are disjoint by +construction rather than by coincidence: 889 distinct binding URIs and 233 +distinct link URLs share **zero** members, exactly or normalized. Bindings carry +what an agent is for; link records carried what it produced. + +They also established that the link plane is legacy. It was adopted by 82 of 655 +declarations, its creation rate decayed from a peak of 45/day on 2026-08-08 to +none after 2026-08-26, **the only reader of a link record in the source tree is +its own `ls`/`read` verb**, and `axe work update --artifact --pty ` +now covers the job — matching the observed `relation` values (`output` 185, +`evidence`, `produces`) and the 8 `pty://` URLs. + +## Decision + +The linked-record plane is **retired**. `st2 resource add|ls|read|remove` and +`resources/links/` are removed, and *resource* names exactly one concept: a +declared Resource binding. The freed verb becomes the binding surface, which the +declared plane never had — bindings were previously visible only through +`st2 agents --json`. + +The 241 existing records are left in place as orphaned files rather than +migrated. Most belong to retired declarations whose worktrees are gone, and +`axe work` writes to a gitignored per-worktree path that no longer exists for +them. + +Producing agents record artifacts through `axe work update --artifact/--pty`. + +## Consequences + +- `resource` is unambiguous across the CLI, the declaration, and the corpus. + The read that produced the friction cannot recur, because there is no second + plane to read. +- `/resources/` is untouched and remains canonical for an agent's + resource files. Only `links/` goes. The directory is the realization surface + for bindings, not a second sense of the word. +- `templates/bus.st2.md` loses the advertisement that produced the adoption. +- Surviving `resources/links/` files become unreferenced. The ontology keeps a + retired entry so a reader who meets one can identify it. +- Nothing downstream breaks: no consumer other than the retired verb read them. + +## Options + +| Option | Result | Reason | +| --- | --- | --- | +| Retire the plane; `st2 resource` becomes the binding surface | Selected | Nothing but its own verb ever read a linked record, adoption was 12.5% and decaying, and `axe work update --artifact/--pty` covers the job. Retiring makes *resource* unambiguous by removing the second plane rather than by wording around it. | +| Freeze read-only: keep `ls`/`read`, drop `add` | Rejected | Keeps records reachable, but *resource* keeps naming two things for as long as any record survives, so the misread stays possible. | +| Migrate the 241 records into `axe work`, then retire | Rejected | Highest fidelity, but `axe work` writes to `/tmp/worklog/`, gitignored and per-worktree; most of the 82 declarations are retired and have no reachable worktree to write into. | +| Keep and invest: add `--json`, a requirement, a consumer | Rejected | Asks the fleet to adopt a second evidence ledger beside the one it already uses. In the plane's whole lifetime nothing consumed it. | +| Unify both planes behind one typed reference ([#122](https://github.com/compoundingtech/st2/issues/122)) | Rejected | With zero measured overlap a shared descriptor deduplicates nothing and joins nothing, and #122's proposed `{_tag, uri}` assumes a field removed in #307. | + +## Evidence and Argument + +The measurement is +[`07-resource/.experiments/2026-08-27-resource-read-surfaces.md`](../07-resource/.experiments/2026-08-27-resource-read-surfaces.md), +taken against one live catalog of 655 declarations. Three findings decide it. + +**The planes are disjoint by construction.** 889 distinct binding URIs and 233 +distinct linked-record URLs share zero members, under exact match and after +normalization on all five schemes both planes used. 81 of the 82 +linked-record-carrying declarations also carried bindings, so they coexisted +constantly and still never named one thing. The cause is semantic: 728 bindings +were self-state carriers and 378 were work inputs, while 196 of 241 linked +records were `output`, `produces`, `evidence`, or `verified`. That is what kills +#122 — a shared type has nothing to deduplicate. + +**The plane was write-only.** A source search for `links_dir` finds exactly two +readers, both inside the `st2 resource ls|read` implementation. No projection, +roster, or doctor consumed a linked record in the plane's lifetime. + +**It was already being abandoned.** Creation peaked at 45 records on 2026-08-08, +fell to 5 on 2026-08-26, and stopped; 14 of the 25 most recent records come from +one declaration on one day. Meanwhile `axe work update` grew `--artifact` and +`--pty`, which match the observed `relation` values and the 8 `pty://` URLs +exactly. + +The counter-argument — that low adoption is a surfacing problem, not obsolescence +— is answered by the second finding: a plane nothing reads cannot be surfaced +into usefulness by adding `--json` to it. diff --git a/docs/vrs/.decisions/0012-working-state-is-a-declared-carrier.md b/docs/vrs/.decisions/0012-working-state-is-a-declared-carrier.md new file mode 100644 index 00000000..4ad0032f --- /dev/null +++ b/docs/vrs/.decisions/0012-working-state-is-a-declared-carrier.md @@ -0,0 +1,106 @@ +# Working state is a declared carrier under `working-state://` + +Status: accepted + +Design decision made by Johannes on 2026-08-27 (interview; supersedes the open +part of [#261](https://github.com/compoundingtech/st2/issues/261), which asked +st2 to pick the name). Merge and acceptance approval required: upstream +maintainers. + +## Context + +An agent's self-state carriers are declared as Resource bindings and realized +under `/resources/`: `dev.schickling.agent-goal://` realizes as +`resources/goal.md`, `decision-tree://` as `resources/context/decisions/`, and +so on for notes, private notes, and the friction log. + +Working state — R09's restored durable context, written through `st2 context` — +is the exception. Measured on one live catalog of 655 declarations, 605 have a +`resources/context/now.md` and **none declares it**; no binding URI anywhere +mentions it. Consumers reach it by joining a literal path onto the declaration's +own directory, which one downstream author annotated in-line as "a CONVENTION, +not a declaration". + +## Decision + +Working state becomes the sixth self-state carrier, declared like its siblings: + +```kdl +resource "working-state" \ + uri="working-state:///" \ + reason="Working state for lossless restart." +``` + +realized at `/resources/context/now.md`, resolver owned by st2 +(`st2 context`). The binding grants no authority, as for every binding. + +The scheme is **`working-state`**, un-prefixed. It takes the ontology's already +canonical term for R09's restored durable context — a term the ontology also +already guards against being read as a liveness or activity signal — and the +scheme inherits that guard. + +## Consequences + +- The carrier is addressable by URI like its five siblings, so a consumer + resolves a declaration instead of walking a path. +- The ontology gains a `working state` entry naming the term, the verb, the + realization path, and the scheme. +- Declaring it across the fleet is a bulk binding write, which is why it + sequences after the mediated write surface (decision 0013). +- This needed no requirements change. It was first drafted as a new requirement + (**R35 st2-owned Resource profiles**) because R20 then read "st2 does not + register schemes", which a scheme st2 resolves would have contradicted. + [#351](https://github.com/compoundingtech/st2/pull/351) landed first and + rewrote R20: a scheme is now the exact lookup key for an optional, + catalog-declared Resource Profile. That removes the barrier R35 existed to lift, + and R35's own text — an exemption from a clause R20 no longer contains — became + incoherent, so it was dropped rather than rebased. + + `working-state` is therefore an ordinary scheme under the merged R20. st2's + `st2 context` writes the carrier at `resources/context/now.md`; whether any + catalog registers a Resource Profile that resolves the scheme is a downstream + choice this decision does not make, and R20's "st2 ships no built-in profiles" + stands untouched. + +## Amendment 1 — 2026-08-28 + +The requirements delta this decision originally carried is withdrawn. Nothing +about the carrier, its scheme name, or its realization changes; only the +justification does, because #351 made the exemption unnecessary. See the bullet +above. + +## Options + +| Option | Result | Reason | +| --- | --- | --- | +| `working-state://` | Selected | Takes the ontology's already canonical term for R09's restored durable context — a term the ontology also already guards against being read as liveness or activity — so the scheme inherits the guard. No known downstream clash. `decision-tree://` sets the un-prefixed precedent. | +| `agent-context://` | Rejected | Matches the `st2 context` verb, the `resources/context/` directory, and the `agent-notes://` prefix pattern, but [#261](https://github.com/compoundingtech/st2/issues/261) states the requesting consumer already uses "Agent Context" for message-envelope relations. Hands them a collision for internal symmetry. | +| `agent-state://` | Rejected before posing | *State* is the most overloaded word in this ontology — session state, observed harness state, desired state, presence — which already carries explicit collision rules. A scheme by that name recreates the ambiguity this work removes. | +| `agent-working-state://` | Rejected | No clash and full prefix symmetry, but `agent-` carries no information: every per-agent carrier is per-agent and the URI authority already names the agent. | +| Leave working state undeclared | Rejected | `st2 context read ` already resolves it, but the five sibling carriers are declared and the asymmetry is what forces downstream path-joining and filesystem crawls. | + +## Evidence and Argument + +Measured on one live catalog of 655 declarations +([experiment](../07-resource/.experiments/2026-08-27-resource-read-surfaces.md)): +605 declarations have a `resources/context/now.md`, **none** declares it, and no +binding URI anywhere mentions it. Every other near-universal per-agent carrier is +declared — `notes` on 618 declarations, and `goal`, `private-notes`, +`friction-log`, `decisions` on 27 each — and each realizes into the same +`resources/` directory that working state realizes into. + +So the asymmetry is not a design boundary, it is an omission: the one carrier st2 +itself writes, through `st2 context`, is the one carrier nothing declares. + +#261 documents what the omission costs a consumer — a TUI joining a literal path +onto the declaration's directory, annotated in-line by its own author as "a +CONVENTION, not a declaration", plus an hourly timer crawling +`find /resources` behind a hand-maintained exclusion list to report which +live declarations lack a `now.md`. Both are downstream workarounds for a fact +that no declaration states. + +The naming argument turns on which layer owns the word. The verb and the +directory both say *context*; the ontology says *working state*, and says it in a +rule that already exists to stop the term being confused with liveness. A scheme +is read far from its verb, so it should carry the term that travels with its own +guard. diff --git a/docs/vrs/.decisions/0013-resource-is-a-mediated-write-surface.md b/docs/vrs/.decisions/0013-resource-is-a-mediated-write-surface.md new file mode 100644 index 00000000..111095cb --- /dev/null +++ b/docs/vrs/.decisions/0013-resource-is-a-mediated-write-surface.md @@ -0,0 +1,101 @@ +# `st2 resource` is a mediated binding write surface + +Status: accepted + +Design decision made by Johannes on 2026-08-27 (interview; answers +[#231](https://github.com/compoundingtech/st2/issues/231)). Merge and acceptance +approval required: upstream maintainers. + +## Context + +Changing one declared binding requires the caller to render a complete candidate +`agent.kdl`, validate it, compute its digest, publish under compare-and-swap, and +read it back. That ceremony is safe and disproportionate. + +It also has a measurable consequence. Of 241 link records on one live catalog, 45 +were not products at all but dependency and reference edges — `current-work` +(which duplicates the `work` binding), `depends-on-slice-*`, `supervises`, +`blocked-design`. Agents wrote them into the linked-record plane because that was +the plane they were permitted to write cheaply. **The drift between the two +planes was caused by write-cost asymmetry, not by two different relations.** + +Retiring the linked-record plane (decision 0011) removes the escape hatch. It +does not remove the pressure that produced it. + +## Decision + +`st2 resource` gains mediated write verbs alongside `ls` and `read`: + +```text +st2 resource add --uri --reason +st2 resource remove +st2 resource rename +``` + +Each performs read-modify-CAS-publish internally. The caller never renders KDL. +Full-catalog validation, exact-target selection, compare-and-swap, and +fail-closed concurrent-change behavior are preserved, and a binding-only change +does not stop, replace, or relaunch healthy work (R21). + +This is the fourth instance of an existing pattern, not new machinery: +`src/agent_author.rs` already mediates `add_stream`/`remove_stream` +(`st2 agent stream`), `set_desired_state` (`st2 agent desired-state`), and +`set_presentation` (`st2 rename` / `st2 describe`). + +## Consequences + +- The reason agents reached for the cheap plane is removed, not just the plane. +- Declaring the working-state carrier (decision 0012) across a fleet becomes one + command per declaration instead of a rendered-and-published candidate each. +- URI possession still grants nothing. A mediated write changes a declaration; + it does not touch the thing the URI names, and confers no access to it + ([#61](https://github.com/compoundingtech/st2/issues/61)). +- Authoring authority is unchanged: whoever may publish the declaration may + mutate its bindings, and no one else. + +## Limits + +This does not address a declaration generated read-only by configuration +management, where the next activation overwrites a runtime edit +([#305](https://github.com/compoundingtech/st2/issues/305)). On the catalog +measured, declarations are writable regular files carrying +`meta { managed-by "agent-spec-authoring" }` with no `/nix/store` symlinks, so +the mediated write applies there. The generated-declaration case stays open in +[DQ-R4](../07-resource/open-questions.md). + +## Options + +| Option | Result | Reason | +| --- | --- | --- | +| Read plus mediated `add`/`remove`/`rename` | Selected | Removes the cause of the drift, not just its symptom. The machinery is the fourth instance of an existing pattern in `src/agent_author.rs`, so the marginal risk is small. | +| Read-only `ls`/`read` | Rejected | Smallest change and a literal reading of #61's read-oriented boundary, but it leaves the write-cost asymmetry intact while decision 0011 removes the escape hatch — pressure with nowhere to go. | +| Ship read-only now, add writes as a follow-up | Rejected | Sequences a breaking rename away from a new write path, which is genuinely safer, but leaves the same interval in which agents have an expensive plane and no cheap one. | + +## Evidence and Argument + +The link-record census supplies the causal evidence. Of 241 records, 196 are +products — the plane's stated purpose. The remaining 45 are not: `supervises` +(11), `reference` (7), `current-work`, `depends-on-slice-1`, +`depends-on-slice-2`, `depends-on-slice-5`, `depends-on-slices-1-4`, +`blocked-design`. `current-work` duplicates the `work` binding outright, and the +`depends-on-slice-*` records are dependency edges filed in a products store. + +Agents did not confuse the two planes. They wrote dependency edges into the +products plane because a binding required publisher authority and +whole-declaration republication under compare-and-swap, while a linked record +required one file write. The observed misfiling is what write-cost asymmetry +looks like from the inside. + +That the machinery already exists is the second half of the argument. +[`src/agent_author.rs`](../../../src/agent_author.rs) implements exactly this +read-modify-CAS-publish shape three times — `add_stream`/`remove_stream` behind +`st2 agent stream`, `set_desired_state` behind `st2 agent desired-state`, and +`set_presentation` behind `st2 rename` and `st2 describe`. Bindings are the +fourth field of the same declaration, mutated by the same protocol; this is a new +caller of proven machinery rather than a new mechanism. + +[#231](https://github.com/compoundingtech/st2/issues/231) asked for precisely +this and listed the properties it must keep — full-catalog validation, exact +target selection, CAS, fail-closed on concurrent change, non-disruption of +healthy work, and machine-readable publication evidence. All are properties the +existing three callers already have. diff --git a/docs/vrs/07-resource/.experiments/2026-08-27-resource-read-surfaces.md b/docs/vrs/07-resource/.experiments/2026-08-27-resource-read-surfaces.md new file mode 100644 index 00000000..cf888e4d --- /dev/null +++ b/docs/vrs/07-resource/.experiments/2026-08-27-resource-read-surfaces.md @@ -0,0 +1,103 @@ +# Resource read surfaces and link-plane adoption + +**Date:** 2026-08-27 +**Catalog:** one live catalog, host `dev3`, 655 agent declarations. + +## Question + +Do the two edges called *resource* — declared bindings and `st2 resource` link +records — name the same things, and is the link plane still carrying work? + +## Method + +Aggregate shell census over every `agent.kdl` and every +`resources/links/*.md` in the catalog: URI and URL sets compared exactly and +after normalization (trailing slash stripped, `%20` decoded) on the schemes both +planes use; link-record creation time read from the `` filename prefix; +readers of the link plane found by searching the source tree for `links_dir`. + +A shell renderer +([`render-read-surfaces.sh`](./render-read-surfaces.sh)) prints candidate +`st2 resource ls` shapes for one declaration, reading the same bytes st2 reads. +It was checked against the two declarations in +[dotfiles#2071](https://github.com/schickling/dotfiles/pull/2071) — +`dotfiles.fb-batch1.docs.worker` (5 bindings, 0 link records) and +`dotfiles.vista.pr1506-final.worker` (2 bindings, 14 link records). + +## Result + +The renderer reproduced both reported outputs exactly, including +`# 0 resources for dev3.dotfiles.fb-batch1.docs.worker`, so it is faithful to +the surface under test. + +Disjointness: + +- 1161 bindings across 647 declarations; 241 link records across 82. +- 889 distinct binding URIs, 233 distinct link URLs, **0 coinciding** under + exact match and under normalization on all five shared schemes (`file`, + `worktree`, `git-commit`, `https`, `st2-message`). +- 81 of 82 link-record-carrying declarations also carry bindings. +- 13 path-prefix hits across 6 declared worktrees: link outputs living inside a + declared worktree. Containment, not identity. + +Link-plane adoption: + +- 82 of 655 declarations (12.5%) ever wrote a link record. +- Creation peaked 2026-08-07 through 08-09 (44, 45, 29 records/day), then + decayed; 5 records on 2026-08-26 and none since. +- 14 of the 25 most recent records come from a single declaration on one day. +- **The only reader of link records in the source tree is the + `st2 resource ls|read` verb itself.** No projection, roster, doctor, or other + consumer reads them. +- `axe work update --artifact --pty ` covers the same job, and + matches the observed `relation` values (`output` 185, `evidence`, `produces`) + and the 8 `pty://` URLs. + +Binding realization: + +- Self-state carriers realize into the agent's `resources/` directory: + `dev.schickling.agent-goal://` → `resources/goal.md`, + `decision-tree://` → `resources/context/decisions/`. +- `resources/context/now.md` exists on 605 of 655 declarations and is declared + by none; no binding URI anywhere mentions it. + +Reason-field quality: + +- 987 of 1161 bindings carry the placeholder + `reason="Legacy binding retained without recorded rationale."` from a + 2026-08-22 backfill. Declarations touched on 2026-08-26 and 2026-08-27 are + 26 of 27 placeholder-free and carry real prose. + +## Conclusion + +The two planes are disjoint by construction, not by coincidence: bindings carry +what an agent is for, link records carry what it produced. A shared descriptor +type would deduplicate nothing. + +The link plane is legacy. It is write-only by construction, adopted by an eighth +of the fleet, decaying, and superseded by `axe work`. The `resources/` directory +is not a third meaning of the word — it is the realization surface for the +bindings, and remains canonical for an agent's resource files. + +`reason` is working; the 987 placeholders are one un-cleaned backfill. + +## VRS Impact + +- [spec.md](../spec.md): the two edges, their disjointness, the write-cost + asymmetry that caused the drift, and the retirement of the link plane. +- [ontology.md](../../ontology.md): `Resource`, `Resource binding`, + `linked record` (retired), `agent resource directory`, `working state`. +- [open-questions.md](../open-questions.md): DQ-R4 (write-cost, unresolved), + DQ-R7 (placeholder cleanup), DQ-R8 (upstream sync). +- No requirements change. A delta was drafted for the working-state carrier and + withdrawn once [#351](https://github.com/compoundingtech/st2/pull/351) rewrote + R20; see [decision 0012](../../.decisions/0012-working-state-is-a-declared-carrier.md) + Amendment 1. + +## Limits + +One catalog, one host, one downstream convention. Zero overlap is strong +evidence that the planes are disjoint *as used here*; it does not prove no +consumer would ever want to name one Resource from both. The renderer covers +read shape only and says nothing about write cost, which +[DQ-R4](../open-questions.md) holds open. diff --git a/docs/vrs/07-resource/.experiments/render-read-surfaces.sh b/docs/vrs/07-resource/.experiments/render-read-surfaces.sh new file mode 100755 index 00000000..4389e1d1 --- /dev/null +++ b/docs/vrs/07-resource/.experiments/render-read-surfaces.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Render candidate `st2 resource ls` output shapes for one agent declaration. +# +# Reads the same bytes st2 reads; changes nothing. Used to check whether a +# candidate surface stops the reported instances from misleading. +# +# render-read-surfaces.sh +# +# Catalog defaults to $CATALOG, else the --catalog path the runner was started +# with. Pass it explicitly with CATALOG=... if neither applies. +set -euo pipefail + +catalog="${CATALOG:?set CATALOG to the catalog root}" +host="${1:?host}"; identity="${2:?identity}"; variant="${3:-both}" +dir="$catalog/agents/$host/$identity" + +bindings() { rg --no-filename -o '^\s*resource "[^"]*"[^\n]*' "$dir/agent.kdl" 2>/dev/null || true; } +links() { fd -t f -e md . "$dir/resources/links" 2>/dev/null | sort || true; } +field() { rg --no-filename -o "^$1: .*" "$2" 2>/dev/null | sed "s/^$1: //" || true; } + +nb=$(bindings | grep -c . || true); nl=$(links | grep -c . || true) +s() { [ "$1" = 1 ] || printf s; } + +case "$variant" in +today) + printf '# %s resource%s for %s.%s\n' "$nl" "$(s "$nl")" "$host" "$identity" + links | while read -r f; do + printf '%s %s %s\n' "$(basename "$f")" "$(field url "$f")" "$(field title "$f")" + done + ;; + +pointer) + printf '# %s recorded link%s for %s.%s\n' "$nl" "$(s "$nl")" "$host" "$identity" + links | while read -r f; do + printf '%s %s %s\n' "$(basename "$f")" "$(field url "$f")" "$(field title "$f")" + done + [ "$nb" -gt 0 ] && printf '# (%s declared Resource binding%s in agent.kdl — see `st2 agent resource ls`)\n' "$nb" "$(s "$nb")" + ;; + +both) + printf '# %s.%s\n\n' "$host" "$identity" + printf '## declared (%s) — publisher-owned, from agent.kdl\n' "$nb" + if [ "$nb" -eq 0 ]; then printf ' (none)\n'; else + bindings | sed 's/^[[:space:]]*resource //' | while read -r line; do + name=${line%%\"*}; name=$(printf '%s' "$line" | sed 's/^"\([^"]*\)".*/\1/') + uri=$(printf '%s' "$line" | rg -o 'uri="[^"]*"' | sed 's/uri="//; s/"$//') + printf ' %-18s %s\n' "$name" "$uri" + done + fi + printf '\n## recorded (%s) — agent-owned, from resources/links/\n' "$nl" + if [ "$nl" -eq 0 ]; then printf ' (none)\n'; else + links | while read -r f; do + rel=$(field relation "$f"); title=$(field title "$f") + printf ' %-10s %-24s %s\n' "${rel:--}" "$(basename "$f")" "${title:-$(field url "$f")}" + done + fi + ;; + +*) echo "unknown variant: $variant" >&2; exit 2 ;; +esac diff --git a/docs/vrs/07-resource/open-questions.md b/docs/vrs/07-resource/open-questions.md new file mode 100644 index 00000000..57148b93 --- /dev/null +++ b/docs/vrs/07-resource/open-questions.md @@ -0,0 +1,48 @@ +# Resource open questions + +Each entry links a spec `DQ-R*`. Questions leave this file when resolved — into +[spec.md](./spec.md) as decisions or `.experiments/` as tested hypotheses. + +- **DQ-R4 Runtime bindings on a generated declaration.** + [Decision 0013](../.decisions/0013-resource-is-a-mediated-write-surface.md) + gives `st2 resource add|remove|rename` mediated read-modify-CAS-publish, which + covers a declaration st2 owns. It does not cover a declaration generated + read-only by configuration management, where the next activation overwrites the + runtime edit — [#305](https://github.com/compoundingtech/st2/issues/305) lists + four candidate shapes (overlay file, dedicated fragment, split field ownership, + stable container Resources) and chooses none. On the catalog measured this is + not yet live: declarations are writable regular files carrying + `meta { managed-by "agent-spec-authoring" }`, with no `/nix/store` symlinks. + Resolves by: a downstream that actually generates declarations stating which + shape it needs, or a decision that st2 does not model this. + +- **DQ-R7 What clears the 987 placeholder reasons?** 987 of 1161 live bindings + carry the literal string + `reason="Legacy binding retained without recorded rationale."`, a mechanical + backfill from 2026-08-22 when + [#307](https://github.com/compoundingtech/st2/pull/307) made `reason` + required. Declarations authored since carry real prose — 26 of 27 touched on + 2026-08-26 and 2026-08-27 are placeholder-free — so the field works and this is + cleanup, not redesign. + [Decision 0013](../.decisions/0013-resource-is-a-mediated-write-surface.md) + makes the rewrite cheap. `inactive-reason` is used once fleet-wide, too little + evidence to judge. Resolves by: a cleanup pass, or a decision to let the + placeholders age out through ordinary republication. + +- **DQ-R8 When does the canonical Agent Spec get synced?** Upstream + `evals/AGENT-SPEC.md` was last touched 2026-08-20; #307 landed 2026-08-23. The + canonical document still describes the envelope as closed at name and `uri`, + and still says unsupported properties such as `_tag` fail validation, without + mentioning that `reason` is now required. st2 did not diverge deliberately — + the document is three days stale, and it is the authority st2's own ontology + cites. Resolves by: an upstream sync. Until then, treat + [`crates/agent-spec/src/spec.rs`](../../../crates/agent-spec/src/spec.rs) as + the live contract. + +- **DQ-R10 Do the 241 orphaned linked-record files get removed?** + [Decision 0011](../.decisions/0011-the-linked-record-plane-is-retired.md) + leaves them in place: most belong to retired declarations, and migrating them + into `axe work` is blocked because it writes to a gitignored per-worktree path + those declarations no longer have. They are unreferenced once the verb is gone. + Resolves by: a decision to sweep `resources/links/` during some later catalog + maintenance, or to leave them as inert history. diff --git a/docs/vrs/07-resource/spec.md b/docs/vrs/07-resource/spec.md new file mode 100644 index 00000000..826a4dde --- /dev/null +++ b/docs/vrs/07-resource/spec.md @@ -0,0 +1,220 @@ +# Resource + +A Resource is an externally identified thing an agent points at. This document +states the one edge that reaches it, why a second edge existed and was retired, +and what the surface looks like once *resource* names one concept. + +Terms are defined in [ontology.md](../ontology.md): [Resource](../ontology.md#resource), +[Resource binding](../ontology.md#resource-binding), +[agent resource directory](../ontology.md#agent-resource-directory), +[working state](../ontology.md#working-state). + +Decisions: [0011](../.decisions/0011-the-linked-record-plane-is-retired.md), +[0012](../.decisions/0012-working-state-is-a-declared-carrier.md), +[0013](../.decisions/0013-resource-is-a-mediated-write-surface.md). + +## The edge + +One agent, one binding, one Resource: + +```kdl +resource "work" reason="PR this agent is preparing." \ + uri="github-pr://github.com/example/project/42" +``` + +The positional name is an agent-local semantic role, unique within one agent. +`uri` is an RFC 3986 absolute URI preserved byte-for-byte, and it is the +Resource's identity. `reason` is required prose saying why this agent carries +it. `inactive-reason`, when present, retains a reference that is no longer +current and explains why. + +The URI is never normalized, and its scheme is the exact lookup key for an +optional, catalog-declared Resource Profile +([`07-resource-profile`](../07-resource-profile/requirements.md), landed in +[#351](https://github.com/compoundingtech/st2/pull/351)). Scheme meaning stays +downstream-owned, st2 ships no built-in profiles, and an unregistered scheme +stays opaque ([R20](../requirements.md)). Possession of a URI grants no +authority, access, or capability +([#61](https://github.com/compoundingtech/st2/issues/61)). + +Bindings are desired state. They change through publication under +compare-and-swap, and a binding-only change never stops, replaces, or relaunches +healthy work ([R21](../requirements.md)). + +The Rust type is +[`crates/agent-spec/src/spec.rs`](../../../crates/agent-spec/src/spec.rs) +`Resource { name, uri, reason, inactive_reason }`. + +## Identity and realization + +A binding names a carrier; the carrier's bytes live in the +[agent resource directory](../ontology.md#agent-resource-directory): + +| binding | scheme | realizes as | +|---|---|---| +| `notes` | `agent-notes` | agent notes | +| `goal` | `dev.schickling.agent-goal` | `resources/goal.md` | +| `decisions` | `decision-tree` | `resources/context/decisions/` | +| `working-state` | `working-state` | `resources/context/now.md` | + +The URI is identity; the path is realization. st2 does not resolve one into the +other, and `resources/` is not a second sense of *resource* — it is where +bindings land. The directory remains canonical for an agent's resource files and +also holds the message planes (`inbox/`, `archive/`, `sent/`) and scratch +material (`tmp/`), which are not carriers. + +`working-state` is the sixth self-state carrier +([decision 0012](../.decisions/0012-working-state-is-a-declared-carrier.md)). +Before it, 605 of 655 declarations on one live catalog had a +`resources/context/now.md` and none declared it. + +## Surface + +`st2 resource` reads and writes the binding plane for one agent: + +```text +st2 resource ls [] [--json] +st2 resource read [] [--json] +st2 resource add --uri --reason [--inactive-reason ] [--agent ] [--json] +st2 resource remove [--agent ] [--json] +st2 resource rename [--agent ] [--json] +``` + +``` +$ st2 resource ls dev3.dotfiles.fb-batch1.docs.worker +# 5 resources for dev3.dotfiles.fb-batch1.docs.worker + decisions decision-tree://dev3/dotfiles.fb-batch1.docs.worker + dotfiles-checkout worktree://dev3/…/2026-08-26-fb-batch1-docs + friction-log dev.schickling.agent-friction-log://dev3/dotfiles.fb-batch1.docs.worker + goal dev.schickling.agent-goal://dev3/dotfiles.fb-batch1.docs.worker + private-notes dev.schickling.agent-private-notes://dev3/dotfiles.fb-batch1.docs.worker +``` + +Rows are ordered by binding name, not declaration order — declaration order has no +meaning. The name column pads to the widest name in the listing. The worktree URI +above is elided at `…`; the command prints it in full. + +The name column is aligned to the widest name; the checkout URI is elided in this +document, not by `ls`, which prints every URI verbatim. + +The read verbs project one agent's declared bindings; before them, bindings were +visible only through `st2 agents --json`. A read takes a leading identity and a +write takes `--agent `, both defaulting to the caller +(`--as` / `$ST_AGENT`); every verb inherits `--catalog`, `--root`, `--as`, and +`--host`. The write verbs perform read-modify-CAS-publish internally and emit a +stable `--json` receipt, so the caller never renders KDL. Full-catalog +validation, exact-target selection, compare-and-swap, and fail-closed +concurrent-change behavior are preserved, and a +binding-only change does not stop, replace, or relaunch healthy work +([R21](../requirements.md)). This is the fourth instance of the pattern in +[`src/agent_author.rs`](../../../src/agent_author.rs), after streams, desired +state, and presentation +([decision 0013](../.decisions/0013-resource-is-a-mediated-write-surface.md)). +Decisions [0011](../.decisions/0011-the-linked-record-plane-is-retired.md) and +0010 carry "merge and acceptance approval required: upstream maintainers". + +Each write is idempotent on its outcome rather than its attempt: `add` upserts +one name, reporting identical bytes as unchanged without republishing them; +`remove` reports an already-absent name as unchanged; and `rename` refuses an +absent name and a collision, since names are unique within one agent. A mutation +refuses an invalid URI, an empty reason, and a Nix-owned declaration, and +preserves the URI byte-for-byte along with the declaration's unrelated bytes. + +Bindings remain projected through `st2 agents --json` as opaque descriptors, per +`INVARIANTS.md`. + +## The retired plane + +`st2 resource add|ls|read|remove` previously managed **linked records** — one +markdown file per record with `url`, optional `title`/`tags`/`relation`, and an +optional body, under `/resources/links/`. An agent wrote them itself, +without publication or CAS, to record what it produced. + +That plane is retired +([decision 0011](../.decisions/0011-the-linked-record-plane-is-retired.md)). +Recording produced artifacts is `axe work update --artifact --pty `. + +`templates/bus.st2.md` advertised `st2 resource add|ls|read|remove` to every +agent on the bus, which is what produced the adoption measured below. It now +carries the binding verbs instead. + +### Why it existed and why it went + +Measured on one live catalog of 655 declarations +([experiment](.experiments/2026-08-27-resource-read-surfaces.md)): + +- 1161 bindings across 647 declarations; 241 linked records across 82. +- 889 distinct binding URIs and 233 distinct linked-record URLs, **zero + coinciding** — exactly, and normalized across all five schemes both planes + used. 81 of the 82 linked-record-carrying declarations also carried bindings, + so the planes coexisted constantly and never named one thing. +- The only reader of a linked record in the source tree was its own `ls`/`read` + verb. No projection, roster, or doctor consumed them. +- Adoption was 12.5% of declarations and decaying: 45 records on 2026-08-08, + five on 2026-08-26, none after. + +The disjointness was semantic. Bindings carried what an agent *is for* — 728 +self-state carriers and 378 work inputs. Linked records carried what it *made* — +196 of 241 were `output`, `produces`, `evidence`, or `verified`. Where a +declared worktree and a linked output shared a path the relation was +containment, the product sitting inside the input, not identity. + +So a shared descriptor type would have deduplicated nothing and joined nothing. +[#122](https://github.com/compoundingtech/st2/issues/122) proposed exactly that, +around a `_tag` field removed in #307. + +### What the split actually cost + +The remaining 19% of linked records name the mechanism: `supervises`, +`reference`, `current-work` (which duplicates the `work` binding), +`depends-on-slice-*`, `blocked-design`. Those are dependency and reference edges +filed in the products plane. + +An agent could not cheaply record a dependency. A binding needed publisher +authority and whole-declaration republication under compare-and-swap. So it +wrote a linked record, because that was the plane it was permitted to write. + +**The drift was caused by write-cost asymmetry, not by two relations.** Retiring +the plane removes the escape hatch; the mediated write verbs +([decision 0013](../.decisions/0013-resource-is-a-mediated-write-surface.md)) +remove the pressure that produced it. + +### Reported friction + +Two declarations, from +[dotfiles#2071](https://github.com/schickling/dotfiles/pull/2071): one with five +bindings and no linked records reported `# 0 resources`; one with two bindings +and fourteen linked records reported fourteen rows containing neither binding. +Both answers were correct and neither surface said which question it answered. + +The read surfaces were also inverted against their authority. Bindings were +governed by R20 and R21 and pinned by an invariant, and had no human surface at +all. Linked records were governed by nothing and owned the short, obvious verb. +An agent looking for its bindings found the linked-record verb first, because it +was the only one whose name matched what it was looking for. + +The boundary had been correct in writing since 2026-07-30: +[#61](https://github.com/compoundingtech/st2/issues/61)'s resolution states that +`st2 resource` manages linked-reference records and does not mutate a binding. +That sentence never reached the command's own words. With one plane, it no +longer needs to. + +### Residue + +The 241 existing records stay in place as orphaned files. Most belong to retired +declarations whose worktrees are gone, and `axe work` writes to a gitignored +per-worktree path that no longer exists for them. The ontology keeps a retired +entry so a reader who meets one can identify it. + +## Boundaries + +Nothing here claims mutation authority over the thing a URI names, and nothing +makes possessing a URI mean something. A mediated write changes a declaration; +it does not touch the referent. + +Resolution is deliberately out of scope for this document, not absent from st2: +[#351](https://github.com/compoundingtech/st2/pull/351) owns the scheme-keyed +Resource Profile registry under +[`07-resource-profile`](../07-resource-profile/requirements.md). The two are +orthogonal — a binding names a Resource whether or not any profile resolves its +scheme, and `st2 resource` neither reads nor requires one. diff --git a/docs/vrs/ontology.md b/docs/vrs/ontology.md index 503e5d9c..ebdb9291 100644 --- a/docs/vrs/ontology.md +++ b/docs/vrs/ontology.md @@ -158,6 +158,86 @@ observed. The origin timestamp is durable; age is a projection and never file mtime. Authority: [native driver diagnostic snapshot](05-harness-state/spec.md#native-driver-diagnostic-snapshot-ohs-r11ohs-r15) +### Resource + +An externally identified thing an agent points at, named by an absolute URI. +st2 preserves the URI's exact bytes and never normalizes them. The scheme is the +exact lookup key for an optional, catalog-declared +[Resource Profile](07-resource-profile/requirements.md); scheme meaning stays +downstream-owned and st2 ships no built-in profiles, so an unregistered scheme +stays opaque. Possession of a URI grants no authority, access, or capability. + +One concept, one edge: a Resource is reached through a +[Resource binding](#resource-binding). The [linked record](#linked-record-retired) +plane that once shared the word is retired. + +Authority: [R20 portable Resource bindings](requirements.md#L161-L168); +[issue #61 resolution](https://github.com/compoundingtech/st2/issues/61) + +### Resource binding + +A publisher-declared edge from one agent to one Resource, written as a +`resource` node in the agent declaration and carrying an agent-local unique +name, the URI, a required `reason`, and an optional `inactive-reason`. Bindings +are desired state: they change only through compare-and-swap publication of the +whole declaration, and a binding-only change never stops, replaces, or relaunches +healthy work. + +A binding says what an agent *is for* — the work it reads and the durable state +carriers it owns. It is not a record of what the agent produced. + +Authority: [`Resource`](../../crates/agent-spec/src/spec.rs#L239-L245) — the +live contract; [R20](requirements.md#L161-L168); [R21](requirements.md#L169-L172). +The canonical [Agent Spec Resource bindings](https://github.com/compoundingtech/evals/blob/main/AGENT-SPEC.md#agent-spec-resource-bindings) +anchor still describes the pre-#307 envelope of name and `uri` only, and would +reject the required `reason`; it is pending sync (07-resource DQ-R8). + +### linked record (retired) + +An agent-owned record of something an agent produced, stored as one markdown +file with `url`, optional `title`/`tags`/`relation`, and an optional body under +`/resources/links/`, written through `st2 resource add`. + +**Retired.** The term is kept only so a reader who meets a surviving record +under `resources/links/` can identify it. Recording produced artifacts is +`axe work update --artifact --pty `. Nothing in st2 reads a linked +record, and *resource* now names only the declared plane. + +Authority: [07-resource spec](07-resource/spec.md); +[decision 0011](.decisions/0011-the-linked-record-plane-is-retired.md) + +### agent resource directory + +The per-agent directory `/resources/`, canonical for an agent's +resource files. It holds the message planes (`inbox/`, `archive/`, `sent/`), +working state and decisions (`context/`), scratch material (`tmp/`), and the +realized carriers themselves (`goal.md` and siblings). + +It is not a separate meaning of *resource*. It is the **realization surface** +for [Resource bindings](#resource-binding): a binding names a carrier by URI and +the carrier's bytes live here. `dev.schickling.agent-goal:///` +realizes as `resources/goal.md`; `decision-tree:///` realizes as +`resources/context/decisions/`. Identity is the URI; the path is realization, +and st2 does not resolve one into the other ([R20](requirements.md#L161-L168)). + +Authority: [`message::with_resolved_state_dir`](../../src/message.rs); +[07-resource spec](07-resource/spec.md) + +### working state + +An agent's restored durable context — what it is doing, what it decided, and what +it ruled out — written through `st2 context` and realized at +`resources/context/now.md`. Never a liveness or activity term; the observed +signal is [observed harness state](#observed-harness-state). + +Addressed as a [Resource binding](#resource-binding) under the scheme +`working-state:///`. st2 writes the carrier through +`st2 context`; resolving the scheme is a catalog's choice via an optional +[Resource Profile](07-resource-profile/requirements.md), not something st2 ships. + +Authority: [R09 state continuity](requirements.md#L131-L132); +[`context`](../../src/context.rs); +[decision 0012](.decisions/0012-working-state-is-a-declared-carrier.md) ### restart policy @@ -409,6 +489,15 @@ the leitwort for harness activity evidence. - Use [agent identity](../../crates/agent-spec/src/spec.rs#L24-L50) for the bare value and [bus ID](../../crates/agent-spec/src/spec.rs#L203-L211) for the host-qualified address. +- **Resource** names one concept: a [Resource binding](#resource-binding) and + nothing else. The [linked record](#linked-record-retired) plane that once shared the + word is retired. Do not reintroduce a second sense. +- The [agent resource directory](#agent-resource-directory) is not a second + sense either — it is where bindings are realized. Say *binding* for the + declared edge and *carrier* for the realized bytes when both are in view. +- Use [working state](#working-state) for R09's restored durable context. The + verb is `st2 context` and the directory is `resources/context/`, but the + canonical term and its scheme are *working state*, not *context*. - Use [message](../../src/message.rs#L26-L46) for the durable record and [DING](../../src/ding/mod.rs#L1-L14) for its terminal notification. - Qualify **event**: a bare *event* in stream context is the durable diff --git a/src/agent_author.rs b/src/agent_author.rs index b2e2e151..5152a8ec 100644 --- a/src/agent_author.rs +++ b/src/agent_author.rs @@ -14,8 +14,8 @@ use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; use std::path::{Path, PathBuf}; use agent_spec::spec::{ - AGENT_DESCRIPTION_MAX_CHARS, AGENT_NAME_MAX_CHARS, StreamLaunch, validate_desired_state_reason, - validate_presentation, + AGENT_DESCRIPTION_MAX_CHARS, AGENT_NAME_MAX_CHARS, Resource, StreamLaunch, + validate_desired_state_reason, validate_presentation, }; use kdl::{KdlDocument, KdlNode}; use serde::Serialize; @@ -135,6 +135,34 @@ pub struct StreamRemoveReceipt { pub name: String, } +/// Stable machine-readable receipt from adding or updating one Resource binding. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ResourceAddReceipt { + pub result: AuthorOutcome, + pub identity: String, + pub name: String, + pub uri: String, + pub reason: String, + pub inactive_reason: Option, +} + +/// Stable machine-readable receipt from removing one Resource binding. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ResourceRemoveReceipt { + pub result: AuthorOutcome, + pub identity: String, + pub name: String, +} + +/// Stable machine-readable receipt from relabelling one Resource binding. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ResourceRenameReceipt { + pub result: AuthorOutcome, + pub identity: String, + pub old: String, + pub new: String, +} + /// A classified authoring refusal. `code` is stable for machine consumers. #[derive(Debug)] pub struct AuthorError { @@ -316,6 +344,167 @@ fn author_stream( Ok((result, target.identity)) } +/// Declare one Resource binding, or update the binding that already carries `name`. +/// +/// st2 preserves the binding for readers; it resolves nothing and grants nothing. `uri` is the +/// exact absolute identity and is stored byte for byte with no normalization. +#[allow(clippy::too_many_arguments)] +pub fn add_resource( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + name: &str, + uri: &str, + reason: &str, + inactive_reason: Option<&str>, +) -> Result { + author_resource( + catalog_root, + selector, + this_host, + actor, + ResourceIntent::Upsert { + name, + uri, + reason, + inactive_reason, + }, + ) + .map(|(result, identity)| ResourceAddReceipt { + result, + identity, + name: name.to_owned(), + uri: uri.to_owned(), + reason: reason.to_owned(), + inactive_reason: inactive_reason.map(str::to_owned), + }) +} + +/// Remove one Resource binding. An already absent binding is an idempotent success. +pub fn remove_resource( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + name: &str, +) -> Result { + author_resource( + catalog_root, + selector, + this_host, + actor, + ResourceIntent::Remove { name }, + ) + .map(|(result, identity)| ResourceRemoveReceipt { + result, + identity, + name: name.to_owned(), + }) +} + +/// Relabel one Resource binding, carrying its `uri`, `reason`, and `inactive-reason` unchanged. +/// +/// An absent `old` and an already declared `new` both refuse: binding names are unique within one +/// agent, so neither request has an outcome that preserves the caller's intent. +pub fn rename_resource( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + old: &str, + new: &str, +) -> Result { + author_resource( + catalog_root, + selector, + this_host, + actor, + ResourceIntent::Rename { old, new }, + ) + .map(|(result, identity)| ResourceRenameReceipt { + result, + identity, + old: old.to_owned(), + new: new.to_owned(), + }) +} + +/// One requested Resource-binding mutation, resolved against the declaration under the lock. +#[derive(Debug, Clone, Copy)] +enum ResourceIntent<'a> { + Upsert { + name: &'a str, + uri: &'a str, + reason: &'a str, + inactive_reason: Option<&'a str>, + }, + Remove { + name: &'a str, + }, + Rename { + old: &'a str, + new: &'a str, + }, +} + +/// The binding state a candidate must read back as before it may be committed. +#[derive(Debug)] +struct ResourceExpectation { + absent: Option, + present: Option, +} + +fn author_resource( + catalog_root: &Path, + selector: &str, + this_host: &str, + actor: Option<&str>, + intent: ResourceIntent<'_>, +) -> Result<(AuthorOutcome, String), AuthorError> { + let catalog_lock = CatalogLock::exclusive(catalog_root).map_err(|error| { + AuthorError::new( + "catalog-lock-failed", + format!("acquire catalog-authoring lock: {error:#}"), + ) + })?; + let found = crate::discover_strict(catalog_root); + if let Some(error) = found.errors.first() { + return Err(AuthorError::new( + "catalog-malformed", + format!( + "cannot prove an exact resource target while {} is malformed: {}", + error.path.display(), + error.message + ), + )); + } + let target = resolve_target(&found.specs, selector, this_host)?; + let actor = actor + .map(|actor| resolve_target(&found.specs, actor, this_host).map(|target| target.identity)) + .transpose()?; + authorize_actor( + &found.specs, + &target.identity, + this_host, + actor.as_deref(), + "resource-not-authorized", + )?; + let result = edit_resource_declaration( + &catalog_lock, + catalog_root, + &crate::catalog_transaction::retained_dir_path(catalog_lock.control()) + .map_err(|error| AuthorError::new("declaration-write-failed", error.to_string()))?, + &target.declaration, + &target.identity, + &target.source_host, + &target.source_identity, + intent, + || {}, + )?; + Ok((result, target.identity)) +} + /// Author one whole-agent desired state without claiming runtime convergence. pub fn set_desired_state( catalog_root: &Path, @@ -855,7 +1044,8 @@ fn verify_stream_candidate( } } -fn edit_declaration( +#[allow(clippy::too_many_arguments)] +fn edit_resource_declaration( catalog_lock: &CatalogLock, catalog: &Path, control: &Path, @@ -863,15 +1053,14 @@ fn edit_declaration( expected_identity: &str, expected_host: &str, expected_agent: &str, - field: PresentationField, - requested: Option<&str>, + intent: ResourceIntent<'_>, 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 {}", + "resource authoring requires canonical KDL, found {}", path.display() ), )); @@ -917,16 +1106,17 @@ fn edit_declaration( ), )); } - let Some(replacement) = presentation_edit(text, target, field, requested)? else { + let Some((replacement, expectation)) = resource_edit(text, target, intent)? else { return Ok(AuthorOutcome::Unchanged); }; - verify_candidate( + verify_resource_candidate( + catalog, + path, &replacement, expected_identity, expected_host, expected_agent, - field, - requested, + &expectation, )?; atomic_replace_checked( catalog_lock, @@ -942,8 +1132,258 @@ fn edit_declaration( Ok(AuthorOutcome::Changed) } -#[allow(clippy::too_many_arguments)] -fn edit_desired_state_declaration( +/// Resolve one intent against the declared bindings, preserving every unrelated byte. +/// +/// `Ok(None)` is the proven no-op: an unchanged upsert, an absent removal, or a self-rename. A +/// changed upsert rewrites exactly the one binding node in place, so its position, its leading +/// trivia, and every sibling binding survive. +fn resource_edit( + text: &str, + target: &KdlNode, + intent: ResourceIntent<'_>, +) -> Result, AuthorError> { + let declared = target + .children() + .into_iter() + .flat_map(|children| children.nodes()) + .filter(|child| child.name().value() == "resource") + .collect::>(); + let declaring = |name: &str| -> Result, AuthorError> { + let matches = declared + .iter() + .copied() + .filter(|child| child.get(0).and_then(|entry| entry.as_string()) == Some(name)) + .collect::>(); + if matches.len() > 1 { + return Err(AuthorError::new( + "duplicate-resource", + format!("target declares resource {name:?} more than once"), + )); + } + Ok(matches.first().copied()) + }; + match intent { + ResourceIntent::Upsert { + name, + uri, + reason, + inactive_reason, + } => { + let authored = declared_resource(name, uri, reason, inactive_reason)?; + let replacement = match declaring(name)? { + Some(node) if parsed_resource(node)? == authored => return Ok(None), + Some(node) => replace_node(text, node, &render_resource(&authored)?)?, + None => insert_node(text, target, &render_resource(&authored)?)?, + }; + Ok(Some(( + replacement, + ResourceExpectation { + absent: None, + present: Some(authored), + }, + ))) + } + ResourceIntent::Remove { name } => { + let Some(node) = declaring(name)? else { + return Ok(None); + }; + Ok(Some(( + remove_field(text, node)?, + ResourceExpectation { + absent: Some(name.to_owned()), + present: None, + }, + ))) + } + ResourceIntent::Rename { old, new } => { + let Some(node) = declaring(old)? else { + return Err(AuthorError::new( + "resource-not-found", + format!("target declares no resource {old:?}"), + )); + }; + if old == new { + return Ok(None); + } + if declaring(new)?.is_some() { + return Err(AuthorError::new( + "resource-already-exists", + format!( + "target already declares resource {new:?}; binding names are unique within one agent" + ), + )); + } + let carried = parsed_resource(node)?; + let renamed = declared_resource( + new, + carried.uri(), + carried.reason(), + carried.inactive_reason(), + )?; + Ok(Some(( + replace_node(text, node, &render_resource(&renamed)?)?, + ResourceExpectation { + absent: Some(old.to_owned()), + present: Some(renamed), + }, + ))) + } + } +} + +/// Enforce the canonical binding invariants — `agent_spec` owns them; this mints no new rule. +fn declared_resource( + name: &str, + uri: &str, + reason: &str, + inactive_reason: Option<&str>, +) -> Result { + match inactive_reason { + None => Resource::new(name.to_owned(), uri.to_owned(), reason.to_owned()), + Some(inactive_reason) => Resource::new_inactive( + name.to_owned(), + uri.to_owned(), + reason.to_owned(), + inactive_reason.to_owned(), + ), + } + .map_err(|error| AuthorError::new("invalid-resource", error)) +} + +fn parsed_resource(node: &KdlNode) -> Result { + let malformed = + |detail: &str| AuthorError::new("malformed-resource", format!("resource binding {detail}")); + if node.children().is_some() { + return Err(malformed("cannot have children")); + } + let mut name = None; + let mut uri = None; + let mut reason = None; + let mut inactive_reason = None; + for entry in node.entries() { + let value = entry + .value() + .as_string() + .ok_or_else(|| malformed("accepts only string values"))?; + let slot = match entry.name().map(|name| name.value()) { + None => &mut name, + Some("uri") => &mut uri, + Some("reason") => &mut reason, + Some("inactive-reason") => &mut inactive_reason, + Some(other) => return Err(malformed(&format!("has unsupported property `{other}`"))), + }; + if slot.replace(value).is_some() { + return Err(malformed("declares one of its fields more than once")); + } + } + let (Some(name), Some(uri), Some(reason)) = (name, uri, reason) else { + return Err(malformed("needs a name, a `uri`, and a `reason`")); + }; + declared_resource(name, uri, reason, inactive_reason) +} + +fn render_resource(resource: &Resource) -> Result { + let mut authored = format!( + "resource {} uri={} reason={}", + quoted(resource.name())?, + quoted(resource.uri())?, + quoted(resource.reason())? + ); + if let Some(inactive_reason) = resource.inactive_reason() { + authored.push_str(&format!(" inactive-reason={}", quoted(inactive_reason)?)); + } + Ok(authored) +} + +/// Replace exactly one node's source span. A KDL node span carries neither the leading trivia nor +/// the trailing terminator, so the surrounding line survives untouched. +fn replace_node(text: &str, node: &KdlNode, authored: &str) -> Result { + let span = node.span(); + let range = span.offset()..span.offset() + span.len(); + text.get(range.clone()).ok_or_else(|| { + AuthorError::new( + "malformed-declaration", + "resource binding span falls outside the declaration", + ) + })?; + // The span can run to the start of trailing trivia, so replacing it verbatim would glue the + // rendered node onto a following `// comment`. Leave that separator in the source. + let kept = text[range.clone()].trim_end_matches([' ', '\t']).len(); + let mut replacement = text.to_owned(); + replacement.replace_range(range.start..range.start + kept, authored); + Ok(replacement) +} + +fn verify_resource_candidate( + catalog: &Path, + path: &Path, + candidate: &str, + expected_identity: &str, + expected_host: &str, + expected_agent: &str, + expectation: &ResourceExpectation, +) -> Result<(), AuthorError> { + let temporary = tempfile::tempdir() + .map_err(|error| AuthorError::new("unsafe-source-edit", error.to_string()))?; + let relative = path.strip_prefix(catalog).map_err(|_| { + AuthorError::new( + "unsafe-declaration-path", + format!( + "declaration {} is outside catalog {}", + path.display(), + catalog.display() + ), + ) + })?; + let candidate_path = temporary.path().join(relative); + fs::create_dir_all( + candidate_path + .parent() + .expect("candidate declaration has a parent"), + ) + .and_then(|()| fs::write(&candidate_path, candidate)) + .map_err(|error| { + AuthorError::new( + "unsafe-source-edit", + format!("stage resource validation: {error}"), + ) + })?; + let (specs, _) = agent_spec::discover_file(temporary.path(), &candidate_path) + .map_err(|error| AuthorError::new("invalid-resource", error.to_string()))?; + let spec = specs + .iter() + .find(|spec| { + spec.identity == expected_agent && spec.bus_id(expected_host) == expected_identity + }) + .ok_or_else(|| { + AuthorError::new( + "unsafe-source-edit", + "resource candidate lost the authored agent", + ) + })?; + let declares = |name: &str| { + spec.resources + .iter() + .find(|resource| resource.name() == name) + }; + if expectation + .absent + .as_deref() + .is_some_and(|name| declares(name).is_some()) + || expectation + .present + .as_ref() + .is_some_and(|expected| declares(expected.name()) != Some(expected)) + { + return Err(AuthorError::new( + "unsafe-source-edit", + "resource candidate did not read back as the authored intent", + )); + } + Ok(()) +} + +fn edit_declaration( catalog_lock: &CatalogLock, catalog: &Path, control: &Path, @@ -951,15 +1391,15 @@ fn edit_desired_state_declaration( expected_identity: &str, expected_host: &str, expected_agent: &str, - state: DesiredStateValue, - reason: Option<&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!( - "desired-state authoring requires canonical KDL, found {}", + "presentation authoring requires canonical KDL, found {}", path.display() ), )); @@ -1005,16 +1445,16 @@ fn edit_desired_state_declaration( ), )); } - let Some(replacement) = desired_state_edit(text, target, state, reason)? else { + let Some(replacement) = presentation_edit(text, target, field, requested)? else { return Ok(AuthorOutcome::Unchanged); }; - verify_desired_state_candidate( + verify_candidate( &replacement, expected_identity, expected_host, expected_agent, - state, - reason, + field, + requested, )?; atomic_replace_checked( catalog_lock, @@ -1030,9 +1470,97 @@ fn edit_desired_state_declaration( Ok(AuthorOutcome::Changed) } -fn desired_state_edit( - text: &str, - target: &KdlNode, +#[allow(clippy::too_many_arguments)] +fn edit_desired_state_declaration( + catalog_lock: &CatalogLock, + catalog: &Path, + control: &Path, + path: &Path, + expected_identity: &str, + expected_host: &str, + expected_agent: &str, + state: DesiredStateValue, + reason: 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!( + "desired-state authoring requires canonical KDL, found {}", + path.display() + ), + )); + } + let metadata = fs::symlink_metadata(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + if !metadata.file_type().is_file() { + return Err(AuthorError::new( + "unsafe-declaration-path", + format!("refusing non-regular declaration path {}", path.display()), + )); + } + let original = fs::read(path).map_err(|error| { + AuthorError::new( + "declaration-read-failed", + format!("reading declaration {}: {error}", path.display()), + ) + })?; + let original_version = SourceVersion::from_metadata(&metadata); + let text = std::str::from_utf8(&original).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("declaration {} is not UTF-8: {error}", path.display()), + ) + })?; + let document = KdlDocument::parse(text).map_err(|error| { + AuthorError::new( + "malformed-declaration", + format!("parsing declaration {}: {error}", path.display()), + ) + })?; + let target = exact_agent_node(&document, expected_identity, expected_host, expected_agent)?; + if is_nix_managed(target) { + return Err(AuthorError::new( + "nix-managed-declaration", + format!( + "agent {expected_identity:?} is Nix-owned; edit its Nix source instead of {}", + path.display() + ), + )); + } + let Some(replacement) = desired_state_edit(text, target, state, reason)? else { + return Ok(AuthorOutcome::Unchanged); + }; + verify_desired_state_candidate( + &replacement, + expected_identity, + expected_host, + expected_agent, + state, + reason, + )?; + atomic_replace_checked( + catalog_lock, + catalog, + control, + path, + &original, + original_version, + replacement.as_bytes(), + metadata.permissions().mode() & 0o7777, + before_commit, + )?; + Ok(AuthorOutcome::Changed) +} + +fn desired_state_edit( + text: &str, + target: &KdlNode, state: DesiredStateValue, reason: Option<&str>, ) -> Result, AuthorError> { @@ -1347,6 +1875,15 @@ fn insert_node(text: &str, target: &KdlNode, authored: &str) -> Result bool { + let rest = tail.trim_start_matches([' ', '\t', '\r']); + rest.is_empty() || rest.starts_with("//") +} + fn remove_field(text: &str, node: &KdlNode) -> Result { let span = node.span(); let start = span.offset(); @@ -1364,9 +1901,7 @@ fn remove_field(text: &str, node: &KdlNode) -> Result { if text[line_start..start] .chars() .all(|value| matches!(value, ' ' | '\t')) - && text[end..line_end] - .chars() - .all(|value| matches!(value, ' ' | '\t' | '\r')) + && is_line_tail_trivia(&text[end..line_end]) { let mut replacement = text.to_owned(); let remove_end = usize::min(line_end + usize::from(line_end < text.len()), text.len()); @@ -2084,4 +2619,470 @@ mod tests { ); assert_eq!(fs::read(declaration_path).unwrap(), original); } + + fn bound(root: &Path, identity: &str, name: &str) -> Resource { + crate::discover(root) + .specs + .into_iter() + .find(|spec| spec.identity == identity) + .expect("catalog declares the agent") + .resources + .into_iter() + .find(|resource| resource.name() == name) + .expect("agent declares the binding") + } + + #[test] + fn resource_add_declares_updates_in_place_and_is_idempotent() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + + let added = add_resource( + root, + "h.worker", + "h", + Some("h.worker"), + "work", + "github-issue://example/project/123", + "release work item", + None, + ) + .unwrap(); + assert_eq!(added.result, AuthorOutcome::Changed); + assert_eq!(added.identity, "h.worker"); + assert_eq!(added.inactive_reason, None); + + add_resource( + root, + "h.worker", + "h", + None, + "source", + "worktree://github.com/example/project/change", + "primary checkout", + None, + ) + .unwrap(); + let two_bindings = fs::read_to_string(&path).unwrap(); + + // An identical request proves the binding rather than rewriting the declaration. + assert_eq!( + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/123", + "release work item", + None, + ) + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!(fs::read_to_string(&path).unwrap(), two_bindings); + + // Re-declaring an existing name updates it in place, keeping its position and siblings. + assert_eq!( + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/456", + "follow-up work item", + Some("superseded by the follow-up"), + ) + .unwrap() + .result, + AuthorOutcome::Changed + ); + let updated = fs::read_to_string(&path).unwrap(); + assert_eq!(updated.matches("resource \"work\"").count(), 1); + assert!( + updated.find("resource \"work\"").unwrap() + < updated.find("resource \"source\"").unwrap() + ); + assert!(updated.contains("// keep this comment")); + assert!(updated.contains("keep \"exact\"")); + + let work = bound(root, "worker", "work"); + assert_eq!(work.uri(), "github-issue://example/project/456"); + assert_eq!(work.reason(), "follow-up work item"); + assert_eq!(work.inactive_reason(), Some("superseded by the follow-up")); + assert_eq!( + bound(root, "worker", "source").uri(), + "worktree://github.com/example/project/change" + ); + + // The request declares the complete binding, so an omitted inactive-reason clears it. + assert_eq!( + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/456", + "follow-up work item", + None, + ) + .unwrap() + .result, + AuthorOutcome::Changed + ); + assert_eq!(bound(root, "worker", "work").inactive_reason(), None); + let cleared = fs::read_to_string(&path).unwrap(); + assert!(!cleared.contains("inactive-reason")); + assert_eq!(cleared.matches("resource \"work\"").count(), 1); + } + + #[test] + fn resource_add_proves_a_hand_authored_binding_without_rewriting_it() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + "agent \"worker\" {\n host \"h\"\n command \"sleep 60\"\n \ + resource \"work\" reason=\"release work item\" uri=\"github-issue://example/project/123\"\n}\n", + ); + let original = fs::read_to_string(&path).unwrap(); + + // Hand-authored property order and spacing are proven, not re-rendered. + assert_eq!( + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/123", + "release work item", + None, + ) + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!(fs::read_to_string(&path).unwrap(), original); + } + + #[test] + fn resource_remove_is_idempotent_and_keeps_unrelated_bindings() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/123", + "release work item", + None, + ) + .unwrap(); + add_resource( + root, + "h.worker", + "h", + None, + "source", + "worktree://github.com/example/project/change", + "primary checkout", + None, + ) + .unwrap(); + + let removed = remove_resource(root, "h.worker", "h", Some("h.worker"), "work").unwrap(); + assert_eq!(removed.result, AuthorOutcome::Changed); + assert_eq!(removed.name, "work"); + let after_remove = fs::read_to_string(&path).unwrap(); + + assert_eq!( + remove_resource(root, "h.worker", "h", None, "work") + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!( + remove_resource(root, "h.worker", "h", None, "never-declared") + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!(fs::read_to_string(&path).unwrap(), after_remove); + assert!(!after_remove.contains("resource \"work\"")); + assert!(after_remove.contains("resource \"source\"")); + assert!(after_remove.contains("// keep this comment")); + assert_eq!(bound(root, "worker", "source").reason(), "primary checkout"); + } + + #[test] + fn resource_rename_carries_the_binding_and_refuses_absent_or_colliding_names() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + let path = write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + add_resource( + root, + "h.worker", + "h", + None, + "work", + "github-issue://example/project/123", + "release work item", + Some("merged and retained for traceability"), + ) + .unwrap(); + add_resource( + root, + "h.worker", + "h", + None, + "source", + "worktree://github.com/example/project/change", + "primary checkout", + None, + ) + .unwrap(); + let before = fs::read_to_string(&path).unwrap(); + + assert_eq!( + rename_resource(root, "h.worker", "h", None, "work", "work") + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + assert_eq!(fs::read_to_string(&path).unwrap(), before); + + let renamed = + rename_resource(root, "h.worker", "h", Some("h.worker"), "work", "task").unwrap(); + assert_eq!(renamed.result, AuthorOutcome::Changed); + assert_eq!(renamed.old, "work"); + assert_eq!(renamed.new, "task"); + + let task = bound(root, "worker", "task"); + assert_eq!(task.uri(), "github-issue://example/project/123"); + assert_eq!(task.reason(), "release work item"); + assert_eq!( + task.inactive_reason(), + Some("merged and retained for traceability") + ); + let authored = fs::read_to_string(&path).unwrap(); + assert!(!authored.contains("resource \"work\"")); + assert!( + authored.find("resource \"task\"").unwrap() + < authored.find("resource \"source\"").unwrap() + ); + + assert_eq!( + rename_resource(root, "h.worker", "h", None, "work", "elsewhere") + .unwrap_err() + .code(), + "resource-not-found" + ); + // An absent `old` refuses even when the rename would otherwise be a self-rename no-op. + assert_eq!( + rename_resource(root, "h.worker", "h", None, "absent", "absent") + .unwrap_err() + .code(), + "resource-not-found" + ); + assert_eq!( + rename_resource(root, "h.worker", "h", None, "task", "source") + .unwrap_err() + .code(), + "resource-already-exists" + ); + assert_eq!(fs::read_to_string(&path).unwrap(), authored); + } + + #[test] + fn resource_authoring_enforces_validation_authority_and_nix_ownership() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/root/agent.kdl", + &declaration("root", "h", None, "catalog"), + ); + let child = write( + root, + "h/child/agent.kdl", + &declaration("child", "h", Some("root"), "catalog"), + ); + write( + root, + "h/sibling/agent.kdl", + &declaration("sibling", "h", Some("root"), "catalog"), + ); + let nix_owned = write( + root, + "h/nix/agent.kdl", + &declaration("nix", "h", Some("root"), "nix"), + ); + let untouched = fs::read_to_string(&nix_owned).unwrap(); + + add_resource( + root, + "h.child", + "h", + Some("h.root"), + "work", + "github-issue://example/project/1", + "supervised work item", + None, + ) + .unwrap(); + + assert_eq!( + add_resource( + root, + "h.sibling", + "h", + Some("h.child"), + "work", + "github-issue://example/project/1", + "reaching across the fleet", + None, + ) + .unwrap_err() + .code(), + "resource-not-authorized" + ); + assert_eq!( + remove_resource(root, "h.sibling", "h", Some("h.child"), "work") + .unwrap_err() + .code(), + "resource-not-authorized" + ); + assert_eq!( + add_resource( + root, + "h.nix", + "h", + Some("h.root"), + "work", + "github-issue://example/project/1", + "Nix owns this declaration", + None, + ) + .unwrap_err() + .code(), + "nix-managed-declaration" + ); + + // A catalog-relative carrier path is admitted since #345, so the refusals worth pinning are + // the ones that escape the catalog, plus empty names and empty explanations. + for (name, uri, reason, inactive_reason) in [ + ("absolute-path", "/etc/passwd", "escapes the catalog", None), + ("parent-escape", "../outside", "escapes the catalog", None), + ("spaced", "issue://example/a b", "unencoded space", None), + ("", "issue://example/1", "empty name", None), + ("blank-reason", "issue://example/1", "", None), + ( + "blank-inactive", + "issue://example/1", + "still explained", + Some(""), + ), + ] { + let error = add_resource( + root, + "h.child", + "h", + None, + name, + uri, + reason, + inactive_reason, + ) + .unwrap_err(); + assert_eq!(error.code(), "invalid-resource", "{name}: {error}"); + } + assert_eq!( + fs::read_to_string(&child) + .unwrap() + .matches("resource ") + .count(), + 1 + ); + + // #345 widened the envelope: a catalog-relative carrier path is a valid binding uri. + add_resource( + root, + "h.child", + "h", + None, + "carrier", + "carriers/goal.md", + "Catalog-relative carrier.", + None, + ) + .expect("a catalog-relative carrier path is admitted"); + assert_eq!(fs::read_to_string(&nix_owned).unwrap(), untouched); + } + + #[test] + fn resource_uri_is_preserved_byte_for_byte() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "h", None, "catalog"), + ); + let exact = "vendor+Thing://Authority.Example/Exact%20Identity?Query=A%2Fb#Frag%20Ment"; + + add_resource( + root, + "h.worker", + "h", + None, + "subject", + exact, + "exact vendor identity", + None, + ) + .unwrap(); + assert_eq!(bound(root, "worker", "subject").uri(), exact); + + // The rename path carries the identity across without normalizing it either. + rename_resource(root, "h.worker", "h", None, "subject", "carried").unwrap(); + assert_eq!(bound(root, "worker", "carried").uri(), exact); + + // A byte-identical re-declaration is a proven no-op, not a rewrite. + assert_eq!( + add_resource( + root, + "h.worker", + "h", + None, + "carried", + exact, + "exact vendor identity", + None, + ) + .unwrap() + .result, + AuthorOutcome::Unchanged + ); + } } diff --git a/src/lib.rs b/src/lib.rs index 17b528e3..34a93833 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,7 +39,6 @@ pub mod pretrust; pub mod provider_session; pub mod reconcile; pub mod request; -pub mod resource; pub mod resync; pub mod run; pub mod service; diff --git a/src/main.rs b/src/main.rs index 15177b38..1d8897bd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -81,7 +81,8 @@ enum Command { /// An agent's working-state context for lossless restart: read/write/append. #[command(subcommand)] Context(ContextCmd), - /// An agent's linked resources (high-value output a peer can find): add/ls/read/remove. + /// An agent's declared Resource bindings (a named, exact URI a peer can resolve): + /// ls/read/add/remove/rename. #[command(subcommand)] Resource(ResourceCmd), /// Install `st2 up` as a systemd-user service on headless Linux. macOS stays manual (TCC). @@ -658,41 +659,73 @@ enum HooksCmd { #[derive(Subcommand)] enum ResourceCmd { - /// Link a resource (a URL you produced or reference) into your resource list. - Add { - /// The resource URL (any `scheme:` — http/https/file/pty/…). - url: String, - #[arg(long)] - title: Option, - /// Comma-separated tags. - #[arg(long = "tag", value_delimiter = ',')] - tags: Vec, - /// A relation label (e.g. `output`, `reference`). - #[arg(long)] - relation: Option, - /// Read a body/notes from stdin. - #[arg(long = "body-stdin")] - body_stdin: bool, - #[command(flatten)] - ctx: MsgCtx, - }, - /// List an agent's resources. Defaults to your own. + /// List an agent's declared Resource bindings. Defaults to your own. Ls { + /// Whose declaration to read — bus id or bare identity. Defaults to you (`$ST_AGENT`). identity: Option, + /// Emit the bindings as a JSON array. + #[arg(long)] + json: bool, #[command(flatten)] ctx: MsgCtx, }, - /// Read one resource. With a leading identity, from that agent; otherwise your own. + /// Read one declared binding. With a leading identity, from that agent; otherwise your own. Read { first: String, second: Option, + /// Emit the binding as a JSON object. + #[arg(long)] + json: bool, #[command(flatten)] ctx: MsgCtx, }, - /// Remove one resource. + /// Declare a Resource binding, or prove the identical binding already exists. + Add { + /// The agent-local binding name. + name: String, + /// The exact absolute URI this binding names (any `scheme:` — the identity is verbatim). + #[arg(long)] + uri: String, + /// Why this reference belongs in the declaration. + #[arg(long)] + reason: String, + /// Preserve the binding as no longer active for this agent, and say why. + #[arg(long = "inactive-reason", value_name = "TEXT")] + inactive_reason: Option, + /// Exact target agent; defaults to --as / $ST_AGENT. + #[arg(long)] + agent: Option, + /// Emit a stable JSON receipt. + #[arg(long)] + json: bool, + #[command(flatten)] + ctx: MsgCtx, + }, + /// Remove one declared binding, or prove it is already absent. Remove { - first: String, - second: Option, + /// The agent-local binding name. + name: String, + /// Exact target agent; defaults to --as / $ST_AGENT. + #[arg(long)] + agent: Option, + /// Emit a stable JSON receipt. + #[arg(long)] + json: bool, + #[command(flatten)] + ctx: MsgCtx, + }, + /// Rename one declared binding's agent-local label, keeping its uri and reasons. + Rename { + /// The current binding name. + old: String, + /// The new binding name. + new: String, + /// Exact target agent; defaults to --as / $ST_AGENT. + #[arg(long)] + agent: Option, + /// Emit a stable JSON receipt. + #[arg(long)] + json: bool, #[command(flatten)] ctx: MsgCtx, }, @@ -3136,100 +3169,210 @@ fn service_cmd(cmd: ServiceCmd) -> Result<()> { } } +/// Read one agent's declared Resource bindings. Selector resolution mirrors the mediated author +/// (`bus_id` first, then bare identity, unique or refuse) so `ls` and `add` always name the same +/// declaration, and a malformed catalog refuses rather than silently hiding an agent. +fn resource_bindings( + root: &Path, + selector: &str, + host: &str, +) -> Result<(String, Vec)> { + let _catalog_lock = st2::CatalogLock::shared(root) + .context("acquire shared catalog-authoring lock for Resource bindings")?; + let found = st2::discover_strict(root); + if let Some(error) = found.errors.first() { + anyhow::bail!( + "cannot prove an exact Resource binding target while {} is malformed: {}", + error.path.display(), + error.message + ); + } + let exact = found + .specs + .iter() + .filter(|spec| spec.bus_id(host) == selector) + .collect::>(); + let matches = if exact.is_empty() { + found + .specs + .iter() + .filter(|spec| spec.identity == selector) + .collect::>() + } else { + exact + }; + match matches.as_slice() { + [] => anyhow::bail!("no agent '{selector}' found in catalog {}", root.display()), + [spec] => Ok((spec.bus_id(host), spec.resources.clone())), + many => { + let mut candidates = many + .iter() + .map(|spec| format!("{} ({})", spec.bus_id(host), spec.path.display())) + .collect::>(); + candidates.sort(); + anyhow::bail!( + "agent selector '{selector}' is ambiguous: {}", + candidates.join(", ") + ) + } + } +} + fn resource_cmd(cmd: ResourceCmd) -> Result<()> { match cmd { - ResourceCmd::Add { - url, - title, - tags, - relation, - body_stdin, + ResourceCmd::Ls { + identity, + json, ctx, } => { - let (root, host) = resolve_ctx(&ctx)?; - let id = acting_id(&ctx)?; - let body = if body_stdin { - std::io::read_to_string(std::io::stdin())? - } else { - String::new() - }; - let f = message::with_resolved_state_dir( - &root, - &id, - &host, - &["resources", "links"], - true, - |dir| { - st2::resource::add( - dir, - &url, - title.as_deref(), - &tags, - relation.as_deref(), - &body, - ) - }, - )?; - println!("{f}"); - Ok(()) - } - ResourceCmd::Ls { identity, ctx } => { let (root, host) = resolve_ctx(&ctx)?; let id = match identity { Some(i) => i, None => acting_id(&ctx)?, }; - let dir = st2::resource::links_dir(&agent_dir_of(&root, &id, &host)?); - let items = st2::resource::list(&dir); - println!("# {} resource{} for {id}", items.len(), plural(items.len())); - for r in &items { - let title = r.title.as_deref().unwrap_or(""); - println!("{} {} {title}", r.filename, r.url); + let (identity, bindings) = resource_bindings(&root, &id, &host)?; + if json { + println!("{}", serde_json::to_string(&bindings)?); + return Ok(()); + } + println!( + "# {} resource{} for {identity}", + bindings.len(), + plural(bindings.len()) + ); + // Align the uri column to the widest name so a long binding name still leaves a + // separator. `{: { + ResourceCmd::Read { + first, + second, + json, + ctx, + } => { let (root, host) = resolve_ctx(&ctx)?; - let (id, filename) = box_target(first, second, &ctx)?; - let dir = st2::resource::links_dir(&agent_dir_of(&root, &id, &host)?); - let r = st2::resource::read(&dir, &filename)?; - println!("url: {}", r.url); - if let Some(t) = &r.title { - println!("title: {t}"); + let (id, name) = box_target(first, second, &ctx)?; + let (identity, bindings) = resource_bindings(&root, &id, &host)?; + let binding = bindings + .iter() + .find(|binding| binding.name() == name) + .with_context(|| format!("no resource binding '{name}' declared by {identity}"))?; + if json { + println!("{}", serde_json::to_string(binding)?); + return Ok(()); } - if !r.tags.is_empty() { - println!("tags: {}", r.tags.join(", ")); + println!("{:<17}{}", "name:", binding.name()); + println!("{:<17}{}", "uri:", binding.uri()); + println!("{:<17}{}", "reason:", binding.reason()); + if let Some(inactive_reason) = binding.inactive_reason() { + println!("{:<17}{}", "inactive-reason:", inactive_reason); } - if let Some(rel) = &r.relation { - println!("relation: {rel}"); + Ok(()) + } + ResourceCmd::Add { + name, + uri, + reason, + inactive_reason, + agent, + json, + ctx, + } => { + let (root, host, actor, target) = resource_author_target(agent, &ctx)?; + let receipt = st2::agent_author::add_resource( + &root, + &target, + &host, + actor.as_deref(), + &name, + &uri, + &reason, + inactive_reason.as_deref(), + )?; + if json { + println!("{}", serde_json::to_string(&receipt)?); + } else { + println!( + "{:?} resource {} on {}", + receipt.result, receipt.name, receipt.identity + ); } - if !r.body.is_empty() { - println!(); - print!("{}", r.body); + Ok(()) + } + ResourceCmd::Remove { + name, + agent, + json, + ctx, + } => { + let (root, host, actor, target) = resource_author_target(agent, &ctx)?; + let receipt = + st2::agent_author::remove_resource(&root, &target, &host, actor.as_deref(), &name)?; + if json { + println!("{}", serde_json::to_string(&receipt)?); + } else { + println!( + "{:?} resource {} on {}", + receipt.result, receipt.name, receipt.identity + ); } Ok(()) } - ResourceCmd::Remove { first, second, ctx } => { - let (root, host) = resolve_ctx(&ctx)?; - let (id, filename) = box_target(first, second, &ctx)?; - anyhow::ensure!( - message::is_message_filename(&filename), - "invalid resource filename {filename:?}" - ); - message::with_resolved_state_dir( + ResourceCmd::Rename { + old, + new, + agent, + json, + ctx, + } => { + let (root, host, actor, target) = resource_author_target(agent, &ctx)?; + let receipt = st2::agent_author::rename_resource( &root, - &id, + &target, &host, - &["resources", "links"], - false, - |dir| st2::resource::remove(dir, &filename), + actor.as_deref(), + &old, + &new, )?; - println!("removed"); + if json { + println!("{}", serde_json::to_string(&receipt)?); + } else { + println!( + "{:?} resource {} -> {} on {}", + receipt.result, receipt.old, receipt.new, receipt.identity + ); + } Ok(()) } } } +/// The catalog root, host, acting actor, and authored target for one mediated binding edit. +fn resource_author_target( + agent: Option, + ctx: &MsgCtx, +) -> Result<(PathBuf, String, Option, String)> { + let (root, host) = resolve_ctx(ctx)?; + let actor = ctx + .as_id + .clone() + .or_else(|| std::env::var("ST_AGENT").ok()) + .filter(|value| !value.is_empty()); + let target = agent + .or_else(|| actor.clone()) + .context("no resource binding target: pass --agent, --as, or set $ST_AGENT")?; + Ok((root, host, actor, target)) +} + /// Resolve an agent's context dir (`/resources/context`). Identity defaults to `$ST_AGENT`. fn resolve_context_dir(identity: Option, ctx: &MsgCtx) -> Result { let (root, host) = resolve_ctx(ctx)?; diff --git a/src/message.rs b/src/message.rs index 2ab71078..094fa757 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1264,7 +1264,7 @@ fn marker_state_exists(agent_dir: &Path) -> anyhow::Result { } let resources = agent_dir.join("resources"); if resources.is_dir() { - for relative in ["inbox", "archive", "context", "context/decisions", "links"] { + for relative in ["inbox", "archive", "context", "context/decisions"] { match fs::symlink_metadata(resources.join(relative)) { Ok(metadata) if metadata.is_dir() && !metadata.file_type().is_symlink() => {} Ok(_) => anyhow::bail!("agent resource path is not a real directory"), diff --git a/src/resource.rs b/src/resource.rs deleted file mode 100644 index c51ec8a0..00000000 --- a/src/resource.rs +++ /dev/null @@ -1,245 +0,0 @@ -//! Native resource links: an agent records high-value output (a PR, a doc, a dashboard) as a durable, -//! listable record so peers and supervisors can find it without digging through the inbox. -//! -//! Each resource is a markdown file with YAML frontmatter (`url`, optional `title`/`tags`/`relation`) -//! and an optional body, named `-.md` (the shared grammar → chronological by name), -//! under `/resources/links/`. (st2 keeps the inbox/archive/context under `resources/`, so -//! link records get their own `links/` subdir rather than colliding.) Append-only + rename/delete, -//! atomic writes. - -use std::fs; -use std::path::{Path, PathBuf}; - -use crate::message; - -/// A resource record. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Resource { - pub filename: String, - pub url: String, - pub title: Option, - pub tags: Vec, - /// e.g. `output`, `reference`, `blocked-by` — free-form. - pub relation: Option, - pub body: String, -} - -/// `/resources/links` — where an agent's resource records live. -pub fn links_dir(agent_dir: &Path) -> PathBuf { - agent_dir.join("resources").join("links") -} - -/// Basic URL shape: a `scheme:` prefix (http, https, file, pty, anything an agent invents). No host -/// validation — schemes are open-ended by design. -fn valid_url(url: &str) -> bool { - match url.split_once(':') { - Some((scheme, _)) => { - !scheme.is_empty() - && scheme.bytes().all(|b| { - b.is_ascii_lowercase() - || b.is_ascii_digit() - || b == b'+' - || b == b'-' - || b == b'.' - }) - } - None => false, - } -} - -/// Render a resource record's file contents (frontmatter + body). -pub fn render( - url: &str, - title: Option<&str>, - tags: &[String], - relation: Option<&str>, - body: &str, -) -> String { - let mut s = String::from("---\n"); - s.push_str(&format!("url: {url}\n")); - if let Some(t) = title { - s.push_str(&format!("title: {t}\n")); - } - if !tags.is_empty() { - s.push_str(&format!("tags: {}\n", tags.join(", "))); - } - if let Some(r) = relation { - s.push_str(&format!("relation: {r}\n")); - } - s.push_str("---\n"); - s.push_str(body); - if !body.is_empty() && !body.ends_with('\n') { - s.push('\n'); - } - s -} - -fn parse(filename: &str, contents: &str) -> Resource { - let mut r = Resource { - filename: filename.to_string(), - url: String::new(), - title: None, - tags: Vec::new(), - relation: None, - body: String::new(), - }; - let rest = contents - .strip_prefix("---\n") - .or_else(|| contents.strip_prefix("---\r\n")); - if let Some(rest) = rest - && let Some(end) = rest.find("\n---") - { - let front = &rest[..end]; - let after = &rest[end + 1..]; - let body = after.split_once('\n').map(|x| x.1).unwrap_or(""); - for line in front.lines() { - let Some((k, v)) = line.split_once(':') else { - continue; - }; - let v = v.trim(); - match k.trim() { - "url" => r.url = v.to_string(), - "title" => r.title = Some(v.to_string()), - "tags" => { - r.tags = v - .split(',') - .map(|t| t.trim().to_string()) - .filter(|t| !t.is_empty()) - .collect() - } - "relation" => r.relation = Some(v.to_string()), - _ => {} - } - } - r.body = body.to_string(); - } else { - r.body = contents.to_string(); - } - r -} - -/// Add a resource record for `url`, returning its filename. Creates `links/` if missing. -pub fn add( - links_dir: &Path, - url: &str, - title: Option<&str>, - tags: &[String], - relation: Option<&str>, - body: &str, -) -> anyhow::Result { - if !valid_url(url) { - anyhow::bail!("resource add: '{url}' is not a URL (needs a `scheme:` prefix)"); - } - fs::create_dir_all(links_dir)?; - let contents = render(url, title, tags, relation, body); - for _ in 0..8 { - let filename = message::new_filename(); - let path = links_dir.join(&filename); - if !path.exists() { - fs::write(&path, &contents)?; - return Ok(filename); - } - } - anyhow::bail!( - "could not allocate a unique resource filename in {}", - links_dir.display() - ) -} - -/// List an agent's resources, sorted by add time (filename). -pub fn list(links_dir: &Path) -> Vec { - let mut out = Vec::new(); - let Ok(rd) = fs::read_dir(links_dir) else { - return out; - }; - for e in rd.flatten() { - let name = e.file_name().to_string_lossy().into_owned(); - if !message::is_message_filename(&name) { - continue; - } - out.push(parse( - &name, - &fs::read_to_string(e.path()).unwrap_or_default(), - )); - } - out.sort_by(|a, b| a.filename.cmp(&b.filename)); - out -} - -/// Read one resource record. -pub fn read(links_dir: &Path, filename: &str) -> anyhow::Result { - let contents = fs::read_to_string(links_dir.join(filename)) - .map_err(|e| anyhow::anyhow!("reading resource {filename}: {e}"))?; - Ok(parse(filename, &contents)) -} - -/// Remove one resource record. -pub fn remove(links_dir: &Path, filename: &str) -> anyhow::Result<()> { - anyhow::ensure!( - message::is_message_filename(filename), - "invalid resource filename {filename:?}" - ); - fs::remove_file(links_dir.join(filename)) - .map_err(|e| anyhow::anyhow!("removing {filename}: {e}")) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn add_list_read_remove_cycle() { - let tmp = tempfile::tempdir().unwrap(); - let dir = links_dir(tmp.path()); - let tags = vec!["pr".to_string(), "st2".to_string()]; - let f = add( - &dir, - "https://github.com/x/y/pull/1", - Some("M2.6 PR"), - &tags, - Some("output"), - "the resource PR", - ) - .unwrap(); - - let all = list(&dir); - assert_eq!(all.len(), 1); - let r = &all[0]; - assert_eq!(r.url, "https://github.com/x/y/pull/1"); - assert_eq!(r.title.as_deref(), Some("M2.6 PR")); - assert_eq!(r.tags, ["pr", "st2"]); - assert_eq!(r.relation.as_deref(), Some("output")); - - assert_eq!(read(&dir, &f).unwrap().body.trim_end(), "the resource PR"); - remove(&dir, &f).unwrap(); - assert!(list(&dir).is_empty()); - } - - #[test] - fn remove_rejects_noncanonical_leaf_paths() { - let tmp = tempfile::tempdir().unwrap(); - let links = links_dir(tmp.path()); - fs::create_dir_all(&links).unwrap(); - fs::write(tmp.path().join("outside.md"), "unchanged").unwrap(); - assert!(remove(&links, "../outside.md").is_err()); - assert_eq!( - fs::read_to_string(tmp.path().join("outside.md")).unwrap(), - "unchanged" - ); - } - - #[test] - fn rejects_non_urls() { - let tmp = tempfile::tempdir().unwrap(); - let dir = links_dir(tmp.path()); - assert!(add(&dir, "not-a-url", None, &[], None, "").is_err()); - assert!(add(&dir, "file:///x", None, &[], None, "").is_ok()); - assert!(add(&dir, "pty://session/abc", None, &[], None, "").is_ok()); - } - - #[test] - fn missing_links_dir_lists_empty() { - let tmp = tempfile::tempdir().unwrap(); - assert!(list(&links_dir(tmp.path())).is_empty()); - } -} diff --git a/templates/bus.st2.md b/templates/bus.st2.md index fd456d6c..08dd6396 100644 --- a/templates/bus.st2.md +++ b/templates/bus.st2.md @@ -96,9 +96,11 @@ Working state (lossless-restart): - `st2 context write []` (reads new content from stdin) - `st2 context append [] --decision "" --why ""` -Resources: -- `st2 resource add [--title T] [--tag T,T] [--relation R]` -- `st2 resource ls []` · `st2 resource read [] ` · `st2 resource remove [] ` +Declared Resource bindings (`resource` nodes in your own declaration; writes republish it under CAS): +- `st2 resource ls [] [--json]` · `st2 resource read [] [--json]` +- `st2 resource add --uri --reason [--inactive-reason ]` +- `st2 resource remove ` · `st2 resource rename ` +- *writes also take `--agent ` (any declaration you may publish) and `--json`* Machine lifecycle hooks (explicit; `up` never installs or refreshes them): - `st2 hooks install [--allow-downgrade]` diff --git a/tests/agent_publish.rs b/tests/agent_publish.rs index e694e481..9f78dd0e 100644 --- a/tests/agent_publish.rs +++ b/tests/agent_publish.rs @@ -458,11 +458,39 @@ fn incomplete_apply_marker_blocks_declarations_but_not_the_state_plane() { String::from_utf8_lossy(&context.stderr) ); + let decision = st2() + .args([ + "context", + "append", + "host.worker", + "--decision", + "still writable", + "--why", + "the state plane is not fenced by an incomplete apply", + "--catalog", + catalog.to_str().unwrap(), + ]) + .output() + .unwrap(); + assert!( + decision.status.success(), + "{}", + String::from_utf8_lossy(&decision.stderr) + ); + + // `st2 resource` authors a declaration now, not a link record, so the marker must fence it + // exactly like `agent publish` above — it is no longer a state-plane write. let resource = st2() .args([ "resource", "add", + "work", + "--uri", "https://example.invalid/result", + "--reason", + "Blocked by the incomplete apply.", + "--agent", + "worker", "--as", "host.worker", "--catalog", @@ -471,8 +499,12 @@ fn incomplete_apply_marker_blocks_declarations_but_not_the_state_plane() { .output() .unwrap(); assert!( - resource.status.success(), - "{}", + !resource.status.success(), + "a binding write is a declaration write and must be fenced" + ); + assert!( + String::from_utf8_lossy(&resource.stderr).contains("apply is incomplete"), + "stderr: {}", String::from_utf8_lossy(&resource.stderr) ); diff --git a/tests/agent_resource.rs b/tests/agent_resource.rs new file mode 100644 index 00000000..19d148ec --- /dev/null +++ b/tests/agent_resource.rs @@ -0,0 +1,467 @@ +//! `st2 resource` operates on declared Agent Spec Resource bindings: `ls`/`read` project them, +//! and `add`/`remove`/`rename` mutate one binding through mediated CAS publication without the +//! caller rendering KDL. + +use std::fs; +use std::path::Path; +use std::process::Command; + +fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); +} + +/// A declaration with `bindings` spliced in verbatim, so tests control exact bytes. +fn declaration(identity: &str, managed_by: &str, bindings: &str) -> String { + format!( + "// unrelated comment\nagent {identity:?} {{\n host \"h\"\n meta {{ managed-by {managed_by:?} }}\n{bindings} command \"sleep 300\"\n}}\n" + ) +} + +fn run(root: &Path, args: &[&str]) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_st2")) + .args(["--catalog", root.to_str().unwrap()]) + .args(args) + .env_remove("ST_AGENT") + .output() + .unwrap() +} + +fn stdout(output: &std::process::Output) -> String { + String::from_utf8_lossy(&output.stdout).into_owned() +} + +fn stderr(output: &std::process::Output) -> String { + String::from_utf8_lossy(&output.stderr).into_owned() +} + +fn ok(root: &Path, args: &[&str]) -> String { + let output = run(root, args); + assert!( + output.status.success(), + "expected success from {args:?}\nstdout: {}\nstderr: {}", + stdout(&output), + stderr(&output) + ); + stdout(&output) +} + +fn spec(root: &Path) -> String { + fs::read_to_string(root.join("h/worker/agent.kdl")).unwrap() +} + +#[test] +fn ls_projects_declared_bindings_and_reports_none_without_pointing_at_another_store() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "catalog", ""), + ); + + let empty = ok(root, &["resource", "ls", "worker"]); + assert!( + empty.contains("0 resource"), + "empty roster should report zero bindings, got: {empty}" + ); + + write( + root, + "h/worker/agent.kdl", + &declaration( + "worker", + "catalog", + " resource \"notes\" reason=\"Durable notes.\" uri=\"agent-notes://h/worker\"\n\ + resource \"work\" reason=\"PR under preparation.\" uri=\"github-pr://github.com/o/r/pull/42\"\n", + ), + ); + + let listed = ok(root, &["resource", "ls", "worker"]); + assert!(listed.contains("2 resource"), "got: {listed}"); + assert!(listed.contains("notes"), "got: {listed}"); + assert!( + listed.contains("agent-notes://h/worker"), + "binding uri must appear: {listed}" + ); + assert!( + listed.contains("github-pr://github.com/o/r/pull/42"), + "binding uri must appear: {listed}" + ); +} + +#[test] +fn ls_json_and_read_expose_every_declared_field() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration( + "worker", + "catalog", + " resource \"work\" reason=\"PR under preparation.\" uri=\"github-pr://github.com/o/r/pull/42\" inactive-reason=\"Superseded by #43.\"\n", + ), + ); + + let listed = ok(root, &["resource", "ls", "worker", "--json"]); + let rows: serde_json::Value = serde_json::from_str(&listed).unwrap(); + let row = &rows.as_array().expect("json array")[0]; + assert_eq!(row["name"], "work"); + assert_eq!(row["uri"], "github-pr://github.com/o/r/pull/42"); + assert_eq!(row["reason"], "PR under preparation."); + // `inactive_reason`, not `inactiveReason`: this is the descriptor `st2 agents --json` + // already emits, and INVARIANTS.md pins that surface to preserve its field names. The + // snake_case is inconsistent with sibling roster fields but predates this change. + assert_eq!(row["inactive_reason"], "Superseded by #43."); + + let read = ok(root, &["resource", "read", "worker", "work"]); + assert!( + read.contains("github-pr://github.com/o/r/pull/42"), + "got: {read}" + ); + assert!(read.contains("PR under preparation."), "got: {read}"); + assert!(read.contains("Superseded by #43."), "got: {read}"); +} + +#[test] +fn add_publishes_one_binding_and_is_idempotent_on_identical_bytes() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "catalog", ""), + ); + + let added = ok( + root, + &[ + "resource", + "add", + "work", + "--agent", + "worker", + "--uri", + "github-pr://github.com/o/r/pull/42", + "--reason", + "PR under preparation.", + "--json", + ], + ); + let receipt: serde_json::Value = serde_json::from_str(&added).unwrap(); + assert_eq!(receipt["result"], "changed"); + + let after = spec(root); + assert!(after.contains("resource \"work\""), "got:\n{after}"); + assert!( + after.contains("// unrelated comment"), + "unrelated bytes must survive:\n{after}" + ); + assert!( + after.contains("command \"sleep 300\""), + "unrelated bytes must survive:\n{after}" + ); + + let repeated = ok( + root, + &[ + "resource", + "add", + "work", + "--agent", + "worker", + "--uri", + "github-pr://github.com/o/r/pull/42", + "--reason", + "PR under preparation.", + "--json", + ], + ); + let receipt: serde_json::Value = serde_json::from_str(&repeated).unwrap(); + assert_eq!( + receipt["result"], "unchanged", + "re-adding identical bytes must not republish" + ); + assert_eq!(spec(root), after, "unchanged add must not rewrite the file"); +} + +#[test] +fn add_preserves_the_uri_byte_for_byte_without_normalization() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "catalog", ""), + ); + + // Percent-encoding, mixed case, and a trailing slash must all survive verbatim. + let exact = "vendor+Thing://Authority/exact%20identity/"; + ok( + root, + &[ + "resource", + "add", + "odd", + "--agent", + "worker", + "--uri", + exact, + "--reason", + "Exactness probe.", + ], + ); + + assert!( + spec(root).contains(exact), + "uri must round-trip unchanged, got:\n{}", + spec(root) + ); + assert!( + ok(root, &["resource", "ls", "worker"]).contains(exact), + "uri must project unchanged" + ); +} + +#[test] +fn remove_is_idempotent_and_rename_refuses_absent_and_colliding_names() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration( + "worker", + "catalog", + " resource \"notes\" reason=\"Durable notes.\" uri=\"agent-notes://h/worker\"\n\ + resource \"work\" reason=\"PR under preparation.\" uri=\"github-pr://github.com/o/r/pull/42\"\n", + ), + ); + + let renamed = ok( + root, + &[ + "resource", + "rename", + "work", + "current-work", + "--agent", + "worker", + "--json", + ], + ); + let receipt: serde_json::Value = serde_json::from_str(&renamed).unwrap(); + assert_eq!(receipt["result"], "changed"); + assert!(spec(root).contains("resource \"current-work\"")); + assert!(!spec(root).contains("resource \"work\"")); + + let absent = run( + root, + &["resource", "rename", "work", "other", "--agent", "worker"], + ); + assert!( + !absent.status.success(), + "renaming an absent binding must fail" + ); + + let collision = run( + root, + &[ + "resource", + "rename", + "notes", + "current-work", + "--agent", + "worker", + ], + ); + assert!( + !collision.status.success(), + "renaming onto an existing name must fail; names are unique per agent" + ); + + let removed = ok( + root, + &[ + "resource", + "remove", + "current-work", + "--agent", + "worker", + "--json", + ], + ); + let receipt: serde_json::Value = serde_json::from_str(&removed).unwrap(); + assert_eq!(receipt["result"], "changed"); + + let again = ok( + root, + &[ + "resource", + "remove", + "current-work", + "--agent", + "worker", + "--json", + ], + ); + let receipt: serde_json::Value = serde_json::from_str(&again).unwrap(); + assert_eq!( + receipt["result"], "unchanged", + "removing an absent binding is an idempotent success" + ); +} + +#[test] +fn mutation_refuses_an_invalid_uri_an_empty_reason_and_a_nix_managed_declaration() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "catalog", ""), + ); + + // #345 widened the envelope: a binding may name a catalog-relative carrier path as well as + // an absolute URI, so `not-absolute` is now a *valid* relative carrier. What stays refused is + // a path that escapes the catalog. + for escaping in ["/etc/passwd", "../escape"] { + let refused = run( + root, + &[ + "resource", "add", "bad", "--agent", "worker", "--uri", escaping, "--reason", + "Probe.", + ], + ); + assert!( + !refused.status.success(), + "a catalog-relative uri that escapes the catalog must be refused: {escaping}" + ); + } + + // `declaration` is reserved by resync (#345) and may not be taken as a binding name. + let reserved = run( + root, + &[ + "resource", "add", "declaration", "--agent", "worker", "--uri", + "https://example.test/x", "--reason", "Probe.", + ], + ); + assert!( + !reserved.status.success(), + "the resync-reserved binding name must be refused" + ); + + let blank = run( + root, + &[ + "resource", + "add", + "bad", + "--agent", + "worker", + "--uri", + "https://example.test/x", + "--reason", + "", + ], + ); + assert!(!blank.status.success(), "an empty reason must be refused"); + assert!( + !spec(root).contains("resource \"bad\""), + "a refused mutation must not write" + ); + + write(root, "h/nixed/agent.kdl", &declaration("nixed", "nix", "")); + let nixed = run( + root, + &[ + "resource", + "add", + "work", + "--agent", + "nixed", + "--uri", + "https://example.test/x", + "--reason", + "Probe.", + ], + ); + assert!( + !nixed.status.success(), + "a Nix-owned declaration must refuse a runtime binding edit" + ); +} + +#[test] +fn the_retired_link_record_plane_is_gone() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "h/worker/agent.kdl", + &declaration("worker", "catalog", ""), + ); + + // `add` took a bare URL under the retired plane. It now requires a name plus --uri/--reason, + // so the old invocation must not silently succeed and write a link record. + let legacy = run(root, &["resource", "add", "https://example.test/output"]); + assert!( + !legacy.status.success(), + "the retired link-record invocation must not be accepted" + ); + assert!( + !root.join("h/worker/resources/links").exists(), + "no link-record store may be created" + ); +} + +/// A hand-authored binding may carry a trailing `//` comment explaining it. Removing the binding +/// removes that explanation with it, and updating one keeps the separator before it. +/// Regression: `remove` previously refused with `unsafe-source-shape`, and `add` glued the +/// rendered node onto the comment. +#[test] +fn a_binding_with_a_trailing_line_comment_is_removable_and_updatable() { + let commented = "// unrelated comment\nagent \"worker\" {\n host \"h\"\n meta { managed-by \"catalog\" }\n resource \"work\" uri=\"github-pr://github.com/o/r/pull/42\" reason=\"PR under preparation.\" // why it is here\n resource \"notes\" uri=\"agent-notes://h/worker\" reason=\"Durable notes.\"\n command \"sleep 300\"\n}\n"; + + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write(root, "h/worker/agent.kdl", commented); + + ok( + root, + &[ + "resource", "add", "work", + "--agent", "worker", + "--uri", "https://example.test/x", + "--reason", "Changed.", + ], + ); + let updated = spec(root); + assert!( + updated.contains(r#"reason="Changed." // why it is here"#), + "the blank before a trailing comment must survive an update:\n{updated}" + ); + + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write(root, "h/worker/agent.kdl", commented); + + ok(root, &["resource", "remove", "work", "--agent", "worker"]); + let after = spec(root); + assert!(!after.contains("resource \"work\""), "got:\n{after}"); + assert!( + !after.contains("why it is here"), + "the binding's own trailing comment goes with it:\n{after}" + ); + assert!( + after.contains("resource \"notes\""), + "the sibling binding must survive:\n{after}" + ); + assert!( + after.contains("// unrelated comment") && after.contains("command \"sleep 300\""), + "unrelated bytes must survive:\n{after}" + ); +} diff --git a/tests/catalog_apply.rs b/tests/catalog_apply.rs index b258c3a9..acd85570 100644 --- a/tests/catalog_apply.rs +++ b/tests/catalog_apply.rs @@ -3248,11 +3248,13 @@ fn marker_time_state_routes_existing_orphans_but_never_flat_falls_back_for_new_a "{}", String::from_utf8_lossy(&context.stderr) ); - let resource = st2() + let message = st2() .args([ - "resource", - "add", - "https://example.test/result", + "message", + "send", + "host.old", + "-m", + "state plane during apply", "--catalog", catalog_text, "--as", @@ -3263,9 +3265,9 @@ fn marker_time_state_routes_existing_orphans_but_never_flat_falls_back_for_new_a .output() .unwrap(); assert!( - resource.status.success(), + message.status.success(), "{}", - String::from_utf8_lossy(&resource.stderr) + String::from_utf8_lossy(&message.stderr) ); let status = st2() .args([ @@ -3290,7 +3292,7 @@ fn marker_time_state_routes_existing_orphans_but_never_flat_falls_back_for_new_a "working during apply" ); assert!( - old.join("resources/links") + old.join("resources/inbox") .read_dir() .unwrap() .next() @@ -3579,7 +3581,7 @@ fn marker_time_status_write_remains_bound_to_its_retained_agent_capability() { } #[test] -fn marker_time_context_and_resource_writes_reject_a_swapped_state_ancestor() { +fn marker_time_state_plane_writes_reject_a_swapped_state_ancestor() { let temp = tempfile::tempdir().unwrap(); let catalog = temp.path().join("catalog"); write_agent(&catalog, "old", false); @@ -3618,15 +3620,17 @@ fn marker_time_context_and_resource_writes_reject_a_swapped_state_ancestor() { "must-not-land", ), ( - "resource", + "decisions", vec![ - "resource", - "add", - "https://example.test/must-not-land", + "context", + "append", + "host.old", + "--decision", + "must-not-land", + "--why", + "must-not-land", "--catalog", catalog.to_str().unwrap(), - "--as", - "host.old", "--host", "host", ],