Windows: verify focus before every keystroke + injection lock (fixes latent CRIT) - #2
Open
holbizmetrics wants to merge 2 commits into
Open
Conversation
Hardens the fail-closed pipeline against a latent bug in the merged port:
SwitchDriver re-asserts foreground before each keystroke but IGNORED the
result, so if the target lost foreground mid-shift (user alt-tabbed, or
SetForegroundWindow was denied) it blind-fired /model / Return / Escape /
digits into whatever window then owned focus — Return in a chat app sends
the user's draft. Now every re-assert is checked and a failure aborts with
NO_FOCUS BEFORE the keystroke.
Also, from a four-lens adversarial audit of the port:
- Injection lock: a session-local named mutex serializes shifts so two
clients (CLI + GUI, or two quick pulls) can't interleave keystrokes; a
LOCKED refusal if it can't be taken quickly.
- Reader no longer prefers an OFF-SCREEN agent pane (SendInput reaches only
the active tab, so acting on an off-screen match would type into the wrong
place): prefer an on-screen pane, even a non-agent one -> NO_AGENT refuse.
- TextPattern.GetText(maxLen) truncates from the START; read the full range
and keep the tail so long scrollback doesn't make bottom-anchored checks
read the top of history.
- IsInputEmpty (Claude) EXACT-matches placeholders like the Codex branch: a
real draft that merely BEGINS with a placeholder ("Ask Claude about ...")
is a draft, not an empty composer to type over. + regression test.
- Effort verify baselined like /model (fresh "Set effort level to <x>" past
a pre-injection count); removed a bare bottom-Contains that could
false-pass on a stale confirmation.
GUI (StickShift.App):
- The shift Task.Run is wrapped in try/catch: a UIA fault still delivers
window.outcome, so the gearbox can't freeze on a phantom "pending" gear.
- UNKNOWN_FINAL_STATE moved to the warn bucket (committed-but-unverified is
not a red error); any committed shift re-reads live state so the knob
never snaps back to the pre-shift model.
- Outbound JS is JSON-encoded (closes newline / U+2028 / quote breakouts a
hand-rolled escaper missed on untrusted pane text); outcome detail no
longer double-prints the reason; setLive sends empty agent when no session.
- Gearbox defaults to auto-confirm (a gear pull IS the confirmation, matching
the macOS app) so pulls don't stall at DIALOG_OPEN mid-conversation.
- App --target parse no longer drops the flag when it is the final argument.
52 Core tests green; full solution (incl. the OS-layer smoke test) builds
clean. No changes to README, .slnx, CI, or the smoke test.
…e CRIT class as 613fd4b) The draft-clear loop called WindowFocus.Focus(hwnd) but IGNORED its bool return, firing PressBackspace() regardless — so a focus drop mid-loop could blind-fire up to 300 destructive backspaces into whatever window then owned foreground. It also ran outside the StickShiftInjectionLock, so it could interleave with a concurrent shift. Same CRIT class 613fd4b closed in SwitchDriver, one file over, on a DESTRUCTIVE key — where it matters most. Fix mirrors SwitchDriver exactly: (1) serialize the whole clear under the named StickShiftInjectionLock mutex (fail-closed, 600ms, inherits an abandoned lock); (2) check Focus()'s bool BEFORE every Backspace and abort with NO_FOCUS (reporting how many landed) rather than typing into the wrong window. Verification bound (honest): CLI project builds 0/0; Core.Tests exit 0 (regression- clean). The injection path itself is SendInput into live windows — not unit-testable; the real check is windows/scripts/live-check.ps1, a human live-acceptance run. Finding: Eve cross-operator review 2026-07-19 (StickShift.Cli/Program.cs:88). Upstream is Mark Kashef's project (Early AI-Dopters / Skool) — offered as a PR suggestion, CI/README conventions respected as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1 (thanks for merging it — and for the CI + OS-layer smoke gate, that's a great addition). This is a code-only hardening pass on the Windows port; it deliberately touches no README,
.slnx, CI, or your smoke test — just the eight source files underwindows/.Why: a latent CRIT the merge inherited
SwitchDriverre-asserts foreground before each keystroke but ignored the result. If the target loses foreground mid-shift — the user alt-tabs, orSetForegroundWindowis denied — it blind-fired/model/Return/Escape/ digits into whatever window then owned focus. AReturninto a chat app sends the user's draft. This PR verifies every re-assert and aborts withNO_FOCUSbefore the keystroke.The rest came out of a four-lens adversarial audit (injection safety / hostile input / cross-file consistency / contract-fidelity vs the macOS shell):
LOCKEDrefusal otherwise. (Yourdocs/WINDOWS.mdnames this "Named mutex.")SendInputreaches only the active tab, so the reader no longer falls back to an off-screen agent pane (it would type into the wrong place). Prefers an on-screen pane, even a non-agent one →NO_AGENTrefuse.GetTexttruncates from the start — it returned the top of long scrollback, so bottom-anchored checks could read history. Now reads the full range and keeps the tail.IsInputEmptyexact-matches placeholders (like the Codex branch already did) — a real draft that merely begins with a placeholder ("Ask Claude about …") was classified as an empty composer and typed over. + a regression test (52 core checks now)./model— a freshSet effort level to <x>past a pre-injection count, instead of a bare bottom-Containsthat could false-pass on a stale confirmation.GUI (
StickShift.App):Task.Runis wrapped in try/catch so a UIA fault still deliverswindow.outcome— the gearbox can no longer freeze on a phantompendinggear.UNKNOWN_FINAL_STATEmoved to the warn bucket (committed-but-unverified isn't a red error), and any committed shift re-reads live state so the knob never snaps back to the pre-shift model.--targetparse no longer drops the flag when it's the final argument.Verified
52 Core checks green; the full solution — including your OS-layer smoke test — builds clean with these changes. No public signatures changed.
Not in this PR (suggestions only — your call, your repo)
A few upstream macOS-shell layers still aren't ported, and I left your README as-is rather than reword it. Noting them here so the gap is explicit:
STALE_FRAME, the 150ms frame clock) — this PR re-verifies foreground before each keystroke but doesn't re-check window identity/geometry between check and act the waySwitch.mdoes.UNSUPPORTED_EFFORT).Happy to take any of these in a separate PR if useful, or to adjust anything here.