Merge pull request #1250 from ouroboros-agent/claude/snapshot-lock-20260923

Keep the private snapshot's index consistent with the copied bytes (Windows follow-up to #1247)

.gitignore

@@ -101,3 +101,6 @@ MagicMock/
 # make_fixtures_v2.py; committing them would pin a snapshot the benchmark
 # deliberately does not measure, and leaving them untracked dirties the seed gate.
 devtools/benchmarks/editbench/fixtures_v2/
+
+# Other
+.devcontainer

docs/architecture/06-agent-core.md

@@ -331,7 +331,7 @@ WHERE a mutating run's changes are destined is the second, separate record — t
 
 A payload target gets a standalone private Git snapshot (`subagent_worktrees.provision_payload_snapshot`): the live payload is never initialized as Git, and capture trusts nothing under the child-writable snapshot's `.git`. Disposition (`integrate_payload_patch`) applies a live, index-free `git apply` in no-repository mode under a whole-payload content-hash CAS (drift = typed conflict; identical content = idempotent applied); `GIT_CEILING_DIRECTORIES` is pinned at the payload's resolved PARENT (git still searches the ceiling entry itself, and an ancestor Git worktree above the runtime data root could otherwise make git skip every hunk at rc=0), and reserved paths refuse the WHOLE apply as `blocked_reserved_paths` with the candidate preserved. The post-apply outcome set is complete: a live loader hash equal to the recorded RESULT hash is the success; a hash equal to the recorded BASELINE hash with a non-empty touched set is a provable non-mutation that RESOLVES the apply intent (typed `INTEGRATE_APPLY_NO_OP`, the `apply_no_op` arm: no success, no disposition, no reconcile queued, retry lane open); anything else is the ambiguous mismatch, whose intent stays PENDING and whose reconcile marker IS queued (the payload did mutate). A successful apply queues the extension reconcile (`request_extension_reconcile`) and the skill's review goes STALE pending fresh `skill_preflight`/`skill_review`; a run whose ONLY change is a mode flip is already refused at CAPTURE time as `unreviewable_metadata_change`, so a live hash still equal to the baseline means nothing was written.
 
-**A mutating run normally executes in a PRIVATE EXECUTION SNAPSHOT** (metered children keep sharing the tree; their patches integrate through `integrate_subagent_patch`: sha256-bound, 3-way `--index`, protected-path gated, genesis refused, `coop_already_in_tree` a no-op). At `delegate_start` the host snapshots tracked, staged and eligible untracked state, deciding sensitive/credential vetoes BEFORE hashing (`git add -A` would put secrets such as `.env` in the shared object database). Git's binary verdict for the whole untracked inventory comes from one index-versus-worktree `git diff --numstat` over a scratch index (two when empty files need their attribute verdict; `workspace_patch_capture.untracked_binary_verdicts`, shared with patch capture; a failed batch falls back to the per-file verdict with a warning), never one process per file by design. The machine-wide worktree ops lock (`subagent_worktrees._ops_lock`) guards SHARED metadata only — the registry file, a target's `.git/worktrees` and its baseline pin — held twice, briefly (row FIRST, then the ref, then `worktree add --no-checkout`, so a crash after the row is GC-nameable); the acting `self_worktree` lane is split the same way (`worktree add --no-checkout -b` + branch under the lock, `reset --hard --no-recurse-submodules` and deletion outside it — the body's own `post-checkout` hook no longer fires at provision either), and only the boot-time `prune_orphans` sweep and the millisecond genesis `git init` still do their work under it; listing, classifying, hashing, populating (`reset --hard --no-recurse-submodules`: what `worktree add` runs internally, minus the target's `post-checkout` hook) and copying run outside it (issue #1241: one 67k-file provision held the lock 40 minutes and every other mutating start timed out). A held lock refuses typed (`cause: lock_busy` + the holder's pid/task/op), a SIGKILLed holder is evicted by the owner-aware stale check, every refused snapshot provision is `definitely_unrun` with a durable `START_FAILED` row, and the start receipt discloses `snapshot` size/time. Disclosed residuals: the registry read-modify-write under the lock is O(registered rows, their per-file baseline maps included — moving those maps out of the row is a follow-up), and every eligible untracked text file is hashed into the TARGET's own object database, where it stays unreachable after the snapshot is removed until that repository's own gc. The baseline is pinned by `refs/ouroboros/delegated/` and checked out as a detached worktree; `scope.root` stays the authority target and `execution.workspaceRoot` names the snapshot. The host appends a separate typed binding after the immutable work order: the snapshot is writable and the authority is read-only until integration. Directory-copy runs use the engine-created copy and never the source folder. Full native access has no filesystem sandbox; the binding names the writable root but does not enforce it. Terminal capture records authority drift as evidence: ready-no-changes stays no-change with unknown authorship, while ready-with-changes keeps its private artifact and the locked baseline proof decides integration. Nested Git directories are excluded and disclosed; skill payloads use content-hash CAS. The binding is durable before POST and retry replays it; a GC-collected snapshot is a typed `execution_snapshot_missing` refusal. Worktrees live in `state/subagent_worktrees.json`; removal is explicit or custody-cross-checked startup GC, fail-closed on unreadable custody. The run still uses `live` from the engine view, so the scoped-HOME/`delegated` marker below applies.
+**A mutating run normally executes in a PRIVATE EXECUTION SNAPSHOT** (metered children keep sharing the tree; their patches integrate through `integrate_subagent_patch`: sha256-bound, 3-way `--index`, protected-path gated, genesis refused, `coop_already_in_tree` a no-op). At `delegate_start` the host snapshots tracked, staged and eligible untracked state, deciding sensitive/credential vetoes BEFORE hashing (`git add -A` would put secrets such as `.env` in the shared object database). Git's binary verdict for the whole untracked inventory comes from one index-versus-worktree `git diff --numstat` over a scratch index (two when empty files need their attribute verdict; `workspace_patch_capture.untracked_binary_verdicts`, shared with patch capture; a failed batch falls back to the per-file verdict with a warning), never one process per file by design. The machine-wide worktree ops lock (`subagent_worktrees._ops_lock`) guards SHARED metadata only — the registry file, a target's `.git/worktrees` and its baseline pin — held twice, briefly (row FIRST, then the ref, then `worktree add --no-checkout`, so a crash after the row is GC-nameable); the acting `self_worktree` lane is split the same way (`worktree add --no-checkout -b` + branch under the lock, `reset --hard --no-recurse-submodules` and deletion outside it — the body's own `post-checkout` hook no longer fires at provision either), and only the boot-time `prune_orphans` sweep and the millisecond genesis `git init` still do their work under it; listing, classifying, hashing, populating (`reset --hard --no-recurse-submodules`: what `worktree add` runs internally, minus the target's `post-checkout` hook), copying the source's exact bytes over the checkout and one `update-index` re-recording their stat (a CRLF-converting checkout otherwise leaves every such file "modified" in the child's `git status`) run outside it (issue #1241: one 67k-file provision held the lock 40 minutes and every other mutating start timed out). A held lock refuses typed (`cause: lock_busy` + the holder's pid/task/op), a SIGKILLed holder is evicted by the owner-aware stale check, every refused snapshot provision is `definitely_unrun` with a durable `START_FAILED` row, and the start receipt discloses `snapshot` size/time. Disclosed residuals: the registry read-modify-write under the lock is O(registered rows, their per-file baseline maps included — moving those maps out of the row is a follow-up), and every eligible untracked text file is hashed into the TARGET's own object database, where it stays unreachable after the snapshot is removed until that repository's own gc. The baseline is pinned by `refs/ouroboros/delegated/` and checked out as a detached worktree; `scope.root` stays the authority target and `execution.workspaceRoot` names the snapshot. The host appends a separate typed binding after the immutable work order: the snapshot is writable and the authority is read-only until integration. Directory-copy runs use the engine-created copy and never the source folder. Full native access has no filesystem sandbox; the binding names the writable root but does not enforce it. Terminal capture records authority drift as evidence: ready-no-changes stays no-change with unknown authorship, while ready-with-changes keeps its private artifact and the locked baseline proof decides integration. Nested Git directories are excluded and disclosed; skill payloads use content-hash CAS. The binding is durable before POST and retry replays it; a GC-collected snapshot is a typed `execution_snapshot_missing` refusal. Worktrees live in `state/subagent_worktrees.json`; removal is explicit or custody-cross-checked startup GC, fail-closed on unreadable custody. The run still uses `live` from the engine view, so the scoped-HOME/`delegated` marker below applies.
 
 At terminal, `delegate_wait` captures the run's diff against the baseline durably into the task's artifact store; NOTHING reaches the target automatically: the nanny explicitly applies or rejects through `integrate_delegated_patch`. Git and skill captures are whole-result operations: omitted `paths` and an exact empty list select the same captured result, disclosed when explicit; nonempty selectors stay directory-only. Engine-directory empty/subset semantics are unchanged. The staging substrate differs: a GIT workspace target applies under the repo git lock after PROVING no touched path drifted from `baseline_sha` (a plain `git apply` relocates hunks by offset; the touched-path set is read from `git apply --numstat` in BOTH directions, each naming only the paths it writes), then applies and STAGES, never commits. A SKILL-PAYLOAD target captures through the payload adapter over a parent-owned trusted index and applies LIVE into the non-Git payload: nothing is staged into any active root and no `.git` or index is created in the payload. The protected-path gate applies only when the target IS the Ouroboros body; a conflict (proven drift) is owned by the still-running nanny, with snapshot and patch persisting until explicit resolution or discard. Mutation rides an apply-intent protocol: a durable `delegate_run_patch_apply_started` row lands before any tree mutation, so on replay a pending intent without a disposition answers typed `INTEGRATE_DELEGATED_APPLY_AMBIGUOUS`, resolved only by explicit `acknowledge_ambiguous=true`, while the provably non-mutating outcomes (a lock error, proven baseline drift, a failed apply, a verified revert, a baseline-equal payload hash) RESOLVE the intent. `patch_verdict.py` is the ONE verdict writer for both pipelines: subjects are minted `run_<rid>` by the writer, never prefix-matched by readers, and each decision lands twice (artifact plus typed `delegate_run_patch_verdict` custody row), a failed artifact write disclosed on the row. `artifacts.delegated_capture_read_target` narrowly rebinds `artifact_store` READS for the owning task's own `delegated_runs/` prefix, and `delegate_shared.orphan_capture_read_target` extends the same read-only, one-directory READ to the terminal-owner ORPHAN the disposition rule authorizes (confirmed by `orphan_disposition_status`), so the actor that may dispose a patch can inspect it without wider write authority. A read-only child stays in Claudexor's default envelope: one transport with one derived difference, not a second pipeline.
 

ouroboros/subagent_worktrees.py

@@ -11,8 +11,9 @@ Mutations of SHARED metadata — a target's ``.git/worktrees`` (add/prune), its
 ``refs/ouroboros/delegated/*`` pins and the registry file — are serialized by a
 portable cross-process lock (the existing repo git lock is drive-root scoped,
 not ``.git`` scoped). Tree-proportional work never runs under it (#1241):
-listing, classifying, hashing, populating, copying and deleting a snapshot's
-files happen outside the lock, so one huge inventory delays only its own task.
+listing, classifying, hashing, populating, copying (and re-recording the copied
+bytes' stat) and deleting a snapshot's files happen outside the lock, so one
+huge inventory delays only its own task.
 """
 
 from __future__ import annotations
@@ -602,8 +603,9 @@ def provision_execution_snapshot(
     the baseline and create the worktree's admin dir — row FIRST, so everything
     after it is nameable by the startup GC — and once to finalize the row.
     Listing, classifying (one git process for every binary verdict), hashing,
-    populating and copying run OUTSIDE it, so a huge untracked inventory delays
-    only its own task instead of refusing every other mutating start.
+    populating, copying and the one ``update-index`` that re-records the copied
+    bytes' stat run OUTSIDE it, so a huge untracked inventory delays only its
+    own task instead of refusing every other mutating start.
     """
     from ouroboros.workspace_patch_capture import (
         binary_verdict_candidates, untracked_binary_verdicts, untracked_capture_veto_reason)
@@ -747,6 +749,7 @@ def provision_execution_snapshot(
         # source's actual working bytes, not checkout's CRLF/smudge rewrite.
         # Read the existing tree inventory so deletions, links and gitlinks
         # keep Git's semantics and excluded paths can never enter the copy.
+        copied: List[bytes] = []
         for item in manifest_raw.split(b"\0"):
             metadata, separator, raw_path = item.partition(b"\t")
             if separator and metadata.split()[0] in (b"100644", b"100755"):
@@ -755,6 +758,17 @@ def provision_execution_snapshot(
                 if original.is_symlink():
                     raise OSError(f"snapshot input changed from a regular file: {relative}")
                 copy_artifact_file(original, wt_path / relative)
+                copied.append(raw_path)
+        # The checkout recorded each entry's stat for the bytes IT wrote; a
+        # CRLF/smudge rewrite (core.autocrlf=true is Git for Windows' default)
+        # then differs in size from the copied source bytes, and git trusts a size
+        # mismatch as a modification without re-hashing — every such file would
+        # read as modified in the child's `git status` for the run's whole life.
+        # One update-index re-hashes the copied bytes through the same clean
+        # filters the baseline used (identical blob) and re-records their stat.
+        if copied:
+            _git_env(wt_path, "update-index", "-z", "--stdin", env=dict(os.environ),
+                     input_bytes=b"\0".join(copied) + b"\0")
         # A concurrent source edit must not appear as the child's work.
         # Use the same Git representation as ordinary patch capture, once
         # for the whole tree, before the separately tracked file inputs.

tests/test_reference_book_budgets.py

@@ -123,7 +123,10 @@ CHAPTER_BYTE_BUDGETS: dict[str, int] = {
     # verbs on a forked execution drive and the predecessor's task files as a lineage
     # read are new facts of the paragraphs they extend. The merged base sat 147 bytes
     # under the previous budget.
-    "docs/architecture/06-agent-core.md": 306800,
+    # 306800 -> 307100 (#1247 fix-forward; measured 306832 on the merged chapter): the
+    # populate sentence names the post-copy stat re-record that keeps a CRLF-converting
+    # checkout clean.
+    "docs/architecture/06-agent-core.md": 307100,
     "docs/architecture/07-configuration.md": 36991,
     # 18947 -> 19287: CI failure collection now documents diagnostic desktop builds while release remains gated.
     "docs/architecture/08-git-branching-ci-and-build.md": 19287,

tests/test_subagent_worktrees_lock_scope.py

@@ -22,7 +22,7 @@ import time
 import pytest
 
 from ouroboros import artifacts, subagent_worktrees as wt, workspace_patch_capture as capture
-from ouroboros.platform_layer import _lock_identity
+from ouroboros.platform_layer import _lock_identity, pid_is_alive
 from tests._delegated_transport_shared import _owned_gateway_uses_each_test_transport  # noqa: F401
 from tests.test_delegated_full_access import full_run  # noqa: F401
 from tests.test_delegated_run_isolation import _git, _nanny_ctx, _seed_target
@@ -147,32 +147,49 @@ def test_a_parked_provision_does_not_block_another(tmp_path, monkeypatch, same_t
 
 
 _HOLDER = """
-import os, pathlib, sys, time
-sys.path.insert(0, sys.argv[3])
+import os, pathlib, sys
+sys.path.insert(0, sys.argv[2])
 from ouroboros.platform_layer import acquire_exclusive_file_lock
 fd = acquire_exclusive_file_lock(
     pathlib.Path(sys.argv[1]), timeout_sec=5, stale_sec=600,
     metadata=f"pid={os.getpid()} task=t-holder op=provision since=2026-09-24T00:00:00Z target=/tmp/with space")
 assert fd is not None
-print("HELD", flush=True)
-time.sleep(float(sys.argv[2]))
+print("HELD", os.getpid(), flush=True)
+sys.stdin.readline()  # hold until the test closes our stdin (or kills us)
 """
 
 
-def _hold_lock(snaps: pathlib.Path, seconds: float) -> subprocess.Popen:
+def _hold_lock(snaps: pathlib.Path) -> "tuple[subprocess.Popen, int]":
+    """A live lock holder in another process; returns it with the pid the holder
+    itself wrote into the lock (on Windows a venv ``python.exe`` is a launcher whose
+    CHILD is the interpreter, so ``proc.pid`` is not that pid)."""
     snaps.mkdir(parents=True, exist_ok=True)
     proc = subprocess.Popen(
-        [sys.executable, "-c", _HOLDER, str(snaps / wt._LOCK_NAME), str(seconds), str(REPO)],
-        stdout=subprocess.PIPE, text=True)
-    assert proc.stdout.readline().strip() == "HELD"
-    return proc
+        [sys.executable, "-c", _HOLDER, str(snaps / wt._LOCK_NAME), str(REPO)],
+        stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True)
+    banner = proc.stdout.readline().split()
+    assert banner[:1] == ["HELD"], banner
+    return proc, int(banner[1])
+
+
+def _release_holder(proc: subprocess.Popen, holder_pid: int) -> None:
+    proc.kill()
+    proc.stdin.close()  # the interpreter behind a launcher exits on EOF too
+    proc.wait()
+    # Wait for the HOLDER (not only the launcher) to be gone: it may still hold
+    # the OS-level lock for a moment after EOF, and the dead-holder phase below
+    # rewrites the lock file by hand.
+    deadline = time.monotonic() + 10
+    while pid_is_alive(holder_pid) and time.monotonic() < deadline:
+        time.sleep(0.05)
+    assert not pid_is_alive(holder_pid)
 
 
 def test_a_live_holder_is_a_typed_refusal_and_a_dead_holder_is_evicted(tmp_path, monkeypatch):
     target = _seed_target(tmp_path)
     snaps, data = tmp_path / "snaps", tmp_path / "data"
     monkeypatch.setattr(wt, "_LOCK_TIMEOUT_SEC", 0.5)
-    holder = _hold_lock(snaps, 30)
+    holder, holder_pid = _hold_lock(snaps)
     try:
         started = time.monotonic()
         with pytest.raises(wt.WorktreeOpsLockBusy) as info:
@@ -181,10 +198,9 @@ def test_a_live_holder_is_a_typed_refusal_and_a_dead_holder_is_evicted(tmp_path,
         # refusal arrives after the one wait, not after a second discard wait.
         assert time.monotonic() - started < 3
     finally:
-        holder.kill()
-        holder.wait()
+        _release_holder(holder, holder_pid)
     busy = info.value
-    assert busy.holder == {"pid": str(holder.pid), "task": "t-holder", "op": "provision",
+    assert busy.holder == {"pid": str(holder_pid), "task": "t-holder", "op": "provision",
                            "since": "2026-09-24T00:00:00Z", "target": "/tmp/with space"}
     assert busy.waited_sec > 0 and "t-holder" in str(busy)
     # Nothing was registered, pinned or checked out for the refused attempt.
@@ -423,11 +439,20 @@ def test_a_failure_after_the_provisional_row_leaves_nothing_and_a_crash_is_gc_re
     assert not list(snaps.glob("dlg_*"))
 
 
-def test_populate_matches_worktree_add_and_runs_no_target_hook(tmp_path, monkeypatch):
+@pytest.mark.parametrize("autocrlf", [False, True])
+def test_populate_matches_worktree_add_and_runs_no_target_hook(tmp_path, monkeypatch, autocrlf):
     """``worktree add --no-checkout`` + ``reset --hard --no-recurse-submodules`` is what
     git's own ``worktree add`` runs — a target with ``submodule.recurse=true`` must
     still snapshot — minus the target's post-checkout hook, which no longer executes
-    project-authored code at provision."""
+    project-authored code at provision. Under ``core.autocrlf=true`` (Git for
+    Windows' default) the checkout writes CRLF and the raw-bytes copy restores the
+    source's LF, so the copied entries' stat must be re-recorded or every such file
+    reads as modified in the child's ``git status`` (CI on windows-latest, #1247)."""
+    # Both legs pin the setting explicitly: on windows-latest the system config
+    # already says true, so an unset "False" leg would not be the working side.
+    config = tmp_path / "gitconfig"
+    config.write_text(f"[core]\n\tautocrlf = {'true' if autocrlf else 'false'}\n", encoding="utf-8")
+    monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config))
     sub = tmp_path / "sub"
     sub.mkdir()
     _git(sub, "init", "-q")
@@ -457,6 +482,7 @@ def test_populate_matches_worktree_add_and_runs_no_target_hook(tmp_path, monkeyp
     assert (exec_root / "vendored").is_dir() and (exec_root / "tracked.txt").read_text(encoding="utf-8") == "one\ntwo\n"
     assert not (exec_root / ".hook_ran").exists() and not (target / ".hook_ran").exists()
     assert _git(exec_root, "status", "--porcelain").stdout == ""
+    assert (exec_root / ".gitmodules").read_bytes() == (target / ".gitmodules").read_bytes()
     assert "160000" in _git(exec_root, "ls-files", "-s", "vendored").stdout
 
     # The guard: without --no-recurse-submodules the populate fails on this target.
@@ -483,7 +509,7 @@ def test_snapshot_facts_ride_the_start_receipt_as_disclosure_only(tmp_path):
     assert _snapshot_facts(None) == {}
     # Facts, not a threshold: no runtime module consumes them to refuse or truncate.
     consumers = sorted(
-        str(path.relative_to(REPO)) for path in (REPO / "ouroboros").rglob("*.py")
+        path.relative_to(REPO).as_posix() for path in (REPO / "ouroboros").rglob("*.py")
         if "provisioning_sec" in path.read_text(encoding="utf-8"))
     assert consumers == ["ouroboros/subagent_worktrees.py", "ouroboros/tools/delegate.py",
                          "ouroboros/tools/delegate_integration.py"]