') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); docs(metadata-protocol,objectql): stop teaching the retired `environment_id` column stamp/filter by claude[bot] · Pull Request #13665 · objectstack-ai/objectstack · GitHub
Skip to content

docs(metadata-protocol,objectql): stop teaching the retired environment_id column stamp/filter - #13665

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13434-environment-id-stale-jsdoc
Aug 31, 2026
Merged

docs(metadata-protocol,objectql): stop teaching the retired environment_id column stamp/filter#13665
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13434-environment-id-stale-jsdoc

Conversation

@claude

@claudeclaudeBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes#13434

Prose only. No behaviour changes, no test moves, and the environmentId option is untouched — its non-comment site counts are byte-for-byte identical before and after (protocol.ts 12, metadata-protocol/src/plugin.ts 9, objectql/src/plugin.ts 6). Verified at 121f2c332c.

What was wrong

Seven passages across three files still documented saveMetaItem stamping an environment_id column on new sys_metadata rows and loadMetaFromDb filtering by it. ADR-0005 (revised 2026-05) / ADR-0006 v4 retired that job when each environment got its own physical database; organization_id is the isolation key that survived.

The danger was not staleness but subject. These sit on the plugin options interfaces — the first thing a reader configuring per-environment scoping meets — and they describe an isolation barrier. An author reading protocol.ts's "every saveMetaItem insert/update and loadMetaFromDb query is filtered by environment_id = environmentId" would believe an environment-level boundary existed inside sys_metadata. It does not.

They also ship: the two options docblocks are emitted verbatim into both packages' published .d.ts on the exported ObjectQLPluginOptions and MetadataProtocolPluginOptions, so this is the editor tooltip a consumer actually reads. That measurement is also why this PR carries a changeset rather than the skip-changeset label — see "Changeset, not skip-changeset" below.

The measurement

environment_id non-comment occurrences, with the card's own filter, at eaba72e48c:

fileenvironment_idpositive control organization_id
packages/metadata-protocol/src/protocol.ts044
packages/metadata-protocol/src/plugin.ts00
packages/objectql/src/plugin.ts00

The zero is a reading, not a broken grep: the control fires at 44 in the same file under the same filter. Stronger than the regex, the complete raw set is small enough to classify by hand — all 12 raw occurrences across the three files sit inside comments, none in code.

Corroborating, in-tree, unchanged by this PR:

  • loadMetaFromDb's actual where-clause is { state: 'active', organization_id: null } — roughly 60 lines below the docstring that claimed an environment_id filter, and its own in-body comment already stated the correct rule.
  • packages/metadata-core/src/objects/sys-metadata.object.ts:138-148@deprecated, "no longer written by saveMetaItem and not consulted by overlay reads".
  • packages/metadata/src/loaders/database-loader.ts:82-89@deprecated, "accepted for back-compat but ignored".
  • packages/metadata/src/loaders/database-loader.test.ts pins the new behaviour.

The seven sites, re-located by needle

The card's coordinates were measured at c813e7ac7 and had drifted by up to +150 lines. Every site was re-found by its quoted prose, never by line number.

#filecard saidactually at eaba72e48cdrift
1packages/objectql/src/plugin.ts63-6962-70~0
1bpackages/objectql/src/plugin.tsnot listed76
2packages/metadata-protocol/src/plugin.ts50-5650-560
3packages/metadata-protocol/src/protocol.ts3976-39884101-4121+125
4packages/metadata-protocol/src/protocol.ts19488-1949019638-19640+150
5packages/metadata-protocol/src/protocol.tsnot listed4492-4493
6packages/metadata-protocol/src/protocol.tsnot listed5086-5091

Three sites beyond the card's four, named explicitly

The card's list was line-anchored and undercounted. Re-running its own git grep as a census turned up three more passages carrying the identical false claim, all in files this PR already touches:

  • Site 5, protocol.ts:4492 — "environmentId keeps its row-scoping jobs — the environment_idstamp/filter and the ADR-0005 overlay-whitelist gate." The same sentence as site 3, in an inline comment.
  • Site 6, protocol.ts:5090, on getProjectId() — "route a read through the protocol's environment_id-filtered lookup." There is no such lookup.
  • Site 1b, objectql/src/plugin.ts:76, the member docstring one line below site 1 — "Scope sys_metadata reads/writes to this project." The same claim in miniature; it contains no literal environment_id, which is why a grep-anchored census missed it.

Leaving site 6 in place would have left "the protocol's environment_id-filtered lookup" standing four lines from a corrected block. All three are the same defect class as the card's four, in the same files, with the correct form already pinned by the evidence above, and no other open PR claims these paths.

The census also found three passages that are already correct and were deliberately left alone, since they are the in-tree corroboration: protocol.ts:5027 ("environment_id is no longer a discriminator"), protocol.ts:14308 ("each env = its own DB, so environment_id is redundant"), and objectql/src/plugin.ts:2017 ("saveMetaItem no longer stamps environment_id").

What the replacements say

Not a bare deletion — that loses the signal for the next reader who wonders whether environment scoping was ever there. Each passage now says what is true now and that the column job was retired, matching the wording already used by database-loader.ts and sys-metadata.object.ts.

environmentId keeps every job it actually has, and the corrected prose lists them from measurement of the 12 live sites in protocol.ts and 6 in objectql/src/plugin.ts: the ADR-0005 overlay-whitelist gate (save and delete doors), the ADR-0010 metadata-lock evaluation, the SchemaRegistry hydration and listing posture, the metadata-service bridge skip, and the local metadata-storage provisioning decision.

Changeset, not skip-changeset

The skip-changeset criterion is "publishes nothing from any package", and a comments-only diff looks like it qualifies. Measured against the built output, it does not: both edited options docblocks appear in the emitted declarations (packages/objectql/dist/index.d.ts:1160+ on the exported ObjectQLPluginOptions, packages/metadata-protocol/dist/index.d.ts:6025+ on the exported MetadataProtocolPluginOptions). A consumer's tooltip changes, so the change is published and a patch changeset is the honest answer.

Verification, at 121f2c332c

  • Comment-only, proven mechanically: every added and removed line in the diff begins with *, // or /*. Nothing else moved.
  • Gate family re-derived from the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (the script reads the change set itself). 31 commands, run with the exit code captured before any pipe. 29 exit 0. The two non-zero are both exit 3 = PREREQUISITE NOT MET, which those gates define as NOT MEASURED rather than a finding: check-test-completeness.mjs grades a saved turbo run test log and none was named, and check:dual-build-cjs-loads needs a whole-tree pnpm build (54 packages have no dist/), which is a repo-wide operation CI owns.
  • ESLint, whole repo, not narrowed: pnpm lint (eslint . --no-inline-config) exit 0.
  • pnpm check:nul-bytes: OK (scanned 7551 text file(s) ... no raw ASCII control bytes).
  • Tests: @objectstack/metadata-protocolTest Files 146 passed | 2 skipped (148), Tests 2040 passed | 10 skipped (2050). @objectstack/objectqlTest Files 248 passed (248), Tests 4287 passed (4287). No test file appears in this diff; nothing was skipped, disabled, quarantined, allow-listed, deleted or re-baselined by this PR.
  • Typecheck: pnpm --filter @objectstack/objectql typecheck exit 0, and tsc --listFiles confirms the edited plugin.ts is genuinely in that program. @objectstack/metadata-protocol carries no typecheck script by design (DEBT ledger); node scripts/check-type-check-coverage.mjs is green, and the package's declaration build reads the file end to end.
  • Build: dependency closures of both packages built before any of the above, so nothing was judged against stale dist/*.d.ts.

Both Test Core shards are expected red on arrival via #13645, which is unrelated to this diff and separately dispatched.

Scope

Not addressed here, and left untouched on purpose: #13416 is not routed for dispatch (still finding, no pm:queue), so packages/metadata-core/src/record-organization.ts is not in this diff. #13433 remains open — it changes code and an index and goes separately. One new finding was filed while measuring this one: #13664, the packages/spec cloud docblock listing sys_metadata as a control-plane table "(with environment_id)", which ships to both the docs site and the published .d.ts; it is not repaired here because it is a different package and the correct wording turns on a fact about the cloud control plane that this tree cannot settle.

Escalation gate: did not fire

Triage armed one: if any current design or review had cited these passages to argue an isolation barrier exists, this stops being a docs card. Searched the whole repo for the distinctive phrases (stamps \environment_id`, ``filtered by environment_id``, environment_id = environmentId, `environment_id = this.environmentId`, `environment_id-filtered`, `stamp/filter`) outside the passages themselves: zero hits. No ADR, doc or review quotes them.


Generated by Claude Code

…ent_id` column stamp/filter
Seven JSDoc/comment passages across three files still documented
`saveMetaItem` stamping an `environment_id` column on new `sys_metadata`
rows and `loadMetaFromDb` filtering by it. ADR-0005 (revised 2026-05) /
ADR-0006 v4 retired that job when each environment got its own physical
database; `organization_id` is the isolation key that survived.
Measured on this tree: zero non-comment `environment_id` occurrences in
the three files, with `organization_id` answering 44 in `protocol.ts`
under the same filter as the positive control. `loadMetaFromDb`'s real
where-clause is `{ state: 'active', organization_id: null }`.
Prose only. The `environmentId` option is untouched: its non-comment
site counts are identical before and after (12 / 9 / 6).
Part of #13434
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/metadata-protocol, @objectstack/objectql, touching 4 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 ObjectStackProtocolImplementation (symbol))

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

  • content/docs/releases/v16.mdx(via ObjectStackProtocolImplementation (symbol))
  • content/docs/releases/v17.mdx(via ObjectStackProtocolImplementation (symbol))

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
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 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; 102 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 — 21 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 575ce836d1c990cb82bb4ba8cb507997d4857a74packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 575ce836d1c990cb82bb4ba8cb507997d4857a74 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

PM verdict: ACCEPT — fully green, releasing

All 32 checks on 3899e15cee are success or skipped, zero failures — including Test Core (1/6) (06:27:21Z), the Test Core rollup (06:27:37Z), Lint & Repo Gates, all four Type Check legs, Governed Surface Queue Guard and Check Changeset.

The central claim, verified independently rather than taken on trust

The PR asserts the diff is comment-only. I checked it hunk by hunk: every added and removed line across all three source files sits inside a JSDoc block (*) or an inline comment (//), and environmentId?: string; appears only as unchanged context. No code line moved.

⭐ The dispatch order's sharpest warning was the one that paid

Zone 2 A2.1 predicted the card's coordinates would be stale and required re-location by needle, never by line number. Measured: the anchors had drifted by up to +150 lines between c813e7ac7 and eaba72e48cprotocol.ts:3976-39884101-4121 (+125), 19488-1949019638-19640 (+150). A line-anchored edit would have landed on unrelated code.

This lane hit line-number rot twice in one night — here, and on #13635's doc-census anchors. Worth carrying forward as a standing reading, not a one-off.

Three things done beyond the brief

  1. The census found seven sites, not four. Re-running the card's own git grep surfaced protocol.ts:4492, protocol.ts:5090 (on getProjectId"route a read through the protocol's environment_id-filtered lookup", describing a lookup that does not exist), and objectql/src/plugin.ts:76. ⭐ The last carries the identical false claim with no literal environment_id, so it was invisible to a grep-anchored census — found only because the seat read the surrounding code rather than trusting the pattern. Leaving site 6 would have left a false sentence standing four lines from a corrected block.
  2. The zero was re-run with its positive control (organization_id answering 44 in the same file under the same filter), then strengthened past the regex: all 12 raw occurrences classified by hand as comment-resident, so no code occurrence exists for a pattern to have missed.
  3. The escalation gate triage armed was actually run. Searching repo-wide for the distinctive phrases returned zero hits outside the passages themselves ⇒ no design or review cites them to argue an isolation barrier exists ⇒ it stays a docs card rather than escalating.

⭐ It argued against the easier path, on measurement

A comments-only diff looks like a clean skip-changeset case. The seat measured that both edited docblocks are emitted into the published declarations (objectql/dist/index.d.ts on ObjectQLPluginOptions, metadata-protocol/dist/index.d.ts on MetadataProtocolPluginOptions) — so a consumer's editor tooltip changes, the diff does publish something, and a patch changeset is the honest answer. Choosing the harder, correct classification against an available shortcut is the right instinct.

Scope held

#13416 untouched (still finding, never routed for dispatch). The liveenvironmentId option untouched — non-comment site counts byte-identical before and after (12 / 9 / 6). #13664 filed for the packages/spec cloud docblock rather than widening this PR, with the bounded in-place exemption deliberately declined because the correct wording turns on a fact about the cloud control plane that this tree cannot settle.

Docs-drift row: checked, and it is a false positive for this defect class

content/docs/concepts/metadata-lifecycle.mdx was flagged only for naming the ObjectStackProtocolImplementation symbol; it does not pair environment_id with sys_metadata. ⭐ And content/docs/references/system/metadata-persistence.mdx is already correct"Deprecated (ADR-0006 v4): legacy environment_id column. New writes leave unset." — so the docs site and these docblocks will agree once this lands. ⛔ The two release-owned pages it flagged are read-only and were not touched.

⚠️ Stated limit: that establishes those pages, not that no page anywhere restates the rule in different wording. Combined with the seat's zero-hit repo-wide phrase search, that is as far as an advisory row is worth taking.

Releasing: draft cleared, auto-merge enabled. ⛔ Not merged by this seat, and no approving review from it.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review August 31, 2026 06:30
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueAug 31, 2026
Merged via the queue into main with commit 9a71af3Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13434-environment-id-stale-jsdoc branch August 31, 2026 06:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mtooling

Projects

None yet

2 participants

@zhuangjianguo@claude