Fix notification hook descriptor inheritance

[chapati23]

tl;dr

Notification hooks could inherit unrelated open pipes and wait until timeout. That could cause hook-failure alerts and false approval banners. This change closes those pipes when a hook starts. The regression test checks that hooks still receive their intended input.

Summary

• Close unrelated parent file descriptors when cmux starts a notification hook. Preserve the hook's standard streams.
• Test the behavior with an unrelated open pipe in the app process.

Testing

Head 6049a5cb4b (rebased on bb03a252a0). Gates: tagged Debug build with Xcode 27 passed; focused cmux-unit passed (16 policy tests and 1 descriptor-isolation test); git diff --check passed.
Manual: the signed notification-fd-main app launched and its isolated socket listed a workspace. The bundled Ghostty, computer-use, and command-palette helpers are present.
Local build setup: Rust 1.88, Zig 0.16, and Apple's Metal Toolchain. macOS 27 required a Rust build-dependency strip override and a temporary verifier command that selected Apple's dwarfdump; the temporary source edit was removed.
Not run: the full test suite (focused first-pass scope).
Not proven: CI on the repository-pinned Xcode 26 and daily notification behavior on the new tag.

Demo Video

• Video URL or attachment: Not applicable. This change has no visible UI state; the descriptor-isolation test exercises the behavior.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptileai review
@cubic-dev-ai review

Checklist

• I tested the change locally
• I added or updated tests for behavior changes
• For iOS connectivity, auth, lifecycle, workspace or terminal changes, I updated the deterministic soak coverage or explained why existing coverage still applies, and recorded the affected workload result (not applicable: macOS notification hooks only)
• I updated docs/changelog if needed (no docs or user-facing strings changed)
• I requested bot reviews after my latest commit (automatic PR reviews are pending)
• All code review bot comments are resolved (new-head review pending)
• All human review comments are resolved (new-head review pending)

Summary by CodeRabbit

• Bug Fixes
  • Improved notification hook process isolation by preventing unrelated application file descriptors from being inherited.
  • Notification hooks continue to receive their configured standard input, output, and error streams, while unrelated application descriptors remain unavailable to the hook. This helps ensure hooks run with only the intended streams.

Changelog

Fixed: Notification hooks no longer inherit cmux's open file descriptors, so a hook can't hang on a pipe it never used and raise a false failure or approval alert

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[vercel]

@chapati23 is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7dbfded8-20db-4ea0-91a3-dd93a511e480

📥 Commits

Reviewing files that changed from the base of the PR and between 6049a5c and 0a7c903.

📒 Files selected for processing (1)

• Sources/TerminalNotificationPolicy.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Notification hooks now use POSIX_SPAWN_CLOEXEC_DEFAULT to prevent inheritance of unrelated parent file descriptors. A test checks that a hook cannot access an unrelated pipe descriptor.

Changes

Notification hook isolation

Layer / File(s)	Summary
Close inherited descriptors and validate isolation Sources/TerminalNotificationPolicy.swift, cmuxTests/NotificationAndMenuBarTests.swift	spawnHook adds POSIX_SPAWN_CLOEXEC_DEFAULT. A Swift Testing suite checks that an unrelated parent pipe descriptor is not visible to the hook process.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0a7c9

Hooks retain their standard-stream input and output while unrelated app descriptors are excluded. No concrete merge-blocking issue is established, so the change is ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0a7c9

The change reduces notification hooks’ access to unrelated app resources while preserving their intended input and output streams. No new security concern was identified in the reviewed change, though validation remains limited.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected exposure is the descriptors available to a notification hook process and its descendants, rather than a newly introduced command-execution entrypoint.

Trust Boundaries and Controls

• observed — Explicit dup2 actions provide the hook’s three standard streams, while the new spawn flag excludes other parent descriptors by default.

🚥 Pre-merge checks | ✅ 24 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
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.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS: The diff changes only notification-hook process spawning and its regression test. It adds POSIX_SPAWN_CLOEXEC_DEFAULT and does not change Cloud terminal creation, persistent cmux-tui transport, …
Cmux Swift Actor Isolation	✅ Passed	The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT to NotificationHookProcessRun.spawnHook and adds comments. It does not introduce or worsen any MainActor, Sendable, service-protocol, shar…
Cmux Swift Blocking Runtime	✅ Passed	PASS. The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT to posix_spawn flags. It does not add a semaphore, blocking wait, sleep, delayed dispatch, polling loop, main-queue sync, or manual …
Cmux Browser Automation Off-Main	✅ Passed	PASS. The pull request changes only notification hook process spawning and notification tests. It does not add or move any browser.* socket command, WebKit wait, worker-lane browser command, or browse…
Cmux Expensive Synchronous Load	✅ Passed	The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT to notification-hook spawn flags. It adds no agent-history loader, file scan, transcript/trajectory/workstream parse, or large JSON/JSONL lo…
Cmux Cache Substitution Correctness	✅ Passed	The production diff changes only NotificationHookProcessRun.spawnHook flags to add POSIX_SPAWN_CLOEXEC_DEFAULT. It does not replace an authoritative read with a cached or opportunistic value in a …
Cmux No Hacky Sleeps	✅ Passed	PASS: The pull request changes only Swift production code and Swift test code. The production change adds the POSIX_SPAWN_CLOEXEC_DEFAULT spawn flag; it adds no sleep, timer, delayed dispatch, polli…
Cmux Algorithmic Complexity	✅ Passed	The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT to the hook spawn flags at Sources/TerminalNotificationPolicy.swift:622. It adds no collection scan or algorithmic work. The new test uses a f…
Cmux Swift Concurrency	✅ Passed	PASS. The diff changes POSIX spawn flags and adds a Swift Testing regression test. It does not introduce or expand background Dispatch queues, Combine state, completion-handler APIs, or fire-and-forge…
Cmux Swift @Concurrent	✅ Passed	The changed production code adds only a synchronous posix_spawn flag. The new async Swift Testing method is not actor-isolated, and it calls TerminalNotificationPolicyEngine.evaluate, which alread…
Cmux Swift Package Boundaries	✅ Passed	PASS: The production diff changes only the existing private NotificationHookProcessRun.spawnHook implementation by adding POSIX_SPAWN_CLOEXEC_DEFAULT to the spawn flags. It does not introduce or m…
Cmux Swiftpm Lockfiles	✅ Passed	The authoritative PR diff changes only Sources/TerminalNotificationPolicy.swift and cmuxTests/NotificationAndMenuBarTests.swift. It contains no Package.swift, Package.resolved, .gitignore, w…
Cmux Swift Logging	✅ Passed	The diff adds no production logging. The production change only adds the POSIX spawn flag and comments. The test adds shell printf/cat output as a test fixture, which is allowed. No print, `debu…
Cmux User-Facing Error Privacy	✅ Passed	PASS — The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT and a developer comment. It does not add or modify user-facing error, alert, command-output, API-body, or recovery text. The added de…
Cmux Full Internationalization	✅ Passed	PASS. The production diff only adds POSIX_SPAWN_CLOEXEC_DEFAULT and a developer comment; it introduces no user-facing Swift text, localization key, catalog entry, web text, or metadata. The added st…
Cmux Swiftui State Layout	✅ Passed	PASS: The PR changes POSIX process-spawn flags and adds a Swift Testing regression test. It does not add or materially expand SwiftUI views, ObservableObject/@published state, GeometryReader, lazy/lis…
Cmux Architecture Rethink	✅ Passed	PASS: The PR adds one local posix_spawn flag in NotificationHookProcessRun and a regression test. It introduces no sleeps, polling, new state owner, observer, side channel, duplicate UI wiring, or…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The PR changes notification hook spawning and adds a test-only descriptor-isolation fixture. It does not add or materially change an NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGro…
Cmux Source Artifacts	✅ Passed	The PR changes only Sources/TerminalNotificationPolicy.swift and cmuxTests/NotificationAndMenuBarTests.swift. The diff contains hand-written source and a regression test. It adds no logs, screensh…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS. The only production-source change adds POSIX_SPAWN_CLOEXEC_DEFAULT to hook spawning. It adds no #if DEBUG block, test/debug-named member, accessor, or widened visibility. The new test scaffo…
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing notification hooks from inheriting unrelated file descriptors.
Description check	✅ Passed	The description includes the main problem, implementation summary, testing results, limitations, demo status, checklist, and changelog entry. It clearly notes that the full suite, pinned Xcode 26 CI, …

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chapati23]

I have read the CLA Document v2.2 and I hereby sign the CLA

[chapati23]

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

[cubic-dev-ai]

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chapati23 cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 282,776 of the 280,000 allowed lines of code this month. Reviews resume on 1 October 2026 (in 28 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

• Stop reviewing test files
• Turn off incremental reviews
• Use the cubic CLI locally
• Upgrade to a pro plan

Learn more →

[coderabbitai]

@chapati23: I will review the changes in #11649.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

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.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-03T08:19:22.473710Z	59aa759	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 59aa759d6f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[teamleaderleo]

Thanks @chapati23! This is picked up in #14789 with your two commits and the conflict with main's signal-mask reset resolved. Your new isolation test passes there; it'll close out once that merges.

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[teamleaderleo]

Merged, thanks @chapati23! Nice catch on the leaked descriptors, and the /dev/fd probe test is a neat way to pin it.

[github-actions]

Merge receipt for 0a7c9033e7: every check was green at merge (15 verified; 15 skipped by policy). Full suite runs on main after merge.