Skip to content

Commit 537e367

Browse files
author
bgagent
committed
fix(pr): reviewer comment
1 parent a9d4c6f commit 537e367

2 files changed

Lines changed: 40 additions & 12 deletions

File tree

agent/src/attachments.py

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,25 @@ def download_attachments(
6161
s3_client = boto3.client("s3")
6262
prepared: list[PreparedAttachment] = []
6363

64-
for att in attachments:
65-
local_path = _download_single(att, attachments_dir, s3_client)
66-
prepared.append(
67-
PreparedAttachment(
68-
attachment_id=att.attachment_id,
69-
type=att.type,
70-
content_type=att.content_type,
71-
filename=att.filename,
72-
local_path=str(local_path),
73-
size_bytes=att.size_bytes,
74-
token_estimate=att.token_estimate,
64+
try:
65+
for att in attachments:
66+
local_path = _download_single(att, attachments_dir, s3_client)
67+
prepared.append(
68+
PreparedAttachment(
69+
attachment_id=att.attachment_id,
70+
type=att.type,
71+
content_type=att.content_type,
72+
filename=att.filename,
73+
local_path=str(local_path),
74+
size_bytes=att.size_bytes,
75+
token_estimate=att.token_estimate,
76+
)
7577
)
76-
)
78+
except Exception:
79+
import shutil
80+
81+
shutil.rmtree(attachments_dir, ignore_errors=True)
82+
raise
7783

7884
log("TASK", f"Downloaded {len(prepared)} attachment(s) to {attachments_dir}")
7985
return prepared

agent/tests/test_attachments.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,25 @@ def test_multiple_attachments_all_verified(self, mock_client, tmp_path):
179179
assert len(result) == 2
180180
assert result[0].filename == "file1.txt"
181181
assert result[1].filename == "file2.txt"
182+
183+
@patch("boto3.client")
184+
def test_partial_failure_cleans_up_attachments_dir(self, mock_client, tmp_path):
185+
"""When download fails mid-loop, already-downloaded files are removed."""
186+
config_ok, content_ok = _make_config(b"good content", "good.txt", "ATT001")
187+
config_bad, _ = _make_config(b"bad content", "bad.txt", "ATT002")
188+
189+
mock_s3 = MagicMock()
190+
mock_client.return_value = mock_s3
191+
# First attachment succeeds, second returns tampered content (checksum mismatch)
192+
mock_s3.get_object.side_effect = [
193+
{"Body": MagicMock(read=lambda: content_ok)},
194+
{"Body": MagicMock(read=lambda: b"tampered")},
195+
]
196+
197+
attachments_dir = tmp_path / ATTACHMENTS_DIR
198+
199+
with pytest.raises(RuntimeError, match="integrity check failed"):
200+
download_attachments([config_ok, config_bad], str(tmp_path))
201+
202+
# The entire .attachments directory should be cleaned up
203+
assert not attachments_dir.exists()

0 commit comments

Comments
 (0)