Skip to content

Harden long-uptime sidebar observation lifecycle (#7929) - #7954

Merged
austinywang merged 4 commits into
mainfrom
issue-7929-uptime-cpu-peg
Jul 13, 2026
Merged

austinywang merged 4 commits into
mainfrom
issue-7929-uptime-cpu-peg

Conversation

@austinywang

@austinywang austinywang commented Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Root Cause And Scope

Both current-main sidebar observation models register continuations synchronously but remove them through a separate Task { @MainActor ... } from onTermination. Under main-actor saturation, terminated continuations remain in the registry and every later publication revisits them, increasing per-event work while cleanup falls further behind.

Publication is now the authoritative reconciliation point: it checks each yield result, removes every .terminated entry after iteration, and preserves the existing unobserved-title replay semantics when no live observer accepted the change. onTermination remains the prompt cleanup path when no later publication occurs. Registries expose internal read access with private setters so tests can inspect membership via @testable while each model remains the sole mutation owner.

This lifecycle hazard was introduced in the post-0.64.17 replacement observation models, so this PR does not claim that it caused the original 0.64.17 incident. It prevents the replacement path from developing the same positive-feedback accumulation class.

Testing

Demo Video

Localization Audit

No user-facing strings changed. The touched Swift files contain only internal observer bookkeeping, comments, and test failure text; Resources/Localizable.xcstrings and web locale catalogs are unaffected.

Review Trigger (Copy/Paste as PR comment)

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

Checklist

  • I tested the change locally — prohibited for this incident; focused GitHub macOS runs are used instead.
  • I added or updated tests for behavior changes.
  • I updated docs/changelog if needed — not needed for this internal lifecycle fix.
  • I requested bot reviews after my latest commit.
  • All code review bot comments are resolved.
  • All human review comments are resolved — none received.

🤖 Generated with Claude Code (via cmux HQ dispatch).

@vercel

vercel Bot commented Jul 13, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jul 13, 2026 1:50am
cmux-staging Building Building Preview, Comment Jul 13, 2026 1:50am

@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

AsyncStream publication now detects terminated continuations and removes them from both sidebar observation models. Tests cover pruning 2,000 terminated observers during runtime and settled process-title publications.

Changes

Observer reconciliation

Layer / File(s) Summary
Publication-time observer pruning
Sources/WorkspaceSidebarAgentRuntimeObservationModel.swift, Sources/WorkspaceSidebarProcessTitleObservationModel.swift
Observer dictionaries allow external reads while retaining internal mutation control; terminated continuations are collected during publication and removed afterward.
Burst pruning tests
cmuxTests/WorkspaceSidebarProcessTitleObservationTests.swift
Tests verify that runtime and settled process-title publications prune 2,000 terminated observers.

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

Possibly related issues

  • manaflow-ai/cmux-dev-artifacts#3839: Updates the same process-title observation test suite and adds related observer-pruning coverage.

Possibly related PRs

  • manaflow-ai/cmux#7754: Modifies related process-title AsyncStream continuation and subscriber termination handling.

Suggested reviewers: lawrencecchen


Important

Pre-merge checks failed

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

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Cmux Swift File And Package Boundaries ❌ Error The fix keeps pure, testable sidebar observation logic in app-target Sources/ instead of the existing SwiftPM sidebar boundary. Move both WorkspaceSidebar*ObservationModel types into Packages/macOS/CmuxSidebar (or a tiny sibling package) and leave only app wiring in Sources/WorkspaceSidebarObservation.swift.
✅ Passed checks (24 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 Swift Actor Isolation ✅ Passed PASS: Both observation models are already @MainActor UI stores; the PR only makes changeObservers read-only and adds @MainActor tests, with no new cross-actor access.
Cmux Swift Blocking Runtime ✅ Passed PASS: The diff only prunes terminated AsyncStream observers and adds deterministic tests; no semaphores, sleeps, sync waits, or locks were introduced.
Cmux Browser Automation Off-Main ✅ Passed PR only changes two @MainActor sidebar observation models; no browser.* commands, worker routing, WebKit/AppKit hops, or browser automation tests were touched.
Cmux Expensive Synchronous Load ✅ Passed Diff only prunes AsyncStream observers in two @MainActor sidebar models; no RestorableAgentSessionIndex.load() or history/transcript parsing was added.
Cmux Cache Substitution Correctness ✅ Passed The diff only hardens AsyncStream observer pruning in sidebar publish paths; it does not replace any authoritative read with cached data in persistence/history/snapshot code.
Cmux No Hacky Sleeps ✅ Passed PASS: The PR only touches Swift observation/test files; no TypeScript, JS, shell, or build/runtime scripts add fixed sleeps or polling waits.
Cmux Algorithmic Complexity ✅ Passed Single-pass pruning over changeObservers in both publish paths; no nested rescans or repeated filtering, and the new 2k-observer tests are fixed-size scaffolding.
Cmux Swift Concurrency ✅ Passed Diff only prunes terminated AsyncStream observers and adds tests; no new background queues, Combine app state, completion handlers, or new fire-and-forget Tasks were introduced.
Cmux Swift @Concurrent ✅ Passed No new @concurrent or nonisolated async work was introduced; the changes stay @MainActor and use explicit MainActor hops for cleanup.
Cmux Swiftpm Lockfiles ✅ Passed PR only changes source and test files; no Package.swift, Package.resolved, .gitignore, Xcode, or workflow/dependency files are touched.
Cmux Swift Logging ✅ Passed Diff only changes observer access and pruning logic; no print/debugPrint/dump/NSLog, Logger, or stdout/stderr logging was added or changed.
Cmux User-Facing Error Privacy ✅ Passed The diff only changes internal observer cleanup and test-only assertions; no user-facing errors, alerts, or API error text was added.
Cmux Full Internationalization ✅ Passed Diff only changes observer cleanup/access control in Swift models; no user-facing strings, catalogs, or locale files were added or modified.
Cmux Swiftui State Layout ✅ Passed Only observation-model files changed; no new ObservableObject/@published, GeometryReader, lazy-row store refs, or render-time state writes were introduced.
Cmux Architecture Rethink ✅ Passed PASS: The PR keeps one clear owner (each model’s observer registry) and adds pruning in existing publish paths; no sleeps, polling, locks, or split lifecycle ownership introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only changes observer pruning and tests; no NSWindow/NSPanel/WindowGroup or cmuxAuxiliaryWindowIdentifiers code was added or modified.
Cmux Source Artifacts ✅ Passed Only hand-written source and test files changed; no logs, caches, build output, temp dirs, or other source-control artifacts appear in the PR diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS — the PR only widens changeObservers from private to private(set) and the tests read it via @testable import; no new debug/test hook is added in Sources.
Cmux No Ambient Global State ✅ Passed The diff only changes instance members and cleanup logic inside existing classes; it adds no file-scope funcs/vars, namespaces, or singletons.
Title check ✅ Passed The title is concise and accurately summarizes the main change: pruning long-lived sidebar observation continuations.
Description check ✅ Passed The description matches the template with Summary, Testing, Demo Video, Review Trigger, and Checklist sections, and it clearly explains the change and verification.
✨ 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-7929-uptime-cpu-peg

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.

@austinywang
austinywang marked this pull request as ready for review July 13, 2026 01:05
@greptile-apps

greptile-apps Bot commented Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the sidebar observation lifecycle during long-running sessions. The main changes are:

  • Prune terminated runtime observers during publication.
  • Prune terminated process-title observers after settled changes.
  • Add burst tests covering cleanup of 2,000 terminated observers.
  • Expose observer registries as read-only internal state for tests.

Confidence Score: 5/5

This looks safe to merge.

  • Terminated observers are removed without changing live-observer delivery.
  • Main-actor isolation keeps registration, publication, and cleanup serialized.
  • The process-title path preserves pending changes when no live observer receives an event.
  • No blocking issue was found in the updated code.

Important Files Changed

Filename Overview
Sources/WorkspaceSidebarAgentRuntimeObservationModel.swift Removes terminated runtime observers during publication and keeps registry mutation private to the model.
Sources/WorkspaceSidebarProcessTitleObservationModel.swift Removes terminated title observers while preserving undelivered-change handling.
cmuxTests/WorkspaceSidebarProcessTitleObservationTests.swift Adds burst tests for runtime and process-title observer reconciliation.

Reviews (2): Last reviewed commit: "Keep sidebar observer registries model-o..." | Re-trigger Greptile

@coderabbitai coderabbitai 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.

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 `@Sources/WorkspaceSidebarAgentRuntimeObservationModel.swift`:
- Line 23: Change the changeObservers property declaration to use private(set),
preserving read access for tests while restricting registry mutations to the
owning model and its reconciliation logic.

In `@Sources/WorkspaceSidebarProcessTitleObservationModel.swift`:
- Line 26: Update the changeObservers property declaration in
WorkspaceSidebarProcessTitleObservationModel to use a private setter while
keeping its getter accessible, so tests can inspect the registry but only the
model can mutate observer membership.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f5e86a13-35fd-4ac1-b0c2-ffdedfd66423

📥 Commits

Reviewing files that changed from the base of the PR and between c85ed9b and d26841d.

📒 Files selected for processing (3)
  • Sources/WorkspaceSidebarAgentRuntimeObservationModel.swift
  • Sources/WorkspaceSidebarProcessTitleObservationModel.swift
  • cmuxTests/WorkspaceSidebarProcessTitleObservationTests.swift

Comment thread Sources/WorkspaceSidebarAgentRuntimeObservationModel.swift Outdated
Comment thread Sources/WorkspaceSidebarProcessTitleObservationModel.swift Outdated

This branch was successfully deployed

1 active deployment
Preview – cmux — 1d5f4434 Deployed Jul 13, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant