[CEL-1328] Harden static preview cleanup - #19
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request tightens cleanup-job authorization in the static website workflow. It requires pull request metadata and collaborator permission checks before preview deletion. It also forces the delete step to use one gcloud process and one thread. Tests cover the new workflow rules and failure cases. ChangesCleanup preview authorization
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change adds authorization rechecks and serializes cleanup workers, with the supplied validation checks passing. No actionable merge-blocking risk remains beyond normal checks and review. Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Independent exact-SHA gate review — reviewer identity/lane: Full SHA: Tests reproduced:
Trace: cleanup revalidation is directly before OIDC and requires closed PR, event actor = current PR author, same-repository head, and current Findings: P0 none; P1 none; P2 none; P3 none. CodeRabbit passed at exact SHA; no unresolved review threads. Cubic check remained pending and produced no blocker at review time. |
|
Independent reviewer mong-x; lane independent_one. Exact SHA: ccf499a. P0: none. Verified full two-file diff and full workflow/boundary-test files. Cleanup matrix: closed PR; actor=author; same-repository head; current admin|maintain|write; authorization immediately before OIDC. gcloud cleanup serialization: process/thread counts 1. Tests: ruby .github/tests/deploy-static-job-boundaries.test.rb; ruby .github/tests/deploy-static-lock-validator.test.rb; ruby .github/tests/discord-metadata-boundary.test.rb; actionlint .github/workflows/deploy-static-website.yaml .github/workflows/validate-static-deploy.yaml; git diff --check 0620ad4...ccf499a — PASS. Skill-perspective check ran: omo:programming, omo:remove-ai-slops. No brittle/tautological/deletion-only or implementation-mirroring test defect; no needless production parsing, normalization, or abstraction. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 5/5
- In
.github/tests/deploy-static-job-boundaries.test.rb, the cleanup test duplicates the existingOpen3.capture3/AsyncFunctionharness, increasing maintenance risk if the two copies drift—extract the shared harness before adding further cases.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/tests/deploy-static-job-boundaries.test.rb">
<violation number="1" location=".github/tests/deploy-static-job-boundaries.test.rb:233">
P3: The new cleanup test block copies the entire Open3.capture3 node AsyncFunction harness that the preview-authorization cases already define in the same file. Both harnesses are byte-near-identical; only the injected env vars differ. Extract the harness (pullRequest/context/core mocks plus the node `-e` invoker) into a shared helper, and have both the preview and cleanup case loops call it, so the two copies cannot drift apart.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
reviewer: independent_b |
|
independent_a |
1 participant
Summary
Follow-up to merged CEL-1328 PR #16.
Linear
CEL-1328
Exact head
0a03a445ac329dcdc7b4689de59b1dbb674b4413Evidence
ruby .github/tests/deploy-static-lock-validator.test.rb: 28 lock fixtures and 40 manifest executions passedruby .github/tests/deploy-static-job-boundaries.test.rb: passedruby .github/tests/discord-metadata-boundary.test.rb: passedactionlint .github/workflows/deploy-static-website.yaml .github/workflows/validate-static-deploy.yaml: passedgit diff --check: passedReview
Manual QA
Exact-head authorization matrix accepts write-capable maintainer and author closers; rejects open PR, forked head, and revoked permission. Authorization runs immediately before OIDC. Cleanup gcloud process/thread counts both equal
1.Merge order
Merge this PR before Producer #674, Importer #279, Admin #251, and E-label #200. Callers pin this exact SHA.
CEL-1422 remains held and untouched.