check: handle Ctrl-C at safe boundaries (#7893)

[mr-raj12]

borg check had no signal handling. The first Ctrl-C did nothing, because nothing polled the soft-interrupt flag, and the second one raised KeyboardInterrupt from wherever execution happened to be. Under --repair that could land mid-rebuild, or before finish() ran, which leaves a stale chunk index behind (#9850).

Now the pack scan, verify_data, rebuild_archives_directory and rebuild_archives check sig_int and stop at a safe boundary: chunk boundaries for the read-only scans, between whole archives for the repair rebuild. finish() runs before we raise either way, so the index and manifest stay consistent. This follows what compact already does (#9890).

A partial repository check (--max-duration) saves its PackTracker, so a later partial check resumes where it stopped; a full check restarts from the beginning.

Includes tests for both the read-only and --repair interrupt paths.

Not yet covered: the two phases that run before any of the above, the chunk-index rebuild (build_chunkindex_from_repo, slow path, always taken on --repair) and the key-recovery fallback in make_key. Both can run for a long time before the first interrupt point is reached, so an early Ctrl-C on a large --repair still appears to do nothing. Tracked in #10042.

Part of #7893.

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.19%. Comparing base (2fe079e) to head (f8b84ef).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
src/borg/archive.py	80.95%	2 Missing and 2 partials ⚠️
src/borg/archiver/check_cmd.py	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9966      +/-   ##
==========================================
+ Coverage   86.15%   86.19%   +0.04%     
==========================================
  Files          96       96              
  Lines       17360    17478     +118     
  Branches     2659     2677      +18     
==========================================
+ Hits        14956    15065     +109     
- Misses       1664     1669       +5     
- Partials      740      744       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[ThomasWaldmann]

Thanks for working on this! The approach is sound and the core behaviour is correct, but the reporting is wrong in three places — one of them badly so for a data-integrity tool. I'd like another round before merging.

I built the PR head in a scratch worktree, ran the new tests plus all of check_cmd_test.py / compact_cmd_test.py (51 passed, 2 skipped), and wrote throwaway probes to observe the actual interrupt paths.

What's right

• It follows the compact / repo-compress precedent correctly: poll sig_int, break at a boundary, run the persistence step, then raise Error("Got Ctrl-C / SIGINT.") — same message, same exception type.
• The for…else in Repository.check() still does the right thing: the new break skips the else, so tracker.clear() is not called and the check cycle survives.
• The key data-safety property holds. I verified with 3 archives, interrupting after the first was rebuilt under --repair: all three archives are still listed afterwards, and a following plain check reports "no problems found".
• black --check is clean.

Findings

1. verify_data() claims full verification after an interrupt — this is the blocker. (src/borg/archive.py, the summary after the loop)

chunks_count = len(self.chunks) is computed up front and the summary is logged unconditionally. Observed, interrupting at chunk 3 of 10:

Starting cryptographic data integrity verification...
Finished cryptographic data integrity verification, verified 10 chunks with 0 integrity errors.

So borg tells the user it cryptographically verified all 10 chunks when it read 3. For --verify-data specifically that is the worst possible false statement. Please count what was actually verified and log an interrupted variant of the line.

2. Repository.check() reports success after an interrupt. (src/borg/repository.py)

Observed, interrupting after 2 packs:

Interrupted repository check, 2 packs checked so far.
Checked 3 index files (0 errors) and 2 packs (0 errors).
Finished full repository check, no problems found.

It contradicts itself two lines apart, and returns True. The --max-duration path has the same wart today, so it is partly pre-existing — but here the command errors out immediately afterwards, which makes it actively confusing.

3. rebuild_archives_directory() logs "completed" after an interrupt. Confirmed by probe: it broke after 3 chunks and still logged Rebuilding missing archives directory entries completed. With --find-lost-archives --repair that reads as "the lost-archive scan finished" when archives further along in the index were never looked at.

4. The resume claim only holds for --max-duration. An interrupted full check saves the tracker, but the next full check does PackTracker.new() + clear() at the top, so it restarts from zero. Only a subsequent partial check picks it up. The commit message says "a later run picks up where it left off" without that qualifier — users will expect a plain borg check to resume.

5. The longest phase of --repair is still not interruptible. ArchiveChecker.check() starts with build_chunkindex_from_repo(..., slow_rebuild=repair) (src/borg/cache.py), which reads the headers of every pack with no sig_int poll, and then make_key() may scan up to 999 chunks. On a large repo that runs before any of the new checks are reachable, so the first Ctrl-C still does nothing — which is literally the complaint in #7893. A partial index is unusable, so this one would have to raise rather than break; at minimum it deserves a note.

6. rebuild_archives breaks only between archives. Justified for --repair (whole-archive rewrite), but for a read-only check a break inside the item loop would be safe and much more responsive — a single huge archive can make Ctrl-C look ignored again.

7. No doc update. repo-compress documents its SIGINT contract in the user-facing epilog (src/borg/archiver/repo_compress_cmd.py). check gains a new and more consequential contract — --repair can now stop half-done and needs a re-run — and the epilog says nothing about it.

8. Minor: raising before the if self.error_found: block drops the "problems found" summary line on interrupt (the individual errors were already logged, so this is cosmetic). Also, the two if not sig_int: blocks in check() could be a single one.

Tests

9. test_check_soft_interrupt sets the flag before anything runs, so both loops break on their first iteration — a mid-run stop is never exercised, and tracker.save() is called with zero packs recorded. It also asserts nothing about the state left behind. Compare the precedent it cites, test_compact_soft_interrupt_persists_valid_index, which interrupts mid-run and then asserts concrete post-conditions. Patching store.hash / repository.get to trip the flag on the Nth call works fine — I used exactly that in my probes. Then you can assert that cache/checked-packs exists and that a following --max-duration check logs "Continuing check cycle".

Also, assert repository.check() is True locks in finding 2: if the return value later grows an "incomplete" signal, this test blocks it.

10. test_check_repair_soft_interrupt only asserts cmd(archiver, "check", exit_code=0). It should assert that all archives survive (I verified they do) and ideally that a second --repair finishes the job. The monkeypatched Archives.create also stays active during the final in-process cmd(); it happens never to fire, but monkeypatch.undo() would make that explicit rather than load-bearing.

11. Function-local imports in both tests (from ...archive import ArchiveChecker, from ...helpers import sig_int, Error). There is no circular-import reason: check_cmd_test.py already imports from ...archive at module top, and compact_cmd_test.py imports sig_int, Error at the top too. Please move them up.

Summary

Findings 1–3 are what I'd hold the merge on — they are small fixes (count what was actually done, branch the final log line on sig_int) and they are the difference between an honest interrupt and one that lies about integrity verification. 4 and 7 are a commit-message tweak and an epilog paragraph. 5, 6, 9 and 10 are worth addressing but could also be follow-ups.