Skip to content

test(rest): opt the OS_REST_LOG suite down, with every fault-log observer declaring its own level and a pairing gate - #18090

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-17865-rest-log-suite-opt-down
Sep 14, 2026
Merged

test(rest): opt the OS_REST_LOG suite down, with every fault-log observer declaring its own level and a pairing gate#18090
os-warren merged 4 commits into
mainfrom
claude/issue-17865-rest-log-suite-opt-down

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #17865

Decision batch #128 item 4 ruled option B. All four items of that ruling land here, and items 1 and 3 are in one PR by the ruling's own instruction: 「It lands in the same PR as 1; ⛔ 1 does not merge without 3.」

⚠️ The card's own body argues for option A; it was written before the ruling and is stale on the decision. The ruling comment 5651668684 is the authority. Its measurements are good and were re-taken here rather than inherited.

What landed

  1. packages/rest/vitest.config.ts opts the suite downOS_REST_LOG: 'silent', in all three blocks. A root-level env is inert for a project run, so both inline projects carry it too. 'silent' is the quiet level and not 'error': logError's own rank is error, so at 'error' the shim still speaks and 100% of the frame population survives.

  2. The 29 fault-log observers declare their own level'info', the SHIPPED default, so each file asserts against what a real caller gets:

    beforeAll(() => { vi.stubEnv('OS_REST_LOG', 'info'); });
    afterAll(() => { vi.unstubAllEnvs(); });
  3. scripts/check-rest-log-spy-declared.mjs makes an undeclared observer a finding by name — 23-case self-test, both directions per rule, wired additively (one package.json script, one lint.yml step). Seam location and the level vocabulary are imported from check-rest-log-declared.mjs rather than re-implemented, so a renamed level moves both gates at once.

  4. The shipped default is untouched. REST_LOG_DEFAULT_LEVEL is still 'info' in packages/rest/src/log.ts, still pinned by check:rest-log-declared (green here). Nothing in this PR changes production behaviour.

The population, re-measured

The claim comment flagged one instrument bound: git grep -l "console.error" returns 4, against the card's "~15 files / 28 assertions". Both readings are correct and they measure different things — 26 of the 29 observer files spell the spy vi.spyOn(console, 'error'), which contains no literal console.error substring at all. 4 was a floor of a different measurement, exactly as the claim said.

reading value
test files in packages/rest 193
observers (spy on console.error/warn, install a mock by assignment, or import the shim) 29
of those, spyOn(console, 'error') 26 files / 29 occurrences
of those, spyOn(console, 'warn') 14 occurrences
files that go RED at 'silent' with no per-file declaration 15
test cases that go RED 28
files carrying a NEGATIVE fault-log assertion ("logs NOTHING") 8 (18 assertion lines)
of those 8, inherit the suite level and would go vacuous 7
of those 8, already immune because it sets the level per case 1 — rest-log-declared-level-seam.test.ts
rest-expected-error-logging.test.ts alone 4 expect(unhandledLogs()).toHaveLength(0) + 1 sibling

The control that makes 15/28 a reading rather than a probe that could only answer one way: the same 191 files, the same command, at 'info'191 passed, 3,196 tests, 0 failures. So the reds are attributable to the level and not to the tree. And the 15 red files are a strict subset of the 29 observers, which is what says the observer detector is not under-counting: no file outside the detected population goes red.

Volume

Same command (pnpm test in packages/rest), same 191 files, three states:

state files / tests captured lines indented at frames
'info' (shipped default, i.e. main today) 191 passed / 3,196 5,705 2,095 (36.7%)
'silent', no per-file declarations 15 FAILED / 28 failed 2,805 0
'silent' + the 29 declarations (this PR, at 6d699ac6f) 191 passed / 3,196 2,508 (−56.0%) 76 (−96.4%)

The card's 2,095 / 36.7% reproduced exactly.

The residual 76 frames are honest and attributable: every one of them belongs to two declaring files — analytics-filter-refusal-envelope.test.ts (23 own-file frames, identical to its baseline count) and rest-endpoint-surfaces-served-only.test.ts (3) — which log through a window their spy does not cover. That residue is also the leak control: if a per-file vi.stubEnv leaked across files sharing a worker process, non-declaring files would still emit. They emit zero — rest-share-user-message-bypass-exits.test.ts 145 to 0, execctx-consumer-census.test.ts 112 to 0.

Why item 2 is load-bearing — a four-leg ablation

The 15 red files announce themselves. The other half does not, and that is the half the card measured and refused as option C. Ablation on rest-expected-error-logging.test.ts, mutating the production predicate isExpectedRouteError to return false so that every expected 4xx logs [REST] Unhandled error loudly:

leg declaration production the 5 "logs NOTHING" cases
L1 present pristine PASS (7/7 green)
L2 present MUTATED FAIL — a real assertion about the code
L3 REMOVED MUTATED PASS — vacuous
L4 REMOVED pristine PASS — identical verdict to L3

L3 against L4 is the whole point: with the declaration gone, those assertions return the same verdict whether the code logs nothing or logs everything. They are then true of a shim that never spoke. Each leg proved its mutation reached the disk by counting the exact text it touched (a bare git diff --stat would go green on any other edit in the same round), and both files were restored from HEAD and verified byte-identical by git hash-object against the HEAD blob — not by a git diff HEAD that a checkout-from-ref can leave clean.

Gates

node scripts/pm/dispatch-gates.mjs --commands was derived against the actual 33-path change set, all 98 commands were run with exit codes recorded to disk before anything read them, and the run was reconciled with --ran.

  • 96 of 98 exit 0.
  • pnpm lint — the FULL union, not a narrowing — exit 0 at the final head 6d699ac6f, 88s. No narrowing was claimed because none was needed.
  • pnpm --filter @objectstack/rest typecheck exit 0; check:test-typecheck reports 0 file(s) / 0 error(s) in test-typecheck-debt.json, so the 29 edited test files moved no TEST_DEBT number.
  • 2 commands exit 3 — PREREQUISITE NOT MET, which is NOT MEASURED and not a finding: check:dual-build-cjs-loads and check:type-check-debt --re-measure both refuse without a built dist/ for the whole workspace (43 and 6 packages named respectively). That prerequisite is CI's Build Core / lint-job build; declared here rather than absorbed. The half of check:type-check-debt my diff can actually move is the packages/rest test layer, and that is measured green above.

One gate reported a defect in this change, and it was fixed rather than weakened. check:pm-dispatch-gates's own self-test named scripts/check-rest-log-spy-declared.mjs as an unlisted repo-root sweeper: the gate walks the workspace to locate the seam but carried no path literal, so no card could ever be told it reads their surface. Repaired the honest way — it now declares ROOT_DIR_WATCH_HINTS in the sibling's idiom, with five live-tree self-test cases holding the declaration against the walk it actually performs plus an anti-vacuity floor on the observer population (battery 18 to 23). check:pm-dispatch-gates, check:declared-population-live and check:watch-hint-literal are all green after it.

The new gate found nothing beyond the ruling's population29 of 193 test file(s) … every one of them declares its own OS_REST_LOG level. No overflow to report.

Changeset: skip-changeset, measured

packages/rest's files[] is ["dist","README.md","CHANGELOG.md"]. Built the package and grepped those paths with a positive control:

symbol hits in the published paths
REST_LOG_DEFAULT_LEVEL (POSITIVE CONTROL — must be present) 2 files
unhandledLogs (only in a changed test file) 0
stubEnv (the declaration this PR adds) 0
check-rest-log-spy (the new gate) 0
any *test* file in dist/ 0

The remaining changed paths are the repo-root package.json (private), .github/workflows/lint.yml and scripts/ — none published by any package. Control hits, subject symbols zero ⇒ no published artefact moves ⇒ skip-changeset.

Acceptance notes

Observations from the surface, noted and deliberately not filed — neither is a reproducible defect, a contract violation, or a metadata-authoring trap:

  • noted, not filed: the brace-matching and env-block readers are now in a third copy across check-registry-log-declared.mjs, check-rest-log-declared.mjs and this gate. The sibling's header already records the extraction as the right follow-up and states why it did not do it (moving that gate's self-test battery floor). This PR imported what mattered (seam location, vocabulary) instead of copying it, so the duplication did not grow in the load-bearing direction. Carrier if it is ever done: whichever PR next edits check-registry-log-declared.mjs's reader.
  • noted, not filed: two declaring files still emit 76 stack frames because their spy does not cover the window in which the fault is logged. Tightening each spy's window would take the suite to 0, but it edits assertions the ruling did not name, on files that are not defective — the frames are correct output at a correctly declared level. Carrier: a future volume card on this suite, if one is filed. No such card exists today.

Related: #15484's execution list reads ruling comment 5651668684 as the authority for its opt-down sentence. That card remains open and is out of scope here.


Generated by Claude Code

Decision batch #128 item 4 ruled option B on this card: the packages/rest
suite is opted down, every test file that observes the fault log declares
its own level, and a pairing gate makes an undeclared observer a finding.

This commit is item 1 alone, committed on purpose so the red population it
produces can be measured against a real base before item 2 lands. The
shipped default in packages/rest/src/log.ts is untouched and still
gate-pinned at 'info'.

Claude-Session: https://claude.ai/code/session_01TbSMtGzMrtPwh925wDEZd5
Co-authored-by: Claude <noreply@anthropic.com>
Ruling item 2. The 29 test files in packages/rest that observe the fault
log -- a console.error/warn spy, or an import of the shim itself -- now
declare 'info', the SHIPPED default, in their own setup instead of
inheriting the suite's level.

Measured on the committed opt-down, same command, same 191 files:
  OS_REST_LOG 'info'   -> 191 passed, 2,095 indented `at ` frames
  OS_REST_LOG 'silent' -> 15 files / 28 tests RED, 0 frames
  + these declarations -> 29/29 observer files green again

Claude-Session: https://claude.ai/code/session_01TbSMtGzMrtPwh925wDEZd5
Co-authored-by: Claude <noreply@anthropic.com>
…ding

Ruling item 3, the half that keeps item 2 true. A test file in the package
that owns the OS_REST_LOG seam is an OBSERVER when it spies on
console.error/warn, installs a mock by assignment, or imports the shim; an
observer must declare OS_REST_LOG in code, at a level loud enough that both
shim sites still emit.

Seam location and the level vocabulary are imported from
check-rest-log-declared.mjs rather than re-implemented, so a renamed level
moves both gates at once. Zero observers in the owning package is a
MEASUREMENT FAILURE, not a clean bill: this detector recognises the
spellings it knows, and a gate that finds nothing passes everything.

18-case self-test, both directions per rule. Wired additively into the root
package.json and lint.yml.

Claude-Session: https://claude.ai/code/session_01TbSMtGzMrtPwh925wDEZd5
Co-authored-by: Claude <noreply@anthropic.com>
check:pm-dispatch-gates named the new gate as an unlisted repo-root sweeper:
its source walks the workspace to locate the seam but carried no path
literal, so no card could be told it reads their surface. It now declares
ROOT_DIR_WATCH_HINTS in the sibling's idiom, and five live-tree self-test
cases hold the declaration against the walk it actually performs plus an
anti-vacuity floor on the observer population.

Self-test battery 18 -> 23 cases.

Claude-Session: https://claude.ai/code/session_01TbSMtGzMrtPwh925wDEZd5
Co-authored-by: Claude <noreply@anthropic.com>
@os-warren os-warren added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 14, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 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 — 14 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 b3d6918b86abcc31f82e963d2fabc37308bc4bb7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0233f381b0c5240a98467fc2af98e978ec3c8ad7 — the merge of head 6d699ac6fbc49e6135ee89f11c793675fc2e99c1 into base b3d6918b86abcc31f82e963d2fabc37308bc4bb7, 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 0233f381b0c5240a98467fc2af98e978ec3c8ad7 && git checkout 0233f381b0c5240a98467fc2af98e978ec3c8ad7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b3d6918b86abcc31f82e963d2fabc37308bc4bb7 6d699ac6fbc49e6135ee89f11c793675fc2e99c1 && git checkout -B drift-repro b3d6918b86abcc31f82e963d2fabc37308bc4bb7 && git merge --no-ff 6d699ac6fbc49e6135ee89f11c793675fc2e99c1

node scripts/docs-audit/affected-docs.mjs --json b3d6918b86abcc31f82e963d2fabc37308bc4bb7

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

@github-actions github-actions Bot added size/l ci/cd dependencies Pull requests that update a dependency file tests labels Sep 14, 2026
@os-warren
os-warren marked this pull request as ready for review September 14, 2026 01:31
@os-warren
os-warren enabled auto-merge September 14, 2026 01:32
@os-warren
os-warren added this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit a26a114 Sep 14, 2026
41 checks passed
@os-warren
os-warren deleted the claude/issue-17865-rest-log-suite-opt-down branch September 14, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd dependencies Pull requests that update a dependency file size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants