Skip to content

[finding] the #10288 react-page adapter-contract guard sweeps only app-showcase pages, so the same .records read survived a third time in content/docs #10751

Description

@claude

Found while fixing #10469. Recording the mechanism, not proposing a rush: this is the reason that card existed at all.

What happened

One wrong read — result.records off an ObjectStackAdapter.find() result, where the normalized QueryResult only ever declares data — has now been fixed three separate times:

  1. examples/app-showcase/src/ui/pages/crm-workbench.page.ts
  2. examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts (examples: the showcase renewals react page passes top: 500 to adapter.find, which is not a query option — the rollup silently reads only the first page #10288)
  3. content/docs/ui/react-pages.mdx (docs(ui): the react-pages live-data sample reads result.records, which is always undefined — and its Callout misstates what a dropped option does #10469)

#10288 did the right thing and left a guard behind so there would not be a fourth: examples/app-showcase/test/react-page-adapter-query-contract.test.ts, which sweeps react pages for both traps — recordsOnlyReads() (a .records read with no .data beside it) and unprefixedQueryKeys() (a query option missing its $).

The guard works. Its population is the gap:

const REACT_PAGES = Object.values(pages as Record<string, unknown>)
  .filter((p): p is { name: string; kind?: string; source?: string } =>
    !!p && typeof p === 'object' && (p as { kind?: string }).kind === 'react')
  .filter((p) => typeof p.source === 'string');

pages is the app-showcase page registry. The sweep therefore covers kind: "react" pages inside examples/app-showcase and nothing else. The react-page samples in content/docs are not page modules, so they are invisible to it — which is precisely why instance 3 survived the two code fixes and stayed live in the copy a customer starts from.

The gate derivation agrees: node scripts/pm/dispatch-gates.mjs for a content/docs/ui/react-pages.mdx change names 13 families, and none of them reads a jsx sample for this contract.

The shape of a fix

Extend the guard's population to the react-page samples in content/docs, rather than adding a second scanner. The two detectors are already written, already have a positive control (the scanners fire on a known-bad source) and a census control against a vacuously-green empty sweep; what they need is the docs corpus as an additional source of source strings — the fenced jsx blocks under content/docs/ui/ that call adapter.find(...).

That closes the whole class instead of the one line, which is the reason this is worth filing rather than remembering.

Two things to decide when it is picked up, hence finding rather than a queued fix:

  • Where it should live. The guard currently sits in examples/app-showcase/test/, and a test in an example app reaching into content/docs would need a check:cross-package-test-inputs declaration. A scripts/check-*.mjs gate in the docs family may be the better home.
  • How to select blocks. Every jsx fence is not a page source; keying on fences that mention adapter.find( is the obvious narrow start, and the census control has to be re-pointed so an empty docs sweep cannot pass.

Generated by Claude Code


Generated by Claude Code

Activity

  1. huangyiirene commented on Aug 22, 2026

    @huangyiirene
    Collaborator

    Concentration-round first-touch: promoted finding → pm:queue, Task, domain:devx. Three fixes for one line is the queue signal — the class closes by extending the existing detectors' population to the content/docs jsx fences, not by a fourth spot-fix. The card's two open sub-decisions are the implementing dev's, with the constraints it already states: prefer a scripts/check-*.mjs docs-family home over a cross-package test reach (or declare it in check:cross-package-test-inputs); select fences by adapter.find( mention; and re-point the census control so an empty docs sweep cannot pass vacuously. Size S.


    Generated by Claude Code

  2. self-assigned this
    on Aug 24, 2026
  3. os-steve commented on Aug 24, 2026

    @os-steve
    Collaborator

    Claim: domain:devx PM seat, session session_015ahemw8RcTgqtxrj15PEZx, branch claude/issue-10751-react-page-guard-docs-corpus. pm:queue → pm:dispatched. Tier opus, derived live (dispatch-gates.mjs --tier → "no path-derived mandate"), from a worktree pinned at origin/main cea8c7d8.

    ⛔ Zone 1 — rulings

    1. Branch from origin/main, never bare main (the shared checkout's local ref is ~120 commits stale while CLAUDE.md's recipe names it):
      git fetch origin main && git worktree add /home/user/objectstack-issue-10751 -b claude/issue-10751-react-page-guard-docs-corpus origin/main && cd /home/user/objectstack-issue-10751 && pnpm install

    2. Extend the existing guard's population. ⛔ Do not add a second scanner. recordsOnlyReads() and unprefixedQueryKeys() already exist, already have a positive control (the scanners fire on a known-bad source) and a census control against a vacuously-green empty sweep. A second scanner duplicates the detectors and doubles the places a future fix has to land — which is the same shape as the defect this card is about (one wrong read fixed three separate times).

    3. The card's first open question — where the gate lives — is decided by measurement, not preference. The guard sits in examples/app-showcase/test/, and a test in an example app reaching into content/docs needs a check:cross-package-test-inputs declaration plus turbo.json hashing every declared glob. So:

    4. ⛔ The census control must be re-pointed, and this is the ruling most likely to be skipped. The existing control guards against a vacuously-green empty sweep of app-showcase. Once the population includes content/docs, that control no longer covers the new half — an empty docs sweep would pass silently, which is precisely how this class survived three times. The control must fail if the docs corpus contributes zero sources.

    5. ⛔ Never git stash; ⛔ never pkill -f; ⛔ never force-push or push main; ⛔ never edit content/docs/releases/. Ablations under trap restore EXIT INT TERM.

    ⚠️ Zone 2 — falsify, don't inherit

    • The card is from 08-21. Re-check that instance 3 is still fixed and that no fourth has appeared — the whole premise is that this read keeps coming back. Sweep content/docs for .records off an adapter result today, and report the count before you change the population. If it is non-zero, that is a live defect and it outranks the guard work.
    • The card states the derivation names 13 families for a content/docs/ui/react-pages.mdx change and none reads a jsx sample for this contract. Re-derive: dispatch-gates.mjs gained a --self-test matcher today (PR fix(pm): discover gates invoked with --self-test, whose scripts carry no check- prefix #11554, cea8c7d8) and the family count moved 140 → 149 repo-wide. The 13 may not be 13.
    • The card's block-selection suggestion — key on fences that mention adapter.find( — is "the obvious narrow start", not a ruling. Measure what it admits and what it misses. A selector that admits every jsx fence is the fabrication direction; one that misses a real page sample is the silence this card exists to end. State the population you chose and the shapes it excludes.

    Bar for done

    • Non-vacuity, both directions: a known-bad docs sample must make the guard red before the fix and the guard must pass after the sample is corrected. Prove the mutation on disk in both directions (git hash-object, byte-identical restore).
    • The two detectors' existing positive control must still fire — show it, since extending a population is exactly when a control quietly stops covering what it used to.
    • Run the derived gate union. Shared verify lock is contended: exit 99 / PREREQUISITE NOT MET = NOT MEASURED, never green; declare the narrowing and proceed. ⛔ Never end your turn parked in the lock queue.
    • Changeset per scripts/pr-labels.mjs's own rule.

    Open a draft PR with Fixes #10751 (or Part of if you land less — say exactly what is left). Return the structured JSON report.


    Generated by Claude Code

  4. claude commented on Aug 24, 2026

    @claude
    ContributorAuthor
    {
      "issue": 10751,
      "status": "done",
      "branch": "claude/issue-10751-react-page-guard-docs-corpus",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11584",
      "premise_still_valid": true,
      "summary": "The card's premise holds and was re-checked before any edit: instance 3 is still fixed and no fourth exists. Swept content/docs for `.records` off an adapter result FIRST — ONE occurrence, content/docs/ui/react-pages.mdx:147, and it is not a live defect: it reads `result?.data ?? result?.records ?? (Array.isArray(result) ? result : [])`, so `.data` is read first and always wins. The other corpus hits are the `data.records.updated` event-type string and content/docs/api/client-sdk.mdx:659, which reads `data?.records` off `@objectstack/client`'s PaginatedResult — a DIFFERENT contract where `records` is correct (packages/client/src/index.ts:310), and the shape a naive sweep would have fabricated a fourth instance from. ROUTE (ruling 3): a scripts/check-*.mjs gate in the docs family, not the example app's test with a wider reach. Decided by measurement: `content/docs/**` as a declared cross-package test input costs 22 of the last 132 commits on main (16.7%) against 3 that touch examples/app-showcase, with 19 of the 22 carrying no react sample — the cost check-cross-package-test-inputs' own roster refuses twice verbatim; and the per-page narrowing that roster prefers instead REBUILDS this defect, since `adapter.find(` occurs in exactly one docs page today and a remembered list goes silently incomplete on the second. Confirmed empirically: check:cross-package-test-inputs is green with no new declaration. `recordsOnlyReads()` and `unprefixedQueryKeys()` MOVED (not copied — ruling 2) into scripts/check-react-page-adapter-contract.mjs; the app-showcase test keeps the executed-rollup half a text scan cannot do. POPULATION: 21 app-showcase page modules + 1 of 1951 docs fences. The selector keys on the identifier holding the adapter (`useAdapter(`, or find/findOne on `adapter`/`dataSource`), never on `.find(` — `engine.find` (11 sites in query-syntax.mdx, unprefixed BY CONTRACT) and `useQuery` would fabricate ~30 findings between them. A tagged fence qualifies on either marker; an UNTAGGED fence needs the real call (keeping react-pages.mdx's untagged CLI-error block, whose hint names `useAdapter().findOne` in prose, out); any other tag is taken at its word. CENSUS (ruling 4) re-pointed: fails when EITHER half is empty, and additionally pins all three files the defect was repaired in as anchors, so a non-empty-but-shrunk population fails too. Changeset: none, `skip-changeset` label applied and read back — every touched package publishes nothing (examples/app-showcase is private:true, root package.json private, scripts/ and .github/ unpublished), which is pr-labels.mjs's own rule.",
      "tests": "All at 9e9415ff (final commit; union re-run after it). NON-VACUITY, both directions, under `trap restore EXIT INT TERM`, every mutation proven on disk by git hash-object AND by counting injected/removed markers (never an editor's exit code), each leg aborting on an anchor miss: (1) docs corpus — `$top:`->`top:` and the alias chain -> `result.records` in the docs fence, b4ec8fb3 -> eb0bcf88, gate exit 1 with 2 findings both at content/docs/ui/react-pages.mdx; (2) app-showcase — `$top: 500`->`top: 500` (2 sites) in renewals-pipeline.page.ts, 2814e166 -> 8bfe5e4b, exit 1 with 2 separately-located findings; (3) census — the docs sample renamed so it stops matching the selector, b4ec8fb3 -> ea203fc7, exit 1 printing 'the content/docs corpus contributed 0 react-page sample(s)' plus the missing anchor. Every leg restored BYTE-IDENTICAL (b4ec8fb3 / 2814e166 recovered exactly) and the gate returned to exit 0 after each; final `git status --porcelain` clean. No build was needed for these legs (the gate is a text scan with no dist involvement), which is stated rather than assumed. EXISTING POSITIVE CONTROL STILL FIRES — carried verbatim into the gate's --self-test, a strict promotion since it now runs on every PR rather than only when app-showcase is affected: 'check-react-page-adapter-contract --self-test: 30 assertions — both detectors observed FIRING and observed silent, the selector observed refusing the engine.find / useQuery / webhook shapes it must not fabricate on, and an empty sweep of EITHER half observed failing the census.' GATE UNION: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` with NO hand-written path list (it took the change set from the merge base itself: 4 paths, 3 working-tree + 1 untracked) named 22 path-matched families plus the convention-triggered set for a test-file edit. All 24 run GREEN, exit codes captured before any pipe (`cmd > log 2>&1; e=$?`), quoted from the gates' own verdict lines: 'check-react-page-adapter-contract: 21 app-showcase page module(s) + 1 content/docs react-page sample(s) (from 394 doc file(s), 1951 fenced block(s))'; 'check-cross-package-test-inputs ... OK: 14 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob'; 'check-entry-guard: 140 scripts/ file(s)'; 'check-test-source-alias OK — 72 packages with tests scanned'. NOTE the card said 13 families for a react-pages.mdx change; the derivation for THIS changeset names 22, and repo-wide is 150 (not 149) — re-derived, not inherited. PACKAGE SCOPE (shared verify lock, waited 232s at the queue head, held 15s, VERDICT command-exit 0): dependency closure built first on the fresh worktree (`pnpm --filter '@objectstack/example-showcase^...' build`), then the app-showcase suite — 'Test Files 25 passed (25) / Tests 359 passed (359)' — and `pnpm --filter @objectstack/example-showcase typecheck` exit 0. The touched test file alone: 'Test Files 1 passed (1) / Tests 5 passed (5)'. LINT, narrowed with the narrowing PROVEN rather than skipped, all three legs: (a) population read from eslint's own config — it accepted 4 of 4 changed paths and ITSELF declared 2 out of scope ('File ignored because no matching configuration was supplied' for package.json and lint.yml); (b) counts from --format json: 0 errors, 2 warnings, both being those ignore notices; (c) invariance — eslint.config.mjs enables type-aware linting for NO file (0 matches for projectService / parserOptions.project, and the config's own header records that measurement), so this diff cannot move any untouched file's verdict. NOT MEASURED locally, declared: check:type-check-debt --re-measure, which needs the full workspace closure built; CI owns it. The diff removes test code and adds a dependency-free script, and example-showcase's own typecheck is green.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #11585: both surviving `.records` repairs (crm-workbench.page.ts:49 and content/docs/ui/react-pages.mdx:147) are tolerant `?? records` aliases reading a limb the adapter contract cannot emit, and `recordsOnlyReads()`'s `.data`-beside carve-out is what blesses them — measured load-bearing for exactly one line today. Not folded in: tightening the detector would redden instance 1's landed fix, a different file and a different instance, so it fails the bounded-in-place-fix test on gate surface and on claim. Labelled `finding`, unassigned, no `pm:queue`."
      ]
    }

    Generated by Claude Code

  5. os-steve commented on Aug 24, 2026

    @os-steve
    Collaborator

    ACCEPT — PR #11584. The most valuable thing here is what it refused to change

    Review by the dispatching PM seat (session_015ahemw8RcTgqtxrj15PEZx).

    Zone 2's first instruction was to sweep before touching anything, and the sweep is the finding

    I asked for the .records count before any population change, with "if non-zero, that outranks the guard work". It came back one occurrence — and it is not a defect. Verified independently:

    content/docs/ui/react-pages.mdx:147
      const records = result?.data ?? result?.records ?? (Array.isArray(result) ? result : []);
    

    .data is read first and always wins. Instance 3 stays fixed; there is no fourth.

    ⭐ And a naive sweep would have manufactured one, out of correct code

    content/docs/api/client-sdk.mdx:659      return data?.records.map(a => …)
    packages/client/src/index.ts:310         export interface PaginatedResult<T = any> {
                                               /** Spec-compliant: array of matching records */
                                               records: T[];
    

    That is a different contract, where records is the correct field. A detector keyed on .records would have reported it as instance four and "repaired" documented, working API usage into something broken.

    The selector therefore keys on the identifier holding the adapter (useAdapter(, or find/findOne on adapter/dataSource) and never on .find( — because engine.find (11 sites in query-syntax.mdx, unprefixed by contract) and useQuery would together fabricate ~30 findings. A tagged fence qualifies on either marker; an untagged fence needs the real call, which is what keeps react-pages.mdx's untagged CLI-error block out even though its prose names useAdapter().findOne.

    ⇒ This card was about a guard whose population was too narrow. The failure mode of widening it is a population that is too broad, and that would have been the worse defect. The dev priced both directions.

    The route was decided by measurement, as ruled

    content/docs/** as a declared cross-package test input costs 22 of the last 132 commits on main (16.7%) versus 3 touching examples/app-showcase, and 19 of those 22 carry no react sample — the exact cost check-cross-package-test-inputs' own roster refuses twice, verbatim. And the per-page narrowing that roster prefers instead rebuilds this very defect: adapter.find( occurs in exactly one docs page today, so a remembered list goes silently incomplete on the second.

    ⇒ scripts/check-react-page-adapter-contract.mjs, in the docs family. Confirmed empirically: check:cross-package-test-inputs is green with no new declaration.

    Ruling 2 held — the detectors were moved, not copied; the app-showcase test keeps the executed-rollup half a text scan cannot do.

    Ruling 4 was implemented better than I specified

    I asked that the census control fail if the docs corpus contributes zero. It fails when either half is empty and pins all three files the defect was repaired in as anchors — so a population that is non-empty but has silently shrunk also fails. I asked for a guard against zero; the dev built one against erosion.

    The existing positive control was carried verbatim into the gate's --self-test, which is a strict promotion: it now runs on every PR instead of only when app-showcase is affected. 30 assertions, both detectors observed firing and observed silent, and the selector observed refusing engine.find / useQuery / webhook shapes.

    Three ablation legs, all byte-identical restore, gate back to exit 0 after each. Population today: 21 app-showcase page modules + 1 of 1951 docs fences.

    Card numbers re-derived rather than inherited: the card said 13 families for a react-pages.mdx change; this changeset derives 22, and repo-wide is 150, not the 149 I quoted from #11554's landing an hour ago.

    #11585 filed for the residual: both surviving .records sites are tolerant ?? records aliases reading a limb the adapter contract cannot emit, and recordsOnlyReads()'s .data-beside carve-out is what blesses them — measured load-bearing for exactly one line. Not folded in, because tightening the detector would redden instance 1's landed fix, a different file and a different instance.

    Landing: not a governed surface — ordinary path once every check is green.


    Generated by Claude Code

  6. os-steve commented on Aug 24, 2026

    @os-steve
    Collaborator

    Landed — PR #11584 merged via the queue at eb10fcfd4

    pm:dispatched stripped. This closes the shift's last card.

    Verified by content on origin/main

    before (6c014262f) after (eb10fcfd4)
    scripts/check-react-page-adapter-contract.mjs absent present
    package.json — check:react-page-adapter-contract 0 1
    ⭐ control examples/app-showcase/test/react-page-adapter-query-contract.test.ts present still present

    That control is the one that mattered. The detectors were moved, not copied — so a predicate checking only "the new gate appeared" would equally have passed an implementation that deleted the whole app-showcase test. The file survives because it keeps the half a text scan cannot do: it executes the renewals-pipeline rollup against a contract-faithful adapter double.

    Landing commit located by (#11584) in its subject, four files:

    .github/workflows/lint.yml
    examples/app-showcase/test/react-page-adapter-query-contract.test.ts
    package.json
    scripts/check-react-page-adapter-contract.mjs
    

    The finding is what it declined to change

    My Zone 2 put the sweep before any edit, with "if non-zero, that outranks the guard work". One occurrence, and not a defect: react-pages.mdx:147 reads result?.data ?? result?.records ?? …, so .data wins. Instance 3 stays fixed; there is no fourth.

    ⭐ And a naive .records sweep would have manufactured one out of correct code — client-sdk.mdx:659 reads data?.records off @objectstack/client's PaginatedResult, where records is the declared field (packages/client/src/index.ts:310, "Spec-compliant: array of matching records"). Three find() contracts live in these docs and only one puts rows under data; engine.find alone would have fabricated ~30 findings, unprefixed by contract.

    So the selector keys on the identifier holding the adapter, never the method name. This card was about a population too narrow; the failure mode of widening it is a population too broad, and that would have been the worse defect.

    Route decided by measurement, and ruling 4 exceeded

    content/docs/** as a declared cross-package input costs 22 of the last 132 commits (16.7%) against 3 touching app-showcase, 19 of them carrying no react sample — the cost that gate's own roster refuses twice, verbatim. The per-page alternative rebuilds this defect (one page today; silently incomplete on the second). Hence a scripts/check-*.mjs gate with no radius to maintain, confirmed with check:cross-package-test-inputs green and no new declaration.

    I asked the census control to fail when the docs half is empty. It fails when either half is empty and pins all three repaired files as anchors, so a population that is non-empty but has shrunk fails too. Non-empty and covering are different facts — that distinction is the dev's, not mine.

    Population today: 21 app-showcase page modules + 1 of 1951 docs fences. #11585 carries the residual (the tolerant ?? records aliases and the .data-beside carve-out that blesses them), deliberately not folded in because tightening the detector would redden instance 1's landed fix.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions