Repository navigation
tech-debt(backups): service_type is a bare string among eight sibling enums, and now needs two spellings of postgres #13578
Description
Found while adding the PostgreSQL backup path (#13307, PR #13487).
models/database.py declares eight (str, enum.Enum) classes — NodeStatus, DeploymentStatus, BackupStatus, CodeStatus, RoleStatus, SyncType, EventType, EventSeverity. service_type is the outlier:
models/database.py:186 service_type = Column(String(32), default="redis")
models/schemas.py:315 service_type: str = "redis"The cost showed up immediately when a second engine was added. api/stateful.py now dispatches on the raw string and has to accept two spellings, because nothing constrains what a caller sends:
runners = {
"redis": backup_service.execute_redis_backup,
"postgres": backup_service.execute_postgres_backup,
"postgresql": backup_service.execute_postgres_backup,
}An unknown value is not rejected at the API boundary — it reaches _run_backup, creates a Backup row, and only then fails with Unsupported service type. A BackupStatus.FAILED row for a typo is indistinguishable from one for a real failure.
Work
-
class BackupServiceType(str, enum.Enum)inmodels/database.py, alongside the eight that already exist -
BackupCreate.service_typetyped as the enum, so FastAPI rejects an unknown value with 422 before a row is written - Dispatch keyed on enum members rather than string literals; the
postgres/postgresqlalias becomes one canonical member - Existing rows keep working — the column stays a string at rest, the enum is the boundary
Refs #13307
Activity
mrveiss commentedon Aug 24, 2026
Delivered in #14957 (branch
issue-13845-enum-union, head17b58390). Member sets, deliberate-fork findings and the per-fork union tables are in the PR body. Not merged — CI in flight.fix(test): give the models.database stub a real BackupServiceType so …
fix(enums): collapse four divergent enum forks to the union of their …
mrveiss commentedon Aug 24, 2026
Delivered and merged as
342eeb05e(PR #14957).python-suiteshards green, including shard 10 — the one that went red on the pin tests.code-reviewerpass. Every fork's member set was reconstructed from the merge-base tree and diffed against the union rather than taken from the PR body.Nothing was dropped.
FORBIDDEN,DANGEROUS,OAUTH_REFRESH_TOKENandANYall have a real home.FORBIDDENandDANGEROUSwere kept as separate members, and the reasoning is now in the source: they are distinct reasons reaching the same verdict —DANGEROUSmeans the command matched a destructive pattern (RISKY_COMMAND_PATTERNS),FORBIDDENmeans the base command is on the executor's deny list (FORBIDDEN_COMMANDS)."dangerous"is also already serialized by two live endpoints. Folding either away would have broken a wire contract or erased a distinction the executor actually makes.The union created a fail-open, and the PR closed it. With two blocking members, an equality check against one let the other producer's blocking verdict through. The enum gained
.blocks, and review confirmed all three production comparison sites migrated with none missed. AHIGH_RISK_COMMAND_LEVELSset that previously drew only fromCommandRiskLevelnow picks upFORBIDDENautomatically.map_risk_to_levelwas fail-open — it defaulted unmapped members toMEDIUM, downgrading a blocking verdict to mid-scale. Now total, with the fallback changed toCRITICAL, and a guard that reads the table via AST and asserts totality against the live enum so a future member without an entry fails loudly.A third
SecretTypethe issues never named was found inapi/schemas_system.py— also missingOAUTH_REFRESH_TOKEN, soPOST /secretscould not accept the kind the database row already stored. And the real OAuth producer was persisting"connector_oauth_token"as a bare string, commented as aSecretTypelabel while belonging to no enum at all.Four deliberate forks preserved with evidence, none collapsed: the #7258 legacy-wire
RiskLevel, the standalone host-sideSecretType,InjectionRisk(grades text, not commands — discovered by the new scan and documented rather than folded), and the three role vocabularies whose own issue states they are correctly separate.The guard caught itself failing open. Mutation M9 re-declared a fork behind
from enum import Enum as _Eand the scan stayed green, because it matched the literal wordEnum. Fixed by resolving the import to its local names. A two-hop re-export remains a known limit, now stated in the docstring and pinned by a test that instructs the next person to update it rather than delete it.A regression this PR caused was found and fixed before merge:
models/schemas.pygained an import thattest_apply_secrets.pycould not satisfy — that test replacesmodels.databasewith aMagicMockand hand-installs one real stand-in, because pydantic needs a genuine enum type. The result was a collection error, so all 9 tests in the file stopped running rather than one failing. The blast radius was then settled by collecting both trees (1697 each) rather than grepping, since collection is exactly where the failure manifests.Remaining and filed: #14956 (169 severity literals — the honest measured count, not the issue's 119, with 43 already out of vocabulary; ratcheted so it can only fall), #14974 (
ANYadvertised on two API surfaces that reject it), #14955 (CRITICALrendering as unclassified grey).