Lock screen first pass: click-swallow fix and locked-state skeleton #3 · notkainoa/not.boring.notch

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 40dae153-d83a-4c06-99e2-86ecac906fd1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Ccubic-dev-aiBotreviewed · 3 inline threads7d ago

3 issues found across 12 files

Prompt for AI agents (unresolved issues)

text

1
2	Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
3
4
5	<file name="docs/lock-screen.md">
6
7	<violation number="1" location="docs/lock-screen.md:50">
8	P3: This section is stale after the fix: it says the current unlock path leaves windows out of the CGS space and never replaces them, but the implementation now reasserts membership and replaces delegated windows. Mark lines 50–54 as the pre-pass behavior or rewrite them to describe the current path.</violation>
9	</file>
10
11	<file name="boringNotch/BoringViewCoordinator.swift">
12
13	<violation number="1" location="boringNotch/BoringViewCoordinator.swift:73">
14	P2: `setScreenLocked` clears only music sneak peeks, so a battery peek already visible when the screen locks can bypass the battery lock-screen policy. Clear every shown sneak peek whose mapped feature is hidden.</violation>
15	</file>
16
17	<file name="boringNotch/managers/NotchWindowManager.swift">
18
19	<violation number="1" location="boringNotch/managers/NotchWindowManager.swift:65">
20	P2: Lock teardown is not persistent: later display or settings callbacks can recreate global drag monitors because `setupDragDetectors()` has no locked-state guard. Return after cleanup when `isScreenLocked` is true.</violation>
21	</file>

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

View guided diff | Turn on auto-fix | Re-trigger cubic}

boringNotch/managers/NotchWindowManager.swiftline 65Open thread

…	…	@@ -53,48 +55,151 @@ final class NotchWindowManager {
	62	+	// Nothing opens over the lock screen (BoringViewModel.open refuses),
	63	+	// so close what's open and stop all pointer and drag input.
	64	+	([primaryViewModel] + contexts.values.map(\.viewModel)).forEach { $0.close() }
	65	+	cleanupDragDetectors()

Ccubic-dev-aiBoton line 657d ago

P2: Lock teardown is not persistent: later display or settings callbacks can recreate global drag monitors because setupDragDetectors() has no locked-state guard. Return after cleanup when isScreenLocked is true.

Prompt for AI agents

text

1	Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At boringNotch/managers/NotchWindowManager.swift, line 65:
2
3	<comment>Lock teardown is not persistent: later display or settings callbacks can recreate global drag monitors because `setupDragDetectors()` has no locked-state guard. Return after cleanup when `isScreenLocked` is true.</comment>
4
5	<file context>
6	@@ -53,48 +55,151 @@ final class NotchWindowManager {
7	+ // Nothing opens over the lock screen (BoringViewModel.open refuses),
8	+ // so close what's open and stop all pointer and drag input.
9	+ ([primaryViewModel] + contexts.values.map(\.viewModel)).forEach { $0.close() }
10	+ cleanupDragDetectors()
11	+
12	if !Defaults[.showOnLockScreen] {
13	</file context>

docs/lock-screen.mdline 50Open thread

…	…	@@ -0,0 +1,140 @@
	47	+
	48	+	### Does our code have it?
	49	+
	50	+	Very likely yes, though it can't be confirmed without locking a real machine. Our lock path is byte-for-byte the code #1346 was written against:

Ccubic-dev-aiBoton line 507d ago

P3: This section is stale after the fix: it says the current unlock path leaves windows out of the CGS space and never replaces them, but the implementation now reasserts membership and replaces delegated windows. Mark lines 50–54 as the pre-pass behavior or rewrite them to describe the current path.

Prompt for AI agents

text

1	Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/lock-screen.md, line 50:
2
3	<comment>This section is stale after the fix: it says the current unlock path leaves windows out of the CGS space and never replaces them, but the implementation now reasserts membership and replaces delegated windows. Mark lines 50–54 as the pre-pass behavior or rewrite them to describe the current path.</comment>
4
5	<file context>
6	@@ -0,0 +1,140 @@
7	+
8	+### Does our code have it?
9	+
10	+Very likely yes, though it can't be confirmed without locking a real machine. Our lock path is byte-for-byte the code #1346 was written against:
11	+
12	+- The same `delegateWindow` (flag 7) and the same `undelegateWindow`.
13	</file context>

boringNotch/BoringViewCoordinator.swiftlines 73–77Open thread

…	…	@@ -49,6 +49,34 @@ final class BoringViewCoordinator: ObservableObject {
	74	+	for (uuid, state) in sneakPeekStates where state.show && state.type == .music {
	75	+	sneakPeekStates[uuid]?.show = false
	76	+	}
	77	+	}

Ccubic-dev-aiBoton lines 73–777d ago

P2: setScreenLocked clears only music sneak peeks, so a battery peek already visible when the screen locks can bypass the battery lock-screen policy. Clear every shown sneak peek whose mapped feature is hidden.

Prompt for AI agents

text

1	Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At boringNotch/BoringViewCoordinator.swift, line 73:
2
3	<comment>`setScreenLocked` clears only music sneak peeks, so a battery peek already visible when the screen locks can bypass the battery lock-screen policy. Clear every shown sneak peek whose mapped feature is hidden.</comment>
4
5	<file context>
6	@@ -49,6 +49,34 @@ final class BoringViewCoordinator: ObservableObject {
7	+ if expandingView.show, let feature = LockScreenFeature(expandingView.type), !isVisible(feature) {
8	+ expandingView.show = false
9	+ }
10	+ if !isVisible(.music) {
11	+ for (uuid, state) in sneakPeekStates where state.show && state.type == .music {
12	+ sneakPeekStates[uuid]?.show = false
13	</file context>

suggestion

1	for (uuid, state) in sneakPeekStates {
2	guard state.show,
3	let feature = LockScreenFeature(state.type),
4	!isVisible(feature) else { continue }
5	sneakPeekStates[uuid]?.show = false
6	}

Lock screen first pass: click-swallow fix and locked-state skeleton#3

Changes

⚠️ Needs rebasing onto compact-only (#1)

Verification

Summary by cubic

Checks

Review skipped