Repository navigation
sys_audit_log.action declares restore with no writer anywhere — actionFor() structurally cannot return it, while a comment and a shipped list view both assert it is covered #8315
Description
Activity
Triage: lands in
packages/plugins/plugin-audit→domain:identity+pm:queue, type Bug (the enum declares a valueactionFor()structurally cannot produce, a shipped comment asserts coverage that does not exist, and a shipped list view filters on a value that can never match — declared ≠ enforced).Inherited ruling, not a new decision. The maintainer's #7675-family ruling —「空 widget + 永远查不到东西的过滤器是可见产品缺陷;审计面宁窄勿谎」— already adjudicated this exact shape for
export/permission_change(#8147 → PR #8200). Per the sibling-reuse rule I re-checked the ruling's reason against this branch before carrying it over:restoreis materially identical (declared enum value, no writer anywhere, permanently-unmatchable shipped filter), and the one candidate difference — that a restore capability might be intended — is already parked by the maintainer (#1883pm:on-hold, #3146status:parked: soft-delete/restore is unbuilt). No real semantic difference ⇒ the ruling carries with its reason.Scope on inherit (the #8147 / PR #8200 shape):
- Retire
restorefrom theactionenum, thedata_changesview filter, and the generated translation labels. - Correct the false coverage comment at
auth-event-audit.ts:11— wrong today under any disposition. - If/when the restore capability lands ([security][立项位] M2 权限生命周期(undelete/purge)功能与 allowRestore/allowPurge RBAC 同批建设(evaluator 已 fail-closed,allowTransfer 已 enforced) #1883/Implement soft delete (recycle bin):
enable.trash/softDeleteare spec-only with zero runtime readers #3146 restart), the enum value returns with its writer — note this in the PR body so the retirement is not read as a permanent product stance.
Veto window open: if the maintainer prefers giving
restorea writer now, that is #1883's scope and reverses this card.Cross-link for the identity seat: this was the last unresolved value from #7675's survey — once this lands, re-check whether #7675 can close.
本评论来自分诊座位 Routine。
Generated by Claude Code
- Retire
Claiming this card.
- Session:
session_01PEVB6w7D7uCszR9Mw1BL73 - Branch:
claude/issue-8315-retire-restore-action - Worktree:
../objectstack-8315
Following the inherited #7675-family ruling as carried by triage (retirement, not re-adjudication), templated on #8147 → PR #8200 / commit
344a22a82.
Generated by Claude Code
- Session:
{ "issue": 8315, "status": "done", "branch": "claude/issue-8315-retire-restore-action", "pr": "https://github.com/objectstack-ai/objectstack/pull/8325", "head_sha": "02694c4701667ee186eb49031baac8625213fe9d", "premise_still_valid": true, "summary": "Retired `restore` from the sys_audit_log.action enum, from the shipped list-view filter (the view is named `writes_only`, not `data_changes` as the card's text says — same line 55, same filter), and from the four generated translation bundles (regenerated with the repo's own generator, real diff: one option label removed per locale). Corrected the false coverage comment at auth-event-audit.ts:11 and, because that comment was an unenforced declaration (the #8011 shape), turned the invariant it claimed into a pin test with the writer inventory as literals. Registered under ADR-0087 as a NEW semantic entry `audit-log-action-restore-retired` (separate from #8147's, per entries/README.md: different entries merge clean, same entry collides) with all three projections regenerated. Followed the #8147 / PR #8200 template throughout, including the breaking-change classification.", "tests": "plugin-audit: 216 tests / 14 files PASS (213 pre-existing + 3 new). plugin-audit typecheck PASS, spec typecheck PASS. Gates all PASS: check:nul-bytes, check:empty-changeset, check:adr-0087-registration, check:changeset-gate-self-tests (covers check-changeset-no-major), check:i18n (9 packages in sync), check:adr-anchors, check:docs-audit-scope, check:engine-double-contract, spec check:migration-registry / check:spec-changes / check:upgrade-guide / check:authorable-surface (1266 defaults unchanged, confirming the surface ratchets did not move). Ablation zero-divergence: predicted 3 failures, measured exactly 3 (213 passed alongside). CI at this head: Check Changeset SUCCESS (the PM-relayed failure is resolved), ESLint battery SUCCESS, Build Core SUCCESS, Dogfood Verify CLI SUCCESS, Temporal Conformance SUCCESS; Test Core 1-3, TypeScript Type Check and Dogfood Regression 2-3 still in_progress at report time — not waited on, per the report-at-draft-PR-time rule.", "open_questions": [], "out_of_scope_findings": [ "filed as cloud#1274: service-cloud's recordAuditRow hand-mirrors the sys_audit_log action vocabulary as a TS union that still names `export` (retired by #8147) and `restore` (retired here). Dormant — both live call sites pass update/delete — but the write path bypasses the engine and the schema, so a future caller reaching for a stale arm would store an undeclared row with nothing going red.", "NOT filed, reported for routing: objectui apps/console/src/pages/system/AuditLogPage.tsx (ACTION_OPTIONS filter dropdown + ACTION_VARIANT) and packages/plugin-detail/src/HistoryTimeline.tsx (ACTION_VARIANT) both carry a `restore` arm. objectui#4476 already exists for the other retired values and needs `restore` added to its scope — PM routes, per dispatch instruction.", "NOT filed, already tracked: #8148 is still open and still needed — system_overview.dashboard.ts ships `filter: { action: 'permission_change' }`. Verified still present on this branch." ] }#7675 closability, as asked
It cannot close when this lands.
restorewas its last unresolved enum value, but two of its five sub-issues are still open, and both are named in #7675's own symptom text (not just its sub-issue list):- audit-log (D): remove the
permission_changewidget fromsystem_overview.dashboard.tsand align the dashboard's audit tiles with the post-retirement action set #8148 —system_overview.dashboard.tsstill ships apermission_changewidget counting a retired value. Verified present onorigin/maintoday. - objectui#4476 — the console
ACTION_OPTIONSsurface, which offers retired values as filter choices. Now needsrestoreadded to its scope.
Also measured while checking: #7675's other named defect —
settings-service.types.ts:16documenting "Emit sys_audit_log rows for every successful write" while the service wrote tosys_setting_audit— was fixed by #8145; that docblock now states the dual-write accurately (以实现定契约). And theimportescalation PR #8200 raised is settled:importis still declared, still has its live writer atplugin-auth/src/admin-import-users.ts:515, and is now pinned in both directions by the writer-inventory assertion this PR adds.
Generated by Claude Code
- audit-log (D): remove the
- added a commit that references this issue
on Aug 13, 2026 { "issue": 8315, "status": "done", "round": "post-merge regeneration (round 2) — no code change, no classification change, no ratchet touched", "branch": "claude/issue-8315-retire-restore-action", "pr": "https://github.com/objectstack-ai/objectstack/pull/8325", "head_sha": "c985c4a61037f1b4b9e242b0916e3a4f73496fe1", "premise_still_valid": true, "summary": "Merged origin/main (7 commits behind by the time I started, not 6) at cca6f857 and regenerated the three ADR-0087 projections on the merged tree at c985c4a6. Regeneration was NOT a no-op: registry.ts came back byte-identical, but spec-changes.json (+26 lines) and protocol-upgrade-guide.md (+10) had both LOST two sibling PRs' entries in the conflict-free merge — view-export-options-pdf-removed (#8010/PR #8324) and engine-dotted-projection-refused (#7589/PR #8327). Regeneration restored both. Your instinct was right; the mechanism is not what either of us assumed, and I correct my own commit message below.", "regeneration_vs_plain_merge": { "packages/spec/src/migrations/registry.ts": "BYTE-IDENTICAL — spliced correctly, regenerated to the same 78 semantic / 27 retired-key / 53 retired-def. Explicitly stated as you asked: for this file the plain merge was already the generator's output.", "packages/spec/spec-changes.json": "DIFFERED, +26 lines — was missing both sibling entries (2 occurrences each of the two ids, matching origin/main's copy exactly after regeneration)", "docs/protocol-upgrade-guide.md": "DIFFERED, +10 lines — was missing both sibling entries (1 occurrence each), including #8324's row in the 'Mechanical (applied for you)' table and its step-17 prose paragraph", "verification_method": "counted occurrences of each id in three versions of each file — origin/main's, the plain-merge result (snapshotted before regenerating), and the regenerated output. My own entry survived the merge on all three; main's two did not survive on two of three." }, "the_mechanism_corrected": { "what_it_is_not": "NOT git silently dropping data on a clean merge. My commit message on c985c4a6 says 'git dropped them while reporting no conflict', which overstates it in exactly that direction. I cannot amend a pushed commit without a force-push, so the correction lives in the PR body and here.", "what_it_is": ".gitattributes routes both files through `merge=os-regen` (scripts/git-merge-regen.mjs) — a driver that DELIBERATELY refuses to text-merge generator-owned artifacts, takes one side, marks them deferred, and lets a commit hook block until the generator has run. That hook fired on my commit and cleared itself: 'os-regen: 2 generated artifact(s) were merged WITHOUT a text merge and must be regenerated from the merged tree before this commit' then '✓ … current — marker cleared.' The 'missing' entries were the designed intermediate state, not corruption.", "why_registry_ts_behaved_differently": "registry.ts is deliberately NOT driver-managed (listed under NOT_DRIVER_MANAGED alongside the conversions registry) and text-merged normally — which is #7297's per-entry-file sharding working as intended at the SOURCE, and the same property that made my new-entry-file choice correct in round 1." }, "severity_measured_not_asserted": { "gates_against_the_UN-regenerated_merge": { "check:spec-changes": "FAIL (exit 1)", "check:upgrade-guide": "FAIL (exit 1)", "check:migration-registry": "PASS" }, "conclusion": "This class does not ship silently — it is caught. What it would have been caught AT is the merge-queue ejection you are trying to avoid, and that is exactly what regenerating beforehand buys. So the round was worth doing, for the reason you gave rather than for data loss." }, "durability_warning": "This round holds only until main next touches those two files. .gitattributes states the bound itself: the driver is a LOCAL git facility and 'the GitHub merge queue rebuilds each PR server-side, where no custom merge driver runs'. The three hottest artifacts were SHARDED for that reason; spec-changes.json and protocol-upgrade-guide.md are single files and were not. If another ADR-0087 entry lands on main before #8325 is queued, the same merge-then-regenerate lap is needed again. Arming sooner is strictly cheaper than arming later here.", "checks_you_asked_for_on_the_merged_tree": { "check:migration-registry": "PASS — self-test ok; registry current (78 semantic, 27 retired-key, 53 retired-def)", "check:spec-changes": "PASS — 'spec-changes.json is up to date.'", "check:upgrade-guide": "PASS — 'protocol-upgrade-guide.md is up to date.'", "check:adr-0087-registration": "PASS — '1 declared-breaking changeset(s), each carrying an ADR-0087 disposition … [BREAKING+bang] registered audit-log-action-restore-retired'; the marker still resolves against the merged registry", "check:i18n": "PASS — 9 packages, all bundles in sync, no undeclared authoring keys (required a full CLI rebuild in the recreated worktree, since the gate runs the built CLI)" }, "extra_checks_i_added_because_main_moved_7_commits": { "reason": "the incoming commits included objectql behaviour changes (#8327 dotted-projection refusal, #8322 number scale by rejection, #8319 preserveAudit primary key) that could plausibly reach plugin-audit", "plugin-audit test": "PASS — 216/216, 14 files, unchanged from round 1", "plugin-audit typecheck": "PASS", "spec typecheck": "PASS", "retirement intact after merge": "verified by reading the merged file: enum is ['create','update','delete','login','logout','config_change','import'] and the writes_only filter is ['create','update','delete']" }, "ci_at_new_head": { "read_once_not_polled": true, "totals": "26 checks: 20 success, 2 skipped, 4 in_progress, 0 failed", "in_progress": ["Test Core 1/3", "Test Core 2/3", "Test Core 3/3", "TypeScript Type Check"] }, "unchanged_as_instructed": [ "changeset classification (still minor + refactor(...)! + BREAKING body)", "no ratchet, ledger or baseline touched", "no content/docs/releases/ edit", "no git stash — the plain-merge snapshot and restore were done with cp into my scratchpad", "PR still DRAFT, not queued, no auto-merge" ], "state_i_did_not_set": "The PR body now carries TWO attribution footers: my session-URL form, immediately followed by a bare-form '_Generated by [Claude Code](https://claude.ai/code)_' that some automation appended during this edit (it was not there after my round-1 edit). Surfacing rather than 'correcting' it, per the don't-touch-another-actor's-state rule. The body still ends with the required footer text either way.", "open_questions": [], "out_of_scope_findings": [ "unchanged from round 1: filed as cloud#1274 (service-cloud recordAuditRow's stale action union naming export + restore); objectui AuditLogPage.tsx / HistoryTimeline.tsx reported for routing to objectui#4476; #8148 still open and still needed" ] }
Generated by Claude Code
- added a commit that references this issue
on Aug 13, 2026 { "issue": 8315, "status": "done", "round": "post-merge regeneration (round 3) — regeneration only", "branch": "claude/issue-8315-retire-restore-action", "pr": "https://github.com/objectstack-ai/objectstack/pull/8325", "head_sha": "5f8870beaa2ec9c1d8ae011137e5f105f15b13dc", "pushed_at_report_time": true, "premise_still_valid": true, "summary": "Merged origin/main (25 commits) at d5be6472, regenerated the three projections at 5f8870be, pushed. Your prediction held exactly. But the headline is elsewhere: this lap was almost certainly UNNECESSARY, and I can now show that with a measurement rather than an argument — which bears directly on the livelock.", "your_prediction_held": { "packages/spec/src/migrations/registry.ts": "BYTE-IDENTICAL to the plain merge, as predicted — 79 semantic / 27 retired-key / 53 retired-def. NOT_DRIVER_MANAGED plus #8341 arriving as a NEW entry file is exactly the #7297 sharding case.", "packages/spec/spec-changes.json": "needed regeneration, +14 lines", "docs/protocol-upgrade-guide.md": "needed regeneration, +3 lines", "what_was_missing": "datasource-config-url-userinfo-refused (#8082 / PR #8341) — present in origin/main's copies, absent after the merge, restored by regeneration. Same shape as round 2.", "nothing_new_to_call_out": "registry.ts did NOT differ, so there is no new information hiding in it this lap." }, "checks_requested": { "check:migration-registry": "PASS (79 semantic, 27 retired-key, 53 retired-def)", "check:spec-changes": "PASS", "check:upgrade-guide": "PASS", "check:adr-0087-registration": "PASS — marker still resolves", "check:i18n": "PASS — 9 packages in sync", "extra_because_main_moved_25_commits": "plugin-audit 216/216 tests PASS; retirement verified intact in the merged file (enum ['create','update','delete','login','logout','config_change','import'], writes_only filter ['create','update','delete'])" }, "THE_LIVELOCK_FINDING": { "headline": "The merge queue's driver-less rebuild would have produced generator-identical artifacts for this pair. No ejection risk existed. This lap did not need to happen.", "already_answered_upstream": "#8344 is CLOSED-OUT by PR #8359 (b3b618b52), which landed on main in this very batch — it used THIS branch as one of its five test cases. Its measured answer: 'never stale-but-clean. Both files are sorted unions and a registration is insertion-only, so the queue's text merge either takes both sides — byte-identical to the regeneration, all three check: gates green on the un-regenerated result — or conflicts. It conflicts only on ADJACENT registry ids.'", "i_confirmed_it_independently_on_this_exact_pair": "Re-ran the merge with the os-regen driver replaced by a plain 3-way text merge (`git -c merge.os-regen.driver='git merge-file %A %O %B' merge-tree --write-tree c985c4a61 origin/main`) — i.e. what the queue does with no custom driver registered. Result: ALL THREE files byte-identical to my regenerated output, spec-changes.json and protocol-upgrade-guide.md included.", "why_my_local_laps_kept_seeing_stale_files": "Because the LOCAL os-regen driver deliberately declines to text-merge and takes one side, leaving a marker. That is the pessimistic path and it is local-only. The queue has no such driver, so it performs the union the artifacts are actually shaped for. My three laps were measuring the driver, not the queue.", "adjacency_for_this_pair": "In the sorted step-17 semantic list (69 entries), `audit-log-action-restore-retired` is #11 and `datasource-config-url-userinfo-refused` is #24 — twelve entries apart. Non-adjacent, so per #8359 the queue unions cleanly, which is exactly what I measured.", "what_this_buys_you": "The merge+regenerate precondition can be narrowed from 'every time main touches the three projections' to 'only when an in-flight entry id is ADJACENT to mine in the sorted step-N list'. Adjacency is cheap to compute from the entries/ directory listing. That breaks the merge -> regenerate -> 25min CI -> outrun cycle without gambling on anything: for non-adjacent ids the queue is measurably correct, and for adjacent ids the failure is a CONFLICT (blocks loudly) rather than a silent stale splice." }, "ordering_question_for_8344": { "answer": "YES — filename/id-deterministic, and independent of merge order.", "evidence": "build-migration-registry.ts reads the directory with `readdirSync`, iterates `[...names].sort()`, and then sorts entries with `entries.sort((a, b) => (a.major - b.major) || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0))`. The file header states it as a rule: 'Order is DERIVED, never declared. Entries are concatenated sorted by id.'", "why_it_matters": "Deterministic id ordering is precisely what makes a driver-less text union able to equal the generator's output: both sides insert at the position the id dictates, so a 3-way merge of two insertions at different positions reproduces the sorted union. It also explains #8359's one exception — adjacent ids insert at the same anchor, which is the only case that conflicts.", "note": "The generator's own header already anticipates this at line ~66: entries that 'sort ADJACENTLY insert at the same anchor and still conflict textually'." }, "unchanged_as_instructed": [ "regeneration only — no code change, no changeset reclassification, no ratchet, no git stash (plain-merge snapshot taken with cp)", "PR still DRAFT; not merged, not queued, no auto-merge; no domain:* label touched" ], "could_not_verify": [ "CI at 5f8870bea — not waited on, per your instruction to report on push. Round 2's head went 26/26 green and this lap changes only two generated prose files plus the merge.", "That the queue will behave identically at the moment it actually rebuilds — my merge-file emulation reproduces the driver-less fallback faithfully, but it is an emulation of a server-side step, and main can move again before the arm." ], "open_questions": [], "out_of_scope_findings": [ "unchanged: cloud#1274 filed; objectui AuditLogPage.tsx / HistoryTimeline.tsx reported for routing to objectui#4476; #8148 still open and still needed" ] }
Generated by Claude Code
- added a commit that references this issue
on Aug 18, 2026 - added a commit that references this issue
on Aug 19, 2026 - added a commit that references this issue
on Sep 28, 2026
Found by the
domain:identityexecution seat while measuring whether #7675 ("4 of 10 declaredsys_audit_logactions have no writer anywhere") can close. Filed unassigned and unlabeled for triage — I am an execution seat and do not grade findings or routedomain:*.Same defect class as #7675, on a value #7675's own survey did not name. Measured on
origin/main, not inferred.What
packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts:125declares:Nothing writes
restore. The record-level audit writer structurally cannot produce it —packages/plugins/plugin-audit/src/audit-writers.ts:245:restoreis not in the return type, andaudit-writers.ts:1105-1106early-returns onnull. A tree-wide grep for any form ofaction: 'restore'acrosspackages/**/*.tsexcluding tests returns no producer — the onlyrestorehits are unrelated (metadata-protocoluses a different'restored'literal on its own revert report;sys-attachment.object.ts:138explicitly declaresfalserather than "claim a restore capability the runtime does not provide").1. A comment states the coverage that does not exist —
packages/plugins/plugin-audit/src/auth-event-audit.ts:11:It names
restoreas one of the four covered actions.actionFor()'s own signature contradicts it. This is the pattern #8011 was about: a declaration near a mechanism, with nothing enforcing it. A declaration-reading audit stops at this comment and scoresrestoreas covered.2. A shipped list view offers it as a filter value —
sys-audit-log.object.ts:55:The
data_changesview advertises a value that can never match. Harmless to the query, but it is the same "empty widget / permanently-unmatchable filter" shape the maintainer ruled on for #7675: 空 widget + 永远查不到东西的过滤器是可见产品缺陷;审计面宁窄勿谎. Four generated translation files also carry the option label (plugin-audit/src/translations/*.objects.generated.ts:27).Status of #7675's named values, for context
Every value #7675 actually named is now resolved, which is why this one surfaced:
loginauth-session-audit.ts:138(#8144 → PR #8244)logoutauth-session-audit.ts:162(#8144 → PR #8244)config_changeexportpermission_changeimportplugin-auth/src/admin-import-users.ts:515restoreNot adjudicated here
⛔ I have deliberately not decided between the two obvious shapes (give
restorea writer, or retire it from the enum/view/translations per the 宁窄勿谎 principle). That turns on whether a restore/undelete capability is intended to exist — #1883 (pm:on-hold, M2 permission lifecycle undelete/purge) suggests the capability itself is unbuilt, which would make retirement the consistent answer, but that is a product call and not an execution seat's to make. Recorded plainly per the file-plainly rule.Whichever way it goes, the false comment at
auth-event-audit.ts:11needs correcting either way — it is wrong today regardless of the disposition.Related: #7675 (parent survey), #8147 (the retirement that this measurement re-checks), #1883, #8011 (same "unenforced declaration" class), #8203 (a
selectoption enum is unenforceable once the field isreadonly— the mechanism that lets this persist undetected).