Skip to content

Commit 57a2612

Browse files
skills(objectstack-upgrade): the §3.3 acceptance replay names the last ported hop and can fail (#29751)
domain:skills#1 · huangyiirene · session_01J8VVAnMUWA9rufXzFaEh4J <!-- claude-projects-attribution: {"github_login":"huangyiirene"} --> _Requested by **Irene** · [project thread](https://claude.ai/code/project/chan_01TJXtQ1tVD26ecLDZxFS67x?thread=cmsg_01TJXtQ1tVD26ecLDZxFS67xDjK2kTDLPATyrvGTNpN73K)_ Fixes #29063 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`objectstack-upgrade` §3.3 的验收回放不再从目标主版本起跑(在最高主版本上那是空区间,`applied` 永远是 `[]`,检查永远过)。改为回放整段移植区间 `--from <authored-major> --to <target-major> --json`(与 quickstart 第 1 步同一区间),要求 `applied: []` 且 JSON 里没有 `replayedNothing`(#29045 合入后,没跑任何一步时 `--json` 会带这个键)。quickstart 第 74 行与文内另三处同一回放的提法改成一致;两条练这项检查的 eval 的期望同步。提示框里举的 `field-required-notnull-explicit` 已在 conversions registry 撤回,提示框不再点名任何 conversion;报告模板(examples-upgrade.md)的示例行改用 registry 真有的 `sharing-rule-access-level-full-to-edit`,其 Verified 行改为全区间回放「replay 16 → 18」。 - 为什么改:按原文执行的 agent 会记录一条从未检查任何东西的「元数据已规范」验收。 - 风险与回滚:只改技能文本与其 eval;无代码、无门禁变动。回滚即 revert。 - 先看:`SKILL.md` L74、L388–411;`evals/protocol-major-upgrade.json` 第 1、4 条;`references/examples-upgrade.md` L48、L54。 - 棘轮:token 棘轮就地付账,无 re-wrap。SKILL.md 删了两处同文件复述(见下),6185 → 6160 tokens,天花板 6188 未动;examples-upgrade.md 1194 → 1189 / 1197。 - 需要你做的:Tier H,等维护者的字。 ## What changes 1. **§3.3 Validate** (`skills/objectstack-upgrade/SKILL.md` L397–411). The replay that closes the gap after a green `validate` is now `os migrate meta --from <authored-major> --to <target-major> --json`, the whole range the sources were ported through (the `--from` of §0, the range quickstart step 1 replays), and the criterion is `applied` must be `[]` **and** `replayedNothing` absent. `composeMigrationChain` keeps `m > fromMajor && m <= toMajor`, so a replay from the major below the target would leave out every earlier hop; the whole range is non-empty whenever the crossing has a step and sees every hop. The paragraph that followed, which said `--to` defaults to one past the installed major so only `todos` carries the next major's residue, is replaced: it explains that `--from <target-major>` proves nothing, because `--to` defaults to the highest major this build carries a step for, so the range is empty (no step runs, `applied` is `[]` whatever the metadata holds, and the JSON says so with `replayedNothing`, whose `widestRange` is `null` there because no wider range exists from that `--from`) or holds only the next major's step. `todos` stays Layer 2's business, not a failure. `--to` is in the command listing at L143 already. 2. **Quickstart L74** now reads `os migrate meta --from 16 --to 18 --json # \`applied\` [] and no \`replayedNothing\` (see §3.3)`, which is §3.3's command for the quickstart's 16 → 18 crossing (step 1 replays `--from 16`). The card read L74 as unaffected; `--to 18` pins the range so the two lines say the same thing on a build that already carries a later step. 3. **The callout's example** (L388–394). `field-required-notnull-explicit` is withdrawn from `packages/spec/src/conversions/registry.ts` (its docblock exists to stop it being re-added), and every conversion this build ships is `retiredFromLoadPath` with its old shape refused at parse, so no migration-chain-only conversion exists on this head. The callout keeps the class and its reason, says this build ships none and a later one can, and names no conversion. The report template's example row (`references/examples-upgrade.md` L54, surface widened by the seat in 6594782842) named the same withdrawn id; it now shows the step-17 conversion `sharing-rule-access-level-full-to-edit` at the site shape the chain prints, `sharingRules[0].accessLevel` `'full'` → `'edit'` (`mapCollection` spells array sites by index; the conversion's own fixture is a `sharingRules` entry). No `notnull-explicit` mention remains under `skills/`. The same template's **Verified** line (L48, surface widened again in 6594814065) said "replay-from-17 applies 0 mechanical changes" under "Chain: 16 → 18", the last-hop form M1 retired; it now says "replay 16 → 18 applies 0 mechanical changes". 4. **Three other mentions of the same replay** in the file, so no line still says "from the target major": the §3.5 evidence row ("Only the acceptance replay sees this class."), the failure-mode row for `--write` ("then replay the ported range to confirm 0 changes") and the row "Replay from the target major still applies changes" → "The acceptance replay still applies changes". 5. **Evals 1 and 4** (`evals/protocol-major-upgrade.json`) expect `os migrate meta --from 16 --to 17 --json` (a one-hop crossing, where the ported range is that hop) with `applied: []` and no `replayedNothing`, and say why `--from 17` alone checks nothing. Eval 4's `must_contain` gains `replayedNothing` and drops `field-required-notnull-explicit`; its `expected_output` no longer names a conversion. Eval fixtures are measured, not ratcheted. The `replayedNothing` reading is taken from the merged CLI, `packages/cli/src/commands/migrate/meta.ts`: `readReplayedNothing` returns `null` when `result.hops.length > 0`, else `{ reason: 'empty-range' | 'inverted-range', widestRange: { from, to, applied, manual } | null }`; `--json` spreads it into the payload only in that case, next to `applied`, `todos` and `absentTodos`. `composeMigrationChain` keeps the majors `m > fromMajor && m <= toMajor`, so `--from 18` on this build (steps to 17 and 18, `CHAIN_TERMINUS_MAJOR` 18) composes zero steps. ### Token ratchet (paid in place, no re-wrap) `skills/objectstack-upgrade/SKILL.md`: 6185 → 6160 tokens, ceiling 6188 (headroom 3 → 28). `references/examples-upgrade.md`: 1194 → 1189 tokens, ceiling 1197 (headroom 3 → 8), one row replaced by a shorter one and one line reworded in place (+2 bytes), no re-wrap. The ceiling row in `scripts/check-skills-token-ratchet.mjs` is not touched (second file, out of surface). Two genuine deletions of same-file restatement pay for the added text: | deleted | home | |:--|:--| | §1 bullet "**Pending data migrations**, when the chain crosses into a major with per-deployment data gates — see below." (2 lines) | the §1 subsection "Data migrations are not metadata migrations", directly below, which says what the run prints and what to carry into the report | | failure-mode row "`validate` green from the start, so 'there was nothing to upgrade' … `validate` green is necessary, not sufficient; see 3.3" (1 line) | §3.3's own callout, titled "A green `validate` does NOT mean the chain has nothing left to do", which the row restated and pointed back to | Every other block is byte-identical to `main`. Line counts: SKILL.md 482 → 482, examples-upgrade.md unchanged. ## Why Card #29063 (filed from the #28947 dev, PR #29045): when the target is the highest major this build carries a step for, `os migrate meta --from <target-major> --json` is an empty range, so an agent following §3.3 records a canonical-metadata acceptance that never examined anything. Now that `--json` carries `replayedNothing` when no step ran, the skill can name a range that can fail and a key whose presence means the check never ran. ## Verification On head 55e64c9 (2063a6b is the contract-review delta for 6594719416: M1, S1 and the withdrawn-example point; 701f463 and 55e64c9 are the two template lines the seat widened the surface to, L54 and L48): - `node scripts/check-skills-token-ratchet.mjs`: exit 0 (SKILL.md 6160 / 6188, examples-upgrade.md 1189 / 1197), and its `--self-test`: exit 0. - `pnpm check:pm-skill-id-lint`, `@objectstack/spec check:skill-docs`, `check:skill-frame-sync`, `check:skill-frame-freshness`, `check:skill-compatibility`, `check:skill-identifier-liveness`, `check-doc-route-spelling --advisory`, `check:doc-authoring`, `check:role-word`, `check:corpus-claim-drift`: each exit 0. - `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`: exit 0 (four commits). - Full derived list on 55e64c9, `node scripts/pm/dispatch-gates.mjs --commands` → 35 families for the three paths, all run with a 900 s timeout each after `pnpm install --frozen-lockfile` (exit 0), exit codes recorded before any pipe (record in the project files as `gates-55e64c92.txt`): - 33 exit 0: `check-closing-keyword-parity` (+ self-test), `check-comment-mask-corpus`, `check-doc-route-spelling --advisory` (+ self-test), `check-dts-emitted --self-test`, `check-skills-token-ratchet` (+ self-test), the three `scripts/ci/*` self-tests, `@objectstack/lint check:doc-formula-expressions` (after `turbo run build --filter=@objectstack/formula --filter=@objectstack/lint`), `@objectstack/spec check:skill-docs`, `check:agent-test-spelling`, `check:ci-filter-parity`, `check:corpus-claim-drift`, `check:cross-package-test-inputs`, `check:doc-authoring`, `check:driver-memory-census`, `check:dts-closure` and `check:sourcemap-no-sources-content` (both swept the 4 packages that build left in `dist`), `check:gitlink-declared`, `check:merge-driver`, `check:nul-bytes`, `check:pm-governed-merges`, `check:pm-skill-id-lint`, `check:refd-timer-probe`, `check:role-word`, `check:skill-compatibility`, `check:skill-frame-sync`, `check:skill-identifier-liveness`, `check:watch-hint-literal`. - 2 exit 3 (prerequisite, not a finding): `check:dual-build-cjs-loads` and `check:lean-entry-closure` want a full `pnpm build` of ~72 packages; the diff touches no `packages/**`, so CI's Build Core is their reading. - `dispatch-gates --ran`: `35 derived famil(ies) accounted for — 33 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)`, exit 0. On head 1cb7088 (first head): - `node scripts/check-skills-token-ratchet.mjs`: exit 0 (SKILL.md 6147 / 6188). - `node scripts/pm/check-skill-id-lint.mjs` (with `--self-test`): exit 0, 108 files clean. - `node scripts/pm/check-skill-line-ratchet.mjs`: exit 0 (its roots do not include `skills/**`; run because the order named the line ratchet). - `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`: exit 0. - `evals/protocol-major-upgrade.json` parses. <details><summary>Local gates on 1cb7088: 34 derived, 30 run exit 0, 4 NOT MEASURED (`dispatch-gates --ran`)</summary> - `node scripts/pm/dispatch-gates.mjs --commands` on this head derived 34 commands for the two paths. All 34 ran with a 900 s timeout each after `pnpm install --frozen-lockfile`; exit codes were recorded before any pipe. - 30 exited 0, including `check-skills-token-ratchet` (and its self-test), `check:pm-skill-id-lint`, `check:skill-frame-sync`, `check:skill-identifier-liveness`, `check:skill-compatibility`, `@objectstack/spec check:skill-docs`, `check-comment-mask-corpus` and `check-doc-route-spelling`. `@objectstack/lint check:doc-formula-expressions` first exited 3 in the fresh tree and exited 0 after `turbo run build --filter=@objectstack/formula --filter=@objectstack/lint`. - 4 exited 3 (prerequisite, not a finding): `check:dts-closure`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:sourcemap-no-sources-content` each want a full `pnpm build` of ~72 packages. The diff touches no `packages/**`; CI's Build Core is the reading for them. - `--ran` printed: `34 derived famil(ies) accounted for — 30 run, 4 NOT-MEASURED (4 DERIVED from a recorded exit 3)`, exit 0. - Not derived for these paths and not run locally: the type-check lanes, the whole-tree artifact rosters, and the main-only push battery (dogfood, temporal conformance). </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. (no names added) - [ ] 3. No formatter ran; `pnpm lint` not run locally (docs-only diff; the derived gate families above are what ran). CI's Lint & Repo Gates is the reading. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (skills docs only, `skip-changeset`) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [ ] The *Before Submitting* list in CONTRIBUTING.md: the gate families derived for these paths ran (above); nothing else on the list applies to a two-file skills diff. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J8VVAoGi31QpTx8NiZpBWm --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2c91a55 commit 57a2612

3 files changed

Lines changed: 22 additions & 22 deletions

File tree

‎skills/objectstack-upgrade/SKILL.md‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ node -e "console.log(require('fs').readFileSync(require.resolve('@objectstack/sp
7171
# 3 · acceptance — all four, not three
7272
os validate # green (compare against validate-before.txt)
7373
tsc --noEmit # tombstones type the retired keys as `never`
74-
os migrate meta --from 17 --json # `applied` must be [] (see §3.3)
74+
os migrate meta --from 16 --to 18 --json # `applied` [] and no `replayedNothing` (see §3.3)
7575
# → write .upgrade/REPORT.md (template in §3.4)
7676
```
7777

@@ -159,8 +159,6 @@ it prints:
159159
Layer 2's input, not a warning to dismiss.
160160
- **`Migrated stack is schema-valid`** — or the warning that it is not yet,
161161
which means a residue item is still blocking the parse.
162-
- **Pending data migrations**, when the chain crosses into a major with
163-
per-deployment data gates — see below.
164162

165163
### ⚠ The one fact that surprises every operator
166164

@@ -391,22 +389,25 @@ that output as the control.
391389
> Some conversions are **migration-chain-only**: the loader deliberately does
392390
> not apply them and no tombstone rejects the old shape, because the change is a
393391
> default flip rather than a rename — auto-applying it would stamp a constraint
394-
> onto sources that deliberately omit it. The 16 → 17 crossing has one:
395-
> `field-required-notnull-explicit`, which writes the physical `storage.notNull`
396-
> that `required` used to imply on its own.
392+
> onto sources that deliberately omit it. This build ships none (every
393+
> conversion it carries is tombstoned, so the parse refuses the old shape too),
394+
> but a later one can, and the chain, not the parse, is what knows.
397395
>
398-
> A project carrying only that shape validates **green** while the chain still
396+
> A project carrying only such a shape validates **green** while the chain still
399397
> has work. So `validate` green is necessary and not sufficient, and the
400-
> criterion that closes the gap is the replay:
398+
> criterion that closes the gap is a replay of the whole range you ported:
401399
>
402400
> ```bash
403-
> os migrate meta --from <target-major> --json # `applied` must be []
401+
> os migrate meta --from <authored-major> --to <target-major> --json
402+
> # `applied` must be [] and `replayedNothing` absent
404403
> ```
405404
>
406-
> Read `applied`, not the headline: `--to` defaults to the highest major this
407-
> build carries a step for, which is one PAST the installed major for most of a
408-
> release line, so `todos` carries the next major's semantic residue — real, but
409-
> not this upgrade's business.
405+
> `--from <target-major>` proves nothing: `--to` defaults to the highest major
406+
> this build carries a step for, so the range is empty — no step runs, `applied`
407+
> is `[]` whatever the metadata holds, and the JSON says so with
408+
> `replayedNothing` (its `widestRange` is `null` there: no wider range exists
409+
> from that `--from`) — or it holds only the next major's step. `todos` is
410+
> semantic residue, Layer 2's business, not a failure.
410411
411412
→ Moved verbatim to [references/examples-upgrade.md](./references/examples-upgrade.md) § 3.4 The report — the human half.
412413
@@ -434,7 +435,7 @@ got — "it passed" is a different fact in every row:
434435
|:--|:--|:--|
435436
| Rejected, error carries the fix-it text | A tombstoned key. Not "unrecognized key", not a deprecation label — the prescription *is* the error. | The refusal itself, quoted. |
436437
| Accepted, and the chain rewrites it | A conversion with a live load-path acceptance window. | The chain's diff — not the parse gate. |
437-
| Accepted, and `validate` stays green | A migration-chain-only conversion ([3.3](#33-validate)). | Only the replay from the target major sees this class. |
438+
| Accepted, and `validate` stays green | A migration-chain-only conversion ([3.3](#33-validate)). | Only the acceptance replay sees this class. |
438439

439440
### 3.6 Stored rows: assert them, do not believe them
440441

@@ -460,10 +461,9 @@ recognises it and answers `0`. Nothing gates on it having run.
460461

461462
| Symptom | What it actually is | Fix |
462463
|:--|:--|:--|
463-
| `migrate meta` reports changes, but the files are unchanged | Working as designed — the default run only lists. | Pass `--write`, or port the printed edits by hand; then replay from the target major to confirm 0 changes. |
464-
| Replay from the target major still applies changes | The port is incomplete, or a source builds metadata at runtime from a shape the chain never saw. | Diff against `--out`; grep for the `conversionId`'s surface in code that constructs metadata dynamically. |
464+
| `migrate meta` reports changes, but the files are unchanged | Working as designed — the default run only lists. | Pass `--write`, or port the printed edits by hand; then replay the ported range to confirm 0 changes. |
465+
| The acceptance replay still applies changes | The port is incomplete, or a source builds metadata at runtime from a shape the chain never saw. | Diff against `--out`; grep for the `conversionId`'s surface in code that constructs metadata dynamically. |
465466
| `validate` green, but a feature silently stopped working | An R2 residue item: code reading a renamed key now reads `undefined`. | Exercise the path for real. A green parse says nothing about a `??` chain in the project's own code. |
466-
| `validate` green from the start, so "there was nothing to upgrade" | A migration-chain-only conversion — no tombstone rejects it, so nothing complains. | Replay the chain anyway. `validate` green is necessary, not sufficient; see [3.3](#33-validate). |
467467
| `validate` reports findings that have nothing to do with retired keys | The author-time rule pass, not the schema pass. | Diff against the pre-upgrade `validate` control. Pre-existing findings are not this upgrade's scope. |
468468
| A retired key round-trips without error | The schema carrying it is not strict and the key is being stripped, or the key still has a live load-path window. | Determine which — the two need different acceptance evidence. See [the reverse check](#reverse-check). |
469469
| `--apply` refused / stored-only flag rejected | `--apply`, `--yes`, `--force`, `--type`, `--database-url` mean something only with `--stored`. | Add `--stored`, or drop the flag; the authored-source chain writes only `--out` and, with `--write`, the sources. |

‎skills/objectstack-upgrade/evals/protocol-major-upgrade.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
{
55
"id": 1,
66
"prompt": "把这个项目升级到 v17。objectstack.config.ts 的 manifest 里写着 engines: { protocol: '^16' }。",
7-
"expected_output": "Preflight: `--from 16` read off `manifest.engines.protocol` (the floor of the range, never guessed from the installed spec); keep `os validate > .upgrade/validate-before.txt` as the control. Mechanical: `os migrate meta --from 16 --step`, `--json`, and `--out .upgrade/migrated.stack.json` — the command rewrites nothing on disk, so port the printed `path: from → to (conversionId)` edits into the sources, one `conversionId` per commit, then replay `os migrate meta --from 17 --json` until `applied` is `[]`. Acceptance: `os validate` green, `tsc --noEmit` green, and `.upgrade/REPORT.md` with the mechanical table, decided residue, open decisions and pending per-deployment gates.",
7+
"expected_output": "Preflight: `--from 16` read off `manifest.engines.protocol` (the floor of the range, never guessed from the installed spec); keep `os validate > .upgrade/validate-before.txt` as the control. Mechanical: `os migrate meta --from 16 --step`, `--json`, and `--out .upgrade/migrated.stack.json` — the command rewrites nothing on disk, so port the printed `path: from → to (conversionId)` edits into the sources, one `conversionId` per commit, then replay the ported hop, `os migrate meta --from 16 --to 17 --json`, until `applied` is `[]` with no `replayedNothing` (`--from 17` names an empty range and checks nothing). Acceptance: `os validate` green, `tsc --noEmit` green, and `.upgrade/REPORT.md` with the mechanical table, decided residue, open decisions and pending per-deployment gates.",
88
"files": [],
99
"assertions": {
1010
"must_contain": ["os migrate meta --from 16", "--out", "tsc --noEmit", "REPORT.md", "applied"],
@@ -34,10 +34,10 @@
3434
{
3535
"id": 4,
3636
"prompt": "We bumped @objectstack/spec 16 → 17 and `os validate` is green, so there was nothing to upgrade, right? Production also has `sys_metadata` rows written under 16.",
37-
"expected_output": "No: `validate` green is necessary, not sufficient — migration-chain-only conversions (16 → 17 has `field-required-notnull-explicit`) reject nothing at parse. Replays `os migrate meta --from 17 --json` and reads `applied` (must be `[]`). Stored rows rehydrate through `applyConversionsToStoredItem` only while the installed release carries their conversions (18.3 drops those retired at or below 16.x), so before moving past 18.2 it runs `os migrate meta --stored` (read-only) then `os migrate meta --stored --apply --yes` (`--yes` is mandatory in a non-TTY), and never hand-edits `sys_metadata`. Any per-deployment data-migration gates the run prints go into the report as pending, by name.",
37+
"expected_output": "No: `validate` green is necessary, not sufficient — migration-chain-only conversions reject nothing at parse (this build ships none, but the chain, not the parse, is the authority). Replays the ported hop, `os migrate meta --from 16 --to 17 --json`, and reads `applied` (must be `[]`) with `replayedNothing` absent — `--from 17` alone names an empty range, so its `applied: []` proves nothing. Stored rows rehydrate through `applyConversionsToStoredItem` only while the installed release carries their conversions (18.3 drops those retired at or below 16.x), so before moving past 18.2 it runs `os migrate meta --stored` (read-only) then `os migrate meta --stored --apply --yes` (`--yes` is mandatory in a non-TTY), and never hand-edits `sys_metadata`. Any per-deployment data-migration gates the run prints go into the report as pending, by name.",
3838
"files": [],
3939
"assertions": {
40-
"must_contain": ["applied", "--stored", "--yes", "field-required-notnull-explicit", "applyConversionsToStoredItem"],
40+
"must_contain": ["applied", "replayedNothing", "--stored", "--yes", "applyConversionsToStoredItem"],
4141
"must_not_contain": ["UPDATE sys_metadata"]
4242
}
4343
},

‎skills/objectstack-upgrade/references/examples-upgrade.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ maintainer can read in five minutes and a year from now. Write
4545

4646
**Status:** complete | complete with N open decisions
4747
**Spec:** <installed @objectstack/spec version> · **Chain:** 16 → 18
48-
**Verified:** `os validate` green · `tsc --noEmit` green · replay-from-17 applies 0 mechanical changes
48+
**Verified:** `os validate` green · `tsc --noEmit` green · replay 16 → 18 applies 0 mechanical changes
4949

5050
## 1 · Mechanical (applied by the chain)
5151

5252
| Site | Change | Conversion |
5353
|:--|:--|:--|
54-
| `objects[crm_lead].fields.name` | `required: true` → `+ storage.notNull: true` | `field-required-notnull-explicit` |
54+
| `sharingRules[0].accessLevel` | `'full'` → `'edit'` | `sharing-rule-access-level-full-to-edit` |
5555
| … | | |
5656

5757
_N sites, M conversions. Ported into sources from `os migrate meta --out`._

0 commit comments

Comments
 (0)