Skip to content

audit-log (A): add login/logout writers on the auth session hooks, and attribute the unattributed last_login_at update row #8144

Closed

Description

@hotlong

Sub-issue A of #7675, split by the triage seat (session session_01GiG1DfMysjbbFLZErAo93G, 2026-08-12) carrying the maintainer ruling of 2026-08-12 (comment 5261744983 on #7675). Parent stays the coordination node; this card is the domain:identity half.

Ruling carried (verbatim, binding — not re-adjudicable)

补 writer(3 个):login / logout(auth 事件已有钩点,顺带解决那条 user_id null 的未归因 last_login_at diff 行)…

Scope

  • Emit sys_audit_log rows with action: 'login' / 'logout' from the auth session lifecycle hooks in packages/plugins/plugin-auth (the hook points already exist per the ruling).
  • Fix the incidental defect named by the ruling: today the only trace of a login is an unattributed update sys_user row (user_id null) diffing last_login_at — the login row must carry the actor, and the last_login_at system write should be attributed or excluded per existing audit conventions.
  • Acceptance: the parent's reproduction inverts — fresh boot, sign-in, then GET /api/v1/data/sys_audit_log?$filter={"action":"login"} returns the event with actor and tenant. The shipped auth_events list view starts showing rows.

Out of scope

  • ⛔ config_change (sub-issue B, domain:services).
  • ⛔ The enum retirement (sub-issue C). Note C edits packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts — if this card also needs edits inside plugin-audit (e.g. audit-writers.ts), it shares a package with C: same-lane serialization applies, do not run A and C in flight together.
  • ⛔ Console/objectui surfaces (sub-issue in objectui).

No Blocked-by: — login/logout remain enum members; this half is independent of the retirement ordering.

Size/model suggestion: M, opus.

Activity

  1. added theissue type on Aug 12, 2026
  2. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 12, 2026
  3. self-assigned this
    on Aug 12, 2026
  4. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Claim: domain:identity 执行席位 #6022 — session session_01PEVB6w7D7uCszR9Mw1BL73,分支 claude/issue-8144-audit-login-logout-writers。派发中(mode:subagent,M / opus,依卡片建议)。

    串行判据的更正 —— 我先前设的门槛比卡片要求的更严,且严错了地方

    我一直把本卡的前置写成「#8175 已合并 且 #8147 已合并」。重读卡片原文后更正:

    • 卡片的约束是「do not run A and C in flight together」—— 约束的是两个 dev 同时在 plugin-audit 里写,不是"C 必须先落地"。
    • 卡片还明确写着「No Blocked-by: —— login/logout 仍是 enum 成员,这一半独立于退役的顺序」。

    C 的状态:PR #8200 已开、CI 26 job 全绿、已武装入队,dev 已完工、worktree 已移除。并发写入者不存在了,所以卡片要防的那件事已经不成立。继续等它合并,只是在为一个已经消失的危险付吞吐的代价。

    另外 #8147 不会因 #8200 合并而关闭(那个 PR 是 Part of 而非 Fixes,import 一项已升级待裁),所以"等 #8147 关闭"本来就是一条永远不会满足的判据 —— 这也是我更正它的原因之一。

    ⚠️ 但真实的残余风险仍要交代给 dev,而不是当作不存在:#8200 此刻在队列里,会在几分钟内落地 plugin-audit,它改的是 src/objects/sys-audit-log.object.ts(action enum + 两个 list view filter)与四个翻译 bundle。本卡若需要动 plugin-audit,大概率落在 src/audit-writers.ts —— 不同文件,git 可干净合并;但 dev 必须在开 PR 前 merge origin/main 并重跑,冲突则报告而不是靠猜解决。

    另一条实测事实,直接影响本卡怎么写测试(来自 #8147 的 dev,已开为 #8203):

    sys_audit_log 的每个字段都是 readonly,而 validateRecord 在两个分支上都跳过 readonly/system 字段 —— 也就是说 action 的 enum 声明了一套两个方向都无人校验的词汇表。⚠️ 对本卡的含义:写入一个 enum 里没有的 action 不会被拒绝、不会变红。所以"我写了 login 行"这件事不能靠"没有报错"来证明,必须靠读回那一行来证明。已写进派发令。

    File surface 申报:packages/plugins/plugin-auth(session 生命周期钩点),可能触及 packages/plugins/plugin-audit/src/audit-writers.ts。本 lane 在飞的另外三个(#8009 实现半在 plugin-auth/src/objectql-adapter.ts、#8193 在 register-sso-provider.ts、#8095 在 plugin-security)与之实测不相交。


    Generated by Claude Code

  5. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor
    {
      "issue": 8144,
      "status": "done",
      "branch": "claude/issue-8144-audit-login-logout-writers",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8244",
      "premise_still_valid": true,
      "summary": "plugin-audit now registers an `audit` service (src/auth-event-audit.ts) — the ledger's write ingress for non-CRUD events — and plugin-auth emits `login` from better-auth's `session.create.after` and `logout` from `session.delete.after` under `/sign-out`, resolving the sink lazily through a locally-declared structural surface so neither package depends on the other. The row shape stays owned by plugin-audit (spec/system/index.ts records that ownership explicitly); the caller hands over an EVENT with a closed `login | logout` union, which is the only structural protection available given #8203. `stampLastLogin` now carries `attributedUserId` (#4586), so the `last_login_at` diff row names the person who signed in. Merged origin/main after #8200 landed — clean merge, no conflict (it touched objects/sys-audit-log.object.ts + translations, this card touches audit-writers.ts and new files); `login`/`logout` were untouched by the retirement, and the enum was not adjusted.",
      "tests": "ALL FOREGROUND, under flock. plugin-audit `pnpm test`: 14 files / 213 passed (6 new sink cases, read back through a real ObjectQL engine). plugin-auth `pnpm test`: 47 files / 1113 passed (20 new composition cases). typecheck plugin-audit + plugin-auth + dogfood: clean. Dogfood acceptance `auth-session-audit-trail.dogfood.test.ts` 5/5, run together with the two neighbouring audit-reading suites (admin-identity-audit-trail, membership-actor-attribution): `Test Files 3 passed (3) / Tests 14 passed (14)`. READ-BACK, not no-throw: the acceptance case issues the issue's own route — `GET /data/sys_audit_log?$filter={\"action\":\"login\"}` as admin — and asserts action + `user_id` + `tenant_id`, with the tenant compared against the `sys_member` row the platform wrote (a different source from anything the writer passes in) and the actor confirmed to resolve to a real `sys_user`. GATES: check:nul-bytes OK (7451 files); check:durability-log-level OK (25 seams, all loud — `persistAuthEventAuditRow` added to DURABILITY_CRITICAL_CALLEES in the same PR); check:engine-double-contract OK (175 pinned / 133 DEBT / 2 exempt, unchanged — the new tests use a real engine + memory driver, not an engine double); check:query-options-erasure OK (baseline key set verified, no files added); check:type-check-debt OK after a FULL workspace build closure from the repo root (`turbo run build --filter=./packages/* --filter=./packages/*/*`, 70/70) — 33 entries re-measured, none above its recorded number, nothing raised, no ledger edited; check:type-check-coverage, check:cross-package-test-inputs, check:test-source-alias, check:docs-audit-scope, check:changeset-gate-self-tests, check:objectui-changeset, check-changeset-no-major all OK. ONE RED, PRE-EXISTING AND NOT MINE: `check:objectui-pin-fresh` reports `.objectui-sha` stale (#3340). That file was last touched by #8137 and this branch changes zero objectui files — the gate is derived only because the diff adds a `.changeset/` entry. Needs a pin refresh at release time, unrelated to this card.",
      "ablation": "PREDICTED FIRST, written to disk before mutating. Attempt 1 (make `loginEventFor` return null) was DISCARDED as a contaminated signal, reported rather than hidden: unreachable code loses narrowing, so the ablation itself failed the DTS build — a different diagnostic class, not the behavioural red being measured. Attempt 2 removed the WRITER at the composition seam (`emitAuthSessionAuditEvent(undefined, loginEventFor(...))`), leaving every symbol referenced and types valid. PREDICTED: 6 plugin-auth cases red (the `session.create.after` block minus the two whose expectation is `not.toHaveBeenCalled`), 0 plugin-audit, 2 dogfood red (the login query and the auth_events view, the latter because it runs before any logout row exists), 3 dogfood green (logout, revoke-is-not-logout, last_login_at attribution). MEASURED: exactly that — 6 / 0 / 2 red, 3 green, with the dogfood failure printing `never returned the member's login — last saw 0 row(s): []`, literally #7675's `total 0`. Direction: PLAIN RED. Not 'more diagnostics', not inverted. ONE PROCESS LESSON WORTH THE PM'S ATTENTION: the first dogfood ablation run passed GREEN because dogfood resolves plugin-auth through its built `dist/`, which still held the pre-ablation build — a false green that would have certified a vacuous test. Caught by grepping the dist for the ablation marker; every ablation run after that rebuilt first.",
      "last_login_at_decision": "ATTRIBUTED, not excluded (the ruling allowed either). Convention followed: `ExecutionContext.attributedUserId` (#4586, `auth-actor-attribution.ts`) — the platform's one channel for 'the human CREDITED for a write the system authorized', which the audit writer already reads as `ctx.provenance.attributedUserId` into `sys_audit_log.user_id`. Passed explicitly rather than via `authSystemWriteContext()`, because the ambient actor scope is filled from the request's session and on `/sign-in/email` there is no session yet when that hook runs. `isSystem: true` is unchanged, so nothing about who may write `sys_user` moves. Excluding was rejected: it would mean adding `last_login_at`/`last_login_ip` to the CRUD writer's repo-wide `NOISE_FIELDS`, which deletes the `last_login_ip` change trail for every object and every deployment — and a login from a new address is exactly what a compliance ledger is read for. The defect the ruling named is the missing actor; this supplies the actor and keeps the row.",
      "consumption_sweep": "39 test files reference `sys_audit_log`; every one triaged. (1) The two dogfood suites that boot AuditPlugin AND sign in for real — `admin-identity-audit-trail` and `membership-actor-attribution` — are the only ones that could see new rows. Both filter by `object_name`/`record_id` (`sys_user`, `sys_member`, `sys_account`); the new rows carry `object_name: 'sys_session'` with a session id, so nothing overlaps. Both re-run GREEN post-merge alongside the new file. (2) `admin-identity-audit-trail` asserts ABSENCE twice (`object_name: 'sys_account'` → 0 rows; `record_id: <account id>` → 0 rows) — neither can match a session-scoped row. (3) plugin-security / plugin-sharing / objectql / spec references are fakes, name constants or registry fixtures with no auth session in play. (4) `storage-growth.dogfood` counts its own `growth_probe_*` objects, not the ledger. (5) EMPIRICAL BACKSTOP: an early full-suite dogfood run (102 files, 689 tests) executed WITH this change and the only failure was my own tenant assertion — no neighbour broke. (6) Intended new-row consumers: the `auth_events` list view and the two `system_overview` widgets start showing data, which is the point of the card. NO test asserted 'no login rows exist', so nothing had to be triaged as a correct failure. Console/objectui `ACTION_OPTIONS` untouched (separate objectui sub-issue).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8245 (label `finding`, unassigned): a user's FIRST session is minted before the ADR-0093 membership reconciler runs (better-auth defers `user.create.after` past the signup transaction), so it carries `activeOrganizationId: null` and every audit row written in that window has a NULL tenant — permanently invisible to the RLS predicate, the exact consequence `audit-writers.ts` documents for the sibling cause. Pre-existing ordering property, not introduced here; #8144 only made it observable by putting a row on the ledger at that moment. Related but distinct from #8208 (same RLS consequence, different producer) and #8158 (same precondition, opposite direction). No fix proposed — settling membership before the first session has a real cost on the signup hot path.",
        "NOT filed, reported for context: `check:type-check-debt` reports 265 raw errors of SURPLUS across 9 ledger entries (plugin-auth TEST_DEBT records 131, measures 114). Pre-existing, explicitly 'not an error' per the gate, and already tracked by #6376. Deliberately not lowered — out of this card's scope, and three other agents are live in `packages/`."
      ]
    }

    Generated by Claude Code

  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor
    {
      "issue": 8144,
      "status": "done",
      "branch": "claude/issue-8144-audit-login-logout-writers",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8244",
      "premise_still_valid": true,
      "summary": "Patch round for the queue CI_FAILURE. Merged current origin/main FIRST (so the re-measure ran against the new ceiling, not the old one), then fixed all 6 TEST_DEBT errors in `auth-session-audit.test.ts`. Root cause: `vi.fn(async () => undefined)` — an implementation declaring no parameters — types `mock.calls` as `[][]`, a ZERO-LENGTH tuple, so every `calls[0][0]` read past the end of a tuple TypeScript believed was empty (TS2493 x3) and the dereference that followed was `possibly undefined` (TS18048 x3). Fixed by declaring what the spy receives rather than softening the read: the spy's implementation names its parameter as `AuthSessionAuditEventInput`, and the call log is pulled through two small helpers (`recordedEvents` / `firstEvent`) that name the 'never called' case instead of letting it surface as a TypeError. Ledger NOT touched, no `any`-cast, no `@ts-expect-error`, no assertion deleted — the fix strengthens the file: pinning the element type means these assertions now type-check against the real event surface, so a renamed field fails at compile time instead of quietly comparing `undefined` to `undefined`, and one `(c: any[])` map disappeared with it. The earlier feature commits are unchanged.",
      "tests": "ALL FOREGROUND under flock, on the merged tree, after a FULL build closure (`turbo run build --filter=./packages/* --filter=./packages/*/*`, 70/70 successful). MEASUREMENT, reproduced before and after: I replicated the gate's own re-measure projection (`remeasureProject` + `defaultTypeRoots`, config written OUTSIDE the repo — 021d8dd10's move confirmed, nothing lands in `packages/**`). BEFORE: 117 total, ceiling 111, and exactly 6 attributable to my file — matching the queue log line for line. AFTER: **111 total, 0 from my file** — exactly at the ceiling. AUTHORITATIVE GATE from the repo root: `check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 248.9s, 1969 raw tsc error(s) total, none above its recorded number.` and now `surplus: none — every entry sits exactly at its measurement, so any new error is red` (#7888's headroom is fully closed and this branch does not re-open it). REGRESSION CHECK: plugin-auth 47 files / 1113 passed, plugin-audit 14 files / 213 passed — identical to the pre-fix counts, so the 20 composition cases still assert what they asserted. typecheck (plugin-auth, plugin-audit, dogfood) clean. check:nul-bytes OK (7532 files). Dogfood re-verified on the merged tree, my file plus the two audit-reading neighbours: `Test Files 3 passed (3) / Tests 14 passed (14)`. Ledger diff against origin/main for `scripts/check-type-check-coverage.mjs`: EMPTY.",
      "open_questions": [],
      "out_of_scope_findings": [
        "already filed as #8245 (label `finding`, unassigned) in the first round: a user's FIRST session is minted before the ADR-0093 membership reconciler runs, so every audit row written in that window carries a NULL tenant and is invisible to the RLS predicate. Unchanged by this patch round.",
        "no new findings this round. The `check:objectui-pin-fresh` red reported previously is still pre-existing and still untouched by this branch (`.objectui-sha` last moved by #8137; zero objectui files in this diff)."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingpm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions