Skip to content

ci: replace pull_request_target demo-video flow - #206

Draft
samcday wants to merge 2 commits into
mainfrom
agent/ci-demo-videos
Draft

ci: replace pull_request_target demo-video flow#206
samcday wants to merge 2 commits into
mainfrom
agent/ci-demo-videos

Conversation

@samcday

@samcday samcday commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • move ordinary PR CI from pull_request_target to read-only pull_request runs, removing the unsafe fork-head checkout and the ci-ok gate
  • keep ordinary CI output to one raw test-recordings MP4 artifact with 30-day retention
  • add an exact, one-shot /demo-videos maintainer command
  • split that command into three explicit trust zones:
    1. resolve — authorize the commenter, snapshot the PR head, select the matching successful CI run, and create one progress comment
    2. render — check out the exact PR commit and run Editly with no network or secrets
    3. publish — on a fresh runner with no PR checkout, upload only the rendered WebPs to B2 and update the same comment
  • narrow build-image/release permissions and stop forwarding secrets to the reusable build

The command workflow is 359 lines. This is the compact rewrite of the initial 740-line version: it keeps the trust boundary and drops the defensive state machine, ownership markers, run-attempt bookkeeping, repeated probes, and duplicate validation.

This replaces the flow currently failing on #205 after actions/checkout began refusing fork code in a privileged pull_request_target run.

Validation

  • this revision's live pull_request run passed build, Clippy, formatting, Wayland tests, and raw recording upload
  • run 31158986225 produced exactly one unexpired test-recordings artifact (1.8 MB)
  • replayed the resolver against a real PR run and confirmed exact SHA/repository matching
  • rendered the real six-MP4 artifact locally with the pinned Editly image and the workflow's sandbox flags; all seven expected WebPs passed publisher validation
  • yq parses all four touched workflows
  • actionlint passes for build.yml, build-image.yml, and demo-videos.yml
  • zizmor --offline .github/workflows/demo-videos.yml: no findings
  • every added shell block passes bash -n and warning-level ShellCheck
  • git diff --check

release.yml still has its two pre-existing ShellCheck findings (SC2129/SC2209); this change does not add any there.

Review / rollout notes

GitHub only loads an issue_comment workflow from the default branch, so this PR can validate the ordinary PR build but cannot exercise /demo-videos end-to-end on itself. The first meaningful integration test is a command on an open PR after merge.

Because the default branch still contains the old trigger, pushes to this transition PR also launch a pull_request_target run. The shared concurrency group cancels it, leaving canceled red entries beside the passing replacement run. That overlap disappears once this lands.

B2 objects use the dedicated immutable ci-previews/v1/ prefix. The application key should be bucket/prefix-limited, and the bucket needs an explicit lifecycle choice: indefinite retention preserves old PR comments; finite retention bounds storage.

A PR changing .github/Dockerfile will need its matching CI image built manually through build-image.yml.

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.

1 participant