Skip to main content
← Back to list
01Issue
BugShippedSwamp CLIPublic
Assigneesskunk-ape

Relationships

#2412 Definitions with no typeVersion get the entire upgrade chain applied, which can corrupt current-shaped arguments

Opened by skunk-ape · 9/23/2026· Shipped 9/23/2026

Split out of swamp-club#900, where it was found during adversarial review of the plan.

What happens

DefinitionUpgradeService treats an absent typeVersion as apply every upgrade in the chain (src/domain/models/definition_upgrade_service.ts, the fromVersion undefined branch feeding applicableUpgrades). For a definition that genuinely predates CalVer versioning that is correct and deliberate: the schema marks typeVersion optional, a preprocess step coerces legacy numeric values to undefined, and two tests pin the behaviour by name, including one commented as a legacy pre-CalVer definition.

The problem is that absence cannot distinguish a genuinely legacy definition from one whose globalArguments are already in the current shape. Running the full chain over current-shaped arguments transforms them a second time and corrupts them.

Definitions under models/ are git-tracked source-of-truth files, so a hand-written or hand-edited one that carries no typeVersion line will hit this path.

Why it is filed separately

Issue 900 removes an unconditional typeVersion restamp in YamlDefinitionRepository.save that currently masks this case, because any ordinary save stamps the definition to the current version. Once 900 also makes every creation path stamp typeVersion explicitly, the unstamped population stops growing, so 900 neither causes nor depends on this defect. It does increase exposure, because unstamped definitions will no longer be silently stamped on save.

Blast radius today

Narrow. The scenario needs an absent typeVersion, a type that declares an upgrade chain, and arguments already in the current shape. Only one first-party model declares an upgrade chain today, @swamp/issue-lifecycle with 18 entries, and its definitions are auto-created by direct type execution, which stamps the version. The realistic exposure is third-party extensions that ship upgrade chains.

Options considered

  1. Treat absent as current and skip the chain. Never corrupts, but genuinely legacy definitions stop auto-migrating.
  2. Keep apply-all but stamp on first load. Reintroduces implicit stamping and still cannot tell the two populations apart.
  3. Refuse and fail loudly when a chain exists but no version is recorded. No silent corruption, but turns currently-working runs into hard failures.

Whichever rule is chosen needs an integration test covering a hand-authored definition with no typeVersion against a type that has an upgrade chain.

02Bog Flow
✓OPEN✓TRIAGED✓IN PROGRESS✓SHIPPED+ 1 MOREASSIGNED+ 5 MOREREVIEW+ 7 MOREPR_MERGED+ 2 MORESESSION_SUMMARIZED

Shipped

9/23/2026, 8:29:26 PM

Click a lifecycle step above to view its details.

03Sludge Pulse
skunk-ape assigned skunk-ape9/23/2026, 7:22:04 PM
Editable. Press Enter to edit.

skunk-ape commented 9/23/2026, 6:44:15 PM

Direction: drop the legacy concept, three explicit states

None of the three options as filed is quite right, because they all accept the premise that an absent typeVersion is a meaningful signal worth preserving. Digging into the history says it is not.

Pre-CalVer was a bare integer, and the window was three days

typeVersion was introduced in #203 (df04d0dc, 2026-02-06) as z.number().int().positive().optional() — the values were 1, 2, 3. There was never a dotted pre-CalVer format. CalVer landed in #230 (9a418faa, 2026-02-09), after which modelDef.version is a CalVer string and nothing can write a number again. Four tagged builds fall in that window.

The restamp that swamp-club#900 just removed then ate the survivors: until c811f8ce, save() overwrote typeVersion on every write, so any numeric definition that was run, edited, or otherwise persisted in the last seven months was stamped to current CalVer. The surviving numeric population is definitions created in a three-day window in February that have not been saved once since.

Separately, the earliest upgrade entry in @swamp/issue-lifecycle — the only first-party chain — targets 2026.03.26.1, six weeks after CalVer. No genuinely legacy definition could need it.

Absence and malformation are different failures

The issue treats both as "no usable version". They are not. Absence means nobody recorded an intent — plausible, since definitions under models/ are git-tracked files people hand-write. A malformed value means someone recorded an intent and got it wrong, and silently ignoring it is the same class of defect as silently corrupting: we throw away something the author expected us to read.

The rule

Delete the "legacy pre-CalVer" concept entirely, including the numeric preprocess, and replace it with three states that each mean one thing:

typeVersion Behaviour
absent Treat as current: skip the chain, do not stamp. Warn on a method run when the type declares an upgrade chain — the remedy is one line of YAML, and the warning is what option 1 was missing.
valid CalVer Unchanged.
anything else (1, "1.0") invalid. DefinitionUpgradeService raises a UserError naming the definition, the bad value, and the expected YYYY.MM.DD.MICRO format.

This is option 1 for the case that actually occurs, with option 3's loud failure reserved for the case where a human demonstrably tried to say something.

Two supporting changes:

  • The schema stops discarding numbers. A number coerces to its string form (1 → "1") rather than to undefined, so a malformed value survives into the object and model get can name it. Rejecting at the Zod boundary instead would make the definition unloadable, and model get is the command you reach for to diagnose exactly this file.
  • resolveStaleness gains an invalid state alongside unknown. It still never throws — a read-only diagnostic has to survive the file it describes — but it reports the difference honestly, so model get says "malformed" rather than "legacy". unknown keeps its name and means unstamped.

swamp-club#900 shipped the DefinitionStaleness value object, the model get fields, and the method-run warning site a day ago, so this hangs on existing scaffolding rather than building new surface.

Cost

Fourteen sites carry the legacy framing, all comments, docs, and one test name, plus the preprocess in definition.ts. The only behavioural loss is that a surviving numeric definition stops auto-migrating — and it now fails loudly with a one-line fix rather than silently applying eighteen upgrades to arguments that may already be current-shaped.

Sign in to post a ripple.