Skip to content

Fix TUI project-root recovery leftovers#245

Merged
TraderSamwise merged 3 commits into
masterfrom
fix/tui-api-cleanup-next-4
Jun 24, 2026
Merged

Fix TUI project-root recovery leftovers#245
TraderSamwise merged 3 commits into
masterfrom
fix/tui-api-cleanup-next-4

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • use the dashboard host project root for project-service recovery, SSE endpoint invalidation, live tmux opens, statusline priming, and session-viewed writes
  • remove the legacy project API /inbox route, runtime inbox mutation routes, and stale inbox/plans SSE view aliases
  • update app/TUI event consumers and regression tests for the API-first notifications/coordination contract

Verification

  • yarn vitest --run src/multiplexer/dashboard-control.test.ts src/multiplexer/project-event-stream.test.ts src/metadata-server.test.ts src/project-api-contract.test.ts src/notifications.test.ts
  • yarn typecheck
  • yarn lint
  • yarn vitest --run
  • yarn build
  • (cd app && yarn typecheck && yarn lint)
  • pre-push hook: yarn typecheck && yarn lint && yarn test

Summary by CodeRabbit

  • Bug Fixes
    • Notification updates now refresh the correct screens without triggering unrelated inbox refreshes.
    • Alert and notification changes are reflected in the notifications and coordination views more reliably.
    • Dashboard and project event handling now use the active project location, improving consistency when recovering or reconnecting.
    • Removed outdated inbox and plans view handling from the app’s visible update flow.

@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jun 24, 2026 12:55pm

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@TraderSamwise, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 16 minutes and 32 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0c73bc2d-5167-48d2-89eb-8747bc258479

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee27e4 and 941ed62.

📒 Files selected for processing (15)
  • src/agent-tracker.test.ts
  • src/agent-tracker.ts
  • src/metadata-server.ts
  • src/multiplexer/dashboard-control.test.ts
  • src/multiplexer/dashboard-control.ts
  • src/multiplexer/session-launch.ts
  • src/notification-context.ts
  • src/notifications.test.ts
  • src/notifications.ts
  • src/notify.test.ts
  • src/notify.ts
  • src/project-events.ts
  • src/session-viewed.test.ts
  • src/session-viewed.ts
  • src/shell-state.ts
📝 Walkthrough

Walkthrough

The PR removes the inbox view and its associated API routes (/inbox, /inbox/read, /inbox/clear) from the project API contract, metadata server, event bus, and all consuming clients. Separately, it fixes dashboard control operations to derive the active project root from host.projectRoot via a new dashboardProjectRoot(host) helper instead of always falling back to process.cwd().

Changes

Inbox View and Route Removal

Layer / File(s) Summary
API contract: remove inbox/plans views and inbox routes
src/project-api-contract.ts, src/project-api-contract.test.ts
Removes the inbox top-level route, inboxRead/inboxClear runtime routes from PROJECT_API_ROUTES, and removes "inbox" and "plans" from PROJECT_API_VIEWS while adding "library" and "notifications". Tests assert the removed entries are absent.
Metadata server: delete inbox handlers and helpers
src/metadata-server.ts
Removes the createRuntimeExchangeStore import, deletes runtimeInboxEntries/runtimeInboxEntry/markRuntimeInboxRead helpers, removes GET /inbox and POST inboxRead/inboxClear route handlers, and narrows notifyProjectChanged views for notifications.read and notifications.clear to exclude "inbox".
Event bus and SSE refresh: remove inbox from view payloads
src/project-events.ts, src/multiplexer/project-event-stream.ts
ProjectEventBus.publishAlert drops "inbox" from publishProjectUpdate views. refreshViews removes inbox from desktop-state and coordination screen refresh triggers, and removes plans from the library screen trigger.
Client handler, debug-state, and tests
app/app/(main)/_layout.tsx, src/debug-state.ts, src/metadata-server.test.ts, src/notifications.test.ts
App layout narrows notification feed refresh to only "notifications". Debug-state removes the "inbox entries" mention. Tests replace inbox-based assertions with notifications-endpoint queries and remove "inbox" from expected project_update view payloads.

Dashboard Project Root Scoping

Layer / File(s) Summary
dashboardProjectRoot helper and path import
src/multiplexer/dashboard-control.ts, src/multiplexer/project-event-stream.ts
Introduces dashboardProjectRoot(host) and projectEventStreamProjectRoot(host) helpers that resolve host.projectRoot or fall back to process.cwd(). Updates the paths import to use getProjectStateDirFor.
Statusline and tmux window management scoped to project root
src/multiplexer/dashboard-control.ts
writeStatuslineTextFile uses getProjectStateDirFor(projectRoot). primeLiveTmuxFooter, openLiveTmuxWindowForEntry, openLiveTmuxWindowForService, markSessionViewed, and noteLastUsedItem all use dashboardProjectRoot(host) instead of process.cwd().
Runtime guard probing, endpoint health, and control plane
src/multiplexer/dashboard-control.ts
startRuntimeGuardRepair, refreshRuntimeGuard, resolveCurrentProjectServiceEndpointForDashboard, endpointMatchesCurrentProjectService, and ensureDashboardControlPlane all use dashboardProjectRoot(host) and getProjectStateDirFor(projectRoot) for path resolution. SSE reconnect cleanup in DashboardProjectEventAdapter uses the per-project root for removeMetadataEndpoint.
Tests: project root assertions
src/multiplexer/dashboard-control.test.ts, src/multiplexer/project-event-stream.test.ts
Adds updateSessionMetadata mock wiring, a postToProjectService test asserting cleanup uses the provided projectRoot, a live target activation test asserting statusline files land under the dashboard project root, and updates SSE reconnect tests to expect "/repo/project" instead of process.cwd().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • TraderSamwise/aimux#184: Modifies src/project-api-contract.ts and src/metadata-server.ts notification routing, directly overlapping with this PR's inbox route and view set changes.
  • TraderSamwise/aimux#231: Changes refreshViews view-triggering logic in src/multiplexer/project-event-stream.ts, the same function this PR modifies to remove inbox and plans triggers.
  • TraderSamwise/aimux#240: Modifies startRuntimeGuardRepair/refreshRuntimeGuard in src/multiplexer/dashboard-control.ts, the same functions this PR updates to use dashboardProjectRoot(host).

Poem

🐇 Hoppin' through the codebase with a tidy little broom,
The inbox routes are swept away to make the notifications room.
No more process.cwd() when the host knows where it lives—
Each dashboard finds its own root path, and that's the gift it gives.
✨ The views are trimmed, the helpers gleam, the state dirs set just right,
This rabbit tidied up the code and called it quite a night! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main theme: fixing TUI project-root recovery behavior and related leftovers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tui-api-cleanup-next-4

Comment @coderabbitai help to get the list of available commands.

@TraderSamwise

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/notifications.test.ts (1)

170-174: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert "inbox" absence explicitly in this test.

Line 173 uses arrayContaining, so the test still passes if "inbox" is present as an extra view.

Suggested assertion hardening
     expect(events).toEqual([
       expect.objectContaining({ type: "alert", sessionId: "codex-1" }),
       expect.objectContaining({
         type: "project_update",
         sessionId: "codex-1",
         views: expect.arrayContaining(["coordination-worklist", "notifications"]),
       }),
     ]);
+    const projectUpdate = events[1] as { views?: string[] };
+    expect(projectUpdate.views ?? []).not.toContain("inbox");
🤖 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 `@src/notifications.test.ts` around lines 170 - 174, The project update
notification assertion in notifications.test.ts is too permissive because
arrayContaining still allows extra views like "inbox". Update the test around
the project_update expectation to explicitly verify that the views array
contains "coordination-worklist" and "notifications" while also asserting that
"inbox" is not present, using the existing project_update object in the test.
🤖 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 `@src/multiplexer/dashboard-control.ts`:
- Line 718: The direct call to markSessionViewed in dashboard-control bypasses
the dashboard API boundary and can miss project-scoped writes for
session/notification metadata. Update the dashboard control flow to invoke this
through an existing request helper or add a new project-root-aware dashboard API
helper, using the markSessionViewed and dashboardProjectRoot(host) path as the
target location to route the write correctly.
- Around line 1162-1167: The endpoint health cache in dashboard-control is keyed
only by projectServiceEndpointHealthKey(endpoint), which can return a stale
"current" result even when projectRoot changes and
endpointMatchesCurrentProjectService now validates projectStateDir against
projectRoot. Update the cache lookup and the cached writer in the same
health-check flow to include the resolved projectRoot in the cache identity, so
both the read and write paths are scoped to the endpoint plus project root.
- Around line 65-67: The dashboardProjectRoot helper is validating
host.projectRoot with trim() but still returning the untrimmed value, so
normalize the returned project root before it is used for state dirs, daemon
calls, and endpoint identity. Update dashboardProjectRoot to return the trimmed
host.projectRoot when it is a non-empty string, and keep the process.cwd()
fallback for missing or blank values.

---

Nitpick comments:
In `@src/notifications.test.ts`:
- Around line 170-174: The project update notification assertion in
notifications.test.ts is too permissive because arrayContaining still allows
extra views like "inbox". Update the test around the project_update expectation
to explicitly verify that the views array contains "coordination-worklist" and
"notifications" while also asserting that "inbox" is not present, using the
existing project_update object in the test.
🪄 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

Run ID: edf73f97-ad93-46ed-a8cf-490bfd192d04

📥 Commits

Reviewing files that changed from the base of the PR and between 0c43f7a and 1ee27e4.

📒 Files selected for processing (12)
  • app/app/(main)/_layout.tsx
  • src/debug-state.ts
  • src/metadata-server.test.ts
  • src/metadata-server.ts
  • src/multiplexer/dashboard-control.test.ts
  • src/multiplexer/dashboard-control.ts
  • src/multiplexer/project-event-stream.test.ts
  • src/multiplexer/project-event-stream.ts
  • src/notifications.test.ts
  • src/project-api-contract.test.ts
  • src/project-api-contract.ts
  • src/project-events.ts
💤 Files with no reviewable changes (1)
  • src/project-api-contract.ts

Comment thread src/multiplexer/dashboard-control.ts
Comment thread src/multiplexer/dashboard-control.ts Outdated
Comment thread src/multiplexer/dashboard-control.ts Outdated
@TraderSamwise

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Resolved the CodeRabbit summary items in 941ed62: trimmed dashboard project roots, scoped endpoint health cache by project state dir, routed TUI notification focus/mark-seen through project-service API, and hardened the inbox-absence assertion.

@TraderSamwise
TraderSamwise merged commit ed3aad8 into master Jun 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant