Restore CLI notification action CI coverage

[lawrencecchen]

Fixes #4524

Summary:

• Replaces the disabled notification action integration test with bundled CLI plus mock socket coverage.
• Keeps assertions for list-notifications extended fields and mark, dismiss, open, and jump notification socket APIs.
• Removes the CI skip for this notification action test.

Testing:

• git diff --check origin/main..HEAD
• ruby -e 'require "yaml"; YAML.load_file(".github/workflows/ci.yml")'\n- xcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -derivedDataPath /tmp/cmux-cli-notify build

---

^{Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.}

---

Note

Low Risk
Test and CI workflow changes only; no production notification or CLI behavior is modified.

Overview
Re-enables CI coverage for notification CLI actions by removing the xcodebuild skip for the old integration test and replacing that test with a lighter harness.

The renamed testNotificationCLIActionsUseSocketAPIAndParseExtendedFields no longer spins up AppDelegate, TabManager, TerminalController, or real window state. It runs the bundled CLI against a mock Unix socket (same pattern as other notify regressions), asserts list-notifications extended JSON fields, and checks mark/dismiss/open/jump commands emit the expected v2 methods (notification.mark_read, notification.dismiss, etc.) via recorded socket traffic and CLI stdout—not by re-listing from TerminalNotificationStore.

Trade-off: the new test validates CLI ↔ socket contract and parsing; it drops end-to-end checks that in-app store/list state changed after each action.

^{Reviewed by Cursor Bugbot for commit 2676824. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Restores CI coverage for CLI notification actions by replacing the integration test with a bundled-CLI + mock Unix socket server. Confirms parsing of extended list fields and correct v2 socket calls for mark, dismiss, open, and jump, including call order.

• Refactors
  • Replaced app-driven harness with a lightweight mock socket server; validates request methods, params, and JSON responses.
  • Added assertions for extended list fields: workspace_id, surface_id, created_at, tab_title; includes scoped mark-read via --workspace/--surface.
  • Removed the CI skip for this test in .github/workflows/ci.yml.

^{Written for commit f293201. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

Release Notes

• Tests
  • Expanded CLI notification command testing to validate routing and output for list, mark-read, dismiss, open, and jump-to-unread operations.
  • Enhanced validation of notification metadata (workspace, surface, creation time, computed tab title) and read/dismiss/open/jump results.
  • CI now runs the previously excluded notification regression test to improve coverage and reliability.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 27, 2026 10:23pm
cmux-staging	Building	Preview, Comment	May 27, 2026 10:23pm

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces an async in-process notification CLI integration test with a synchronous mocked unix-socket test that validates CLI subcommands route to socket methods and parse extended notification fields, and re-enables the test in CI by removing it from xcodebuild's skip list.

Changes

Notification CLI Integration Test Refactor

Layer / File(s)	Summary
Mocked socket notification CLI test cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift	Adds testNotificationCLIActionsUseSocketAPIAndParseExtendedFields, which starts a unix-socket mock server per CLI invocation and verifies list-notifications, mark-notification-read (including --workspace/--surface scoping), dismiss-notification, open-notification, and jump-to-unread route to the expected socket methods and that --json output exposes id, workspace_id, surface_id, created_at, tab_title, and result flags; also asserts exact ordered socket call sequence.
Re-enable notification CLI test in CI .github/workflows/ci.yml	Deletes one CLINotifyProcessIntegrationRegressionTests entry from the xcodebuild -skip-testing list so the new mocked-socket test runs in the unit-test job.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• manaflow-ai/cmux#4874: Also modifies the CI workflow's xcodebuild -skip-testing list to change which XCTest cases are excluded.
• manaflow-ai/cmux#4885: Similar edits to .github/workflows/ci.yml altering which CLINotifyProcessIntegrationRegressionTests cases are skipped.
• manaflow-ai/cmux#4362: Related CI changes to the macOS xcodebuild invocation and job parameters.

Poem

🐰 I mocked a socket, quiet and neat,
CLI commands hopped in a row to meet,
UUIDs gleamed, timestamps in sight,
Tests now deterministic, tidy, and right,
CI wakes — the rabbit does a happy beat.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Cmux Swift Concurrency	❌ Error	CLI/cmux.swift introduces 9 new DispatchQueue.global patterns for ordinary async work (I/O, process monitoring) that could use async/await.	Replace DispatchQueue.global async blocks with async/await, Task, or TaskGroup for I/O and background operations.
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 (16 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR successfully addresses the linked issue #4524 by removing the CI skip for testNotificationCLIActionsMutateSocketStateAndListExtendedFields and replacing it with a narrower, deterministic mock-socket-based test that validates CLI-socket contract without false-green risks.
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped to the linked issue objectives: removing a CI skip and refactoring the test to use mock sockets instead of app-host integration, with no unrelated modifications.
Cmux Swift Actor Isolation	✅ Passed	No production Swift code modified—changes limited to test file refactoring and CI workflow update. Per rules, test code is exempted from actor isolation checks.
Cmux Swift Blocking Runtime	✅ Passed	Blocking constructs (DispatchSemaphore, XCTestExpectation.wait) are in cmuxTests directory test code used for deterministic test scaffolding, which the rule explicitly allows.
Cmux No Hacky Sleeps	✅ Passed	PR changes are Swift tests and CI YAML, both out of scope. Rule covers production TypeScript/JavaScript/shell scripts; this PR has no such changes with hacky sleeps.
Cmux Algorithmic Complexity	✅ Passed	PR changes are limited to CI config and test code; no production algorithmic complexity issues introduced. Per review rules, test code passes algorithmic complexity checks.
Cmux Swift @Concurrent	✅ Passed	PR adds test-only Swift code with synchronous test functions and DispatchQueue operations—no @concurrent violations, no nonisolated async missing annotations, no network-heavy async without hops.
Cmux Swift File And Package Boundaries	✅ Passed	Only test file modified (cmuxTests/) with balanced 158+/163- lines in existing test fixture. No production Swift code changed, satisfies allowed test fixture exception.
Cmux Swift Logging	✅ Passed	No Swift logging violations found. The PR modifies only a test file and CI configuration with no added print/debugPrint/dump/NSLog calls; test files are explicitly allowed per swift-logging.md.
Cmux User-Facing Error Privacy	✅ Passed	PR contains only test and CI changes; no production code modifications. Per review rules, tests and CI workflows are allowed cases, and no user-facing error messages expose sensitive data.
Cmux Full Internationalization	✅ Passed	PR changes only test files and CI configuration. Per internationalization policy, tests and developer-only/operational docs are explicitly allowed and exempt from localization requirements.
Cmux Swiftui State Layout	✅ Passed	PR contains no SwiftUI code changes; modifications are limited to CI workflow configuration and unit test refactoring, making the SwiftUI state layout check inapplicable.
Cmux Architecture Rethink	✅ Passed	PR replaces flaky app-host integration test with deterministic mocked-socket CLI test; test-only synchronization in helpers is allowed; no production architecture changes.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR changes test-only fixtures and CI configuration. No user-visible NSWindow, NSPanel, NSWindowController, SwiftUI Window/WindowGroup code is introduced or modified.
Title check	✅ Passed	The title accurately summarizes the main change: restoring CI coverage for CLI notification actions by re-enabling a previously skipped test.
Description check	✅ Passed	The PR description covers the key sections: summary of changes, rationale, and testing approach; however, the template sections are not clearly structured.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-4524-cli-notify

---

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 and usage tips.}

[greptile-apps]

Greptile Summary

This PR re-enables CI coverage for the notification CLI action tests by replacing the heavy app-driven integration test (which required AppDelegate, TabManager, TerminalController, and a real NSWindow) with a lightweight mock Unix socket harness that follows the established pattern in CLINotifyProcessTestSupport.

• CI skip removed: The -skip-testing entry for the old testNotificationCLIActionsMutateSocketStateAndListExtendedFields is dropped; the renamed test runs unconditionally.
• Test rewritten as synchronous throws: Each of the seven socket commands (list-notifications ×2, mark-read by id, dismiss, mark-read scoped, open-notification, jump-to-unread) is driven through startMockServer + runProcess + wait(for:) in sequence, with the final state.snapshot() asserting the full ordered method list.
• Extended field coverage: workspace_id, surface_id, created_at, and percent-decoded tab_title from list-notifications are all asserted against the mock server response.

Confidence Score: 5/5

Test and CI workflow changes only; no production notification or CLI behaviour is modified.

Both changed files are test infrastructure. The mock-socket pattern is already well-established in the suite, the helpers in CLINotifyProcessTestSupport are shared and correct, and the sequential run/wait approach is sound. The only gap is a missing openPayload["id"] assertion, which is a minor test-coverage nit with no impact on production code or CI reliability.

No files require special attention.

Important Files Changed

Filename	Overview
.github/workflows/ci.yml	Removes the xcodebuild -skip-testing entry for the old testNotificationCLIActionsMutateSocketStateAndListExtendedFields test, which has been replaced by the new mock-socket test. The remaining skip entries are untouched. Change is correct.
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift	Replaces the @MainActor async integration test (which required AppDelegate, TabManager, TerminalController, and real window state) with a synchronous mock-socket test following the established CLINotifyProcessTestSupport pattern. Validates CLI ↔ socket method names and params for list, mark-read, dismiss, open, and jump. Minor gap: openPayload["id"] is not asserted, unlike the parallel jumpPayload["id"] check.

Sequence Diagram

sequenceDiagram
    participant T as Test (main thread)
    participant S as Mock Unix Socket Server (background)
    participant C as Bundled CLI Process

    T->>S: startMockServer(listenerFD, handler)
    T->>C: runProcess(["list-notifications", ...])
    C->>S: connect + send "list_notifications\n"
    S-->>C: notification row response
    C-->>T: stdout (JSON rows) + exit 0
    T->>T: wait(for: serverHandled)
    T->>T: assert extended fields (workspace_id, surface_id, created_at, tab_title)

    Note over T,S: Pattern repeats for mark-read, dismiss, open, jump-to-unread

    T->>T: state.snapshot() → assert ordered method list

Loading

_{Reviews (2): Last reviewed commit: "Restore CLI notification action CI cover..." | Re-trigger Greptile}