Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Repository navigation
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

InboxNotification.actor_name is declared but no producer fills it and no consumer reads it #5203

Closed
#5487
Closed
InboxNotification.actor_name is declared but no producer fills it and no consumer reads it#5203
#5487
Assignees
os-support-ai
Labels
pm:dispatched

Description

@os-support-ai
os-support-ai
opened on Aug 18, 2026

Found while implementing objectui#5190 (which removed the sibling source_object/source_id pair from the same interface). Not fixed there — the ruling on that card named the two source fields only, and the correct disposition here is not mechanically pinned. Filed unassigned as an observation.

What is inconsistent

InboxNotification (packages/app-shell/src/layout/inboxGrouping.ts) declares:

actor_name?: string | null;

On the inbox path that field is dead at both ends:

  • No producer fills it. mergeInboxRows in packages/app-shell/src/hooks/sharedUserFeeds.ts is the single producer of every row both the bell and Home render, and its returned object maps id, notification_id, receipt_id, type, title, body, action_url, is_read and created_at — not actor_name.
  • No consumer reads it. InboxPopover renders title, body and created_at; Home's action centre does not read it either.

Note the field name also occurs on a different shape — the sys_activity row that sharedUserFeeds.ts maps to ActivityItem (user: String(r.actor_name ?? '')), and the ACTIVITY_ROW fixture in AppHeader.inboxVariant.test.tsx. Those are live and are not this field; a grep for actor_name alone will conflate the two.

Why this is only an observation

After objectui#5190 this is the last declared-but-unfilled field on InboxNotification, so it is the same class as the pair removed there. It is milder in one respect that matters: the source pair was read by a click handler that then silently did nothing, which is what made that card a behavioural defect. Nothing reads actor_name, so there is no user-visible symptom today — the cost is the same trap for the next reader, plus the standing invitation to wire it up.

Two candidate dispositions, and choosing is a product call rather than a mechanical fix:

  1. Retire it — same treatment as the source pair: nothing fills it, nothing reads it, and remove beats declare-and-maintain.
  2. Make it live — an actor name on a notification row is a plausible UI affordance (the row could read "Li Si assigned you…" rather than just the title). That would need the producer to map it and sys_inbox_message to carry it, which is a capability expansion nobody has asked for — worth confirming against the object definition before anyone assumes the column exists.

Notes

  • Severity deliberately not judged here; nothing in the shipped tree misbehaves because of this.
  • objectui#5190 deliberately did not widen into this field: its ruling scoped to source_object/source_id, and the veto clause on that card discouraged expanding the contract question inside the fix.

Activity

  1. os-support-ai
    added
    finding
    on Aug 18, 2026

  2. os-warren commented on Aug 19, 2026

    @os-warren
    os-warren
    on Aug 19, 2026 – with Claude
    Collaborator

    Concentrated finding round #4 — first-touch grading (triage seat, session session_01JEhkB6ShK2HHDzozy7Qwgz): promoted to pm:queue (retire the field). Verified on origin/main @ 12841b6: declaration at packages/app-shell/src/layout/inboxGrouping.ts:41 (actor_name?: string | null); mergeInboxRows (packages/app-shell/src/hooks/sharedUserFeeds.ts:675) maps the row fields and not actor_name; a full actor_name grep over packages/app-shell/src hits only the decoy shapes the body itself flags (the sys_activity→ActivityItem map at sharedUserFeeds.ts:587, approvals at useRecordApprovals.ts:141 / RecordApprovalsPanel.tsx:303, fixtures) and zero inbox renderers — with those sibling files as the positive control.

    No fresh escalation needed: the ruling is inherited — #5190 ruled removal for two sibling fields of the same interface (landed as PR #5204), and this card is strictly milder (no reader at all, vs a reader of never-produced data). "Make it live" would be a capability expansion with zero measured demand. Scope: delete the one line + any type-echo; a changeset note for the published type narrowing (an unfilled optional field has no plausible external reader — if the dev finds one, that is a fork report, not a silent widen). Serial: the file's recent churn (#5204) is merged; no open PR. Size suggestion: S, sonnet–opus.

    Generated by Claude Code

  3. os-warren
    added
    pm:queue
    and removed
    finding
    on Aug 19, 2026
  4. claude
    added
    pm:dispatched
    and removed
    pm:queue
    on Aug 21, 2026
  5. claude
    assigned
    os-support-ai
    on Aug 21, 2026

  6. claude commented on Aug 21, 2026

    @claude
    claudeBot
    on Aug 21, 2026 – with Claude
    Contributor

    Claim: PM loop round 21
    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-5203-inbox-actor-name-retire
    Worktree: objectui-issue-5203
    Domain: repo:objectui
    File surface: packages/app-shell/src/layout/inboxGrouping.ts + packages/app-shell/src/hooks/sharedUserFeeds.ts (stop on breach; explain in the report)
    Container & model: S, mode:subagent
    Clause-②: no — retires a declared field with no producer and no consumer; confirm both ends before removing
    Serial constraints cleared: ⚠️ PR #5480 (#5418) is IN FLIGHT in packages/app-shell but in preview/ and views/ only — ⛔ do not touch either of those directories

    ⛔ Governed-surface check run before dispatch: the declared surface hits none of AGENTS.md / CLAUDE.md / .claude/** / docs/adr/** / skills/**. If the work turns out to need one, stop and report — published skills/** ownership is an open question with the maintainer and no seat may land it.

    Generated by Claude Code

  7. os-support-ai commented on Aug 21, 2026

    @os-support-ai
    os-support-ai
    on Aug 21, 2026 – with Claude
    CollaboratorAuthor

    os-dev-report

    {
  "issue": 5203,
  "status": "done",
  "branch": "claude/issue-5203-inbox-actor-name-retire",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5487",
  "premise_still_valid": true,
  "summary": "Removed the dead `actor_name` member from `InboxNotification` (packages/app-shell/src/layout/inboxGrouping.ts) and documented the retirement on the interface, including the name-conflation warning so the next grep does not confuse it with the three live `actor_name` fields elsewhere in the package. The producer needed no edit: `mergeInboxRows` never mapped the field, so the diff is one deleted line plus two pins and a changeset (4 files, +218/-1). The card's premise held on all three legs I measured independently of it — no producer, no consumer, and no `sys_inbox_message` column to have mapped FROM, which settles the 'make it live' option the card left open as a backend capability expansion rather than a one-line producer change. Removing the field changes NO published type surface: `InboxNotification` is absent from `src/index.ts`, `src/layout/index.ts` and the built `dist/index.d.ts`, and app-shell's `exports` map has no deep subpath, so it is package-internal; the changeset therefore carries an empty frontmatter (the repo's explicit publishes-nothing declaration) and states that finding in its body. Not Clause-2 territory, and said so explicitly rather than by omission.",
  "tests": "BOTH-ENDS-DEAD, two independent instruments, every zero counter-probed. (1) Text: on pristine origin/main `actor_name` occurs 26x in tracked ts/tsx with exactly 6 reads, each on a DIFFERENT declaration (sys_activity inline map at sharedUserFeeds.ts:587; ApprovalActivity useRecordApprovals.ts:141 read at RecordApprovalsPanel.tsx:353; RecordDetailView.tsx:1573; plugin-detail RecordActivityRow recordActivityFeed.ts:94 read at :159 and record-history.tsx:100; approvalsApi.ts:156 read at ApprovalsInboxPage.tsx:1920) and zero on InboxNotification. (2) Type-aware probe, immune to that conflation: renaming the member in the interface and running BOTH projects. Baseline green. PROBE on actor_name predicted 0 errors, observed `SRC=0 TESTS=0`. COUNTER-PROBE: identical rename applied to `action_url`, a sibling known produced+read, predicted errors at producer/consumers/fixtures, observed 3 src + 12 test errors -- `sharedUserFeeds.ts(700,7) TS2353 Object literal may only specify known properties`, `useHomeInbox.ts(141,22) TS2339`, `InboxPopover.tsx(171,48) TS2339`, plus 9 fixture TS2353s across 3 suites; `SRC=2 TESTS=2`. Instrument fires on all three binding kinds, so the zero is a reading. (3) Third leg: objectstack@7679f8b54 packages/services/service-messaging/src/objects/inbox-message.object.ts declares id/user_id/notification_id/delivery_id/topic/title/body_md/severity/action_url/created_at -- no actor column. REVERSE VERIFICATION, direction predicted before running, both legs ablating a COMMITTED state. REBUILD: none required, and this is not a skipped step -- both suites reach their subject through RELATIVE specifiers (`../inboxGrouping`, `../sharedUserFeeds`), so no package dist is on the resolution path and no stale artifact can fake a green; the dependency closure was built once up front (`turbo run build --filter='@object-ui/app-shell^...'`, 28 tasks) because tsconfig.test.json sets paths:{} and resolves @object-ui/* through built d.ts. LEG 1 (re-declare actor_name): predicted test type-check RED + runtime GREEN; observed `inboxGrouping.test.ts(89,7): error TS2578: Unused '@ts-expect-error' directive. TYPECHECK=2` with `Test Files 2 passed (2) Tests 10 passed (10)`. LEG 2 (producer `...m` raw-row pass-through): predicted runtime RED + type pin GREEN + `satisfies` silent; observed `SRC_TYPECHECK=0 TEST_TYPECHECK=0` and `Test Files 1 failed | 1 passed (2) Tests 2 failed | 8 passed (10)` on `expect(Object.hasOwn(row,'actor_name')).toBe(false)` receiving true. Neither pin substitutes for the other. Both legs restored via `git checkout branch -- path`; `git status --porcelain` empty and `git diff HEAD` empty afterwards. FINAL VERIFICATION, all at 7c0a3fecb on a clean tree: `pnpm exec vitest run packages/app-shell/` from repo root (the invocation AGENTS.md mandates; `pnpm --filter test` is the documented false-green trap) = `Test Files 460 passed (460) Tests 4458 passed | 1 skipped (4459)`, that single skip pre-existing on origin/main and none introduced here; the 20 suites touching this surface = 20 passed / 270 tests; `tsc --noEmit` = 0 and `tsc -p tsconfig.test.json` = 0; `eslint .` in app-shell exit 0 (0 errors, 2502 pre-existing warnings, all in views/ which I did not touch), and `eslint --max-warnings=0` on my 3 changed files exit 0. GATES (derived from the actual diff -- objectui has no scripts/pm/dispatch-gates.mjs): check:control-bytes green (4533 files), check:phantom-deps green, check:self-import green, check:esm-specifiers green, check:changeset-presence green (recognised the empty frontmatter as the explicit exemption), changeset:check green; plus a manual control-byte scan of the 3 changed files, clean. check:eager-closure NOT run and reported as such: its gauge reads apps/console/dist/eager-closure.json, which exists only after a console vite build this worktree never ran -- it fails identically on pristine main here and is unrelated to this diff.",
  "open_questions": [],
  "out_of_scope_findings": []
}

    Generated by Claude Code

  8. os-support-ai
    closed this as completedin #5487on Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

pm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions