Skip to content

Commit d99886e

Browse files
bai-uipathclaude
andcommitted
refactor(orchestrator): trim to interrupt-proof teardown; drop preservation-prune
Preservation-path pruning is covered downstream by coder_eval_uipath #57 and isn't reachable on current configs; keep this PR to the teardown fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1361c0b commit d99886e

4 files changed

Lines changed: 94 additions & 287 deletions

File tree

‎src/coder_eval/orchestrator.py‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,16 +2116,11 @@ async def _cleanup(self) -> None:
21162116
logger.info(f"Sandbox preserved to: {preserved_path}")
21172117
elif self.preservation_mode == PreservationMode.DIRECT_WRITE and self.result:
21182118
# Sandbox already lives in run_dir/artifacts — nothing to move.
2119-
# Set sandbox_path first, then prune capture-ignored entries
2120-
# (MOVE_ON_WRITE gets this inside preserve_to; DIRECT_WRITE
2121-
# never copies, so the raw workspace — agent-created
2122-
# .venv/node_modules with sandbox-only symlinks, credential
2123-
# stores — would otherwise persist in run_dir/artifacts and
2124-
# break artifact publishing), then grant a+rX (a fallible
2125-
# chmod) so artifacts written by a root-owned docker container
2126-
# stay traversable across the host uid boundary.
2119+
# Set sandbox_path first, then grant a+rX (a fallible chmod) so
2120+
# artifacts written by a root-owned docker container stay
2121+
# traversable across the host uid boundary (MOVE_ON_WRITE gets
2122+
# this via preserve_to; DIRECT_WRITE skips it, so apply it here).
21272123
self.result.sandbox_path = str(self.sandbox.sandbox_dir)
2128-
await asyncio.to_thread(self.sandbox.prune_preserved)
21292124
await asyncio.to_thread(self.sandbox.grant_read_access)
21302125
logger.info(f"Sandbox preserved (in-place): {self.sandbox.sandbox_dir}")
21312126
elif self.preservation_mode == PreservationMode.NONE and self.result:

‎src/coder_eval/sandbox.py‎

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -63,55 +63,6 @@
6363
)
6464

6565

66-
def _prune_capture_ignored(root: Path) -> list[str]:
67-
"""Remove :data:`_WORKSPACE_CAPTURE_IGNORE` entries from a preserved tree.
68-
69-
``capture_to`` filters these at copy time (``shutil.ignore_patterns`` applies
70-
at every directory level), but the other two preservation paths kept the raw
71-
workspace: ``preserve_to`` is a ``shutil.move`` and DIRECT_WRITE never copies
72-
at all. That leaks the same two classes the ignore tuple exists for — the
73-
credential-store names, and agent-created ``.venv``/``node_modules`` bulk
74-
whose ``bin/`` symlinks point at sandbox-only interpreter paths (dangling on
75-
the host, they break artifact publishing: Azure DevOps'
76-
PublishPipelineArtifact refuses ANY symlink). Walk the preserved tree with
77-
the same matcher and drop every hit, so preserved workspaces look the same
78-
regardless of which preservation path produced them.
79-
80-
Per-entry deletion failures are warning-logged and skipped — pruning is
81-
hygiene and must never fail a preservation that already succeeded. Returns
82-
the pruned paths relative to ``root`` (for the caller's log line).
83-
"""
84-
if not root.is_dir():
85-
return []
86-
matcher = shutil.ignore_patterns(*_WORKSPACE_CAPTURE_IGNORE)
87-
pruned: list[str] = []
88-
for dirpath, dirnames, filenames in os.walk(root, topdown=True):
89-
base = Path(dirpath)
90-
for name in sorted(matcher(dirpath, dirnames + filenames)):
91-
target = base / name
92-
try:
93-
# is_dir() follows symlinks: unlink a symlinked dir rather than
94-
# rmtree THROUGH it into content outside the preserved tree.
95-
if target.is_dir() and not target.is_symlink():
96-
shutil.rmtree(target)
97-
else:
98-
target.unlink(missing_ok=True)
99-
except OSError as exc:
100-
logger.warning("Failed to prune %s from preserved workspace: %s", target, exc)
101-
continue
102-
pruned.append(str(target.relative_to(root)))
103-
if name in dirnames:
104-
dirnames.remove(name) # deleted — don't descend into it
105-
if pruned:
106-
logger.info(
107-
"Pruned %d capture-ignored entrie(s) from preserved workspace %s: %s",
108-
len(pruned),
109-
root,
110-
", ".join(pruned),
111-
)
112-
return pruned
113-
114-
11566
def _grant_read_traverse(root: Path) -> None:
11667
"""Recursively apply ``chmod a+rX`` semantics under ``root``.
11768
@@ -1086,21 +1037,6 @@ def grant_read_access(self) -> None:
10861037
if self.sandbox_dir is not None and self.sandbox_dir.exists():
10871038
_grant_read_traverse(self.sandbox_dir)
10881039

1089-
def prune_preserved(self) -> list[str]:
1090-
"""Drop capture-ignored entries from the (preserved) sandbox tree, in place.
1091-
1092-
For DIRECT_WRITE preservation the sandbox already lives in the artifacts
1093-
dir, so neither ``capture_to``'s copy-time filter nor ``preserve_to``'s
1094-
post-move prune ever runs — agent-created ``.venv``/``node_modules``
1095-
(symlink-bearing bulk) and any credential-store names would persist in
1096-
``run_dir/artifacts`` verbatim. Called by the orchestrator's DIRECT_WRITE
1097-
cleanup arm so all three preservation paths agree on what a preserved
1098-
workspace contains. Returns the pruned paths (relative), for callers/tests.
1099-
"""
1100-
if self.sandbox_dir is None or not self.sandbox_dir.exists():
1101-
return []
1102-
return _prune_capture_ignored(self.sandbox_dir)
1103-
11041040
def preserve_to(self, artifact_dir: Path) -> Path:
11051041
"""Preserve sandbox contents to an artifact directory.
11061042
@@ -1134,12 +1070,6 @@ def preserve_to(self, artifact_dir: Path) -> Path:
11341070
old_sandbox_dir = self.sandbox_dir
11351071
shutil.move(str(old_sandbox_dir), str(preserve_path))
11361072

1137-
# Match capture_to's filtering: the move carried the raw workspace,
1138-
# including capture-ignored entries (credential stores; .venv/node_modules
1139-
# bulk whose symlinks break artifact publishing). Prune them from the
1140-
# preserved copy — before the a+rX grant, so we don't chmod doomed files.
1141-
_prune_capture_ignored(preserve_path)
1142-
11431073
# mkdtemp creates the sandbox root at 0700. Under driver:docker the
11441074
# container runs as root, so the preserved tree lands on the host
11451075
# bind-mount owned by root with that 0700 top dir -- the host user

‎tests/test_preserved_workspace_prune.py‎

Lines changed: 0 additions & 208 deletions
This file was deleted.

0 commit comments

Comments
 (0)