Skip to content

Inject FocusLogStore instead of a shared singleton - #5239

Merged
azooz2003-bit merged 2 commits into
mainfrom
feat-focuslogstore-di
Jun 2, 2026
Merged

azooz2003-bit merged 2 commits into
mainfrom
feat-focuslogstore-di

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jun 2, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to the CmuxUpdater extraction (#5132), which de-singletoned UpdateLogStore. FocusLogStore (the DEBUG-only focus-event diagnostic logger that lives in the same file) still self-vended a static let shared.

This applies the same fix: AppDelegate owns the single FocusLogStore instance next to updateLog, so its lifecycle has one composition root. The in-AppDelegate call sites use the instance directly; the GhosttyTerminalView and Workspace sites reach it via AppDelegate.shared?.focusLog, mirroring the existing AppDelegate.shared?.updateLog precedent. The type is now a plain injectable class with a documented @unchecked Sendable (its mutable state stays confined to the serial queue, unchanged).

The second commit compile-time guards the FOCUSDBG NSLog in GhosttyNSView.focusLog behind #if DEBUG (it was runtime-gated only), per the repo debug-log requirement.

No production behavior change. FocusLogStore.append is already #if DEBUG; this only moves ownership.

Testing

  • ./scripts/reload.sh --tag focuslog-di builds green.
  • Verified no test target references the removed FocusLogStore.shared symbol.
  • CI tests-build-and-lag and the CoreAnimation main-thread startup regression pass on the build.
  • No new behavior to exercise at runtime: focus logging (~/Library/Logs/cmux-focus.log, the Copy Focus Logs menu action) works exactly as before, now sourced from the AppDelegate-owned instance.

🤖 Generated with Claude Code

FocusLogStore self-vended a `static let shared`. AppDelegate now owns the single
instance (next to updateLog), the in-AppDelegate diagnostic sites use it directly,
and the GhosttyTerminalView/Workspace sites reach it via AppDelegate.shared?.focusLog,
matching the UpdateLogStore de-singletoning. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 2, 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 Jun 2, 2026 8:34pm
cmux-staging Building Building Preview, Comment Jun 2, 2026 8:34pm

@coderabbitai

coderabbitai Bot commented Jun 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

FocusLogStore is converted from a singleton to an externally-constructible instance; AppDelegate now owns a FocusLogStore instance and all DEBUG focus-log writes (AppDelegate internals, Workspace, GhosttyTerminalView) are redirected to AppDelegate.shared?.focusLog. Clipboard export uses the instance snapshot/logPath.

Changes

Focus Logging Injection Refactoring

Layer / File(s) Summary
FocusLogStore instance-based API
Sources/Update/UpdateLogStore.swift
Removes static let shared singleton and private init(), exposes public init(), and adds @unchecked Sendable conformance to enable external lifecycle composition.
AppDelegate focusLog ownership and adoption
Sources/AppDelegate.swift
Adds focusLog instance property; updates internal DEBUG logging call sites to use focusLog.append(...) and changes copyFocusLogs to use focusLog.snapshot() / focusLog.logPath().
Logging call-site updates in terminal and workspace
Sources/GhosttyTerminalView.swift, Sources/Workspace.swift
Redirects DEBUG focus logging from FocusLogStore.shared.append() to AppDelegate.shared?.focusLog.append() using optional chaining at workspace and terminal call sites.

Sequence Diagram

sequenceDiagram
    participant AppDelegate
    participant FocusLogStore
    participant Workspace
    participant GhosttyNSView

    AppDelegate->>FocusLogStore: init() on app startup
    Workspace->>AppDelegate: AppDelegate.shared?.focusLog.append(message)
    AppDelegate->>FocusLogStore: append(message)
    GhosttyNSView->>AppDelegate: AppDelegate.shared?.focusLog.append(message)
    AppDelegate->>FocusLogStore: append(message)
    AppDelegate->>FocusLogStore: snapshot() + logPath() for clipboard
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I was once a global, shouting logs at will—
Now AppDelegate tends me, steady and still.
Messages hop through optional doors,
Snapped to clipboard and saved in drawers.
A rabbit cheers composition, soft and trill 🐇✨

🚥 Pre-merge checks | ✅ 17 | ❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Inject FocusLogStore instead of a shared singleton' accurately and concisely summarizes the main architectural change in the pull request—replacing a singleton pattern with dependency injection.
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 FocusLogStore marked @unchecked Sendable with serial queue isolation; AppDelegate @MainActor owns instance; all call sites appropriately access from MainActor/MainThread contexts.
Cmux Swift Blocking Runtime ✅ Passed PR refactors FocusLogStore from singleton to instance injection without introducing blocking operations. Existing queue.sync is only called from UI menu action.
Cmux No Hacky Sleeps ✅ Passed Check out of scope: PR changes only Swift files, but this check covers TypeScript, JavaScript, shell, and non-Swift build/runtime scripts only.
Cmux Algorithmic Complexity ✅ Passed All focusLog.append() calls are single debug-log events, most #if DEBUG guarded, never in loops. FocusLogStore uses bounded 400-entry buffer; no algorithmic complexity violations detected.
Cmux Swift Concurrency ✅ Passed PR refactors FocusLogStore from singleton to injectable pattern; pre-existing serial DispatchQueue usage is retained unchanged, no new legacy async patterns introduced.
Cmux Swift @Concurrent ✅ Passed PR converts FocusLogStore from singleton to injectable; introduces no async functions, @concurrent annotations, or concurrency violations.
Cmux Swift File And Package Boundaries ✅ Passed Refactoring adds minimal lines to existing oversized files (7, 3, 5, 2 lines) all below 250-line threshold. No new responsibilities, no mixed concerns; appropriate app-target glue code.
Cmux Swift Logging ✅ Passed NSLog in GhosttyTerminalView is properly guarded by #if DEBUG; all focusLog calls are debug-only (#if DEBUG blocks) with no secrets exposed; file logging follows established UpdateLogStore pattern.
Cmux User-Facing Error Privacy ✅ Passed Focus log messages are developer-only debug info guarded by #if DEBUG, stored locally in ~/Library/Logs/, not user-facing. No sensitive information exposed.
Cmux Full Internationalization ✅ Passed PR contains debug-only focus logging refactoring with #if DEBUG guards; no new user-facing text or unlocalized strings added.
Cmux Swiftui State Layout ✅ Passed PR introduces no new SwiftUI observable state. FocusLogStore is a plain logging utility with @unchecked Sendable. Usage in AppKit views and existing ObservableObjects are incidental and approved.
Cmux Architecture Rethink ✅ Passed Converts FocusLogStore from singleton to AppDelegate-injected instance. No prohibited patterns (sleeps, locks, observers, channels). Uses safe optional chaining with proper serial queue thread safety.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR refactors FocusLogStore (logging utility) from singleton to injectable—no NSWindow, NSPanel, NSWindowController, Window, or WindowGroup additions.
Description check ✅ Passed The pull request description provides a clear summary of changes, testing approach, and the rationale for the dependency injection pattern. All required template sections are addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 feat-focuslogstore-di

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

@azooz2003-bit

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@greptile-apps

greptile-apps Bot commented Jun 2, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR de-singletones FocusLogStore to match the existing UpdateLogStore DI pattern: AppDelegate now owns a single focusLog instance, and all call sites in GhosttyTerminalView and Workspace are updated to reach it via AppDelegate.shared?.focusLog. The refactor is clean and consistent with the established pattern.

  • UpdateLogStore.swift: Removes static let shared, opens init(), and adds an @unchecked Sendable conformance with a clear queue-confinement comment.
  • AppDelegate.swift: Adds let focusLog = FocusLogStore() as a stored property; all in-file call sites use it directly.
  • GhosttyTerminalView.swift: Switches to AppDelegate.shared?.focusLog; also wraps the previously bare NSLog(\"[FOCUSDBG]\") in #if DEBUG, which is a correct tightening of production logging.

Confidence Score: 5/5

Safe to merge; this is a clean DI refactor with no behavioral change in debug builds and a negligible side effect in release builds.

All changed code is mechanical: singleton removed, stored property added to AppDelegate, call sites updated. The only noteworthy difference is that FocusLogStore.init() — and therefore ensureLogFile() — now runs unconditionally in release builds, creating an empty log file that was previously never created. This is cosmetic and has no functional impact on users.

Sources/Update/UpdateLogStore.swift — FocusLogStore.init() calls ensureLogFile() unconditionally, creating an empty log file in release builds that did not exist before.

Important Files Changed

Filename Overview
Sources/Update/UpdateLogStore.swift Removes static let shared from FocusLogStore, makes init() public, and adds @unchecked Sendable conformance with a clear documentation comment explaining the queue-confinement invariant.
Sources/AppDelegate.swift Adds let focusLog = FocusLogStore() as a stored property next to updateLog. All in-file call sites updated. Because stored properties initialise unconditionally, FocusLogStore.init() (including ensureLogFile()) now always runs in release builds, which was not the case before this PR.
Sources/GhosttyTerminalView.swift Updates focusLog() to use AppDelegate.shared?.focusLog; also wraps the pre-existing bare NSLog inside #if DEBUG, which is an improvement per the logging rule.
Sources/Workspace.swift Two call sites inside #if DEBUG blocks updated from FocusLogStore.shared to AppDelegate.shared?.focusLog; mechanical change with no behavior difference.

Sequence Diagram

sequenceDiagram
    participant AD as AppDelegate
    participant FLS as FocusLogStore
    participant GTV as GhosttyNSView
    participant WS as Workspace

    Note over AD: App startup
    AD->>FLS: "init() → let focusLog = FocusLogStore()"

    Note over GTV: focusDebugEnabled check
    GTV->>AD: AppDelegate.shared?.focusLog
    AD-->>GTV: FocusLogStore instance
    GTV->>FLS: "append(message) [#if DEBUG only]"

    Note over WS: Inside #if DEBUG block
    WS->>AD: AppDelegate.shared?.focusLog
    AD-->>WS: FocusLogStore instance
    WS->>FLS: append(message)

    Note over AD: copyFocusLogs action
    AD->>FLS: snapshot() / logPath()
    FLS-->>AD: entries / path
Loading

Reviews (2): Last reviewed commit: "Compile-time guard the FOCUSDBG NSLog in..." | Re-trigger Greptile

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

No issues found across 4 files

Re-trigger cubic

CodeRabbit's "Cmux Swift Logging" check flagged the NSLog in
GhosttyNSView.focusLog as runtime-gated only. Wrap it in #if DEBUG so it is
stripped from release builds, matching the repo debug-log requirement.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit 67014e3 into main Jun 2, 2026
21 checks passed
@azooz2003-bit
azooz2003-bit deleted the feat-focuslogstore-di branch June 2, 2026 20:43

This branch was successfully deployed

1 active deployment
Preview – cmux — b1f952c6 Deployed Jun 2, 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