Skip to content

refactor(ci): harden relay check parsing - #107874

Merged
trunk-io[bot] merged 3 commits into
fix/depot-relay-root-failurefrom
refactor/depot-relay-check-parsing
Sep 29, 2026
Merged

trunk-io[bot] merged 3 commits into
fix/depot-relay-root-failurefrom
refactor/depot-relay-check-parsing

Conversation

@rnegron

@rnegron rnegron commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Problem

  • Malformed or mismatched check data can contaminate the Depot relay's verdict and output.

Changes

  • Reject checks outside the expected app, identity, organization, SHA and PR.
  • Bound API responses and validate URL and state values.
  • Preserve the preceding layer's root-failure reporting.

How did you test this code?

  • Direct pytest covers identity mismatches, API failures and existing relay behavior.
  • Hogli rejects the scripts directory; live verification and repo-wide mypy remain pending.

Release status

  • No feature flag controls this change

Automatic notifications

  • Publish to changelog?

Docs update

  • Relay guidance follows in the collector layer.

🤖 Agent context

  • Human-driven; Codex, GPT-6; gh and shell. Split from the existing draft; synthetic fixtures.
  • CodeRabbit skipped at the author's request.
  • Skills: stacking-prs, depot-ci, authoring-ci-workflows, writing-tests, running-ci-preflight, hogli, writing-dataclasses, writing-code-comments, simplify, reviewing-with-coderabbit, shipping-a-pr, writing-pr-descriptions.

@rnegron
rnegron added this pull request to stack #107875 September 28, 2026 16:23
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

🚨 Trunk lane — universal lane

This 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) — clean

New 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) — clean

New 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.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 37399227-2ab3-4eb0-a59e-541df5a8562a

📥 Commits

Reviewing files that changed from the base of the PR and between 64564a1 and 64564a1.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 584cbb10-bdeb-44b5-919d-d2ecc6bbe57b

📥 Commits

Reviewing files that changed from the base of the PR and between 7603442 and 64564a1.

📒 Files selected for processing (2)
  • .github/scripts/ci_backend_relay.py
  • .github/scripts/test_ci_backend_relay.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The relay accepts workflow URLs only for the configured Depot organization and maps unrecognized check states to "unknown". Check-run reads enforce response and pagination limits, handle malformed data, and filter results by app, name, SHA, optional PR number, and Depot workflow URL. The entry point passes the event’s PR number to the reader. Tests cover API-shaped payloads and mismatched identity fields.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 64564

Valid Depot checks are retained when another check is malformed or has no pull-request association. No actionable merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 64564

The relay now applies narrower identity and URL checks before accepting a successful CI result. No newly introduced security bypass was established, but checks without pull-request association data are still accepted, and that upstream case remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The security-sensitive outcome is the relay verdict for the event’s repository and commit. The examined change does not establish a new credential, service, or deployment boundary.

Trust Boundaries and Controls

  • observed — GitHub API records cross into the gate only after identity and workflow checks; unknown states cannot become an explicit successful conclusion.

Resilience and Maintainability Implications

  • observed — Incomplete or invalid reads do not replace the cached result, and a paginated read does not retain a first-page ETag for later cache validation.

Hardening Proposals

  • proposed — Confirm whether Depot check runs can legitimately omit PR association; if they cannot, require an explicit matching association rather than treating its absence as a match.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description includes all main template sections and clearly states the problem, changes, testing status, release status, documentation status, and agent involvement. The agent context omits the re…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@rnegron
rnegron force-pushed the refactor/depot-relay-check-parsing branch from 468575b to cba8d6b Compare September 28, 2026 17:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 8422f0f6-8590-48ff-80fd-5580cfffd44f

📥 Commits

Reviewing files that changed from the base of the PR and between 468575b and cba8d6b.

📒 Files selected for processing (2)
  • .github/scripts/ci_backend_relay.py
  • .github/scripts/test_ci_backend_relay.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

Comment thread .github/scripts/ci_backend_relay.py Outdated
@rnegron
rnegron removed this pull request from stack #107875 September 28, 2026 19:08
@rnegron
rnegron added this pull request to stack #107951 September 28, 2026 19:13
@rnegron
rnegron force-pushed the refactor/depot-relay-check-parsing branch from cba8d6b to 7603442 Compare September 28, 2026 19:17
@rnegron
rnegron marked this pull request as ready for review September 28, 2026 19:24
@rnegron
rnegron force-pushed the refactor/depot-relay-check-parsing branch from 7603442 to 64564a1 Compare September 28, 2026 19:34
@rnegron rnegron added the stamphog Request AI approval (no full review) label Sep 28, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

This hardens CI check-parsing logic (risky CI tooling territory), but the author has STRONG familiarity (100% of touched lines last-authored by them, 65 merged PRs in this path), which substitutes for independent review assurance, and the change is thoroughly covered by new tests with no unresolved reviewer concerns on the current head.

  • Author wrote 100% of the modified lines and has 65 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 63L, 1F substantive, 118L/2F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (118L, 2F, single-area, refactor)
stamphog 2.2.0 .stamphog/policy.yml @ 64564a1 · reviewed head 64564a1

@rnegron
rnegron removed this pull request from stack #107951 September 29, 2026 13:54
@rnegron
rnegron added this pull request to stack #108368 September 29, 2026 13:55
@trunk-io
trunk-io Bot merged commit 3eeb090 into master Sep 29, 2026
238 checks passed
@trunk-io

trunk-io Bot commented Sep 29, 2026

Copy link
Copy Markdown

This pull request was merged into master as part of stacked PR 107914.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant