Skip to content

Protect assistant uploads with Rewind exclusions#7191

Open
kodjima33 wants to merge 1 commit intomainfrom
fix/issue-7098
Open

Protect assistant uploads with Rewind exclusions#7191
kodjima33 wants to merge 1 commit intomainfrom
fix/issue-7098

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • block Rewind-excluded apps before proactive assistant frame tracking/distribution
  • make Memory, Insight, and Focus assistant exclusion checks respect Rewind privacy exclusions
  • keeps the Rewind indexer gate unchanged while preventing sensitive frames from reaching LLM upload paths

Fixes #7098

Verification

  • Mac mini: cd ~/projects/omi/desktop/Desktop && swift build 2>&1 | tail -5
  • Build complete on Mac mini

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 6, 2026

Greptile Summary

This PR adds Rewind privacy exclusions to the proactive assistant pipeline: excluded apps are now blocked from frame tracking/distribution in ProactiveAssistantsPlugin, and all three assistant settings classes (Memory, Insight, Focus) now consult RewindSettings.shared.isAppExcluded before their own exclusion lists.

  • Plugin-level gate: trackFrame and distributeFrame calls are wrapped in if !isRewindExcluded, preventing sensitive frames from reaching any assistant or the AssistantCoordinator.
  • Per-assistant gate: isAppExcluded in each of the three settings classes now checks Rewind exclusions first, adding a defence-in-depth layer at the analysis level.
  • Context-switch side-effect: because checkContextSwitch runs before the new isRewindExcluded guard, lastTrackedFrame is never updated while the user is in an excluded app; when they leave, onContextSwitch fires with a potentially very stale departingFrame instead of nil.

Confidence Score: 3/5

The privacy gate itself is sound, but skipping trackFrame for excluded apps leaves lastTrackedFrame stale in AssistantCoordinator, so context-switch handlers receive an incorrect departing frame whenever the user leaves an excluded app.

The three assistant-settings changes are clean. The plugin change correctly blocks sensitive frames from reaching LLM paths, but AssistantCoordinator assumes lastTrackedFrame is always current — now it can be arbitrarily old when the user was in an excluded app, so any context-switch analysis triggered on leaving an excluded app will act on stale data rather than nil.

desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift and AssistantCoordinator.swift (not in this PR) both need attention: the former now conditionally calls trackFrame, while the latter still reads lastTrackedFrame unconditionally as departingFrame in checkContextSwitch.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift Gates frame tracking and distribution behind the Rewind exclusion check; introduces a stale lastTrackedFrame when leaving an excluded app, which causes onContextSwitch to fire with an incorrect departing frame. Also has a redundant if !isRewindExcluded in the macOS 13.x path.
desktop/Desktop/Sources/ProactiveAssistants/Assistants/Focus/FocusAssistantSettings.swift Adds RewindSettings.shared.isAppExcluded as an early check in isAppExcluded, consistent with the plugin-level gate and identical to the Insight/Memory changes.
desktop/Desktop/Sources/ProactiveAssistants/Assistants/Insight/InsightAssistantSettings.swift Adds RewindSettings.shared.isAppExcluded as a first check in isAppExcluded, mirroring the Focus and Memory changes without functional issues.
desktop/Desktop/Sources/ProactiveAssistants/Assistants/MemoryExtraction/MemoryAssistantSettings.swift Adds RewindSettings.shared.isAppExcluded as a first check in isAppExcluded, mirroring the Focus and Insight changes without functional issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Capture frame] --> B{isRewindExcluded?}
    B -- Yes --> C[Skip trackFrame / distributeFrame / RewindIndexer]
    B -- No --> D[AssistantCoordinator.trackFrame]
    D --> E{isInDelayPeriod?}
    E -- No --> F[distributeFrameIfChanged]
    E -- Yes --> G[distributeFrameDuringDelay]
    F --> H[Assistant.analyze]
    G --> H
    H --> I{isAppExcluded? Rewind or built-in or user}
    I -- Yes --> J[Skip analysis]
    I -- No --> K[Process frame / emit event]
    B -- No --> L[RewindIndexer.processFrame]
Loading

Comments Outside Diff (1)

  1. desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift, line 861-874 (link)

    P2 The macOS 13.x fallback path evaluates isRewindExcluded twice in two immediately adjacent if blocks. Both blocks share the same condition and could be merged into one to avoid redundancy and make the guard easier to follow.

Reviews (1): Last reviewed commit: "Protect assistant uploads with Rewind ex..." | Re-trigger Greptile

Comment on lines +791 to 801
if !isRewindExcluded {
// Track and distribute only frames that pass the shared screen privacy gate.
AssistantCoordinator.shared.trackFrame(frame)

if !isInDelayPeriod {
distributeFrameIfChanged(frame)
} else {
// During delay, still distribute to assistants that need it (e.g. refocus detection)
AssistantCoordinator.shared.distributeFrameDuringDelay(frame)
}
}
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.

P1 Stale departingFrame when leaving an excluded app

checkContextSwitch is called unconditionally (line 706) and reads lastTrackedFrame as the departing frame before calling trackFrame. Because trackFrame is now gated on !isRewindExcluded, lastTrackedFrame is never updated while the user is in an excluded app. When the user later switches away from that excluded app, onContextSwitch fires with a departingFrame holding a frame from before they entered the excluded app — potentially minutes stale. Assistants that rely on departingFrame to represent "what the user was just doing" (e.g. memory/focus extraction on context leave) will act on incorrect data. The intent in the comment at line 704 ("departing frame is from the previous context") is broken for excluded apps. Clearing lastTrackedFrame to nil inside AssistantCoordinator when the excluded gate is hit, or passing nil explicitly when isRewindExcluded, would fix this.

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.

Desktop: Rewind privacy exclusions do not protect proactive assistant LLM uploads

1 participant