Skip to content

fix(mac): count the HUD timer from the helper's first frame - #1054

Merged
EtienneLescot merged 1 commit into
mainfrom
fix/901-mac-hud-timer-start
Oct 7, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
fix/901-mac-hud-timer-start

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • The macOS start IPC now returns the helper's recording-started timestamp, the instant its first frame reached the file, and the HUD timer counts from it instead of from the reply's arrival. The cursor offset uses the same anchor. A helper that sends no timestamp keeps today's behaviour.
  • The ~20 s gap in the report was createdAt (the recording id, stamped before the permission prompts) taken for the start of capture; the stall itself was removed by fix: handle macOS capture permission failures #886.
  • Native follow-up, not in this PR: in a picker session, a take whose start stalls past main's 10 s timeout can still start capturing after its stop, because ScreenCaptureRecorder.start() never re-checks shutdownTask after its awaits (the microphone prompt can wait 30 s in ensureRequestedPermissions). Plan: check shutdownTask after each await before setupWriter/startCapture, and align the 30 s wait with main's 10 s.

Related issue

Part of #901

Type of change

  • Bug fix

Release impact

  • Patch

Desktop impact

  • macOS

Testing

  • New test in useScreenRecorder.nativeMacStartWarning.test.tsx: a helper stamp 20 s old shows 20 s on the HUD. It fails without the fix.
  • vitest --run src/hooks electron/ipc: 147 passed. Both tsc configs and Biome.
  • Not done: a take on macOS.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed the elapsed recording timer on macOS so it reflects when capture actually began, rather than when the start request finished. This keeps the timer accurate when there is a delay before the recording-start response arrives.

The start IPC now returns the helper's recording-started timestamp, the instant its first frame reached the file, and the HUD anchors to it instead of to the reply's arrival. The cursor offset uses the same anchor.
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e8d236ed-2194-4c29-a216-f3789b64f0ea
📥 Commits

Reviewing files that changed from the base of the PR and between 6e27423 and c8da1d7.

📒 Files selected for processing (4)
  • electron/ipc/handlers.ts
  • src/hooks/useScreenRecorder.nativeMacStartWarning.test.tsx
  • src/hooks/useScreenRecorder.ts
  • src/lib/nativeMacRecording.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The native macOS start flow now passes the helper’s capture-start timestamp through the IPC response. The screen recorder uses that timestamp to initialize elapsed time, with Date.now() as a fallback.

Changes

Native macOS start timing

Layer / File(s) Summary
Propagate the capture-start timestamp
electron/ipc/handlers.ts
The capture-start wait returns a finite helper timestamp or null. The start handler uses the timestamp when available, falls back to Date.now(), and returns it as startedAtMs.
Initialize elapsed time from the start result
src/lib/nativeMacRecording.ts, src/hooks/useScreenRecorder.ts, src/hooks/useScreenRecorder.nativeMacStartWarning.test.tsx
The start result adds optional startedAtMs. The recorder uses it to initialize timing. A test checks for 20 elapsed seconds when the start reply arrives 20 seconds after the reported start.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: arhxam

Merge Risk: 🔵 Low · up to c8da1

The change is mergeable; a system-clock rollback could briefly make the HUD show negative elapsed time, with no evidence of recording loss.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: the macOS HUD timer now starts from the helper’s first-frame timestamp.
Description check ✅ Passed The description covers the change, related issue, bug-fix and release impact, macOS impact, and testing. The omitted Screenshots / video section is not critical for this non-visual change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

@EtienneLescot
EtienneLescot merged commit f261fa4 into main Oct 7, 2026
19 checks passed
@EtienneLescot
EtienneLescot deleted the fix/901-mac-hud-timer-start branch October 7, 2026 11:53
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.

1 participant