Skip to content

fix(desktop): make snapshot extension enable idempotent - #11630

Open
AKolenda wants to merge 1 commit into
pingdotgg:mainfrom
AKolenda:fix/gnome-snapshot-enable-idempotent
Open

fix(desktop): make snapshot extension enable idempotent#11630
AKolenda wants to merge 1 commit into
pingdotgg:mainfrom
AKolenda:fix/gnome-snapshot-enable-idempotent

Conversation

@AKolenda

@AKolenda AKolenda commented Sep 13, 2026

Copy link
Copy Markdown

What Changed

Treat enabling the GNOME snapshot extension as a successful no-op when GNOME already reports it as active. A focused regression test verifies that setup resolves successfully without sending a redundant EnableExtension call.

Why

A repeated or raced setup action currently turns the valid enabled state into a DesktopSnapShotSetupError. This makes the app report that it could not set up the GNOME extension even though capture is already ready.

Checklist

  • This PR is small and focused
  • I explained what changed and why

Verification

  • vp test run apps/desktop/src/snapShot/GnomeCaptureSetup.test.ts
  • vp lint apps/desktop/src/snapShot/GnomeCaptureSetup.ts apps/desktop/src/snapShot/GnomeCaptureSetup.test.ts
  • vp fmt --check apps/desktop/src/snapShot/GnomeCaptureSetup.ts apps/desktop/src/snapShot/GnomeCaptureSetup.test.ts
  • vp run --filter @t3tools/desktop typecheck

Model: GPT-5
Harness: Codex desktop app

Summary by CodeRabbit

  • Bug Fixes
    • Fixed GNOME capture setup so enabling an already active capture extension completes successfully without triggering an unnecessary enable request.
  • Tests
    • Added coverage for the already-enabled extension scenario.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 13, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a82f999

Macroscope's review found this PR approvable — This is a focused idempotency bug fix that avoids a redundant GNOME D-Bus enable call when the extension is already active, with a regression test covering the new behavior. Other setup actions and existing states are unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2b07ff99-0119-4871-b515-3a89f605f777

📥 Commits

Reviewing files that changed from the base of the PR and between 77bca8b and a82f999.

📒 Files selected for processing (2)
  • apps/desktop/src/snapShot/GnomeCaptureSetup.test.ts
  • apps/desktop/src/snapShot/GnomeCaptureSetup.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The GNOME capture setup now returns early when enable-extension targets an already-enabled extension. A test confirms successful completion without an EnableExtension D-Bus request.

Changes

GNOME capture enable behavior

Layer / File(s) Summary
Enable guard and coverage
apps/desktop/src/snapShot/GnomeCaptureSetup.ts, apps/desktop/src/snapShot/GnomeCaptureSetup.test.ts
perform("enable-extension") now succeeds when the extension state is enabled. The test verifies that EnableExtension is not called.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: bil0000

Merge Risk: ⚪ Minimal · up to a82f9

The enabled-extension path now completes without an unnecessary D-Bus request, and no concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: making GNOME snapshot extension enabling idempotent.
Description check ✅ Passed The description explains what changed, why it changed, the focused scope, and verification steps. The omitted UI checklist items are not applicable because this change does not modify the UI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant