fix(actions): add safety timeout to long-press repeat intervals (#284)#298
Conversation
Prevent fuel and replay control buttons from looping indefinitely when keyUp events are missed (observed on Mirabox devices). Both FuelService and ReplayControl now auto-stop their repeat intervals after 15 seconds and log a warning when the safety timeout triggers.
📝 WalkthroughWalkthroughThe PR adds a 15-second safety timeout mechanism to prevent long-press repeat actions from running indefinitely. When a repeat starts, a safety timeout is scheduled to automatically stop the repeat and log a warning. If Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/actions/src/actions/replay-control.ts (1)
565-574: Consider adding a brief comment explaining the dual-clear pattern.The
stopRepeatmethod calls bothclearTimeoutandclearIntervalonentry.timer(lines 569-570). This is intentional becausetimerholds asetTimeouthandle initially, then gets overwritten with asetIntervalhandle. A short inline comment would help future maintainers understand this design choice.💡 Suggested comment
private stopRepeat(contextId: string): void { const entry = this.repeatTimers.get(contextId); if (entry) { + // timer may be setTimeout (initial delay) or setInterval (repeat phase) clearTimeout(entry.timer); clearInterval(entry.timer); clearTimeout(entry.safety); this.repeatTimers.delete(contextId); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/actions/src/actions/replay-control.ts` around lines 565 - 574, Add a brief inline comment in stopRepeat explaining why both clearTimeout and clearInterval are called: mention that repeatTimers stores an entry whose timer field starts as a setTimeout handle and may later be replaced by a setInterval handle, so both clear* calls are used defensively to ensure the handle is cleared regardless of type; reference the stopRepeat method and the entry.timer and entry.safety fields when adding the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/actions/src/actions/replay-control.ts`:
- Around line 565-574: Add a brief inline comment in stopRepeat explaining why
both clearTimeout and clearInterval are called: mention that repeatTimers stores
an entry whose timer field starts as a setTimeout handle and may later be
replaced by a setInterval handle, so both clear* calls are used defensively to
ensure the handle is cleared regardless of type; reference the stopRepeat method
and the entry.timer and entry.safety fields when adding the comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40689c81-6f7f-4daa-9d06-be553f9be490
📒 Files selected for processing (4)
packages/actions/src/actions/fuel-service.test.tspackages/actions/src/actions/fuel-service.tspackages/actions/src/actions/replay-control.test.tspackages/actions/src/actions/replay-control.ts
Summary
FuelServiceandReplayControllong-press repeat intervals that auto-stops the repeat and logs a warning when triggeredkeyUpevents are missed (observed on Mirabox devices)stopRepeatCloses #284
Test plan
Summary by CodeRabbit
Release Notes