feat(config): delete_with_cascade for agents (#7175)

[Nillth]

📋 Stacked series — 3 of 8

Typed delete-with-cascade (#7175) + alias rename (#7468), sliced for review. This is the active slice (3 of 8) — ready for review; slices 4–8 trail as drafts for CI and merge strictly in order. Based directly on master, so this diff is just its own slice (no cumulative inflation).

Sequence: 1 #7785 ✅ · 2 #7830 ✅ · 3 #7837 ← this PR · 4 #7838 · 5 #7839 · 6 #7840 · 7 #7841 · 8 #7842

This slice: crates/zeroclaw-config/src/alias_refs.rs only — +265 / −12 (based directly on master; no cumulative inflation).

Summary

• Adds the agent arm of delete_with_cascade (feat(config): typed delete-with-cascade for aliased entries (providers, agents, channels) #7175) — the config-reference cascade for deleting agents.<alias> — on top of the provider arm landed in feat(config): delete_with_cascade for model providers (#7175) #7830.
• Refuses when a HARD agent config ref would dangle — an enabled heartbeat.agent naming the target, or a channel the agent is the sole enabled owner of (whatever collect_agent_refs marks Hard); otherwise scrubs the SOFT refs (clears heartbeat.agent when disabled + acp.default_agent; drops the alias from every agent's delegates, workspace.access map key, read_memory_from, and each peer group's members), removes the agents.<alias> entry, and verifies no dangling ref remains. DryRun mutates nothing.
• Scope: config references only. The agent's owned non-config state (memory/workspace/cron/acp/session) is cascaded by the surface in 4 of 8. Channels + TTS/transcription remain NotImplemented.
• New arm on an unused-by-callers function → zero runtime change.

Notable correctness

scrub_agent_refs mirrors collect_agent_refs's trim split exactly — trims the sites validate() trims (heartbeat, acp.default_agent, delegates) and leaves the three AgentAlias-keyed sites raw (workspace.access key, read_memory_from, peer_groups.agents). A self-referencing agent (its own delegates/read_memory_from naming itself) deletes cleanly — the scrub strips the self-ref before the entry is removed (test cascade_agent_self_reference_is_scrubbed).

Validation

cargo test -p zeroclaw-config → 907 passed; 0 failed (incl. agent-cascade tests) · cargo clippy --workspace --all-targets clean · cargo fmt --all --check clean. Full CI matrix runs on this PR.

Security & Privacy — all No (pure in-memory config mutation; synthetic-alias tests).

Compatibility — backward compatible; no surface change.

Rollback — git revert <sha>; no migration, no persisted-state effect.

Related #7175.

[singlerider]

I read this slice against current master: the new delete_agent / scrub_agent_refs arm, the collect_agent_refs it mirrors, plan_delete / find_all_references, and the agent-cascade tests. The cascade design is genuinely good and the trim-split discipline is exactly right. One blocker: a new doc comment misstates the HARD-ref invariant in a way that will mislead the scrub work in slices 4-8.

🔴 Blocking: the "only HARD agent ref is enabled heartbeat.agent" claim is wrong

The new module/function docs assert there is a single hard agent ref. From the diff:

• delete_agent: // The only HARD agent ref is an enabled heartbeat.agent (see collect_agent_refs).
• PR summary: "Refuses when the one HARD agent config ref would dangle (an enabled heartbeat.agent ...)."

collect_agent_refs recognizes a second HARD ref: channel ownership. When the target is the sole enabled owner of a channel, it pushes RefSite::hard("agents.<alias>.channels[i]", ScrubAction::Refuse, ...). plan_delete sets allowed = blockers.is_empty(), so that hard site refuses the delete too. Your own agent_delete_blocks_on_solely_owned_channel test pins exactly this.

The runtime behavior is correct: delete_agent calls plan_delete and returns Refused on any hard blocker, so a sole-owned-channel agent is refused before scrub_agent_refs runs, and nothing dangles. The problem is the comment, not the code. scrub_agent_refs is the mutating mirror of collect_agent_refs, and slices 4-8 extend that mirror; a maintainer reading "the only HARD agent ref is heartbeat" will reason that every non-heartbeat agent ref is soft and scrubbable, which is false for channel ownership. The comment documents the scrub/collect safety contract, so an inaccurate invariant here is a latent footgun. Reword to name both hard refs (enabled heartbeat.agent and sole-owned channels), or state that the hard set is whatever collect_agent_refs marks RefStrength::Hard and stop enumerating. Fix the same phrasing in the PR summary.

While here: there is no mutating-path test that the agent arm of delete_with_cascade refuses on a sole-owned channel. agent_delete_blocks_on_solely_owned_channel covers plan_delete only; cascade_agent_refuses_when_heartbeat_enabled covers the heartbeat blocker only. The channel-ownership refuse is transitively exercised through plan_delete, so this is a coverage gap rather than a defect, but a cascade_agent_refuses_when_solely_owned_channel case would lock the mutating arm's refuse path against future drift. Not required to land, but cheap and directly on point given the blocker above.

🟢 What looks good: the trim-split mirror is precise and well-tested

• scrub_agent_refs matches collect_agent_refs's trim discipline site for site: it trims exactly where validate() trims (heartbeat, acp.default_agent, delegates) and matches raw on the three AgentAlias-keyed sites (workspace.access key, read_memory_from, peer_groups.agents). cascade_agent_scrub_trim_split_mirrors_find pins that a padded read_memory_from ref is left untouched while padded trimmed sites scrub, which is the subtle correctness property here.
• The self-reference case is handled correctly and proven: the scrub loop iterates cfg.agents.values_mut() including the to-be-deleted agent, so a bot.delegates = ["bot"] self-ref is stripped before cfg.agents.remove, and the post-condition find_all_references confirms nothing dangles. cascade_agent_self_reference_is_scrubbed covers it.
• The post-condition re-walk after mutation is the right belt-and-suspenders: it converts any future scrub/collect drift into a loud PostCondition error instead of a silent dangling ref.
• The fail-closed ordering is correct: refuse on hard before any mutation, DryRun mutates nothing (cascade_agent_dry_run_mutates_nothing), NotFound short-circuits, and retain is index-shift-safe.
• Scoping the slice to config refs only and deferring owned non-config state (memory/workspace/cron/acp/session) to the surface in slice 4 is the right boundary, and the updated docs make the handoff explicit.

[Nillth]

Thanks — good catch, fixed in c02c8aa.

You're right the comment understated the hard set. delete_agent now names both HARD agent refs — an enabled heartbeat.agent and a channel the agent solely owns — and cites collect_agent_refs's RefStrength::Hard as the source of truth instead of enumerating, so the scrub mirror in slices 4–8 won't read channel ownership as soft. Fixed the same phrasing in the PR summary.

Took the suggested test too: cascade_agent_refuses_when_solely_owned_channel locks the mutating delete_with_cascade(Agent) refuse path on a sole-owned channel (prior coverage hit it through plan_delete only). Full zeroclaw-config suite green.

[singlerider]

Re-reviewed at c02c8aa against my CHANGES_REQUESTED on 72f3f0b. Both blocking items are resolved cleanly, and I confirmed the new test green locally (cargo test -p zeroclaw-config cascade_agent - 7/7, including the new case). Approving.

RESOLVED ✅ - HARD-ref doc comment now names both hard refs

The delete_agent gate comment previously claimed the only HARD agent ref was an enabled heartbeat.agent, which understated the set: collect_agent_refs also marks sole-owned channel ownership RefStrength::Hard. The new comment fixes this the right way - it states the gate refuses on any HARD ref plan_delete found, names both current hard refs (enabled heartbeat.agent and a channel the agent solely owns), and grounds the invariant in collect_agent_refs/RefStrength::Hard rather than re-enumerating in a way that drifts. That framing keeps slices 4-8 honest: a maintainer extending the scrub/collect mirror reads the source of truth, not a stale hand-maintained list. PR summary corrected to match.

RESOLVED ✅ - mutating arm now has its own sole-owned-channel refuse test

cascade_agent_refuses_when_solely_owned_channel locks the agent arm of delete_with_cascade against future drift: it asserts the refuse fires on agents.bot.channels[0] and that the agent plus its channel ownership survive intact (refuse-before-mutate). Previously this path was only transitively exercised through plan_delete; now the mutating arm pins it directly, alongside cascade_agent_refuses_when_heartbeat_enabled. Exactly the coverage I flagged as cheap and on point.

🟢 Everything from the prior pass still holds

The trim-split mirror, self-reference scrub, fail-closed ordering, post-condition re-walk, and DryRun-mutates-nothing properties are unchanged by this commit - it is doc + test only, no behavior change to the cascade logic. Clean slice. Nice turnaround.