From 39dec1c69e389dac382e97b66a5aa3b466893c23 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 20 Sep 2026 21:20:54 -0700 Subject: [PATCH 1/8] ci: harden R2 canary cleanup and coverage --- .github/workflows/ci-artifact-canary.yml | 2 +- .github/workflows/ci-artifact-transport.yml | 1 + scripts/ci/r2-canary-cloudflare.py | 5 ++++ tests/test_ci_r2_canary.py | 32 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-artifact-canary.yml b/.github/workflows/ci-artifact-canary.yml index ae90a19880ff..3f50168ce132 100644 --- a/.github/workflows/ci-artifact-canary.yml +++ b/.github/workflows/ci-artifact-canary.yml @@ -15,7 +15,7 @@ jobs: canary: if: github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} - timeout-minutes: 15 + timeout-minutes: 20 env: WRANGLER_SEND_METRICS: 'false' steps: diff --git a/.github/workflows/ci-artifact-transport.yml b/.github/workflows/ci-artifact-transport.yml index 1ed374adfc9b..ba503bcc1fa2 100644 --- a/.github/workflows/ci-artifact-transport.yml +++ b/.github/workflows/ci-artifact-transport.yml @@ -21,6 +21,7 @@ on: branches: [main] paths: - .github/workflows/ci-artifact-transport.yml + - .github/workflows/ci.yml - workers/ci-artifacts/** - scripts/ci/restore-r2-artifact.py - scripts/ci/node_product_cache.py diff --git a/scripts/ci/r2-canary-cloudflare.py b/scripts/ci/r2-canary-cloudflare.py index b62943d003bc..8d44d90052d9 100644 --- a/scripts/ci/r2-canary-cloudflare.py +++ b/scripts/ci/r2-canary-cloudflare.py @@ -73,6 +73,11 @@ def main(): for name in ("GITHUB_ARTIFACT_TOKEN", "CANARY_ACCESS_TOKEN"): try: api(f"workers/scripts/{worker}/secrets/{name}", "DELETE") + except CloudflareError as error: + # A failed deployment may never have uploaded the secret. + # Treat that already-absent state as an idempotent cleanup. + if error.status != 404: + errors.append(str(error)) except RuntimeError as error: errors.append(str(error)) if errors: diff --git a/tests/test_ci_r2_canary.py b/tests/test_ci_r2_canary.py index 71d3a6747c72..2def7aa6f776 100644 --- a/tests/test_ci_r2_canary.py +++ b/tests/test_ci_r2_canary.py @@ -104,6 +104,38 @@ def test_worker_cleanup_requires_ownership_and_removes_bound_durable_objects(sel self.run_mode('delete-worker') self.assertEqual(self.calls, [(f'workers/scripts/{self.resource}?force=true', 'DELETE', None)]) + def test_secret_cleanup_ignores_absent_secrets(self): + def missing_secret(path, method='GET', value=None): + self.calls.append((path, method, value)) + if '/secrets/' in path: + raise cf.CloudflareError(method, path, 404) + return {} + with patch.object(cf, 'api', missing_secret), patch.object(cf, 'CREATED', self.created), \ + patch('sys.argv', ['canary', 'delete-secret']), \ + patch.dict(os.environ, {'GITHUB_RUN_ID': '123', 'GITHUB_RUN_ATTEMPT': '1'}): + cf.main() + self.assertEqual(len(self.calls), 2) + self.assertTrue(all(call[1] == 'DELETE' for call in self.calls)) + + def test_secret_cleanup_preserves_non_not_found_errors(self): + def forbidden_secret(path, method='GET', value=None): + self.calls.append((path, method, value)) + if '/secrets/' in path: + raise cf.CloudflareError(method, path, 403) + return {} + with patch.object(cf, 'api', forbidden_secret), patch.object(cf, 'CREATED', self.created), \ + patch('sys.argv', ['canary', 'delete-secret']), \ + patch.dict(os.environ, {'GITHUB_RUN_ID': '123', 'GITHUB_RUN_ATTEMPT': '1'}): + with self.assertRaises(RuntimeError): + cf.main() + + def test_workflow_covers_transport_pushes_and_canary_measurement_budget(self): + canary = (ROOT / '.github/workflows/ci-artifact-canary.yml').read_text() + self.assertIn('timeout-minutes: 20', canary) + transport = (ROOT / '.github/workflows/ci-artifact-transport.yml').read_text() + push = transport.split(' push:', 1)[1].split('\n\npermissions:', 1)[0] + self.assertIn('- .github/workflows/ci.yml', push) + def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): workflow = (ROOT / '.github/workflows/ci-artifact-canary.yml').read_text() delete_step = workflow.split("name: Remove only the canary's artifact copy", 1)[1] From 986a02280857b675afff64cc9c25a8e137d11a5b Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 20 Sep 2026 21:24:35 -0700 Subject: [PATCH 2/8] test: exercise canary cleanup retries --- tests/test_ci_r2_canary.py | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/test_ci_r2_canary.py b/tests/test_ci_r2_canary.py index 2def7aa6f776..b71d49192eea 100644 --- a/tests/test_ci_r2_canary.py +++ b/tests/test_ci_r2_canary.py @@ -145,6 +145,60 @@ def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): self.assertIn('sleep "$((attempt * 5))"', delete_step) self.assertIn('exit 1', delete_step) + # Execute the workflow's shell block against a fake Wrangler so this + # contract covers the retry count, backoff, key, and terminal status. + script = delete_step.split(' run: |\n', 1)[1] + script = '\n'.join(line[10:] for line in script.splitlines() if line.startswith(' ')) + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + wrangler = root / 'node_modules/.bin/wrangler' + wrangler.parent.mkdir(parents=True) + wrangler.write_text( + '#!/bin/sh\n' + 'count_file="$FAKE_WRANGLER_COUNT"\n' + 'count=$(cat "$count_file" 2>/dev/null || echo 0)\n' + 'count=$((count + 1))\n' + 'echo "$count" > "$count_file"\n' + 'printf "%s\\n" "$*" >> "$FAKE_WRANGLER_ARGS"\n' + '[ "$count" -gt "$FAKE_WRANGLER_FAILURES" ]\n') + wrangler.chmod(0o755) + fake_bin = root / 'bin' + fake_bin.mkdir() + sleep = fake_bin / 'sleep' + sleep.write_text('#!/bin/sh\nprintf "%s\\n" "$1" >> "$FAKE_SLEEP_ARGS"\n') + sleep.chmod(0o755) + + def execute(failures): + count = root / 'count' + args = root / 'wrangler-args' + sleeps = root / 'sleep-args' + for path in (count, args, sleeps): + path.unlink(missing_ok=True) + env = os.environ | { + 'CANARY_RESOURCE': 'cmux-ci-artifacts-canary-123-1', + 'FAKE_WRANGLER_COUNT': str(count), + 'FAKE_WRANGLER_ARGS': str(args), + 'FAKE_SLEEP_ARGS': str(sleeps), + 'FAKE_WRANGLER_FAILURES': str(failures), + 'PATH': f'{fake_bin}:{os.environ["PATH"]}', + } + result = subprocess.run(['bash', '-c', f'set -euo pipefail\n{script}'], + cwd=root, env=env, capture_output=True, text=True) + return result, int(count.read_text()), args.read_text().splitlines(), \ + sleeps.read_text().splitlines() if sleeps.exists() else [] + + result, count, args, sleeps = execute(2) + self.assertEqual(result.returncode, 0) + self.assertEqual(count, 3) + self.assertEqual(sleeps, ['5', '10']) + self.assertIn('cmux-ci-artifacts-canary-123-1/github/manaflow-ai/cmux/10610975375/', args[0]) + + result, count, _, sleeps = execute(4) + self.assertEqual(result.returncode, 1) + self.assertEqual(count, 4) + self.assertEqual(sleeps, ['5', '10', '15']) + self.assertIn('failed to remove the canary artifact after 4 attempts', result.stderr) + class MeasurementTests(unittest.TestCase): def test_hashes_real_bytes_and_distinguishes_fill_from_hit(self): From 06be7bcdec9aae475ab35a9fde88914768c438a2 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 20:51:33 -0700 Subject: [PATCH 3/8] ci: retry R2 canary artifact cleanup From 1ba71d865f3e3651d66c654d94b68820ffc7c1d2 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 20:51:35 -0700 Subject: [PATCH 4/8] ci: cover main CI transport changes --- .github/workflows/ci-artifact-transport.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci-artifact-transport.yml b/.github/workflows/ci-artifact-transport.yml index ba503bcc1fa2..1ed374adfc9b 100644 --- a/.github/workflows/ci-artifact-transport.yml +++ b/.github/workflows/ci-artifact-transport.yml @@ -21,7 +21,6 @@ on: branches: [main] paths: - .github/workflows/ci-artifact-transport.yml - - .github/workflows/ci.yml - workers/ci-artifacts/** - scripts/ci/restore-r2-artifact.py - scripts/ci/node_product_cache.py From 3ff1c8c71a85798f72db8940730ba50de449686c Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 20:51:37 -0700 Subject: [PATCH 5/8] ci: make absent R2 canary secrets idempotent From 3f3bb9f0fdff57a1291728a1461ced80c6e708a2 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 20:51:39 -0700 Subject: [PATCH 6/8] test: execute R2 canary cleanup retry contract --- tests/test_ci_r2_canary.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_ci_r2_canary.py b/tests/test_ci_r2_canary.py index b71d49192eea..9e3ce618588a 100644 --- a/tests/test_ci_r2_canary.py +++ b/tests/test_ci_r2_canary.py @@ -104,6 +104,16 @@ def test_worker_cleanup_requires_ownership_and_removes_bound_durable_objects(sel self.run_mode('delete-worker') self.assertEqual(self.calls, [(f'workers/scripts/{self.resource}?force=true', 'DELETE', None)]) + def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): + workflow = (ROOT / '.github/workflows/ci-artifact-canary.yml').read_text() + delete_step = workflow.split("name: Remove only the canary's artifact copy", 1)[1] + delete_step = delete_step.split('name: Remove an empty bucket created by this run', 1)[0] + self.assertIn('for attempt in 1 2 3 4; do', delete_step) + self.assertIn('wrangler r2 object delete "$key" --remote', delete_step) + self.assertIn('sleep "$((attempt * 5))"', delete_step) + self.assertIn('exit 1', delete_step) + + def test_secret_cleanup_ignores_absent_secrets(self): def missing_secret(path, method='GET', value=None): self.calls.append((path, method, value)) @@ -145,8 +155,6 @@ def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): self.assertIn('sleep "$((attempt * 5))"', delete_step) self.assertIn('exit 1', delete_step) - # Execute the workflow's shell block against a fake Wrangler so this - # contract covers the retry count, backoff, key, and terminal status. script = delete_step.split(' run: |\n', 1)[1] script = '\n'.join(line[10:] for line in script.splitlines() if line.startswith(' ')) with tempfile.TemporaryDirectory() as directory: From fb316c03c4a92117160dfd3f3a63e8e2971e402e Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 20:54:38 -0700 Subject: [PATCH 7/8] ci: trigger artifact transport on main CI changes --- .github/workflows/ci-artifact-transport.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci-artifact-transport.yml b/.github/workflows/ci-artifact-transport.yml index 1ed374adfc9b..ba503bcc1fa2 100644 --- a/.github/workflows/ci-artifact-transport.yml +++ b/.github/workflows/ci-artifact-transport.yml @@ -21,6 +21,7 @@ on: branches: [main] paths: - .github/workflows/ci-artifact-transport.yml + - .github/workflows/ci.yml - workers/ci-artifacts/** - scripts/ci/restore-r2-artifact.py - scripts/ci/node_product_cache.py From 2c1cb81db0b9fdb8d0eba83f1de15b32cf49609b Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 22 Sep 2026 06:41:51 -0700 Subject: [PATCH 8/8] ci: point the transport push trigger at ci-macos.yml The push-path trigger named ci.yml, but #13405 moved the macOS jobs that consume these artifacts into ci-macos.yml, so pushes that changed the consuming lane no longer re-validated transport. Point the trigger and its test at ci-macos.yml. Also drop a duplicate definition of test_workflow_retries_remote_artifact_delete_before_bucket_cleanup. The later definition shadowed the earlier one, so the shorter version never ran; the surviving copy is the superset that also executes the wrangler stub. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci-artifact-transport.yml | 2 +- tests/test_ci_r2_canary.py | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci-artifact-transport.yml b/.github/workflows/ci-artifact-transport.yml index ba503bcc1fa2..ca1a57f76185 100644 --- a/.github/workflows/ci-artifact-transport.yml +++ b/.github/workflows/ci-artifact-transport.yml @@ -21,7 +21,7 @@ on: branches: [main] paths: - .github/workflows/ci-artifact-transport.yml - - .github/workflows/ci.yml + - .github/workflows/ci-macos.yml - workers/ci-artifacts/** - scripts/ci/restore-r2-artifact.py - scripts/ci/node_product_cache.py diff --git a/tests/test_ci_r2_canary.py b/tests/test_ci_r2_canary.py index 9e3ce618588a..d645ddf7a939 100644 --- a/tests/test_ci_r2_canary.py +++ b/tests/test_ci_r2_canary.py @@ -104,16 +104,6 @@ def test_worker_cleanup_requires_ownership_and_removes_bound_durable_objects(sel self.run_mode('delete-worker') self.assertEqual(self.calls, [(f'workers/scripts/{self.resource}?force=true', 'DELETE', None)]) - def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): - workflow = (ROOT / '.github/workflows/ci-artifact-canary.yml').read_text() - delete_step = workflow.split("name: Remove only the canary's artifact copy", 1)[1] - delete_step = delete_step.split('name: Remove an empty bucket created by this run', 1)[0] - self.assertIn('for attempt in 1 2 3 4; do', delete_step) - self.assertIn('wrangler r2 object delete "$key" --remote', delete_step) - self.assertIn('sleep "$((attempt * 5))"', delete_step) - self.assertIn('exit 1', delete_step) - - def test_secret_cleanup_ignores_absent_secrets(self): def missing_secret(path, method='GET', value=None): self.calls.append((path, method, value)) @@ -144,7 +134,10 @@ def test_workflow_covers_transport_pushes_and_canary_measurement_budget(self): self.assertIn('timeout-minutes: 20', canary) transport = (ROOT / '.github/workflows/ci-artifact-transport.yml').read_text() push = transport.split(' push:', 1)[1].split('\n\npermissions:', 1)[0] - self.assertIn('- .github/workflows/ci.yml', push) + # The app-host lane that consumes these artifacts lives in + # ci-macos.yml since #13405, so that is the workflow whose pushes + # must re-validate transport. + self.assertIn('- .github/workflows/ci-macos.yml', push) def test_workflow_retries_remote_artifact_delete_before_bucket_cleanup(self): workflow = (ROOT / '.github/workflows/ci-artifact-canary.yml').read_text()