Skip to content

feat(client): wire meta.saveItem's ifMatch option to the If-Match OCC header, on both declarations - #12189

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-11713-saveitem-ifmatch
Aug 25, 2026
Merged

feat(client): wire meta.saveItem's ifMatch option to the If-Match OCC header, on both declarations#12189
os-trump merged 2 commits into
mainfrom
claude/issue-11713-saveitem-ifmatch

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#11713

Maintainer ruling on the card (2026-08-25, verbatim 「同意」, accepting option A): ifMatch becomes an optional member of the SaveMetaItemOptions bag #11391 landed, wired to the If-Match request header, on bothsaveItem declarations.

The defect

meta.saveItem's own docstring has always named the ADR-0008 optimistic-concurrency protocol:

The resolved version is the ADR-0008 optimistic-concurrency token: echo it back as the If-Match request header on the next write to the same item and a concurrent edit is reported as 409 metadata_conflict instead of silently overwriting.

Both REST PUT doors read if-match and thread it as parentVersion, so that sentence was true of a raw-HTTP caller. It was false of a first-party SDK caller: neither saveItem declaration accepted a header, an ifMatch, or anything that became one. A caller who did exactly what the docstring said had nowhere to put the token, and their concurrent edit overwrote the other author's write — answered 200, with no signal at the call site. Declared, not enforced.

The change

  • ifMatch?: string joins SaveMetaItemOptions — the same bag, extended rather than a second parameter, so the two clients keep one type between them.
  • Both declarations wired — the unscoped ObjectStackClient.meta.saveItem and the environment-scoped ScopedProjectClient.meta.saveItem, through one shared metaSaveHeaders builder placed beside the existing shared metaSaveQuery. The twins cannot drift in what they put on the wire; that is the same reason the query builder is one function.
  • Purely additive. Only a non-empty token reaches the wire: undefined and '' both omit the header, and the builder returns undefined rather than {} so the call site omits the headers key from the init entirely. An un-pinned save therefore puts exactly the header set on the wire it always did, and last-write-wins stays the default on both the client and the door.
  • The three doc pointers now name a reachable mechanism rather than a bare header (saveItem, publishItem, packages.publishDrafts). Option C — docs going on teaching a move the SDK cannot make — is closed, as the ruling requires; the B fallback (deleting the pointers) was not needed.
constsaved=awaitclient.meta.saveItem('object','customer',doc);// …later, guarded against a concurrent edit:awaitclient.meta.saveItem('object','customer',next,{ifMatch: saved.version});// a stale token now answers 409 METADATA_CONFLICT instead of overwriting

Measurements the ruling asked for

1. The premise, re-verified on the merged ref.packages/client/src/index.ts at 22c42c9b does contain if (opts?.ifMatch) headers['If-Match'] = String(opts.ifMatch); — twice, on data.update (:5007) and data.delete (:5105), not on meta.saveItem. The two saveItem declarations measured at :854 (unscoped) and :5508 (scoped) offered no way to send the header. Premise intact.

2. The existing #11391 pins measure the URL only — confirmed, and no headers pin on saveItem existed. Every assertion in the two [#11391] describe blocks reads fetchMock.mock.calls[i][0] (the URL) plus init.method / init.body. That is why a URL pin cannot see this defect: a swallowed ifMatch leaves the URL byte-identical, which is exactly what those pins assert. The only header-reading helper in the file belonged to the locale suite.

3. Alignment with @object-ui/data-objectstack's MetadataClient — measured, not assumed (objectui at 090927f, packages/data-objectstack/src/metadata-client.ts). The two first-party clients agree on every behavioural dimension:

MetadataClient.savemeta.saveItem (this PR)
memberifMatch?: string on the save-options bagifMatch?: string on SaveMetaItemOptions
headerIf-MatchIf-Match
guardif (options.ifMatch) — falsy omitsif (!options?.ifMatch) return undefined — falsy omits
valueverbatim, unquotedverbatim, unquoted
bag roleone envelope carrying the header and the three query parameterssame, after this change

One spelling difference, reported rather than reconciled: this client wraps the value in String(...), matching its own data.update / data.delete siblings in the same file; MetadataClient assigns it directly. For every value the declared string type admits the two are byte-identical, and fetch stringifies header values regardless — so the behaviour is one behaviour, and the coercion only differs for untyped JS callers passing a non-string. In-file consistency was preferred there, since divergence within one file is the defect class this card is about.

Wording note, also reported rather than reconciled: MetadataClient names the token "the checksum returned by the last read"; this SDK names it "the resolved version". Both are the row's content hash — SaveMetaItemResponseSchema.version is declared as "Content hash of the just-committed body, and the token the ADR-0008 optimistic-concurrency chain runs on", and the repository compares parentVersion against currentHead — so the two names describe one token reached from each client's own surface.

4. ifMatch reaches BOTH doors — unlike the bag's mode. The compound-name twin PUT /meta/:type/:section/:name (rest-server.ts:6805) reads if-match and strips ETag-style quotes at :6868, exactly as the single-segment door does at :5633. So saveItem('object', 'views/all_leads', item, { ifMatch }) is OCC-guarded like any other save. This is called out in the member's doc comment beside mode's COMPOUND NAMES DO NOT STAGE warning, so the next reader does not generalise the wrong way, and it is pinned.

Pins — on the HEADERS the method builds, in both directions

packages/client/src/client.test.ts, three new [#11713] describe blocks, 12 cases. Each asserts the header present with the caller's token when supplied and absent when it is not — a pin asserting only that the option is accepted, or that the URL is unchanged, cannot see a value that goes nowhere.

Covered on both clients: the token on the wire verbatim and unquoted · absent when unpinned, asserted as the exact header set on the wire rather than merely "no If-Match" (byte-identity; the client's private fetch always merges a header object before calling fetchImpl, so an absence claim has to be spelled on the names it emits) · absent for a bag with no token · absent for '' · the URL byte-identical to an unpinned save, proving this is a header and not a smuggled query parameter · riding alongside the three #11391 query parameters without disturbing them · a compound name · the twins in step, pinned and unpinned. Plus an end-to-end case: save, pin the resolved version, and the stale write is refused with the real envelope (code: 'METADATA_CONFLICT', httpStatus: 409 — the envelope pinned in packages/rest/src/rest.test.ts), asserted on the envelope the caller branches on rather than a bare .toThrow().

Reverse verification — the pins go red without the fix

The narrow ablation, not a whole-file revert: the ifMatch member stays declared, metaSaveHeaders stays and still runs — only the two ...(headers ? { headers } : {}), spreads that put the built header on the request were removed. That reproduces the defect exactly as it stood: the option is accepted, and the value goes nowhere.

Mutation confirmed on disk in both directions before the run (an editor's exit code is not evidence — a zero-hit sed exits 0):

DISK CHECK: removed-text count before=2 after=0 (expect 2 -> 0)
DISK CHECK: builder still called: 2 (expect 2 — option still accepted)
DISK CHECK: option still declared: 1 (expect 1)
git diff --numstat -- packages/client/src/index.ts → 0 2 packages/client/src/index.ts

Result — direction red, as predicted:

Tests 7 failed | 203 passed (210)

All 7 failures are [#11713] cases asserting the header is present, on both clients, including the end-to-end conflict loop. No rebuild was involved and none was needed: client.test.ts imports ./index — the source module, not the package exportsdist/ path — so the mutation is on the code the run executes. The script restores the tree from a trap … EXIT INT TERM, and the restore was verified after it fired (0 modified path(s), spread count back to 2).

Two things this measures that a URL pin cannot. Under the same ablation, all fourteen pre-existing [#11391] URL pins stayed green — which is the point: a swallowed ifMatch leaves the URL byte-identical, so the existing pins are structurally blind to this defect. And the five [#11713] cases asserting the header absent also stayed green, correctly: they are regression guards for the un-pinned path, ⛔ not red-before evidence, and are named as such rather than counted.

Verification

Every heavy step ran through scripts/pm/os-verify-lock.sh; verdicts are read from the lock's own VERDICT line, never a bare $?. Run on 7d312be1, the final commit.

Run 1VERDICT command-exit 0 · held the lock 416s (6m56s) · waited 160s (2m40s)

stepresult
pnpm --filter '@objectstack/client^...' build (dependency closure first, on a fresh worktree)BUILD_EXIT=0
pnpm --filter @objectstack/client exec vitest run --maxWorkers=2 src/client.test.tsTEST_EXIT=0Test Files 1 passed (1), Tests 210 passed (210)
pnpm --filter @objectstack/client typecheck (tsc --noEmit && check:test-typecheck)TC_EXIT=0 — "@objectstack/client's test layer compiles under packages/client/tsconfig.test.json; 0 file(s) / 0 error(s) held in test-typecheck-debt.json"

Run 2 — the reverse verification above, then the gate family. Per-gate results are posted in a follow-up comment on this PR.

Gate family re-derived against the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed — the script takes its own change set from the merge base; the --repo assertion held against this checkout's origin). It reported the change set as the three paths in this PR and placed all 167 discovered families. Also run: the convention-triggered set for adds or edits a test file, and this lane's standing full-repo pnpm lint (not narrowed).

Scope, and what was left alone

  • packages/spec read-only, ⛔ packages/rest/src/rest-server.ts untouched (held by feat(rest): mount the compound-name per-item promotion door POST /meta/:type/:section/:name/publish (#11932) #12105), ⛔ no content/docs/releases/ edit, ⛔ no ratchet or debt-ledger edit. The whole diff is packages/client/src/index.ts, packages/client/src/client.test.ts and one changeset. No governed surface is touched.
  • Contract review. This widens the published SDK surface, so the card carries needs:contract-review and it is hung here too. ⛔ Not draft-flipped, ⛔ no auto-merge armed — the gate is cleared by the review chain, and the verdict is recorded on the card, not on this PR.

Filed out of scope

#12181meta.deleteItem sends none of the three carriers the REST reset door reads: no If-Match, no ?state=draft, no ?dropStorage, on either declaration. Same defect shape one method over, found while measuring this one; the sibling MetadataClient.reset already sends the header and the state parameter, and the spec's parentVersion describe text names the header on that exact door. Filed unlabelled and unassigned for triage rather than taken here — three separate widenings of the published surface are a ruling of their own, exactly as this card was reported out of #11391 rather than folded into it.


Generated by Claude Code

`saveItem`'s docstring named the ADR-0008 OCC protocol — echo the resolved
`version` back as `If-Match` — while neither declaration accepted a header or
anything that became one. Both REST PUT doors read `if-match` and thread it as
`parentVersion`, so the instruction was true of a raw-HTTP caller and false of
a first-party SDK caller, whose concurrent edit overwrote and was answered 200.
`ifMatch?: string` joins the `SaveMetaItemOptions` bag on BOTH declarations,
through one shared `metaSaveHeaders` builder beside the existing shared
`metaSaveQuery` — the twins cannot drift. Only a non-empty token reaches the
wire; unset, the `init` carries no `headers` key at all, so every existing
call is byte-identical and last-write-wins stays the default.
Pins assert the built HEADERS in both directions (present with the token,
absent without) on both clients — the pre-existing #11391 pins measure the URL
only, and a URL pin cannot see a value that goes nowhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/client, touching 8 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx(via getItem (sdk), meta.getItem (sdk))
  • content/docs/api/environment-routing.mdx(via /meta/:type/:section/:name (route))
  • content/docs/api/error-catalog.mdx(via /meta/:type/:section/:name (route))
  • content/docs/kernel/contracts/metadata-service.mdx(via /meta/:type/:section/:name (route))
  • content/docs/plugins/adding-a-metadata-type.mdx(via /meta/:type/:section/:name (route))
  • content/docs/protocol/kernel/metadata-service.mdx(via meta.saveItem (sdk), saveItem (sdk))
  • content/docs/protocol/objectql/state-machine.mdx(via /meta/:type/:section/:name (route))
  • content/docs/ui/doc-pages.mdx(via getItem (sdk))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx(via /meta/:type/:section/:name (route))
  • content/docs/releases/v15.mdx(via ifMatch (symbol))
  • content/docs/releases/v17.mdx(via /meta/:type/:section/:name (route))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 177: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 107 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 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 2c4c59ed20caf4f6cde3c979ae9205ee69e04256packageMentionDocs.

Which tree this was computed on

This run read content/docs from eab2c2f998898f0cfdb698c969eea2c93fb8e4c2 — the merge of head cff19f835b0d211132177152b3d167ac5cbd087b into base 2c4c59ed20caf4f6cde3c979ae9205ee69e04256, 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 eab2c2f998898f0cfdb698c969eea2c93fb8e4c2 && git checkout eab2c2f998898f0cfdb698c969eea2c93fb8e4c2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2c4c59ed20caf4f6cde3c979ae9205ee69e04256 cff19f835b0d211132177152b3d167ac5cbd087b && git checkout -B drift-repro 2c4c59ed20caf4f6cde3c979ae9205ee69e04256 && git merge --no-ff cff19f835b0d211132177152b3d167ac5cbd087b
node scripts/docs-audit/affected-docs.mjs --json 2c4c59ed20caf4f6cde3c979ae9205ee69e04256

⚠️ 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 2c4c59ed20caf4f6cde3c979ae9205ee69e04256 → 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 25, 2026
@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Local gate results — 22 green, 1 refusal (declared, not counted)

Run 2 under the shared verify lock: VERDICT command-exit 0 · held the lock 555s (9m15s) · waited 0s. Each gate's exit code was captured before any pipe (each wrote to its own log, then $?), so no tail status is being read as a verdict.

gateexithow it was named
check:nul-bytes0any edit
check:changeset-gate-self-tests0path — .changeset
check:cross-package-test-inputs0path + convention
check:objectui-changeset0path — .changeset
check:published-files0path — packages/*
check:slot-lookup0path — packages/**
check:test-source-alias0path — packages/*
check:type-source-resolution0path — packages/*
scripts/check-adr-0087-registration.mjs0path — .changeset
scripts/check-changeset-no-major.mjs0path — .changeset
scripts/check-ci-filter-parity.mjs0path — packages/client/src/**
scripts/check-cross-package-test-inputs.mjs0path — packages/client/src/**
scripts/check-empty-changeset.mjs0path — .changeset
scripts/check-plugin-teardown-shape.mjs0path — packages/**
scripts/docs-audit/check-affected-docs.mjs0CI trigger — packages/**
scripts/docs-audit/check-drift-comment.mjs0CI trigger — packages/**
scripts/pm/release-rehearsal-clone.mjs --self-test0path — .changeset
check:query-options-erasure0convention — adds test code
check:type-check-coverage0convention — adds test code (structural half)
check:type-check-debt1 — REFUSALconvention — adds test code (ratchet half)
check:engine-double-contract0convention — adds test code
check:where-matcher0convention — adds test code
pnpm lint (eslint . --no-inline-config, full repo, not narrowed)0this lane's standing addition

Two ratchets confirmed their baselines against the merge base rather than merely passing: slot-lookup — "107 unswept site(s) in 25 file(s), none new … baseline key set verified against 22c42c9: no files added"; where-matcher — "0 silently-wrong and 0 unjudged matcher(s) … baseline key set verified against 22c42c9: no files added". check:nul-bytes scanned 6750 tracked text files, 0 raw control bytes.

The one non-zero is a refusal — ⛔ not a failure, and ⛔ not a pass

check:type-check-debt (check-type-check-coverage.mjs --re-measure) threw before measuring anything:

--re-measure cannot run: 25 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk … "measuring now would not fail, it would silently measure a DIFFERENT WORLD" … "Build the closure first, exactly as lint.yml does before this step: pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'"

Its --self-test passed and the structural half (check:type-check-coverage) is green at exit 0; only the ratchet re-measure refused, because this worktree was built with --filter '@objectstack/client^...' — the dependency closure of the package under test, which by construction excludes the package itself and everything downstream. That is exactly the state the gate is written to refuse, and the refusal means NOT MEASURED — never "not applicable". It is being re-run against the full closure the gate names; the result follows here. CI runs it after that same build regardless.


Generated by Claude Code

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

check:type-check-debt — now MEASURED, and green

Follow-up to the refusal in the comment above. The closure was built exactly as lint.yml does before this step, then the ratchet re-run — both under the shared verify lock, one hold: VERDICT command-exit 0 · held the lock 946s (15m46s) · waited 425s (7m05s).

stepresult
pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'FULLBUILD_EXIT=0 — 202 build successes, Time: 9m3.747s
pnpm check:type-check-debtTYPE_CHECK_DEBT_EXIT=0

Its own verdict lines, quoted rather than paraphrased:

check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 399.3s, 1843 raw tsc error(s) total, none above its recorded number.
surplus: none — every entry sits exactly at its measurement, so any new error is red.

So the one gate that could not answer earlier now answers, on the same commit 7d312be1, with tsc resolving every workspace import through built dist/*.d.ts — the world the ledger's numbers were recorded in. "surplus: none" is the sharp half: every entry sits exactly at its recorded number, so this change added no tsc error anywhere in the ledger, and there was no headroom for it to hide in.

Local tally is now 23 of 23 measured, 23 green — nothing left declared unmeasured on this PR. CI still owns the authoritative run of the whole farm.


Generated by Claude Code

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Contract-review verdict: PASS (#11713)

Reviewed by the contract-review chain at CONTRACT_REVIEW_TIER. Tier fuse, machine reading this session: get_session.external_metadata.last_served_model = claude-fable-5.

Reviewed: the full diff, the PR body's evidence, and the ruling record on #11713.

Conformance to ruling A (maintainer 2026-08-25, 「同意」 — option A, quoted in the PR body):

  • ifMatch?: string lands on bothSaveMetaItemOptions declarations, and both routes build their headers through the one shared metaSaveHeaders site — the ruled shape, with the two-declarations drift hazard closed at the builder rather than by discipline.
  • The header pins run in both directions (present when passed, absent when not) across the surface, the compound-door path carries If-Match too, and the narrow ablation went red exactly as predicted in writing — the pins demonstrably bite on the unfixed tree.
  • Additive optional member, no default change: a caller that passes nothing sees byte-identical requests. The widening is declared where it belongs.
  • Out-of-scope finding filed as meta.deleteItem sends none of the three carriers the REST reset door reads — no If-Match, no ?state=draft, no ?dropStorage, on either declaration #12181 rather than ridden — scope discipline held.

Clearing needs:contract-review on this PR and on #11713 in the same stroke. Landing stays with the owning seat.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 25, 2026 15:23
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing completed by the contract-review chain — ready + auto-merge armed (merge queue lands it when CI is green). This supersedes the "landing stays with the owning seat" line in the PASS verdict above, on maintainer instruction (2026-08-25, live PM chat, verbatim and untranslated): 「审核通过你应该直接负责合并吧,还要等原始的项目经理吗」. Standing practice from here: a contract-review PASS on a non-governed code PR completes the landing in the same stroke.


Generated by Claude Code

Merged via the queue into main with commit 6274a1aAug 25, 2026
37 checks passed
@os-trump
os-trump deleted the claude/issue-11713-saveitem-ifmatch branch August 25, 2026 16:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

meta.saveItem's own docstring tells the caller to echo version back as an If-Match header — the SDK offers no way to send one, on either declaration

2 participants

@os-trump@claude