Skip to content

Reject dead control targets - #188

Merged
TraderSamwise merged 6 commits into
masterfrom
fix/dead-control-targets
Jun 20, 2026
Merged

Reject dead control targets#188
TraderSamwise merged 6 commits into
masterfrom
fix/dead-control-targets

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • require API-resolved managed windows to still be alive before returning control targets
  • cover dead focus-window and notification-target resolution paths

Verification

  • yarn typecheck
  • yarn vitest run src/metadata-server.test.ts
  • git diff --check
  • yarn lint
  • yarn --cwd app typecheck
  • yarn --cwd app test
  • yarn vitest run
  • yarn --cwd app export:web --output-dir /tmp/aimux-epic4-review-fix14-web-export
  • yarn build

Summary by CodeRabbit

  • Bug Fixes

    • Windows that are no longer active or have been closed are now properly excluded from agent switch controls, expose item lists, and notification target operations for improved reliability.
  • Tests

    • Comprehensive test coverage added for window liveness validation across switch controls, expose items, notification targets, and focus workflows to ensure inactive windows are properly filtered.

@vercel

vercel Bot commented Jun 20, 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 20, 2026 6:06am

@coderabbitai

coderabbitai Bot commented Jun 20, 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 37 minutes and 25 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: 1664e5b4-0c12-48bd-8a32-405f0820ea5f

📥 Commits

Reviewing files that changed from the base of the PR and between 2a602de and a2613e5.

📒 Files selected for processing (6)
  • scripts/tmux-control.sh
  • src/fast-control.test.ts
  • src/fast-control.ts
  • src/metadata-server.test.ts
  • src/metadata-server.ts
  • src/tmux/control-script.test.ts
📝 Walkthrough

Walkthrough

Adds tmux.isWindowAlive liveness filtering to listSwitchableAgentItems and findProjectManagedWindow, and updates openNotificationTarget resume paths to use the liveness-aware findProjectManagedWindow. Existing mocks across three test files are updated to include isWindowAlive, and new tests verify that dead windows are excluded from switch-control lists, expose items, and notification-target/focus-window HTTP handlers.

Changes

Window liveness filtering for managed windows

Layer / File(s) Summary
Core liveness filtering implementation
src/fast-control.ts, src/metadata-server.ts
listSwitchableAgentItems filters listManagedWindows results via tmux.isWindowAlive. findProjectManagedWindow's predicate adds the same liveness check. openNotificationTarget resume paths switch from tmux.findManagedWindow to the liveness-aware findProjectManagedWindow with a sessionId filter.
fast-control and expose-model test updates
src/fast-control.test.ts, src/tmux/expose-model.test.ts
All existing TmuxRuntimeManager mocks gain isWindowAlive: vi.fn(() => true). New test in fast-control.test.ts asserts dead windows are excluded from switch-control lists and that resolveAttentionAgent returns null for a dead candidate. New test in expose-model.test.ts asserts dead windows are excluded from listExposeAgentItems.
metadata-server test updates
src/metadata-server.test.ts
Existing notification-target and focus-mutation tests now stub and restore TmuxRuntimeManager.prototype.isWindowAlive. New HTTP tests cover: dead resumed-service window → 404, dead resumed-agent window → 404, dead notification-target window → 404 "window not found", dead managed focus-window target → 404 for both focus: false and focus: true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • TraderSamwise/aimux#122: Extends listSwitchableAgentItems and listExposeAgentItems — the same functions this PR modifies to add liveness filtering.
  • TraderSamwise/aimux#134: The Exposé scope ladder uses listSwitchableAgentItems for worktree/project rungs, so its tile set is directly affected by the liveness filter added here.
  • TraderSamwise/aimux#187: Modifies the same findProjectManagedWindow helper and openNotificationTarget resume/focus flow in src/metadata-server.ts.

Poem

🐇 Hop, hop — dead panes must go,
No ghost windows in our show!
isWindowAlive now stands the gate,
Only living tmux panes navigate.
The rabbit checks before you switch,
Dead windows vanish without a glitch! 🪟✨

🚥 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 'Reject dead control targets' directly and clearly summarizes the main change: filtering out dead/invalid control targets across the codebase.
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/dead-control-targets

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

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

🤖 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/metadata-server.test.ts`:
- Around line 889-890: The issue is that await server.start() is called after
mutating TmuxRuntimeManager.prototype but before entering the try/finally block
that manages these prototype stubs, causing stubs to leak into later tests if
start throws an error. Move the await server.start() calls (at lines 889, 960,
and 1020) inside the try block that contains the corresponding
TmuxRuntimeManager.prototype mutations so that the finally block can properly
clean up the stubs regardless of whether start succeeds or fails.
🪄 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: d51d6719-7e97-4d03-8c4e-73a9438ba77e

📥 Commits

Reviewing files that changed from the base of the PR and between ece1b05 and 2a602de.

📒 Files selected for processing (5)
  • src/fast-control.test.ts
  • src/fast-control.ts
  • src/metadata-server.test.ts
  • src/metadata-server.ts
  • src/tmux/expose-model.test.ts

Comment thread src/metadata-server.test.ts Outdated
@TraderSamwise

Copy link
Copy Markdown
Owner Author

Review loop resolutions:

  • Fixed CodeRabbit summary finding in a2613e5: server.start() for the affected metadata-server prototype-stub tests now runs inside the matching try/finally cleanup block.
  • Fixed sub-agent finding in d8bddec/a2613e5: fast-control and tmux-control now keep the current dead pane only for context/index calculation, while excluding dead panes from target selection.

Verification after fixes: yarn typecheck, yarn lint, yarn vitest, yarn build; PR checks are green.

@TraderSamwise
TraderSamwise merged commit 0bb0bd9 into master Jun 20, 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