Skip to content

stella-cli: enable_recap_survives_the_scope_merge / create_worktrees_survives_the_scope_merge race on STELLA_MANAGED_SETTINGS #3312

Description

@macanderson

Problem

crates/stella-cli/src/settings/tests.rs::settings::tests::enable_recap_survives_the_scope_merge and ::create_worktrees_survives_the_scope_merge are flaky under the default multi-threaded cargo test runner. Roughly 1 in 3-4 runs of cargo test -p stella-cli --bin stella settings:: fails one or both with:

settings load: "cannot securely read managed settings /var/folders/.../managed.json: managed settings must be a single-link regular file owned by root or the process user and not group/other writable"

--test-threads=1 never reproduces it (10/10 clean), confirming it is a parallel-test race, not a real defect in the settings merge logic.

Root cause (as far as diagnosed)

Several other tests in the same file (untrusted_project_cannot_redirect_a_builtin_credential, untrusted_project_cannot_enable_tools_or_replace_an_agent_prompt, untrusted_project_may_narrow_trusted_tool_grants, and the migration tests around lines 800-1040) legitimately mutate the process-global STELLA_MANAGED_SETTINGS env var to point at a scratch fixture, guarded by crate::test_env::lock() + crate::test_env::EnvRestore.

enable_recap_survives_the_scope_merge (tests.rs:1079) and create_worktrees_survives_the_scope_merge (tests.rs:1119) call Settings::load(workspace) directly — they don't touch STELLA_MANAGED_SETTINGS and don't take test_env::lock() at all. Settings::load still reads the ambient STELLA_MANAGED_SETTINGS from the real process environment. Because these two tests never opt into the lock, they can run concurrently with a locked test mid-mutation (i.e. after set_var but before its EnvRestore guard drops), and briefly observe a STELLA_MANAGED_SETTINGS pointing at another test's tempdir fixture — which then fails stella's "managed settings must be a single-link regular file owned by root/process-user, not group/other-writable" security check because that fixture wasn't set up to satisfy it.

test_env::lock() only serializes tests that opt in; it provides no protection to a bystander test that reads the same ambient global without joining the lock.

Fix

Either:

  1. Have enable_recap_survives_the_scope_merge and create_worktrees_survives_the_scope_merge take crate::test_env::lock() (read-only participation is enough to be serialized against the mutating tests), or
  2. Have them explicitly redirect STELLA_MANAGED_SETTINGS to a known-absent path via test_env::EnvRestore, matching the pattern used by the sibling tests in the same file (see crates/stella-cli/src/settings/tests.rs:707-1040 for the established pattern).

Option 1 is smaller and matches this file's stated convention ("Redirecting without touching the process environment" doc in crates/stella-cli/src/paths.rs explicitly reserves test_env::lock()/EnvRestore for "genuinely env-shaped fixtures, such as provider credential variables" — STELLA_MANAGED_SETTINGS is exactly that).

Verify

cargo test -p stella-cli --bin stella settings::enable_recap_survives_the_scope_merge settings::create_worktrees_survives_the_scope_merge -- --test-threads=8

run in a loop (~20x) should be 100% green before and after; the actual regression test is running the whole settings:: module a handful of times under default parallelism and confirming zero flakes where today it fails roughly 1 in 3-4 runs.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions