Repository navigation
feat(content): ✨ live refresh for AdminCP lists and comment threads - #908
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
React Doctor found 2 new issues in 2 files · 2 warnings · score 84 / 100 (Needs work) · 0 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22e7487292
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const session = await new SessionAdminModel(c).getSession({ extend: false }); | ||
| if (!session) return { code: "FORBIDDEN", ok: false }; |
There was a problem hiding this comment.
When a visitor signs into AdminCP after the shared WebSocket has opened, this authorization reads the upgrade request's original cookies, which do not contain the newly issued admin session. Both admin sign-in actions in tanstack/admin/actions.ts navigate client-side without reconnecting, and WebSocketAuthSync only reconnects when the member user ID changes. Consequently, the new list subscription receives FORBIDDEN and live refresh remains disabled until a reload or unrelated reconnect. Refresh the shared connection when the AdminCP session changes so the join uses the current admin cookie.
Useful? React with 👍 / 👎.
| if (definition.thread.enabled && touchesPublic) { | ||
| const location = await threadLocation(c, definition, contentId, row); | ||
| if (location) { |
There was a problem hiding this comment.
When an admin moves a published reply to another parent or root, this resolves only its post-update location and signals the destination branch. Such updates are allowed by assertContentThreadPlacement (including moving a leaf between roots), but viewers of the original branch receive no signal to remove the reply. Within one thread, refreshing only the destination can leave the same comment rendered under both parents; across roots, the original thread retains the moved comment until a full refetch. Preserve the previous placement and refresh both affected branches or threads.
Useful? React with 👍 / 👎.
22e7487 to
c523070
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c523070f8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const key = shareKey(membership.c, membership.scope); | ||
| let pending = key === null ? undefined : shared.get(key); | ||
| if (!pending) { | ||
| pending = authorize(membership.c, membership.scope).catch( |
There was a problem hiding this comment.
When an administrator's content permissions are revoked while their socket remains open, reusing membership.c makes subsequent authorization checks retain the original grants. Both isStaff(..., { live: true }) and checkStaffPermissionOfUser consult the context-keyed memo in api/lib/staff-permission-cache.ts; permission mutations invalidate their own request context and Redis entries, not this retained WebSocket context. Consequently, sweeps continue approving the subscription after HTTP access has been revoked. Use a fresh authorization context or explicitly clear its request-scoped permission memo before each recheck.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| // Not on screen: "Load more" reads it fresh when it is reached, and at | ||
| // the end of the thread the reader decides when to show it. | ||
| if (current && current.pages.at(-1)?.pageInfo.hasNextPage === false) { | ||
| newBranches.add(key); | ||
| setSnapshot({ hasNew: true }); |
There was a problem hiding this comment.
If an older top-level comment is approved or unhidden after the reader has loaded past its creation time, while more pages remain, its branch is absent from loadedBranches and this condition silently discards its signal. readContentThread paginates by (createdAt, id), so subsequent Load more requests cannot retrieve that newly visible comment before the current cursor. It remains missing until a full refetch or reconnect. Retain these unknown-branch signals and reconcile branches that may have become visible inside the already-loaded range instead of assuming Load more will reach them.
Useful? React with 👍 / 👎.
| items: page.items.flatMap(item => | ||
| gone.has(keyOf(item.id)) | ||
| ? page.items.some( |
There was a problem hiding this comment.
When a removal signal arrives for a comment with an open edit or reply form and the branch request fails, applyRemovals ignores the tracked drafts: a leaf is removed outright, while a parent has its record cleared. Both cases unmount the composer in the shared comment renderer and discard the user's unsaved text. The successful merge path already preserves these nodes through keepDrafts; apply equivalent draft preservation here, marking the retained comment as removed so submission is unavailable without destroying the form.
Useful? React with 👍 / 👎.
Content Engine changes now reach open screens over the existing shared
WebSocket as small, typed hints; HTTP stays the source of truth for content,
visibility and permissions.
- Server: authorized `content/refresh` socket with join acknowledgement,
per-tab memberships, limits, re-authorization once a minute (including
sessions that ended while a socket stayed open) and immediate re-checks when
a thread's root changes. Signals are published from `emitContentEvent` after
the commit and cache invalidation.
- New `GET /{publicApi.path}/thread/branches` route on the shared thread
reader, for targeted, permission-aware branch refreshes.
- Generated AdminCP lists refresh the page on screen (filters, sort, page and
selection kept), with coalescing and a non-blocking failure notice.
- `useContentComments` gains `branches` and `live`: in-place branch merges,
"Show new comments" for new top-level comments, drafts kept when a comment
is removed, structural sharing and scroll anchoring. Blog and Newsroom use
it; the blog thread no longer remounts an open reply form on a first reply.
- `useContentLiveRefresh` for custom screens.
- Tests for the server socket, authorization, publishing order and routing,
multi-instance delivery, the branches route, the live thread controller
(with a request-cost benchmark) and the subscription hook.
- Docs: Live comments, Live refresh, Comments API and AdminCP updates.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CnAiFbH6Ps6sXRHUdJUAZk
1d54efe to
a23e102
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| return; | ||
| } | ||
|
|
||
| const result = await authorize(c, message.scope); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-defer-await (warning)
This await blocks the function before an early-return that doesn't use the awaited value, so the skip path waits for nothing. Move the await below the guard so it only runs when you need the data
Fix → Move the await below the early-return guard so the skip path stays fast and avoids unnecessary async work.
| start += CONTENT_THREAD_BRANCHES_MAX | ||
| ) { | ||
| if (disposed) return; | ||
| await request(keys.slice(start, start + CONTENT_THREAD_BRANCHES_MAX)); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This for-loop waits before starting the next iteration. If iterations perform independent asynchronous work, consider bounded concurrency; await syntax alone does not establish a speedup.
Fix → Consider concurrent calls only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics; observe all callback promises.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a23e102b4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): null | string => { | ||
| if (scope.kind === "admin") return null; | ||
|
|
||
| return `${contentRefreshRoom(scope)}|${c.get("user")?.id ?? "guest"}`; |
There was a problem hiding this comment.
When two devices belonging to the same user follow the same thread, this key groups their authorization results even though authorizeContentRefresh checks each connection's session through SessionModel.getUser(). If one device's session is revoked or expires, recheck uses whichever connection is encountered first for both: a valid session can keep the revoked socket subscribed to a hidden root's signals, or an invalid session can revoke the other device's valid subscription. Validate sessions separately before sharing the root-access lookup, or include session identity in the grouping key.
Useful? React with 👍 / 👎.
| await queryClient.refetchQueries({ | ||
| exact: true, | ||
| queryKey: threadKey, | ||
| type: "active", | ||
| }); |
There was a problem hiding this comment.
If a reader keeps an edit or reply form open through a disconnect and that comment is deleted or hidden meanwhile, the successful reconnect reconciliation replaces the infinite-query pages directly without applying keepDrafts. The comment consequently disappears or gets record: null, causing ContentCommentItem to unmount the composer and discard its unsaved text. The branch merge preserves tracked drafts, but this full-refetch path needs equivalent preservation when the thread itself remains accessible.
Useful? React with 👍 / 👎.
Content Engine changes now reach open screens over the existing shared
WebSocket as small, typed hints; HTTP stays the source of truth.
content/refreshsocket with join acknowledgement,per-tab memberships, limits, periodic re-authorization and immediate
re-checks when a thread's root changes. Signals are published from
emitContentEventafter the commit and cache invalidation.GET /{publicApi.path}/thread/branchesroute reusing the threadreader, for targeted, permission-aware branch refreshes.
selection kept), with coalescing and a non-blocking failure notice.
useContentCommentsgainsbranchesandlive: branch merges in place,"Show new comments" for new top-level comments, drafts kept when a comment
is removed, structural sharing and scroll anchoring. Blog and Newsroom use it.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01CnAiFbH6Ps6sXRHUdJUAZk