Repository navigation
Commit f6189a4
fix(service-settings): an unmounted audit ledger is a configuration, not a fault (#18703)
Fixes #18368
Clause-②: no — no new key on a published payload. No symbol is added to
any export, `src/index.ts` is untouched, and `CONFIG_CHANGE_ACTION` /
`CONFIG_CHANGE_OBJECT_NAME` keep their shapes; the new
`AUDIT_LEDGER_OBJECT_NAME` and the `debug?` member are module-local.
Re-verified against the diff by the reviewing seat, ⛔ not taken from the
report.
`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` — `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_log` before the
write. Absent, it
skips at `debug` and attempts nothing.
## The premise, re-taken — and one half of it is FALSE on this tree
The card (and the `repo:cloud` seat's reading behind it) says the sink
"logs an **ERROR**
on **every** tenant settings write". Measured on `origin/main` at the
branch point
`1bc22b3`, neither half of that severity holds in this repository, and
the roster below is
how it was measured rather than recalled:
| the card's claim | measured here | where |
|---|---|---|
| level is `ERROR` | `warn` | the sink's own catch preferred
`logger.warn` over `logger.error` |
| once per **write** | once per **process** | `failureReported` has
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 terminal
`catch` logs `Insert operation failed` — at `warn` since #17052, and
with nothing
reported-once about it. Probed directly against a real `ObjectQL` with
`sys_audit_log`
absent from the registry:
```
=== sys_audit_log NOT registered, driver throws "no such table" ===
THREW: Error: no such table: sys_audit_log
[debug] x2 Insert operation starting / No hooks registered for event
[warn] x1 Insert operation failed
```
⛔ 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-settings` on a hosted plane,
and this session
cannot reach `cloud#1975` / `cloud#1963` to reconcile the two — that is
declared, not
verified. 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.
- **QUIET ARM** — registry has no `sys_audit_log`: the settings write
lands, the
`sys_setting_audit` row lands, **no insert is attempted** (that is what
silences the
engine's per-write line, and it is asserted on the seam rather than on
the log), nothing
on `error`, nothing on `warn`, and exactly one `debug` line naming the
remedy.
- **CONTROL ARM** — ledger IS mounted and the insert genuinely fails:
the insert WAS
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 probe records nothing and is re-taken per call** — a ledger
registered later in
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.
- **"cannot tell" is not "absent"** — `getSchema` is an ObjectQL member,
not an
`IDataEngine` one. 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.
- **the `warn` fallback survives the level flip** — a sink declaring
only `warn` still
gets the line, spelled as a property access so a class-based `Logger`
keeps its receiver.
## Why the fault arm got LOUDER
What is left in the `catch` once the expectation is skipped is a mounted
ledger whose
insert 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 to
`error`-first. This is the
card'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 generic
`eng.insert`, and that script's header is explicit that a name that
broad trades a miss
for 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 `HEAD` blob) before the tests ran, and each restored by `git
checkout HEAD -- path`
with the restore proven by hash equality, not by an exit code.
| leg | mutation | result |
|---|---|---|
| A | the registry probe removed (`if (false)`) | **2 failed** — QUIET
ARM, and the non-memoization pin |
| B | the fault report put back on `warn`-first | **2 failed** — CONTROL
ARM, and the unanswerable-engine pin |
Restored blob `2f9535bb` both times, equal to
`HEAD: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
--listFiles`
confirms the program reaches all 33 `*.test.ts` in the package, this
file included, so
the 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**.
- Gate roster derived with `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-loads` and `check:type-check-debt`, both exit
**3**
(`PREREQUISITE NOT MET` — each needs a full workspace build, which CI
performs); ⛔ an
exit 3 is recorded as not-measured, never as a pass and never as a
finding.
- Beyond the derived roster, the two families this change most plausibly
implicates were
run by hand and are green: `pnpm check:durability-log-level` (36 seams,
all loud) and
`pnpm check:startup-registry-verdict` (43 seams, none recording a
contradictable verdict).
- Two gates went red on the first pass and were FIXED, not routed
around:
`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-test` first exited 3 on a fixture
commit this shallow
clone could not reach; resolved with a targeted `git fetch` of that
commit, then green.
⛔ Not a finding about this diff.
Not merged with `origin/main` (3 commits ahead: `packages/spec`
contracts, a spec docs
page, 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:
- **The ledger object name is spelled inline at its two pre-existing
sites, deliberately.**
Folding `makeFieldProbe`'s `getSchema` call and the `insert` onto the
new module constant
moved a corpus-scale ratchet —
`content/docs/permissions/tenant-audit-census.mdx`, "object
name 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 worth
doing on a card that owns the census regeneration. Successor: whoever
next touches this
census. The constant carries the reason at its definition.
- **This durability seam is invisible to `check:durability-log-level`**,
because its callee
is the generic `eng.insert` rather than a named `persistXxxRow` helper
of the kind every
other entry in `DURABILITY_CRITICAL_CALLEES` names. Extracting one and
declaring it would
edit `scripts/`, outside the declared surface. Successor: none currently
queued — noted so
the next reader does not read the gate's green as vouching for this
line's level.
- **`makeFieldProbe` memoizes its `organization_id` answer for the
process**, which was a
latent version of the same shape this card fixes: a "not registered"
cached before
`plugin-audit` finished registering would leave `organization_id`
unstamped forever, and
the SecurityPlugin's RLS predicate would then hide every `config_change`
row from
non-platform readers — the empty `config_changes` view, one layer down.
This PR closes it
incidentally rather than by design: the probe now returns early on an
unmounted ledger, so
`makeFieldProbe` is only ever consulted on a deployment where the ledger
IS registered.
Recorded because the closure is a side effect and nothing pins it.
- **Route (1) is untouched**, as ruled: nothing here asks the hosted
policy to force-mount
`audit`, and the fix is independent of whether it ever does — `--preset
minimal` and an EE
host that mounts no `audit` reach this same line.
- `packages/spec` is untouched. No exported symbol, parameter or option
was added:
`buildConfigChangeAuditSink(engine, logger?)`, the row shape,
`CONFIG_CHANGE_ACTION` and
`CONFIG_CHANGE_OBJECT_NAME` are unchanged, so `Clause-②: no` still holds
and the changeset
is `patch`.
Dispatched by the `domain:services` PM seat (objectstack#6021) under the
claim comment on
the card; this PR is the dev's record.
---
_Generated by [Claude Code](https://claude.ai/code)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent a7e9a66 commit f6189a4
3 files changed
Lines changed: 376 additions & 14 deletions
File tree
- .changeset
- packages/services/service-settings/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
Lines changed: 209 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
83 | 110 | | |
84 | 111 | | |
85 | 112 | | |
| |||
174 | 201 | | |
175 | 202 | | |
176 | 203 | | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
177 | 210 | | |
178 | 211 | | |
179 | 212 | | |
| |||
195 | 228 | | |
196 | 229 | | |
197 | 230 | | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
198 | 235 | | |
199 | 236 | | |
200 | 237 | | |
| |||
211 | 248 | | |
212 | 249 | | |
213 | 250 | | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
214 | 260 | | |
215 | | - | |
216 | | - | |
217 | | - | |
218 | | - | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
219 | 265 | | |
220 | 266 | | |
221 | 267 | | |
| |||
259 | 305 | | |
260 | 306 | | |
261 | 307 | | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
262 | 312 | | |
263 | 313 | | |
264 | 314 | | |
| |||
528 | 578 | | |
529 | 579 | | |
530 | 580 | | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
531 | 584 | | |
532 | 585 | | |
533 | 586 | | |
| |||
580 | 633 | | |
581 | 634 | | |
582 | 635 | | |
583 | | - | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
584 | 640 | | |
585 | 641 | | |
586 | 642 | | |
| |||
596 | 652 | | |
597 | 653 | | |
598 | 654 | | |
599 | | - | |
| 655 | + | |
600 | 656 | | |
601 | 657 | | |
602 | 658 | | |
| |||
609 | 665 | | |
610 | 666 | | |
611 | 667 | | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
0 commit comments