fix(discovery): an agent's own subtree is state, not more catalog

[schickling-assistant]

Note

CLOSED, not merged. This PR does not fix the row it was filed for, and the guard it proposes
has a false-positive class that can silently empty a catalog. The closing comment carries the
measurements, including a correction to the claim below about a601331.

Correction: the section "Scope changed under this branch" originally stated that a601331
already fixed FB-397's reported case. That was an inference and it is wrong — measured, FB-397
still reproduces on current main, and this branch does not fix it either. See the closing
comment.

What

Discovery descended into every subdirectory of an agent's declaration directory and treated any
*.{toml,json,kdl} it found there as a spec candidate. Agents routinely write JSON evidence under
their own directory — launch receipts, captured output, scratch — so one cause produced two
failures.

1. A malformed evidence file fails the whole catalog. One agent's [{"run":1},{"run":2}] makes
   st2 validate report an error and st2 up --materialize-only refuse for every unrelated agent
   on the host.
2. A well-formed one is worse, because it is silent. A launch receipt carrying an argv field
   lowers into a real agent with a real task while the catalog reports 0 errors, 0 warnings.

A declaration directory holds exactly one agent, so nothing nested below it is another declaration.
This stops the walk descending once the current directory anchors a declaration. The existing
resources/archive/inbox exclusions were an incomplete enumeration of that same rule, which
is_catalog_path's own doc comment and src/watch.rs already describe as the intent.

Feedback row: FB-397 (friction, 2 occurrences) —
https://app.notion.com/p/st2-validate-recursively-parses-every-json-under-an-agent-dir-as-catalog-config-so-agents-storing--3ace3d41f4a3817db354eac8d8b1529d

Changed lines: 105 (105 insertions, 0 deletions) across 2 files —
crates/agent-spec/src/discovery.rs (11), crates/agent-spec/tests/discovery.rs (94).

Scope changed under this branch — read this before reviewing the fixture

This branch was written against 19485cf. Since then, a601331 (#351) added
is_canonical_bundle_descendant, which skips any path more than three components below agents/.
That covers the canonical agents/<host>/<identity> bundle positionally.
It is the shape FB-397 was originally filed against, so the row's literal reported case is already
fixed on current main, not by this PR. That was wrong. FB-397's only recorded path is
.state/<identity>/axe/agent-launch-receipts/*.json, whose first component is not agents, so
is_canonical_bundle_descendant cannot cover it. Measured: the reported case still reproduces on
main and is not fixed by this branch. See the closing comment. The original fixture here stopped discriminating, and the earlier
revision of this PR body claimed evidence that no longer reproduced. Both have been re-measured
against 4c4e7f8.

What a601331 does not cover is a declaration directory placed anywhere other than a canonical
bundle, which discovery still supports. That is the case this PR now closes, and the case the test
now pins. The two rules are complementary, not competing: is_canonical_bundle_descendant is
positional, this one is semantic (this directory anchors a declaration, so its subtree is that
agent's state).

Not addressed here, and left deliberately: a spec-shaped file sitting directly in a declaration
directory (agents/h/real.worker/receipt.json) still lowers as a phantom on both main and this
branch. Closing that requires guarding files, not just directory descent — a behaviour change beyond
this PR's scope. Flagged rather than folded in.

The catalog root is excluded from the rule

Worth reviewing closely, because the naive form of this change is a catalog-wiping regression.

is_declaration_parent is asked about every directory the walk visits, including the catalog root
itself. A root catalog.kdl may declare an agent inline beside a profile — that is a live surface,
and root_catalog_envelope_allows_a_profile_beside_an_agent already covers it. Without an explicit
exclusion, that one file makes the root "hold a declaration", the walk refuses to descend into
agents/, and the entire catalog disappears from discovery:

# guard applied at the root too — the bug
PROBE-ROOT identities=["root-agent"]
# clean main, and this branch as submitted
PROBE-ROOT identities=["real.worker", "root-agent"]

So the guard is dir != root && is_declaration_parent(root, dir), and
a_root_envelope_declaration_does_not_hide_the_agents_subtree pins it. That test fails with the
exclusion removed, naming exactly the collapse above.

Checked at real scale as well, read-only against this host's live 664-agent catalog:

$ <branch build> st2 ls --catalog <live catalog>   # 664 agents
$ st2 ls --catalog <live catalog>                  # 0.1.0+4c4e7f8, clean main — 664 agents
$ diff <(branch) <(main)   # differs only in each binary's own argv[0] path in the ding sidecar

Fixture

Three agents. One is a canonical bundle that has written evidence under its own directory; one is a
non-canonical declaration directory (team/) that has done the same.

agents/h/evidence.worker/agent.kdl
agents/h/evidence.worker/axe/agent-launch-receipts/run-1.json   {"schema":"axe.agent-launch-receipt.v3","argv":["axe","agent","launch"]}
agents/h/evidence.worker/axe/agent-launch-receipts/run-2.json   [{"run":1},{"run":2}]
agents/h/other.worker/agent.kdl
team/agent.kdl
team/evidence/agent-launch-receipts/run-1.json                  {"identity":"phantom.receipt","host":"h","argv":["true"]}
team/evidence/agent-launch-receipts/run-2.json                  [{"run":1},{"run":2}]

Before — clean main @ 4c4e7f8

$ st2 validate --catalog <catalog>
ERROR  team/evidence/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0
─ 1 error, 0 warnings across 4 agents
rc=1

$ st2 ls --catalog <catalog>
h.evidence.worker  [service] (1 task)
    <catalog>/agents/h/evidence.worker/agent.kdl
      - pty agent: argv ["true"]
h.other.worker  [service] (1 task)
    <catalog>/agents/h/other.worker/agent.kdl
      - pty agent: argv ["true"]
h.team.worker  [service] (1 task)
    <catalog>/team/agent.kdl
      - pty agent: argv ["true"]
h.phantom.receipt  [service] (1 task)
    <catalog>/team/evidence/agent-launch-receipts/run-1.json
      - pty agent: argv ["true"]
error: <catalog>/team/evidence/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0

$ st2 up --catalog <catalog> --host h --materialize-only
error: <catalog>/team/evidence/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0
materialized 0 operations for host 'h'; 1 error
Error: materialization failed
rc=1

h.phantom.receipt is the silent failure: a receipt file listed as a real agent with a real task,
and 4 agents where the catalog declares three. The axe/… artifacts under the canonical bundle
are already ignored here — that is a601331 working.

After — this branch

$ st2 validate --catalog <catalog>
─ 0 errors, 0 warnings across 3 agents
rc=0

$ st2 ls --catalog <catalog>
h.evidence.worker  [service] (1 task)
    <catalog>/agents/h/evidence.worker/agent.kdl
      - pty agent: argv ["true"]
h.other.worker  [service] (1 task)
    <catalog>/agents/h/other.worker/agent.kdl
      - pty agent: argv ["true"]
h.team.worker  [service] (1 task)
    <catalog>/team/agent.kdl
      - pty agent: argv ["true"]

$ st2 up --catalog <catalog> --host h --materialize-only
materialized 0 operations for host 'h'; 0 errors

(0 operations in both cases because no fixture agent declares a render block. The change being
demonstrated is 1 error / Error: materialization failed becoming 0 errors, and the phantom
disappearing from st2 ls.)

Both binaries were built from this worktree in the same target directory, and each build was
confirmed to recompile agent-spec and st2 rather than reuse a cached rlib.

Tests

One regression test, asserting on found.specs and not only on found.errors — the phantom-agent
case produces zero errors, so an error-only assertion would pass vacuously. It runs under both
discover and discover_strict, and covers the non-canonical declaration directory (the
discriminating case) plus the canonical bundle (a regression guard if the positional rule ever
moves).

Confirmed failure-capable against current main: with the test in place and discovery.rs copied
from origin/main, it fails and names the phantom.

test an_agents_own_subtree_is_state_and_never_another_declaration ... FAILED

assertion `left == right` failed: an evidence artifact became a declaration
  left: ["evidence.worker", "other.worker", "team.worker", "phantom.receipt"]
 right: ["evidence.worker", "other.worker", "team.worker"]

test result: FAILED. 62 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

With the fix:

crates/agent-spec --test discovery   test result: ok. 63 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

cargo fmt --check reports no diff inside either hunk this PR adds. (The crate does carry
pre-existing formatting drift from a601331 at src/discovery.rs:10, src/discovery.rs:292,
tests/discovery.rs:{13,33,1298}, src/lib.rs, src/profile*.rs and examples/wasm_bench.rs;
none of it is touched here, and nix flake check does not gate cargo fmt.)

CI attribution

main is green in CI, so any suite failure seen locally on the development host is
environment-specific to that host, not an upstream regression. Four such pre-existing failures were
confirmed on clean main — three disclosed in the first revision of this PR, plus a fourth that
neither producer disclosed at the time:

tests/agent_publish.rs      incomplete_apply_marker_blocks_declarations_but_not_the_state_plane
tests/materialize.rs        up_materialize_only_writes_the_overlay_without_needing_pty
tests/targeted_reconcile.rs targeted_once_real_pty_preserves_sibling_generation_across_selected_lifecycle
src/run.rs                  run::tests::persistent_advisory_warnings_surface_once_not_per_pass

These were confirmed on clean main at 19485cf0. This branch is now rebased onto 4c4e7f8; the
authoritative check for whether any of them still fail is this branch's own CI run, not that earlier
local measurement.

Posted on behalf of @schickling

field	value
agent_identity	dev3.dotfiles.fb-batch1.st2.worker
session	dev3.17884542
agent_persona	worker
agent_supervisor	dev3.dotfiles-lead
agent_tool	Claude Code
agent_tool_version	2.1.251
agent_runtime	Claude Code 2.1.251
tooling_profile	dotfiles@6048b77

[schickling-assistant]

Closing unmerged — superseded in intent, and it does not fix the row it was filed for

Decision by dev3.dotfiles-lead (option (a)). Recording the measurements here so the next person to
reach for this guard does not re-derive them.

1. Correction to this PR's own body: the reported case is fixed by neither a601331 nor this PR

The body above claims a601331 (#351) already fixed the case FB-397 was filed against. That claim
was an inference and it is wrong. I measured it and it does not hold.

FB-397's only recorded evidence path is its occurrence 2:

/tmp/st2e-2136495/.state/orchestrator/axe/agent-launch-receipts

st2 eval mints its catalog at exactly temp_dir()/st2e-<pid> (src/eval_run.rs:800), so that path
is catalog-relative .state/<identity>/axe/agent-launch-receipts/*.json. Its first component is
not agents. is_canonical_bundle_descendant requires the first component to be agents plus at
least four more, so it cannot cover this path regardless of depth. (And if the recorded path were
somehow not catalog-relative, then the row carries no usable path at all and "fixed upstream" would
be unverifiable rather than merely false. Either way the claim does not stand.)

Reproduced against that shape. Minimal fixture — one real agent, one malformed JSON, no identity
or host field anywhere, so nothing here depends on what a receipt looks like:

<catalog>/agents/evalhost/orchestrator/agent.kdl
<catalog>/.state/orchestrator/axe/agent-launch-receipts/run-2.json     [{"run":1},{"run":2}]

$ st2 validate --catalog <catalog>          # 0.1.0+08b67b3 — a commit on main containing a601331
ERROR  .state/orchestrator/axe/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0
─ 1 error, 0 warnings across 1 agent
rc=1

$ <this branch, df4cec7> st2 validate --catalog <catalog>
ERROR  .state/orchestrator/axe/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0
─ 1 error, 0 warnings across 1 agent
rc=1

With the spec-shaped receipt added back, both binaries also lower the same phantom agent:

$ st2 ls --catalog <catalog>                # identical on main and on this branch
evalhost.phantom.receipt  [service] (1 task)
    <catalog>/.state/orchestrator/axe/agent-launch-receipts/run-1.json
      - pty agent: argv ["true"]
evalhost.orchestrator  [service] (1 task)
    <catalog>/agents/evalhost/orchestrator/agent.kdl
      - pty agent: argv ["true"]
error: <catalog>/.state/orchestrator/axe/agent-launch-receipts/run-2.json: JSON parse error: invalid type: sequence, expected struct RawSpec at line 1 column 0

Byte-identical output from both binaries. FB-397 still reproduces on current main, and this PR
does not fix it. That is the first reason to close: the change does not buy the row it was filed
for.

2. The second reason: the guard's false positives can silently empty a catalog

This is the reason the change should not land even for the cases it does cover.

The guard prunes descent from a property of the current directory. "This directory is an agent's
home" is indistinguishable from "this directory holds a spec file and also hosts other agents'
bundles" without descending into it. So every predicate expressible here has false positives, and
the costs are not symmetric: a false negative is one phantom agent, which is main's status quo,
while a false positive makes an entire subtree vanish from discovery while reporting 0 errors.

Measured, each with a verified Compiling agent-spec (a shared CARGO_TARGET_DIR across worktrees
will otherwise reuse a sibling's rlib and quietly invalidate the comparison):

fixture	clean main	this branch
root catalog.kdl declaring an agent, beside agents/h/real.worker/	real.worker, root-agent	root-agent — subtree lost (fixed by dir != root)
agents/legacy.kdl beside agents/h/real.worker/	real.worker, legacy.worker	legacy.worker — subtree lost
agents/h/legacy.kdl beside agents/h/real.worker/	legacy.worker, real.worker	legacy.worker — subtree lost

The only formulation with no false positives is positional — skip only at exactly
<root>/agents/<host>/<identity> — but for spec collection that is already what
is_canonical_bundle_descendant does. So the safe version adds nothing, and the version that adds
something is not safe. That is the whole argument, and it is why this needs a design decision rather
than another predicate.

A read-only check against a live 664-agent catalog returned an identical roster for this branch and
clean main, so the non-canonical shape this PR defends against does not occur there either.

3. Three distinct classes — please keep them apart

Anyone reopening this should not collapse these:

1. Canonical bundle descendants (agents/<host>/<identity>/…, more than three components below
   agents/) — fixed on main by a601331, positionally.
2. A spec-shaped file in a directory that is neither a canonical bundle descendant nor a
   declaration directory — this is FB-397's actual recorded case (.state/<identity>/…). Open
   on main, and not fixed by this PR.
3. A spec-shaped file sitting directly in a declaration directory
   (agents/h/real.worker/receipt.json) — open on main and on this branch, because the guard
   proposed here is on ft.is_dir() and never inspects files. Silent: rc=0, no error, phantom
   listed beside the real agent. Filed separately; closing it is a behaviour change, not a bug fix.

Disposition

Closed, not merged. Nothing on main changes. The branch is left in place; FB-397 stays open as
needing a design decision rather than a predicate, and class 3 is filed as its own row.

Posted on behalf of @schickling

field	value
agent_identity	dev3.dotfiles.fb-batch1.st2.worker
session	dev3.17884542
agent_persona	worker
agent_supervisor	dev3.dotfiles-lead
agent_tool	Claude Code
agent_tool_version	2.1.251
agent_runtime	Claude Code 2.1.251
tooling_profile	dotfiles@6048b77