Commit cb648cb
fix(plugin-audit): key the lost auth-event row report per CAUSE, and name the real cause in its first line (#18246)
Fixes #17452
Clause-②: no
## What this is
`packages/plugins/plugin-audit/src/auth-event-audit.ts` — the writer
behind the `login` / `logout` rows of the compliance ledger — carried a
second, independent copy of both defects that #15166 removed from
`audit-writers.ts`:
1. **its own process-level `failureReported` boolean**, so after the
first failure of ANY cause, every later failure of every OTHER cause
degraded to `debug` for the life of the process;
2. **its own fixed message literal**, printing the ADR-0057 §3.6 /
`OS_TELEMETRY_DB` datasource guidance unconditionally, regardless of
what actually failed.
`persistAuthEventAuditRow` is registered in
`DURABILITY_CRITICAL_CALLEES`
(`scripts/check-durability-degradation-log-level.mjs:361`), whose entire
purpose is that durability loss is reported at `error`. That register is
the declaration this restores — ⛔ it is not edited here.
## Separation re-verified before choosing the approach
The dispatch asked whether the two writers have converged since #15166.
Measured on `origin/main` at `b3b43b6ea` (this branch's base), they had
**not**:
| | `audit-writers.ts` | `auth-event-audit.ts` (before) |
|:--|:--|:--|
| reporter | `reportAuditWriteFailure` | `reportAuthEventWriteFailure` |
| dedupe state | `reportedAuditFailureCauses` (a `Set`) |
`failureReported` (a boolean) |
| message | cause-led, conditional remedy | one fixed literal |
| logger | `(engine as any).logger` | injected `AuthEventAuditLogger`,
`error` OPTIONAL |
| tables written | `sys_audit_log` + `sys_activity` | `sys_audit_log`
only |
So the fix lands in place rather than merging two structurally different
reporters. What **is** shared is the part the card called a port: the
cause-key helpers.
## What changed
- `audit-writers.ts` — `auditFailureCauseKey` and
`auditFailureCauseSummary` become module exports so the auth-event sink
can use them. ⛔ Not added to `src/index.ts`: the sharing is internal to
the package. This mirrors the existing `createFieldPresenceProbe` import
that already crosses these two files.
- `auth-event-audit.ts` — the boolean becomes
`reportedAuthEventFailureCauses`, a `Set` keyed by
`auditFailureCauseKey(SESSION_OBJECT, err)`. The first `error` line
leads with `auditFailureCauseSummary(err, detail)` — the code and
message that were already computed one line above the branch and passed
only into the `debug` payload. The datasource guidance is kept and made
conditional on `isMissingTableError(err, 'sys_audit_log')`, asked for
the one table this writer writes.
- `auth-event-audit.test.ts` — 7 new cases (see below).
- A `patch` changeset.
⛔ Importing the helpers rather than re-spelling them is deliberate: a
second copy of this key is how the defect reached this file, so a third
spelling would be the same mistake again.
## Evidence
### Reproduce first — the new block against the unfixed source
Commit `3985dd2bb` is the pin block alone, on top of unmodified source.
`pnpm --filter @objectstack/plugin-audit exec vitest run
src/auth-event-audit.test.ts`:
```
Tests 4 failed | 10 passed (14)
FAIL reports a SECOND, DIFFERENT cause at error — a new cause is a new degradation
AssertionError: expected [ { level: 'error', …(2) } ] to have a length of 2 but got 1
FAIL prints the datasource remedy for the cause it is the remedy FOR, and not for others
AssertionError: expected 'Auth-event audit write FAILED — the c…' not to match /OS_TELEMETRY_DB/
Received: "… Fix: confirm `sys_audit_log` is reachable … Set `OS_TELEMETRY_DB=0` …"
(the cause driven through the reporter was ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED)
```
Both defects the card names, reproduced through the real reporter: a
second cause silenced to `debug`, and the datasource hint printed for a
failure it is not the remedy for.
### After the fix
`Tests 14 passed (14)`. Whole package: `Test Files 23 passed (23) ·
Tests 341 passed (341)`; `pnpm --filter @objectstack/plugin-audit
typecheck` exit 0 (`check:test-typecheck: OK — 0 file(s) / 0 error(s)`).
### Reverse verification — three legs, mutated on disk, each restored
byte-identically
Every leg proves the mutation landed (`grep -c` on the anchor text and
on the injected text, plus the on-disk blob hash moving), runs the
suite, then restores with `git checkout HEAD -- path` and proves `git
hash-object` equals the HEAD blob and `git diff HEAD` is empty. HEAD
blob `ba07853195ca57faf56734d32e005a836cd0aae2` before and after all
three. No build/`dist` leg is owed: the suite imports
`./auth-event-audit.js` relatively and `vitest.config.ts` aliases
`@objectstack/types` to source, so nothing on the tested path resolves
through `dist`.
| leg | mutation | anchor→inject | result |
|:--|:--|:--|:--|
| A | cause key collapsed to one constant bucket (= the old boolean) |
1→0 / 0→1, hash `31b8e4c…` | **2 failed** — `reports a SECOND, DIFFERENT
cause at error`, `[#9657] the warn fallback is per-cause too` |
| B | `missingTable` forced to `true` (= the old unconditional hint) |
1→0 / 0→1, hash `13edd5b…` | **1 failed** — `prints the datasource
remedy for the cause it is the remedy FOR, and not for others` |
| C | dedupe removed entirely (the **named falsifier**) | 1→0 / 0→1,
hash `7d998a1…` | **4 failed** — the three anti-noise controls, plus the
pre-existing #8144 `reported at ERROR, once` case |
### The discriminating control for a per-cause dedupe
Leg C is the point. "A different cause now reports" is satisfied by
simply deleting the boolean, which is the outcome AGENTS.md names as
this rule's falsifier. Three cases hold the other half, and they stay
**green** under leg A (12 of 14 passed there, and none of the three is
in that leg's FAIL list) while going **red** under leg C:
- `still degrades a REPEAT of an already-reported cause to debug` — 5
sign-ins, same cause ⇒ 1 `error` + 4 `debug`, and the `debug` lines
carry the same `cause` key.
- `keys on the error CODE, never its message, so a per-row fault cannot
flood error` — 200 sign-ins, 200 distinct per-row messages, one code ⇒ 1
`error`.
- `folds a fault carrying NO code into ONE bucket rather than growing
one` — 200 sign-ins, no code at all ⇒ 1 `error`.
⇒ the delivered behaviour is *cause-keyed dedupe*, not *no dedupe*. The
premise #15166's ruling hung on — that cause-keying does not reintroduce
#4420's unreadable flood — holds on this seam too, and for a stronger
reason: the object dimension is constant here (`sys_session`), so the
key reduces to the driver's own closed code vocabulary.
### Clause-② re-derived from the DELIVERED diff
`src/index.ts` is untouched (`git diff origin/main --stat` on it is
empty), and `tsup` builds the single entry `src/index.ts`.
- `.d.ts` export list of the published entry (`dist/index.d.ts`): 27
names, neither `auditFailureCauseKey` nor `auditFailureCauseSummary`
among them.
- runtime probe, `name in await import('dist/index.mjs')` (the path
`exports["."]` names):
```
auditFailureCauseKey not reachable
auditFailureCauseSummary not reachable
createAuthEventAuditSink REACHABLE [POSITIVE CONTROL]
installAuditWriters REACHABLE [POSITIVE CONTROL]
thisSymbolDoesNotExist not reachable [NEGATIVE CONTROL]
total runtime exports: 11
```
⇒ **`Clause-②: no`**, derived by reachability rather than by the word
`export`. The changeset is graded `patch`.
### Gate denominator
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived **65** families at HEAD `d20bd8765`.
All 65 were run with exit codes recorded before any pipe, and
reconciled:
```
Run reconciliation — 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN.
EXIT CODES — all 65 accounted famil(ies) carry one, so the NOT-MEASURED count above is DERIVED from them.
✓ dispatch-gates --ran: 65 derived famil(ies) accounted for — 65 run, 0 NOT-MEASURED
(a DERIVED zero — all 65 recorded an exit code and none of them is 3).
```
Three first answered **exit 3, PREREQUISITE NOT MET** — `check:i18n`,
`check:dual-build-cjs-loads`, `check:type-check-debt`. ⛔ Not read as
green: their prerequisite closures were built (`turbo run build` over
the i18n gate's named closure, then over `./packages/*`
`./packages/*/*`) and all three re-run to exit 0 — `check-i18n-bundles:
OK (9 package(s) — all bundles in sync…)` and `check-type-check-coverage
--re-measure: OK — 5 ledger entr(ies) re-measured, 55 raw tsc error(s)
total, none above its recorded number`.
Outside the derived set and run anyway because it is the declaration
this card restores: `pnpm check:durability-log-level` exit 0 — `✓
durability-degradation log levels: 36 durability-critical catch seam(s),
all loud…`.
### eslint — a declared narrowing, with its three readings
Repo-wide `pnpm lint` is CI's run. Narrowed here to the three source
files this diff touches, at HEAD `d20bd8765`:
① Population read from eslint's own `eslint.config.mjs`, not guessed:
the base block is `files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']` with
further `packages/**/*.{ts,tsx,mts,cts}` blocks — all three files are
inside both.
② Count read from `--format json`: **3 files linted, 0 errors, 0
warnings**, exit 0.
③ Invariance over untouched files: `eslint.config.mjs` states at its own
line 328 that this repo "runs one `eslint.config.mjs`, which never
enables type-aware linting (no `parserOptions.project`, no typed
`@typescript-eslint` rules) for ANY file" — measured there with a
positive control. With no cross-file type program, this diff cannot move
the verdict on any file it does not touch.
## Acceptance notes
Out of scope for this PR, ⛔ not fixed here, and ⛔ not filed either — the
dispatch reserved filing to the PM:
- **A THIRD copy of the same pair lives in
`packages/plugins/plugin-audit/src/read-audit.ts` (lines 484 onward).**
`reportReadAuditWriteFailure` has its own process-level
`failureReported` boolean and its own fixed literal carrying the same
unconditional ADR-0057 §3.6 / `OS_TELEMETRY_DB` guidance — and its
callee `persistReadAuditRows` is registered in
`DURABILITY_CRITICAL_CALLEES`
(`scripts/check-durability-degradation-log-level.mjs:357`), exactly as
the two already dealt with. Same declared invariant, same blast radius,
on the record-view audit path. Now a cheap port: the helpers this PR
exports are the whole shape it needs. Dedupe read: one targeted semantic
search over this repo, 12 hits, no open duplicate — with #17452 and
#15166 both returning as the firing control. Dedupe words for whoever
files it: `read-audit.ts` · `reportReadAuditWriteFailure` ·
`persistReadAuditRows` · `failureReported` · third copy.
- **Noted, not filed** —
`packages/services/service-settings/src/config-change-audit.ts:157`
carries the same process-wide `failureReported` shape, but it is **not**
the same class: its callee is a bare `eng.insert` that no register
names, its first line already carries `Cause: ` plus the real detail,
and its remedy text is cause-agnostic (it explains that `plugin-audit`
is optional). An observation, not a contract violation. Successor:
whoever takes the `read-audit.ts` card above, as the same sweep.
## Notes for review
- ⛔ `DURABILITY_CRITICAL_CALLEES` is untouched, per the card's fence.
- `packages/spec` is untouched.
- No test was skipped, disabled or weakened. The pre-existing #8144 case
`a failed ledger write is reported at ERROR, once, and never breaks the
caller` drives the **same** cause twice, so it stays green under the new
key unchanged — and leg C shows it is load-bearing.
- `@objectstack/types` and its `paths` / vitest-alias entries were
already added by PR #17450, so no manifest or tsconfig change is owed
for `isMissingTableError`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c81e7ff commit cb648cb
4 files changed
Lines changed: 295 additions & 20 deletions
File tree
- .changeset
- packages/plugins/plugin-audit/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
726 | 726 | | |
727 | 727 | | |
728 | 728 | | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
729 | 738 | | |
730 | | - | |
| 739 | + | |
731 | 740 | | |
732 | 741 | | |
733 | 742 | | |
| |||
743 | 752 | | |
744 | 753 | | |
745 | 754 | | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
746 | 758 | | |
747 | | - | |
| 759 | + | |
748 | 760 | | |
749 | 761 | | |
750 | 762 | | |
| |||
Lines changed: 200 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
350 | 350 | | |
351 | 351 | | |
352 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
0 commit comments