Skip to content

Fix: Add timeout safety valve to phrase stabilization polling#106

Merged
cmorten merged 3 commits intoguidepup:mainfrom
what-name:fix/max-poll-count-safety-valve
Jan 25, 2026
Merged

Fix: Add timeout safety valve to phrase stabilization polling#106
cmorten merged 3 commits intoguidepup:mainfrom
what-name:fix/max-poll-count-safety-valve

Conversation

@what-name
Copy link
Contributor

@what-name what-name commented Dec 25, 2025 •
edited
Loading

Problem

LogStore#pollForSpokenPhrases uses an unbounded while (true) loop that can hang indefinitely when VoiceOver output never stabilizes. This occurs when:

  • Elements disappear after interaction (e.g., dismissing dialogs)
  • Focus moves to transient UI states
  • VoiceOver announcement patterns don't reach stable state

When phrases don't stabilize, the polling loop runs forever, blocking all subsequent commands and potentially freezing the entire test suite.

Solution

Add MAX_SPOKEN_PHRASES_POLL_COUNT constant (60 polls ≈ 3 second timeout) to bound the stabilization loop:

  // Before
  while (true) {
    // Poll for stable phrase...
  }

  // After  
  while (pollCount < MAX_SPOKEN_PHRASES_POLL_COUNT) {
    // Poll for stable phrase...
  }

After ~3 seconds, the method returns the best phrases collected so far instead of hanging forever.

Testing

Tested on 30+ interactive sequences with dynamic content (modals, disappearing elements, state transitions):

  • Before: Infinite hangs requiring test termination
  • After: Graceful timeouts, zero hangs

Breaking Changes

None. Adds safety timeout while preserving existing stabilization behavior.

Files changed:

  • src/macOS/VoiceOver/constants.ts - Added MAX_SPOKEN_PHRASES_POLL_COUNT = 60
  • src/macOS/VoiceOver/LogStore.ts - Changed while (true) to while (pollCount < MAX_SPOKEN_PHRASES_POLL_COUNT)

cmorten
cmorten reviewed Jan 3, 2026
View reviewed changes
@cmorten
Copy link
Collaborator

cmorten commented Jan 3, 2026

@what-name apologies for stepping on toes - I've updated the MAX_SPOKEN_PHRASES_POLL_COUNT value per my comment, but please shout if want to refute! Open to discussion on best value.

I've added a couple of non-golden path tests, not super thorough, but hopefully hints at the scenario and prevents regression.

Really appreciate the detailed description and thought/testing gone into this!

@what-name
Copy link
Contributor Author

what-name commented Jan 25, 2026 •
edited
Loading

Hey, sorry for the late response @cmorten and thanks for adding the tests! Looks good imo

Quick heads up though; I've been running guidepup a lot for an AI agent doing accessibility testing, and noticed the polling loop can cause screen recording frame drops. looks like each lastSpokenPhrase() call spawns osascript, and at 50-100 polls per action that adds up. Not a problem for this PR specifically, just something I discovered while debugging why my recordings were choppy (on dedicated M1 Mac Mini). Might be worth a note in docs someday for folks doing screen recording alongside guidepup....or they just find this comment :) I am using my own fork since this anyways, so feel free to merge this PR if you want!

Thanks for Guidepup!

edit: the checks failing is interesting, it says "unable to activate chrome". not sure why tbh. I am using webkit driver personally, the PR doesnt touch anything in that codepath to break chrome in any way. perhaps a CI issue. but I had the same error when tried to switch to chrome from webkit, didn't do it because of this, but didn't investigate further either.

@cmorten cmorten merged commit 307f754 into guidepup:main Jan 25, 2026
7 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmorten cmorten cmorten left review comments

Assignees

@what-name what-name

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@what-name @cmorten