test: pin the deletion scope with regression tests

[lesandie]

Why

The suite proved that a candidate list is deleted, batched and checkpointed correctly. It never proved the list contains only orphans.

FakeCH.query_row_block_stream ignores the anti-join SQL and returns pre-canned blocks, so the one statement that forms the entire safety boundary was never asserted on. The two tests that did read stream_query only checked ORDER BY.

Evidence

Mutation testing established the gap rather than assuming it — the repo was copied to a scratch dir, the delete scope was broken eleven ways, and the full suite was run against each.

Mutation	Before	After
LEFT ANTI JOIN → LEFT JOIN — candidates include referenced objects	65 passed	caught
--dry-run performs real deletions	65 passed	caught
Drop clusterAllReplicas fan-out — blobs referenced only by another replica look orphaned	65 passed	caught
Point disk_name at a nonexistent disk — orphans the whole bucket	65 passed	caught
Drop the remote_path = objpath join key	65 passed	caught
Drop the --useage grace window	65 passed	caught
Drop s3o.active=true	65 passed	caught
Remove the preflight_cluster() call	65 passed	caught
Preflight accepts any replica count	65 passed	caught
Ignore S3 delete errors	caught	caught
Tombstone unconfirmed rows	caught	caught

2 of 11 → 11 of 11.

What this does not change

s3gc.py is deliberately untouched. The anti-join is correct today and no behaviour changes. These tests exist so that a future edit which quietly widens the deletion scope fails a test instead of reaching a customer bucket.

What is now pinned

13 tests asserting the scope directly: anti-join semantics, the remote_path = objpath join key, the disk_name predicate, the clusterAllReplicas fan-out when a cluster is configured, active=true, the --useage window, candidates being drawn only from the auxiliary table, that a dry run cannot reach S3 at all, and that the cluster/replica preflight gates the delete path.

Defects recorded, not fixed

Each needs an explicit decision, so they are documented in CHANGELOG.md and tracked in TODO.md rather than changed here:

• USEAGE_HOURS=0 silently removes the only guard against deleting a part mid-write. ClickHouse uploads a part's blobs to S3 and registers them in system.remote_data_paths a moment later; in that window a live blob looks orphaned. There is no per-object re-check before the S3 delete, so the --useage clause is the whole safety margin — and if args.useage else "" emits no clause for 0, which render.py accepts (only negatives are rejected). Covered by test_useage_zero_disables_the_age_guard, which documents the hazard.
• --useafter is interpolated unquoted, the only unquoted value in the anti-join WHERE clause. Recorded as a strict xfail so it flips to a failure the moment it is fixed.
• The cluster/replica preflight is point-in-time, run once, while the anti-join is re-issued per sample over what can be hours. A replica lost mid-run is not re-detected.

Checks

pytest -v -m "not dev_cluster"     78 passed, 1 xfailed
render.py example.env              OK (94 lines)
kubeconform -strict                Valid: 1, Invalid: 0, Errors: 0

🤖 Generated with Claude Code

[lesandie]

Review record — how this gap was found

Posting the method so the evidence is on the record and reproducible, not just the conclusion.

The deletion scope was audited by mutation testing: the repo was copied to a scratch directory, the delete-scoping logic was deliberately broken eleven different ways, and the full suite was run against each. A mutation that "survives" is one the tests did not notice.

Mutation	Before this PR	After
LEFT ANTI JOIN → LEFT JOIN — candidates include referenced objects	65 passed ⚠️	caught
--dry-run performs real deletions	65 passed ⚠️	caught
Drop clusterAllReplicas fan-out — blobs referenced only by another replica look orphaned	65 passed ⚠️	caught
Point disk_name at a nonexistent disk — orphans the whole bucket	65 passed ⚠️	caught
Drop the remote_path = objpath join key	65 passed ⚠️	caught
Drop the --useage grace window	65 passed ⚠️	caught
Drop s3o.active=true	65 passed ⚠️	caught
Remove the preflight_cluster() call	65 passed ⚠️	caught
Preflight accepts any replica count	65 passed ⚠️	caught
Ignore S3 delete errors	caught	caught
Tombstone unconfirmed rows	caught	caught

2 of 11 → 11 of 11.

Root cause of the gap

FakeCH.query_row_block_stream ignores the SQL and returns pre-canned blocks:

def query_row_block_stream(self, query):
    self.stream_query = query      # stored, never evaluated
    return FakeStream(self.blocks)

The candidate list was hardcoded by the fake. The suite proved "given a list of orphans, deletion, batching and checkpointing behave correctly" — which it did well — but never "that list contains only orphans". The two tests that read stream_query checked ORDER BY and nothing else.

What this PR does and does not claim

The anti-join is correct today, and there is no evidence s3gc has ever deleted referenced data. s3gc.py is untouched here and no behaviour changes. The point is narrower: a future edit that quietly widens the deletion scope now fails a test instead of reaching a customer bucket.

Findings surfaced during the audit

Three defects were found and deliberately not fixed in this PR, so each got an explicit decision rather than a drive-by change:

1. USEAGE_HOURS=0 removes the only guard against deleting a part mid-write — decided during review, fixed in fix: enforce a 24 hour floor on the deletion age window #9 (hard floor of 24, default 24).
2. --useafter is interpolated unquoted — recorded as a strict xfail, so it flips to a failure the day it is fixed.
3. The cluster/replica preflight is point-in-time, run once while the anti-join is re-issued per sample over hours — tracked in TODO.md.

Anyone can reproduce the table above; the mutation driver is a short script that patches s3gc.py in a scratch copy and re-runs pytest per mutation.