Skip to content

fix(objectql): surface a failed lifecycle governance row-count probe instead of skipping the object (#8906) - #9105

Draft
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-8906-check-governance-count-probe
Draft

fix(objectql): surface a failed lifecycle governance row-count probe instead of skipping the object (#8906)#9105
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-8906-check-governance-count-probe

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8906

LifecycleService.checkGovernance() probed every declared object's row count and threw the
result away on any failure:

try{rowCount=awaitdriver.count(obj.name);}catch{continue;}

A driver outage was therefore indistinguishable from a quiet, healthy object — and the
damage outlived the sweep it happened in: the object got no quota-exceeded and no
growth alert, and it dropped out of the map that becomes the next sweep's baseline,
so the following sweep had no last count to diff against either. Nothing was logged and
nothing appeared in the sweep report.

What changed

packages/objectql/src/lifecycle/lifecycle-service.ts — the probe's catch, and the
import the predicate needs (this seam had none; the sibling in engine.ts did):

  • Benign — unprovisioned table (isMissingTableError, @objectstack/metadata/errors):
    skipped silently, unchanged. A table that does not exist holds no rows, so there is no
    quota to breach and no growth to measure. It stays out of the baseline deliberately —
    seeding a 0 for a table that does not exist would fire a phantom growth alert on the
    first sweep after it is provisioned and seeded.
  • Everything else (connection drop, timeout, permission denial, a dialect error):
    surfaces. One entry per object in the sweep report's existingerrors list, plus a
    warn — both naming the consequence and the lost baseline, which is the half an
    operator cannot infer from a report that otherwise looks healthy. Other objects are
    unaffected; the sweep still finishes.

The predicate is asked through the shared, declared isMissingTableError, never a
hand-rolled code test — so the front-exclusion holds too: Postgres phrases a missing
column on an existing relation as column "x" of relation "y" does not exist, which
contains a legal missing-table phrase; that error surfaces rather than passing as benign.

The question this card had to answer, and why this answer

The card offered two shapes for the non-benign case — log at the durability level, or
carry an explicit incomplete fact in the sweep report — while the #8833 ruling forbids a
new response field. The fork dissolves on inspection: LifecycleSweepReport already has
the field.
errors: Array< { object; error } > is documented as "isolated per-object
failures", is filled the same way by sweep()'s own object loop 200 lines above, and is
already counted in the sweep's summary line. So this PR does both halves through
channels that already exist, and adds no new report field and no new error code.

Rethrowing — the #8895 / PR #9006 shape — is deliberately not the shape here, and not
for uniformity's sake in either direction. There the caller is a delete() that must fail.
Here the only caller is sweep(), whose scheduler entry point is void this.sweep(): a
throw would land as an unhandled rejection, abandon governance for every object still
queued, skip this.lastCounts = nextCounts entirely — losing every object's baseline
rather than one — and break the documented invariant that a sweep failure is isolated and
never thrown into the scheduler. The PR #9067 shape (delete the catch) does not fit either:
nothing upstream discriminates, engine.getDriverForObject() hands back the raw driver and
driver.count() throws dialect errors straight out.

Not taken, and named so it is a decision rather than an omission: carrying the previous
count forward as the baseline on a failed probe. That changes what a growth delta means
(two sweep intervals instead of one) and is a behaviour decision this card did not ask for.
The lost baseline is therefore reported, not repaired — and pinned that way.

Tests

packages/objectql/src/lifecycle/lifecycle-service.test.ts, one new describe, six cases,
every expectation a literal rather than anything derived from the code under test:

  • positive control — a healthy probe alerts on the quota and reports no error, so the
    refusals below cannot be satisfied by a fixture that never alerts;
  • non-benign failure → exact report.errors entry, exact warn text, errornot
    called (the level is a decision: reduced alerting, not a write that claimed to persist);
  • isolation — one failed probe still leaves the sibling object its quota alert;
  • the lost baseline is reported, not repaired: the recovered sweep still has no delta, and
    the sweep after that does;
  • benign missing table → silent, and the injected throw is asserted to have actually
    fired
    (count.mock.results[0].value rejects) — otherwise "the sweep continued" would
    also describe a harness that never probed;
  • missing column on a provisioned table → surfaces, despite carrying a missing-table
    phrase as a substring.

Reverse verification (fix committed first, ablated with git restore --source=origin/main,
restored with git checkout HEAD --): 4 of the 6 go red, the positive control and the
benign case stay green — the expected direction, and the reason the benign case is paired
with the non-benign ones rather than trusted alone.

Verification

All at 5c4ba963c, the final commit.

  • pnpm --filter @objectstack/objectql exec vitest run src/lifecycle/lifecycle-service.test.ts
    → 87 passed; whole-package pnpm --filter @objectstack/objectql test → 3735 passed;
    pnpm --filter @objectstack/objectql typecheck → clean.
  • Consumer direction (...@objectstack/objectql, downstream — 43 packages): runtime
    (165 files / 2456 tests), service-queue, service-storage, service-knowledge,
    service-settings all green via turbo run test; the remainder rides CI.
  • check:durability-log-level: byte-identical before and after
    67 read seam(s) … (7 … discriminated) (1 pass … through) (1 baselined) on both trees,
    measured by ablating the fix and re-running. Predicted and confirmed: the rule classifies
    a catch by the expression it returns, and both exits here are valueless (continue),
    which inventedEmptyValue and identityPassThrough each decline — the same
    byte-identical result ObjectQL.cascadeDeleteRelations fails OPEN: a failed dependents probe skips the restrict guard entirely, so a delete that should be refused succeeds silently #8895 measured at cascadeDeleteRelations. The gate is blind to
    this seam in both states; that is the measured blind spot, not a certification.
  • Also green: check:changeset-gate-self-tests, check:objectui-changeset,
    check:query-options-erasure, check:engine-double-contract, check:where-matcher,
    check:nul-bytes, check-adr-0087-registration, check-changeset-no-major,
    check-empty-changeset, check-engine-split-ratio.

Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

13 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx(via packages/objectql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/objectql)
  • content/docs/kernel/services.mdx(via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx(via packages/objectql)
  • content/docs/plugins/index.mdx(via @objectstack/objectql)
  • content/docs/plugins/packages.mdx(via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx(via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/objectql)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Consumer-direction addendum, after the body was written: @objectstack/dogfood's lifecycle-sweep test was run too — test/storage-growth.dogfood.test.ts, 6 passed, against a rebuiltpackages/objectql/dist (dogfood resolves the code under test from dist/, so the rebuild was verified by grepping the new probe message out of dist/index.js before the run, 2 hits).

Generated by Claude Code


Generated by Claude Code

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

2 participants

@os-zhuang@claude