Repository navigation
fix(service-settings): an unmounted audit ledger is a configuration, not a fault - #18703
Conversation
…not a fault `buildConfigChangeAuditSink` attempted the `sys_audit_log` `config_change` insert unconditionally and reported the throw it got back as a degradation. A host that never mounted the OPTIONAL `@objectstack/plugin-audit` has no ledger to write to, so that report described a deployment behaving exactly as composed — and the insert it never should have attempted reached `ObjectQL.insert`, whose own terminal catch logs `Insert operation failed` once per settings write. The sink now probes the engine registry for `sys_audit_log` before the write. Absent, it skips at `debug` and attempts nothing. The probe records nothing and is re-taken per call, so a ledger registered later in the same boot starts recording; an engine that cannot be asked (no `getSchema`) still gets the write attempted, which is the pre-change behaviour. What is left in the catch is a mounted ledger whose insert genuinely failed — a write that claims to be audited and is not — so it reports on the `error` channel, with `warn` kept as the receiver-safe fallback for a sink with no `error`. Both arms are pinned, plus the probe's non-memoization, the unanswerable-engine path and the `warn` fallback. Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP Co-authored-by: Claude <noreply@anthropic.com>
…counted sites Two gate findings from the derived roster, both on the new code: `check:doc-authoring` — the tracker id had been spelled inside the operator-facing failure string. A runtime string reaches authors and operators, none of whom can resolve `#NNNN`; the id stays in the adjacent source comments, where the reader who can resolve it is already looking. `check:tenant-audit-census` — folding the pre-existing `getSchema` and `insert` spellings onto the new module constant moved a corpus-scale ratchet (inline 109 to 108, const 40 to 41) and a hand-written prose count in `content/docs/permissions/`, a tree this change has no business in. Both spellings are restored verbatim; the constant is used only by the new mount probe, and says why. Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d29e257b636301745e64bfb3188215bc7dd47853 && git checkout d29e257b636301745e64bfb3188215bc7dd47853
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f8eaf670454a69ebb965d9ec94aeed31303b1f4b 5ec4b174135bb4e2a14336f13cbbefc6234b5846 && git checkout -B drift-repro f8eaf670454a69ebb965d9ec94aeed31303b1f4b && git merge --no-ff 5ec4b174135bb4e2a14336f13cbbefc6234b5846
node scripts/docs-audit/affected-docs.mjs --json f8eaf670454a69ebb965d9ec94aeed31303b1f4b
|
Fixes #18368
Clause-②: no — no new key on a published payload. No symbol is added to any export,
src/index.tsis untouched, andCONFIG_CHANGE_ACTION/CONFIG_CHANGE_OBJECT_NAMEkeep their shapes; the newAUDIT_LEDGER_OBJECT_NAMEand thedebug?member are module-local. Re-verified against the diff by the reviewing seat, ⛔ not taken from the report.buildConfigChangeAuditSinkattempted thesys_audit_logconfig_changeinsertunconditionally and reported the throw it got back as a degradation. A host that never
mounted the OPTIONAL
@objectstack/plugin-audit—objectstack serve --preset minimal,an EE host that mounts no
audit, a hosted tenant kernel — has no ledger to write to,so that report described a deployment behaving exactly as composed. An unmounted ledger
is a configuration, not a fault.
The sink now probes the engine registry for
sys_audit_logbefore the write. Absent, itskips at
debugand attempts nothing.The premise, re-taken — and one half of it is FALSE on this tree
The card (and the
repo:cloudseat's reading behind it) says the sink "logs an ERRORon every tenant settings write". Measured on
origin/mainat the branch point1bc22b3, neither half of that severity holds in this repository, and the roster below ishow it was measured rather than recalled:
ERRORwarnlogger.warnoverlogger.errorfailureReportedhas guarded that report since #8145⭐ What IS per-write, and what the card was almost certainly hearing, is one frame down:
the insert this sink should never have attempted reaches
ObjectQL.insert, whose terminalcatchlogsInsert operation failed— atwarnsince #17052, and with nothingreported-once about it. Probed directly against a real
ObjectQLwithsys_audit_logabsent from the registry:
⛔ The defect the card names is real and this PR fixes it; only the LEVEL and the
FREQUENCY in its prose are wrong for this tree. The cloud seat's readings were taken
against a published
@objectstack/service-settingson a hosted plane, and this sessioncannot reach
cloud#1975/cloud#1963to reconcile the two — that is declared, notverified. The fix does not depend on which is right: skipping the write removes BOTH lines,
and the acceptance criterion ("no ERROR") holds under either reading.
Both arms, and neither is optional
The quiet arm alone would pass just as well on a sink whose log line had been deleted
outright, so every case carries its opposite.
sys_audit_log: the settings write lands, thesys_setting_auditrow lands, no insert is attempted (that is what silences theengine's per-write line, and it is asserted on the seam rather than on the log), nothing
on
error, nothing onwarn, and exactly onedebugline naming the remedy.attempted, and the report still fires, on
error, carrying its consequence and its fix.Three more properties, each of which a plausible wrong implementation would break:
the same boot starts recording. A memoized "not mounted" would be a verdict the same
boot can contradict (AGENTS.md → Startup registry reads), and it would pass every
other case in the file.
getSchemais an ObjectQL member, not anIDataEngineone. An engine that does not carry it still gets the write attempted,which is the pre-change behaviour. Read the other way, this sink would go permanently
silent on every lean host engine: the same defect, inverted.
warnfallback survives the level flip — a sink declaring onlywarnstillgets the line, spelled as a property access so a class-based
Loggerkeeps its receiver.Why the fault arm got LOUDER
What is left in the
catchonce the expectation is skipped is a mounted ledger whoseinsert genuinely failed: the settings write is on disk and claims to be audited while the
compliance row is not, and nothing retries it. That is AGENTS.md → Degradation log
levels to the letter, so the report moved from
warn-first toerror-first. This is thecard's own control arm ("the ERROR still fires"), and it is why the level claim above
being false did not make it unimplementable.
⛔ The seam is NOT added to
DURABILITY_CRITICAL_CALLEES: the callee here is the genericeng.insert, and that script's header is explicit that a name that broad trades a missfor a false-positive rate that gets gates disabled. Noted below, not filed.
Ablation — both new arms proven able to fail
Run from the committed state, each leg proving the mutation reached disk (blob hash moves
off the
HEADblob) before the tests ran, and each restored bygit checkout HEAD -- pathwith the restore proven by hash equality, not by an exit code.
if (false))warn-firstRestored blob
2f9535bbboth times, equal toHEAD:packages/services/service-settings/src/config-change-audit.ts.Verification
Commands below ran at
5ec4b17, the final commit.pnpm --filter @objectstack/service-settings test— 33 files, 584 tests, all pass(16 in
config-change-audit.test.ts, 5 of them new).pnpm --filter @objectstack/service-settings typecheck— clean;tsc --listFilesconfirms the program reaches all 33
*.test.tsin the package, this file included, sothe green covers the new cases.
pnpm --filter '@objectstack/service-settings^...' build— dependency closure, clean.eslint . --no-inline-config --format json— the WHOLE repo population, not a narrowing:6819 files, 0 errors, 0 warnings.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(no paths — merge-base derived), reconciled with
--ran:59 derived, 57 run and green, 2 NOT MEASURED, 0 unrun. The two are
check:dual-build-cjs-loadsandcheck:type-check-debt, both exit 3(
PREREQUISITE NOT MET— each needs a full workspace build, which CI performs); ⛔ anexit 3 is recorded as not-measured, never as a pass and never as a finding.
run by hand and are green:
pnpm check:durability-log-level(36 seams, all loud) andpnpm check:startup-registry-verdict(43 seams, none recording a contradictable verdict).check:doc-authoring(a tracker id had been spelled inside the operator-facing string)and
check:tenant-audit-census(see the acceptance note below).check-plugin-teardown-shape --self-testfirst exited 3 on a fixture commit this shallowclone could not reach; resolved with a targeted
git fetchof that commit, then green.⛔ Not a finding about this diff.
Not merged with
origin/main(3 commits ahead:packages/speccontracts, a spec docspage, and
scripts/pm/check-half-states.mjs) — zero path intersection with this diff,so there is no overlap for §10's joint-breakage re-check to scope onto. The merge queue
validates the rebuilt generation regardless.
Acceptance notes
Noted, not filed — none is a reproducible defect, a declared-contract violation or a
metadata-authoring trap:
Folding
makeFieldProbe'sgetSchemacall and theinsertonto the new module constantmoved a corpus-scale ratchet —
content/docs/permissions/tenant-audit-census.mdx, "objectname spelled inline" 109 to 108 and "named through a const" 40 to 41 — plus a hand-written
prose count in a docs tree, and this card's declared file surface is
packages/services/service-settings/src/. The consolidation is worth doing; it is worthdoing on a card that owns the census regeneration. Successor: whoever next touches this
census. The constant carries the reason at its definition.
check:durability-log-level, because its calleeis the generic
eng.insertrather than a namedpersistXxxRowhelper of the kind everyother entry in
DURABILITY_CRITICAL_CALLEESnames. Extracting one and declaring it wouldedit
scripts/, outside the declared surface. Successor: none currently queued — noted sothe next reader does not read the gate's green as vouching for this line's level.
makeFieldProbememoizes itsorganization_idanswer for the process, which was alatent version of the same shape this card fixes: a "not registered" cached before
plugin-auditfinished registering would leaveorganization_idunstamped forever, andthe SecurityPlugin's RLS predicate would then hide every
config_changerow fromnon-platform readers — the empty
config_changesview, one layer down. This PR closes itincidentally rather than by design: the probe now returns early on an unmounted ledger, so
makeFieldProbeis only ever consulted on a deployment where the ledger IS registered.Recorded because the closure is a side effect and nothing pins it.
audit, and the fix is independent of whether it ever does —--preset minimaland an EEhost that mounts no
auditreach this same line.packages/specis untouched. No exported symbol, parameter or option was added:buildConfigChangeAuditSink(engine, logger?), the row shape,CONFIG_CHANGE_ACTIONandCONFIG_CHANGE_OBJECT_NAMEare unchanged, soClause-②: nostill holds and the changesetis
patch.Dispatched by the
domain:servicesPM seat (objectstack#6021) under the claim comment onthe card; this PR is the dev's record.
Generated by Claude Code