Repository navigation
hosted runner label filter + integration test CI - #56
Merged
Merged
Conversation
Workflows that use self-hosted runners, custom runner labels, runner groups, or expression-based runs-on are now excluded from lockfile onboarding. Only workflows where every job runs on a known GitHub-hosted runner label (ubuntu-*, macos-*, windows-*) are eligible. The UX mirrors local-action skipping: already-onboarded workflows get an error-severity finding telling the user to migrate; not-yet-onboarded workflows get a warning and are silently skipped. The fast path in pipeline.Run also skips network resolution for these workflows. New category: self-hosted-runner (parallels local-action).
Replace prefix matching (ubuntu-*, macos-*, windows-*) with the exact label set from github/hosted-compute-core imageconfigs/imageconfigs.go. Prefix matching had false positives (accepting EOL labels like ubuntu-20.04, macos-13) and false negatives (missing codespaces-prebuild, ubuntu-slim). The hardcoded set covers standard, ARM, Intel, large/xlarge, slim, codespaces-prebuild, and firewall tech-preview labels. Staff/canary/beta labels are excluded. This list has a short shelf life — update when new images ship or old ones EOL.
The inline condition was duplicated and growing with each new skip category. Now a named function with a doc comment.
Four scenarios covering the skip/error matrix: - self-hosted label (skipped, and onboarded→error) - custom runner label (skipped) - runner group (skipped)
renderPinSummary only checked pin record state (pinned, investigated, unresolved) to decide the exit code and 'All valid' message. Error- severity findings from local-action and self-hosted-runner categories on already-onboarded workflows produced no pin records, so the summary incorrectly reported success with exit 0. Add report.IsValid() to the allClean gate and errSilent return. When the report is invalid, temporarily detach the narration log and re- render findings so the error text reaches the terminal — the log sink (io.Discard in terminal mode) was swallowing PresentResults output.
report.IsValid() catches not-pinned findings which are expected in the pre-fix report — pinning resolves them. Using it in renderPinSummary caused every live scenario to exit 1 after a successful pin. Replace with reportHasUnfixableErrors which only checks for error- severity LocalAction and SelfHostedRunner findings. These are the categories that can't be resolved by pinning and need a workflow edit. Also remove test-matrix Makefile target (was read-only catalog view, not a test runner) and its --matrix flag in run.rb.
Scenarios with input_spec (like onboarded_corrupt_recovery) need a real TTY so the binary's confirm dialog renders. The harness was using run_captured (Open3.capture3, no TTY) for all scenarios, causing the binary to detect no terminal and fail with exit 2. Switch to run_pty when input_spec is present. PTYResult already implements the same interface as Result so assertions work unchanged.
Enable dbot_impostor_blocks and dbot_forgery_blocks with full HTTP stub wiring (GraphQL resolve + reachability, REST repo metadata + branch listing + compare). Both need --rescan to bypass the lockfile fast path that skips network verification for already-recorded refs. Fix JSON exit code: impostor-commit and lockfile-forgery errors now trigger non-zero exit in --json mode. Previously only Investigated() records (autofix attempts) caused exit 1, but detection-only findings like impostor/forgery have nothing to investigate — they just fail. Add integration-stub CI job to test.yml so stub scenarios run on every PR and push to main.
Build-time version stamp differs between local go build (devel) and CI module-aware builds (v0.0.0-timestamp-hash). Strip it from both sides before diffing so the same golden fixtures work everywhere.
Runs the full integration suite (stubs + live scenarios) with GH_TOKEN from secrets. Marked continue-on-error so flakes don't block merge — integration-stub remains the hard gate.
The default Actions token has read access to public repos which is all the live scenarios need (actions/checkout, etc). No custom secret required.
The binary checks CI=true to suppress interactive prompts. On GitHub Actions this is always set, so PTY-based scenarios that need to answer a confirm dialog would fail. Unsetting CI in the PTY subprocess env lets the binary prompt normally.
There was a problem hiding this comment.
⚠️ Not ready to approve
The new stub route for branches?protected=true matches req.path (no query string), and skippedRescan accounting now includes “skipped” workflows, both of which can produce incorrect behavior/output.
Pull request overview
This PR tightens onboarding so only workflows that run on GitHub-hosted runner labels are eligible for lockfile onboarding, while ensuring already-onboarded workflows that become ineligible surface as hard errors. It also expands the integration harness to support new stubbed “impostor commit” and “lockfile forgery” detection scenarios and adds CI coverage for the stub suite.
Changes:
- Add hosted-runner label detection (
runs-on) and plumb a newself-hosted-runnerfinding through parsing/diagnosis/rendering/JSON filtering. - Improve integration harness behavior (PTY for prompt scenarios, stable golden JSON) and wire shared HTTP stubs for impostor/forgery scenarios.
- Add an
integration-stubGitHub Actions job and remove the legacy--matrix/test-matrixpath.
File summaries
| File | Description |
|---|---|
| test/scenarios/catalog.yml | Adds runner-label scenarios; enables stub scenarios for impostor/forgery with --rescan. |
| test/integration/run.rb | Adds stub helpers and constants for detection scenarios; normalizes golden JSON; removes --matrix. |
| test/integration/harness.rb | Runs scenarios with PTY when prompt input is specified; unsets CI for interactive prompt behavior. |
| Makefile | Removes test-matrix target now that --matrix is removed. |
| internal/workflowfile/runson.go | Implements hosted-runner label allowlist and YAML extraction of runs-on labels. |
| internal/workflowfile/runson_test.go | Adds unit tests for hosted label detection and runs-on parsing/flagging. |
| internal/pipeline/run.go | Skips resolver work earlier for workflows that will be skipped at diagnose time (local actions / non-hosted runners). |
| internal/pipeline/parse.go | Populates ParsedWorkflow.NonHostedRunner from workflow runs-on labels. |
| internal/pipeline/diagnose.go | Emits self-hosted-runner warning/error findings depending on onboarding state. |
| internal/pipeline/diagnose_test.go | Adds tests for the new self-hosted-runner finding behavior. |
| internal/pipeline/checks/parsed.go | Adds NonHostedRunner field to parsed workflow model. |
| internal/pipeline/checks/finding.go | Treats self-hosted-runner as non-warning when error severity, and as “valid” when not error. |
| internal/pipeline/checks/category.go | Introduces SelfHostedRunner category string. |
| internal/pipeline/checks/category_test.go | Freezes the new category string and categorization behavior in tests. |
| cmd/gh-actions-lock/pin_summary.go | Adds reportHasUnfixableErrors and ensures unfixable errors surface on terminal runs. |
| cmd/gh-actions-lock/format/terminal.go | Renders self-hosted-runner warnings in the terminal summary buckets. |
| cmd/gh-actions-lock/format/json.go | Refactors JSON suppression logic; omits self-hosted-runner warnings from JSON while keeping errors. |
| cmd/gh-actions-lock/check.go | Uses reportHasUnfixableErrors to ensure JSON mode exits non-zero on unfixable error findings. |
| .github/workflows/test.yml | Adds integration-stub job and keeps integration-live as non-blocking. |
Copilot's findings
- Files reviewed: 19/19 changed files
- Comments generated: 4
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Local-action and non-hosted-runner workflows are excluded, not trusted — they shouldn't count toward the 'Trusted lockfile for N already-pinned workflows' summary line. Also fix misleading comment in HasNonHostedRunnerLabels about run-only workflows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Filters onboarding to hosted-runner-only workflows, fixes exit code bugs, wires impostor/forgery stub scenarios, adds integration-stub CI job.