Skip to content

ci: automatic advisory Quality Keeper runs on same-repo PRs (#146)#127

Open
sophiecarreras wants to merge 2 commits into
mainfrom
claude/auto-pr-runs-146
Open

ci: automatic advisory Quality Keeper runs on same-repo PRs (#146)#127
sophiecarreras wants to merge 2 commits into
mainfrom
claude/auto-pr-runs-146

Conversation

@sophiecarreras

Copy link
Copy Markdown
Contributor

What this changes

Implements the SDK pilot in backblaze-labs/quality-keeper#146: Quality Keeper now runs automatically on same-repo pull requests to main, and stays advisory / non-blocking. Extends the existing .github/workflows/quality-run-demo.yml (no new file).

  • Trigger: pull_request (NOT pull_request_target) on branches: [main], types opened/synchronize/reopened/ready_for_review. workflow_dispatch retained for manual demo/debug.
  • Same-repo only / fork-safe: both jobs carry if: event == workflow_dispatch || head.repo.full_name == github.repository, so fork PRs skip entirely — untrusted fork code never runs in a job that can reach secrets or PR-write authority. (GitHub also withholds secrets / forces a read-only token on fork pull_request runs — defense in depth.)
  • Two-job split preserved: quality-run (contents: read, runs the target's own code, QK_INSTALL_TOKEN only, no App key) → comment (pull-requests: write + actions: read, runs no target code, mints the App token downscoped to pull-requests: write).
  • Advisory / non-blocking: run-found-checks exits non-zero when a safe check fails; that exit code is captured as advisory evidence (execution.json is written first), so a failing SDK check does not turn QK into a red/blocking check — while genuine QK errors (assess / post-execute) still fail. This workflow is not a required check and does not gate merge.
  • Injection-safe: comment target = inputs.issue_url (manual) or github.event.pull_request.html_url (auto), passed via env: and quoted — no PR-controlled text expanded inline.
  • Idempotent one-comment upsert, --expect-repo, and Repository Activation preserved. Per-PR concurrency with cancel-in-progress. All actions SHA-pinned with version comments.

No seeds, branches, issues, PRs, or auto-fixes are created.

Security / verification

  • zizmor clean at regular and pedantic personas.
  • YAML valid; triggers = pull_request + workflow_dispatch (no pull_request_target); all uses: SHA-pinned.
  • The repo's GitHub Actions security check (security.ymlgha-security) runs on this PR and should pass.
  • This PR is itself the first live automatic proof (a same-repo PR, not Gonzalo's): opening it triggers the advisory QK run, which should post/update one comment as quality-keeper[bot]. The run + comment will be linked back to quality-keeper#146.

Residual risk (documented, follow-up proposed)

QK_INSTALL_TOKEN is reachable by same-repo (trusted-collaborator) PR code in quality-run, because quality-keeper is private and must be pip-installed there. It must remain a read-only, repo-scoped credential. Safer follow-up: eliminate the install secret from the code-running job by publishing quality-keeper to an internal registry / prebuilt image / making it public.

Refs backblaze-labs/quality-keeper#146

🤖 Generated with Claude Code

…blocking)

Enable the SDK pilot (#146): Quality Keeper now runs automatically when a
same-repo pull request to main is opened/updated, staying advisory and
non-blocking. Manual workflow_dispatch is retained for demo/debug.

- Trigger: pull_request (NOT pull_request_target) on branches [main],
  types opened/synchronize/reopened/ready_for_review.
- Same-repo only: both jobs carry an `if:` guard
  (event is workflow_dispatch, or head.repo.full_name == github.repository),
  so fork PRs skip entirely — untrusted fork code never runs in a job that can
  reach secrets or PR-write authority.
- Two-job split preserved: quality-run (contents: read, runs target code,
  QK_INSTALL_TOKEN only, no App key) → comment (pull-requests: write +
  actions: read, runs no target code, mints App token downscoped to
  pull-requests: write).
- Advisory/non-blocking: run-found-checks exits non-zero on a failing safe
  check; that exit code is captured as advisory evidence (execution.json is
  written first) so findings don't turn QK into a red/blocking check, while
  genuine QK errors (assess/post-execute) still fail. Not a required check.
- Comment target derives from inputs.issue_url on manual runs and
  github.event.pull_request.html_url on auto runs; passed via env and quoted
  (no PR-controlled text expanded inline). --expect-repo + Repository
  Activation + idempotent one-comment upsert preserved.
- Concurrency grouped per PR with cancel-in-progress so rapid pushes never
  stack or double-comment. All actions remain SHA-pinned with version comments.

Verified: YAML valid; zizmor clean (regular + pedantic); no pull_request_target
trigger; SHA pins intact. No seeds/branches/issues/PRs/auto-fixes.

Refs backblaze-labs/quality-keeper#146

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@quality-keeper

Copy link
Copy Markdown

Quality Keeper — Quality Run

Source: backblaze-labs/b2-sdk-typescript

Quality Keeper assessed the source, executed checks, and collected results. This is a read-only run overview; it changes no Evidence State, and seeded scaffolds are next actions, not evidence, until applied and re-evaluated.

10 of 14 expectations verified or exercised. Remaining outcomes are listed below — executed-but-not-exercised, pending, missing, and review are separate from verified/exercised and are not counted as passing.

Outcome Count
Verified before execution 3
Exercised this run 7
Selected, not executed 2
Missing — no seed proposal 2

Gaps and next actions

  • Missing — no seed proposal · Contract Testing · public_or_network_boundaries_need_contract_testing — No seed proposal — an operator must design acceptable evidence for this expectation.
  • Selected, not executed · Environmental Isolation Testing · external_resource_assets_need_environmental_isolation_testing — It was selected but produced no result — opt in (e.g. --include-expensive) or fix the command; do not just re-select it.
  • Selected, not executed · Integration Testing · published_sdk_packages_need_integration_testing — It was selected but produced no result — opt in (e.g. --include-expensive) or fix the command; do not just re-select it.
  • Missing — no seed proposal · Static Security Analysis · code_and_configuration_assets_need_static_security_analysis — No seed proposal — an operator must design acceptable evidence for this expectation.

New to these terms? See the Quality Keeper glossary.

Review follow-ups on PR #127:
- Skip draft PRs: the same-repo guard now also requires
  `github.event.pull_request.draft == false`, so the advisory comment appears
  only once the author marks the PR ready (`ready_for_review` re-triggers with
  draft == false). Manual dispatch is unaffected.
- Correct the inline rationale: check FINDINGS do not fail the job, but a genuine
  QK error (no execution.json written) still fails step 3 — replaces the
  inaccurate "execution.json is always written first" claim.
- Note the concurrency trade-off: a superseded intermediate commit may not get
  its own comment (latest SHA wins) — intended for an advisory signal.
- Kept the mirrored guard on the write-capable `comment` job deliberately
  (defense in depth), with a comment explaining why it is not removed despite
  being redundant with `needs: quality-run` today.

Verified: YAML valid; both job guards carry the same-repo + non-draft condition;
zizmor clean (regular + pedantic).

Refs backblaze-labs/quality-keeper#146

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sophiecarreras

Copy link
Copy Markdown
Contributor Author

Review findings addressed (commit a9556c7)

Verified: YAML valid; both guards carry same-repo + non-draft; zizmor clean (regular + pedantic). This push re-triggers the advisory run on this (non-draft) PR, confirming the draft gate doesn't affect normal same-repo PRs.

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