Skip to content

fix: exclude unowned same-TTY processes from memory attribution - #16559

Merged
teamleaderleo merged 2 commits into
mainfrom
fix/11004-detached-tty-current-main
Oct 2, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
fix/11004-detached-tty-current-main

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

A detached process can retain a terminal's controlling TTY after being reparented to PID 1. v2AnnotateTopSurface treated every same-TTY PID as a root, so an unrelated REPL or daemon inflated cmux and workspace memory totals.

This PR requires positive ownership evidence before charging a TTY process to a surface. It preserves explicit cmux-scoped helpers, descendants, process-group members, and shells launched by cmux-owned app processes. Same-TTY collisions are reported in unattributed_tty_process_pids and unattributed_resources.

The regression fixture covers a detached PPID-1 process, a foreign off-TTY parent, a reparented scoped helper, and a background process group. Existing launchd-parented WebKit attribution remains covered by CmuxTopSnapshotScopeTests.

Commits are intentionally ordered so the first commit adds the failing regression test and the second commit adds the fix. Based on the earlier outside-contributor implementation in #11869, rebased onto current main and tightened around cmux launch evidence.

Validation completed:

  • ./scripts/sync-test-wiring --check
  • python3 scripts/check-package-resolved-policy.py
  • python3 scripts/check-workspace-package-groups.py --check
  • git diff --check and conflict-marker scan

Fixes #11004

— Cattail g1 🔸


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes memory attribution so detached processes that keep a surface's TTY after being reparented to PID 1 no longer inflate cmux and workspace totals. Previously every same-TTY process was treated as a root; now attribution requires positive ownership evidence via cmux process scope, the cmux-scoped process tree, a descendant, or a process-group relationship. Same-TTY collisions are reported separately in unattributed_tty_process_pids and unattributed_resources, while explicit cmux-scoped helpers and launchd-parented WebKit processes stay attributed. Adds regression coverage for detached, foreign-parent, reparented-scoped, and background-process-group cases. Fixes #11004.

Written for commit 824b293. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits October 1, 2026 17:10
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Require cmux process-tree, scope, descendant, or process-group evidence before charging TTY processes to a surface. Report collisions separately while preserving reparented scoped helpers and WebKit roots.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: db148bda-4cac-4c10-b6d8-2bee93493052

📥 Commits

Reviewing files that changed from the base of the PR and between 8b8762a and 824b293.

📒 Files selected for processing (5)
  • Sources/CmuxTopSnapshot.swift
  • Sources/CmuxTopTTYOwnership.swift
  • Sources/TerminalControllerTopSupport.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/CmuxTopTTYCollisionAttributionTests.swift
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

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

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

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

@teamleaderleo teamleaderleo added the full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. label Oct 2, 2026

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 5 files

You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Sources/TerminalControllerTopSupport.swift
Comment thread Sources/CmuxTopTTYOwnership.swift
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on 824b2932d7 (run 36945571611 attempt 2): 7 code.

Job Verdict Why
macos / CLI product tests code a test failed
macos / app-host unit tests (4/7) code a test failed
macos / app-host unit tests (5/7) code a test failed
macos / app-host unit tests (6/7) code a test failed
macos / app-host unit tests (7/7) code a test failed
macos / app-host unit tests (1/7) code a test failed
macos / app-host unit tests (2/7) code a test failed
Matched log lines
macos / CLI product tests: ✘ Test inboxListsMessagesAndCanMarkThemRead() recorded an issue at CLIAgentMessageCommandTests.swift:117:9: Expectation failed: (readParams["ids"] as? [String] → ["abcdef0123456789", "fedcba9876543210"]) == ["abcdef0123456789"]
macos / app-host unit tests (4/7): /tmp/cmux-ci/src/cmuxTests/WorkspaceUnitTests.swift:5019: error: -[cmuxTests.WorkspaceSplitWorkingDirectoryTests testNewTerminalSplitFallsBackToRequestedWorkingDirectoryWhenReportedDirectoryIsStale] : XCTAssertEqual failed: ("Optional("/tmp")") is not equal to ("Optional("/tmp/cmux-requested-split-c
macos / app-host unit tests (5/7): /tmp/cmux-ci/src/cmuxTests/WorkspaceUnitTests.swift:7544: error: -[cmuxTests.WorkspacePanelGitBranchTests testSidebarBranchDirectoryEntriesStayStableAcrossFocusedSplitChanges] : XCTAssertEqual failed: ("[Optional("/repo/left/live"), Optional("/"), Optional("/repo/right/requested")]") is not equal to
macos / app-host unit tests (6/7): ✘ Test zeroWaitCodexPermissionSurfacesTransientNeedsInputAttention() recorded an issue at FeedCoordinatorTests.swift:671:9: Expectation failed: (attention.events.count → 0) == 1
macos / app-host unit tests (7/7): ✘ Test failedDifferentIdentityCleanupOnSameRelayPreventsReplacementStartup() recorded an issue at RemoteSessionCleanupLifecycleTests.swift:226:9: Expectation failed: (runner.nonCleanupRequestCount → 2) == (requestsBeforeReplacement → 1)
macos / app-host unit tests (1/7): /tmp/cmux-ci/src/cmuxTests/WorkspaceSplitStartupCommandTests.swift:279: error: -[cmuxTests.WorkspaceSplitStartupCommandTests testRespawnTerminalSurfacePreservesPaneTabAndSurfaceIdentity] : XCTAssertEqual failed: ("Optional("/tmp")") is not equal to ("Optional("/tmp/cmux-respawn-31CAC6FA-F2C7-493E-A2
macos / app-host unit tests (2/7): ✘ Test "A narrow machine row keeps the generated name's ending" recorded an issue at CloudTreeMachineResourcesTests.swift:86:25: Expectation failed: Self.descendants(of: host).compactMap { $0 as? NSTextField }

Not re-run automatically: macos / CLI product tests, macos / app-host unit tests (4/7), macos / app-host unit tests (5/7), macos / app-host unit tests (6/7), macos / app-host unit tests (7/7) are not machine failures.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@teamleaderleo
teamleaderleo merged commit abdf121 into main Oct 2, 2026
189 of 217 checks passed
@teamleaderleo
teamleaderleo deleted the fix/11004-detached-tty-current-main branch October 2, 2026 02:08
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for 824b2932d7, merged 2026-10-02 02:08:57 UTC

  • Not verified at merge: app-host unit tests (8) (in progress), CLI product tests (in progress)
  • Verified: ci-status, macOS compile admission, CI fast guards, CI timing, Claude wrapper regressions, Fast static checks, full-suite-coverage, GhosttyKit release check, guards (18), late-placement, linux-preflight, macOS admission gate, and 8 more
  • Skipped by policy: admission-placement, browser, Claude request, Dogfood build #​${{ github.event.pull_request.number }}, remote-daemon, suite-coverage, ui-tests, web, web-build, web-database-tests, web-tests
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 2, 2026
rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 2, 2026
abdf121 fix: exclude unowned same-TTY processes from memory attribution (manaflow-ai#16559)
075dbef test: fix remote paste test failures on main from manaflow-ai#16523 (manaflow-ai#16596)
7d7a9d1 Stop US key positions from hijacking shortcuts on non-US layouts (manaflow-ai#16237)
134c9d9 Let AppKit cycle windows with the System Settings shortcut on ISO keyboards (manaflow-ai#16238)
d0dd457 iOS: prevent toolbar flash when switching primary tabs (manaflow-ai#15712)
6c4b727 test(cloud): re-enable the Cloud header width tests by measuring each row (manaflow-ai#16590)
51b60f3 fix(remote): keep reconnect cleanup fixture process-free (manaflow-ai#16586)
6090053 fix(ci): reserve only queued release slots (manaflow-ai#16588)
0440a5d fix: make browser import hint cover all supported browsers (manaflow-ai#16483)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory attribution includes detached PPID-1 processes that merely share a cmux TTY

1 participant