diff --git a/docs/architecture/01-high-level-architecture.md b/docs/architecture/01-high-level-architecture.md index 1f7e4c363..cf0f3de74 100644 --- a/docs/architecture/01-high-level-architecture.md +++ b/docs/architecture/01-high-level-architecture.md @@ -236,7 +236,7 @@ server.py (Starlette+uvicorn) ← HTTP + WebSocket on configurable host:port (de ├── subagent_messages.py ← Bounded durable child-message identity shared by the final frame, recovery, persistence and replay; `executor_observation_meta` validates task-bound progress actor facts ├── subagents.py ← Subagent envelopes + bounded legacy compatibility; `configured_subagent` snapshots dispatch through subagent_runtime ├── subagent_history.py ← Existing compact helper receipt: dated API attempts, session settlement/recovery and typed unrun starts; dynamic context and owner UI only, never admission (§6 Route health) - ├── subagent_worktrees.py ← Worktree lifecycle + durable registry `state/subagent_worktrees.json`; `provision_genesis_project` (never registry/GC); execution snapshots pinned by `refs/ouroboros/delegated/`; standalone payload snapshots; removal only explicit or custody-cross-checked startup GC, fail-closed on an unreadable custody log; the ops lock covers only registry/admin-dir/ref writes (issue #1241) (§6 Delegated subagents) + ├── subagent_worktrees.py ← Worktree lifecycle + durable registry `state/subagent_worktrees.json`; `provision_genesis_project` (never registry/GC); execution snapshots pinned by `refs/ouroboros/delegated/`; standalone payload snapshots; removal only explicit or custody-cross-checked startup GC, fail-closed on an unreadable custody log; for delegated snapshots the ops lock covers only registry/admin-dir/ref writes (issue #1241; the acting checkout/delete still runs under it) (§6 Delegated subagents) ├── artifacts.py ← Attachment staging into `artifact_store/attachments/`; artifact records; scratch fingerprints (`.scratch_manifest.json`) that gate patch exclusion only while content matches; the undeclared-output guard; `delegated_capture_read_target`; partial tool evidence first reads its verified actor source, then a matching redacted observability projection; recovered text uses the existing exact-text source writer best-effort, and a later budget cut without a verified handle remains source-unavailable ├── retention.py ← Unified GC retention SSOT: clamp/age-cutoff + legacy-key seed picker ├── workspace_preflight.py ← Read-only external-workspace git/manifest/toolchain snapshot used by gateway task creation diff --git a/docs/architecture/06-agent-core.md b/docs/architecture/06-agent-core.md index 04ab18755..41abf96a8 100644 --- a/docs/architecture/06-agent-core.md +++ b/docs/architecture/06-agent-core.md @@ -323,7 +323,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, because 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 because 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 because `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 (`workspace_patch_capture.untracked_binary_verdicts`, shared with patch capture), never one process per file. 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 for milliseconds (row FIRST, then the ref, then `worktree add --no-checkout`, so a crash after the row is GC-nameable); 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 pre-POST provisioning refusal is `definitely_unrun` with a durable `START_FAILED` row, and the start receipt discloses `snapshot` size/time. The baseline is pinned by `refs/ouroboros/delegated/` and checked out as a detached worktree; `scope.root` remains 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 because `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 (`workspace_patch_capture.untracked_binary_verdicts`, shared with patch capture), never one process per file. 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` checkout/delete is the one path still doing its tree work under it, bounded by the body's own size); 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` remains 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 remain 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, and the touched-path set is read from `git apply --numstat` in BOTH directions because each direction names 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_` by the writer, never prefix-matched by readers, and each decision lands twice — artifact plus typed `delegate_run_patch_verdict` custody row — with 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 widened write authority. A read-only child stays in Claudexor's default envelope — one transport with one derived difference, not a second pipeline. diff --git a/docs/development/06-rules-by-change-class.md b/docs/development/06-rules-by-change-class.md index 5f8f07272..1418a0da0 100644 --- a/docs/development/06-rules-by-change-class.md +++ b/docs/development/06-rules-by-change-class.md @@ -326,7 +326,9 @@ and 23 (`delegated_transport`), both critical. The imperatives: staged-never-committed) is unchanged (`tests/test_delegated_run_isolation_orphans.py`). A copy failure or a source change against the baseline leaves no registered snapshot or pinned - ref; no tree walk or per-file git process runs under the worktree ops lock + ref; on the delegated snapshot and payload paths no tree walk or per-file git + process runs under the worktree ops lock — the acting `self_worktree` + checkout/delete still does, bounded by the body's own size (`tests/test_snapshot_file_inputs.py`, `tests/test_subagent_worktrees_lock_scope.py`). - Outcome honesty: a delegating parent must not produce a clean no-tool final answer while direct children run undecided — one bounded absorption diff --git a/docs/inventories/DATA_LAYOUT_INVENTORY.md b/docs/inventories/DATA_LAYOUT_INVENTORY.md index ccfae73b4..d30bcfde2 100644 --- a/docs/inventories/DATA_LAYOUT_INVENTORY.md +++ b/docs/inventories/DATA_LAYOUT_INVENTORY.md @@ -2,7 +2,7 @@ Machine extraction of the `docs/ARCHITECTURE.md` "Data layout (`~/Ouroboros/`)" tree — the durable-file orientation carrier (this tree's counterpart of the reference PERSISTENCE_OWNERS derivation checklist) — regenerated by `python scripts/regenerate_inventories.py`. Do not edit. Every entry is probed against reality: repo entries must exist as tracked paths; data-plane entries must appear as a literal in the runtime sources that construct them. A durable file renamed or removed in code while its tree row survives = red (`tests/test_generated_inventories.py`). -Source: `docs/architecture/01-high-level-architecture.md`, physical LF lines 596-685; UTF-8 SHA-256 `73e06bc0d2f0d0c68930ca59a26f7436de44ada0e8c82b9e6043f49797f8b4e0`. +Source: `docs/architecture/01-high-level-architecture.md`, physical LF lines 596-685; UTF-8 SHA-256 `e2916a21ff25410a7825601414b1a8ee50f4aa8677eb680c628896430749349c`. - entries: **79** (code-ref: 72, repo-dir: 6, repo-path: 1) diff --git a/ouroboros/agent_dispatch.py b/ouroboros/agent_dispatch.py index 870b65ef8..f175a4343 100644 --- a/ouroboros/agent_dispatch.py +++ b/ouroboros/agent_dispatch.py @@ -222,6 +222,9 @@ def executor_blocked_outcome( "⚠️ EXECUTOR_UNAVAILABLE: this subagent was pinned to the delegated substrate " f"(executor='harness') and the route cannot run: {decision.reason}." + (f" It resets at {decision.reset_at}." if decision.reset_at else "") + # The producer's own words (a refused snapshot provision names the lock + # holder here), so the parent reads WHY, not only a reason code (#1241). + + (f" {availability['detail']}" if availability.get("detail") else "") + " The task was NOT run on metered API tokens, because that spend is exactly " "what the pin exists to prevent. Reschedule once the route recovers, or " "explicitly select another Available subagent." diff --git a/ouroboros/delegate_evidence.py b/ouroboros/delegate_evidence.py index 6ccb50c87..e066d7511 100644 --- a/ouroboros/delegate_evidence.py +++ b/ouroboros/delegate_evidence.py @@ -319,6 +319,8 @@ def acceptance_substrate_facts(ctx: Any, task_id: str) -> Dict[str, Any]: refusal = getattr(ctx, "_configured_startup_refusal", None) if isinstance(refusal, dict): out["startup_refused"] = str(refusal.get("reason") or "") + if refusal.get("detail"): + out["startup_refused_detail"] = str(refusal.get("detail")) return out diff --git a/ouroboros/subagent_bootstrap.py b/ouroboros/subagent_bootstrap.py index 11b1d1cbe..2d44ce9a7 100644 --- a/ouroboros/subagent_bootstrap.py +++ b/ouroboros/subagent_bootstrap.py @@ -5,7 +5,7 @@ from __future__ import annotations import json from hashlib import sha256 from pathlib import Path -from typing import Any, Mapping +from typing import Any, Dict, Mapping, Optional from ouroboros.subagent_history import snapshot_handle from ouroboros.subagent_work_order import compile_external_work_order @@ -84,10 +84,23 @@ def _startup_refusal_definite(payload: Mapping[str, Any]) -> bool: ) +# The typed facts a refused snapshot provision carries (`delegate_shared.lock_busy_facts`): +# they ride the refusal payload, the $0 terminal and the START_FAILED row unchanged. +_REFUSAL_FACT_KEYS = ("cause", "holder", "waited_sec", "retryable", "retry_hint") + + +def refusal_facts(payload: Mapping[str, Any]) -> Dict[str, Any]: + """The producer's typed refusal facts present on ``payload`` (never a handle).""" + return {key: payload[key] for key in _REFUSAL_FACT_KEYS if key in payload} + + def _record_startup_refusal( ctx: Any, task: Mapping[str, Any], *, reason: str, reset_at: str = "", + detail: str = "", facts: Optional[Mapping[str, Any]] = None, ) -> None: - """Stash the typed unrun refusal for the caller's zero-spend terminal.""" + """Stash the typed unrun refusal for the caller's zero-spend terminal — with + the producer's detail and facts (a busy lock's holder), so the parent-facing + outcome says WHY instead of a bare reason code.""" from ouroboros.subagent_runtime import current_subagent_alternatives from ouroboros.utils import utc_now_iso @@ -100,6 +113,8 @@ def _record_startup_refusal( "reason": str(reason or "configured_session_unavailable"), "reset_at": str(reset_at or ""), "requested": "harness", + "detail": str(detail or ""), + **dict(facts or {}), } availability = dict(task.get("subagent_availability") or {}) if isinstance( task.get("subagent_availability"), dict) else {} @@ -108,6 +123,8 @@ def _record_startup_refusal( "status": "unavailable", "reason": str(reason or "configured_session_unavailable"), "reset_at": str(reset_at or ""), + "detail": str(detail or ""), + **dict(facts or {}), "alternatives": alternatives, "host_fallback": False, "route_kind": "agent_session", @@ -245,6 +262,8 @@ def _pre_start_leaf( ctx, task, reason=str(payload.get("reason") or ""), reset_at=str(payload.get("reset_at") or ""), + detail=str(payload.get("detail") or ""), + facts=refusal_facts(payload), ) return "" # Everything else — started_uncustodied, fence refusals raced in by the diff --git a/ouroboros/subagent_worktrees.py b/ouroboros/subagent_worktrees.py index e8c1e1878..fe2edbb57 100644 --- a/ouroboros/subagent_worktrees.py +++ b/ouroboros/subagent_worktrees.py @@ -205,11 +205,12 @@ def _lock_holder(lock_path: Path) -> Dict[str, str]: @contextlib.contextmanager -def _ops_lock(root: Path, *, op: str, task_id: str = "", target: str = ""): +def _ops_lock(root: Path, *, op: str, task_id: str = "", target: str = "", + timeout_sec: Optional[float] = None): """Serialize SHARED-METADATA mutations in-process (threading.Lock) and across processes via the portable file-lock SSOT (platform_layer). - Held for milliseconds plus one registry read-modify-write — never for a tree + Held for milliseconds plus a registry read-modify-write — never for a tree walk (#1241). The holder names itself in the lock file, ``pid=`` first: the owner-aware stale check reads it, so a SIGKILLed holder is evicted at once instead of blacking out every waiter for ``_LOCK_STALE_SEC``; a live holder @@ -221,8 +222,8 @@ def _ops_lock(root: Path, *, op: str, task_id: str = "", target: str = ""): with _inproc_lock: started = time.monotonic() fd = acquire_exclusive_file_lock( - lock_path, timeout_sec=_LOCK_TIMEOUT_SEC, stale_sec=_LOCK_STALE_SEC, - metadata=metadata, owner_aware_stale=True) + lock_path, timeout_sec=_LOCK_TIMEOUT_SEC if timeout_sec is None else timeout_sec, + stale_sec=_LOCK_STALE_SEC, metadata=metadata, owner_aware_stale=True) if fd is None: raise WorktreeOpsLockBusy(lock_path, time.monotonic() - started, _lock_holder(lock_path)) try: @@ -315,17 +316,19 @@ def _unregister_snapshot(snapshot_id: str, data_dir: Optional[Any], op: str) -> def _discard_snapshot_checkout(target: Path, wt_path: Path, ref: str, snapshot_id: str, *, - root: Path, data_dir: Optional[Any], task_id: str) -> None: - """Undo a Git execution snapshot once its row and pin exist: the checkout's + root: Path, data_dir: Optional[Any], task_id: str, + lock_wait_sec: Optional[float] = None) -> None: + """Undo a Git execution snapshot once its row and pin may exist: the checkout's files go first, OUTSIDE the lock (a 1.8 GB tree takes minutes), then one short section forgets the admin dir, unpins the baseline and drops the row — the postcondition ``tests/test_snapshot_file_inputs.py`` asserts (no row, no ref, no ``dlg_*`` directory). Best-effort: the startup GC reconciles what a crash - leaves.""" + leaves. ``lock_wait_sec`` shortens the wait when the failure WAS a busy lock: + the same holder is still there, and the typed refusal must not wait twice.""" if _is_within(wt_path, root) and wt_path.exists(): _force_rmtree(wt_path) try: - with _ops_lock(root, op="discard", task_id=task_id, target=str(target)): + with _ops_lock(root, op="discard", task_id=task_id, target=str(target), timeout_sec=lock_wait_sec): _git_quiet(target, "worktree", "prune") _git_quiet(target, "update-ref", "-d", ref) _unregister_snapshot(snapshot_id, data_dir, "discard_execution_snapshot") @@ -366,18 +369,23 @@ def provision_worktree( root = _resolve_root(worktree_root) _assert_root_isolated(root, repo_dir, _data_dir(data_dir)) safe_task = _safe_name(task_id) + wt_path = (root / safe_task).resolve() + branch = f"{_BRANCH_PREFIX}{safe_task}" + # A stale checkout left by a crashed run is plain files: deleted OUTSIDE the + # lock (#1241); its admin dir and branch are replaced under it. + if _is_within(wt_path, root) and wt_path.exists(): + _force_rmtree(wt_path) with _ops_lock(root, op="worktree", task_id=str(task_id or ""), target=str(repo_dir)): if base_sha: _git(repo_dir, "rev-parse", "--verify", f"{base_sha}^{{commit}}") base_sha = _git(repo_dir, "rev-parse", base_sha).stdout.strip() else: base_sha = _git(repo_dir, "rev-parse", "HEAD").stdout.strip() - wt_path = (root / safe_task).resolve() - branch = f"{_BRANCH_PREFIX}{safe_task}" - # Clear any stale checkout/branch left by a crashed run. - _remove_paths(repo_dir, wt_path, branch, allowed_root=root) + _git_quiet(repo_dir, "worktree", "prune") + _git_quiet(repo_dir, "branch", "-D", branch) wt_path.parent.mkdir(parents=True, exist_ok=True) - _git(repo_dir, "worktree", "add", "--force", "-b", branch, str(wt_path), base_sha) + # Admin dir + branch under the lock; the checkout itself is populated below. + _git(repo_dir, "worktree", "add", "--no-checkout", "--force", "-b", branch, str(wt_path), base_sha) handle = WorktreeHandle( task_id=str(task_id), path=str(wt_path), @@ -402,7 +410,17 @@ def provision_worktree( # on every retry, without bound. _remove_paths(repo_dir, wt_path, branch, allowed_root=root) raise - return handle + try: + # Populate OUTSIDE the lock: what `worktree add` runs internally (no + # submodule recursion, no post-checkout hook of the body). + _git(wt_path, "reset", "--hard", "--quiet", "--no-recurse-submodules") + except Exception: + try: + remove_worktree(path=str(wt_path), worktree_root=root, data_dir=data_dir) + except Exception: + log.warning("Failed to discard acting worktree %s after a populate failure", wt_path, exc_info=True) + raise + return handle def provision_genesis_project( @@ -575,7 +593,8 @@ def provision_execution_snapshot( populating and copying 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 untracked_binary_verdicts, untracked_capture_veto_reason + from ouroboros.workspace_patch_capture import ( + binary_verdict_candidates, untracked_binary_verdicts, untracked_capture_veto_reason) target = Path(target_root).resolve() if not (target / ".git").exists(): @@ -635,7 +654,8 @@ def provision_execution_snapshot( file_inputs: List[str] = [] capture_warnings: List[Dict[str, Any]] = [] # git's binary verdict for the whole inventory in ONE process (#1241). - binary_verdicts = untracked_binary_verdicts(target, untracked, warnings=capture_warnings) + binary_verdicts = untracked_binary_verdicts( + target, binary_verdict_candidates(target, untracked), warnings=capture_warnings) from ouroboros.workspace_file_outputs import _side for rel in untracked: candidate = target / rel @@ -690,16 +710,19 @@ def provision_execution_snapshot( manifest_digest=manifest_digest, target_head=target_head, created_at=time.time(), entry_count=entry_count, excluded_untracked=tuple(excluded), untracked_baseline=untracked_baseline, capture_warnings=tuple(capture_warnings)) - with _ops_lock(root, op="provision", task_id=task, target=str(target)): - # Row FIRST, then the pin, then the admin dir: a crash after any of these - # leaves a REGISTERED snapshot custody never opened, which the startup GC - # removes (checkout, ref, row). A pin without a row would be invisible. - _git_quiet(target, "worktree", "prune") - _register_snapshot(ExecutionSnapshotHandle(**fields), excluded, data_dir) - _git(target, "update-ref", baseline_ref, baseline_sha) - wt_path.parent.mkdir(parents=True, exist_ok=True) - _git(target, "worktree", "add", "--detach", "--no-checkout", str(wt_path), baseline_sha) try: + with _ops_lock(root, op="provision", task_id=task, target=str(target)): + # Row FIRST, then the pin, then the admin dir: a crash after any of these + # leaves a REGISTERED snapshot custody never opened, which the startup GC + # removes (checkout, ref, row). A pin without a row would be invisible. + # The provisional row is LIGHT (no per-file maps): the GC needs only + # path, ref and snapshot id, and the maps are O(files) to serialize. + _git_quiet(target, "worktree", "prune") + _register_snapshot(ExecutionSnapshotHandle(**{**fields, "untracked_baseline": {}, + "excluded_untracked": ()}), [], data_dir) + _git(target, "update-ref", baseline_ref, baseline_sha) + wt_path.parent.mkdir(parents=True, exist_ok=True) + _git(target, "worktree", "add", "--detach", "--no-checkout", str(wt_path), baseline_sha) # Populate outside the lock: the same reset git's own `worktree add` runs # (no submodule recursion) — minus the target's post-checkout hook. _git(wt_path, "reset", "--hard", "--quiet", "--no-recurse-submodules") @@ -733,8 +756,9 @@ def provision_execution_snapshot( "file_baseline": file_baseline, "provisioning_sec": round(time.monotonic() - started, 3)}) with _ops_lock(root, op="provision", task_id=task, target=str(target)): _register_snapshot(handle, excluded, data_dir) - except Exception: - _discard_snapshot_checkout(target, wt_path, baseline_ref, snap, root=root, data_dir=data_dir, task_id=task) + except Exception as exc: + _discard_snapshot_checkout(target, wt_path, baseline_ref, snap, root=root, data_dir=data_dir, task_id=task, + lock_wait_sec=5.0 if isinstance(exc, WorktreeOpsLockBusy) else None) raise return handle @@ -1134,21 +1158,25 @@ def remove_worktree( match = entry break root = _resolve_root(worktree_root) - with _ops_lock(root, op="remove_worktree", task_id=str(task_id or "")): - if match is not None: - _remove_paths(Path(match.get("repo_dir") or "."), Path(match.get("path") or ""), match.get("branch") or "", allowed_root=root) - survivors = [ - e for e in _load_registry(data_dir, strict=True, op="remove_worktree") - if e.get("path") != match.get("path") - ] - _save_registry(survivors, data_dir) - return True + if match is None: # Unregistered path: best-effort directory removal, but ONLY inside the - # configured worktree root (never an arbitrary path supplied by a caller). + # configured worktree root (never an arbitrary path supplied by a caller); + # no shared metadata is involved, so no lock. if want_path and Path(want_path).exists() and _is_within(Path(want_path), root): _force_rmtree(Path(want_path)) return True - return False + return False + wt_path = Path(match.get("path") or "") + if str(wt_path).strip() and _is_within(wt_path, root) and wt_path.exists(): + _force_rmtree(wt_path) # the checkout's files go first, OUTSIDE the lock (#1241) + with _ops_lock(root, op="remove_worktree", task_id=str(task_id or "")): + _remove_paths(Path(match.get("repo_dir") or "."), wt_path, match.get("branch") or "", allowed_root=root) + survivors = [ + e for e in _load_registry(data_dir, strict=True, op="remove_worktree") + if e.get("path") != match.get("path") + ] + _save_registry(survivors, data_dir) + return True def prune_orphans( diff --git a/ouroboros/tools/delegate.py b/ouroboros/tools/delegate.py index 273ba39da..d10e66907 100644 --- a/ouroboros/tools/delegate.py +++ b/ouroboros/tools/delegate.py @@ -709,11 +709,13 @@ def _settle_refused_provision(ctx: ToolContext, gateway: Any, refusal: ToolResul invocation durably (START_FAILED), so the refusal exists outside this process — the incident's bootstrap refusals left no row at all (#1241).""" from ouroboros.delegate_shared import delegate_payload + from ouroboros.subagent_bootstrap import refusal_facts + payload = delegate_payload(refusal) _retire_orphaned_registration( ctx, gateway, "", definite_refusal=True, invocation_id=invocation_id, - reason=str(delegate_payload(refusal).get("reason") or "execution_snapshot_failed"), - history_facts=history_facts) + reason=str(payload.get("reason") or "execution_snapshot_failed"), + history_facts={**(history_facts or {}), **refusal_facts(payload)}) def _snapshot_facts(handle: Any) -> Dict[str, Any]: diff --git a/ouroboros/workspace_patch_capture.py b/ouroboros/workspace_patch_capture.py index e06dce81b..4fe623733 100644 --- a/ouroboros/workspace_patch_capture.py +++ b/ouroboros/workspace_patch_capture.py @@ -141,7 +141,7 @@ def write_workspace_patch_artifacts( # git's binary verdict for the whole inventory in ONE process (#1241); the loop # below keeps its per-file order and reads the verdict instead of spawning # ``git diff --numstat`` per file. - binary_verdicts = untracked_binary_verdicts(root, untracked, warnings=diagnostics) + binary_verdicts = untracked_binary_verdicts(root, binary_verdict_candidates(root, untracked), warnings=diagnostics) for rel in untracked: _want_sha = scratch_sha_by_rel.get(rel) or scratch_sha_by_abs.get(os.path.normcase(str((root / rel).resolve(strict=False)))) if _want_sha: @@ -705,6 +705,33 @@ def _untracked_blob_exclude_reason(root: pathlib.Path, rel: str, *, file_outputs return "" +def binary_verdict_candidates(root: pathlib.Path, rels: Sequence[str]) -> List[str]: + """The untracked paths that still need git's binary verdict. + + The per-file predicate decides the dotenv policy, the name rules, the PEM head + and the size cap BEFORE it ever asks git; the batch keeps that order, so a + vetoed or oversized file is never handed to git — its clean filters and + encodings run only over files that may become Git inputs, exactly as before.""" + from ouroboros.config import get_runtime_mode + from ouroboros.runtime_mode_policy import mode_has_unrestricted_agency + + unrestricted = mode_has_unrestricted_agency(get_runtime_mode()) + out: List[str] = [] + for rel in rels: + if not rel or _sensitive_untracked_reason(rel) or _patch_exclude_reason(rel): + continue + try: + info = os.lstat(root / rel) + except OSError: + continue + if not stat.S_ISREG(info.st_mode) or info.st_size > _PATCH_MAX_UNTRACKED_FILE_BYTES: + continue + if not unrestricted and pem_private_key_reason(root, rel): + continue + out.append(rel) + return out + + def untracked_binary_verdicts(root: pathlib.Path, rels: Sequence[str], *, warnings=None) -> Optional[Dict[str, bool]]: """git's own binary verdict for every path in ``rels`` from ONE ``git diff``. @@ -713,14 +740,17 @@ def untracked_binary_verdicts(root: pathlib.Path, rels: Sequence[str], *, warnin one index-versus-worktree ``git diff --numstat -z`` then runs exactly the machinery a per-file ``git diff --no-index --numstat`` ran — attributes, diff drivers, clean filters and working-tree encodings included — so the answer is git's, not a - re-implementation (parity probed on git 2.53 for 16 path classes). ``-\\t-`` is - binary; a text file, an empty file (absent from the diff) and a file that vanished - meanwhile (``0\\t0``) are text, exactly as the per-file verdict reads them. - Symlinks and other non-regular paths are text (git diffs the link text). One - inventory of tens of thousands of files therefore costs one process, not one per - file (#1241). ``None`` — the "did not batch" signal — when git cannot answer, after - an advisory warning: the callers keep the per-file verdict (the same answer, one - process per file), never a new refusal.""" + re-implementation (parity probed on git 2.53 for 19 path classes). ``-\\t-`` is + binary; a text file and a file that vanished meanwhile (``0\\t0``) are text, exactly + as the per-file verdict reads them. A path the diff OMITS is identical to the staged + blob — an empty file — which git still classifies by attribute (an empty ``*.dat`` + under ``-diff`` is binary): those are asked once more, staged as a one-byte blob, so + the empty-file set costs a second process, never one per file. Symlinks and other + non-regular paths are text (git diffs the link text). One inventory of tens of + thousands of files therefore costs one or two processes (#1241). ``None`` — the + "did not batch" signal — when git cannot answer, after an advisory warning: the + callers keep the per-file verdict (the same answer, one process per file), never a + new refusal.""" regular: List[str] = [] for rel in rels: try: @@ -731,37 +761,47 @@ def untracked_binary_verdicts(root: pathlib.Path, rels: Sequence[str], *, warnin if not regular: return {} env = dict(os.environ) - fd, scratch = tempfile.mkstemp(prefix="ouroboros-binary-verdict-", suffix=".index") - os.close(fd) - env["GIT_INDEX_FILE"] = scratch + scratch = "" def _git(*args: str, data: bytes = b"") -> bytes: return subprocess.run(["git", *args], cwd=str(root), capture_output=True, input=data, env=env, timeout=300, check=True).stdout - try: - empty_blob = _git("hash-object", "-w", "--stdin").strip() + def _diff_against(staged: bytes, paths: List[str]) -> Dict[str, bool]: + """One index-versus-worktree diff with every path staged as ``staged``: the + verdict of each path that produced a row (absent = identical to ``staged``).""" + blob = _git("hash-object", "-w", "--stdin", data=staged).strip() _git("read-tree", "--empty") _git("update-index", "-z", "--index-info", - data=b"".join(b"100644 " + empty_blob + b"\t" + os.fsencode(rel) + b"\0" for rel in regular)) - rows = _git("diff", "--numstat", "-z", "--no-renames", "--no-ext-diff", "--no-color") + data=b"".join(b"100644 " + blob + b"\t" + os.fsencode(rel) + b"\0" for rel in paths)) + seen: Dict[str, bool] = {} + for row in _git("diff", "--numstat", "-z", "--no-renames", "--no-ext-diff", "--no-color").split(b"\0"): + added, sep, rest = row.partition(b"\t") + deleted, sep2, path = rest.partition(b"\t") + if sep and sep2: + seen[os.fsdecode(path)] = added == b"-" and deleted == b"-" + return seen + + try: + fd, scratch = tempfile.mkstemp(prefix="ouroboros-binary-verdict-", suffix=".index") + os.close(fd) + env["GIT_INDEX_FILE"] = scratch + verdicts = _diff_against(b"", regular) + omitted = [rel for rel in regular if rel not in verdicts] + if omitted: # empty files: a second batch, staged as a one-byte blob + verdicts.update(_diff_against(b"\n", omitted)) except Exception as exc: if warnings is not None: warnings.append({"reason": "binary_verdict_batch_unavailable", "advisory": True, "detail": f"{type(exc).__name__}: {exc}"[:300]}) return None finally: - try: - os.unlink(scratch) - except OSError: - pass - verdicts = {rel: False for rel in regular} - for row in rows.split(b"\0"): - added, sep, rest = row.partition(b"\t") - deleted, sep2, path = rest.partition(b"\t") - if sep and sep2: - verdicts[os.fsdecode(path)] = added == b"-" and deleted == b"-" - return verdicts + if scratch: + try: + os.unlink(scratch) + except OSError: + pass + return {rel: verdicts.get(rel, False) for rel in regular} def untracked_capture_veto_reason(root: pathlib.Path, rel: str, *, file_outputs: Optional[List[str]] = None, diff --git a/tests/test_delegated_directory.py b/tests/test_delegated_directory.py index c43df7f83..ad46757b5 100644 --- a/tests/test_delegated_directory.py +++ b/tests/test_delegated_directory.py @@ -169,7 +169,8 @@ def test_a_git_workspace_treats_the_named_default_as_omission_and_still_refuses_ # differ by construction; every OTHER key and value must match, including the # key set itself — that is what "took the omitted path" means here. per_case = ("root", "execution_root", "snapshot_id", "baseline_sha", "baseline_id", - "baseline_manifest_read", "run_id", "invocation_id", "authority_target_root") + "baseline_manifest_read", "run_id", "invocation_id", "authority_target_root", + "snapshot") # the receipt's provisioning facts carry wall-clock seconds (#1241) compared = lambda payload: {key: ("" if key in per_case else value) for key, value in payload.items()} omitted, omitted_engine = _git_workspace_start(tmp_path, monkeypatch, "omit") diff --git a/tests/test_reference_book_budgets.py b/tests/test_reference_book_budgets.py index 2fc852efa..e61fda9fe 100644 --- a/tests/test_reference_book_budgets.py +++ b/tests/test_reference_book_budgets.py @@ -26,7 +26,8 @@ CHAPTER_BYTE_BUDGETS: dict[str, int] = { # extension_isolated_deps.py barrier and the widget_list.js request seam land # beside the handoff/schedule rows the base added; none displaces older text. # +400 (#1213): two new module rows (focus.py, room_consolidation.py) in the tree map. - "docs/architecture/01-high-level-architecture.md": 164800, + # 164800 -> raised for the subagent_worktrees module-map row (issue #1241 lock scope). + "docs/architecture/01-high-level-architecture.md": 165000, # 15517 -> 16200 (#1195): the session-custodied startup historical audit is a # new node of the startup flow (readiness no longer waits for the historical # seal diagnostic); the chapter had no older description of that pass to replace. @@ -96,10 +97,10 @@ CHAPTER_BYTE_BUDGETS: dict[str, int] = { # the digest-selected historical read and the reader admission rule. # 295650 -> 297250: document the new diagnostic-only source/coverage contract, # unavailable evidence and no-effects ordering without removing review/custody rules. - # 297250 -> 298400: the private-snapshot paragraph now states the worktree ops + # 297250 -> 299000: the private-snapshot paragraph now states the worktree ops # lock's scope (issue #1241: shared metadata only, row-then-ref order, batched # binary verdict, typed busy refusal) — rationale-layer text BIBLE P6 requires. - "docs/architecture/06-agent-core.md": 298400, + "docs/architecture/06-agent-core.md": 299000, "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, @@ -142,9 +143,9 @@ CHAPTER_BYTE_BUDGETS: dict[str, int] = { # never does; a pre-check's refusal takes the exact read). The one sentence it touches # (the lock's caller wait) is replaced; the rest is a rule the chapter lacked, and the # chapter had 5 bytes left. Sized to the text: 5 bytes of margin. - # 94520 -> 94650: the delegated-lane bullet names the worktree ops lock rule + # 94520 -> 94900: the delegated-lane bullet names the worktree ops lock rule # (issue #1241: no tree walk or per-file git process under the lock). - "docs/development/06-rules-by-change-class.md": 94650, + "docs/development/06-rules-by-change-class.md": 94900, "docs/development/07-managed-update-rule.md": 4166, "docs/development/08-mutation-attribution-rule.md": 2899, "docs/development/09-process-custody-rule.md": 10028, diff --git a/tests/test_subagent_worktrees_lock_scope.py b/tests/test_subagent_worktrees_lock_scope.py index d3537988a..c2efcde92 100644 --- a/tests/test_subagent_worktrees_lock_scope.py +++ b/tests/test_subagent_worktrees_lock_scope.py @@ -42,8 +42,14 @@ def _provision(target, snaps, data, snapshot_id="snap1", task_id="t1"): def _phase_spies(monkeypatch, snaps): """Record whether the lock was held when each phase ran.""" seen: dict = {} - real_verdicts, real_copy, real_git, real_save = ( - capture.untracked_binary_verdicts, artifacts.copy_artifact_file, wt._git, wt._save_registry) + real_verdicts, real_copy, real_git, real_git_env, real_save = ( + capture.untracked_binary_verdicts, artifacts.copy_artifact_file, wt._git, wt._git_env, wt._save_registry) + + def spy_git_env(repo_dir, *args, **kw): # baseline staging goes through _git_env, not _git + for marker in ("update-index", "write-tree", "commit-tree"): + if marker in args: + seen[marker] = _lock_held(snaps) + return real_git_env(repo_dir, *args, **kw) def spy_verdicts(root, rels, **kw): seen["classify"] = _lock_held(snaps) @@ -66,6 +72,7 @@ def _phase_spies(monkeypatch, snaps): monkeypatch.setattr(capture, "untracked_binary_verdicts", spy_verdicts) monkeypatch.setattr(artifacts, "copy_artifact_file", spy_copy) monkeypatch.setattr(wt, "_git", spy_git) + monkeypatch.setattr(wt, "_git_env", spy_git_env) monkeypatch.setattr(wt, "_save_registry", spy_save) return seen @@ -78,6 +85,7 @@ def test_tree_walk_runs_outside_the_lock_and_shared_metadata_inside(tmp_path, mo handle = _provision(target, snaps, data) assert seen["classify"] is False and seen["copy"] is False and seen["reset"] is False + assert seen["update-index"] is False and seen["write-tree"] is False and seen["commit-tree"] is False assert seen["update-ref"] is True and seen["worktree_add"] is True assert seen["save_registry"] == [True, True] # provisional row, then the final row assert not _lock_held(snaps) @@ -174,7 +182,7 @@ def test_a_live_holder_is_a_typed_refusal_and_a_dead_holder_is_evicted(tmp_path, busy = info.value 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.5 and "t-holder" in str(busy) + assert busy.waited_sec > 0 and "t-holder" in str(busy) # Nothing was registered, pinned or checked out for the refused attempt. assert wt.find_execution_snapshot("snap1", data_dir=data) is None assert _git(target, "for-each-ref", "refs/ouroboros/").stdout == "" @@ -247,6 +255,76 @@ def test_a_provisioning_refusal_leaves_a_durable_start_failed_row(full_run, monk assert failed[0]["reason"] == "execution_snapshot_failed" and failed[0]["run_id"] == "" +def test_configured_child_bootstrap_keeps_the_refusal_facts(tmp_path, monkeypatch): + """The host pre-start of a configured leaf (the incident's first refusal) ends the + child at $0 AND keeps the producer's facts: the $0 terminal names the lock holder, + the availability row and the acceptance evidence carry the detail.""" + from ouroboros import subagent_bootstrap, subagent_runtime + from ouroboros.agent_dispatch import executor_blocked_outcome + from ouroboros.delegate_shared import _fail, lock_busy_facts + from ouroboros.subagents import SubagentExecutorResolution + + busy = wt.WorktreeOpsLockBusy(tmp_path / "lock", 120.0, {"pid": "4242", "task": "t-other", "op": "provision"}) + refusal = _fail("delegate_start", "execution_snapshot_failed", + f"A private execution snapshot could not be provisioned ({busy}).", + definitely_unrun=True, **lock_busy_facts(busy)) + monkeypatch.setattr(subagent_runtime, "delegate_start_entry", lambda ctx, prompt, **kw: refusal) + monkeypatch.setattr(subagent_runtime, "current_subagent_alternatives", lambda selected: []) + + class _Ctx: + task_id = "t-child" + + ctx = _Ctx() + task = {"configured_subagent": {"selected_subagent_id": "codex=gpt-6-astra/xhigh"}} + wake = subagent_bootstrap._pre_start_leaf(ctx, task, {}) + + assert wake == "" # a definite refusal: the child ends unrun at $0, no model round + stash = ctx._configured_startup_refusal + assert stash["reason"] == "execution_snapshot_failed" and stash["cause"] == "lock_busy" + assert stash["holder"]["task"] == "t-other" and "t-other" in stash["detail"] + availability = task["subagent_availability"] + assert availability["status"] == "unavailable" and availability["holder"]["pid"] == "4242" + text, usage = executor_blocked_outcome( + SubagentExecutorResolution(requested="harness", executor="blocked", reason=stash["reason"]), + availability=availability) + assert "t-other" in text and usage["reason_code"] == "subagent_executor_unavailable" + + +def test_acting_worktree_add_and_remove_keep_tree_work_outside_the_lock(tmp_path, monkeypatch): + """The acting self_worktree lane follows the same split: admin dir + branch under + the lock, the checkout populated and deleted outside it.""" + target = _seed_target(tmp_path) + snaps, data = tmp_path / "snaps", tmp_path / "data" + seen: dict = {} + real_git, real_rmtree = wt._git, wt._force_rmtree + + def spy_git(repo_dir, *args, **kw): + if "worktree" in args and "add" in args: + seen["worktree_add"] = (_lock_held(snaps), "--no-checkout" in args) + if "reset" in args: + seen["reset"] = _lock_held(snaps) + return real_git(repo_dir, *args, **kw) + + def spy_rmtree(path): + seen["rmtree"] = _lock_held(snaps) + return real_rmtree(path) + + monkeypatch.setattr(wt, "_git", spy_git) + monkeypatch.setattr(wt, "_force_rmtree", spy_rmtree) + handle = wt.provision_worktree(repo_dir=target, task_id="acting1", worktree_root=snaps, data_dir=data) + assert seen["worktree_add"] == (True, True) and seen["reset"] is False + assert (pathlib.Path(handle.path) / "tracked.txt").read_text(encoding="utf-8") == "one\n" # HEAD content + assert _git(target, "rev-parse", "--verify", handle.branch).returncode == 0 + assert any(row.get("task_id") == "acting1" for row in wt.list_worktrees(data_dir=data)) + + assert wt.remove_worktree(task_id="acting1", worktree_root=snaps, data_dir=data) + assert seen["rmtree"] is False + assert not pathlib.Path(handle.path).exists() + assert _git(target, "rev-parse", "--verify", handle.branch, check=False).returncode != 0 + assert not any(row.get("task_id") == "acting1" for row in wt.list_worktrees(data_dir=data)) + assert not _lock_held(snaps) + + def test_removal_deletes_files_outside_the_lock_and_forgets_metadata_inside(tmp_path, monkeypatch): target = _seed_target(tmp_path) snaps, data = tmp_path / "snaps", tmp_path / "data" @@ -322,6 +400,12 @@ def test_populate_matches_worktree_add_and_runs_no_target_hook(tmp_path, monkeyp (hooks / "post-checkout").chmod(0o755) _git(target, "config", "core.hooksPath", str(hooks)) snaps, data = tmp_path / "snaps", tmp_path / "data" + # Positive control: the hook DOES fire on a plain `worktree add`, so its absence + # below is the populate path's doing, not a dead fixture. + control = tmp_path / "control-wt" + _git(target, "-c", "submodule.recurse=false", "worktree", "add", "--detach", str(control), "HEAD") + assert (control / ".hook_ran").exists() + _git(target, "worktree", "remove", "--force", str(control)) handle = _provision(target, snaps, data) diff --git a/tests/test_untracked_binary_verdict.py b/tests/test_untracked_binary_verdict.py index 2e76a09f9..5c7c3f346 100644 --- a/tests/test_untracked_binary_verdict.py +++ b/tests/test_untracked_binary_verdict.py @@ -49,12 +49,22 @@ def _fixture_repo(root: pathlib.Path) -> tuple[pathlib.Path, list[str]]: "nul.lfs": b"x\0y", # clean filter strips the NUL: text "text.u16": "hi\n".encode("utf-16"), # working-tree encoding: text "empty.md": b"", + "empty.dat": b"", # empty AND -diff: binary by attribute (grok triad finding) + "empty.bin": b"", # empty AND the binary macro + "empty.drv": b"", # empty AND a binary=true driver + "empty.drt": b"", # empty AND a binary=false driver: text "target.bin2": b"x\0y", - "new\nline.md": b"nl\n", # a newline in the name survives -z "vanish.md": b"gone\n", } + if os.name != "nt": # illegal in Win32 file names; the -z path is a POSIX fact + files["new\nline.md"] = b"nl\n" # a newline in the name survives -z for name, data in files.items(): (repo / name).write_bytes(data) + try: # a non-UTF-8 byte round-trips through fsencode; APFS and NTFS refuse such names + (repo / os.fsdecode(b"caf\xe9.md")).write_bytes(b"latin\n") + files[os.fsdecode(b"caf\xe9.md")] = b"latin\n" + except OSError: + pass os.symlink("target.bin2", repo / "link_to_bin") os.symlink("nowhere", repo / "dangling") rels = sorted(files) + ["link_to_bin", "dangling"] @@ -84,10 +94,12 @@ def test_batch_verdict_matches_git_for_every_path_class(tmp_path): # The classes that a NUL sniff or an attribute lookup alone would get wrong. assert verdicts["nonul.drv"] and not verdicts["nul.drt"] and not verdicts["nul.lfs"] and not verdicts["text.u16"] assert verdicts["edge7999.md"] and not verdicts["edge8000.md"] + assert verdicts["empty.dat"] and verdicts["empty.bin"] and verdicts["empty.drv"] + assert not verdicts["empty.drt"] and not verdicts["empty.md"] assert "link_to_bin" not in verdicts and "dangling" not in verdicts # symlinks: text, never followed - # Only the empty blob entered the target's object database: no content was hashed. + # Only the two staging blobs (empty and "\n") entered the target's object database. objects = _git(repo, "count-objects").stdout.decode() - assert objects.startswith("4 objects"), objects + assert objects.startswith("5 objects"), objects def test_capture_asks_one_process_and_never_one_per_file(tmp_path, monkeypatch): @@ -105,12 +117,44 @@ def test_capture_asks_one_process_and_never_one_per_file(tmp_path, monkeypatch): assert manifest["status"] == "ready_with_changes", manifest["errors"] numstat_calls = [c for c in calls if "--no-index" in c and "--numstat" in c] assert numstat_calls == [], "the per-file --no-index --numstat spawn is back" - assert sum(1 for c in calls if c[:2] == ["git", "diff"] and "--numstat" in c) == 1 + assert sum(1 for c in calls if c[:2] == ["git", "diff"] and "--numstat" in c) == 2 # inventory + empty-file pass excluded = {row["path"]: row["reason"] for row in manifest["untracked_excluded"]} expected_binary = {rel for rel in rels if _oracle(repo, rel)} assert {rel for rel, reason in excluded.items() if reason == "binary file"} == expected_binary +def test_vetoed_and_oversized_files_never_reach_git(tmp_path, monkeypatch): + """The batch runs git's clean filters and encodings; the per-file predicate never + asked git about a dotenv secret, a junk artifact or an over-cap file, so the + batch must not either (scope review, gpt-6-astra).""" + repo, _rels = _fixture_repo(tmp_path) + _git(repo, "config", "filter.observe.clean", "tee observed-filter-input") + (repo / ".gitattributes").write_text(".env filter=observe\n", encoding="utf-8") + _git(repo, "add", ".gitattributes") + _git(repo, "commit", "-qm", "observe") + (repo / ".env").write_text("SYNTHETIC_SECRET=fixture\n", encoding="utf-8") + (repo / "big.md").write_text("x" * 300, encoding="utf-8") + (repo / "junk.pyc").write_bytes(b"\x00junk") + monkeypatch.setattr(capture, "_PATCH_MAX_UNTRACKED_FILE_BYTES", 100) + monkeypatch.setenv("OUROBOROS_RUNTIME_MODE", "standard") + batched: list = [] + real = capture.untracked_binary_verdicts + + def spy(root, rels, **kw): + batched.extend(rels) + return real(root, rels, **kw) + + monkeypatch.setattr(capture, "untracked_binary_verdicts", spy) + _artifacts, manifest = capture.write_workspace_patch_artifacts(repo, tmp_path / "artifacts", task={}) + + assert manifest["status"] == "ready_with_changes", manifest["errors"] + assert ".env" not in batched and "big.md" not in batched and "junk.pyc" not in batched + assert "plain.dat" in batched and "nul.md" in batched + assert not (repo / "observed-filter-input").exists(), "an excluded file's bytes reached a clean filter" + reasons = {row["path"]: row["reason"] for row in manifest["untracked_excluded"]} + assert "size cap" in reasons["big.md"] and "junk" in reasons["junk.pyc"] + + def test_a_failed_batch_falls_back_to_the_per_file_verdict_with_a_warning(tmp_path, monkeypatch): repo, rels = _fixture_repo(tmp_path) real_run = subprocess.run @@ -129,3 +173,8 @@ def test_a_failed_batch_falls_back_to_the_per_file_verdict_with_a_warning(tmp_pa reason = capture.untracked_capture_veto_reason(repo, rel, binary_verdicts=None) assert (reason == "binary file") == _oracle(repo, rel), rel assert capture.untracked_binary_verdicts(repo, [], warnings=warnings) == {} + # Scratch-index allocation is inside the same guarded lifecycle. + monkeypatch.setattr(capture.tempfile, "mkstemp", lambda **kw: (_ for _ in ()).throw(OSError("no tmp"))) + warnings.clear() + assert capture.untracked_binary_verdicts(repo, rels, warnings=warnings) is None + assert warnings[0]["reason"] == "binary_verdict_batch_unavailable" and "no tmp" in warnings[0]["detail"]