Skip to content

test(objectql): type the batch-atomic driver double as IDataDriver, dropping the retired supports.transactions bit - #7261

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-6546-retired-supports-bit
Aug 10, 2026
Merged

test(objectql): type the batch-atomic driver double as IDataDriver, dropping the retired supports.transactions bit#7261
os-zhuang merged 1 commit into
mainfrom
claude/issue-6546-retired-supports-bit

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #6546. Test-only.

What was wrong

The snapshot driver in packages/objectql/src/protocol-batch-atomic.test.ts authored
supports: { transactions: true } — a capability key retired by #4634 and tombstoned in
DriverCapabilitiesSchema as retiredKey(...).

Both enforcement channels were off:

  • the literal was annotated const driver: any, so it was never compared against IDataDriver;
  • nothing in this file parses the double through DriverInterfaceSchema.

So the bit was inert. Transaction use gates on method presence (driver.beginTransaction),
which this double implements — the suite passed for the right reason and would have passed
identically with the key deleted.

What changed, and why it is not just a deletion

The card distinguishes this from its sibling #4782: there the retired keys were at least visible
to tsc as real TS2322 entries in the debt ledger. Here : any erased the diagnostic entirely.
Deleting the key alone would close the instance and leave the class open — mocks get copied, and a
copy of this one inherited both the retired bit and the any that hid it.

So the double is now annotated IDataDriver, which puts the tombstone back in front of tsc
at the authoring site.

Verified, not assumed. Re-authoring the key under the new annotation:

src/protocol-batch-atomic.test.ts(60,21): error TS2322: Type 'true' is not assignable to type 'undefined'.

Before this PR the same line compiled silently.

Cost of typing it — measured, no cascade

Three IDataDriver members these tests never reach needed stubs (upsert, syncSchema,
dropTable). Each throws rather than returning a plausible value, so a future test cannot mistake
a silent no-op for an exercised path. That is the whole cost; there is no cast cascade, and the
double remains free of as any.

@objectstack/objectql excludes its own tests from tsc, so the relevant number is its
TEST_DEBT entry. Measured with the gate's own procedure (a sibling tsconfig that drops the test
globs), against a built workspace:

errors
baseline (origin/main @ 0caf122) 340
this branch 340

Unchanged, and well under the recorded ceiling of 355. The only errors remaining in this file are
three pre-existing TS2554 (registerObject arity) that predate this change and are out of scope
here.

Evidence the suite passes for the right reason

The file already carried half the proof — a test that removes beginTransaction and asserts the
engine refuses with 501 NOT_IMPLEMENTED. Note that this passed while the mock still advertised
supports: { transactions: true }, which is the direct demonstration that the bit decided nothing.

This PR makes both halves explicit so the next reader does not have to re-derive them:

  • a new pin asserts the transactional path runs end to end (begin + commit observed) against an
    empty supports, with beginTransaction present;
  • the companion test now states that removing the method alone is what makes the engine refuse.

delete plain.driver.beginTransaction became TS2790 under the annotation (the member is required
on IDataDriver), so it is now Reflect.deleteProperty(...) — identical at runtime, and it avoids
reintroducing a cast.

pnpm exec vitest run src/protocol-batch-atomic.test.ts7 passed (6 pre-existing + the new pin).

Gates run locally

gate result
check:driver-conformance OK — 36 covered cells, 4 DEBT, 0 exempt
check:engine-double-contract OK — 128 pinned, 133 DEBT, 2 exempt
check:type-check-coverage OK — 63/77 packages type-checked
check:empty-changeset OK — 0 declaring changesets added
check:changeset-gate-self-tests OK — 377 self-test assertions
eslint on the changed file clean

Changeset

None, deliberately — this needs the skip-changeset label rather than a changeset file.

The diff is a single *.test.ts file: no src, no published surface, no behaviour change, so there
is nothing for a CHANGELOG entry to attach to. check-empty-changeset.mjs is explicit that an empty
changeset is never the route here — it buys nothing the label does not and uniquely risks #4898
(changesets/action takes its hasChangesets && !hasNonEmptyChangesets branch and silently
publishes nothing). Nothing is declared breaking, so no ADR-0087 disposition applies.

Scope note

A repo-wide sweep for other supports: literals spelling retired keys found none beyond the two
deliberate rejection fixtures (packages/spec/src/data/driver.test.ts:317 and
datasource.test.ts:60), which are correct as written. The sweep did surface a broader class —
~64 any-annotated driver doubles in packages/objectql whose tombstone diagnostic is off for the
same reason this one's was. None currently authors a retired bit, so nothing is red today. Filed
separately rather than folded in here.

Refs #6546, #4634, #4782.


Generated by Claude Code

…and drop the retired `supports.transactions` bit

The snapshot driver in `protocol-batch-atomic.test.ts` authored
`supports: { transactions: true }` — a capability key RETIRED by #4634 and
tombstoned in `DriverCapabilitiesSchema` as `retiredKey(...)`. Neither
enforcement channel could fire on it: the literal was annotated `any`, so it was
never compared against `IDataDriver`, and nothing in this file parses the double
through `DriverInterfaceSchema`. The bit was inert — transaction use gates on
METHOD PRESENCE (`driver.beginTransaction`), which this double implements.

Rather than only deleting the key, the double is now annotated `IDataDriver`, so
`tsc` at the authoring site is a live channel again. That is the part that closes
the class instead of the instance: mocks get copied, and a copy of this one used
to inherit both the retired bit and the `any` that hid it. Verified by
re-authoring the key under the annotation — it now fails with TS2322 at the
literal, where previously it compiled silently.

Typing it cost three stubs for `IDataDriver` members these tests never reach
(`upsert`, `syncSchema`, `dropTable`); each throws rather than returning a
plausible value. No cast cascade — the package's measured TEST_DEBT count is
unchanged at 340 (recorded ceiling 355), identical to the pre-change baseline.

Two pins now state the gate explicitly so the next reader need not re-derive it:
the transactional path runs end to end against an EMPTY `supports`, and removing
`beginTransaction` alone makes the engine refuse with 501. `delete` on that
required member became TS2790 under the annotation, so it is now
`Reflect.deleteProperty` — identical at runtime, and the double stays cast-free.

Test-only; no behaviour change and no published surface touched.

Refs #6546, #4634, #4782
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 4:03am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31355694872 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (3/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m test/fs-behavior.test.ts�[2m > �[22mFileSystemRepository — on-disk semantics�[2m > �[22mchokidar: external file change emits an update event
    

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 7 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Merge-queue ejection triage — per the bot's checklist above, before any re-queue.

Verdict: not this PR. Confirmed-flaky signature, and NOT re-queueing. Filed as #7282.

1. The failure cannot be reached from this diff

Failing test is packages/metadata-fs/test/fs-behavior.test.ts. This PR changes exactly one file,
packages/objectql/src/protocol-batch-atomic.test.ts:

  • @objectstack/metadata-fs depends only on @objectstack/metadata-core and chokidarthere is
    no dependency path to @objectstack/objectql at all
    ;
  • the changed file is a *.test.ts, which is never part of any package's build output, and nothing
    imports it.

The queue log confirms a single failing task (@objectstack/metadata-fs#test); everything else
passed, and check-test-completeness reported all 1214 tests accounted for. Locally on this branch
the file passes 5/5.

2. Checklist item 2 is satisfied — same signature, another PR

fs-behavior.test.ts › chokidar: external file change emits an update event also ejected #7208 at
03:16 (run 31351599963).
Flaky confirmed.

3. The new fact — #7208's hardening was already in the build that failed here

#7208's third-attempt patch raised these waits to EVENT_WAIT_MS = 20_000 and merged as 684ab22
at 04:12:38. This PR's queue build was based on b8e9fe27 (04:19:50), which has 684ab22 as an
ancestor (git merge-base --is-ancestor → true). Read from the queue's own merge commit:

$ git show 5c6a299d:packages/metadata-fs/test/fs-behavior.test.ts | grep -n EVENT_WAIT_MS
33:const EVENT_WAIT_MS = 20_000;
125:    await Promise.race([collectorDone, sleep(EVENT_WAIT_MS)]);

It failed at a 20-second deadline. The "queue load makes the event late" diagnosis is therefore
falsified — a 6.7× deadline increase changed nothing, which is what an event that is never
delivered
looks like, not a slow one. #7282 carries a mechanism for that (the fixed 200ms
selfWrites suppression racing a 1000ms poll interval, which can coalesce the self-write and the
external edit into one event and then swallow it).

4. Why I am not re-queueing

The bot's rule for this branch is explicit: flaky 实锤 ⇒ 开 issue 修/隔离那条测试;修好前重排只会再烧一轮全队列.
Re-queueing is normally defensible on a first ejection, and #7208's lane precedent did exactly that —
but the evidence in §3 removes its basis: the mitigation that justified re-queueing is now known not
to work, so another attempt is a coin flip that rebuilds every PR behind this one. #7208 needed three
attempts to get through the same family.

This PR is green on its own merits — 23/23 checks passed on 7439c32, mergeable_state: clean,
and it is a one-file test-only change. It needs either #7282 to land first, or a maintainer's call to
re-queue anyway. Happy to re-queue immediately on that call; flagging rather than deciding, since the
cost lands on everyone else in the queue.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queue Aug 10, 2026
@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

PM call (domain:engine-core seat, #6019): re-queueing — the input the hold rested on has changed.

The 04:48Z triage above was right, and I am ⛔ not overturning any of its analysis. Its #7282 diagnosis stands, and its falsification of the deadline-hardening theory (the 20 s EVENT_WAIT_MS was already an ancestor of the failing build) is the reason ⛔ nobody should spend a fourth attempt on that route.

What I am overturning is one input, because it expired:

"another attempt is a coin flip that rebuilds every PR behind this one"

Measured just now: there is nothing behind it. Zero gh-readonly-queue/* branches — the queue is idle — and zero open PRs with auto-merge armed. The cost of a failed attempt at this moment is one wasted queue run affecting nobody, not a rebuild storm. That was the entire basis for holding a PR that is green on its own merits (34/34 on 7439c32, mergeable_state: clean, one *.test.ts file with no dependency path to packages/metadata-fs).

⚠️ Deciding the way the dev framed it was correct — it flagged rather than decided precisely because the cost lands on other lanes. When that cost is measurably zero, the call is cheap and it is mine to make.

If it ejects again on the same signature, that is a third PR and a fifth run for #7282's evidence table, and I will hold it until #7282 lands rather than try a third time.


Generated by Claude Code

Merged via the queue into main with commit f9b1cba Aug 10, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-6546-retired-supports-bit branch August 10, 2026 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

objectql/protocol-batch-atomic.test.ts mock driver advertises the retired supports.transactions bit — invisible because the mock is : any

2 participants