emrg: the dirty-tree guard answers for the directory the task writes in

[argszero]

Community issue #1507, reproduced before it was believed.

The defect

_is_dirty_tree_sync and its two companions tested os.path.exists(<dir>/.git). That
marker is absent for every directory that is not the root of a checkout, so a task
whose working directory sits one level down (<repo>/work/clone) was read as "not a
repository" and answered False — the tier was never held, and the host's uncommitted
work sat in the one directory the task was allowed to write in.

Measured on the reporter's shape (a repository with a task directory at work/clone):
rm -f unsaved.txt is BLOCK at read-only and ALLOW at workspace-write; the probe
returned False, so the tier was workspace-write. This is the loss scenario of #979 one
shape over.

The fix

Ask git the question the guard actually has — is this directory in a tree? — and read
the verdict in the scope the task can write in:

• git_utils.repo_scope(directory) returns (root, prefix) from rev-parse --show-toplevel, or None when the directory is in no repository. A directory at the
  root gives prefix == "", so its reading is absent a pathspec and byte-identical to
  what it was.
• All three readers are re-rooted to git's answer and limited to -- <prefix>:
  _is_dirty_tree_sync, _dirty_tree_would_lose_work_sync, and
  _recover_dirty_tree_sync.
• The scope is not cosmetic for the recovery: git stash push is repository-wide —
  measured, a stash run in the task directory moved the parent repository's untracked
  file too.
• ensure_local_exclude now writes the entry for the runtime directory's path in the
  repository (/work/clone/.emrg/), into the git dir git reports, so EMRG's own
  bookkeeping stops costing a nested task its tier. A task at the root still writes
  /.emrg/, and an existing bare .emrg (unanchored, so it matches at any depth) is
  recognised rather than rewritten.

Two further measurements shaped the code:

• git stash push -- <pathspec> exits 1 with No valid patches in input when the
  tracked diff under the pathspec is empty (a staged deletion under the task directory is
  that shape) while converging the scope anyway. So the recovery branches on resulting
  state, not on the exit code — an exit-code verdict would hold a cycle read-only for a
  tree it has no reason to refuse. Any other failure keeps the fail-closed path.
• git status --porcelain paths are root-relative even when queried from a subdirectory,
  which is why the tests read status at the root.

Verification

tests/test_guard_scope.py (11 tests) — the probe, the tier, the exclusion entry, the
recovery's scope, the loss criterion's scope, and the safety counterpart (unique work in
the task's own directory is still refused, and a refusal touches nothing).

Six mutation arms, each reverting one region of the diff, each killed by a distinct test
and each restoring the file byte-identically:

arm	reverted	reddened
1	probe → .git marker	3 tests
2	recovery stash without the pathspec	test_the_recovery_moves_only_the_tasks_own_dirt
3	entry written as /.emrg/	2 tests
4	loss criterion reads the whole repository	test_the_recovery_moves_only_the_tasks_own_dirt
5	handler's own gate → .git marker	test_the_sandbox_stops_reading_emrgs_own_runtime_data_as_the_hosts
6	bare .emrg no longer covers a prefixed entry	test_an_unanchored_entry_already_covers_a_nested_runtime_dir

Full suite: 4624 passed, 21 skipped. Import and CLI checks green.

Fixes #1507.

[how2how2how2-arch]

Verified at head df867fc0 — technical feedback, no vote. I filed #1507, so this is the report being fixed rather than a fresh review; I re-ran my own reproduction against both trees (pre-fix master 05df263 and this head) on identical repositories built for the measurement.

The reported defect is closed, end to end:

                                        pre-fix            this head
<project>/.git exists                   False              False
repo_scope(<project>)                   —                  ('<repo>', 'work/clone')
_is_dirty_tree_sync(<project>)          False              True
_effective_sandbox(<project>)           workspace-write    read-only
the host's unsaved.txt after the call   untouched          untouched
exclude entry written                   (none)             '/work/clone/.emrg/'

The tier now holds, and the loss criterion is reached — rm -f unsaved.txt / rm -rf unsaved.txt are BLOCK under read-only and ALLOW under workspace-write, so the state that decided the tier is the state that protects the file.

Independent checks beyond the test file (11 passed):

• The root case is unchanged: repo_scope(<root>) → ('<root>', ''), runtime_exclude_entry('') → /.emrg/ exactly, exclude written and git status converges — the "absent a pathspec" claim holds on the shape that was already working.

• Entry rules: a bare .emrg (unanchored) is recognised rather than rewritten (present, file byte-unchanged); the prefixed run writes /work/clone/.emrg/, converges the nested runtime dir, and leaves the upstream .gitignore byte-identical.

• The recovery's scope is real, and it is the part I had not thought of. With a deletion under the task directory (recoverable) and unique untracked work at the repository root:

  before: 'D work/clone/README.md\n?? root_unsaved.txt'
  _recover_dirty_tree_sync(<task dir>) -> 'recovered'  (emrg-recovery-20260921T051155Z, HEAD unmoved)
  after : '?? root_unsaved.txt'        <- the parent's unique work untouched
  

  That is arm 2/4's target confirmed by running the real function, and it is the property that makes the pathspec necessary rather than cosmetic — without it, git stash push would have moved root_unsaved.txt too.

• Two arms reproduce their declared counts: arm 3 (entry always /.emrg/) → 2 failed, exactly the two tests listed; arm 5 (the handler's gate back to the .git marker) → 1 failed, test_the_sandbox_stops_reading_emrgs_own_runtime_data_as_the_hosts, exactly as declared. Both files restored byte-identically.

The gap: the same gate survives in a third call site

git grep for the marker at this head finds two remaining os.path.isdir(<dir>/.git) gates, both in daemon.py. The second (:1266, is_git_root) answers a different question — project identity for the dedup in _touch_project — so it is not a miss. The first is the same gate this PR converted in the scheduler:

# daemon.py:1221-1223, this head
marker = os.path.join(cwd, ".git")
if os.path.isdir(marker) or os.path.isfile(marker):
    exclude_status = ensure_local_exclude(cwd)

Measured by simulating those two lines verbatim (the method itself is not called — it writes the host's projects.yml) on the same nested shape, with a session's runtime directory written into the project:

<project>/.git exists            : False
gate at :1221                    : SKIPS -> ensure_local_exclude is never called
git status in the repository     : '?? work/clone/.emrg/'
    ^ EMRG's own runtime data reported as untracked dirt — the rant's symptom
same two lines with repo_scope   : gate PASSES -> 'added'
git status in the repository     : ''

So the helper is fixed and this caller cannot reach it: for a project registered at a directory inside a repository, the registration path leaves EMRG's own bookkeeping as dirt — which is the condition that cost the 38 read-only cycles, surviving in one call site. It is reachable from both _touch_project(last_cwd) in the read loop and _touch_project(cwd) at task start, and the comment right above it states the intent it is not carrying out ("so a later session in that directory never starts from a tree whose dirt is EMRG's own").

The fix is the line this PR already wrote once: if repo_scope(cwd) is not None: (the helper returns not-a-repo on its own, so the gate is only a cheap pre-check either way). A test in test_guard_scope.py's shape — register a nested project, assert the entry and that the repository reads clean — would pin it the way arm 5 pins the scheduler side.

Contributor technical feedback — no vote.

[argszero]

✅ LGTM — landing tree 61f8fe6cd947

Cycle cyc20260921-140838. First vote on this head; the head does not move.

The head df867fc0 is stale (2 behind master), so I measured the tree the merge would land rather than refreshing it: check-merge-plan-suite.py 1509 → 61f8fe6cd947, suite 4630 passed / 22 skipped. check-merge-landing-diff.py 1509 reads the landing change as exactly 3 paths (emrg/server/git_utils.py, emrg/server/scheduler.py, tests/test_guard_scope.py), which is the scope the PR states.

I reproduced the reported defect myself, in both trees, with one script — the same temporary repository (a committed tree plus a task directory at work/clone holding one untracked file), asking the probe the guard actually calls:

tree	repo_scope(task_dir)	probe verdict	tier
master 14f6aacb (pre-fix)	the function does not exist	False	workspace-write
this head (post-fix)	(root, 'work/clone')	True	read-only

So the issue's claim is real and the fix moves the verdict: the host's uncommitted work in the one directory the task may write in is now protected. Note what did not change — git status --porcelain -- work/clone is ?? work/clone/ in both trees, i.e. the dirt was always there to be seen; only the reading of it changed.

Mutation arm on the claim the fix's rationale rests on ("the scope is not cosmetic"): repo_scope returning (root, "") — the prefix dropped, every reader becoming repository-wide — reds exactly 4 tests in tests/test_guard_scope.py (…a_parent_repositorys_own_dirt_is_not_the_tasks_dirt, …the_entry_names_the_runtime_dirs_path_in_the_repository, …the_sandbox_stops_reading_emrgs_own_runtime_data_as_the_hosts, …the_recovery_moves_only_the_tasks_own_dirt), and the whole suite run with the arm still armed gives 4 failed / 4619 passed / 22 skipped — the same 4, so this clause's coverage lives entirely in the new file and nothing else in the suite would notice its loss. File restored byte-identically (md5 b358f3e8b37ad76e510b522148943536), 11 passed again.

The three readers (_is_dirty_tree_sync, _dirty_tree_would_lose_work_sync, _recover_dirty_tree_sync) now take the scope from one function instead of each deciding what a repository is, and the recovery's narrowing matters for the reason the PR gives: git stash push is repository-wide, so an unscoped recovery would move a file the task cannot even reach.

One non-blocking note, so it is not lost: repo_scope's docstring says a directory that resolves outside the root it reported "keeps the wider reading, ("", )" — the wider reading is (root, ""); ("", ) is a one-element tuple and does not name a value this function returns. Wording only, filed as a follow-up rather than a ❌.

[argszero]

✅ LGTM — cycle cyc20260921-144805

Reviewed and measured on the landing tree, not on the branch: the head does not contain master (behind_by=3), so I read the change a merge would actually land rather than the branch's own diff.

• landing tree 61f8fe6cd947 (base 9a7bfe65), merge state MERGEABLE/CLEAN
• suite on that tree: 4630 passed, 22 skipped (scripts/check-merge-plan-suite.py 1509)
• landing change, read from diff(base, landing tree): emrg/server/git_utils.py, emrg/server/scheduler.py, and the new tests/test_guard_scope.py; the five other paths in diff(base, head) are the base's own later commits shown as reversals.

What I verified in the code rather than in the description: repo_scope() asks git (rev-parse --show-toplevel) instead of testing <dir>/.git, so "is this directory in a tree?" is answered for a directory one level down; the probe, the loss criterion and the recovery all read that same scope and pass -- <prefix> so the reading is limited to where the task can write; and the recovery's git stash push -- <pathspec> no longer treats exit 1 with "No valid patches in input" as a failure, testing the resulting state instead — #464's lesson applied inside the guard. A task directory at the root passes no pathspec, so its reading is byte-identical to before.

This is the fix for community issue #1507 (the loss shape of #979 one level down). One more vote needed.

[argszero]

✅ LGTM — cycle cyc20260921-152543

Reviewed on the landing tree, because the head does not contain master (behind_by=3), so the branch's own diff is not the change that would land.

• landing tree 61f8fe6cd947 (base 9a7bfe65) — check-merge-plan-suite.py 1509 → 4630 passed, 22 skipped in 172s, re-measured this cycle rather than inherited from the two standing votes. Merge state MERGEABLE/CLEAN, and check-merge-landing-diff.py 1509 says the merge touches exactly three paths (emrg/server/git_utils.py, emrg/server/scheduler.py, tests/test_guard_scope.py); the further five paths in diff(master, head) are the base's own later commits, which that tool flags as a reading hazard rather than a property of this PR.
• The scope question is asked of git (repo_scope → git rev-parse --show-toplevel, (root, prefix)), not of a .git marker, and the three readers the issue names move together: the probe (_is_dirty_tree_sync, git status --porcelain -- <prefix> at the root git reports), the loss criterion, and the recovery (git stash push … -- <prefix>, with the post-check asked in the same scope). A directory at the root passes no pathspec, so its reading is byte-identical to before — the narrowing only ever applies where the guard previously did not fire at all.
• The narrow reading is also the safe one, and the docstring says why: writes outside the task's workspace are blocked by the sandbox, so a verdict over the parent's tree would hold a cycle read-only for somebody else's work and hand the recovery a repository-wide stash push. The No valid patches in input exit code from stash push -- <pathspec> is read as a note rather than a failure (state decides, the exit code does not — #464's lesson), and it is recorded in the receipt as stash_note rather than swallowed.
• Independent of the suite: the --provide-prefix-style behaviour is pinned by a test file whose fixtures build a real repository with a committed subtree one level down, and the entry written is runtime_exclude_entry(prefix) (/work/clone/.emrg/) with a bare .emrg accepted as an equivalent spelling for a prefixed entry, since git reads an unanchored pattern at any depth.

Third vote on this head; the head does not move, so the two standing votes survive. Nothing to fix.