Conversation
|
@EtanHey is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis change introduces an offscreen NSWindow mechanism to enable background surface creation without a real window attachment. It extends the PortalLifecycleState enum to include closing and closed states, adds offscreen window lifecycle management during surface initialization, and clears resources during teardown. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/GhosttyTerminalView.swift (1)
2743-2753: Optional: set explicit frame/autoresizing for offscreen host attachment.This makes the temporary-host path more robust if default view sizing changes later.
Suggested patch
offscreenHostWindow = window - window.contentView?.addSubview(hostedView) + if let contentView = window.contentView { + hostedView.frame = contentView.bounds + hostedView.autoresizingMask = [.width, .height] + contentView.addSubview(hostedView) + } // viewDidMoveToWindow fires on surfaceView → attachToView → createSurface🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 2743 - 2753, The offscreen host window and its hostedView are added without an explicit frame or resizing behavior which can break if default sizing changes; update the block that creates offscreenHostWindow (the temporary NSWindow assigned to offscreenHostWindow and its hostedView) to set a concrete frame for hostedView (e.g., match window.contentView!.bounds) and configure autoresizingMask or Auto Layout constraints so the hostedView always fills the contentView; ensure this change is applied where window is created and before calling viewDidMoveToWindow / createSurface so the attachment has deterministic sizing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2743-2753: The offscreen host window and its hostedView are added
without an explicit frame or resizing behavior which can break if default sizing
changes; update the block that creates offscreenHostWindow (the temporary
NSWindow assigned to offscreenHostWindow and its hostedView) to set a concrete
frame for hostedView (e.g., match window.contentView!.bounds) and configure
autoresizingMask or Auto Layout constraints so the hostedView always fills the
contentView; ensure this change is applied where window is created and before
calling viewDidMoveToWindow / createSurface so the attachment has deterministic
sizing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2b51307-c4bb-42a1-9d16-286ad7eebd86
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
|
I have the same need — I'm building an orchestrator pattern where a conductor pane sends commands to other panes via Reviewed the diff and the approach looks solid:
One minor thought: the Would love to see this merged — it unblocks programmatic multi-workspace orchestration for both MCP servers and CLI-driven workflows. Happy to help test once a build is available. |
…ation Background workspaces created via `workspace.create` (socket API) have dead PTY surfaces because ghostty_surface_new() requires a view backed by a live NSWindow for Metal layer setup and backing context. When requestBackgroundSurfaceStartIfNeeded() is called for a workspace that hasn't been displayed, the surfaceView has no window — so the attachToView → createSurface chain silently defers creation. The PTY is never forked, leaving the terminal as a ghost: listed in surface health but unable to accept input or return screen content. Fix: when the surfaceView has no window, create a small offscreen NSWindow positioned at (-10000, -10000) and temporarily host the hostedView in it. This triggers the standard viewDidMoveToWindow → attachToView → createSurface path with a valid window context. The offscreen window is released when the view migrates to a real window (workspace is displayed) or on teardown. Fixes manaflow-ai#1472 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
579040c to
7b85f3f
Compare
|
Rebased onto current main, resolved conflicts from portal sync changes. Also applied CodeRabbit's frame/autoresizing suggestion. Issue #1472 is still open — would appreciate a maintainer review when you get a chance! 🙏 |
|
Rebased onto current main on April 9, 2026, and issue #1472 is still open. This still unblocks background workspace PTY initialization; would appreciate a maintainer review when convenient. |
|
Thanks for this! Background-workspace terminals now start via an offscreen host, the same idea as yours landed on main in #4233. You opened this first, so you got there first. Closing since main covers it now. |
3 participants
Fixes #1472
Summary
workspace.createwithselect: false) having dead PTY surfaces that rejectsend,read-screen, andsend-keycommands.NSWindowhost soghostty_surface_new()gets a valid Metal-backed window context even when the workspace has never been displayed.Why
We built an MCP server on top of cmux for multi-agent AI orchestration — spawning Claude Code, Codex, and Gemini CLI sessions in separate workspaces programmatically. When creating workspaces via the socket API without selecting them, the surfaces are listed correctly but all commands fail with "Surface is not a terminal."
Root cause:
requestBackgroundSurfaceStartIfNeeded()callsattachToView, which requiressurfaceView.window != nil. Background workspaces have noNSWindow, socreateSurfacesilently defers and the PTY is never forked.Testing
cmux new-workspace(without--focus)cmux read-screen --surface <new-surface>Could not verify local build —
zigis required forGhosttyKit.xcframework. CI should handle build verification.