fix(subscriptions): registering a subscription or deleting the instance key adopts the credential-less fleet (#2572)

[trinity-ability]

Summary

On an instance with no Anthropic API key, registering a subscription — the one action the product tells the operator to take — assigned nobody. Every pre-existing agent stayed in api_key auth mode with nothing behind it, and the fleet kept reading "Not logged in".

The hole sits between two things that were each individually correct. #442 made "the platform has no usable credential" a precondition of subscription auth, and SUB-003 only ever moves an agent that already has a subscription (its precondition 2 is literally where the gap was written down). POST /api/subscriptions assigned none. So nothing in the system closed the distance between "an operator just supplied a credential" and "the agents that have none can use it".

Three triggers now cover it, with no periodic sweep:

1. Subscription registration (new) — POST /api/subscriptions, which the Settings form and the MCP register_subscription tool both reach. A re-register (upsert) re-runs it; the sweep is idempotent.
2. Instance-key deletion (new) — the canonical migration off a metered key is register, then delete the key, and in that order trigger 1 correctly adopts nobody (the key still resolves, so the sweep short-circuits) and the deletion strands the fleet with no trigger left. Both clear routes are hooked — the dedicated DELETE /api/settings/api-keys/anthropic and the generic DELETE /api/settings/{key} — because db.delete_setting carries no delete-side twin of ent#435's write sink guard, so the second route is reachable rather than theoretical.
3. Agent creation — already shipped by Auto-assign subscription to new agents (round-robin) #74, unchanged here, and now pinned by a regression test so a later edit cannot quietly drop it.

The predicate is no usable credential, not a preference between two working ones — five conditions, all of which must hold: no key resolvable at instance level via settings_service.get_anthropic_api_key() (env fallback included, never the DB-only has_secret_setting()); subscription_id IS NULL; use_platform_api_key true; a trinity.agent-runtime label that is present AND Claude; not ephemeral. An unreadable Docker adopts nobody.

Phase split. Phase A (decide + persist) is awaited inside the triggering request, so GET /api/subscriptions and the auth badge are correct on the panel's immediate refetch. Phase B (container apply) is backgrounded, per-agent, under the #799 switch lock, and re-verifies the assignment before restarting — SUB-003 may legitimately have moved the agent in between. Credential-less → subscription is an auth-mode change, so it recreates rather than hot-reloading. Target selection reuses select_subscription_for_new_agent — the same #2409 headroom ranker all three triggers already share.

Fixes #2572

What changed

Backend (5 files)

• services/subscription_service.py — adopt_for_credentialless_agents(), the sweep itself: one fleet-wide DB read, one batch Docker read, per-agent select/assign under the bug: SUB-003 auto-switch has no per-agent lock — concurrent 429s race the restart #799 lock, every blocking call in asyncio.to_thread. Module-level _SWEEP_LOCK; a concurrent sweep skips, never queues. Keyword-only trigger parameter feeds details.trigger — the sweep itself is trigger-agnostic and is not forked per trigger.
• routers/subscriptions.py — Trigger A1 on POST /api/subscriptions, swallow-everything so it can never fail a registration whose credential is already stored. The body parameter is renamed request → payload because the handler now also takes the injected Request; two things named request in one handler is how a log line inside an except raises AttributeError and returns a 500 with the credential already stored.
• routers/settings.py — Trigger A2 on both clear paths, each gated on the route's existing deleted truthiness. The hook sits on the routes, not on clear_secret_setting — that leaf also serves github_pat and the Slack keys, and hooking it would run a Claude-subscription sweep on unrelated credential deletions.
• services/docker_service.py — agent_container_runtime_labels(), a label-strict sibling of agent_container_runtimes(). Same tri-state, same keying, same sparse=True cost bound; the one difference is the whole reason it exists. agent_container_runtimes() resolves a missing label to "claude-code" — right for a UI affordance, wrong for deciding whether to hand an agent a Claude subscription, and trinity-system carries no runtime label at all. Absence is read as no evidence, never as Claude.
• services/docker_utils.py — the async form, following the existing function-local-import rule so a suite that stubs services.docker_service still resolves through sys.modules.

Audit (the one deliberate scope expansion, for AC 5)

AC 5 asks that adoption be recorded "alongside manual assignment" — which is only literally true if the manual move is recorded too. #2421 (still open) found this router carried no platform_audit_service.log(...) call at all, so an auditor asking "how did agent X get onto this subscription?" got a false negative for every manual move. This PR adds subscription_assign / subscription_clear alongside the new subscription_auto_adopt / subscription_auto_adopt_sweep. #2421's remaining actions (register / delete / auto-switch settings) stay unclaimed and cannot collide. No token enters any audit sink — subscription id and name only (Invariant #12).

Tests (2 files) — tests/unit/test_2572_credentialless_adoption.py (new, 38 tests) and tests/unit/test_subscription_reassign_hotreload.py (updated for the payload rename).

Docs (8 files) — requirements/security.md (§20.3 workflow, §20.3a rewritten to own both triggers, §20.5a's "Cut" sentence amended rather than silently contradicted, SEC-001 event-action list); feature-flows/subscription-management.md (primary owner); feature-flows/subscription-auto-switch.md (one-line cross-reference, so SUB-003's failure-scoped overview is not contradicted); feature-flows/platform-settings.md; feature-flows/workspace-model-choice.md; feature-flows.md (Recent Updates row); architecture/backend.md; architecture/agent-lifecycle.md.

No schema change — dual-track migration Rule #9 N/A. Mechanically confirmed: the changed-path set hits neither src/backend/db/{schema,migrations,tables}.py nor src/backend/migrations/versions/. No new table, no new column, no Alembic revision, no migrations.py entry.

No frontend change. Zero files under src/frontend/.

Acceptance criteria

• On an instance with no API key, registering the first subscription results in credential-less agents using it — no per-agent clicks.
  Trigger A1 on POST /api/subscriptions, Phase A awaited. Evidence: test_2572_credentialless_adoption.py drives the service and the router boundary with a stubbed Docker read and asserts the assignment is persisted before the POST returns. Mutation-tested — swapping the predicate to a DB-only check fails 2 tests.
• Agents with a working API key keep API-key auth and are not moved.
  Predicate condition 1 short-circuits the whole sweep on any instance where get_anthropic_api_key() resolves anything — encrypted row → legacy row → ANTHROPIC_API_KEY env fallback. This is the structural guarantee, not a per-agent filter. Proven on the isolated stack by a negative control: with ANTHROPIC_API_KEY still exported, deleting the settings row returned {"deleted":true,"fallback_configured":true} and produced 0 adoption rows.
• Agents already assigned to another subscription are not moved.
  Condition 2 (subscription_id IS NULL), re-checked inside the per-agent lock so a concurrent SUB-003 switch cannot be overwritten. Covered in the unit suite.
• The adoption is visible: the agent header badge reflects the subscription, and GET /api/subscriptions reports the agents on it.
  Reuse only — no new surface. GET /api/subscriptions builds agents / agent_count from agent_ownership.subscription_id, and the badge reads GET /api/subscriptions/agents/{name}/auth → derive_auth_mode(), both purely DB. Because Phase A is awaited, both are correct on the panel's very next refetch.
• Adoption is recorded in the audit trail alongside manual assignment (see bug: subscription register/delete/assign and the auto-switch settings are not audit-logged — a wiped subscription leaves no trace (SEC-001 gap) #2421).
  One subscription_auto_adopt row per adopted agent, one subscription_auto_adopt_sweep summary row, plus the previously-missing manual subscription_assign / subscription_clear. See the audit note above.

Decisions made by the issue owner

These were put to the issue owner during planning and decided; they are recorded here so a reviewer reads them as settled, not as proposals.

• Trigger set = register + create + key-deletion. No periodic sweep. The original decision was "register + create"; review surfaced that the canonical register-then-delete ordering walks around both, and the owner added the key-deletion hook. services/subscription_recovery_service.py is deliberately not extended — that would be the periodic sweep that was declined.
• "No Auth" agents (use_platform_api_key = false) are excluded. The operator explicitly opted them out of platform credentials; the agent-side arm_subscription_auth_guard() (bug: .env-resident ANTHROPIC_API_KEY shadows subscription auth in the per-spawn execution env (#1999), defeating SUB-003 auto-switch #2114) would force-unset a key the backend structurally cannot see.
• No toast, no inline count, no new response field on the Subscriptions panel. The panel already refetches and shows the agents on the subscription.
• Badge copy is untouched. derive_auth_mode still reads use_platform_api_key rather than key existence, so an agent the sweep skips still renders "API Key" on a keyless instance. That is bug: agent chat shows Claude Code's raw "run /login" error when no model credential is configured #2570's to fix, not this one's.
• Key deletion restarts adopted agents now. The removed key is not revoked at Anthropic and stays live in each running container until recreate, so deferring would leave agents running on a key the operator just withdrew. Blast radius, stated plainly: the manual assign route already does exactly this per agent; the sweep does it for the fleet, as a side effect of the key deletion. In-flight turns on affected agents are interrupted once. trinity-system is adopted in the DB but never restarted by the sweep (bug: trinity-system never adopts a rebuilt base image — ensure_deployed short-circuits on 'already running' #1816), and ephemeral ghosts are excluded from the sweep entirely (a recreate destroys their workspace).

Verification

/verify-local --skip-agent on 68a2bc55 → status = pass (project trinity-verify-0b7f263a):

• unit 15,375 passed / 31 skipped / 0 failed
• build + import-smoke OK; boot + health OK
• integration 70 passed / 13 skipped / 2 registry-deselected / 0 failed

Affected neighbourhood, re-run on the final tree:

tests/unit/test_2572_credentialless_adoption.py                     → 38 passed
9-file subscription set, --randomly-seed=12345                      → 167 passed
auth/invariant guard superset (1310 wiring + 1310 consolidation +
  293 admin-gate + 186 enumeration + models-centralized + ent435)    → 120 passed

Isolated-stack end-to-end proof — PARTIAL, stated honestly. Plan §13 lists 11 steps. 4 ran, plus 4 extra negative controls; 7 did not run, for two structural reasons rather than for convenience:

1. An agent created from the isolated backend would land on the shared global trinity-agent-network owned by the live dev stack. verify-local exports TRINITY_AGENT_NETWORK, but no backend code reads it — all three agent-create sites hard-code the global name. Creating an agent there would have reached into live infrastructure.
2. The isolated backend inherits the worktree's real ANTHROPIC_API_KEY, so predicate condition 1 short-circuits by design — which is correct behaviour, and also means the positive adoption path cannot be driven from that stack without removing a real credential from the environment.

What did run, and what it showed:

Probe	Result
POST /api/subscriptions with a fabricated sk-ant-oat01-…	200 — registration never probes Anthropic, so a syntactic token is enough to exercise the trigger
DELETE /api/settings/api-keys/anthropic (dedicated route, key present)	{"success":true,"deleted":true,"fallback_configured":true} — row gone, settings_change audit row written, 0 adoption rows (env-fallback negative holds)
DELETE /api/settings/anthropic_api_key_encrypted (generic route, key present)	{"success":true,"deleted":true} — same, 0 sweep rows
No-op DELETE on both routes	{"deleted":false} — no rows of any kind; the deleted gate holds
Unrelated key PUT then DELETE	{"deleted":true} with a settings_change row only — no adoption, no sweep
Unauthenticated probe, both A2 routes + POST /api/subscriptions	401 on all three — the gate is live

The positive adoption path, Phase B restart, and the "system agent is never restarted" guarantee are proven by the 38 unit tests, which execute the real service and router paths against a stubbed Docker read. Each guard was mutation-tested rather than merely asserted:

• swapping the predicate to the DB-only check → 2 tests fail
• disabling the trinity-system exclusion → 1 test fails
• removing the lock skip → T7b hangs (it uses asyncio.wait_for, so absence of the skip manifests as a hang rather than a failure — see follow-ups)

Known conditions / pre-existing reds

All three reproduce on base (origin/dev @ 7a40408b4) and none is introduced here.

1. test_2703_skill_assign_delivery.py — 5 failures in the full unit island at --randomly-seed=12345 (branch: 15,336 passed / 35 skipped / 5 failed). Reproduced on a pristine _ref-dev checkout at 7a40408b4: 5 failed / 15,298 passed. Minimal reproducer is 2 files — that file plus test_skill_service_user_agent.py. Order-dependent state leakage in bug(skills): a library skill assigned to an agent is missing from the Workspace and every playbook list until a manual Sync or restart — and the lists never refresh on assign/unassign #2703's neighbourhood, unrelated to subscriptions.
2. test_2638_subscription_switch_on_turn.py — 3 failures in the 8-file set under -p no:randomly in one specific fixed order. The file alone is 54/54 green, every pairwise combination is green, and the default (randomized, i.e. what CI runs) mode is green. Reproduced on base.
3. test_subscription_auto_switch_pingpong.py — 10 setup ERRORS at --randomly-seed=7. Reproduced on base.

A zero-adoption sweep writes no audit row on the not names / not candidates / not assignable early returns — the summary row is written on the Docker-unreadable abort and after a real loop. So on a keyed install neither trigger leaves a trace. This is by design (the docstring's stated purpose is to distinguish "0 of 40 because Docker was unreadable" from "nothing to do"), and it is why the isolated-stack probes above correctly show 0 rows. A DEBUG line for the silent returns is a follow-up.

Follow-ups (not filed — for the maintainer)

Product, shown to the issue owner with the shipped option and not overridden:

• N2 — an Agent Detail page open during a sweep shows a stale badge until reload; the header does not poll and no WS event exists for auth changes (fleet badges self-heal in 60 s).
• N6 — a failed background apply is a WARNING log + the sweep-summary audit row, not an operator-queue item.
• N7 — no hard cap: above 50 adoptions the sweep warns and continues, because a cap would silently strand the remainder with no trigger to recover them.

Engineering, found during this work:

• system_agent_service._create_system_agent never calls select_subscription_for_new_agent, so the create-time trigger does not cover trinity-system (A1/A2 now do).
• ent#435 delete-side gap: db.delete_setting has no twin of the set_setting sink guard, so DELETE /api/settings/{credential-shaped key} bypasses the secret-settings policy generally. This PR's second hook makes it harmless for this one key; the general gap remains.
• Prod runs uvicorn --workers 2, so _SWEEP_LOCK and bug: SUB-003 auto-switch has no per-agent lock — concurrent 429s race the restart #799's lock are per-worker. This contradicts the comment at subscription_auto_switch.py:64-70; the Redis SETNX path is already documented there. Worth closing as its own issue.
• The new agent_container_runtime_labels() is a ~45-line near-duplicate of agent_container_runtimes() — deliberate (their fail-open/fail-strict resolutions differ), but it deserves a parity test.
• subscription_clear writes an audit row on a no-op DELETE (no subscription was assigned).
• Without the lock skip, T7b hangs instead of failing (asyncio.wait_for); a hang is a worse failure mode than an assertion.
• A DEBUG line on the sweep's silent early returns.
• The backend should honour TRINITY_AGENT_NETWORK at the three agent-create sites. That single change would make plan §13's full 11-step proof runnable beside a live stack — the reason 7 steps are unrun above.
• A comment on bug: subscription register/delete/assign and the auto-switch settings are not audit-logged — a wiped subscription leaves no trace (SEC-001 gap) #2421 naming subscription_assign / subscription_clear as landed, so its remaining scope is unambiguous.
• The .claude/agents/test-runner.md catalog row for test_2572_credentialless_adoption.py is owed. .claude is a private submodule and read-only from this worktree, so it is not in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_017YoUiMQgPj3BeFMpyYpfkB

[vybe]

merge-train: pushed fc7eaebc1 to this branch — a merge of origin/dev resolving the docs/memory/feature-flows.md index-row append collision with #2739 (kept both rows; nothing else changed). Mechanical, per the train's routine-conflict class; no code touched.

[vybe]

merge-train: batch validated on train/20260913-0705 (#2745) — lane B, /validate-pr + /review clean; 38 executing tests, every new symbol has a live caller.