Skip to content

Fix dashboard client placeholder relink#237

Merged
TraderSamwise merged 6 commits into
masterfrom
chore/next-20260623-tui-connection-adapter-12
Jun 24, 2026
Merged

Fix dashboard client placeholder relink#237
TraderSamwise merged 6 commits into
masterfrom
chore/next-20260623-tui-connection-adapter-12

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace client dashboard placeholders atomically with tmux link-window -k instead of killing the placeholder first
  • reuse strict dashboard target validation in metadata-server open-dashboard paths
  • cover placeholder relink and stricter dashboard validation in tests

Verification

  • yarn typecheck
  • yarn lint
  • yarn vitest --run
  • yarn build
  • AIMUX_RELEASE_VERSION=local-dashboard-link-repair-20260624a yarn release:asset
  • scripts/install.sh release/aimux-darwin-arm64.tar.gz
  • aimux restart --json
  • aimux doctor versions --json

Summary by CodeRabbit

  • Bug Fixes

    • Improved dashboard window replacement with more reliable atomic operations.
    • Enhanced dashboard target validation for more accurate reuse detection.
  • Tests

    • Added comprehensive tests for dashboard window linking behavior.
    • Expanded test coverage for dashboard initialization and runtime management scenarios.

@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 1:36am

@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 3 hours, 12 minutes, and 39 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 refill rate.

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 850e8e3c-4e2d-472a-9bf6-28befb6601c4

📥 Commits

Reviewing files that changed from the base of the PR and between ca493f9 and 1d03101.

📒 Files selected for processing (2)
  • src/tmux/runtime-manager.test.ts
  • src/tmux/runtime-manager.ts
📝 Walkthrough

Walkthrough

isUsableDashboardTarget is exported from targets.ts and consumed in findExistingDashboardTarget in metadata-server.ts, which now derives a build stamp via getDashboardCommandSpec to filter candidate dashboard windows. Separately, ensureLinkedWindow in runtime-manager.ts replaces an explicit killWindow + link-window sequence with a single link-window -k, delegating atomic slot replacement to tmux. Both changes are covered by updated and new tests.

Changes

Dashboard Window Management

Layer / File(s) Summary
Dashboard target eligibility filter by build stamp
src/dashboard/targets.ts, src/metadata-server.ts, src/metadata-server.test.ts
isUsableDashboardTarget is exported; findExistingDashboardTarget fetches a build stamp via getDashboardCommandSpec and gates candidates through isUsableDashboardTarget instead of isWindowAlive. Tests stub getWindowOption, getSessionOption, displayMessage, and captureTarget on the TmuxRuntimeManager prototype in two test cases with matching finally-block restores.
Atomic window replacement via link-window -k
src/tmux/runtime-manager.ts, src/tmux/runtime-manager.test.ts
ensureLinkedWindow removes the explicit killWindow call for an occupied windowIndex and instead appends -k to the link-window arguments. Tests assert kill-window is never invoked; a new test validates that opening a dashboard placeholder slot emits link-window without kill-window and ends with a client switch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A stamp on the window, a -k in the link,
No killing required — just blink and think!
The dashboard finds targets by build and by name,
Atomic replacement, the tmux-bunny's game.
Less code, fewer calls — the warren is clean! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 directly describes the main change: fixing dashboard client placeholder relinking through atomic replacement with tmux link-window -k.
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 chore/next-20260623-tui-connection-adapter-12

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.

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Sub-agent review finding addressed: the first version used tmux link-window -k after pre-validating the dashboard slot, which could force-replace a non-dashboard if the slot changed before the tmux command executed. Fixed in 069b40d by preserving one-window client sessions with a temporary keepalive window, killing only the known dashboard placeholder by window id, and linking without -k so a changed slot fails instead of deleting the new occupant. Added a regression test for the no--k behavior.

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Round 2 sub-agent finding addressed: with renumber-windows on, the keepalive window could slide into index 0 after deleting the dashboard placeholder. Fixed in 9013ab7 by temporarily disabling renumber-windows while the keepalive/dashboard replacement is in progress and restoring the original value in a finally block. Added assertions covering renumber-windows on/off restore around the relink.

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Final sub-agent finding addressed: renumber-windows restore now covers keepalive creation failures too. Fixed in 237cd96 by moving keepalive creation under the same try/finally as link/cleanup, and added a regression where new-window throws after renumber-windows is disabled; the test asserts the original option is restored and no windows are killed.

@TraderSamwise
TraderSamwise merged commit ac70fb9 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