fix(previews): fence stale hogbox teardown - #71052
Conversation
Generated-By: PostHog Code Task-Id: 8603304a-c1cf-4544-b309-b1dc1a767ce7
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR hardens the hogbox preview teardown path against race conditions where an older/cancelled workflow run can “win” cleanup after a newer run has already repointed the same preview pen, risking deletion of the replacement preview.
Changes:
- Removes name-based sweeping of boxes during provisioning/teardown to avoid cross-run deletion.
- Switches teardown to a fenced (conditional) pen delete via Hogland’s
expected_current_box_idmechanism. - Adds a direct HTTP call path (URL-quoted pen name +
raise_for_status) to support the fence before the workflow’s pinned SDK is updated.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…gbox-preview-teardown
…gbox-preview-teardown
Treat an explicit box id as invocation-owned when a pen is already gone, and skip unsafe conditional cleanup when a name-only pen has no current box. Generated-By: PostHog Code Task-Id: 8603304a-c1cf-4544-b309-b1dc1a767ce7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb2e8260b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Delete invocation-owned explicit boxes after a pen conflict and remove empty pens during authoritative name-only teardown. Generated-By: PostHog Code Task-Id: 8603304a-c1cf-4544-b309-b1dc1a767ce7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbb0c115c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Preview-environment teardown tooling with a real race-condition fix; every substantive concern raised by Codex/Copilot across several rounds was addressed with a matching code change (verified in the diff), and the author has STRONG familiarity with this exact file, giving independent assurance for this CI-adjacent tooling change.
- Author wrote 100% of the modified lines and has 6 merged PRs in these paths (familiarity STRONG).
- chatgpt-codex-connector[bot] reviewed the current head.
- Fenced delete for the name-only teardown path reads the pen and immediately reuses its current_box_id as the expected value, which only protects the GET→DELETE window rather than a wider race — acceptable per the author's design rationale since production callers only invoke name-only teardown from PR-closed/label-removed/stale-sweep flows serialized by a per-PR concurrency group, but worth a human's eyes given no test exercises this happy-path branch.
- Uses private hogland SDK internals (_http.delete, hogland._http.raise_for_status) as a stopgap until the pinned SDK version is bumped; disclosed in the PR description along with an explicit warning that the companion Hogland API change must be deployed first — an operational sequencing risk outside this diff's control.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 109L, 1F substantive, 289L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (289L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 8823c9e · reviewed head cbb0c11 |
Problem
A cancelled preview workflow can complete after a newer workflow for the same PR has repointed its preview pen. Name-based cleanup then risks deleting the replacement preview.
Changes
Warning
Deploy the Hogland API change before this workflow change. The safety fence is enforced server-side.
How did you test this code?
I used Codex to run
ruff formatandruff checkontools/hogbox-preview/hogbox_preview/hogland_backend.py.The commit hook also ran Python linting, formatting, and
ty checksuccessfully. I did not add tests in this repository; the behavior is exercised by the accompanying Hogland API and SDK regression tests.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
No PostHog documentation update is needed. The Hogland API and SDK documentation is updated in the linked server PR.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
I used Codex CLI. No repository skills were invoked. I removed client-side cross-run deletion rather than trying to make name matching more selective, because the stable pen pointer is the authoritative ownership boundary. The workflow remains compatible with its currently pinned SDK by issuing the fenced request directly.
Created with PostHog Code