Skip to content

chore(ci): isolate privileged docs automation from pr events - #3075

Merged
Piotr1215 merged 6 commits into
mainfrom
devops-1803/pr-event-migration
Nov 10, 2026
Merged

Piotr1215 merged 6 commits into
mainfrom
devops-1803/pr-event-migration

Conversation

@Piotr1215

@Piotr1215 Piotr1215 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Content description

Replace pull_request_target workflows with an unprivileged PR signal and four trusted workflow_run consumers. Before port or Linear actions, the consumers use the GitHub API to resolve and validate current PR state.

The signal has permissions: {} and runs on opened, edited, labeled, closed, reopened, and synchronize events. Consumers check out only the local resolver at the default workflow revision, without persisted credentials. The resolver validates the run attempt, workflow, repository, and PR head repository, branch, SHA, and base. Each resolution is capped at 30 API calls, 10 candidates, and three comment pages.

Ports require a merged PR. Linear comments require main and exclude Dependabot. Current labels control port and follow-up actions. Follow-up creation checks trusted bot markers and exact legacy Linear URLs and serializes by PR head. Backport alerts cover resolver failures. No pull_request_target references remain under .github.

The shared resolver in loft-sh/github-actions#349 supports merged PRs targeting the default branch. Docs also needs open PRs and ports to other base branches. This PR uses a local resolver with the same head repository check; consolidating that logic needs a broader shared contract. No unreleased resolver tag is referenced.

Validation:

  • 29 pytest tests and 90 subtests passed. All 125 Node tests passed using existing dependencies from the main checkout. The existing allow-git policy blocked a fresh npm ci.
  • npm run validate-glossary, actionlint on changed workflows, offline zizmor on changed workflows and the action, and git diff --check passed. Zizmor reported zero findings and zero dangerous triggers for the changed files.
  • Repository-wide checks still report existing actionlint/shellcheck errors in four files and a zizmor issue_comment dangerous trigger in claude.yml. Those files are unchanged.

Rollout needs live branch, fork, merged PR, and Linear acceptance checks. The signal must reach next and other target branches. GitHub fork approvals and conflicted PR restrictions may affect runs. Linear issue creation and its marker write are not atomic. A failure between them can leave an issue without a marker.

Preview link

No page changes.

Internal reference

Closes DEVOPS-1803

AI review: mention @claude in a comment to request a review. Claude reads the PR and posts review comments. See CONTRIBUTING.md for example commands.

Only members of the loft-sh organization can trigger @claude. If you're contributing from outside vCluster Labs, ask a maintainer to request the review.

@netlify /docs

Public PR automation loses its default event allowance on 2 December. A read-only metadata relay keeps fork and conflicted PR events available while port and Linear jobs consume verified workflow_run metadata.
@Piotr1215
Piotr1215 requested a review from a team as a code owner November 4, 2026 15:21
@netlify

netlify Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for vcluster-docs-site ready!

Name Link
🔨 Latest commit a826e5f
🔍 Latest deploy log https://app.netlify.com/projects/vcluster-docs-site/deploys/6aed06332e25d6a4d4ad1bde
😎 Deploy Preview https://deploy-preview-3075--vcluster-docs-site.netlify.app/docs
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@djwfyi

djwfyi commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this. The design looks sound, and I checked two things that could have broken our docs flows:

  • Dropping actions/checkout is safe. docs-backport-action and docs-forwardport-action work entirely through the API (listFiles, getContent).
  • Dropping the path filters is safe. Both actions already skip unmerged PRs, check labels, and filter files themselves, so the cost is a few extra no-op runs.

Before this merges, a few concerns:

  1. Dependency on chore(ci): remove pull_request_target from privileged workflows github-actions#349. That PR is still open and awaiting review. If this merges before resolve-pr-event/v1 is released, backport, forwardport, Linear links, and follow-up issues all stop working.
  2. Release timing. Backport labels carry the 0.38/5.0 release work, with tags on 11-18 and docs publishing on 11-19. Since live acceptance is still pending, please hold the merge until after 11-19, or run the live acceptance first: a merged backport, a fork PR, and a conflicted PR.
  3. Failure visibility. workflow_run runs don't appear in the PR's checks, so we lose the backport run on the PR. The Slack alert now fires only when steps.backport.outcome == 'failure'. If the resolver step fails, for example on a missing artifact or a policy denial, the backport silently doesn't happen and nobody is alerted. Can the notify step also fire when steps.event fails?
  4. PRs into next. pull_request_target runs the relay from the base branch, so PRs into next won't trigger any of these until next has pull-request-events.yaml. That's probably fine, but please confirm it's intended.

Docs automation still needed a privileged event policy exception because its metadata relay used a privileged PR trigger. A fork could also replay another repository head without a repository identity check.

Resolve current PR state through the API in trusted default-branch consumers, bind each signal to its head repository, branch and commit, and keep credentials out of signal runs. Docs needs open PRs and nondefault base branches that the shared resolver does not support. Serialize follow-up creation and recognize prior bot links to avoid duplicate issues on replay.

Closes DEVOPS-1803
@djwfyi

djwfyi commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

The isolation design looks good. I checked that dropping the full checkout and the paths: filters is safe, because both port actions read through the API and filter files themselves. I also checked that /commits/{sha}/pulls resolves merged PRs into both main and next. A few concerns:

1. Ports re-run on every signal after merge (blocking). The resolver always gives a merged PR the action closed. docs-backport-action handles labeled by processing only the newly added label, but closed processes every backport-v* label. So any edit, new label, or reopen on a merged PR re-runs all of its backports, and duplicate detection has two gaps:

  • If a version has no backport PR because every file conflicted or nothing was copied, each signal creates another orphan backport/...-<timestamp> branch. It also posts another conflict comment (createComment, not an update) and sends another Slack warning.
  • checkExistingBackportPR and checkExistingForwardportPR only scan the 100 newest open and 100 newest closed PRs. After that window, editing an old merged PR re-creates its ports.

Could the consumers call the port actions only for the original closed or matching labeled signal? Making the actions idempotent would also work.

2. More duplicate Linear comments. linear-pr-commenter now runs on synchronize, labeled, closed, and reopened, not just opened and edited. It calls ListComments with no options, so it only sees the first 30 comments. On busy PRs, each push can repost the Linear link. The bug isn't new, but this change makes it fire much more often.

3. Minor.

  • The bootstrap test passes --require straight to node. It never exercises the quoted NODE_OPTIONS string that the workflows use.
  • The backport Slack alert fires only on resolver or backport failures, so a failed checkout sends no alert.

Before merge, it's worth running the live check of adding a backport-v* label to an already merged PR, since that's the main backport flow.

A failed resolver checkout stops backports before the existing alert condition can see a failure. Alert on any prior job failure and exercise the generated Node preload options in the bridge test.
Edits and unrelated labels on merged pull requests retrigger every port because the resolver treats each signal as a close event. Carry the signal as an untrusted hint and verify current labels before invoking a port.
Busy pull requests can receive repeated Linear links because the shared commenter only examines the first page. Read every comment page before creating any links so repeated signals remain safe.

@djwfyi djwfyi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good to merge to me, but only after we get 5.0 docs released. So after 19 Nov 2026.

@djwfyi djwfyi added the hold for release Waiting on eng or design dependencies to merge label Nov 9, 2026
@Piotr1215
Piotr1215 merged commit 2c9c17c into main Nov 10, 2026
14 checks passed
@Piotr1215
Piotr1215 deleted the devops-1803/pr-event-migration branch November 10, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold for release Waiting on eng or design dependencies to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants