Repository navigation
Commit 029bdaf
fix(app-shell): the activity feed asks only a caller who may read sys_activity, and a failed read is not an empty feed (objectui#12081 item 8) (#12095)
Part of #12081 (item 8)
Clause-②: no
Item 8 only, as triage directed. Items 2, 3, 4 and 7 stay on the card,
and #12081 remains open.
## Measured first
**objectui `main` at `023f00d4`.** `useSharedActivityFeed` sent
`find('sys_activity', { $orderby: { timestamp: 'desc' }, $top: 20 })`
for every signed-in user. A failure other than 404 went to
`markFailed()`, so the store's status became `error`. The hook still
returned `.value` alone. Three surfaces read that value: the header
bell's Activity tab, the `global:notifications` page block (the same
popover), and Home's activity card (through `useHumanActivityFeed`).
Each received `[]` and showed "No recent activity", followed by a "View
all activity" link to a list page that refuses the same user. The card
says the widget "shows nothing". It actually shows the empty-state
message.
Reproduced on `023f00d4` with only the test file changed. The new block
in `AppHeader.inboxVariant.test.tsx` was 2 red and 2 green:
- "a member without the grant issues no sys_activity request…":
`expected [ { object: 'sys_activity', … } ] to have a length of +0 but
got 1`.
- "a 403 from the server (a stale grant) reads as failed…": the failure
notice was never rendered, and the DOM held "No recent activity".
**objectstack `origin/main` at `86da1949`, read only.**
- **The 403 is object-level.** `member_default` in plugin-security's
`default-permission-sets.ts` grants read on the two inbox objects. Its
own comment says "`sys_activity` is deliberately NOT included: it is not
a per-user-scoped shape". `git grep sys_activity 86da1949 -- examples/`
returns nothing, so no shipped example app grants it either. The admin
sets reach it through their `*` entries.
- **The server already narrows an allowed read.** plugin-audit's
`activity-read-visibility.ts` adds a parent-record filter to every
non-system `find`, `findOne`, `count` and `aggregate` on `sys_activity`.
A reader who holds the object grant sees only rows about records they
can open.
- **So a narrower query cannot help a user without the grant.** The
record Discussion tab's activity half is also a `find('sys_activity',
…)`, filtered by `object_name` and `record_id`. It hits the same object
check, and for that user it shows its refusal state (objectui#11195),
not rows.
PM mechanism assumption 2 holds. That is why this PR takes triage's
second branch: decide from the user's grant before asking.
## Changes
**1. The grant is checked before the read (`sharedUserFeeds.ts`).** The
hook reads `usePermissions().can('sys_activity', 'read')`. This is the
permission check the shell already uses for reads: `ObjectDataPage`'s
route gate and `UnifiedSidebar`'s nav gate read it the same way. I
looked at the affordance map from objectui#12084 and did not use it. Its
rows are write affordances only, and its census explicitly leaves
`can(x, 'read')` read gates outside its family. The check has three
outcomes, beside the unchanged presence rule:
- permissions not loaded (no provider mounted, or a provider whose
refetch failed): unknown, so the feed reads, as before;
- loaded and granted: the feed reads;
- loaded and refused: no cache key and no request, and the hook reports
`readable: false`.
**2. The status reaches the surfaces.** The hook now returns
`ActivityFeedReading`, which is `value`, `status` and `readable`
together. `readable` is a separate field rather than a fifth status
value, so the four-value status set (#4300) is unchanged.
**3. The surfaces.**
- `InboxPopover` takes a required `activity` prop in place of
`activities`. With `readable: false` it shows no Activity tab trigger,
no tab body and no "View all activity" link, and it ignores a remembered
Activity tab choice. With `error` it shows "An unexpected error
occurred.". With `idle` or `loading` it shows "Loading…". It shows "No
recent activity" only when the status is `ready`.
- `AppHeader` and `global-notifications-renderer` pass the new value
through. `AppHeader`'s exported props are unchanged: rows a host passes
as `activities` are treated as `ready`.
- Home is an extension beyond the claimed files, named here as the
dispatch asked. `useHomeInbox` returns `activity` instead of
`activities`. `HomeActivity` renders nothing when `readable` is false
and shows the same three states otherwise. `HomePage` removes the 360px
grid column when the card is absent.
**4. The cache key includes the signed-in user.** This is a small,
bounded fix in the same file; the reasons are below.
The messages reuse existing locale keys, `errors.unknown` and
`common.loading`, the same pair `HomeActionCenter` shows for an inbox
read that has not answered. No locale key is added. Nothing is added to
the package entry: `sharedUserFeeds`, `InboxPopover`, `HomeRail` and
`useHomeInbox` are not re-exported from `src/index.ts`. `AppHeader` is
re-exported, and its props are unchanged. So Clause-② stays `no`.
### The fix in the same file: the activity cache key
The store keyed the activity feed on the adapter alone. Two facts make
that wrong:
- the server narrows the rows per user;
- signing out does not reload the page (`AuthProvider` notes that "no
sign-out call site reloads the page"), so this module-level store keeps
running.
As a result, the next user to sign in on the same browser tab was shown
the previous user's rows as `ready`, within the 30s freshness window and
until a refetch landed. The key is now the adapter plus the signed-in
user id, as the inbox feed's key already is. The user id is part of the
key only: the query names no user, so a host without auth still reads.
The test "re-keys on the signed-in user" pins it.
Why this fits in this PR rather than a separate card:
- it is the same defect class as item 8 (the feed must show only what
this user can see);
- the fix is mechanical, and its shape already exists in the same file
(the inbox feed's adapter-plus-user key);
- the claim names no other holder of this file;
- it runs under the same gates and adds no new verification surface.
## Evidence
**Head.** All runs below are on `f1f0ba557`: `023f00d4` plus this
change, merged with `origin/main` at `de302c73`. That merge brought in
objectui#12088 and #12090, and neither touches any file in this diff.
**Type-check.** `pnpm --filter @object-ui/app-shell type-check` (`tsc
--noEmit && tsc -p tsconfig.test.json`) exits 0.
**Tests, narrowed.** I ran every test file that names any of the seven
changed modules or the components built on them (`sharedUserFeeds`,
`useHomeInbox`, `AppHeader`, `InboxPopover`,
`global-notifications-renderer` / `GlobalNotificationsRenderer`,
`HomeRail`, `HomePage`, `HomeLayout`, `ConsoleLayout`,
`global:notifications`), selected by `git grep -l`, plus
`affordanceGrantMap-12082.test.tsx`. That is 121 files, in two runs.
- First run: 60 of 61 files passed, 709 of 710 tests.
- The one failure was `ApiConsolePage.requestPreset-10591`. It hit a 10s
timeout while the page still showed "Initializing application…", under a
full shared box. That happens before the shell renders anything this PR
touches. Run alone on the same head it passes 12 of 12.
- Second run: 60 of 60 files, 735 of 735 tests.
The rest of app-shell's 1,250 test files are left to CI: no vi.mock of
the changed modules exists outside the files above.
**Reverse checks.** Each check was run from the committed `d74a507` with
`ablation-replace.mjs`, which checks that the anchor matched and that
the restore leaves the file identical to HEAD (`git diff HEAD` empty
after each leg). The tests run were the new `objectui#12081` cases in
the hook suite and the `AppHeader` suite.
- Grant check forced open (`readable = true || …`): 3 red, namely the
two refusal cases in the hook suite and the `AppHeader` member case.
- User dropped from the cache key: 1 red, "re-keys on the signed-in
user".
- Popover empty message no longer gated on `ready`: 1 red, the 403 case.
**Gates.**
| Gate | Result |
|---|---|
| `check:new-line-citations` | 0 new citations |
| `check:control-bytes` | OK |
| `check:i18n-keys` | exit 0 |
| `check-changeset-presence` | 1 changeset for 27 source files |
| `check-changeset-no-major` | exit 0 |
| `check-vi-mock-inherit` / `-override-shape` / `-specifiers` | OK |
| `check-test-path-roots` | OK |
| `check:esm-specifiers` / `check:self-import` / `check:phantom-deps` |
exit 0 |
**Lint.** I ran eslint on the 27 touched `.ts`/`.tsx` files only:
- result (`--format json`): 27 files, 0 errors, 96 warnings, and none of
the warnings is on a line this PR adds;
- the full lint run is left to CI;
- searching `eslint.config.js` for `projectService`,
`parserOptions.project` or `TypeChecked` finds nothing, so type-aware
linting is not enabled, and this diff cannot change the lint result of a
file it does not touch.
**Eager closure.** `check:eager-closure` passes at `d74a507`: 3169.0 KB
gzipped, headroom 35.6 KB. Base `023f00d4`, built in a separate
worktree, measures 3168.6 KB. The difference is +349 bytes gzipped
(+1,202 raw), and the eager chunk count is 290 at both. The head figure
was taken before the merge.
## Acceptance notes
- The `useHomeInbox` mocks in seven `HomePage.*` tests never carried
`notificationsStatus`. This PR only replaces their `activities` key. The
missing key is fixture drift; it was not changed here and no card was
filed.
- I did not check whether `MePermissionsProvider` refetches when a
different user signs in on the same tab. If it does not, the next user's
grant check reads the previous user's grants. The result is then either
a refused read, which now shows as an error, or a missing Activity tab
until reload. This is unmeasured, so no card was filed. Who picks it up:
no one.
- No guide describes the bell's Activity tab, so no documentation became
false. `content/docs/guide/notifications.md` covers only the
notifications half.
Session: `https://claude.ai/code/session_01B1gHb9baeX7oioD5sHVm7z`
---
_Generated by [Claude
Code](https://claude.ai/code/session_01B1gHb9baeX7oioD5sHVm7z)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: objectstack-fleet[bot] <332303061+objectstack-fleet[bot]@users.noreply.github.com>1 parent 5af42db commit 029bdaf
28 files changed
Lines changed: 718 additions & 123 deletions
File tree
- .changeset
- packages/app-shell/src
- console/home
- __tests__
- hooks
- __tests__
- layout
- __tests__
- views
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
244 | 244 | | |
245 | 245 | | |
246 | 246 | | |
247 | | - | |
| 247 | + | |
248 | 248 | | |
249 | 249 | | |
250 | 250 | | |
| |||
599 | 599 | | |
600 | 600 | | |
601 | 601 | | |
602 | | - | |
603 | | - | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
604 | 612 | | |
605 | | - | |
| 613 | + | |
606 | 614 | | |
607 | 615 | | |
608 | 616 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
279 | 279 | | |
280 | 280 | | |
281 | 281 | | |
282 | | - | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
283 | 294 | | |
284 | 295 | | |
| 296 | + | |
| 297 | + | |
285 | 298 | | |
286 | 299 | | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
287 | 316 | | |
288 | | - | |
289 | | - | |
290 | | - | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
291 | 322 | | |
292 | 323 | | |
293 | 324 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
102 | | - | |
| 102 | + | |
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
106 | | - | |
| 106 | + | |
107 | 107 | | |
108 | 108 | | |
109 | 109 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
95 | | - | |
| 95 | + | |
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
103 | | - | |
| 103 | + | |
104 | 104 | | |
105 | 105 | | |
106 | 106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
70 | | - | |
| 70 | + | |
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| |||
0 commit comments