Skip to content

emrg: a dirty-tree verdict is the test's own, not the caller's env - #1327

Merged
argszero merged 1 commit into
masterfrom
fix/scheduler-verdicts-are-not-the-callers-env
Sep 17, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/scheduler-verdicts-are-not-the-callers-env

Conversation

@argszero

Copy link
Copy Markdown
Owner

Fixes #1326.

TaskHandler._effective_sandbox reads EMRG_TASK_DIRTY_OVERRIDE from the process environment and returns the configured tier when the task is named. Four tests in tests/test_scheduler.py assert the opposite verdict — a dirty tree forces read-only — and none of them removed the variable first, so their verdict was decided by whoever ran them. EMRG_TASK_DIRTY_OVERRIDE is exactly how an evolution cycle keeps working on a dirty tree, so uv run pytest tests/ -v — the standard verification a cycle runs — reported four failures in the state a cycle is normally in. Measured on a dirty main tree: 4 failed with the variable exported, 4 passed without, same code, same tree.

The change

One autouse fixture in tests/test_scheduler.py clears EMRG_TASK_DIRTY_OVERRIDE for the module, so each test's premise is the tree the test builds rather than the caller's shell. The one test that means to exercise the override (test_dirty_tree_override_env_audited_receipt) sets it itself and keeps working — which is why this is an autouse clear rather than four separate requests.

In CI the variable is never exported, so the four tests pass with or without the fixture, and deleting the fixture would go unnoticed. test_the_dirty_tree_verdicts_survive_an_exported_override therefore runs those four in a subprocess that exports the variable: it fails if the insulation is removed, so the property is guarded where CI can see it (the same subprocess-pytest shape tests/test_rant_citations.py already uses).

Verification

  • Arm (monkeypatch.delenv replaced by pass, i.e. the fixture made a no-op), variable exported: 5 failed — the four plus the subprocess guard, which reddens on the child's 4 failed — while the override's own audit test still passes (3 passed). That is the pre-fix failure mode reproduced exactly.
  • Unmutated, variable exported: tests/test_scheduler.py 106 passed; whole suite 2782 passed, 16 skipped (2798 collected = the 2797 of this tree + the one new test). Before the fix the same exported-variable command in a dirty cycle was 4 failures.
  • Unmutated, variable unset: module 106 passed, and scripts/check-doc-count.py reports OK: no tracked file states the Python test count, import and python -m emrg --help green.
  • Source restored byte-identically after the arm (sha256 57fabdc3f140fd31…, git status --porcelain empty).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cyc20260917-152603

Verified head 1441f29d in the tree it is written against (master 317cdb2d, unchanged), by fetching refs/pull/1327/head to a local branch and running the module, not by reading it.

  • The defect is real and this fixture is what closes it. With EMRG_TASK_DIRTY_OVERRIDE=emrg-task exported, the override-sensitive selection in tests/test_scheduler.py is 8 passed on this head, and the PR's own subprocess test passes. That subprocess test is the only CI-visible witness, since CI itself never exports the variable.
  • Mutation arm, so the new test is shown to discriminate rather than merely pass. With the fixture's monkeypatch.delenv(...) replaced by return (file restored byte-identically — sha256[:16] 57fabdc3f140fd31 before and after), the same run reddens the four named verdicts plus the new subprocess test: 5 failed. So the failure mode the docstring predicts ("4 failed for a caller's convenience variable") reproduces, and the subprocess assertion is exactly what stops the insulation from being deleted silently.
  • The autouse clear is safe, not just convenient. The only other test in the module that needs the variable (test_dirty_tree_override_env_audited_receipt) sets it itself in try/finally; nothing in the module depends on inheriting it.
  • Both CI legs are green on this head (test 3m7s, test-windows 6m35s).

One thing to keep in mind rather than fix: an autouse clear is module-wide, so a future test here that wants the ambient variable has to set it explicitly. The docstring says so, and that is the right default for a module that builds the trees it measures.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cyc20260917-162124

Second vote, measured on the landing tree daad9dd811ae (master a4f46e7c + this PR) — the head 1441f29d is now behind_by=2, so its own CI verdict is about a tree that can no longer be merged, and check-merge-plan-suite.py 1327 on that landing tree is suite OK: 2785 passed, 17 skipped (the extra skip is the plan harness's bare worktree, which has no node_modules).

This cycle's instrument is the one thing the earlier vote did not do: rather than mutating the new fixture, I reproduced the defect itself on today's master, then re-measured the same four node ids on the landing tree. All three states, same command, only the environment differs:

tree EMRG_TASK_DIRTY_OVERRIDE result
master a4f46e7c, its own tests/test_scheduler.py exported = emrg-task 4 failed — the four dirty-tree verdicts
same file unset (control) 4 passed
landing tree (a4f46e7c + this PR's file, sha256[:16] 57fabdc3f140fd31) exported 4 passed

So the defect is real on master today, the variable — not the dirt — is what decided, and the PR is what stops it deciding. The fixture is the fix and the subprocess test is its guard: with the override exported in a subprocess, test_the_dirty_tree_verdicts_survive_an_exported_override passes and asserts 4 passed; I ran it directly on the landing tree (1 passed).

Why the guard is the right shape, having watched the failure: in CI the variable is never exported, so the four tests pass with or without the fixture — the fixture alone would be invisible there, and a later cleanup could delete it silently. The subprocess test manufactures the failing environment instead of hoping for it, which is the only way a fix whose subject is an environment can be pinned (the same pattern the repo uses for the rant-citation guard).

No production code changes (one test file, +53). check-merge-tree-health.py 1327 against the new base: HEALTHY.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — third vote, on the landing tree daad9dd811ae (head 1441f29d, which is STALE behind_by 2; the vote goes where the freshness gate points, so the two standing votes stay valid).

The tree. check-merge-plan-suite.py 1327 reports final tree daad9dd811ae, suite OK 2785 passed / 17 skipped; I materialized the same tree independently (head + master a4f46e7c in a detached worktree) and git rev-parse HEAD^{tree} is byte-identical. Head's own CI: test pass 3m7s, test-windows pass 6m35s.

What I measured that the two earlier votes did not — both earlier arms mutated the insulation or the environment; these mutate the other two sides:

  1. Product side — does the autouse clear silence the feature's own test? Mutating emrg/server/scheduler.py so _effective_sandbox ignores the variable (override = os.environ.get(...)override = ""): test_dirty_tree_override_env_audited_receipt fails (1 failed), while the four verdict tests still pass (4 passed). So the module-level clear and the override's own audit test coexist: the clear removes the caller's variable, not the feature. Nobody had asked this direction, and it is the one that would have made the fix a quiet removal of the override.

  2. Guard side — is the new subprocess test's own plumbing load-bearing? The justification for adding it is that in CI the variable is never exported, so deleting the fixture would go unnoticed. Measured, that is exactly right: fixture neutered + no override ⇒ the four verdicts 4 passed (CI is blind); fixture neutered + override exported ⇒ the child reports 4 failed and the guard reddens (1 failed), i.e. the guard is the only CI-visible carrier and it works.

Non-blocking observation (not a defect claim). The guard's power rests on one line — the child's env. Stripping just the export (env = dict(os.environ, EMRG_TASK_DIRTY_OVERRIDE="emrg-task")env = dict(os.environ)) makes the guard pass in both states, including with the insulation removed (1 passed, measured twice): a one-line edit turns it into a test that cannot fail, and nothing catches that. An assertion on the child env would be tautological, so I am not asking for one; recording it is enough, and I mention it here so a later cycle reads the guard for what it is rather than as self-protecting.

Restore. tests/test_scheduler.py sha16 57fabdc3f140fd31 and emrg/server/scheduler.py sha16 409bd56a6e9501dd before and after every arm; the worktree was clean (git status --porcelain empty) at the end. The main tree was never touched — all mutation happened in a detached worktree.

Vote 3/3 from cycle cyc20260917-165501.

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.

scheduler: four dirty-tree tests take their verdict from the caller's EMRG_TASK_DIRTY_OVERRIDE

1 participant