Skip to content

fix(input): harden readiness around persistent background leases - #311

Open
lymerin wants to merge 1 commit into
Tencent:mainfrom
lymerin:codex/242-background-input-hardening
Open

lymerin wants to merge 1 commit into
Tencent:mainfrom
lymerin:codex/242-background-input-hardening

Conversation

@lymerin

@lymerin lymerin commented Sep 21, 2026 •

Copy link
Copy Markdown

Problem

This is follow-up hardening for #242, not a claim that the original 0.2.1 bug remains unfixed.

withInputReady can temporarily toggle focus emulation for an unowned hidden page. A dispatcher-controlled page may already have a persistent background-execution lease. If temporary cleanup disables that lease’s override without updating BackgroundExecution’s applied-state cache, later synchronization may skip reapplying it.

Change

  • Make persistent lease ownership available to withInputReady through a read-only ownsBackgroundExecution predicate.
  • Continue sampling visibility when a lease is owned. If the page is visible, dispatch input without a temporary focus toggle or readiness screenshot. If it is hidden, return cdp_failed with reason: input_not_ready and effect_state: none before sending input. Do not toggle focus emulation locally.
  • Preserve the bounded temporary-focus and rendering-flush fallback for unowned hidden pages.
  • Cover lease-before-handler ordering and the hidden-leased-click behavior in dispatcher and interaction tests. The tests check that no temporary focus command or input is sent; a subsequent visible click succeeds.
  • Clarify that the ownership predicate reports desired lease ownership, not whether the override is currently applied.

This takes the fail-closed option from the review. Automatically restoring delivery when an owned lease is out of sync is separate work tracked in #355.

Validation

  • Post-review dispatcher and interaction tests: 163 passed.
  • Real-Chrome click and background-execution tests: 3 passed.
  • Full-stack fault injection on Windows 11 with Google Chrome for Testing 153 at DPR 1.5: started a --no-focus session, minimized the Agent Window, and disabled focus emulation from the extension Service Worker. The page reported hidden; bsk click returned input_not_ready with effect_state: none, and the click count remained zero. The temporary test and browser download were removed afterward.
  • Before these review changes, the full extension suite, TypeScript check, Biome, and git diff --check passed as reported in the earlier PR description.

@iuyo5678

Copy link
Copy Markdown
Collaborator

Thanks for tracking this down, and for the dispatcher-level coverage. I reviewed the PR against 0.3.1 and confirmed the state desync in code: withInputReady toggles Emulation.setFocusEmulationEnabled directly, and BackgroundExecution.synchronize returns early while its applied cache still matches the attachment, so the override is never re-enabled. A probe using the real ToolDispatcher and ChromiumCdp against a simulated debugger reproduces this on 0.3.1 and passes with this PR.

One point on reachability: on Chrome 153 (macOS, headless and headful), enabling focus emulation flips document.visibilityState to visible synchronously, even when the window is minimized. With a lease applied, withInputReady therefore never reaches the temporary path. The desync can only occur when a lease is owned but the page still reports hidden, for example on older Chrome versions or on other platforms (not verified), or in the attach/synchronize window.

Requested change
In that case, the PR skips the visibility sample and the rendering flush entirely (input-readiness.ts, the persistentLease branch) and dispatches straight away. That is the condition behind the original #242 silent success, so I'd rather not drop the safety net there. Could you keep sampling visibility while a lease is owned?

  • visible: dispatch directly, as the PR does now.
  • hidden: don't toggle focus emulation locally. Either have BackgroundExecution invalidate its applied state and re-synchronize, then flush a frame before dispatching, or return input_not_ready.

Smaller items

  • The new unit tests assert that visibility is never sampled, which pins the implementation rather than the symptom. Please add a test that runs a leased click on a hidden page and then asserts focus emulation is still enabled, with no re-enable needed before the next tool.
  • The shared.ts comment says "currently owns", but ownsBackgroundExecution reports desired ownership rather than applied state. Please adjust the wording.
  • The branch is well behind main. Please rebase so CI, including the browser job, can run.

If you can check it, a manual run on Windows 11 at DPR 1.5 (the environment in #242) would help most. Start a --no-focus session and minimize the Agent Window. Then turn the override off from the extension service worker to simulate an owned lease that is no longer applied, and confirm the click is still delivered.

Happy to re-review once updated. Thanks again!

@lymerin
lymerin force-pushed the codex/242-background-input-hardening branch 2 times, most recently from d24d66c to 009ea5f Compare September 27, 2026 08:45
@lymerin
lymerin force-pushed the codex/242-background-input-hardening branch from 009ea5f to 8e85b3e Compare September 27, 2026 16:43

This branch has not been deployed

No deployments
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.

2 participants