feat(expose): add pane output taps#371
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds tmux pane-piping helpers, a bounded pane-output tap with ownership-aware expiration cleanup, and MetadataServer integration that tracks switchable-agent output alongside preview caching. ChangesPane output tap
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SwitchableAgentsControl
participant MetadataServer
participant ExposePaneOutputTap
participant TmuxRuntimeManager
participant TapFile
SwitchableAgentsControl->>MetadataServer: request items with includePreview
MetadataServer->>ExposePaneOutputTap: trackItems(items)
ExposePaneOutputTap->>TmuxRuntimeManager: pipeTargetToFile(target, filePath, ownership)
TmuxRuntimeManager->>TapFile: append pane output
ExposePaneOutputTap-->>MetadataServer: read bounded tap snapshot
MetadataServer-->>SwitchableAgentsControl: return previewSnapshot
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/expose-pane-output-tap.ts`:
- Around line 118-143: Update startTracked and stopTracked to track and verify
ownership of the pane pipe rather than assuming attachment succeeded or remains
unchanged. Make pipe acquisition report a verifiable ownership token atomically,
persist it with the tracked target, and have stopTracked call stopPanePipe only
when that token still identifies this manager’s pipe; otherwise remove only this
manager’s file and leave other consumers’ pipes untouched.
In `@src/metadata-server.ts`:
- Around line 2654-2660: Update the item serialization flow in the
includePreview block to read the captured output via exposePaneOutputTap.read
for each target window, then merge that tap snapshot into the preview payload,
using it as a fallback when exposePreviewCache.get returns no snapshot. Ensure
serializeFastControlItem receives the combined preview data so the captured tap
output is included in the Exposé response.
🪄 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: 59cc0839-cc11-4a19-b0df-afa5c889bd9d
📒 Files selected for processing (6)
src/expose-pane-output-tap.test.tssrc/expose-pane-output-tap.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/tmux/runtime-manager.test.tssrc/tmux/runtime-manager.ts
|
Sub-agent review finding resolved in 5e2e22c: active tap files were previously compacted only on read or renewed demand. The tap manager now runs periodic maintenance compaction while demand is live, with focused coverage for compaction without reads. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/expose-pane-output-tap.ts (1)
152-155: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDo not treat the 10 ms token poll as a failed acquisition.
pipe-panelaunches the token-writing shell command asynchronously. If it starts after this poll, cleanup runs first, then the child recreates the token/log and begins piping—but no target is tracked. Later demand sees an already-piped pane, so this orphaned pipe never expires and its output is unbounded. Keep provisional ownership until a safe asynchronous confirmation/cleanup path completes, and add delayed-token coverage.🤖 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/expose-pane-output-tap.ts` around lines 152 - 155, Update the ownership handling around waitForOwnershipToken so a single 10 ms poll timeout does not immediately delete the files or abandon tracking. Retain provisional ownership and use a safe asynchronous confirmation/cleanup path that handles a token written shortly after the initial poll, while preserving cleanup for genuine acquisition failure; add coverage for delayed token creation.
🤖 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.
Duplicate comments:
In `@src/expose-pane-output-tap.ts`:
- Around line 152-155: Update the ownership handling around
waitForOwnershipToken so a single 10 ms poll timeout does not immediately delete
the files or abandon tracking. Retain provisional ownership and use a safe
asynchronous confirmation/cleanup path that handles a token written shortly
after the initial poll, while preserving cleanup for genuine acquisition
failure; add coverage for delayed token creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78bb795d-595b-4627-ac04-cbf63bcfadbb
📒 Files selected for processing (6)
src/expose-pane-output-tap.test.tssrc/expose-pane-output-tap.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/tmux/runtime-manager.test.tssrc/tmux/runtime-manager.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/metadata-server.test.ts
- src/metadata-server.ts
|
Sub-agent review finding resolved in 34df134: when a pane is already piped, the tap manager now checks the deterministic Aimux token/log files, adopts a live owned tap after service restart, and removes dead tap files without detaching unrelated pipes. Added regression coverage for restart adoption. |
|
Sub-agent final-round findings resolved in 02c322a: pending starts are now tracked until the ownership token appears or demand expires, so delayed token reporting can be adopted/stopped safely; the metadata route now starts tap demand only for panes missing warm capture snapshots. |
There was a problem hiding this comment.
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/expose-pane-output-tap.ts`:
- Around line 121-126: The pending-start flow around promotePendingStart must
preserve token ownership: require the live ownership token to equal item.token,
and on a mismatch discard only the pending entry without adopting or stopping
the other tap. Update stopTracked to avoid deleting files when a live different
token owns the target, and add a regression test covering a pending start
encountering another live token, verifying it neither stops nor removes that
tap.
🪄 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: 267e5cb4-0d65-4af6-805e-eaea1b09220f
📒 Files selected for processing (4)
src/expose-pane-output-tap.test.tssrc/expose-pane-output-tap.tssrc/metadata-server.test.tssrc/metadata-server.ts
💤 Files with no reviewable changes (1)
- src/metadata-server.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/metadata-server.ts
- src/expose-pane-output-tap.test.ts
|
Resolved final tap-ownership audit findings in 4a60fcc. Tracked tap reads/renewals now re-check live ownership before returning output, so stale log files cannot keep serving frozen previews after the token disappears. Pending starts also keep their original start deadline when ownership never arrives, then stop and retry instead of renewing forever while Exposé remains open. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Verification
Summary by CodeRabbit