fix(validate): retired declarations leave the root slot - #405
Merged
Conversation
Root counting in validate's root-count invariant and the graph's admitted topology counted every supervisor-less spec, so legacy `retired #true` tombstones held a host's root slot and the live dev3 catalog failed with "found 8". The folded desired-state model already maps both retirement spellings to AgentDesiredState::Retired; both consumers now share one predicate (supervisor_chain::is_counted_root) so a retired declaration never counts, a suspended root still does, and zero counted roots remains an error. The graph declarations view folds legacy `retired #true` to desiredState "retired"; an absent lifecycle stays null (lowers to running). Also repairs six tests/validate.rs expectations to the #399 semantics (that target is not CI-gated, so the drift shipped unnoticed): a dangling supervisor is now an error, driver/deliver conflicts refuse at parse time, native drivers require delivery-readiness, and fixtures satisfy the one-root invariant. agent-identity: dev3.direct.omp.wk3aavmh agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@1defc60
schickling-assistant
marked this pull request as ready for review
August 31, 2026 15:58
Contributor
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec355e32fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review finding on #405 (Codex P1): with retired declarations excluded from root counting, one active root plus a retired root still supervising an active worker validated clean — root-count saw exactly one counted root while the worker's chain terminated at the tombstone, and the graph published complete: true with two disconnected trees. An active agent's chain must now terminate at a counted root (retired-root error); retired chains under a retired root stay legal. Live dev3 catalog verified: zero new errors beyond the pre-existing delivery-readiness ones (#406). agent-identity: dev3.direct.omp.wk3aavmh agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@1defc60
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
st2 validateandst2 catalog graphon currentmaincount declarations that use legacyretired #trueas active roots. On the live dev3 catalog (intended root:cos) the new one-root invariant reportsroot-count: host dev3 must declare exactly one root, found 8—cosplus seven root-shaped legacy-retired tombstones — and graph v2 exposes those tombstones withdesiredState: nullindeclarations[]. The deployed pre-#399 st2 had no root-count check at all, which is why the same catalog validated clean.Root cause: #399 introduced the invariant in two places —
validate.rs(root-count) andcatalog_graph.rs::admitted_topology— and both count every spec withsupervisor: none, ignoring the folded desired state. The spec layer already folds both spellings (retired #true,desired-state "retired") intoAgentDesiredState::Retired; only the invariant's predicate and the declaration view missed it.Goal
Retired declarations never participate in the one-root invariant; the graph declarations view shows the fold; genuine topology faults still error.
Decisions
supervisor_chain::is_counted_root, so validate and admitted topology cannot drift apart again.found 0. No live host has this shape today.retired #true→"retired"; an absent lifecycle staysnull, which keeps one meaning ("nothing declared" → running).PartialAgentremains a declared-fields view.tests/validate.rsrepairs ride in this PR (q3): Make catalog delivery facts authoritative #399 rewrote validate semantics without updating that target, and the target is not in any CI gate, so main ships with it red (verified: 6 failures on unmodified67b18b7via keyed-stash baseline). Repairs pin the Make catalog delivery facts authoritative #399 semantics; nosrc/changes beyond the fix.Verification
retired #trueroots →found 4,complete: false, declarationsdesiredState: null(unfixed binary).found 1,cosgetsrootId, depth 0); suspended-only root green; tombstone-only and headless hosts →found 0; two running roots →found 2(regression intact).dev3.cos); 622 declaration entries now publish folded"retired".cargo test --test validate --test catalog_graph: 50/50 + 6/6 green after fix + repairs (was 44-pass/6-fail on main). All CI-gated flake targets green (--workspace --lib --bins --test discovery/codex_hooks/hooks/run/driver_expansion).docs/vrs/spec.mdamended (counting fold + declarations fold); experiment record atdocs/vrs/.experiments/2026-08-31-root-count-retirement-fold.md;axe vrs check --profile strictclean for all touched artifacts.Complexity
One new predicate (
is_counted_root) shared by both consumers — a net simplification. No new modules or dependencies.Concerns
rootId= themselves, depth 0) instead of null: with the invariant satisfied,admitted_topologyadmits tombstones too. This is an honest observation of declared edges (they are not in the active tree); flagging in case a consumer prefers null for retired rows.found 0— intentional (headless org is a fault), per the decision above.Friction & bottlenecks
tests/validate.rs(andcatalog_graph) are not CI-gated, so Make catalog delivery facts authoritative #399 shipped with the target red. Logged here rather than silently absorbed; see Follow-ups.cargo fmt --checkdisagrees with the committed style on 236 pre-existing locations (rustfmt version drift); fmt is explicitly not a gate (flake.nix). New lines follow committed style.Follow-ups
delivery-readiness-missingerrors from Make catalog delivery facts authoritative #399's delivery gates (cos, janitors, leads) — separate defect from this one, tracked in delivery-readiness-missing errors on live catalog: #399 gate flags 18 running agents #406.--test validate --test catalog_graphto the flake'scargoTestFlagsso this drift class cannot recur (needs a hermetic-sandbox verification run first).References
Closes #402. RCA of the #399 introduction in
aa8eb48.Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile