Repository navigation
refactor(plugin-audit)!: retire export / permission_change from the sys_audit_log action enum (#8147) - #8200
Merged
Conversation
…_audit_log action enum (#8147) Retires the two action values with no writer anywhere in the repo, per the maintainer ruling of 2026-08-12 on #7675. Narrows the auth_events and config_changes list-view filters, regenerates the translation bundles, and registers the retirement under ADR-0087 as `audit-log-action-enum-retired`. `import` is deliberately NOT retired: plugin-auth's admin user-import writes a real run-level row with that action, pinned by dogfood case W4. Escalated on the issue for a maintainer ruling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 107 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
This was referenced Aug 12, 2026
os-zhuang
marked this pull request as ready for review
August 12, 2026 20:32
This was referenced Aug 12, 2026
Merged
This was referenced Aug 13, 2026
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 17, 2026
…he last_login_at update (objectstack-ai#8244) * feat(audit): write login/logout rows to sys_audit_log and attribute the last_login_at update (objectstack-ai#8144) sys_audit_log.action declares `login` and `logout`, the shipped `auth_events` list view filters on them, and two System Overview widgets chart them — but nothing ever wrote either row: the audit writers subscribe to the ObjectQL CRUD lifecycle, so create/update/delete/restore were the only actions that could materialize. The whole trace a sign-in left behind was an unattributed `update sys_user` row (user_id null) diffing last_login_at. - plugin-audit registers the `audit` service — the ledger's ingress for events that are not CRUD. The row shape stays owned by plugin-audit; the caller hands over an EVENT with a closed `login | logout` union, which is the only structural protection available on an object whose action enum nothing validates in either direction (objectstack-ai#8203). - plugin-auth emits from better-auth's session lifecycle hooks: session.create.after => login (covers every sign-in method, not just /sign-in/email), session.delete.after under /sign-out => logout. Revokes, bans, erasure and expired-row collection are deliberately NOT logout — they already carry their cause on the ADR-0069 D4 tombstone, and naming them logout would be a wrong record rather than a vague one. - stampLastLogin now carries attributedUserId (objectstack-ai#4586), so the last_login_at diff row names the person who signed in. Attributed rather than excluded: the write still authorizes as the system, and suppressing it would delete the last_login_ip trail repo-wide. Neither package depends on the other; a stack without plugin-audit writes no auth rows, exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 * test(dogfood): read the auth_events filter from the running registry (objectstack-ai#8144) objectstack-ai#8200 retired `permission_change` / `export` from the action enum and narrowed the `auth_events` view in the same PR. A hard-coded copy of the old filter kept querying a value nothing can hold while still reporting success — the view has exactly the shape that hides it, since the login rows alone satisfy the assertion. Read the shipped filter instead, so the test tracks the view. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 * test(plugin-auth): type the audit sink spy so its call log is a real tuple (objectstack-ai#8144) `vi.fn(async () => undefined)` types `mock.calls` as `[][]` — a zero-length tuple — so every `calls[0][0]` in this file reached past the end of a tuple the type system believed was empty (TS2493 x3), and the dereference that followed was `possibly undefined` (TS18048 x3). Reading the argument back is the entire point of these cases, so the fix is to declare what the spy receives rather than to soften the read: the spy's implementation now names its parameter, and the call log is pulled through `recordedEvents` / `firstEvent`, which name the "never called" case instead of letting it surface as a TypeError. Pinning the element type to `AuthSessionAuditEventInput` also makes these assertions type-check against the real event surface instead of `any`: a renamed field now fails at compile time rather than quietly comparing `undefined` to `undefined`. One `(c: any[])` map goes away with it. Behaviour unchanged — 1113/1113 plugin-auth tests pass, same 20 cases. Why now: objectstack-ai#8225 lowered this package's TEST_DEBT ceiling 131 -> 111 after this branch was cut, so these six errors stopped being slack and became a violation in the merge queue. Measured at 117 against the merged tree, 111 after this commit — exactly the ceiling, ledger untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #8147
Part of, notFixes— deliberately. This lands 2 of the 3 values the2026-08-12 ruling named. The third (
import) is referred back to the maintainerbecause its stated premise is measurably false; see "The escalation" below.
Merging this must not close #8147.
What landed
exportandpermission_changeare retired from thesys_audit_log.actionenum. Neither has ever been written. The repo has exactly two
sys_audit_logwriters:packages/plugins/plugin-audit/src/audit-writers.ts(generic hook writer)actionFor()maps afterInsert/afterUpdate/afterDelete tocreate/update/deleteand nothing elsepackages/plugins/plugin-auth/src/admin-import-users.tsaction: 'import', run-level,record_id: nullNeither emits
exportorpermission_change, so the shipped list viewsfiltering on them and the dashboard tiles counting them were permanently empty —
the visible product defect the ruling names (空 widget + 永远查不到东西的过滤器).
export+permission_changeremovedauth_eventslist view filter: narrowed to['login', 'logout']config_changeslist view filter:exportdropped,config_changeandimportkeptplugin-audittranslation bundles regenerated (4 locales, viacheck-i18n-bundles --write— not hand-edited)audit-log-action-enum-retiredon D3 step 17, withspec-changes.jsonand the upgrade guide regeneratedsys-audit-log-retired-actions.test.tsservice-analyticsrouting fixture reseeded offpermission_changeWhy a semantic entry and not a D2 conversion
sys_audit_logis a platform-owned append-only object whose every field isreadonly: true. Nobody authors an audit row and nobody authors this enum, sothere is no source for the chain to rewrite — the same disposition
BatchOptions.validateOnlyand the notification cursor already take in thismajor. As an enum-VALUE retirement nothing lands in
RETIRED_KEYS_BY_MAJORandthe four surface ratchets are byte-identical by construction (no def and no
authorable key changed) — verified, they did not move.
The escalation:
importis NOT retiredThe ruling named
importon the premise 无此 feature. That premise is false:plugin-auth/src/admin-import-users.tswrites a real run-level audit row onevery admin user-import run, and its docblock records the design decision
(corrected rationale, superseding an earlier stale claim) that this row is a
deliberate complement to the per-row rows, not a duplicate.
packages/qa/dogfood/test/admin-identity-audit-trail.dogfood.test.tsasserts that row exists, with
record_id: nullandmetadata.event === 'user.import_run'.Retiring it would make the enum deny a value the platform writes — and do it
silently:
validateRecordskipsreadonlyfields on insert, every fieldhere is readonly, so nothing rejects the write and nothing goes red. The result
would be written-but-not-declared, which for an audit surface is strictly worse
than the declared-but-never-written defect this card exists to fix, because the
row exists and the contract denies it.
config_changesis also the only shippedview that lists those rows.
Ablation (predicted before measured)
Restoring both values to the enum, everything else held:
importcases stay greencheck:i18n(plugin-audit)DRIFTED (4)check:spec-changes/check:upgrade-guide/check:authorable-surfaceservice-analyticsrouting testdelete, never validates against the object schemaZero divergence. The negative arms matter as much as the red one: the ADR-0087
registration is not verified against reality by any gate, and the
service-analytics fixture is not a detector of this retirement. The only
mechanical detectors are the new pin test and the i18n bundle ratchet — which is
precisely why the pin test is in this PR, with its expectations written as
literals so that expectation and reality do not derive from the same source.
Gates
check:nul-bytes,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check:migration-registry,check:spec-changes,check:upgrade-guide,check:authorable-surface,check:api-surface,check:liveness,check:i18n,check:adr-anchors,check:merge-driver,check:cross-package-test-inputs,check:test-source-alias,check:docs-audit-scope,check:engine-double-contract,check:query-options-erasure— all PASS.check:type-check-debtPASS from therepo root after a full build closure (no entry above its ceiling; the surplus
notes are pre-existing and in packages this PR does not touch — ledger not
lowered).
plugin-audit207 tests pass,service-analytics1655 pass,plugin-audit+spectypecheck pass.Downstream, deliberately untouched
packages/platform-objectswidget + translations + tile test → audit-log (D): remove thepermission_changewidget fromsystem_overview.dashboard.tsand align the dashboard's audit tiles with the post-retirement action set #8148ACTION_OPTIONS/HistoryTimeline→ objectui#4476PR's finding, both need correcting to two — dropping
importfrom theconsole badge map would leave real, existing rows rendering without a label.
Generated by Claude Code
Generated by Claude Code