fix: make durable data mean one thing, and the backup gate satisfiable - #33
Merged
Conversation
Two hosts of field testing found the migration backup gate could not be used at all, and that three separate places were each guessing at the same undefined meaning. A second opinion reframed it usefully: this is not a new default, it is a published default the loader made unreachable. ## What an absent `persistence` block means The contract publishes `persistence.mode` defaulting to `durable`, but the block is optional and the default was only applied when the block already existed (defaults.go: `if w.Persistence != nil && w.Persistence.Mode == ""`). So the default was unreachable exactly when it mattered, and `ob doctor`, the migration backup requirement and the protection gate each read absence as "not durable". A workload with a managed named volume now holds durable data whether it says so or not, and the inference is materialised so `ob canonical` shows it as `# default` rather than hiding it. `persistence` becomes a way to downgrade (`ephemeral`, `external`), not a prerequisite for being noticed. A bind mount is deliberately not durable: onebox neither created the host path nor can tell configuration from data by looking at it. Counting them would demand backup evidence for every `./config` mount, and a warning that fires on everything is one nobody reads. `ob doctor` mentions them instead. Inference does not fire `stateful_replicas`. That refusal still requires an authored block, because the contract promises a constraint is not tightened against a project that already loads — the hazard is reported by `ob doctor` instead. The rule is now also expressed in the published schema, so an editor underlines it too. ## The gate could not be satisfied at all `reflect.DeepEqual` treats an empty slice and a nil slice as different. A policy requiring no key material leaves the requirement nil while a receipt with none carries a zero-length slice, so with the default configuration every receipt `ob backup-evidence create` produced was refused. Verified on a host: declaring one key made the same flow pass. The comparison is now `slices.Equal` behind a named helper with a test. The requirement also counted only workloads, so the standard shape — replicated stateless application plus a managed database — could not satisfy it: the gate wanted a durable workload, `stateful_replicas` refuses a durable workload with replicas, and services counted for nothing. Managed services are now resources. `migration_backup_maximum_age` gets a 24h default. Enabling one boolean made it mandatory, and its absence produced an untyped complaint about an empty duration for a field the author had never heard of. ## The manifest is no longer authored blind It is the only artifact a person must write, and the docs named its schema version without ever giving its shape. `ob backup-evidence template --plan` emits a skeleton with the plan's resources already filled in, generated from the same types so it cannot drift, and the guide now carries the field table. ## And a regression of my own The naming PR moved `ob abort --force` to `--break-migration-gate` and did not follow it into free-text errors. Four messages were wrong, two of them in HALT-AND-PAGE guidance — read at the moment a migration has already run and the release is stuck. The remedy test now resolves the flag against the command, not just the command; verified by reverting a string and watching it fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review found the previous commit's central mechanism was wrong. Materialising
the inferred `persistence` block into the document and carrying "this was
inferred" as in-memory state does not survive a serialisation boundary, and
there are two:
- deepCopy round-trips through JSON, so `Resolve` re-ran the cross-field rules
against a clone that had the block but not the exemption. Any project with a
named volume, replicas and ANY environment override loaded fine and was then
refused at resolve time — blamed on a `replicas` override nobody wrote. The
new conformance cases passed only because none of them had overrides.
- `ob canonical` printed the materialised block, so its own output no longer
reloaded, and contradicted the schema rule this branch adds.
The inference is now a derived read (`HoldsDurableData`) used by `ob doctor` and
the migration backup requirement. The document is never edited, nothing
tightens, and `stateful_replicas` keys on the authored block as before. The cost
is that `ob canonical` no longer shows the inference; doctor states it instead,
which is where an operator is asking the question.
Also from the review:
- `persistence: {mode: external}` was silently dropped from the backup
requirement. The old guard skipped only `ephemeral`, and the old error text
said "durable or external". Declared modes other than ephemeral count again;
the inference applies only when nothing was declared.
- doctor told an operator with volumes and replicas to declare durable
persistence, which the loader then refuses. It now says to run one instance
first, and only suggests the declaration where it would actually load.
- the 24h default was applied but published nowhere, so the contract said
there was none. `default:"24h"` on the tag reaches the field table, the
policy reference and the JSON schema.
- the schema rule missed `persistence: {}`, where mode defaults to durable —
the one shape it was added for.
- the guide said restore_test's method, tested_at and validation_digest were
optional. `passed` requires all three and `not_tested` refuses them.
- the template hardcoded `not_tested`, so a project requiring a passed restore
test got a skeleton its own plan refuses, with none of the fields it needs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vishr
added a commit
that referenced
this pull request
Aug 18, 2026
fix: make durable data mean one thing, and the backup gate satisfiable
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.
Two throwaway hosts of field testing found the migration backup gate unusable, and that three places were each guessing at the same undefined meaning. A second opinion reframed it usefully: this is not a new default, it is a published default the loader made unreachable — which makes the fix conservative rather than lenient.
What an absent
persistenceblock meansThe contract publishes
persistence.modedefaulting todurable. The block is optional, and the default was only applied when the block already existed:So the default was unreachable exactly when it mattered.
ob doctor, the migration-backup requirement and the protection gate each read absence as "not durable" — and 15 of 16 real projects declare volumes while none declarespersistence.ob canonicalshowsmode: durable # defaultrather than hiding the inference../configmount, and a warning that fires on everything is one nobody reads.ob doctormentions them instead.stateful_replicas. That refusal still needs an authored block, because the contract promises a constraint is not tightened against a project that already loads.ob doctorreports the hazard instead. The rule is now also expressed in the published schema, so an editor underlines it.ob doctoron gitea, before and after:The gate could not be satisfied at all
reflect.DeepEqualtreats an empty slice and a nil slice as different. A policy requiring no key material leaves the requirementnilwhile a receipt with none carries a zero-length slice — so with the default configuration every receiptob backup-evidence createproduced was refused. Verified on a host: declaring one key made the identical flow pass.The requirement also counted only workloads, so the standard shape — replicated stateless app plus managed database — could not satisfy it at all: the gate wanted a durable workload,
stateful_replicasrefuses a durable workload with replicas, and services counted for nothing. Managed services are now resources.migration_backup_maximum_agegets a24hdefault; enabling one boolean previously made it mandatory and its absence produced an untyped complaint about an empty duration.The manifest is no longer authored blind
It is the only artifact a person must write, and the docs named its schema version without ever giving its shape — three refusals, one field at a time, even with source access.
ob backup-evidence template --plan ob-plan.json > backup-facts.jsonGenerated from the same types, so it cannot drift. The guide now carries the field table too.
A regression of my own
The naming PR moved
ob abort --forceto--break-migration-gateand didn't follow it into free-text errors. Four messages were wrong, two in HALT-AND-PAGE guidance — read when a migration has already run and the release is stuck. The remedy test now resolves the flag against the command, not just the command; verified by reverting a string and watching it fail.just cigreen; Docker e2e green at 202s.🤖 Generated with Claude Code