Skip to content

fix(desktop): start screen capture after chat onboarding step#5967

Merged
kodjima33 merged 1 commit into
mainfrom
worktree-fix-auth-activate-app
Mar 24, 2026
Merged

fix(desktop): start screen capture after chat onboarding step#5967
kodjima33 merged 1 commit into
mainfrom
worktree-fix-auth-activate-app

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • Screen capture only started after the full onboarding (Tasks step), but permissions are granted during the chat step
  • Users would finish onboarding, click Rewind, and see no screenshots because capture hadn't started yet
  • Now startMonitoring() fires right after the chat step (step 0) completes, giving 2-3 minutes of screenshots to accumulate while the user goes through the remaining onboarding steps (floating bar, voice shortcut, tasks)

Test plan

  • Reset onboarding, go through chat step, complete it
  • While on floating bar / voice shortcut steps, verify screenshots are being captured (check log for "captureFrame")
  • Finish onboarding, open Rewind tab — should already have screenshots

🤖 Generated with Claude Code

Screen capture was only starting after the full onboarding (step 3 Tasks),
but permissions are granted during the chat step (step 0). This meant
the Rewind tab was empty when users first opened it after onboarding.

Now startMonitoring() fires right after the chat step completes, so
screenshots accumulate while the user goes through floating bar demo,
voice shortcut, and tasks steps.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kodjima33 kodjima33 merged commit d822ed0 into main Mar 24, 2026
2 checks passed
@kodjima33 kodjima33 deleted the worktree-fix-auth-activate-app branch March 24, 2026 02:47
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR fixes a UX bug where the Rewind tab had no screenshots immediately after onboarding because screen-capture monitoring only started at the very end of onboarding (Tasks step), well after screen-recording permission was granted during the chat step. The fix calls ProactiveAssistantsPlugin.shared.startMonitoring() as soon as the chat step (step 0) completes or is skipped, giving the capture pipeline 2-3 minutes to accumulate frames while the user proceeds through the remaining onboarding steps.

Key details:

  • startMonitoring() is guarded by both the new if !isMonitoring check and startMonitoring()'s own internal isMonitoring && isStartingMonitoring guard, so the unconditional call that already existed in handleOnboardingComplete() remains safe (it just returns early).
  • If screen recording permission is not yet granted at skip time, startMonitoring() automatically calls ScreenCaptureService.requestAllScreenCapturePermissions() and retries with exponential back-off (2 s / 4 s / 8 s), so no extra permission handling is needed at the call site.
  • The onSkip path starts monitoring without recording an analytics event for step 0 being skipped — unlike steps 1 and 2 which both pass a _Skipped suffix to onboardingStepCompleted. This is pre-existing, but could be addressed alongside this change.

Confidence Score: 5/5

  • Safe to merge — the change is small, targeted, and protected by existing guards in startMonitoring().
  • The fix is a two-location addition of a well-guarded startMonitoring() call. The method itself handles duplicate invocations, permission retries, and race conditions internally, so no new failure modes are introduced. The only open item (missing analytics on the skip path) is pre-existing and non-blocking.
  • No files require special attention.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/OnboardingView.swift Adds startMonitoring() call after the chat step (step 0) completes or is skipped, so screenshots begin accumulating before onboarding finishes. Logic is correct and protected by both the new isMonitoring guard and startMonitoring's own internal race-condition guards.

Sequence Diagram

sequenceDiagram
    participant User
    participant OnboardingView
    participant ProactiveAssistantsPlugin

    User->>OnboardingView: Complete / Skip chat step (step 0)
    OnboardingView->>ProactiveAssistantsPlugin: startMonitoring() [new]
    ProactiveAssistantsPlugin-->>ProactiveAssistantsPlugin: refreshScreenRecordingPermission()
    alt Permission granted
        ProactiveAssistantsPlugin-->>ProactiveAssistantsPlugin: isMonitoring = true, captureTimer starts
    else Permission not yet granted
        ProactiveAssistantsPlugin-->>ProactiveAssistantsPlugin: requestAllScreenCapturePermissions() + retry (2s/4s/8s)
    end
    OnboardingView->>OnboardingView: currentStep = 1 (FloatingBar)
    User->>OnboardingView: Complete steps 1–3
    OnboardingView->>OnboardingView: handleOnboardingComplete()
    OnboardingView->>ProactiveAssistantsPlugin: startMonitoring() [pre-existing, no-op if already running]
    ProactiveAssistantsPlugin-->>OnboardingView: completion(true, nil) [guard short-circuits]
Loading

Reviews (1): Last reviewed commit: "fix(desktop): start screen capture after..." | Re-trigger Greptile

Comment on lines 91 to 95
onSkip: {
if !ProactiveAssistantsPlugin.shared.isMonitoring {
ProactiveAssistantsPlugin.shared.startMonitoring { _, _ in }
}
currentStep = 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing analytics for skip path

The onSkip handler starts monitoring but does not record an analytics event for step 0 being skipped, while steps 1 and 2 both call AnalyticsManager.shared.onboardingStepCompleted with a _Skipped suffix in their onSkip closures. This is pre-existing, but the PR adds monitoring to the skip path without closing the gap.

Consider adding:

Suggested change
onSkip: {
if !ProactiveAssistantsPlugin.shared.isMonitoring {
ProactiveAssistantsPlugin.shared.startMonitoring { _, _ in }
}
currentStep = 1
onSkip: {
AnalyticsManager.shared.onboardingStepCompleted(step: 0, stepName: "Chat_Skipped")
if !ProactiveAssistantsPlugin.shared.isMonitoring {
ProactiveAssistantsPlugin.shared.startMonitoring { _, _ in }
}
currentStep = 1
}

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…ardware#5967)

## Summary
- Screen capture only started after the full onboarding (Tasks step),
but permissions are granted during the chat step
- Users would finish onboarding, click Rewind, and see no screenshots
because capture hadn't started yet
- Now `startMonitoring()` fires right after the chat step (step 0)
completes, giving 2-3 minutes of screenshots to accumulate while the
user goes through the remaining onboarding steps (floating bar, voice
shortcut, tasks)

## Test plan
- [ ] Reset onboarding, go through chat step, complete it
- [ ] While on floating bar / voice shortcut steps, verify screenshots
are being captured (check log for "captureFrame")
- [ ] Finish onboarding, open Rewind tab — should already have
screenshots

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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