fix(enums): collapse four divergent enum forks to the union of their members (#13845, #13846, #13597, #13578)

[mrveiss]

Closes #13845, #13846, #13597, #13578

Thinking Path

Four issues, one shape: an enum forked into two or more definitions, and in every case
the divergent tail was the functionality. So the method was member sets, not names — a
same-name sweep is precisely what missed these, since each pair used different names for
one concept.

1. Member sets first. Enumerated every side of every fork as a set before touching
anything. That immediately turned up two forks the issues did not name:

• a third SecretType in api/schemas_system.py — the request/response vocabulary,
  also missing OAUTH_REFRESH_TOKEN. So POST /secrets could not even accept the one kind
  the database row could already store. canonical(enums): SecretType and SecretRequirement duplicate the secret taxonomy — agents cannot express an OAuth requirement #13846 described the gap at the agent layer; it was
  live at the API layer too.
• "connector_oauth_token" — a bare string persisted by knowledge/connectors/credential_store.py,
  commented "SecretType label" while being absent from every SecretType that existed.
  The real OAuth producer was already bypassing the taxonomy.

2. Deliberate forks checked before collapsing. Read the docstrings first. Three found,
none collapsed:

enum	why it stays	evidence
models/command_execution.py::RiskLevel	uppercase wire format already serialized by legacy Redis/SQLite records, 8+ producers	its own docstring, #7258 — named as out of scope by #13845
security/prompt_injection_detector.py::InjectionRisk	grades text for injection patterns, not a command for execution policy; scales differ at both ends (LOW only there, DANGEROUS/FORBIDDEN only here)	found by the new fork scan; docstring added recording the distinction, and it is allowlisted in the guard with a test that the allowlist entry still names a real enum
autobot-infrastructure/shared/scripts/secrets_manager.py::SecretType	runs standalone on infrastructure hosts and never imports application packages — the same reason ChatSecretScope is duplicated there (#11759)	canonical-pointer comment added; WEBHOOK_URL / CUSTOM are infra-only

3. Wire and persistence surface checked per fork. "dangerous" is emitted by
POST /terminal/command and by the terminal WebSocket; "postgresql" was a live dispatch
key and so is already in stored rows; secrets.type stores SecretType values; the SLM
service_type column is a plain string at rest. Every one of those spellings still parses
and, where it was emitted, is still emitted.

4. Then union. One canonical name per concept, plain concept names, no compat aliases
left behind — every call site migrated.

What Changed

#13845 — CommandRisk ∪ CommandRiskLevel

Canonical CommandRisk now lives in autobot_shared/status_enums.py, the documented enum
home. Both fork sites were deleted and all 14 importers migrated.

member	CommandRisk (executor)	CommandRiskLevel (wire schema)	canonical member	value
SAFE	yes	yes	CommandRisk.SAFE	safe
MODERATE	yes	yes	CommandRisk.MODERATE	moderate
HIGH	yes	yes	CommandRisk.HIGH	high
CRITICAL	yes	—	CommandRisk.CRITICAL	critical
FORBIDDEN	yes	—	CommandRisk.FORBIDDEN	forbidden
DANGEROUS	—	yes	CommandRisk.DANGEROUS	dangerous

Nothing dropped. FORBIDDEN and DANGEROUS are kept as separate members with separate
values rather than one folded into the other, because "dangerous" is already on the wire
and they are genuinely different reasons: DANGEROUS = matched a destructive pattern,
FORBIDDEN = base command is deny-listed. They reach the same verdict, so the enum grew a
.blocks property and every blocking decision now asks it. Comparing against one member
alone would have let the other producer's blocking verdict through — a fail-open the union
created and .blocks closes.

Two hand-written rank tables (_RISK_ORDER in the executor, _RISK_LEVELS in the approval
manager) listed five members each and would both have rated DANGEROUS as off-scale. Both
are gone; ordering is now CommandRisk.rank, derived from declaration order.

services/agent_terminal/utils.py::map_risk_to_level defaulted an unmapped member to
RiskLevel.MEDIUM — a blocking verdict silently downgraded to mid-scale. It now has an
explicit entry per member, a fail-closed CRITICAL default, and a guard asserting totality.

#13846 — SecretType ∪ SecretRequirement ∪ the schema fork

member	models.secret	schemas_system	SecretRequirement	canonical member
SSH_KEY	yes	yes	yes	SecretType.SSH_KEY
PASSWORD	yes	yes	yes	SecretType.PASSWORD
API_KEY	yes	yes	yes	SecretType.API_KEY
TOKEN	yes	yes	yes	SecretType.TOKEN
CERTIFICATE	yes	yes	yes	SecretType.CERTIFICATE
DATABASE_URL	yes	yes	yes	SecretType.DATABASE_URL
OAUTH_REFRESH_TOKEN	yes	—	—	SecretType.OAUTH_REFRESH_TOKEN
INFRASTRUCTURE_HOST	yes	yes	—	SecretType.INFRASTRUCTURE_HOST
OTHER	yes	yes	—	SecretType.OTHER
ANY	—	—	yes	SecretType.ANY
connector_oauth_token	(bare string, no enum)	—	—	SecretType.CONNECTOR_OAUTH_TOKEN

Nothing dropped, and one kind recovered from a bare string. The # nosec B105 /
# nosemgrep suppressions survive on the canonical definition.

ANY is preserved as a real member but is not a storable kind: SecretType.concrete()
excludes it (derived by exclusion, so a kind added later is included without a second edit),
GET /secrets/types serves concrete(), and SecretCreateRequest rejects it with a named
validator. SecretType.expand() resolves ANY to the whole taxonomy — without which a
requirement of {ANY} would be looked up as the literal type "any", which no stored secret
carries, so the broadest possible requirement fetched nothing at all.

AgentSecretMapping.required_types / optional_types were Set[str] of bare strings while
SecretRequirement sat unused beside them. They are now Set[SecretType], so an agent
mapping can name OAUTH_REFRESH_TOKEN directly instead of falling back to the blanket grant.

#13597 — severity

The enum half (AlertSeverity / ErrorSeverity → canonical Severity) had already landed
on base; this PR asserts it stayed landed, by AST, so re-declaring either as its own enum
fails even though the name would still exist.

The second half is the literal sweep, and the honest number is 169, not 119 — measured
on this tree with the matcher the ratchet uses, across 43 files. It grew by 50 between filing
and being worked. 43 of the 169 are already outside the canonical vocabulary
(warning ×25, HIGH ×9, CRITICAL ×3, MEDIUM ×3, error, degraded, Extreme).

Converting 169 call sites alongside three enum collapses is not a safely reviewable change,
so this PR ships the ratchet — pinned at exactly 169, to be lowered as literals convert,
never raised — and files the remainder with the full distribution as #14956. A guard that
stops regrowth is worth more than a partial sweep: the count moved 119 → 169 precisely
because nothing stopped it. Closes #13597 is claimed because the enum consolidation the
issue is titled for is complete and the literal half is now tracked, bounded and guarded.

#13578 — service_type

BackupServiceType(str, enum.Enum) added to models/database.py beside the eight siblings.
BackupCreate, ReplicationCreate and DataVerifyRequest are typed with it, so an unknown
engine is a 422 before a Backup row is written — no more FAILED row for a typo that is
indistinguishable from a real failure. Dispatch is keyed on members.

spelling	before	canonical member
redis	dispatch key	BackupServiceType.REDIS
postgres	dispatch key	BackupServiceType.POSTGRES
postgresql	second dispatch key for the same engine	BackupServiceType.POSTGRES via _missing_

postgresql still parses — it was a live key and is therefore already in stored rows — but
it collapses to one member at the boundary, so nothing downstream branches twice. The column
stays a string at rest, so existing rows are untouched. Response schemas keep service_type: str, so the response wire shape is unchanged.

Every persistence and comparison site uses .value explicitly (#14944): str() of a
(str, Enum) member is "BackupServiceType.REDIS", and these values reach SQL parameters.

Verification

Guard: repo_tests/enum_union_guard_test.py, 40 tests, all passing. Run against a
scratch copy of the tree outside the repo; no mutation was ever committed or pushed.

Mutation pairs — each removal and each rename must go red, because green after a rename
is what going blind looks like:

#	mutation	result
M1	drop CommandRisk.FORBIDDEN	RED (import fails closed)
M2	rename DANGEROUS → DESTRUCTIVE, value kept	RED (import fails closed)
M3	drop CommandRisk.CRITICAL	RED — 3 tests, incl. ...is_present_by_name[CRITICAL]
M4	rename SAFE → NONE, value kept	RED — 4 tests, incl. ...is_present_by_name[SAFE]
M5	drop SecretType.OAUTH_REFRESH_TOKEN	RED — 3 tests, incl. the OAuth-expressibility test
M6	rename SecretType.ANY → WILDCARD	RED — 5 tests
M7	add one bare {"severity": "high"}	RED — ratchet ceiling
M8	drop BackupServiceType.POSTGRES	RED
M9	re-declare CommandRiskLevel with from enum import Enum as _E	GREEN — fail-open, see below
M10	re-declare SecretRequirement with a plain Enum base	RED

M9 caught a fail-open in my own guard. The fork scan matched the literal word Enum in
the base list, so a base imported under an alias made a re-declared CommandRiskLevel
invisible and the scan reported the fork collapsed. Fixed by resolving the enum import to
its local names, and pinned by
test_the_scan_sees_an_enum_whose_base_was_imported_under_an_alias, which drives synthetic
source rather than depending on the tree containing an example. After the fix, M9 is RED and
the clean tree is 40/40.

Anti-false-pass measures, since an empty result reads as a clean one:

• test_the_enumeration_reaches_the_repo floors the tracked-Python walk at 3000.
• test_the_enum_scan_reaches_the_repo floors the enum scan at 150.
• test_the_severity_literal_matcher_still_matches proves the regex still matches, and still
  does not match a non-literal.
• test_every_known_distinct_entry_still_names_a_real_enum fails if an allowlist entry is
  stranded by a rename — an entry naming nothing exempts nothing, silently.
• Every mutation asserted its own target existed before mutating.

Enum semantics verified in a scratch copy outside the repo: rank/blocks/strictest, both wire
spellings parsing, concrete()/expand(), _missing_ resolving postgresql/POSTGRES/
" Redis " and rejecting mysql/""/"any", and pydantic honouring _missing_ while
still rejecting an unknown engine. The str() trap was confirmed live:
str(SecretType.API_KEY) is 'SecretType.API_KEY', not 'api_key' — hence .value at
every boundary.

api_endpoint_migrations_test.py asserted on source text (self.assertIn("risk_level = CommandRiskLevel.SAFE", source)), so it could not tell "the classification still works" from
"the identifier was renamed" and went red on a no-behaviour-change union. Rewritten to drive
the endpoint and assert on what it returns.

Not verified locally, left to CI: the full Python suite, and any runtime import of backend
modules — this repo's rule is that CI verifies correctness and the host verifies runtime.

Model Used

Claude Opus 5 (1M context)

---

Discovered and filed, not fixed here

• canonical(enums): 169 bare severity literals remain, 43 of them outside the canonical vocabulary #14956 — the 169 severity literals, with the full value distribution and the 43 already
  out of vocabulary.
• fix(frontend): getRiskClass keys two levels that no producer emits, so CRITICAL renders as unclassified #14955 — useCommandApproval.ts::getRiskClass keys MODERATE and DANGEROUS, which
  no producer emits; the actual producer emits MEDIUM and CRITICAL, so a CRITICAL
  command renders in the same neutral grey as an unclassified one. Pre-dates this change and
  survives it unchanged; fixing it needs the producer set confirmed and a duplicate map in
  ApprovalRequestCard.vue reconciled, which is a different blast radius.
• AgentSecretsIntegration.get_secrets_for_agent has no caller outside its own module —
  the agent secret-injection path is built but unwired. Noted here rather than filed blind;
  it is the seam feat(secrets): agents have no write path to the vault — add store_secret_for_agent with provenance #13711 is due to wire.

---

Generated types, and one consequence worth a reviewer's eye

The OpenAPI schema changes here, so both api.ts files drift. They were not hand-edited:
the repo's own autofix-types job regenerated and committed them
(e19554fa chore(types): regenerate generated API types from OpenAPI schema(s)), which is the
builtin path rather than a side-channel. CommandRiskLevel was never in api.ts — it was
never a pydantic field type — so nothing drifts there.

That regeneration surfaced a real consequence of preserving ANY as an enum member:
SecretType types both SecretCreateRequest.type and SecretModel.type, so the public type
now advertises "any" on a request and a response that always reject it. It is not a
functional bug — the _reject_the_wildcard validator returns 422, GET /secrets/types serves
concrete(), and nothing can therefore be stored or returned with that value — but the
declared type is wider than the accepted type.

Filed as #14974 with three options and a recommendation, rather than resolved unilaterally:
narrowing it correctly requires verifying a real schema dump rather than predicting one, and
the alternative (moving the wildcard out of the enum) contradicts #13846's explicit acceptance
criterion that ANY be preserved. Flagging it here so the reviewer sees it in the diff rather
than discovering it in api.ts.

---

Review fix — a confirmed regression, and the sweep that followed it

Review found one blocking regression, and it was mine. autobot-slm-backend/models/schemas.py
now imports BackupServiceType from models.database, but
autobot-slm-backend/tests/api/test_apply_secrets.py replaces models.database with a bare
MagicMock() and hand-installs one real stand-in (NodeStatus) precisely because pydantic
needs a genuine enum type to build a schema. There was no equivalent for BackupServiceType,
so it resolved to a MagicMock attribute and pydantic raised
PydanticSchemaGenerationError — a collection error, so all 9 tests in the file stopped
running rather than one failing.

Fixed by adding a real _BackupServiceType stand-in beside _NodeStatus, with a comment
saying why it must be a real enum.

I did not stop at the file the log named. Grep could not answer "is anything else stubbed the
same way", so I settled it by collecting instead:

tree	result
this branch, autobot-slm-backend/tests/	1697 collected, 0 collection errors
origin/Dev_new_gui, same invocation	1697 collected

Identical counts, so parity is restored and nothing else has the same shape. The 9 tests in
test_apply_secrets.py were then run directly: 9 passed.

The local run also reports a multipart sys.modules leak from
autobot-slm-backend/tests/api/conftest.py. That is not mine: the base tree produces a
byte-identical leak report, including the same 7 known leakers. It is an artifact of running
the whole SLM tests directory in one invocation locally, which pytest.ini documents. I
checked rather than assumed, after a truncated grep | head first made the base look clean —
the same empty-result-reads-as-clean trap this PR's guard is built against, this time in my
own tooling.

Two smaller review items, in the same commit (579eeca9, whose message names only the
blocking fix):

• repo_tests/enum_union_guard_test.py — the fork scan resolves a direct
  from enum import ..., so a two-hop re-export is invisible. Narrower than M9 and
  evasion-only, but real. Now stated in _local_enum_names's docstring and pinned by
  test_a_two_hop_re_export_is_a_known_blind_spot, which asserts the current limit and tells
  the next person to update it rather than delete it if the scan learns to follow re-exports.
  A limit nobody wrote down is the one mistaken for coverage.
• autobot-slm-backend/services/replication.py — dropped a redundant
  BackupServiceType(service_type).value re-wrap on an already-typed member.

Guard after these edits: 41 passed (40 + the new boundary test).

---

CI went red on python-suite shard 10/12 — root-caused and fixed

Two failures, both mine, both in autobot-backend/api/secrets_test.py:

FAILED TestChatSecretScopeEnum::test_secret_type_values_pinned
FAILED test_get_secret_types_response_pinned

These are pin tests over the secret-kind vocabulary, and they went red for exactly the right
reason: the vocabulary changed. test_secret_type_values_pinned imported SecretType from
api.schemas_system — the 8-member fork — and now sees the 11-member union;
test_get_secret_types_response_pinned pins the /secrets/types payload, which now serves
concrete() (10) instead of 8.

Updated rather than deleted, and made sharper in the process. The enum and the endpoint are
now deliberately different populations, so the test says so instead of asserting they are
equal — asserting equality would have hidden the one thing most worth pinning:

• EXPECTED_TYPES — the 10 concrete kinds the endpoint serves, gaining oauth_refresh_token
  and connector_oauth_token.
• test_secret_type_values_pinned — pins all 11 enum values and that concrete() is the
  10, and that any is in neither served nor concrete.
• test_the_oauth_kinds_are_expressible — new, asserting canonical(enums): SecretType and SecretRequirement duplicate the secret taxonomy — agents cannot express an OAuth requirement #13846's actual functional gap is
  closed rather than inferring it from a list length.

Verified against the reproduction: secrets_test.py 10 passed. Shard 10 reported
2 failed, 2467 passed; the other 9 finished shards were green, so these two were the whole
of the red.

A third hand-written risk-rank table, found by sweeping rather than by CI

Grepping the risk vocabulary while fixing the above turned up
services/approval_memory.py::RISK_LEVELS — a third literal rank table (after the two in
secure_command_executor.py and command_approval_manager.py), listing the same five members
and therefore blind to DANGEROUS. Now derived: {risk.value: risk.rank for risk in CommandRisk}.
Only relative order matters there (both sides of the comparison read the same table) and the
order is unchanged; the integers are never persisted. Also replaced a hardcoded
"risk": "forbidden" in secure_command_executor.py with CommandRisk.FORBIDDEN.value.

Branch was BEHIND, and the comparison that revealed it was wrong first

While checking whether the change dropped any tests, a backend collection showed base at
23783 and this branch at 23714 — 69 fewer. The removals were all in RBAC and auth-parity
files this PR never touches, which is what made it worth chasing rather than rounding off.

Cause: base had moved two commits since this branch was cut, one of them
#14936 "establish the canonical role vocabulary" — which reparametrizes exactly those
role/permission tests. I had been diffing my branch against a newer base than it was built
on, so the delta was someone else's work, not a regression here.

Branch updated by merging origin/Dev_new_gui in (clean, no conflicts; merged rather than
rebased because CI has pushed commits to this branch and a force-push would discard them).
Two files were touched by both sides — autobot-backend/security_layer.py and
autobot-frontend/src/types/generated/api.ts — and both retain both sides' changes. Branch is
now 0 commits behind base.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!