settings: versioned store names, migrations and schema drift (plan 3) - #484
Merged
Merged
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r review) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…older names Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ort newer ones Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ery name; pruneStoreName Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…unmigratable values Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…onfirmation Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t fields in --json OlderName now carries authored (the as-stored value) so a forced prune of a diverged name can print it, and print it before the delete rather than after pruneStoreName returns. Secret defs now omit value/olderValue/ currentValue/authored from --json entirely instead of substituting a placeholder string, matching check.ts. Text output keeps the "(secret)" placeholder. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… /prune Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…migrateFrom, in CI and at release Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…etic shipped-ref tests Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…in the current schema Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…seeding on unvalidated leaves sampleValues no longer loses union branches or enum values past its per-node cap: branch lists round-robin interleave before any downstream slice, and gen() filters every candidate against its own node's schema so an invalid leaf seed (0 under exclusiveMinimum 0, "" under minLength 1) never poisons every sibling variant built from it. Lifted the duplicate isObject/isSchema plain-object guard into schema.ts as isSchema, used by both schema-diff.ts and sample-values.ts. migration-proof.ts now reuses def.layerSchema when present instead of recomputing it, matching checkSchema. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…igrations Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d rename drafts Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…st when it started empty Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # packages/rt-client/src/settings/__tests__/schema-examples.ts
Preflight's new settings-stores row runs the checkout's own `settings check --json` read-only against the real stores, so a release cannot ship a migration that fails against what people actually have on disk. index-surface.test.ts pins the migration verbs on rt-client's public entry point and keeps the authoring tools off it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Documents the key@N store-version scheme end to end: the release skill's stale-row guidance, settings-architecture.md's new section and footguns, and the rt-client/settings-kit READMEs (including the listUnregisteredSettings/defs "newer" flag). Fixes resolve.ts and server.ts doc comments that still called explain's value AS AUTHORED, now that value is migrated and authored is the stored form; server.ts's WireIssue doc also covers the diverged kind's fields. command-tree-def.ts and checks.yml now describe the real storeVersion + migrateFrom acceptance rule instead of the old "bump and acknowledge" wording (bun run docs:gen ran after). Bumps rt-client to 0.33.0 and settings-kit to 0.5.0 (peer range >=0.33.0 <1); never published. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the release skill docs/settings-architecture.md's acceptance paragraph still described the old "bump and acknowledge" rule; it now says what checkLockAgainst actually enforces: a storeVersion bump by one plus a matching migrateFrom entry, with the breaking-schema-changes.json reason standing in only for a never-shipped key in CI, and a removed key needing a rename heir or that same reason. rt-release/SKILL.md's parenthetical claimed the never-shipped hatch applies at release time; it does not there (a key in the tag lock has shipped), so it now names the one case the hatch still covers at release: a removed key with no rename heir. Also lists the new settings-stores check in step 1's preflight summary. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
POST /prune skipped the composite, scopes and isWritable checks /unset enforces, so a composite key was refused by /set and /unset but pruned anyway. Add the same gates in the same order with the same messages, and a test proving /prune refuses a composite key under the default allowComposite. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A property renamed while another was added in the same object drafted a delete plus an add with no note, so every gate accepted a data-dropping migration. Every drafted delete now carries a note pointing at renameProperty as the alternative; existing delete tests that asserted empty notes are updated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- write.ts: correct removeFromStore's doc comment (it takes a path
planner, not a single jsonPath, and detects a no-op by text
comparison, not by zero edits from modify).
- react.ts: rephrase a process citation ("spec 3's Remove the older
name") as the console's own action name.
- write.ts: the diverged-older-name unset refusal now also says a
team-store name needs --team and that prune lists every name it
will delete before confirming.
- lib/release/preflight.ts: build a settings-check finding's location
from scope and repo unconditionally, so a repo-only finding (no
scope) still names its repo; add a test for it.
- settings-schema.test.ts: pass shippedLock: null on the --draft test
so it never shells out to real git.
- resolve.ts: reword listUnregistered's warning to drop its em dash.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 56 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 87 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (62)
Comment |
Two removed keys of the same shape as one added key no longer both draft a rename onto that added key. Every rename draft now carries a confirm note. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…cope older-name findings check.ts and resolve.ts's invalid branches dropped the migration error when the authored old-shape value also failed the type check; both now report the migration error first. checkSection now applies the same scope and repoScoped allow-list planStoreMigrations uses before labeling an older name diverged, stale or leftover, leaving invalid and nonconforming untouched. renderExplainRow now shows the store-name suffix and older-name lines on an invalid row too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
null, an array or a string under $migrated cannot take a property edit underneath it (jsonc-parser's modify throws adding an index to it). The first write of a bumped key over one now replaces $migrated wholesale with a fresh object holding the new baselines, in the same write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
/prune's scope-not-allowed message was copied from /unset and kept its wording. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A bare --force, or one immediately followed by another flag, silently dropped instead of erroring, so --force --yes never swallowed --yes as a key but also never told the caller why nothing was forced. Both are now a usage error. A --force key that matches no older store name in the plan now warns instead of silently doing nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The "in" operator sees Object.prototype names (constructor, toString, hasOwnProperty), so renameProperty, deleteProperty, setDefault and the path walker could act on an inherited member that was never an own property of the stored value. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The schema-lock row's release-notes bump list only matched keys present under the same name in both locks, so a key that was renamed and bumped in the same change (present in the tag lock only under its old name) never appeared. It now also lists a heir's bump against the old key's storeVersion, as "old -> heir oldVersion -> newVersion". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-ref cases Three settingsSchemaDiff tests called real git against this checkout (HEAD, a nonexistent branch, a nonexistent tag) instead of a fake, the way the neighboring git-failure tests already do. None of them need a real git checkout to pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…omment Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
Problem
A composite settings key could not change shape between releases. Plan 1 blocks a breaking schema change; nothing carried stored values forward, and old and new rt-clients share the same stores (synced user store, team store, apps pinned to an older rt-client).
Fix
Plan 3 of the settings work (spec
docs/superpowers/specs/2026-09-25-settings-migrations-design.md, plandocs/superpowers/plans/2026-09-25-settings-migrations.md).storeVersion; the key is stored askey@N. Code keeps the plain key. An older reader keeps reading the name it knows.migrateFromsteps (migrations/index.ts, zod-free) carry the highest older name forward in memory. A step that throws, or a result failing the schema, leaves the stored value in effect and labels the rownonconforming.key@Ninto a section records$migratedhashes for older names there, in the same write. Older names are thenleftover,staleordiverged(an old writer edited or recreated them).storeName,storedVersion,authored,olderNames;rt settings checkfails on a diverged name; newrt settings migrate(dry run,--write,--prunewith confirmation,--team,--force <key>);pruneStoreName; unset removes every name. Secret values never leave through any of them.issues[]gainskind: "diverged", andPOST {base}/prunedeletes one older name behind the same gates as/unset.migrateFromentry equivalent to main's lock (a never-shipped key, renames included, may still be acknowledged in CI). Release preflight checks the whole chain against the last tag, lists bumps for the release notes, and runs the candidate'srt settings checkagainst the real stores.rt settings schema diff --draftwrites the step and the previous schema (mechanical when replaying ops reproduces the new schema, otherwise aTODOstub; every drafted delete carries a note).@mattstack/rt-client0.33.0,@mattstack/settings-kit0.5.0 (peer>=0.33.0 <1). Not published.No registry key is bumped here; the registries ship empty.
Acceptance
bun run cli.ts settings checkagainst real stores exits 0 (read-only)Side decision
v2.13.1 has no
schema.lock.json, so until the next tag every key counts as never shipped (plan Decision 3). Cutting a release soon after merge closes that window.🤖 Generated with Claude Code