refactor(plugin-audit)!: retire restore from the sys_audit_log action enum (#8315) - #8325
Conversation
… enum (#8315) `restore` was declared by `sys_audit_log.action` with no writer anywhere in the repo, and the record-level writer structurally cannot produce it: `actionFor()` returns `'create' | 'update' | 'delete' | null`. Retired under the maintainer ruling of 2026-08-12 on #7675, carried by triage — the same operation #8147 performed on `export` / `permission_change`. Also corrects the false coverage comment in `auth-event-audit.ts`, which named `restore` among the actions the writer emits, and narrows the `writes_only` list-view filter to what `actionFor()` can actually emit. 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
|
📓 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:
|
…og-action-restore-retired (#8315) `registry.ts`'s semantic table is GENERATED from `entries/` (#7297), and `spec-changes.json` + `docs/protocol-upgrade-guide.md` are projections of that registry. The entry file alone leaves the id absent from all three, which is exactly what `check-adr-0087-registration` reads when it resolves a changeset's `registered <id>` marker. Ran, in order: pnpm --filter @objectstack/spec gen:migration-registry pnpm --filter @objectstack/spec gen:spec-changes pnpm --filter @objectstack/spec gen:upgrade-guide Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
… retirement (#8315) Four locales lose the `sys_audit_log.action.restore` option label. Produced by `node scripts/check-i18n-bundles.mjs --write --filter=audit` (which runs the built CLI's `os i18n extract` with the flags the config's own docstring documents) — not hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
…ire-restore-action
…8315) The merge with main was textually conflict-free, but a conflict-free merge of two independently-regenerated projections is not the generator's output. Measured on this tree: the plain merge result was MISSING two sibling PRs' entries from `spec-changes.json` and `docs/protocol-upgrade-guide.md` — `view-export-options-pdf-removed` (#8010 / PR #8324) and `engine-dotted-projection-refused` (#7589 / PR #8327). Both are present in origin/main's copies of those artifacts; git dropped them while reporting no conflict. `registry.ts` spliced correctly and regenerated byte-identical (78 semantic entries) — the loss was confined to the two prose projections. Not a silent class: against the un-regenerated merge, `check:spec-changes` and `check:upgrade-guide` both FAIL (exit 1) while `check:migration-registry` passes. So this would have been caught — in the merge queue, as an ejection. Regenerating before arming is what makes it cost nothing. Ran on the merged tree, merge committed first: pnpm --filter @objectstack/spec gen:migration-registry pnpm --filter @objectstack/spec gen:spec-changes pnpm --filter @objectstack/spec gen:upgrade-guide Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
…ire-restore-action
…gain (#8315) Third lap of merge-then-regenerate; main moved 25 commits while CI was green and the arm was held. The conflict-free merge again left the two driver-managed prose projections stale, this time missing `datasource-config-url-userinfo-refused` (#8082 / PR #8341): present in origin/main's copies, absent after the merge, restored by regeneration. Round-2's prediction held exactly. `registry.ts` is byte-identical to the plain merge (79 semantic entries) because it is NOT_DRIVER_MANAGED and #8341 added a NEW entry file — the #7297 per-entry-file sharding removes the conflict at the source. The two `merge=os-regen` projections are the ones that needed the generator: spec-changes.json +14 lines, protocol-upgrade-guide.md +3. Regeneration only — no code change, no changeset reclassification, no ratchet. Merge committed first, then: pnpm --filter @objectstack/spec gen:migration-registry pnpm --filter @objectstack/spec gen:spec-changes pnpm --filter @objectstack/spec gen:upgrade-guide Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
Fixes #8315
restoreis retired from thesys_audit_log.actionenum. It is the last unresolvedvalue from #7675's survey, and the least ambiguous member of the family: the
record-level audit writer could not have produced it even by accident, because
actionFor()inplugin-audit/src/audit-writers.tsis typedand its caller early-returns on
null. A tree-wide sweep (below) finds no otherproducer in this repo. There is no undelete capability behind the value either —
soft delete/restore is unbuilt and parked (#1883
pm:on-hold, #3146status:parked).This is the inherited #7675-family ruling —「空 widget + 永远查不到东西的过滤器是可见产品
缺陷;审计面宁窄勿谎」— carried by triage from #8147 / PR #8200, which performed the same
operation on
exportandpermission_change. Not re-adjudicated here.undelete. If the restore capability lands (#1883 / #3146 restart), the enum value
returns with its writer — the emission point, its tests, and the view that surfaces
it — never as a bare enum row again. That sentence is in the code comment, the
changeset, and the ADR-0087 ledger entry, so it survives this PR.
What made it a card and not a tidy-up
Two shipped declarations asserted the opposite, so a declaration-reading audit stopped
and scored the action as covered:
writes_onlylist view offeredrestoreas a filter value — narrowed here to
['create', 'update', 'delete'], which is nowexactly what
actionFor()can emit. (The card's text calls this viewdata_changes;its actual name in
sys-audit-log.object.tsiswrites_only. Same line, samefilter — noting it so the discrepancy is not read as a second surface.)
plugin-audit/src/auth-event-audit.ts:11, which saidthe writer emits "
create/update/delete/restoreand nothing else". That wasfalse the day it was written and contradicted by the type signature four files away —
the ADR-0049 declared-≠-enforced shape in its purest form ([security] verify the unverified "hashed / encrypted at rest" claims on
sys_scim_provider.scim_tokenandsys_oauth_application.client_secret— both are plainField.textand API-readable #8011): a sentence next toa mechanism, with nothing in CI able to tell. Corrected, with the correction recorded
in place rather than silently overwritten.
The invariant the comment was really claiming — every declared action has a writer —
is now a pin test with the writer inventory written as literals, not prose. It
asserts set equality in both directions, so it also catches the strictly worse inverse:
an action the platform writes and the object does not declare.
Consumer sweep for
restoreEvery hit read, not counted.
plugin-audit/.../sys-audit-log.object.ts:125plugin-audit/.../sys-audit-log.object.ts:55writes_onlyview filterplugin-audit/src/translations/*.objects.generated.ts:27(4 locales)plugin-audit/src/auth-event-audit.ts:11platform-objects/.../system_overview.dashboard.tsrestorewidget exists — the open widget card #8148 is aboutpermission_changeonlyNo test, fixture, dogfood case or dashboard dataset in this repo references the audit
restoreaction. Everything else the sweep surfaced is a different vocabulary thatmerely shares the word, and is deliberately untouched: the
ObjectPermissionoperationenum (
read/create/update/delete/transfer/restore/purgein plugin-security,spec/src/security/explain.zod.ts,spec/src/api/protocol.zod.ts,client), thederived
ApiOperationverb list (spec/src/data/object.zod.ts:35,64), therestore_api_keyrecord action onsys_api_key, andsys-attachment.object.ts:138'srestore: falsecapability declaration.Downstream, in other repos — reported, not touched
apps/console/src/pages/system/AuditLogPage.tsxlistsrestoreinACTION_OPTIONS(the audit-log filter dropdown, so this is the user-visible half:an operator can pick a value that returns nothing) and in
ACTION_VARIANT;packages/plugin-detail/src/HistoryTimeline.tsxhas arestorearm in its ownACTION_VARIANT. objectui#4476 already covers this console surface for the otherretired values and needs
restoreadded to its scope. Not edited from this PR.packages/service-cloud/src/audit-helpers.ts:60hand-mirrors the actionvocabulary as a TS union on
recordAuditRow, and that union still names bothrestoreandexport(the latter already retired by audit-log (C): retireexport/import/permission_changefrom thesys_audit_logaction enum and its in-repo consumer surfaces (ADR-0087 registration) #8147). No caller passeseither — the two live call sites pass
updateanddelete— so nothing writes anundeclared row today; the union is a stale copy of a contract that has moved twice.
Reported for routing, not edited.
Ablation (predicted before measured)
Mutation: put
restoreback in both places insys-audit-log.object.ts, everythingelse held.
restore is not declared by the action enumrestore is not named by any shipped list-view filterwrites_only)every list-view action filter names a value the enum still declaresthe action enum declares exactly the actions that have a writerimportcase + rest of suitecheck:i18n(plugin-audit)check:migration-registry/check:spec-changes/check:upgrade-guideplugin-audittypecheckZero divergence, arms predicted at 3 failures and measured at exactly 3. The negative
arms are the load-bearing half: the ADR-0087 registration is not a detector, and
neither the type system nor the runtime can see this class of defect — every
sys_audit_logfield isreadonly: trueandvalidateRecordskips readonly fields, sore-adding a value rejects nothing anywhere. The only two mechanical detectors are the
pin test and the i18n bundle ratchet, which is exactly why the pin test's expectations
are literals: expectation and reality must not derive from the same source.
Breaking-change classification
minoron@objectstack/plugin-auditand@objectstack/spec, with arefactor(...)!commit and a BREAKING changeset body — the #8147 / PR #8200 classification,
unchanged. Retiring a declared enum value is breaking;
majoris refused bycheck-changeset-no-majorunder the launch-window lockstep convention, so the break iscarried by the
!and the changeset prose rather than by the semver digit. Both packagessit in the same
fixedgroup in.changeset/config.json, so they move together anyway.ADR-0087: registered as the semantic migration
audit-log-action-restore-retired(D3 step 17) — a separate entry from #8147's
audit-log-action-enum-retired, perentries/README.md: two cards registering different entries merge clean, while two cardsediting the same entry collide in git, which is the behaviour that layout deliberately
buys on a registry where a dropped entry produces no error anywhere. An enum-VALUE
retirement, so nothing lands in
RETIRED_KEYS_BY_MAJORand the four surface ratchets arebyte-identical by construction — verified,
check:authorable-surfacepasses with 1266defaults unchanged.
check-adr-0087-registration, and not for a missing marker. The marker was presentfrom the first commit; the gate resolves the id against the generated
registry.ts,which had not been regenerated yet. The entry file alone is not enough — the three
projections (
registry.ts,spec-changes.json,protocol-upgrade-guide.md) have to beregenerated and committed, which is the second commit here.
Verification
All run locally in this worktree, foreground, serialized on the shared lock.
plugin-audit— 216 tests / 14 files pass (213 + the 3 new cases).plugin-audittypecheck,spectypecheck — pass.check:nul-bytes,check:empty-changeset,check:adr-0087-registration,check:changeset-gate-self-tests(coverscheck-changeset-no-major),check:i18n(all 9 packages in sync),
check:adr-anchors,check:docs-audit-scope,check:engine-double-contract, andspec'scheck:migration-registry,check:spec-changes,check:upgrade-guide,check:authorable-surface— all pass.node scripts/check-i18n-bundles.mjs --write --filter=audit(which runs the built CLI'sos i18n extractwith the flags theconfig's own docstring documents) — not hand-edited. It produced a real diff: one
removed option label in each of the four locales.
#7675 cannot close on this PR
restorewas its last unresolved enum value, but two sub-issues remain open and bothare named in #7675's own symptom text:
permission_changewidget fromsystem_overview.dashboard.tsand align the dashboard's audit tiles with the post-retirement action set #8148 —system_overview.dashboard.tsstill shipsfilter: { action: 'permission_change' }, a widget that counts a retired value. Verified still present.ACTION_OPTIONS/ACTION_VARIANTsurface, which nowneeds
restoreadded to its scope alongsideexport/permission_change.Post-merge regeneration round (main moved 7 commits underneath this branch)
origin/mainwas merged in atcca6f857and the three ADR-0087 projections wereregenerated on the merged tree at
c985c4a6. Regeneration was not a no-op, and themeasurement is worth recording because it is the whole reason this round exists.
packages/spec/src/migrations/registry.tspackages/spec/spec-changes.jsondocs/protocol-upgrade-guide.mdWhat the plain merge was missing, in both prose projections, was two sibling PRs'
entries —
view-export-options-pdf-removed(#8010 / PR #8324) andengine-dotted-projection-refused(#7589 / PR #8327). Both are present inorigin/main's copies; both were absent after the conflict-free merge; both came back onregeneration. Verified by counting occurrences in all three versions of each file.
The mechanism, stated correctly
This is not git silently losing data, and the commit message on
c985c4a6overstatesit in exactly that direction — I am correcting it here rather than force-pushing an
amended message.
.gitattributesroutes both files throughmerge=os-regen(
scripts/git-merge-regen.mjs), a driver that deliberately refuses to text-mergegenerator-owned artifacts: it takes one side, marks the artifact deferred, and a commit
hook then blocks the commit until the generator has run. That hook fired on my commit and
cleared itself once both files were current:
So the "missing" entries were the designed intermediate state, not corruption.
registry.tsis deliberately not driver-managed (it is listed underNOT_DRIVER_MANAGED) and text-merged fine, which is #7297's per-entry-file shardingworking as intended at the source.
Severity, measured rather than asserted
Against the un-regenerated merge:
check:spec-changescheck:upgrade-guidecheck:migration-registrySo this class does not ship silently — it is caught. The cost it would have been caught
at is the merge-queue ejection, which is what regenerating beforehand buys.
merge driver is a local facility:
.gitattributessays so explicitly, and notes that theGitHub merge queue rebuilds each PR server-side where no custom merge driver runs.
spec-changes.jsonandprotocol-upgrade-guide.mdare single files, not sharded. Ifanother ADR-0087 entry lands on main before this PR is queued, the same
merge-then-regenerate lap is needed again.
Re-verified on the merged tree
check:migration-registry(78 semantic / 27 retired-key / 53 retired-def),check:spec-changes,check:upgrade-guide,check:adr-0087-registration(marker stillresolves),
check:i18n(9 packages in sync) — all PASS. Plus, since main moved sevencommits including objectql behaviour changes:
plugin-audit216/216 tests pass,plugin-audittypecheck PASS,spectypecheck PASS. The retirement itself is intactafter the merge — the enum is
['create', 'update', 'delete', 'login', 'logout', 'config_change', 'import']and thewrites_onlyfilter is['create', 'update', 'delete'].Changeset classification untouched, no ratchet touched — this round was regeneration only.
Generated by Claude Code
Generated by Claude Code