Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report🚨 Trunk lane — universal laneThis PR is assigned to the universal lane. It cannot merge in parallel with other PRs, so it can take longer to merge. Ask dev-ex if you think this is wrong. ✅ Duplication (Python) — cleanNew Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. ✅ Duplication (TypeScript) — cleanNew TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe relay now records selected check identifiers and creates a diagnostics request when it fails. A separate workflow validates the request and collects bounded Depot evidence. The collector checks workflow and attempt identity, sanitizes report text, and classifies retryability from failure evidence. Tests and handbook guidance cover relay behavior, workflow isolation, evidence validation, and unavailable diagnostics. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to An ambiguous gate result may break CI failure reporting, and collector polling may exhaust the API budget used by the required gate. Resolve or explicitly accept these risks before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The collector is separated from PR code, checks that requests match the originating run, and returns bounded, sanitized diagnostics without changing the CI result. Credential availability and the complete path have not yet been verified in a live run. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/scripts/ci_backend_relay.py (1)
94-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the path regex from
DEPOT_ORGinstead of a second hardcoded org literal.Line 94 hardcodes
ntsdt08fpt. Line 46 definesDEPOT_ORG, and Line 97 and Line 111 use it. IfDEPOT_ORGchanges and Line 94 does not,from_apiclears everydetails_url.CheckRunReader.readthen drops every check, and the relay reportsABSENTfor all runs. The path instructions require that code "says everything once and only once."♻️ Proposed fix
- match = re.fullmatch(r"/orgs/ntsdt08fpt/workflows/([a-z0-9]+)", parsed.path) + match = re.fullmatch(rf"/orgs/{re.escape(DEPOT_ORG)}/workflows/([a-z0-9]+)", parsed.path)As per path instructions: "says everything once and only once, and has no superfluous parts."
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: afb2c53a-2bdb-48e3-85b3-ebefbc41ab7e
📒 Files selected for processing (11)
.depot/workflows/ci-backend.yml.github/scripts/ci_backend_audit.py.github/scripts/ci_backend_diagnostics.py.github/scripts/ci_backend_relay.py.github/scripts/test_ci_backend_audit.py.github/scripts/test_ci_backend_diagnostics.py.github/scripts/test_ci_backend_relay.py.github/workflows/ci-backend-diagnostics.yml.github/workflows/ci-backend.ymldocs/published/handbook/engineering/fork-pull-requests.mdtools/workflow-plan/tests/workflows.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
|
[High risk] Adds diagnostic collection for CI backend test failures. The PR should not merge until the request SHA check is corrected so the collector can deliver the diagnostics this change introduces. Reviews (1) · Last reviewed commit: "fix(ci): explain depot backend failures" |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
af5cf92 to
0bdae28
Compare
rnegron
changed the title
HostHog preview —
|
HostHog preview —
|
468575b to
cba8d6b
Compare
0bdae28 to
b488447
Compare
cba8d6b to
7603442
Compare
b488447 to
f324dd0
Compare
7603442 to
64564a1
Compare
f324dd0 to
1a72557
Compare
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
Re-add the stamphog label to request another review once you have addressed this.
This pull request was refused automatically and cannot be merged as-is.
- deny-list gate: FAILED — the change touches CI/CD infrastructure (
infra_cicd), specifically.depot/workflows/ci-backend.yml,.github/workflows/ci-backend.yml, and the new.github/workflows/ci-backend-diagnostics.yml, which fall under a category this policy never auto-clears. - tier gate: FAILED — classified as
T2-never, since it's a cross-cuttingfeatspanning 9 files and 1143 lines (including new scripts, tests, and workflow definitions), which exceeds what automated review is permitted to approve.
These are hard policy stops, not a judgment on the code itself. To move forward, ask a human reviewer (ideally one with CI/CD infra ownership) to review and approve this manually, or split the change into smaller, non-infra pieces (e.g. docs updates, script logic, and workflow changes as separate PRs) so each can go through the normal automated gates.
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: infra_cicd |
| size | ✓ | 657L, 5F substantive, 1143L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (1143L, 9F, cross-cutting, feat) |
| stamphog 2.2.0 | .stamphog/policy.yml @ unknown · reviewed head 1a72557 |
webjunkie
left a comment
webjunkie
left a comment
There was a problem hiding this comment.
Note
Agent review, actively prompted for and steered.
Commenting. The trust boundary looks sound. The cost and behavior points below need an answer first.
Open points
- Dormant cost. Without
DEPOT_CI_CANCEL_TOKEN, the collector uploads no report, butreceivestill polls for 300 s. Every failed Depot gate fails 5 minutes later. - Idle runner.
workflow_run: in_progressholds a GitHub-hosted runner for each Depot-routed run until the run completes. It polls two API calls every 30 s on the repoGITHUB_TOKENbudget, also when nothing fails. - Retry guidance removed. The relay drops the
depot ci retrycommands and theci-backend-githublabel. The description does not mention this. - Least privilege. The cancel token becomes a read credential. A read-only token fits better.
- Doc location. The new section is general Depot guidance, not fork guidance, so
fork-pull-requests.mdis the wrong home.
Suggested direction
Trigger the collector on completed and post the report as a check run. That removes the in-run wait and the idle runner, and likely most of the code.
This branch has not been deployed
2 participants
Problem
Changes
Before:
flowchart LR D[Depot gate] --> R[GitHub relay] classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; class D,R phBlue;After:
flowchart LR D[Depot checks] --> R[GitHub relay] R -->|Request artifact| C[Trusted collector] C -->|Sanitized report| R classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; class D,R,C phBlue;How did you test this code?
Release status
Automatic notifications
Docs update
🤖 Agent context