Skip to content

devices widget: open file manager on dispvm block attach#300

Open
Sahilll10 wants to merge 1 commit intoQubesOS:mainfrom
Sahilll10:feature/auto-open-filemanager-dispvm-block-attach
Open

devices widget: open file manager on dispvm block attach#300
Sahilll10 wants to merge 1 commit intoQubesOS:mainfrom
Sahilll10:feature/auto-open-filemanager-dispvm-block-attach

Conversation

@Sahilll10
Copy link
Copy Markdown

@Sahilll10 Sahilll10 commented Mar 1, 2026

Opens the file manager automatically in the DispVM when a block
device is attached via the devices widget.

Uses the qubes.StartApp+qubes-open-file-manager qrexec service,
consistent with OpenFileManagerItem in qui/tray/domains.py.

Only triggers for block device class.

Fixes: QubesOS/qubes-issues#10709

@andrewdavidwong
Copy link
Copy Markdown
Member

andrewdavidwong commented Mar 2, 2026

Suggestion: If you intend for this pull request to resolve the associated issue and would like for it to be linked to the issue automatically, you can replace Fixes: #10709 with Fixes QubesOS/qubes-issues#10709 in the PR message. (The QubesOS/qubes-issues part is necessary, because this and qubes-issues are two different repos.) This will make it so that if and when the PR is merged, GitHub will automatically close the associated issue.

Comment thread qui/devices/actionable_widgets.py Outdated
if self.device.device_class == 'block':
try:
new_dispvm.run_service("qubes.StartApp+qubes-open-file-manager")
except Exception: # pylint: disable=broad-except
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I linked to an example providing the exception: QubesOS/qubes-issues#10709 (comment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please reread the link I pointed above. It is not broad Exception, it is a custom exception for Qubes. Follow the links and you will find it.

Comment thread qui/devices/actionable_widgets.py Outdated
if self.device.device_class == 'block':
try:
new_dispvm.run_service("qubes.StartApp+qubes-open-file-manager")
except Exception: # pylint: disable=broad-except
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ben-grande
Copy link
Copy Markdown

Also see CI, black is complaining.

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch 2 times, most recently from 137e262 to 70c63f8 Compare March 6, 2026 13:20
@Sahilll10
Copy link
Copy Markdown
Author

Hey @ben-grande, pushed an update addressing both your review points:

  1. Added wait=False to run_service() — non-blocking now, widget won't freeze
  2. Updated both exception handlers to use logger.exception() instead of logger.warning(..., exc_info=True)
  3. Applied the same fix to DetachAndAttachDisposableWidget as well
  4. Ran black — CI should be happy now

Also fixed the issue link to Fixes QubesOS/qubes-issues#10709 as @andrewdavidwong suggested.

Please let me know is anything else needs adjustment.

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from 70c63f8 to f7a000f Compare March 6, 2026 17:55
@Sahilll10
Copy link
Copy Markdown
Author

Hey @ben-grande

Thanks for pointing that out - and sorry for missing it earlier. I've now updated both handlers to catch exc.QubesException specifically and removed the # pylint: disable=broad-except comment since it’s no longer needed.

Hopefully this change now matches what you intended to be fixed.

I’m still learning and improving with each iteration, so I really appreciate your guidance and feedback during the review process.

Please let me know if any further adjustments are needed .

Comment thread qui/devices/actionable_widgets.py Outdated
)
except exc.QubesException:
logger.exception(
"Failed to open file manager in %s",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please read the example above: QubesOS/qubes-issues#10709 (comment)

The code needs to log the exception also, to let the user know what happened.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, you already have a logger in new_dispvm.log.

Comment thread qui/devices/actionable_widgets.py Outdated
)
except exc.QubesException:
logger.exception(
"Failed to open file manager in %s",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Sahilll10
Copy link
Copy Markdown
Author

Hi @ben-grande

Updated both handlers to use new_dispvm.log.exception() and capture as ex so the full exception detail is logged. Thanks for the guidance.

Please let me know if further adjustments are required.

@ben-grande
Copy link
Copy Markdown

Latest commit looks fine: 94a89fe. Please squash the commits. Do you have a Qubes machine to test it?

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from 94a89fe to cfbdc62 Compare March 10, 2026 12:44
@Sahilll10
Copy link
Copy Markdown
Author

Hi @ben-grande

I have squashed into a single commit and cleaned up the message.

Sir I don't currently have a dedicated Qubes machine. I've been working from the codebase and tracing patterns in the existing code. I'm working on a standard Linux machine for coding currently . I understand that's a real gap and I'm working on setting up a proper Qubes environment.

Happy to make any further adjustments

Comment thread qui/devices/actionable_widgets.py Outdated
from . import backend
import time

logger = logging.getLogger(__name__)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Extraneous.

Comment thread qui/devices/actionable_widgets.py Outdated
import asyncio
import functools
import pathlib
import logging
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Extraneous.

@ben-grande
Copy link
Copy Markdown

Sir

There is no need to "Sir" me. "Ben" is fine.

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from cfbdc62 to b8fbc63 Compare March 10, 2026 14:05
@Sahilll10
Copy link
Copy Markdown
Author

Hi @ben-grande

I’ve removed the extraneous import logging and logger = logging.getLogger(name) lines and amended the existing commit to keep the history clean.

Please let me know if any further adjustments are needed.

@ben-grande
Copy link
Copy Markdown

Please let me know if any further adjustments are needed.

Black complains. Please wait for the CI to pass or to be broken beyond your fixing (something unrelated to this PR), to request a review.

@ben-grande
Copy link
Copy Markdown

Please test with the MockQubesComplete. Although no file manager will open, it will log the call. Let me know how far this setup without QubesOS allows you to test GUI things out.

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from b8fbc63 to 81787aa Compare March 11, 2026 18:25
@ben-grande
Copy link
Copy Markdown

Marek, CI is failing due to unexpected call to admin.vm.List: https://gitlab.com/QubesOS/qubes-desktop-linux-manager/-/jobs/13455975135. Since adding that to klass, do you think it should be added as expected per test or for all tests?

@ben-grande
Copy link
Copy Markdown

I think the code looks fine. Added openqa label.

@ben-grande
Copy link
Copy Markdown

I think the code looks fine. Added openqa label.

The @dispvm rules seem relevant to this PR.

Details

=========================== short test summary info ============================
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_handler_load_state - AssertionError: assert ['TestService * test-blue @dispvm ask default_target=@dispvm:default-dvm', 'TestService * test-red @dispvm deny'] == ['TestService * test-blue @dispvm ask default_target=@dispvm:default-dvm', 'TestService * test-red @dispvm deny', 'TestService * test-vm @dispvm allow target=@dispvm']
  Right contains one more item: 'TestService * test-vm @dispvm allow target=@dispvm'
  Full diff:
    [
     'TestService * test-blue @dispvm ask default_target=@dispvm:default-dvm',
     'TestService * test-red @dispvm deny',
  -  'TestService * test-vm @dispvm allow target=@dispvm',
    ]
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_add_rule - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd67bc20>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd67bc20> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd7c3260>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_add_rule_error - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd56fa40>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd56fa40> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd4e2d50>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_add_rule_twice - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd4737a0>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd4737a0> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd3daa80>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_edit_rule - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Deny>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd3774d0>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd3774d0> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd2da450>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_edit_double_click - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd26ea20>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd26ea20> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd2d9250>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_edit_cancel - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd176030>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd176030> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dd4e2840>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_close_all_fail - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd071d90>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dd071d90> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dcfe10d0>.list_handler
FAILED qubes_config/tests/test_policy_exceptions_handler.py::test_policy_handler_reset - AssertionError: assert False
 +  where False = compare_rule_lists([<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>], [<Rule service='TestService' argument=None source='test-vm' target='@dispvm' action=<Allow target='@dispvm' user=None>>, <Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>])
 +    where [<Rule service='TestService' argument=None source='test-red' target='@dispvm' action=<Ask target=None default_target='@dispvm:default-dvm' user=None>>] = <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dcede600>.current_rules
 +      where <qubes_config.global_config.policy_exceptions_handler.PolicyExceptionsHandler object at 0x7f81dcede600> = <qubes_config.global_config.policy_exceptions_handler.DispvmExceptionHandler object at 0x7f81dcede720>.list_handler

@Sahilll10
Copy link
Copy Markdown
Author

Hi @ben-grande @marmarek

I have confirmed that the test_policy_exceptions_handler.py failures are pre-existing on main and completely unrelated to this PR. Identical results on both:
On main (commit d56bb9e):

FAILED test_policy_exc_handler_load_state
FAILED test_policy_exc_add_rule
FAILED test_policy_exc_add_rule_error
FAILED test_policy_exc_add_rule_twice
FAILED test_policy_exc_edit_rule
FAILED test_policy_exc_edit_double_click
FAILED test_policy_exc_edit_cancel
FAILED test_policy_exc_close_all_fail
FAILED test_policy_handler_reset

On this PR (commit 81787aa): exact same 9 failures, no new failures introduced.

Regarding MockQubesComplete testing — the mock initialized successfully. No block devices exist in the mock by default so run_service is never triggered, but the device_class == "block" code path is confirmed correct.
Without a real Qubes machine, triggering an actual device attach event to fully exercise the new code is not possible from this environment

@ben-grande
Copy link
Copy Markdown

FAILED test_policy_exc_handler_load_state

Git bisect on core-admin-client pointed to QubesOS/qubes-core-admin-client@84edd42. For git bisect run, I used:

#!/bin/sh
cd ~/qubes-desktop-linux-manager
PYTHONPATH=../qubes-core-admin-client xvfb-run python3 -m pytest -vv qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_handler_load_state

I did not expect that for the @dispvm token on the policy... I expected that for the admin.vm.List calls.

@ben-grande
Copy link
Copy Markdown

Created another issue for the broken tests: QubesOS/qubes-issues#10770.

@Sahilll10
Copy link
Copy Markdown
Author

Sahilll10 commented Mar 12, 2026

Hi @ben-grande

I have submitted a fix for the broken tests in
QubesOS/qubes-core-admin-client#450

The fix registers the per-VM admin.vm.List call inside _add_to_vm_list so all MockQube instances automatically get it without per-test duplication.

Please let me know if further adjustments are required.

@Sahilll10
Copy link
Copy Markdown
Author

Sahilll10 commented Mar 12, 2026

FAILED test_policy_exc_handler_load_state

Git bisect on core-admin-client pointed to QubesOS/qubes-core-admin-client@84edd42. For git bisect run, I used:

#!/bin/sh
cd ~/qubes-desktop-linux-manager
PYTHONPATH=../qubes-core-admin-client xvfb-run python3 -m pytest -vv qubes_config/tests/test_policy_exceptions_handler.py::test_policy_exc_handler_load_state

I did not expect that for the @dispvm token on the policy... I expected that for the admin.vm.List calls.

Hi @ben-grande

PR QubesOS/qubes-core-admin-client#450 addresses this. The fix registers the per-VM admin.vm.List call inside MockQube._add_to_vm_list, so all mock VMs automatically get it without per-test duplication — which is what you hoped for in the issue.
Regarding the @dispvm token surprise — our fix resolves that too. The DispvmExceptionHandler was crashing during initialization due to the missing admin.vm.List registration before it could even process the @dispvm rules. With the fix applied:

WITHOUT fix: 9 failed, 2 passed
WITH fix: 11 passed, 0 failed

All test_policy_exceptions_handler tests now pass cleanly

@ben-grande
Copy link
Copy Markdown

PipelineRetryFailed

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.29%. Comparing base (d56bb9e) to head (9fee2cf).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #300      +/-   ##
==========================================
- Coverage   92.99%   91.29%   -1.70%     
==========================================
  Files          64       66       +2     
  Lines       13312    13837     +525     
==========================================
+ Hits        12379    12632     +253     
- Misses        933     1205     +272     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ben-grande
Copy link
Copy Markdown

Please test with the MockQubesComplete. Although no file manager will open, it will log the call. Let me know how far this setup without QubesOS allows you to test GUI things out.

Fixing my phrase above:

Although no file manager will NOT open, it will log the call.

Hi. Can you test this before I send to OpenQA? https://doc.qubes-os.org/en/r4.3/developer/general/developing-gui-applications.htm

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from 81787aa to ac7c01f Compare March 13, 2026 11:04
@Sahilll10
Copy link
Copy Markdown
Author

Sahilll10 commented Mar 13, 2026

Hi @ben-grande

I have tested with MockQubesComplete. Added tests in qui/devices/tests/test_actionable_widgets.py:

test_block_device_opens_file_manager — verifies run_service("qubes.StartApp+qubes-open-file-manager", wait=False) is called when a block device is attached to a dispvm
test_non_block_device_no_file_manager — verifies run_service is NOT called for USB devices
test_file_manager_exception_logged — verifies QubesException is caught and logged via dispvm.log.exception

All 5 tests pass locally:

rootdir: /home/sahil_kumar/qubes-desktop-linux-manager
collecting ... collected 5 items

qui/devices/tests/test_actionable_widgets.py::TestAttachDisposableWidget::test_block_device_opens_file_manager PASSED [ 20%]
qui/devices/tests/test_actionable_widgets.py::TestAttachDisposableWidget::test_non_block_device_no_file_manager PASSED [ 40%]
qui/devices/tests/test_actionable_widgets.py::TestAttachDisposableWidget::test_file_manager_exception_logged PASSED [ 60%]
qui/devices/tests/test_actionable_widgets.py::TestDetachAndAttachDisposableWidget::test_block_device_opens_file_manager PASSED [ 80%]
qui/devices/tests/test_actionable_widgets.py::TestDetachAndAttachDisposableWidget::test_file_manager_exception_logged PASSED [100%]

============================== 5 passed in 0.94s ===============================

As expected, no actual file manager opens since there is no real Qubes machine — but the run_service call is confirmed correct via mock assertions.

Please let me know if further changes are needed.

@ben-grande
Copy link
Copy Markdown

ben-grande commented Mar 13, 2026

Please let me know if further changes are needed.

Please wait for CI before asking for a review:

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch 4 times, most recently from e343d25 to 1ad3a9d Compare March 14, 2026 14:30
@Sahilll10
Copy link
Copy Markdown
Author

Sahilll10 commented Mar 14, 2026

Hi @ben-grande

The remaining checks:tests failure is test_global_config_init — a GTK abort deep inside rule_list_widgets.py and gtk_widgets.py. This PR does not touch either of those files. The three files changed in this PR are:

qui/devices/actionable_widgets.py
qui/devices/tests/init.py
qui/devices/tests/test_actionable_widgets.py

The crash traceback points to gtk_widgets.py:450-rule_list_widgets.py:115 — both untouched by this PR. The test also passes locally with the same codebase, suggesting this is an Xvfb/GTK environment instability in CI rather than a regression introduced here.

The pylint and black checks are now both passing cleanly.

@ben-grande
Copy link
Copy Markdown

PipelineRetryFailed

@Sahilll10
Copy link
Copy Markdown
Author

Hi @ben-grande

Apologies for being off topic.

I had previously reached out on the Qubes-devel mailing list regarding my interest in the System Health Monitor project for the upcoming GSoC . Since the proposal submission phase has now started, I wanted to follow up and see if it might be possible to briefly discuss the direction of the project idea and any technical considerations that I should keep in mind while preparing my proposal.

My goal is to make sure that the proposal aligns well with the project's expectations and architecture.

Ben where can we discuss it?
I would appreciate your guidance.

@ben-grande
Copy link
Copy Markdown

Ben where can we discuss it?

I think that the qubes-devel is the best place for this.

@ben-grande
Copy link
Copy Markdown

Changes looks good. I haven't tested yet. Can you address disabling this change if the user wants to: QubesOS/qubes-issues#10709 (comment). Should it be enabled by default?

Copy link
Copy Markdown
Member

@marmarta marmarta left a comment

Choose a reason for hiding this comment

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

This is just a little bit of nitpicking about the tests. I like the PR in general.

@pytest.fixture
def mock_qapp():
app = MockQubesComplete()
app.update_vm_calls()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That shouldn't be needed? The Mock object updates its call on creation. This is needed only when you're adding something to the object (e.g. a new qube) or changing some properties.

return device


def make_mock_vm(qubes_app):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, I don't think this is 100% wrong - it definitely works - but a more future-resilient way is using the existing mocks and overwriting only the method you want to verify. E.g. here you can use any of the existing mock vms in the MockQubesComplete object and later when you want to check that run_service was called, just patch the run_service object. That approach should be more resilient to later changes in the code, while your approach means that potentially if some part of the code changes and e.g. asks for more VM properties, this mock has to be re-done.

@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from 1ad3a9d to 2cb88ac Compare March 17, 2026 18:06
When a block device is attached to a new disposable via the devices
widget, automatically open the file manager inside the dispvm using
the qubes.StartApp+qubes-open-file-manager qrexec service.

Only triggers for block device class. Uses new_dispvm.log for
logging, consistent with existing patterns in the codebase.

Fixes QubesOS/qubes-issues#10709
@Sahilll10 Sahilll10 force-pushed the feature/auto-open-filemanager-dispvm-block-attach branch from 2cb88ac to 9fee2cf Compare March 17, 2026 18:32
@Sahilll10
Copy link
Copy Markdown
Author

Sahilll10 commented Mar 17, 2026

Hi @ben-grande @marmarta

Addressed both review comments:

Feature flag : Added FEATURE_OPEN_FILE_MANAGER = "device-open-file-manager-on-attach" constant to backend.py. Enabled by default — user can opt out by setting the feature to an empty string on the appvm. Both AttachDisposableWidget and DetachAndAttachDisposableWidget check self.vm.vm_object.features.get(backend.FEATURE_OPEN_FILE_MANAGER, "1") before calling run_service.

Test refactor : Removed redundant app.update_vm_calls(). Updated make_mock_vm to use qubes_app._qubes["test-vm"] as vm_object directly — uses the real MockQube instance including its features dict.
Added test_feature_flag_disables_file_manager to both test classes to cover the opt-out path.

The checks:pylint failure is a CI infrastructure issue unrelated to this PR. The job fails at pip3 install -r ci/requirements.txt while building pygobject — gobject-introspection-1.0 is missing from the runner environment.
The code is never reached and pylint never runs.
Previous CI runs on this PR installed pygobject successfully.

Could you trigger a pipeline retry?

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.

Add a "Format with DispVM" button for USB drives in the Qubes Devices widget

4 participants