Skip to content

Commit ff01dbf

Browse files
committed
fix(e2b): count and warn on malformed porcelain records (#967)
Both reviewers caught the same leftover: `_parse_porcelain`'s `rejected` counter was never incremented. Deleting the C-unquoter removed the only thing that incremented it, and I left the variable and its docstring claim behind — so a malformed record was silently dropped, contradicting this PR's own "never silently dropped" framing for AC2. Real git cannot emit a record that isn't "XY PATH", so a malformed one means the sandbox's git is not git — precisely the case worth surfacing. Now warned, counted, and folded into the count emitted to the user alongside containment rejections.
1 parent 3ea1c11 commit ff01dbf

2 files changed

Lines changed: 35 additions & 0 deletions

File tree

codeframe/adapters/e2b/adapter.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,10 @@ def _parse_porcelain(stdout: str) -> tuple[list[str], int]:
396396
index += 1
397397
# "XY PATH" — exactly two status characters and a space.
398398
if len(entry) < 4 or entry[2] != " ":
399+
# Counted and warned, not dropped: real git cannot emit this,
400+
# so a malformed record means the sandbox's git is not git.
401+
rejected += 1
402+
logger.warning("Rejected malformed porcelain record: %r", entry)
399403
continue
400404
status, raw = entry[:2], entry[3:]
401405

tests/adapters/test_e2b_trust_boundary_967.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,37 @@ def test_a_filename_containing_the_rename_arrow_is_not_mangled(self, workspace):
245245
files, _ = adapter._download_changed_files(sbx, workspace, lambda *a, **k: None)
246246
assert files == ["a -> b.py"]
247247

248+
def test_a_malformed_record_is_counted_and_warned_not_dropped(self, workspace, caplog):
249+
"""AC2 applies to parse rejections too, not just containment ones."""
250+
import logging
251+
252+
with caplog.at_level(logging.WARNING):
253+
files, count = _download(_sbx("xx", " M ok.py", content="x"), workspace)
254+
255+
assert files == ["ok.py"]
256+
assert count == 1
257+
assert any("xx" in r.getMessage() for r in caplog.records), caplog.text
258+
259+
def test_the_parse_reject_count_reaches_the_user(self, workspace):
260+
from codeframe.adapters.e2b.adapter import E2BAgentAdapter
261+
262+
emitted: list[str] = []
263+
E2BAgentAdapter(timeout_minutes=5)._download_changed_files(
264+
_sbx("xx", "y", " M ../outside", content="x"),
265+
workspace,
266+
lambda kind, msg, *a: emitted.append(msg),
267+
)
268+
# 2 unparseable + 1 escaping the workspace
269+
assert any("3" in m and "reject" in m.lower() for m in emitted), emitted
270+
271+
def test_parse_rejects_are_reported_separately_from_containment(self):
272+
"""_parse_porcelain's own return value must mean something."""
273+
from codeframe.adapters.e2b.adapter import E2BAgentAdapter
274+
275+
paths, rejected = E2BAgentAdapter._parse_porcelain("xx\0 M ok.py\0y\0")
276+
assert paths == ["ok.py"]
277+
assert rejected == 2
278+
248279
def test_porcelain_is_requested_nul_separated(self, workspace):
249280
"""-z is what removes the separator ambiguity above."""
250281
sbx = _sbx(" M ok.py", content="x")

0 commit comments

Comments
 (0)