Skip to content

Commit feb086a

Browse files
committed
fix: keep per-file probe errors from aborting manifest resync loop
Move the is_symlink()/is_file() probes in _resync_manifest_after_registration() inside the per-file try so an OSError from either (e.g. an inaccessible path) is caught by the existing best-effort warning-and-continue handler instead of the outer handler, which was aborting the whole resync and leaving later files' hashes stale.
1 parent 0d31bec commit feb086a

2 files changed

Lines changed: 59 additions & 2 deletions

File tree

‎src/specify_cli/integrations/_helpers.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,9 +503,9 @@ def _resync_manifest_after_registration(
503503
changed = False
504504
for rel in new_manifest.files:
505505
abs_path = new_manifest.project_root / rel
506-
if abs_path.is_symlink() or not abs_path.is_file():
507-
continue
508506
try:
507+
if abs_path.is_symlink() or not abs_path.is_file():
508+
continue
509509
new_manifest.record_existing(rel)
510510
changed = True
511511
except (ValueError, OSError) as file_err:

‎tests/specify_cli/integrations/test_command_upgrade.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,63 @@ def fake_record_existing(self, rel_path, **kwargs):
14131413
assert reloaded["files"]["ok.md"] == ok_hash
14141414
assert reloaded["files"]["bad.md"] == stale_hash
14151415

1416+
def test_resync_manifest_probe_error_does_not_abort_remaining_files(
1417+
self, tmp_path, capsys
1418+
):
1419+
"""An ``OSError`` from the pre-rehash filesystem probes must warn
1420+
and continue, not abort the whole resync loop.
1421+
1422+
``is_symlink()``/``is_file()`` run before the per-file ``try`` that
1423+
wraps ``record_existing()``. If one of those probes raises (e.g. an
1424+
inaccessible path), it must not jump past the remaining files in
1425+
``new_manifest.files`` and leave their hashes stale.
1426+
"""
1427+
from specify_cli.integrations._helpers import (
1428+
_resync_manifest_after_registration,
1429+
)
1430+
from specify_cli.integrations.manifest import IntegrationManifest
1431+
1432+
project = tmp_path / "proj"
1433+
project.mkdir()
1434+
(project / "bad.md").write_text("bad content\n", encoding="utf-8")
1435+
(project / "ok.md").write_text("ok content\n", encoding="utf-8")
1436+
1437+
manifest = IntegrationManifest("claude", project, version="test")
1438+
manifest.record_existing("bad.md")
1439+
manifest.record_existing("ok.md")
1440+
manifest.save()
1441+
1442+
# Both files' bytes changed on disk after the manifest was saved
1443+
# (simulating registration overwriting them), but "bad.md" fails
1444+
# during the pre-rehash filesystem probe, not during rehashing.
1445+
(project / "bad.md").write_text("bad content v2\n", encoding="utf-8")
1446+
(project / "ok.md").write_text("ok content v2\n", encoding="utf-8")
1447+
1448+
real_is_file = Path.is_file
1449+
1450+
def fake_is_file(self):
1451+
if self.name == "bad.md":
1452+
raise OSError("permission denied")
1453+
return real_is_file(self)
1454+
1455+
import unittest.mock as mock
1456+
1457+
with mock.patch.object(Path, "is_file", fake_is_file):
1458+
_resync_manifest_after_registration(
1459+
manifest, "claude", continuing="Continuing."
1460+
)
1461+
1462+
captured = strip_ansi(capsys.readouterr().out)
1463+
assert "Warning:" in captured
1464+
assert "bad.md" in captured
1465+
1466+
reloaded = json.loads(manifest.manifest_path.read_text(encoding="utf-8"))
1467+
ok_hash = hashlib.sha256((project / "ok.md").read_bytes()).hexdigest()
1468+
assert reloaded["files"]["ok.md"] == ok_hash, (
1469+
"a probe error on an earlier file must not abort rehashing of "
1470+
"the remaining tracked files"
1471+
)
1472+
14161473
def test_upgrade_non_active_agent_preserves_active_agent_skills(self, tmp_path):
14171474
"""Upgrading a non-active agent must not touch the active agent's skills.
14181475

0 commit comments

Comments
 (0)