Admit focused untracked standard windows before non-managed fallback

[biswadip-paul]

Summary

• Fixes a race condition where a focused standard window can be treated as non-managed if the app activation event arrives before OmniWM has tracked the window in WorkspaceManager
• Adds admitFocusedUntrackedWindowIfNeeded to the activation path — checks if the focused AX window is a valid managed candidate and admits it before falling through to the non-managed focus fallback
• Respects the existing managed-replacement delay path (shouldDelayManagedReplacementCreate)

Fixes #387

Test plan

• Added regression test focusedUntrackedStandardWindowIsAdmittedBeforeNonManagedFallback that verifies:
  • Untracked Chrome window is admitted as tiling
  • Window becomes the focused managed target
  • AX window subscriptions are created
  • .axWindowCreated relayout is triggered
  • Non-managed focus is NOT entered

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved handling so focused, previously untracked windows are admitted into managed tiling state instead of falling back to non-managed focus.
• Tests
  • Added a test verifying focused untracked windows are admitted, become the managed focus target, and trigger expected layout and subscription behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca2cd2a4-693d-4c07-826a-945d611dfc5a

📥 Commits

Reviewing files that changed from the base of the PR and between 9b329ed and 8e71b75.

📒 Files selected for processing (1)

• Tests/OmniWMTests/AXEventHandlerTests.swift

🚧 Files skipped from review as they are similar to previous changes (1)

• Tests/OmniWMTests/AXEventHandlerTests.swift

---

📝 Walkthrough

Walkthrough

AXEventHandler now attempts to admit a focused untracked standard window into the managed model before falling back to non-managed focus handling; a new helper performs admission or defers via delayed replacement, and a regression test verifies the admission and resulting focus/subscription behavior.

Changes

Window Admission Before Non-Managed Fallback

Layer / File(s)	Summary
Window admission logic Sources/OmniWM/Core/Controller/AXEventHandler.swift	handleAppActivation gains an early branch that attempts to admit a previously untracked focused window into managed state before the non-managed fallback. Adds admitFocusedUntrackedWindowIfNeeded(token:windowId:) which resolves window info, prepares a managed create candidate, enqueues delayed replacement creation when applicable, or tracks the prepared create and returns the managed entry (or nil when deferred).
Admission test coverage Tests/OmniWMTests/AXEventHandlerTests.swift	New test focusedUntrackedStandardWindowIsAdmittedBeforeNonManagedFallback simulates focusing an untracked Chrome standard window and verifies it is admitted as .tiling, becomes the focused managed target, receives AX subscriptions, triggers relayout with .axWindowCreated, and does not enter the non-managed fallback path.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• BarutSRB/OmniWM#251: Both PRs add an admission/rehydration attempt in AXEventHandler.handleAppActivation when no managed entry exists before falling back to unmanaged focus handling.

Poem

🐰
A hop, a peek, a focused window found,
Admitted to the stack before it hits the ground.
Subscriptions set, the layout sings anew,
Tracked and tidy — that's the hopping view!

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately and concisely describes the main change: admitting focused untracked standard windows before falling back to non-managed handling.
Linked Issues check	✅ Passed	All coding requirements from issue #387 are met: untracked focused windows are admitted before non-managed fallback, managed-replacement delay is respected, and the regression test verifies admission, focus handling, subscriptions, and avoidance of non-managed fallback.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #387: the new admitFocusedUntrackedWindowIfNeeded helper, integration into handleAppActivation, and the regression test are all necessary to implement the fix.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

Tests/OmniWMTests/AXEventHandlerTests.swift (1)

1783-1803: ⚡ Quick win

Assert fallback trace is never entered (not just final state).

This test checks final state (isNonManagedFocusActive == false), but it can still miss a transient fallback entry. Add a trace assertion to ensure .nonManagedFallbackEntered never occurred for this activation.

Proposed assertion hardening

         controller.axEventHandler.handleAppActivation(
             pid: getpid(),
             source: .focusedWindowChanged
         )
         await controller.layoutRefreshController.waitForRefreshWorkForTests()
+        let trace = createFocusTraceEvents(on: controller)

@@
         `#expect`(controller.focusBridge.focusedTarget?.token == observedToken)
         `#expect`(controller.focusBridge.focusedTarget?.isManaged == true)
         `#expect`(subscriptions == [[observedWindowId]])
         `#expect`(relayoutReasons.contains(.axWindowCreated))
+        `#expect`(!trace.contains { event in
+            if case let .nonManagedFallbackEntered(pid, source) = event.kind {
+                return pid == getpid() && source == .focusedWindowChanged
+            }
+            return false
+        })

🤖 Prompt for 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.

In `@Tests/OmniWMTests/AXEventHandlerTests.swift` around lines 1783 - 1803, Add an
assertion after awaiting layout refresh and before the existing guards to ensure
the transient fallback trace was never entered: use the test's expectation style
(e.g. `#expect`) to assert that the trace collection on the controller/workspace
manager does not contain the .nonManagedFallbackEntered event (for example:
`#expect`(!controller.workspaceManager.traces.contains(.nonManagedFallbackEntered))).
Insert this immediately after await
controller.layoutRefreshController.waitForRefreshWorkForTests() and before
checking the admitted entry.

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

Nitpick comments:
In `@Tests/OmniWMTests/AXEventHandlerTests.swift`:
- Around line 1783-1803: Add an assertion after awaiting layout refresh and
before the existing guards to ensure the transient fallback trace was never
entered: use the test's expectation style (e.g. `#expect`) to assert that the
trace collection on the controller/workspace manager does not contain the
.nonManagedFallbackEntered event (for example:
`#expect`(!controller.workspaceManager.traces.contains(.nonManagedFallbackEntered))).
Insert this immediately after await
controller.layoutRefreshController.waitForRefreshWorkForTests() and before
checking the admitted entry.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2abc0c11-a650-4250-80af-de26a4990ff3

📥 Commits

Reviewing files that changed from the base of the PR and between 777f1c7 and 9b329ed.

📒 Files selected for processing (2)

• Sources/OmniWM/Core/Controller/AXEventHandler.swift
• Tests/OmniWMTests/AXEventHandlerTests.swift

[coderabbitai]

Actionable comments posted: 0

[biswadip-paul]

Closing — v0.4.9.6 independently implemented this fix via admitFocusedWindowBeforeNonManagedFallback in AXEventHandler.swift (line 1405). Same approach, different implementation. No longer needed.