Skip to content

feat(metadata): watch(_, since) replays from sys_metadata_history, and what a bare watch() owes is written down - #11018

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-10842-watch-resumability-face1
Aug 22, 2026
Merged

feat(metadata): watch(_, since) replays from sys_metadata_history, and what a bare watch() owes is written down#11018
os-elon merged 2 commits into
mainfrom
claude/issue-10842-watch-resumability-face1

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10842

Option B per the maintainer ruling (2026-08-22): implement face 1 only, and write down what a watch() with no since owes. A (full replay matching InMemoryRepository) and C (carving engine-backed repositories out of invariant 6) are not taken.

What this buys, stated honestly

It makes the written MUST true. It fixes nothing a user experiences today — no production caller passes a numeric since (enumeration below), so the face implemented here has no consumer but the contract suite itself. That is a defensible buy because runRepositoryContractTests is re-exported through @objectstack/metadata-core/testing, a published exports subpath: the suite is a shipped product, not an internal helper. Please do not read this as a user-facing fix.

The four deliverables

1. Face 1 — a numeric since replays from the durable log.SysMetadataRepository.watch() read since only as a drop filter on live events, so an event that had already committed was unreachable however low since was set — while the repository held a durable per-org event_seq log and already read it org-wide in nextEventSeq(). It now replays through that same find('sys_metadata_history', { where: { organization_id } }) query. The row-to-event mapping was extracted out of history() into a shared rowToEvent() rather than copied — two copies would be two answers to "what is actor when recorded_by is null", and the #4556 answer is exactly the kind that drifts silently when duplicated. The org-wide replay stamps each row's own(type, name), which history() does not need to do because it resolved a single ref first; there is a pin for that.

The live listener registers synchronously, before the durable read is issued, so an event committing mid-read is buffered rather than dropped. It then appears in both the replay batch and the live queue, and a delivered set of seq collapses the pair. A failed durable read is raised to the consumer instead of degraded into a silent live-only tail — #4867's rule one seam over: a cursor we could not read is not a cursor we may invent.

2. The contract-text amendment (packages/metadata-core/src/repository.ts, invariant 6 and the watch() doc). The sentence:

Called with NO since, watch() owes live events only — the events that commit after the subscription is established; an implementation MAY additionally deliver events that had already committed, but a caller MUST NOT rely on it, and a caller that needs the already-committed prefix MUST pass a numeric since (or read history()).

3. The suite's face-2 clause rewritten.watch filters by type and name wrote twice and then opened a watch, expecting the match back — an assertion of a replay the contract does not owe. It is now watch filters by type and name — over the live stream: subscription first, writes after. It asserts the floor every implementation owes, so it is unconditional across all three.

4. declaredDivergences: { resumableWatch: '#10842' } deleted. The pin at contract-suite.ts:496 went red the moment replay landed, which is the mechanism working — it is what told the call site to delete the line.

FINDING — the ruling's suggested literal wording would break two of the three implementations

The ruling said the amendment was "most likely live events only". Written as a hard MUST NOT ("a watch() with no since MUST NOT replay"), that sentence makes two of the three shipped implementations non-conforming:

ImplementationBare watch(filter) behaviourUnder a MUST-NOT wording
InMemoryRepository (packages/metadata-core/src/in-memory-repository.ts)Builds a replay buffer from the whole matching log at subscribenon-conforming
FileSystemRepository (packages/metadata-fs/src/repository.ts)Snapshots this.log.readAll() into replay at subscribe; createWatchIterable filters it by since only when since is a numbernon-conforming
SysMetadataRepositoryLive events onlyconforming

Forbidding it is a behaviour change to two implementations plus whatever leans on them (the metadata-fs tests pass since: 999 precisely to suppress that replay, so it is known and load-bearing there), and that is a separate decision nobody has taken. Requiring it is option A, explicitly declined. So the landed sentence states the guarantee floor and denies reliance on anything above it — which is a strictly stronger statement than the silence it replaces, and the one wording under which the invariant table keeps having no per-implementation columns. Reported rather than quietly chosen, per the dispatch's compensating item 2. The invariant-6 block in repository.ts carries this reasoning inline so the next reader is not left to rediscover it.

Deviation, declared: where the "must fail if it replays without since" control lives

The dispatch asked that face 2's control fail if the repository starts replaying without since. That cannot be a shared-suite clause: under the sentence above, replaying is a MAY, so such a clause would red for InMemoryRepository and FileSystemRepository — a per-implementation column in the one table whose value is having none (the exact reason C was declined). The control therefore lives beside the implementation, in packages/metadata-protocol/src/sys-metadata-repository.contract.test.ts, under SysMetadataRepository — invariant 6, both halves (#10842). It is falsified below, and the falsification also demonstrates the shared suite's blindness to that mutation, which is why the split is necessary rather than convenient.

Evidence

Head 75a71f7a22. Every gate node scripts/pm/dispatch-gates.mjs (no path arguments) named for this change set was run at that commit; exits captured before any pipe.

Face 1's real defect control — red pre-fix, green post-fix. With only deliverable 4 applied (declaration deleted, no implementation change), packages/metadata-protocol, vitest run src/sys-metadata-repository.contract.test.ts:

 × watch(sinceSeq) replays subsequent events then goes live 5009ms
→ Test timed out in 5000ms.
× watch filters by type and name 205ms
→ AssertionError: expected +0 to be 1
Tests 2 failed | 32 passed (34)

Post-fix, same command: Tests 42 passed (42), with both clauses green by name.

All three implementations against the amended table@objectstack/metadata-core was rebuilt first, because metadata-protocol and metadata-fs resolve @objectstack/metadata-core/testing through exports to dist/ with no vitest alias, so an unbuilt suite change is invisible to them:

  • InMemoryRepository (via ../src/contract-suite.js, source): 34 passed
  • FileSystemRepository (via dist): 31 passed
  • SysMetadataRepository (via dist): 42 passed

Falsifications (each: mutate, run, restore, prove git hash-object equals git rev-parse HEAD:PATH, git diff --exit-code 0, porcelain empty):

MutationExpectedObserved
watch() replays even with no since (option A by accident)attach-time guards redexpected [ Array(2) ] to deeply equal [] on all three subscriber shapes, plus a fourth. The shared suite stayed green — the evidence for the split above
drop the replay-to-live dedup on the queue drainduplicate deliveredexpected [ 2, 3, 3 ] to deeply equal [ 2, 3 ]
disable the name filter in matchesFilterrewritten face-2 clause redexpected [ 'a', 'b' ] to deeply equal [ 'a' ] — the rewritten clause is not vacuous

The accidental-option-A check, as a named result. After this change, both production subscribers replay nothing at attach time: MetadataManager.startRepositoryWatch() (repo.watch({}), packages/metadata/src/metadata-manager.ts:2807) and MetadataCache.start() (repo.watch(this.watchFilter), packages/metadata-core/src/cache.ts:113, default filter {}). Pinned as three it.each cases naming those call shapes, and falsified above.

Caller enumeration, re-derived. Production callers of MetadataRepository.watch(): metadata-manager.ts:2807 ({}, no since), cache.ts:113 (this.watchFilter, no since), and layered-repository.ts:178 (layer.repo.watch(filter, since) — pure forward of whatever it was given). No production caller passes a numeric since; the card's assertion holds. Non-repository false positives: chokidar.watch (metadata-fs/src/repository.ts:534, cli/src/commands/dev.ts:546), Redis WATCH (service-cluster-redis/src/kv.ts), and md.watch('*', cb) in plugin-security/src/security-plugin.ts:903, which is MetadataManager's callback API, a different method entirely.

Other suites:metadata-core 165, metadata-fs 56, metadata-protocol 1831 (+10 skipped), metadata 615, and the objectqlwatch() consumers (sys-metadata-repository.test.ts, layered-overlay-integration.test.ts) 42 — all passing.

Changeset

minor on @objectstack/metadata-protocol and @objectstack/metadata-core. Not a pure bug fix in the no-changeset sense: watch(filter, since) changes observable behaviour for any caller passing a number, and the shared suite's filter clause changes shape for anyone running runRepositoryContractTests against their own implementation. The changeset body carries that FROM/TO so an upgrading author can act on it from CHANGELOG.md.


Generated by Claude Code

…ite down what a bare watch() owes
Invariant 6 (resumability) had one written half and one inherited half.
Face 1 — the written MUST. `SysMetadataRepository.watch()` read `since` only
as a drop filter on live events, so an event that had already committed was
unreachable however low `since` was set, even though the repository holds a
durable per-org `event_seq` log and already reads it org-wide in
`nextEventSeq()`. A numeric `since` now replays every logged event with
`seq > since` before any live event, through that same query and the row→event
mapping extracted out of `history()`. The live listener registers before the
durable read starts and a `delivered` set of `seq` closes the replay→live
seam, so an event committing during the read is delivered exactly once. A read
failure is raised to the consumer rather than downgraded to a silent live-only
tail.
Face 2 — the inherited half, now written. `repository.ts` spoke only of
`seq > since`; with no `since` there is no such set, so "no `since` replays
everything" lived only as `InMemoryRepository`'s implementation and the shared
contract suite silently leaned on it. Invariant 6 now states the floor: a bare
`watch(filter)` is owed live events only, an implementation MAY additionally
deliver what already committed, and a caller MUST NOT rely on it. The suite's
filter clause is rewritten to that floor — subscription first, writes after —
because its old shape asserted a replay the contract does not owe.
`declaredDivergences: { resumableWatch: '#10842' }` is deleted; the pin it
swapped in went red when replay landed, which is the mechanism working.
Fixes#10842
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/metadata-core, @objectstack/metadata-protocol, touching 6 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via MetadataRepository (symbol), SysMetadataRepository (symbol))
What this run could not see
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 45204a5e3e96efabd92a128f3d0028b2de99fd3apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 14376970b68791a27d8dc8d6670ceb913c1151a9 — the merge of head 75a71f7a22dbe297bca64645c85a515dae580e7f into base 45204a5e3e96efabd92a128f3d0028b2de99fd3a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 14376970b68791a27d8dc8d6670ceb913c1151a9 && git checkout 14376970b68791a27d8dc8d6670ceb913c1151a9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 45204a5e3e96efabd92a128f3d0028b2de99fd3a 75a71f7a22dbe297bca64645c85a515dae580e7f && git checkout -B drift-repro 45204a5e3e96efabd92a128f3d0028b2de99fd3a && git merge --no-ff 75a71f7a22dbe297bca64645c85a515dae580e7f
node scripts/docs-audit/affected-docs.mjs --json 45204a5e3e96efabd92a128f3d0028b2de99fd3a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 45204a5e3e96efabd92a128f3d0028b2de99fd3a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 22, 2026
@os-elon
os-elon marked this pull request as ready for review August 22, 2026 08:35
@os-elon
os-elon added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit f334d66Aug 22, 2026
32 checks passed
@os-elon
os-elon deleted the claude/issue-10842-watch-resumability-face1 branch August 22, 2026 08:50
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.05s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

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

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

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

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-elon@claude