Skip to content

Commit 4c00bd2

Browse files
committed
fix(spec): 重锚 authorable-surface.base.json 改为显式动作,构建不再顺手推进删除门的锚点 (#5358)
packages/spec/authorable-surface.base.json 是 #4650 删除门在无法访问 origin/main 时 使用的基线锚点(#5235)。它不是本包源码的投影,而是某个**上游 commit** 的快照 —— 正因为受测 commit 改不了它,它才有资格当基线。 在此之前,只要它与 git 解析出的基线有差异,每一次 gen:schema 都会重写它。而 gen:schema 是 pnpm build 的第一步,于是任何一个依赖闭包里含 @objectstack/spec 的包 的构建、以及 check:docs,都会触发。三位 dev 在三个互不相关的任务里各自撞上 (#4990 净删 110 键、#5155 同样 110 键、#5660 +3 键),那 110 个正是 #4988/#5321 刚 退役的 ui/ComponentAnimation 族 —— 锚点越过退役点之后,删除门就再也看不见那次退役, 而且推进前后两种状态门禁**都判绿**。三次都只是靠提交前逐行读 git status 拦下的。 改法是给锚点一个属于它自己的模式: - 新增 --update-base(脚本 gen:authorable-surface-base),是唯一写入该文件的路径; - gen:schema 与任何构建都不再写它,滞后只打印一行 ℹ️ 并给出显式命令(滞后本来就不 是错误:main 上 merge base 即 HEAD,该文件必然落后自身 surface 一个 PR); - --check 保持严格只读,缺文件仍然致命;--check 与 --update-base 互斥并在生成前拒绝。 锚点真实性语义完全未动:baseRev 仍须是 origin/main 的祖先且键集与该 commit 一致, 仍只从 git 解析的基线写入(绝不从被检查的构建),写入仍发生在删除门裁决之后 —— 所以显式模式同样无法把基线推过一次未获证明的删除。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01559M8FVm6W6vDLABL3jvdW
1 parent 1624f4a commit 4c00bd2

7 files changed

Lines changed: 465 additions & 46 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): re-anchoring `authorable-surface.base.json` is an explicit act, not a build side effect (#5358)
6+
7+
`packages/spec/authorable-surface.base.json` is the in-tree anchor the #4650 deletion gate
8+
falls back to where `origin/main` is out of reach (#5235). It is not a projection of this
9+
package's source — it is a snapshot of an **upstream** commit, and it is the baseline
10+
precisely because the commit under test cannot rewrite it.
11+
12+
Until now every `gen:schema` run rewrote it whenever it had drifted from the git-resolved
13+
baseline. `gen:schema` is `pnpm build`'s first step, so this fired on any build of any
14+
package that merely has `@objectstack/spec` in its dependency closure, and on `check:docs`
15+
(whose first step is `gen:schema`). Three developers hit it independently, from three
16+
unrelated tasks:
17+
18+
- a plain `pnpm build`: `baseRev` advanced to HEAD, **−110 keys**;
19+
- `pnpm --filter "@objectstack/cli^..." build`: the same 110;
20+
- `pnpm --filter "@objectstack/service-automation^..." build`: `baseRev` advanced, +3 keys.
21+
22+
The 110 were the `ui/ComponentAnimation` family that had just been retired. An anchor
23+
advanced past a retirement cannot see that retirement any more — and the gate is green
24+
before *and* after, because both states are internally consistent. All three were caught
25+
only by reading `git status` line by line before committing; a `git add -A` would have
26+
carried the moved baseline into a PR about something else.
27+
28+
The anchor now moves only in a mode of its own:
29+
30+
```bash
31+
pnpm --filter @objectstack/spec gen:authorable-surface-base # build-schemas.ts --update-base
32+
```
33+
34+
- `gen:schema` and any build: never write it. A lagging anchor prints one ℹ️ line saying so,
35+
naming this command — lag was already not an error (on `main` the merge base is HEAD, so
36+
the file necessarily trails its own surface by one PR).
37+
- `check:authorable-surface`: unchanged, still strictly read-only, and still fatal when the
38+
committed anchor is missing, malformed, or inauthentic.
39+
- `--check --update-base` is refused: a check that repairs what it detects can never report it.
40+
41+
Nothing about anchor **authenticity** changes: `baseRev` must still be an ancestor of
42+
`origin/main` with keys matching that commit's `authorable-surface.json`, the anchor is
43+
still written only from a git-resolved baseline (never from the build being checked), and
44+
the write still happens after the deletion gate has adjudicated the run, so the explicit
45+
mode cannot walk the baseline past an unproven deletion either.

packages/spec/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@
186186
"dev": "tsc --watch",
187187
"clean": "rm -rf dist",
188188
"gen:schema": "OS_EAGER_SCHEMAS=1 tsx scripts/build-schemas.ts",
189+
"gen:authorable-surface-base": "OS_EAGER_SCHEMAS=1 tsx scripts/build-schemas.ts --update-base",
189190
"gen:openapi": "tsx scripts/build-openapi.ts",
190191
"gen:docs": "tsx scripts/build-docs.ts",
191192
"check:docs": "pnpm gen:schema && tsx scripts/build-docs.ts --check",

packages/spec/scripts/build-schemas-check-mode.test.ts

Lines changed: 212 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
// then a faithful model of a build environment with no route to GitHub — the
4848
// last describe block below drives exactly that.
4949

50-
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
50+
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
5151
import { spawnSync } from 'node:child_process';
5252
import fs from 'node:fs';
5353
import os from 'node:os';
@@ -705,7 +705,10 @@ describe('build-schemas.ts — an in-tree anchor carries the deletion gate offli
705705
expect(status).toBe(1);
706706
expect(output).toContain('is not the baseline it claims to be');
707707
expect(output).toContain(SHED_FROM_ANCHOR);
708-
expect(output).toContain('gen:schema');
708+
// The remedy names the one command that may write this file — `gen:schema`
709+
// stopped being it at #5358, and a prescription pointing at a command that
710+
// no longer touches the file is the defect that issue is about.
711+
expect(output).toContain('gen:authorable-surface-base');
709712
},
710713
);
711714

@@ -782,8 +785,8 @@ describe('build-schemas.ts — an in-tree anchor carries the deletion gate offli
782785
);
783786

784787
it(
785-
'is a committed artifact: --check reports it missing without writing it, gen:schema creates it from the git baseline',
786-
{ timeout: SPAWN_TIMEOUT_MS * 2 },
788+
'is a committed artifact: --check reports it missing without writing it, and only --update-base creates it',
789+
{ timeout: SPAWN_TIMEOUT_MS * 3 },
787790
() => {
788791
fs.rmSync(surfaceBasePath);
789792

@@ -793,7 +796,15 @@ describe('build-schemas.ts — an in-tree anchor carries the deletion gate offli
793796
// A check reports; it does not repair (#4711).
794797
expect(fs.existsSync(surfaceBasePath)).toBe(false);
795798

796-
const write = run([]);
799+
// A plain build says so loudly and still does not create it (#5358) — the
800+
// gate above is where a missing committed artifact goes red, not here.
801+
const build = run([]);
802+
expect(build.status).toBe(0);
803+
expect(build.output).toContain('a build no longer creates it (#5358)');
804+
expect(build.output).toContain('gen:authorable-surface-base');
805+
expect(fs.existsSync(surfaceBasePath)).toBe(false);
806+
807+
const write = run(['--update-base']);
797808
expect(write.status).toBe(0);
798809
expect(write.output).toContain('authorable-surface.base.json created at');
799810
const doc = JSON.parse(readSurfaceBase()) as { baseRev: string; keys: string[] };
@@ -803,3 +814,199 @@ describe('build-schemas.ts — an in-tree anchor carries the deletion gate offli
803814
},
804815
);
805816
});
817+
818+
// ─────────────────────────────────────────────────────────────────────────────
819+
// #5358 — the anchor moves only when a human asks for it.
820+
//
821+
// #5235 gave the anchor two properties: it may only be written from a
822+
// git-resolved baseline, and it is verified against origin/main wherever that is
823+
// reachable. Both are about WHAT gets written. Neither says WHEN — and the answer
824+
// was "on every run that regenerates", which meant every `pnpm build`, every
825+
// `pnpm --filter '<pkg>^...' build` whose closure contains @objectstack/spec, and
826+
// every `check:docs` (whose first step is `gen:schema`).
827+
//
828+
// Three developers reported the consequence independently, from three unrelated
829+
// tasks (#4990, #5155, #5660): a file they had never opened showed up modified in
830+
// `git status`, with `baseRev` advanced to the tip they branched from. Twice the
831+
// same run also dropped 110 keys — the `ui/ComponentAnimation` family #4988/#5321
832+
// had just retired. An anchor advanced past a retirement is an anchor that can no
833+
// longer SEE that retirement, and the #4650 deletion gate is green before and
834+
// after, because both states are internally consistent. The only thing standing
835+
// between that and a merged PR was three people reading `git diff` line by line.
836+
//
837+
// The fix is a mode, not a smarter heuristic: `--update-base` writes the anchor,
838+
// and nothing else does. What these tests pin is the negative — that a run which
839+
// is NOT that mode leaves the working tree exactly as it found it — because that
840+
// is the property `git add -A` can silently violate.
841+
describe('build-schemas.ts — only --update-base moves the in-tree anchor (#5358)', () => {
842+
/** A live key held back from the older commit, so the anchor legitimately lags. */
843+
const LAGGING_KEY = 'data/Object:label';
844+
845+
let older: string;
846+
let tip: string;
847+
let laggingAnchor: string;
848+
849+
beforeEach(() => {
850+
// The real-world shape, built honestly: an OLDER upstream commit whose
851+
// baseline is one key short, then the current origin/main tip carrying the
852+
// full baseline. The anchor mirrors the older commit — authentic (its keys ARE
853+
// that commit's baseline) and legitimately behind the merge base, which is
854+
// exactly the state every checkout is in right after a surface change lands.
855+
seedManifest((s) => s);
856+
older = seedBase((s) => s.filter((k) => k !== LAGGING_KEY));
857+
tip = seedBase((s) => s);
858+
seedSurface((s) => s);
859+
laggingAnchor = seedSurfaceBase(older, (k) => k.filter((x) => x !== LAGGING_KEY));
860+
// Commit the fixture so the tree is CLEAN — `git diff --exit-code` is the
861+
// acceptance criterion, and it can only mean something from a clean start.
862+
// Only the two tracked artifacts: `git add -A` here would track the ~1600-file
863+
// json-schema/ output, which every subsequent run rewrites, and the cleanliness
864+
// assertions below would then measure the generator's own scratch space.
865+
git('add', 'authorable-surface.base.json');
866+
git('commit', '-q', '-m', 'fixture: lagging but authentic anchor');
867+
git('update-ref', 'refs/remotes/origin/main', tip);
868+
expect(git('status', '--porcelain', '-uno')).toBe('');
869+
});
870+
871+
afterEach(() => {
872+
git('update-ref', 'refs/remotes/origin/main', tip);
873+
});
874+
875+
it(
876+
'a plain build leaves the anchor byte-identical and the working tree clean',
877+
{ timeout: SPAWN_TIMEOUT_MS },
878+
() => {
879+
// THE regression test. Before #5358 this run rewrote the anchor to
880+
// {baseRev: tip, keys: full} and exited 0, so `git status` showed a file the
881+
// build's author never touched. `gen:schema` is `pnpm build`'s first step,
882+
// which is why an unrelated PR was one `git add -A` away from carrying it.
883+
const { status, output } = run([]);
884+
885+
expect(status).toBe(0);
886+
expect(readSurfaceBase()).toBe(laggingAnchor);
887+
expect(git('status', '--porcelain', '-uno')).toBe('');
888+
// Silent is not enough: the run says the anchor lags, that this is not an
889+
// error, and what the deliberate act would be.
890+
expect(output).toContain('trails the baseline at');
891+
expect(output).toContain('not an error');
892+
expect(output).toContain('gen:authorable-surface-base');
893+
expect(output).not.toContain('⚓');
894+
},
895+
);
896+
897+
it(
898+
'a --check run leaves the anchor byte-identical and the working tree clean',
899+
{ timeout: SPAWN_TIMEOUT_MS },
900+
() => {
901+
// The literal acceptance criterion of #5358: run the gate on a clean tree,
902+
// then `git diff --exit-code`. Unlike the case above this one already held
903+
// on `main` — the anchor's write branch was `!CHECK` before this change too
904+
// — so it is a pin, not a repair. It is here because "a check is read-only"
905+
// is the property the whole file exists for (#4711), and the entry point
906+
// that violated it was the neighbouring one.
907+
const { status } = run(['--check']);
908+
909+
expect(status).toBe(0);
910+
expect(readSurfaceBase()).toBe(laggingAnchor);
911+
expect(git('status', '--porcelain', '-uno')).toBe('');
912+
},
913+
);
914+
915+
it(
916+
'--update-base re-anchors to the git-resolved baseline, and says so',
917+
{ timeout: SPAWN_TIMEOUT_MS },
918+
() => {
919+
const { status, output } = run(['--update-base']);
920+
921+
expect(status).toBe(0);
922+
expect(output).toContain('⚓');
923+
expect(output).toContain(tip.slice(0, 12));
924+
const doc = JSON.parse(readSurfaceBase()) as { baseRev: string; keys: string[] };
925+
// From the merge base, never from this build's own emitted surface (#5235).
926+
expect(doc.baseRev).toBe(tip);
927+
expect(doc.keys).toEqual((JSON.parse(pristineSurface) as { keys: string[] }).keys);
928+
expect(doc.keys).toContain(LAGGING_KEY);
929+
// Restore the fixture for the sibling cases — beforeEach re-commits anyway,
930+
// but a dirty tree between tests would make a failure here read as a failure
931+
// there.
932+
fs.writeFileSync(surfaceBasePath, laggingAnchor);
933+
},
934+
);
935+
936+
it(
937+
'--update-base on an already-current anchor writes nothing and says nothing to do',
938+
{ timeout: SPAWN_TIMEOUT_MS },
939+
() => {
940+
const current = seedSurfaceBase(tip, (k) => k);
941+
git('add', 'authorable-surface.base.json');
942+
git('commit', '-q', '-m', 'fixture: anchor already at the merge base');
943+
944+
const { status, output } = run(['--update-base']);
945+
946+
expect(status).toBe(0);
947+
expect(output).toContain('nothing to re-anchor');
948+
expect(readSurfaceBase()).toBe(current);
949+
expect(git('status', '--porcelain', '-uno')).toBe('');
950+
},
951+
);
952+
953+
it(
954+
'refuses --check --update-base: a check that repairs what it detects can never report it',
955+
{ timeout: SPAWN_TIMEOUT_MS },
956+
() => {
957+
const { status, output } = run(['--check', '--update-base']);
958+
959+
expect(status).toBe(1);
960+
expect(output).toContain('mutually exclusive');
961+
expect(readSurfaceBase()).toBe(laggingAnchor);
962+
expect(git('status', '--porcelain', '-uno')).toBe('');
963+
// Refused before the 1600-schema generation, not after it.
964+
expect(output).not.toContain('Generating JSON Schemas');
965+
},
966+
);
967+
968+
it(
969+
'never writes the anchor from the build being checked — --update-base is powerless offline',
970+
{ timeout: SPAWN_TIMEOUT_MS },
971+
() => {
972+
// #5235's rule survives the new mode: with origin/main unreachable there is
973+
// no git-resolved baseline, so there is nothing the flag may write FROM. A
974+
// `--update-base` that fell back to this build's own surface would be the
975+
// tree anchoring itself — the #4650 defect with a flag in front of it.
976+
git('update-ref', '-d', 'refs/remotes/origin/main');
977+
try {
978+
const { status, output } = run(['--update-base']);
979+
980+
expect(status).toBe(0);
981+
expect(output).toContain('origin/main is not resolvable');
982+
expect(readSurfaceBase()).toBe(laggingAnchor);
983+
expect(git('status', '--porcelain', '-uno')).toBe('');
984+
} finally {
985+
git('update-ref', 'refs/remotes/origin/main', tip);
986+
}
987+
},
988+
);
989+
990+
it(
991+
'still refuses to bless an unproven deletion in --update-base mode, and leaves the anchor alone',
992+
{ timeout: SPAWN_TIMEOUT_MS },
993+
() => {
994+
// Order is load-bearing (see the anchor block in build-schemas.ts): the
995+
// deletion gate adjudicates first, so the re-anchoring mode cannot be used to
996+
// walk the baseline past a deletion nothing proved. Without this, #5358's
997+
// explicit command would be a laundering route the old side effect never was.
998+
// The sabotage goes in the merge base — the anchor's own keys stay honest, so
999+
// only the gate, not the authenticity check, can be what fires.
1000+
seedBase((s) => [...s, DELETED_LIVE].sort());
1001+
seedSurface((s) => s);
1002+
1003+
const { status, output } = run(['--update-base']);
1004+
1005+
expect(status).toBe(1);
1006+
expect(output).toContain('deleted without proof (#4650)');
1007+
expect(output).toContain(DELETED_LIVE);
1008+
expect(readSurfaceBase()).toBe(laggingAnchor);
1009+
expect(output).not.toContain('⚓');
1010+
},
1011+
);
1012+
});

0 commit comments

Comments
 (0)