refactor(ui): establish reusable React foundations - #5188
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThis PR adds reusable UI primitives and hooks, migrates Agent World and flow interfaces to them, centralizes async request and polling coordination, standardizes status and approval presentation, and adds extensive accessibility, concurrency, error, and lifecycle tests. ChangesReact reuse foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
# Conflicts: # app/src/services/api/flowsApi.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94d6adbddd
ℹ️ 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (15)
app/src/agentworld/pages/BountiesSection.tsx (1)
590-590: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute form-level errors through
FormField/role="alert"for consistency.
CommentModal(Line 732) wireserrorintoFormField, which renders it withrole="alert"and setsaria-invalid.CreateBountyModalandSubmitWorkModalkeep a bare<p>, so their validation failures ("Title is required", "URL is required") aren't announced to screen readers. Since these are field-level validations, attaching them to the relevantFormField(plusrequired) would make the three modals behave alike.Also applies to: 678-678
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/agentworld/pages/BountiesSection.tsx` at line 590, Update the validation error handling in CreateBountyModal and SubmitWorkModal to pass each field’s error into its relevant FormField instead of rendering a standalone p element. Mark the corresponding fields as required, preserving the existing validation messages and ensuring FormField provides the alert and aria-invalid behavior consistently with CommentModal.app/src/components/settings/panels/__tests__/TeamMembersPanel.test.tsx (1)
218-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAwait the settled request before ending the test.
request.resolve({})is the last statement, so the promise continuation (refreshTeamMembers,setRemovingId/setChangingRoleId, dialog close) runs after the test body returns — outsideact()and potentially after teardown, which surfaces as flaky act warnings. Same pattern at Line 350.♻️ Suggested tightening
- expect(mockChangeRole).toHaveBeenCalledTimes(1); - expect(screen.getByRole('dialog', { name: 'Change Role' })).toBeInTheDocument(); - request.resolve({}); + expect(mockChangeRole).toHaveBeenCalledTimes(1); + expect(screen.getByRole('dialog', { name: 'Change Role' })).toBeInTheDocument(); + + request.resolve({}); + await waitFor(() => { + expect(screen.queryByRole('dialog', { name: 'Change Role' })).not.toBeInTheDocument(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/__tests__/TeamMembersPanel.test.tsx` around lines 218 - 221, Update the relevant TeamMembersPanel tests, including the case around mockChangeRole and the similar case near the second referenced location, to await completion of the resolved request before the test ends. Keep request.resolve({}) inside the test’s async act/wait flow and wait for the resulting refresh, state updates, and dialog transition so no promise continuation runs after teardown.app/src/pages/WorkflowRunsPage.test.tsx (1)
147-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAsserting
bg-amber-50re-testsFlowRunStatus's internal accent map.The badge's colour mapping lives in
app/src/components/flows/FlowRunStatus.tsxand is covered by its own tests; asserting the utility class here breaks this page test on any token rename without catching a page-level regression. ThetoHaveTextContent('pending approval')assertion above already pins the behaviour this test cares about.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/WorkflowRunsPage.test.tsx` at line 147, Remove the bg-amber-50 class assertion from the WorkflowRunsPage test and retain the existing toHaveTextContent('pending approval') assertion, leaving FlowRunStatus color mapping coverage to its dedicated tests.app/src/components/channels/mcp/McpCatalogBrowser.test.tsx (2)
35-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAsserting
h-9couples this test toInput's internal size classes.
type="search"is a real contract of this component; theh-9utility class is an implementation detail of the sharedInputprimitive and will break this test on any size-token change. Consider dropping the class assertion (it's already covered byInput's own tests) or asserting via a stable prop/testid instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/channels/mcp/McpCatalogBrowser.test.tsx` around lines 35 - 38, Update the search input test around the `getByPlaceholderText('Search MCP servers...')` assertion to remove the `h-9` class check, retaining only the stable `type="search"` contract; do not replace it with another assertion tied to `Input` styling internals.
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
deferred<T>()test helper. The same promise-control helper is defined verbatim in both MCP test files (and again inapp/src/components/flows/FlowRunsDrawer.test.tsx); hoisting it into a shared test util keeps one definition as more suites adopt the pattern.
app/src/components/channels/mcp/McpCatalogBrowser.test.tsx#L10-L16: replace the local helper with an import from the shared test util.app/src/components/channels/mcp/McpServersTab.test.tsx#L26-L32: replace the local helper with the same shared import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/channels/mcp/McpCatalogBrowser.test.tsx` around lines 10 - 16, The deferred<T> helper is duplicated across MCP test suites. Add or reuse a shared test utility exporting deferred, then remove the local definitions and import it in app/src/components/channels/mcp/McpCatalogBrowser.test.tsx lines 10-16 and app/src/components/channels/mcp/McpServersTab.test.tsx lines 26-32; update each test’s references to use the shared import.app/src/services/api/__tests__/flowsApi.test.ts (1)
61-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOverlapping
buildWorkflowcoercion coverage across twoflowsApitest files.
app/src/services/api/flowsApi.test.ts(lines 476-526) already parameterizesbuildWorkflowproposal coercion, including the valid-payload case. Keeping a second parity test in a sibling__tests__/file for the same module makes it easy for the two suites to drift. Consider consolidating thebuildWorkflowproposal assertions into one file.Also, the file's header docblock (lines 1-6) still describes only
importFlow— worth extending now that this suite also coversbuildWorkflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/api/__tests__/flowsApi.test.ts` around lines 61 - 88, Consolidate the duplicate buildWorkflow proposal-parity coverage by removing the valid-payload test and related setup from the sibling __tests__ suite, preserving the existing parameterized coercion coverage in flowsApi.test.ts. Update the suite header docblock to describe both importFlow and buildWorkflow coverage.app/src/pages/WorkflowRunsPage.tsx (1)
37-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the redundant synchronous resets at the top of this effect.
flowNamesLoadingalready startstrueandflowNamesErrorstartsnull, so these mount-time updates don’t change behavior and still triggerreact-hooks/set-state-in-effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/WorkflowRunsPage.tsx` around lines 37 - 41, Remove the initial setFlowNamesLoading(true) and setFlowNamesError(null) calls from the useEffect that invokes listFlows, leaving the asynchronous flow-loading logic and its existing state updates unchanged.Source: Learnings
app/src/agentworld/pages/ExploreSection/index.tsx (1)
1-769: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFile exceeds the ~500-line guideline for
.tsxfiles.This file bundles 4 independent live-section hooks, their skeleton/card/list presentational components, and the root component in one ~768-769 line module. Consider splitting per-section (communities/jobs/bounties/agents) into sibling files under
ExploreSection/, similar to how the folder already separates the test file.As per coding guidelines,
**/*.{rs,ts,tsx}: "Prefer files of approximately 500 lines or fewer."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/agentworld/pages/ExploreSection/index.tsx` around lines 1 - 769, Split the oversized ExploreSection module into sibling files under ExploreSection/, grouping each independent section’s hook, skeleton, card/row, and list/grid components for communities, jobs, bounties, and agents. Keep shared helpers and primitives in a shared module, then update ExploreSection and imports so existing rendering, navigation, loading, empty, and error behavior remains unchanged.Source: Coding guidelines
app/src/hooks/flowPendingApprovalsStore.ts (2)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
nextRequestId/activeRequestIdadd bookkeeping that duplicatesinFlight.
activeRequestIdis only read in thefinallyto decide whether to nullinFlight; sincestartRefreshalways overwritesinFlightwith the newest request, comparing promise identity (if (inFlight === request) inFlight = null;) expresses the same invariant without the two extra module-level counters.Also applies to: 109-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/hooks/flowPendingApprovalsStore.ts` around lines 45 - 48, Remove the redundant nextRequestId and activeRequestId bookkeeping from the pending-approvals refresh flow. Update startRefresh to retain the created request promise and, in its finally cleanup, clear inFlight only when it still matches that promise; preserve the behavior that newer refreshes are not cleared by older requests.
101-107: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winError path re-clones the retained approvals, churning identity every failed poll.
makeSnapshotrunsfreezeApprovalsagain oversnapshot.approvals, which is already deep-cloned and frozen. Each failed tick therefore produces a brand-new array identity, invalidating theuseMemo([source.approvals])selectors inuseFlowPendingApprovalsanduseRunsPendingApprovalSet(newSet→ new props) even though the data is unchanged.♻️ Reuse the already-frozen array
+const makeSnapshotFromFrozen = ( + approvals: PendingApproval[], + error: string | null, + polling: boolean +): FlowPendingApprovalsSnapshot => Object.freeze({ approvals, error, polling });- emit(makeSnapshot(snapshot.approvals, normalizeError(error), retainCount > 0)); + emit(makeSnapshotFromFrozen(snapshot.approvals, normalizeError(error), retainCount > 0));The same applies to the
retainFlowPendingApprovalsPollingemits on Lines 142 and 154.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/hooks/flowPendingApprovalsStore.ts` around lines 101 - 107, Update the error path in the polling flow and the emits in retainFlowPendingApprovalsPolling to reuse the existing frozen snapshot.approvals array instead of passing it through makeSnapshot and freezeApprovals again. Preserve the updated error and retention state while keeping approvals’ reference identity unchanged across failed polls.app/src/hooks/useClipboardFeedback.ts (1)
25-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the option syncing out of render.
Assigning to
writeTextRef.currentandresetAfterMsRef.currentduring render mutates refs from the render body, which React’srefslint rule flags and concurrent rendering can discard mid-render.♻️ Effect-based option sync
- writeTextRef.current = options.writeText; - resetAfterMsRef.current = options.resetAfterMs ?? DEFAULT_RESET_AFTER_MS; + useEffect(() => { + writeTextRef.current = options.writeText; + resetAfterMsRef.current = options.resetAfterMs ?? DEFAULT_RESET_AFTER_MS; + }, [options.writeText, options.resetAfterMs]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/hooks/useClipboardFeedback.ts` around lines 25 - 29, Move the assignments updating writeTextRef.current and resetAfterMsRef.current out of the render body in useClipboardFeedback and into an effect that runs when options.writeText or the resolved options.resetAfterMs changes. Keep the existing default reset duration behavior and ensure callbacks use the synchronized refs.app/src/components/flows/FlowRunsDrawer.test.tsx (1)
65-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
deferred<T>()test helper across files. Both files independently define the same manually-resolvable-promise pattern for deterministic async testing; extracting a shared test utility would remove the duplication (and therejectsupport difference between the two copies).
app/src/components/flows/FlowRunsDrawer.test.tsx#L65-L71: replace the localdeferred<T>()with an import from a shared test-utils module (e.g.app/src/testUtils/deferred.ts).app/src/components/intelligence/SyncConfirmDialog.test.tsx#L33-L41: same — import the shared helper (already includesreject, so no functional change needed here) instead of a local copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/flows/FlowRunsDrawer.test.tsx` around lines 65 - 71, The deferred test helper is duplicated across two test files. Create or reuse a shared deferred utility with resolve and reject support, then replace the local deferred<T>() definitions with imports in app/src/components/flows/FlowRunsDrawer.test.tsx#L65-L71 and app/src/components/intelligence/SyncConfirmDialog.test.tsx#L33-L41; no other behavior changes are needed.app/src/components/chat/FlowApprovalRequestCard.tsx (1)
39-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated approval-action descriptor logic.
This
actionDecisions/actionsconstruction is structurally identical to the one inFlowRunPendingApprovalCard.tsx(differs only in id prefixes and i18n keys). Consider extracting a shared factory (e.g.,buildApprovalActions(idPrefix, t)) to avoid the two approval surfaces drifting out of sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/chat/FlowApprovalRequestCard.tsx` around lines 39 - 65, The approval action descriptors are duplicated between FlowApprovalRequestCard and FlowRunPendingApprovalCard. Extract a shared buildApprovalActions factory accepting the action ID prefix and translation function, use it in both components, and preserve each component’s existing prefixes and i18n keys.app/src/components/intelligence/SyncConfirmDialog.tsx (2)
30-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
useLatestAsyncinstead of a local cancel flag. This effect already follows the shared stale-response guard pattern; switching to the hook keeps async handling consistent and removes duplicated lifecycle logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/SyncConfirmDialog.tsx` around lines 30 - 48, Replace the local cancelled flag and cleanup logic in the SyncConfirmDialog useEffect with the shared useLatestAsync hook. Route the openhuman.memory_sources_estimate_sync_cost request through that hook while preserving the existing estimate and error state updates and sourceId dependency behavior.
36-40: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse
useLatestAsynchere. Replace the localcancelledflag inapp/src/components/intelligence/SyncConfirmDialog.tsx:30-48with the shared guard so stale-response handling stays consistent across async effects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/SyncConfirmDialog.tsx` around lines 36 - 40, Replace the local cancelled flag in the SyncConfirmDialog async effect with the shared useLatestAsync guard. Apply the guard around the openhuman.memory_sources_estimate_sync_cost call and only update estimate through the guard, removing the local cancellation state while preserving the existing RPC parameters and result handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/agentworld/pages/JobsSection.tsx`:
- Line 296: Update the error message rendered by the Post-a-Job form near the
`error` conditional in `JobsSection` to include `role="alert"`, matching the
accessible error announcement behavior used by `ApplyModal` and `DisputeModal`.
Preserve the existing styling and message content for both submission and
deadline validation errors.
In `@app/src/components/channels/mcp/McpServersTab.tsx`:
- Around line 430-435: Update the catalog-error retry handler in McpServersTab
to call fetchCatalog with debouncedCatalogFilters.query and
debouncedCatalogFilters.transport instead of the raw searchQuery and
transportFilter values. Keep the existing retry page and reset behavior
unchanged so retries use the same filter source as handleLoadMore and the fetch
effect.
In `@app/src/components/flows/FlowRunsSidebar.tsx`:
- Around line 89-99: Memoize the callbacks passed to useFlowRunStarted and
useFlowRunFinished in FlowRunsSidebar so their references remain stable across
renders, while preserving the existing logging and refreshSilently behavior. Use
the appropriate dependency list, including flowId and refreshSilently, and pass
the memoized callbacks to both hooks.
In `@app/src/components/intelligence/SyncConfirmDialog.test.tsx`:
- Around line 79-97: Update the dismissal simulation in the test “cancels from
Escape and backdrop and restores focus after dismissal” to dispatch
fireEvent.pointerDown on the dialog’s backdrop/parent element instead of
fireEvent.click, matching the useDismissLayer and ModalShell
onPointerDownCapture behavior. Keep the existing Escape assertion flow and
expect onCancel to be called twice.
In `@app/src/components/layout/ChipTabs.test.tsx`:
- Around line 31-37: Update the ChipTabs component’s keyboard interaction
handling, not just its tabIndex assignments: add ArrowLeft/ArrowRight plus
Home/End key handling to move focus among tabs and select the focused tab, while
preserving roving tabIndex behavior and wrapping as appropriate. Extend the
existing ChipTabs tests to simulate these key interactions and verify focus and
onChange selection outcomes.
In `@app/src/components/layout/ChipTabs.tsx`:
- Around line 114-117: Update the click handler in ChipTabs to route the
diagnostic helper’s development-mode configuration through the existing
app/src/utils/config.ts value instead of allowing the helper to read
import.meta.env.DEV directly. Preserve the current select logging and onChange
behavior while replacing the direct environment access with the config symbol.
- Around line 109-120: Restore keyboard access for inactive tabs in the chip tab
rendering around the active tabIndex and onClick handlers. Either add complete
roving-tabindex keyboard navigation, including arrow, Home, and End key handling
that moves focus between tabs, or keep every non-navigation chip tabbable;
preserve the existing selected-state and change behavior.
In `@app/src/components/ui/ConfirmDialog.test.tsx`:
- Around line 67-69: Update the backdrop interaction in the ConfirmDialog test
to dispatch the pointer-down event that ModalShell handles through capture,
rather than a click. Keep the existing assertion that props.onCancel is not
called, ensuring the busy-state behavior is exercised through the actual
dismissal event.
In `@docs/plans/2026-07-24-react-reuse-foundation.md`:
- Around line 26-30: Remove the developer-specific absolute path from the
command instructions in the plan. Since commands are already stated to run from
the repository root, delete the cd command block or replace it with a
repository-relative placeholder that works across checkouts.
- Around line 163-172: Update the verification sections in
docs/plans/2026-07-24-react-reuse-foundation.md lines 163-172 and
docs/specs/2026-07-24-react-reuse-foundation-design.md lines 163-172 to include
E2E coverage for the migrated flows and explicit Rust/JSON-RPC verification, or
state why those layers are unchanged and exempt. Preserve the existing
unit/Vitest and frontend checks while documenting compliance with the specify →
Rust → JSON-RPC → UI → unit/E2E workflow.
In `@docs/specs/2026-07-24-react-reuse-foundation-design.md`:
- Around line 99-109: Align the hook ownership described in “Consolidate shared
hooks and query coordination” with the existing design: remove “pending-run
derivation” from useFlowRunsQuery unless its documented return contract is
expanded to include that responsibility. Ensure pending approvals remain owned
by useRunsPendingApprovalSet and the shared pending-approvals source.
---
Nitpick comments:
In `@app/src/agentworld/pages/BountiesSection.tsx`:
- Line 590: Update the validation error handling in CreateBountyModal and
SubmitWorkModal to pass each field’s error into its relevant FormField instead
of rendering a standalone p element. Mark the corresponding fields as required,
preserving the existing validation messages and ensuring FormField provides the
alert and aria-invalid behavior consistently with CommentModal.
In `@app/src/agentworld/pages/ExploreSection/index.tsx`:
- Around line 1-769: Split the oversized ExploreSection module into sibling
files under ExploreSection/, grouping each independent section’s hook, skeleton,
card/row, and list/grid components for communities, jobs, bounties, and agents.
Keep shared helpers and primitives in a shared module, then update
ExploreSection and imports so existing rendering, navigation, loading, empty,
and error behavior remains unchanged.
In `@app/src/components/channels/mcp/McpCatalogBrowser.test.tsx`:
- Around line 35-38: Update the search input test around the
`getByPlaceholderText('Search MCP servers...')` assertion to remove the `h-9`
class check, retaining only the stable `type="search"` contract; do not replace
it with another assertion tied to `Input` styling internals.
- Around line 10-16: The deferred<T> helper is duplicated across MCP test
suites. Add or reuse a shared test utility exporting deferred, then remove the
local definitions and import it in
app/src/components/channels/mcp/McpCatalogBrowser.test.tsx lines 10-16 and
app/src/components/channels/mcp/McpServersTab.test.tsx lines 26-32; update each
test’s references to use the shared import.
In `@app/src/components/chat/FlowApprovalRequestCard.tsx`:
- Around line 39-65: The approval action descriptors are duplicated between
FlowApprovalRequestCard and FlowRunPendingApprovalCard. Extract a shared
buildApprovalActions factory accepting the action ID prefix and translation
function, use it in both components, and preserve each component’s existing
prefixes and i18n keys.
In `@app/src/components/flows/FlowRunsDrawer.test.tsx`:
- Around line 65-71: The deferred test helper is duplicated across two test
files. Create or reuse a shared deferred utility with resolve and reject
support, then replace the local deferred<T>() definitions with imports in
app/src/components/flows/FlowRunsDrawer.test.tsx#L65-L71 and
app/src/components/intelligence/SyncConfirmDialog.test.tsx#L33-L41; no other
behavior changes are needed.
In `@app/src/components/intelligence/SyncConfirmDialog.tsx`:
- Around line 30-48: Replace the local cancelled flag and cleanup logic in the
SyncConfirmDialog useEffect with the shared useLatestAsync hook. Route the
openhuman.memory_sources_estimate_sync_cost request through that hook while
preserving the existing estimate and error state updates and sourceId dependency
behavior.
- Around line 36-40: Replace the local cancelled flag in the SyncConfirmDialog
async effect with the shared useLatestAsync guard. Apply the guard around the
openhuman.memory_sources_estimate_sync_cost call and only update estimate
through the guard, removing the local cancellation state while preserving the
existing RPC parameters and result handling.
In `@app/src/components/settings/panels/__tests__/TeamMembersPanel.test.tsx`:
- Around line 218-221: Update the relevant TeamMembersPanel tests, including the
case around mockChangeRole and the similar case near the second referenced
location, to await completion of the resolved request before the test ends. Keep
request.resolve({}) inside the test’s async act/wait flow and wait for the
resulting refresh, state updates, and dialog transition so no promise
continuation runs after teardown.
In `@app/src/hooks/flowPendingApprovalsStore.ts`:
- Around line 45-48: Remove the redundant nextRequestId and activeRequestId
bookkeeping from the pending-approvals refresh flow. Update startRefresh to
retain the created request promise and, in its finally cleanup, clear inFlight
only when it still matches that promise; preserve the behavior that newer
refreshes are not cleared by older requests.
- Around line 101-107: Update the error path in the polling flow and the emits
in retainFlowPendingApprovalsPolling to reuse the existing frozen
snapshot.approvals array instead of passing it through makeSnapshot and
freezeApprovals again. Preserve the updated error and retention state while
keeping approvals’ reference identity unchanged across failed polls.
In `@app/src/hooks/useClipboardFeedback.ts`:
- Around line 25-29: Move the assignments updating writeTextRef.current and
resetAfterMsRef.current out of the render body in useClipboardFeedback and into
an effect that runs when options.writeText or the resolved options.resetAfterMs
changes. Keep the existing default reset duration behavior and ensure callbacks
use the synchronized refs.
In `@app/src/pages/WorkflowRunsPage.test.tsx`:
- Line 147: Remove the bg-amber-50 class assertion from the WorkflowRunsPage
test and retain the existing toHaveTextContent('pending approval') assertion,
leaving FlowRunStatus color mapping coverage to its dedicated tests.
In `@app/src/pages/WorkflowRunsPage.tsx`:
- Around line 37-41: Remove the initial setFlowNamesLoading(true) and
setFlowNamesError(null) calls from the useEffect that invokes listFlows, leaving
the asynchronous flow-loading logic and its existing state updates unchanged.
In `@app/src/services/api/__tests__/flowsApi.test.ts`:
- Around line 61-88: Consolidate the duplicate buildWorkflow proposal-parity
coverage by removing the valid-payload test and related setup from the sibling
__tests__ suite, preserving the existing parameterized coercion coverage in
flowsApi.test.ts. Update the suite header docblock to describe both importFlow
and buildWorkflow coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6fb72a29-7257-4f92-808d-1dcc91f3af19
📒 Files selected for processing (89)
app/src/agentworld/components/ConfirmDialog.test.tsxapp/src/agentworld/components/ConfirmDialog.tsxapp/src/agentworld/components/ExpandableResourceRow.test.tsxapp/src/agentworld/components/ExpandableResourceRow.tsxapp/src/agentworld/components/FormActions.test.tsxapp/src/agentworld/components/FormActions.tsxapp/src/agentworld/components/FormField.test.tsxapp/src/agentworld/components/FormField.tsxapp/src/agentworld/components/StatusBlock.test.tsxapp/src/agentworld/components/StatusBlock.tsxapp/src/agentworld/components/WalletAddressChip.test.tsxapp/src/agentworld/components/WalletAddressChip.tsxapp/src/agentworld/hooks/useMyAgentId.test.tsapp/src/agentworld/hooks/useMyAgentId.tsapp/src/agentworld/pages/BountiesSection.test.tsxapp/src/agentworld/pages/BountiesSection.tsxapp/src/agentworld/pages/DirectorySection.tsxapp/src/agentworld/pages/ExploreSection/ExploreSection.test.tsxapp/src/agentworld/pages/ExploreSection/index.tsxapp/src/agentworld/pages/FeedSection.tsxapp/src/agentworld/pages/IdentitiesSection.test.tsxapp/src/agentworld/pages/IdentitiesSection.tsxapp/src/agentworld/pages/JobsSection.test.tsxapp/src/agentworld/pages/JobsSection.tsxapp/src/agentworld/pages/LedgerSection.tsxapp/src/agentworld/pages/MessagingSection.tsxapp/src/agentworld/pages/ProfileViewer.test.tsxapp/src/agentworld/pages/ProfileViewer.tsxapp/src/agentworld/pages/ProfilesSection.tsxapp/src/components/approvals/ApprovalDecisionCard.test.tsxapp/src/components/approvals/ApprovalDecisionCard.tsxapp/src/components/channels/mcp/McpCatalogBrowser.test.tsxapp/src/components/channels/mcp/McpCatalogBrowser.tsxapp/src/components/channels/mcp/McpServersTab.test.tsxapp/src/components/channels/mcp/McpServersTab.tsxapp/src/components/chat/FlowApprovalRequestCard.tsxapp/src/components/chat/__tests__/FlowApprovalRequestCard.test.tsxapp/src/components/flows/FlowRunInspectorDrawer.tsxapp/src/components/flows/FlowRunPendingApprovalCard.test.tsxapp/src/components/flows/FlowRunPendingApprovalCard.tsxapp/src/components/flows/FlowRunStatus.test.tsxapp/src/components/flows/FlowRunStatus.tsxapp/src/components/flows/FlowRunsDrawer.test.tsxapp/src/components/flows/FlowRunsDrawer.tsxapp/src/components/flows/FlowRunsSidebar.test.tsxapp/src/components/flows/FlowRunsSidebar.tsxapp/src/components/flows/__tests__/FlowRunInspectorDrawer.test.tsxapp/src/components/intelligence/SyncConfirmDialog.test.tsxapp/src/components/intelligence/SyncConfirmDialog.tsxapp/src/components/layout/ChipTabs.test.tsxapp/src/components/layout/ChipTabs.tsxapp/src/components/settings/panels/TeamMembersPanel.tsxapp/src/components/settings/panels/__tests__/TeamMembersPanel.test.tsxapp/src/components/ui/ConfirmDialog.test.tsxapp/src/components/ui/ConfirmDialog.tsxapp/src/components/ui/Input.test.tsxapp/src/components/ui/Input.tsxapp/src/components/ui/LoadingState.test.tsxapp/src/components/ui/LoadingState.tsxapp/src/components/ui/ModalShell.test.tsxapp/src/components/ui/ModalShell.tsxapp/src/components/ui/index.tsapp/src/hooks/__tests__/flowPendingApprovalsStore.test.tsapp/src/hooks/__tests__/useClipboardFeedback.test.tsapp/src/hooks/__tests__/useDebouncedValue.test.tsapp/src/hooks/__tests__/useDismissLayer.test.tsapp/src/hooks/__tests__/useFlowPendingApprovals.test.tsapp/src/hooks/__tests__/useFlowRunsQuery.test.tsapp/src/hooks/__tests__/useLatestAsync.test.tsapp/src/hooks/__tests__/useRunsPendingApprovalSet.test.tsapp/src/hooks/flowPendingApprovalsStore.tsapp/src/hooks/useClipboardFeedback.tsapp/src/hooks/useDebouncedValue.tsapp/src/hooks/useDismissLayer.tsapp/src/hooks/useFlowPendingApprovals.tsapp/src/hooks/useFlowRunsQuery.tsapp/src/hooks/useLatestAsync.tsapp/src/hooks/useRunsPendingApprovalSet.tsapp/src/lib/workflows/workflowProposal.test.tsapp/src/lib/workflows/workflowProposal.tsapp/src/pages/Invites.test.tsxapp/src/pages/Invites.tsxapp/src/pages/WorkflowRunsPage.test.tsxapp/src/pages/WorkflowRunsPage.tsxapp/src/services/api/__tests__/flowsApi.test.tsapp/src/services/api/flowsApi.test.tsapp/src/services/api/flowsApi.tsdocs/plans/2026-07-24-react-reuse-foundation.mddocs/specs/2026-07-24-react-reuse-foundation-design.md
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
Validation
The broad full Vitest run was started locally and produced only the repository's known jsdom/socket diagnostics before publication; targeted and aggregate suites are green.
Summary by CodeRabbit
New Features
Bug Fixes