Skip to content

trunk-merge/pr-110575/2f31bc42-b18c-4088-9bda-f85f04b865c3-bisection - #110851

Closed
trunk-io[bot] wants to merge 6 commits into
masterfrom
trunk-merge/pr-110575/2f31bc42-b18c-4088-9bda-f85f04b865c3-bisection
Closed

trunk-io[bot] wants to merge 6 commits into
masterfrom
trunk-merge/pr-110575/2f31bc42-b18c-4088-9bda-f85f04b865c3-bisection

Conversation

@trunk-io

@trunk-io trunk-io Bot commented Oct 2, 2026

Copy link
Copy Markdown
Trunk Merge Pull Request Banner

This pull request was created and is being managed by Trunk Merge.

This pull request is based on the master branch at SHA e6b3dfe3fa152fd136497b749e3287220b23368f.

See more details about each PR in the batch here:

When CI completes, this pull request will be closed automatically.

Pull Requests Being Tested

This pull request is testing a batch with the changes from pull requests 110575 and 110736 - batching documentation.

Batch Bisection

This pull request is in a batch bisection. Pull requests successfully tested by this PR will re-enter the main queue.

puemos and others added 6 commits October 2, 2026 10:40
…ull screen

The canvas now only covers the whole page, like the artifact full page view. It no longer asks the browser for full screen. The button labels say "Open full page" and "Exit full page".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: c378b147-7317-4d99-af9e-2ba7c7a17997
1 updated
Run: c371b7b0-1eeb-47bb-8d9a-d9c485d9043d

Co-authored-by: puemos <13174025+puemos@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (14)
.cursor/rules/react-typescript.mdc — auto-discovered
.agents/skills/using-kea-disposables/SKILL.md — configured
.agents/skills/writing-ui-components/SKILL.md — configured
.agents/skills/authoring-ci-workflows/SKILL.md — configured
.agents/skills/gating-production-deploys/SKILL.md — configured
.agents/security.md — configured
docs/published/handbook/engineering/type-system.md — configured
.agents/skills/writing-tests/SKILL.md — configured
.agents/skills/adopting-generated-api-types/SKILL.md — configured
.agents/skills/placing-product-frontend-code/SKILL.md — configured
.agents/skills/writing-kea-logics/SKILL.md — configured
.agents/skills/writing-user-facing-copy/SKILL.md — configured
.claude/commands/conventions.md — configured
.agents/skills/writing-code-comments/SKILL.md — configured
📝 Walkthrough

Walkthrough

Hogbox preview lifecycle updates now use sections in the shared CI report, with workflow calls for building, ready, failed, and torn-down states. CI report updates can use PR_NUMBER when the event has no pull-request number. Canvas controls use full-page wording, and fullscreen logic no longer invokes browser fullscreen APIs or listens for fullscreenchange. One light-theme snapshot hash changed.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to dfd9c

This change moves preview reporting into the shared CI report and updates Canvas full-page behavior. Teardown may leave a stale ready section in rare duplicate-comment cases. The remaining findings are CI hygiene and test strength, so the change is mergeable with minor follow-up.

Security Architecture Review

Security architecture risk: 🔵 Low · up to dfd9c

Preview reporting is consolidated without an observed increase in privileges or exposure to untrusted code. Remaining uncertainty concerns concurrent status updates and recovery after interrupted reporting.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed reporting operations target comments on the resolved repository and PR. Existing deployment jobs retain their prior privileged roles; the reporting migration does not demonstrate expanded infrastructure authority. Effective downstream IAM and tailnet exposure were not established by this review.

Trust Boundaries and Controls

  • observed — Token-bearing reporting executes default-branch scripts rather than PR code. The separate PR frontend build has restricted permissions, and manual-dispatch fork guards precede sensitive deployment work. Shared reports require an allowed bot author and an exact leading marker; legacy deletion additionally requires github-actions[bot] authorship and the supplied Hogbox prefix.
🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is a Trunk Merge batch-bisection notice, not a stand-alone explanation of this pull request. It omits the required Problem, Changes, testing and test rationale, Release status, and Age… Replace the batch-bisection text with a repository-compliant description. Explain the user or developer problem, list the observable changes and any mechanical changes, document automated tests and test rationale, select exactly one release…
Full details: Description check

Explanation

The description is a Trunk Merge batch-bisection notice, not a stand-alone explanation of this pull request. It omits the required Problem, Changes, testing and test rationale, Release status, and Agent context sections. It does not describe the Hogbox CI report changes or canvas full-page behavior changes.

Resolution

Replace the batch-bisection text with a repository-compliant description. Explain the user or developer problem, list the observable changes and any mechanical changes, document automated tests and test rationale, select exactly one release-status option, and complete Agent context if an agent authored or assisted with the work.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (2)
frontend/bin/ci-report/update-ci-report.mjs-266-266 (1)

266-266: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check every report comment before skipping an update.

updateSectionIfPresent checks only the first report comment. If a duplicate contains the hogbox-preview section and the first comment does not, teardown leaves the section showing a ready preview. postSection already merges duplicate comments. Check all report comments for the section before deciding there is nothing to update.

frontend/bin/ci-report/update-ci-report.test.ts-366-366 (1)

366-366: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert that an unrelated section stays unchanged.

The “untouched” case passes even if updateSectionIfPresent overwrites or removes bundle-size. Assert that the report body, or at least its parsed bundle-size section, equals the original. As per coding guidelines, an assertion should be concrete enough that “it shouldn't be possible to break the code and the test pass.”

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: becbb57a-920b-4673-871c-93361857c825

📥 Commits

Reviewing files that changed from the base of the PR and between e6b3dfe and dfd9cd4.

📒 Files selected for processing (11)
  • .github/scripts/post-ci-sections.test.mjs
  • .github/scripts/post-hogbox-preview-section.mjs
  • .github/workflows/hogbox-preview-env.yml
  • frontend/bin/ci-report/update-ci-report.mjs
  • frontend/bin/ci-report/update-ci-report.test.ts
  • frontend/snapshots.yml
  • products/canvas/frontend/scene/CanvasFullscreenExit.tsx
  • products/canvas/frontend/scene/CanvasFullscreenToggle.tsx
  • products/canvas/frontend/scene/canvasFullscreenLogic.test.ts
  • products/canvas/frontend/scene/canvasFullscreenLogic.ts
  • tools/hogbox-preview/README.md

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

Comment on lines +280 to +282
sparse-checkout: |
.github/scripts
frontend/bin/ci-report

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.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Disable cone mode for this sparse checkout.

Add sparse-checkout-cone-mode: false so the announce job checks out only the paths it needs. Cone mode also materializes repository-root files. As per coding guidelines, “Always set sparse-checkout-cone-mode: false.”

Proposed change
                   sparse-checkout: |
                       .github/scripts
                       frontend/bin/ci-report
+                  sparse-checkout-cone-mode: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sparse-checkout: |
.github/scripts
frontend/bin/ci-report
sparse-checkout: |
.github/scripts
frontend/bin/ci-report
sparse-checkout-cone-mode: false
🧰 Tools
🪛 zizmor (1.30.1)

[warning] 276-282: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

Source: Coding guidelines

@trunk-io trunk-io Bot closed this Oct 2, 2026
@trunk-io
trunk-io Bot deleted the trunk-merge/pr-110575/2f31bc42-b18c-4088-9bda-f85f04b865c3-bisection branch October 2, 2026 14:07
@trunk-io

trunk-io Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Author

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants