Skip to content

fix(setup): let the most specific sandbox.enabled win in configured() - #2011

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:fix/isolated-configured-precedence
Nov 5, 2026
Merged

potiuk merged 1 commit into
apache:mainfrom
potiuk:fix/isolated-configured-precedence

Conversation

@potiuk

@potiuk potiuk commented Nov 5, 2026

Copy link
Copy Markdown
Member

Summary

  • configured() treated any sandbox.enabled: true as enabled and ORed project and user scope, so a project enabled: false did not override a user true. /sandbox writes that false to the project's settings.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 rule sandbox-status-line.sh already follows.
  • sandbox_enabled_in_project() keeps its "any project file enables it" answer, because isolated_setup_findings() uses it to ask whether the project uses the setup at all. A /sandbox toggle-off does not stop the update findings.

Type of change

  • Python package (tools/*/ with pyproject.toml)
  • Other: the setup_preflight package shipped in the magpie-setup plugin

Test plan

  • uv run --project tools/setup-preflight pytest tools/setup-preflight/tests: 480 passed
  • test_the_most_specific_sandbox_setting_wins covers the cases where a false must win
  • test_a_sandbox_toggled_off_locally_still_gets_the_update_findings pins the update-findings gate
  • prek run --files <changed files> passes

RFC-AI-0004 compliance

  • Sandbox - no new host access; the check reports a disabled sandbox it used to miss

Linked issues

Refs #2002

Notes for reviewers (optional)

One behaviour to weigh: configured() now reads false after a temporary /sandbox toggle-off, so the session warning says the setup is not configured and complete-upgrade offers 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-empty isolated_setup stamp, 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

/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
@potiuk potiuk added family:setup setup-* skills capability:fix Implement code change to resolve an issue labels Nov 5, 2026
@github-actions github-actions Bot added family:tools tools/* capability:platform Framework / agent substrate skills (install, verify, doctor, override, status, setup bootstrap) substrate:setup Tool substrate: adopter-side setup state resolved at runtime (lock parsing, floors, fingerprints) labels Nov 5, 2026
@potiuk
potiuk merged commit 8ab23ba into apache:main Nov 5, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

capability:fix Implement code change to resolve an issue capability:platform Framework / agent substrate skills (install, verify, doctor, override, status, setup bootstrap) family:setup setup-* skills family:tools tools/* substrate:setup Tool substrate: adopter-side setup state resolved at runtime (lock parsing, floors, fingerprints)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant