Skip to content

docs(adr): ADR-0120 D4 amendment — the duplicate pre-flight is per index class - #11387

Draft
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-11032-adr-0120-d4-per-class-preflight
Draft

docs(adr): ADR-0120 D4 amendment — the duplicate pre-flight is per index class#11387
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-11032-adr-0120-d4-per-class-preflight

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11032

Governed surface — docs/adr/**. This PR stays draft. Review requested from os-zhuang; the human merge IS the review and the audit record. Never flipped ready, never enqueued, no auto-merge armed.

What is wrong

ADR-0120 D4 decides:

os migrate plan gains a duplicate pre-flight probe per affected index

That was written for the declared index class — the recreate_index drift ops the reconciler can see — and it is true there. It cannot reach a second class. Three kernel:ready migrations in packages/metadata-protocol tighten an index at runtime (ensureMetadataOverlayIndexes on sys_metadata, ensureViewDefinitionActiveIndex on sys_view_definition, ensureSysSettingIdentityIndex on sys_setting), and each is invisible to the drift differ by construction: after the tightening isRuntimeManagedIndex excludes the index (isSyncReproducibleIndex is false for a partial index and for a COALESCE key part over a non-tenant column — correctly, or a boot would propose rebuilding away the guarantee it just created), and before it each migration deliberately reuses the DECLARED index's name, so the name-matched slot reads as filled either way.

PR #11031 shipped the behaviour for that class. D4's text did not move with it, so the three modules' doc comments cite D4 while naming os migrate duplicates — accurate about behaviour, inaccurate about D4's text, and the next author reads that as an error in the code rather than a split in the decision.

The ruling this records — quoted verbatim

From the maintainer ruling recorded on #8725 (2026-08-22, decision-inbox digest; the maintainer accepted the batch, verbatim: 「接受所有」). Quoted rather than paraphrased, and the Chinese is kept untranslated:

Ruled: Option B — the read-only reporting path that flags the duplicates: a probe on os migrate duplicates (which already boots read-only and owns the "inventory, never repair" contract) surfaces the three kernel:ready migrations' blocking duplicate rows before the operator restarts the server, keeping os migrate plan's drift contract untouched. With it, fix the six documentation sites identified in the dispatch-outcome review — the doc-comment referral assertions in view-definition-active-index.ts, sys-setting-identity-index.ts, and overlay-index.ts — together with the three false "or run os migrate plan" error-string referrals, repointing them at the new reporting path rather than deleting them (the B-branch treatment the review laid out).

The change

One file, 41 insertions / 1 deletion. The D4 summary row (line 38) and an amendment blockquote at the end of the D4 section — the ADR-0112 amendment convention: the original decision text is left standing, the amendment records what changed and why.

Summary row, after:

| D4 | Tightening migrates through ceremony | recreate_index drift + duplicate pre-flight, routed per index class — declared/differ-visible through os migrate plan, runtime-managed/differ-excluded through os migrate duplicates (2026-08-22 amendment); auto-apply only on a clean probe |

The amendment's operative wording:

Ruled (maintainer, 2026-08-22, on #8725). The pre-flight is per class:

  • declared, differ-visible indexes → reported through os migrate plan, exactly as decided above. Its drift contract is untouched by this amendment.
  • runtime-managed indexes the differ excludes by construction → reported through os migrate duplicates, which boots read-only and owns the "inventory, never repair" contract.

Everything else D4 decides is unchanged and holds for both classes: the previous index stays in place, the report names the key that is not enforced and the rows that block it, and no op is auto-applied on a dirty probe.

The amendment also carries the by-construction invisibility argument and the matched-control measurement (one database carrying the same duplicate damage under both classes; plan named the declared index in full and said nothing about the runtime-managed one) — both already recorded in runtime-index-preflight.ts and view-definition-active-index.ts, now stated where the decision lives.

Route 2 retires the split

The amendment says so in its own last paragraph, and it is worth stating here too: the split exists only because these three platform indexes are tightened at runtime rather than declared. ADR-0120 Route 2 — NULL-safe uniqueness declared in the spec, so a declaration states its own row identity and no runtime migration is needed — was parked to the ADR-0120 / v18 train by #8629's ruling. If Route 2 lands, all three runtime migrations retire, the class collapses back into the declared one, and this per-class split goes with them. This amendment is therefore a record of a temporary shape, with its expiry named.

No source file is touched — verified, not assumed

The claim under test: amending D4 makes the three migration modules' doc comments correct where they already stand. Every ADR-0120 D4 and os migrate reference in packages/metadata-protocol/src/migrations/ was read against the amended wording (read-only; nothing edited):

SiteSaysAgainst amended D4
overlay-index.ts L59-64"reports every failure the way ADR-0120 D4 requires: keep the previous index, name the key that is NOT enforced, ship the exact query …, point at os migrate duplicates, never block the boot"Correct. D4 now routes this class to os migrate duplicates; the rest of the disposition is unchanged and explicitly held for both classes.
overlay-index.ts L66-73"The referral named os migrate plan until #8725, and it was FALSE for this class … ruling 2026-08-22 routes the class to os migrate duplicates"Correct. Reads as history of the code's referral string, and D4's text now agrees with the outcome.
overlay-index.ts L98, L150a re-keying gets "its own ADR-0120 D4 ceremony"Unaffected. The ceremony itself is untouched.
overlay-index.ts L194-201, L317, L377D4's "name the offending rows" / "wording contract"; error string points at os migrate duplicatesCorrect.
view-definition-active-index.ts L114-117"handled the way ADR-0120 D4 requires: the PREVIOUS index stays in place, … points at os migrate duplicates, and the boot continues"Correct.
view-definition-active-index.ts L119-128referral used to name os migrate plan and was FALSE; matched-control measurement; ruling routes the classCorrect, and the amendment now carries the same measurement.
view-definition-active-index.ts L235-241, L341-344, L393-396, L405D4's row-listing / wording contract; os migrate duplicatesCorrect.
sys-setting-identity-index.ts L125-128"the operator then gets ADR-0120 D4's full disposition in one error line … without reaching for os migrate duplicates first"Correct.
sys-setting-identity-index.ts L130-138referral named os migrate plan until #8725 and was FALSE; ruling routes the class, leaving plan's drift contract untouchedCorrect, verbatim the split D4 now states.
sys-setting-identity-index.ts L305-312, L487-490, L556-557D4's row-listing / wording contract; error string names os migrate duplicates and cites D4Correct.
partial-index-probe.ts L34"ADR-0120 D4 requires naming the key that is not enforced and the consequence"Unaffected — the wording contract is unchanged.
runtime-index-preflight.ts L18-56D4 refusal disposition; "os migrate plan cannot carry it … by construction, twice over"; ruling routes to os migrate duplicatesCorrect, and it is the closest prose match to the amendment's argument.

Result: zero comments remain inaccurate, so no source file needs an edit and none was made. ⛔ packages/metadata-protocol/** was not touched — protocol.ts is held by in-flight #11095 under a hard file serial.

The two helpers the amendment names were verified in packages/drivers/driver-sql/src/schema-drift.ts (isSyncReproducibleIndex L895 returns false for index.partial === true and for a COALESCE part whose column is not the tenant field; isRuntimeManagedIndex L916 is true in that case, so the differ skips), and os migrate duplicates exists at packages/cli/src/commands/migrate/duplicates.ts, whose own header records the same 2026-08-22 ruling for its runtimeIndexPreflight section.

Changeset: none, skip-changeset applies

Repo convention for an ADR-only change, checked rather than assumed: comparable merged ADR-only PRs — #10718, #10154, #9233 — each carry documentation + skip-changeset and add no.changeset/*.md. pr-automation.yml's changeset-check has no path-based exemption, so the label is the mechanism. This PR publishes nothing, so it takes the same treatment: label applied additively, then read back.

Verification — all five derived gate families green at 484ae001

Gate set derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed; it took the changeset from the merge base itself: 1 path, docs/adr/0120-…md). Verdict lines are the gates' own:

  • check:adr-anchorscheck-adr-anchors: OK (52 anchored file(s), every governing ADR still referenced; 123 decision number(s) …; 27580 citation(s) across 3415 file(s) resolve). Self-test: ✓ … 74 assertions.
  • check:doc-authoring✓ doc authoring guard: 389 files clean — no bare metadata literals. Self-test green.
  • check:doc-formula-expressions✓ check:doc-formula-expressions: 22 record-scoped formula example(s) across 418 files / 1447 TS blocks judged clean by @objectstack/formula. Self-test: ✓ … 30 cases passed. (Needed pnpm --workspace-concurrency=2 --filter '@objectstack/lint^...' build first — @objectstack/formula's dist/ is its input.)
  • check:pm-governed-merges✓ check-governed-merges --self-test: 129 assertions ….
  • check-adr-links.mjs✅ check-adr-links: 551 relative link destination(s) under docs/adr/ resolve.

Exit codes captured before any pipe (cmd > file 2>&1; EXIT=$?), never through tail.

Repo-level pnpm lint: a proven narrowing to zero, not a skip. The three pieces of evidence, all read from eslint itself rather than from the config globs:

  1. Population, from eslint's own resolution: for docs/adr/0120-…md, ESLint.isPathIgnored() returns true and calculateConfigForFile() returns undefined — the file matches no configuration block at all.
  2. Count, from the run:lintFiles() on the changed file yields 1 result with 0 errors and 1 warning, that warning being File ignored because no matching configuration was supplied. — i.e. 0 linted files in this diff's reachable population.
  3. Invariance for untouched files: type-aware linting is not enabled — for a TS file that is in the population, parserOptions.project and parserOptions.projectService both resolve to null — so no markdown edit can move any untouched file's verdict, and this diff touches no eslint config, baseline or ratchet file.

Control-byte self-scan on the changed file: grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' → no match.

⛔ No behaviour, gate or test was changed. ⛔ content/docs/releases/** untouched. ⛔ No other ADR amended, and no neighbouring text in ADR-0120 tidied.


Generated by Claude Code


Generated by Claude Code

…dex class
D4 decides that `os migrate plan` gains a duplicate pre-flight probe per
affected index. That was written for the declared class — the `recreate_index`
drift ops the reconciler can see — and it is true there. It cannot reach the
three `kernel:ready` runtime index tightenings in `packages/metadata-protocol`,
which are invisible to the drift differ by construction.
The maintainer's 2026-08-22 ruling on #8725 routes that class to
`os migrate duplicates` instead, explicitly leaving `os migrate plan`'s drift
contract untouched. PR #11031 shipped that behaviour; this records the split in
D4's text, where the three migration modules' doc comments already point.
Documentation only: no behaviour, gate or test changes.
Governed surface (docs/adr/**) — draft PR, maintainer merges by hand.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actionsgithub-actionsBot added size/s documentation Improvements or additions to documentation labels Aug 23, 2026
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 23, 2026
@claude
claudeBot requested a review from hotlongAugust 23, 2026 15:13
@os-zhuangos-zhuang self-assigned this Aug 23, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM: review routing corrected to the ruled fallback. Content of the PR is accepted — this is about who gets notified.

The body says "Review requested from os-zhuang"; the actual requested_reviewers is hotlong. Both are repo admins, so nothing improper happened, but neither is the route the skill rules for this exact case.

os-zhuangis the author of this PR, and GitHub refuses a review request to the PR author (author-identity 422). The skill already anticipates that and names the substitute:

能请审则请审;PR 作者身份即 os-zhuang 的席位,请审必失败(GitHub 拒绝向 PR 作者请审,author-identity 422)—— 改为把 PR assign 给 os-zhuang 替代通知,并在轮次报告点名说明走了 assignee 兜底

The maintainer's stated intent when that rule was set (2026-08-19) was the phone notification: 「发给 os-zhuang 审核。我的手机 github 应该会收到推送消息吧」.

Applied: the PR is now assigned to os-zhuang — the ruled notification substitute. ⛔ I did not withdraw the hotlong review request: hotlong is a real admin, adding a route is safe and subtracting one I cannot justify. If hotlong is the wrong human for a docs/adr/** amendment, say so and I will clear it.

Per the same rule, the assignee fallback is named explicitly in this round's report rather than left as a silent substitution.

Nothing else changes: still draft, ⛔ not enqueued, ⛔ not armed, skip-changeset intact, and the human merge remains the review and the audit record.

One note for whoever merges: the PR body's "Review requested from os-zhuang" line is inaccurate as written. It is a body claim on a draft, not a defect in the amendment, so I have not spent a patch round on it — the actual routing is what this comment records.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM correction to my own comment above — the @hotlong request was right, and better-targeted than the generic fallback.

5386732759 said "neither is the route the skill rules for this exact case", which reads as though the reviewer was picked without basis. That is unfair to the dev and I am correcting it on the record rather than leaving it on a governed-surface PR.

The dev's report shows it did the right thing in the right order:

  1. It did request os-zhuang first, as dispatched, and got the exact refusal the skill predicts — 422 Review cannot be requested from pull request author (this seat's token authored the PR).
  2. It then chose @hotlongbecause .github/CODEOWNERS routes this path there, not because they are some available admin.
  3. It flagged the substitution in its report rather than making it silently.

I verified point 2 rather than repeating it: .github/CODEOWNERS:29 is /docs/adr/ @hotlong, and the file's own header quotes the maintainer — 「adr 只能由维护者自己确认,人工合并,ai 不得擅自合并。」

So for docs/adr/** the CODEOWNERS route is more precise than the seat post's generic "request review from os-zhuang", and @hotlong is the correct reviewer here. My assignee addition stands as the notification substitute the skill names, but it is the belt to CODEOWNERS' braces, not a correction of it.

Platform fact worth carrying forward, surfaced by this dev and not previously recorded in this lane: a DRAFT PR does not trigger CODEOWNERS auto-request. Governed-surface PRs are draft by rule, so on exactly the PRs where CODEOWNERS matters most it does not fire on its own — the explicit request is required, and omitting it would leave the PR silently unrouted while looking correctly governed.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM: CI is clean at 484ae001. Ready for the human merge. ⛔ Not flipped ready, ⛔ not enqueued, ⛔ not armed — governed surface, the merge is yours.

Verified job-by-job rather than from the aggregate. Everything is success or legitimately skipped (the filter job correctly skipped the heavy suites for a one-file docs/adr/** diff).

⚠️Two rows read cancelled, and they are not failures — read this before they give you pause:

checkcancelled inbut success in
No other open PR may claim the same single-writer pathrun …831537 @ 15:12:54run …842629 @ 15:13:09
Part-of PR must not also close its cardrun …831557 @ 15:12:53run …842752 @ 15:13:09

Both are superseded duplicate runs, each replaced by a later run of the same-named check that passed. I am naming them explicitly because cancelled is notsuccess — a gate that did not run is not measured, and glossing that as "all green" is exactly the reading error this repo's discipline forbids. These two are measured; they are measured in the newer run.

The five path-derived gate families the dev reported all pass on their own verdict lines: check:adr-anchors (52 anchored files, 27580 citations resolve), check:doc-authoring, check:doc-formula-expressions, check:pm-governed-merges, check-adr-links (551 link destinations resolve).

What you are merging: one file, +41 / −1 — the D4 summary row plus an amendment blockquote, following the ADR-0112 convention that leaves the original decision text standing. No source file was touched, and the PR body carries the per-site verification (12 reference points across five modules) showing why none needed to be: amending D4 makes the three migration modules' doc comments correct where they already stand.

Routing: @hotlong per .github/CODEOWNERS:29 (/docs/adr/ @hotlong), plus os-zhuang as assignee — the notification substitute the skill names, since a review request to the PR author returns 422.


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude