Repository navigation
fix(setup): let the most specific sandbox.enabled win in configured() - #2011
Merged
Merged
Conversation
/sandbox writes enabled: false to the project's settings.local.json, but configured() ORed every scope, so a user-scope true hid it and the session got no warning. Take the first explicit value instead, as Claude Code and sandbox-status-line.sh do. sandbox_enabled_in_project() keeps its any-file answer: the update findings use it to ask whether the project uses the setup at all, and a toggle-off should not stop them. Generated-by: Claude Opus 5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
configured()treated anysandbox.enabled: trueas enabled and ORed project and user scope, so a projectenabled: falsedid not override a usertrue./sandboxwrites thatfalseto the project'ssettings.local.json, so a session switched out of the sandbox still counted as configured and got no warning.configured()now takes the first explicit value, most specific file first, the rulesandbox-status-line.shalready follows.sandbox_enabled_in_project()keeps its "any project file enables it" answer, becauseisolated_setup_findings()uses it to ask whether the project uses the setup at all. A/sandboxtoggle-off does not stop the update findings.Type of change
tools/*/withpyproject.toml)setup_preflightpackage shipped in themagpie-setuppluginTest plan
uv run --project tools/setup-preflight pytest tools/setup-preflight/tests: 480 passedtest_the_most_specific_sandbox_setting_winscovers the cases where afalsemust wintest_a_sandbox_toggled_off_locally_still_gets_the_update_findingspins the update-findings gateprek run --files <changed files>passesRFC-AI-0004 compliance
Linked issues
Refs #2002
Notes for reviewers (optional)
One behaviour to weigh:
configured()now reads false after a temporary/sandboxtoggle-off, so the session warning says the setup is not configured andcomplete-upgradeoffers an install to someone who has it. Declining that offer writes"isolated_setup": {"enabled": false}, which silences the update findings after the sandbox is back on. Separating "installed" from "in effect" would fix that, and is a larger change than this one.Two gaps stay:
configured()still returns early on a non-emptyisolated_setupstamp, and in a linked worktree it reads the worktree's settings rather than the main checkout's. Managed settings are not read either.🤖 Generated with Claude Code