Port OpenRig-inspired UI surfaces to native app#41
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces three new project exploration tabs (topology, project, library) to the mobile and desktop navigation, refactors the notifications screen to render a unified "For You" feed built from security events and notifications, and adds supporting domain models for topology visualization, project observability, and runtime branding. The feed classifies notifications into actionable kinds and merges them with agent/service attention states. Library documents are served from the metadata server and browsable in the library screen. ChangesNew tabs, refactored feed, and supporting infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/app/(main)/(tabs)/(inbox)/notifications.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/app/(main)/(tabs)/library.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/app/(main)/(tabs)/project.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/app/`(main)/(tabs)/(inbox)/notifications.tsx:
- Around line 475-486: The UI currently gates rendering of locally-derived
attention cards on the notifications fetch (checks endpoint/feed before
visibleCards), causing securityEvents/desktopState cards built by
buildForYouFeed(...) to disappear when notifications are offline; change the
conditional order in the render so that if visibleCards.length > 0 you render
those cards immediately (use visibleCards and the buildForYouFeed output), and
only show the "Project host offline" / "Loading feed..." Card in the branch that
specifically represents the notifications-backed portion (i.e. when endpoint or
feed are missing but visibleCards is empty); update the JSX conditionals that
reference endpoint, feed, and visibleCards accordingly so local data
(securityEvents, desktopState) is not hidden while notifications are
unavailable.
- Around line 216-235: Replace the hard-coded icon color "`#fafafa`" with a
theme-aware value so outline buttons match the button text color: remove
color="`#fafafa`" from the icon usages and either add className="text-foreground"
to the icon components (ExternalLink, Check, etc.) or pass color="currentColor"
so they inherit the button text color; update the instances in notifications.tsx
where ExternalLink and Check are used for the Open and onRead buttons (and the
similar occurrences around the other noted blocks).
In `@app/app/`(main)/(tabs)/library.tsx:
- Around line 71-80: The current fetch path (around the block using setLoading,
getToken, listProjectLibrary, setDocuments, setSelectedId) can show stale
documents from the previous project until the new response arrives; before
initiating the async request, clear the UI state by calling setDocuments([]) and
setSelectedId(null) (or an appropriate empty state) so the library list is
emptied while loading, then proceed with getToken/listProjectLibrary and
repopulate on success; apply the same change to the other fetch block referenced
(around lines 142-173) to prevent cross-project stale content on refresh or
project switch.
In `@app/app/`(main)/(tabs)/project.tsx:
- Around line 173-175: When handling fetch failures in project.tsx, the catch
blocks currently call setTaskError but leave previous tasks intact, causing
stale Queue data to show; update those catch blocks (the ones calling
setTaskError) to also clear the queued tasks state (call setTasks([]) or the
equivalent task-reset setter) so the UI doesn't display stale items; apply the
same change to the second catch block referenced around the setTaskError usage
at the other occurrence (lines ~263-270) so both refresh failure paths reset
tasks as well.
In `@src/metadata-server.ts`:
- Around line 87-93: LIBRARY_DOC_ALLOWLIST currently includes
".aimux/config.json", which can leak sensitive local config; remove that entry
from the LIBRARY_DOC_ALLOWLIST constant and add a defensive server-side
exclusion in the /library response code (the route handler that reads
LIBRARY_DOC_ALLOWLIST) to skip any paths that start with ".aimux/" or match
"*config.json"; update any other places that reference LIBRARY_DOC_ALLOWLIST
(the other occurrence flagged in the review) to ensure they also do not expose
config files.
- Around line 95-114: The listLibraryDocuments function currently calls statSync
and readFileSync directly for each entry from LIBRARY_DOC_ALLOWLIST which can
throw and break the entire response; wrap the per-entry filesystem work
(existsSync/statSync/readFileSync and content slicing) in a try/catch inside the
flatMap callback (or extract to a helper like readLibraryEntry) so any exception
for a single file is caught, logged or ignored, and that entry is skipped
(return [] for that entry) instead of propagating the error; reference the
symbols listLibraryDocuments, LIBRARY_DOC_ALLOWLIST, statSync, readFileSync and
ensure truncated/size/updatedAt logic remains inside the guarded block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c27f722-7653-4b81-849b-54c7b79a10e5
📒 Files selected for processing (23)
app/app/(main)/(tabs)/(inbox)/notifications.tsxapp/app/(main)/(tabs)/_layout.tsxapp/app/(main)/(tabs)/library.tsxapp/app/(main)/(tabs)/project.tsxapp/app/(main)/(tabs)/topology.tsxapp/app/index.tsxapp/components/DetailPanel.tsxapp/components/MobileTabBar.tsxapp/components/ProjectSidebar.tsxapp/components/RuntimeBadge.tsxapp/lib/api.tsapp/lib/for-you-feed.test.tsapp/lib/for-you-feed.tsapp/lib/main-tabs.tsapp/lib/openrig-topology.test.tsapp/lib/openrig-topology.tsapp/lib/project-observability.test.tsapp/lib/project-observability.tsapp/lib/runtime-brand.test.tsapp/lib/runtime-brand.tssrc/daemon.test.tssrc/daemon.tssrc/metadata-server.ts
Summary
Verification
Summary by CodeRabbit
New Features
Tests
Chores