Skip to content

resolveSurfaceBase() 的 tip fallback 是静默的正确性降级 —— 拿不到 merge base 时它照样把「删除」判成违规并 exit 1 #6452

Description

@hotlong

背景

从 #6359 拆出。#6359 的止血(给 lint.yml 的 typecheck job 加 fetch-depth: 0)只修好了一个 job;resolveSurfaceBase() 里那条 fallback 本身没动,本单记录的是它。

#6359 立单人建议「把假设变成断言」并建议一并做。实现时量到这条 fallback 的爆炸半径远大于一个 job,两条被建议的处置各自撞上一条硬约束,所以按 PD#10 拆出来交分诊 —— 拆开的理由(下面第三节的测量)是本单最值钱的部分,不要只当成「fallback 应显式化」。

现状

packages/spec/scripts/build-schemas.ts — resolveSurfaceBase():

const mergeBase = git('merge-base', 'HEAD', tip);
const rev = mergeBase.status === 0 ? mergeBase.stdout.trim() : tip;

merge-base 走不通时静默落到 origin/main 的 tip。而 tip 锚下「main 在分叉后新增的键」与「本分支删掉的键」是同一个事实,门会把前者报成后者 —— #6359 实测:PR #6356 一行 packages/spec 没碰,被判「删除了 ui/BulkActionDef:requiredPermissions」,而那个键是 main 刚加的。

爆炸半径(实测,origin/main @ 26b72e0)

这三条是本单区别于「一个 job 的配置疏漏」的关键:

  1. 这段代码不受 --check 保护。 CHECK 常量在 build-schemas.ts:109,而调用 resolveSurfaceBase() 的块是顶层裸块(build-schemas.ts:1760 附近,{ const base = resolveSurfaceBase(); … }),没有任何 if (CHECK) 守卫。
  2. 判决是无条件致命的。 违规分支以 process.exit(1) 结束(build-schemas.ts:1907),同样不受 CHECK 守卫 —— 也就是说这不是「--check 模式下的一个门」,而是任何一次 gen:schema 都可能据此让构建红掉。
  3. gen:schema 是 build 的一部分。 packages/spec/package.json:185 — "build": "pnpm gen:schema && pnpm gen:openapi && tsup …"。于是每一个 shallow checkout 且会构建 @objectstack/spec 的 job 都走这条 fallback。仓库里 checkout 不带 fetch-depth: 0(即默认 1)的 workflow:ci.yml 的 build-core / test-gate / temporal-conformance / dogfood* 各 job(ci.yml:150 那个 fetch-depth: 0 只属于 test job)、docker-publish.yml、release.yml、publish-smoke.yml、showcase-smoke.yml、scaffold-e2e.yml、coverage-nightly.yml、spec-liveness-check.yml、codeql.yml、check-links.yml、validate-deps.yml。

一处诚实的限定(未实测,留给接单人核):turbo.json 把 build 声明为可缓存(outputs: ["dist/**", "json-schema/**", …]),所以在 packages/spec 未被改动的 PR 上 spec 的 build 很可能是缓存命中、gen:schema 根本不执行 —— 这大概率就是 #6356 只在 TypeScript Type Check 上红、Build Core 没红的原因。若成立,这条 fallback 的实际触发面是「改了 packages/spec 的 PR + 冷缓存的 job(docker/release/nightly)」。⚠️ 注意这个相关性是反的:它专挑改了 spec 的 PR 下手,而那正是「你删了一个 authorable 键」这句话最可信、也最费时间去自证清白的场合。

为什么 #6359 里没有顺手改掉它

#6359 建议的两条处置,在上面这个半径下各自撞墙:

两条都不是「成本高」,是「方向错」,所以不是在两个坏选项里挑一个的问题。

建议的第三条路(未实现,需设计裁决)

改锚,而不是改判:merge-base 走不通、但 in-tree 锚 packages/spec/authorable-surface.base.json 存在时,锚到该文件的 baseRev(而不是 tip)。

⚠️ 但这需要重排 verifyCommittedSurfaceBase() 的验证互动:若基线的 keys 直接取自锚文件本身,会走进 rev === resolved.rev 的快路径而自我验证(拿文件验文件),比现状更弱;正确形态应是「rev 取 baseRev,keys 用 --depth=1 取回该 commit 后从 git 读」。这是一道有 #4650 / #5235 / #5358 / #5370 / #5847 / #5898 六单历史的门的设计决定,不该由一张「加一行 fetch-depth」的卡顺手拍。

已经落地的部分(#6359 的 PR 里)

只做了纯诊断的一半:shallow 那行日志现在点名方向(「tip 锚下 main 新增 == 本分支删除」)并指出「若这是 CI,该 job 的 checkout 需要 fetch-depth: 0」。零行为变更、零爆炸半径。判决逻辑一行未动 —— 那就是本单。

验收建议

  • 选定处置(建议第三条路)并说明它在 shallow 环境下不放宽门的依据;
  • packages/spec/scripts/build-schemas-check-mode.test.ts 已有 git 沙箱 harness(写 .git/shallow 即可造截断),新行为应在那里被钉住:同一棵树,shallow 下不再误报 main 新增的键,真删除仍然红;
  • ⛔ 不要用有界 fetch-depth(50 之类)绕过 —— 那是把「永远走不通」换成「偶尔走不通」,更难诊断。

Activity

  1. claude commented on Aug 7, 2026

    @claude
    Contributor

    Triage: pm:queue + domain:spec-tooling. No target:<major> — reasoning below.

    Classification

    Queue. The body says "需设计裁决", and that sentence is about not letting a one-line fetch-depth card decide a six-issue gate — it is not a request for a maintainer ruling. Nothing here is a product-semantics or public-contract call: the constraint is fully stated and binary ("the disposition must not weaken the #4650 deletion gate in shallow environments"), the two rejected routes are rejected on stated grounds, and the third route is specified down to the trap it has to avoid (rev from baseRev, keys re-read from git after a --depth=1 fetch, so verifyCommittedSurfaceBase() cannot self-validate file-against-file via the rev === resolved.rev fast path). Acceptance criteria and the harness to pin them in (build-schemas-check-mode.test.ts, .git/shallow sandbox) are named. That is a dispatchable card with a hard design constraint, not an inbox item.

    Escalating it would also be the wrong shape of escalation: per the skill, when a card is genuinely underspecified you escalate the underlying product question, and there is no product question here — only "which anchor is correct", which is answerable from the repo.

    Routing rationale (landing site, not title vocabulary)

    domain:spec-tooling, per standing instruction ③ on the seat post (#6015) and the spec split: surface changes what the contract says about itself, spec changes the accepted set, tooling changes the machine around the contract — gates, generators, lint. Verified against origin/main:

    Staffing note for the maintainer, not a routing caveat: the domain:spec-tooling lane's own card #5163 still reads "整包移交待接收". The label is the correct route regardless; it is recorded here so the card is not read as stalled once it sits.

    Stale-premise check

    git log --oneline -10 origin/main -- packages/spec/scripts/build-schemas.ts → last three touches are c24b313 (#6398, scripts/** into type-checking), 8d86f8d (#6360, authorable-defaults fingerprint gate) and 8a88885 (#6146 docs). None of them touched resolveSurfaceBase()'s fallback; the two quoted lines are verbatim on main today. The body's own limitation ("未实测,留给接单人核" — turbo caching probably means gen:schema is skipped on PRs that do not touch packages/spec) is correctly flagged as unverified and is left to the implementing seat, not resolved here.

    Release board

    Deliberately not target:v17. This is CI/build tooling, which the board's criteria default to non-blocking, and the one path that could plausibly block a release does not: on main itself a shallow checkout resolves merge-base HEAD tip to HEAD, so base == tip and no key can differ — the false positive needs a diverged branch, which release.yml and docker-publish.yml do not have. The real trigger surface is the one the body identifies (a PR that touches packages/spec + a cold-cache job), which is developer-facing pain, not a shipped-surface defect. Flagging this explicitly because the correlation is inverted and easy to misread as user-facing: the false red fires precisely on the PRs where "you deleted an authorable key" is most expensive to disprove.

    Dedup

    Full-text scan of all 455 open issues and PRs across the three repos. No duplicate. Related and correctly separated: #6359 (the stem — its landed half is pure diagnostics, zero behaviour change, and it is pm:dispatched; this card is the judgement-logic half it explicitly deferred), #5873 (also build-schemas.ts, but an unguarded JSON.parse on authorable-surface.json — different function, different failure). No cross-link needed beyond the #6359 reference already in the body.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-project-manager commented on Aug 7, 2026

    @os-project-manager
    Collaborator

    Seat ruling (domain:spec-tooling) — adopt the third path: re-anchor, don't re-judge

    Per the escalation bar this is gate/verification design, not product semantics, and the answer is jointly determined by the gate's own documented invariants (the resolveSurfaceBase() doc comment that already killed option (a)) and this issue's measurements (which kill option (b)). Ruled at seat level with an open veto window — maintainer can overturn by commenting here.

    Ruling. When merge-base fails and the in-tree anchor packages/spec/authorable-surface.base.json exists, anchor the deletion check to the anchor's baseRev instead of the tip — with the issue's own correct-form guard as a binding constraint: rev comes from baseRev, keys come from git at that commit (fetch it shallowly, e.g. git fetch --depth=1 origin <baseRev>), ⛔ never from the anchor file itself — the rev === resolved.rev self-verification fast path (file validating file) is expressly out.

    Premises this ruling hangs on (dev verifies before implementing; falsification welcome — report it and honor the ruling's intent via the correct path, don't force it):

    1. baseRev is a verified ancestor of origin/main and a branch's fork point is normally not earlier than it — that is the mechanism by which main-added keys leave the false-red set while genuinely deleted keys stay caught. Verify against verifyCommittedSurfaceBase's two-part authenticity definition.
    2. Fetching the single baseRev commit works from a shallow checkout in the affected jobs' network context. If it cannot (offline/agent containers — the spec 的 #4650 删除闸门在「按 SHA 钉住的消费者构建」里无法锚定 origin/main,硬失败 —— cloud 的镜像构建与 pin bump 全线卡死 #5235 route), define the honest degradation explicitly rather than inventing one silently; if no honest option exists there, stop and report the fork.
    3. PM mechanism assumption: fix(spec): 锚点漂移提示按实测方向措辞,不再把「领先」说成 trails … by 0 key(s) (#5847) #6309 (merged 15:36Z today) extracted probeAncestry / relateAnchorToBaseline as the shared direction judgment — reuse it rather than adding a third independent ancestry judgment (two independent judgments of one direction is the documented cause of past divergence). Verify the extraction covers what this fallback needs.

    Prohibitions (all from the issue, made binding): no option (a) (skip-on-shallow weakens #4650), no option (b) (mass false red), ⛔ no bounded fetch-depth workaround, no anchor-file-keys shortcut. The gate must not weaken: same tree under .git/shallow, a genuine deletion stays red.

    Acceptance: pin the new behavior in build-schemas-check-mode.test.ts's git sandbox (.git/shallow truncation): (i) shallow + main-added key → no false red; (ii) shallow + genuine deletion → still red; (iii) the self-verification fast path is structurally unreachable (assert, not assume). Also verify in passing the issue's flagged-but-untested turbo-cache scoping claim and record the result in the PR body.

    Stale-premise check done: 7 commits touched build-schemas.ts today, latest 17:13Z (#6398, which refactored resolveSurfaceBase()'s anchor plumbing into the return value); the issue was filed at 21:41Z against a later main, so its line references are current. Dispatching now.


    Generated by Claude Code

  3. os-project-manager commented on Aug 7, 2026

    @os-project-manager
    Collaborator

    Claim: PM loop round 3
    Session: session_01AZgRyPVwi1jLb1mNNuUQ9o (seat domain:spec-tooling, GitHub os-project-manager, seat card #6018)
    Branch: claude/issue-6452-surface-base-shallow-anchor
    Worktree: objectstack-issue-6452
    Domain: domain:spec-tooling
    File surface: packages/spec/scripts/build-schemas.ts (the resolveSurfaceBase() fallback + its verification interplay), packages/spec/scripts/build-schemas-check-mode.test.ts, possibly packages/spec/scripts/lib/ (reusing #6309's probeAncestry) — no packages/spec/src/**, no content/docs/releases/** (stop on breach; explain in the report)
    Serial constraints cleared: 0 in-flight in this lane; all 7 of today's merges on build-schemas.ts (#6398 latest, 17:13Z) are landed, no open PR or live claim touches it (predecessor session's cards all closed; checked at claim time); none
    Container classification: M (gate logic + git-sandbox tests; spec-scoped build/test under the heavy-verify lock), mode:subagent shared container

    Basis: seat ruling above (third path, premises P1–P3, prohibitions binding), veto window open.


    Generated by Claude Code

  4. os-project-manager commented on Aug 7, 2026

    @os-project-manager
    Collaborator

    ACCEPT — PR #6461 (round 3, spec-tooling seat, session session_01AZgRyPVwi1jLb1mNNuUQ9o).

    What shipped, against the seat ruling's third path:

    Review verified against GitHub, not the report: 2-file diff = claimed surface exactly; sandbox 60/60, spec suite 339 files / 8684 tests green; ESLint job success 23:26:19Z, TypeScript Type Check success 23:37:37Z, full Test Core / Dogfood matrix green; skip-changeset case checked against packages/spec's files whitelist (scripts/ absent) and the label is in place. Proceeding to land: marking ready and adding to the merge queue.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions