Skip to content

feat: add v1 PR workflows with opt-in deb checks - #60

Merged
mairas merged 4 commits into
mainfrom
feat/v1-pr-checks
Sep 17, 2026
Merged

mairas merged 4 commits into
mainfrom
feat/v1-pr-checks

Conversation

@mairas

@mairas mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the v1 PR-side workflows from the interface plan (issue 49). A repository no longer opts out of deb checks: checks.yml has no packaging in it, and a deb repository adds an explicit call to build-deb.yml. The legacy pr-checks.yml stays unchanged for callers on @main.

Changes

  • checks.yml: tests, version-check (optional action, now .yml or .yaml), and the nested version-bump-check. runs-on applies to every job.
  • build-deb.yml: builds with the caller's build-deb action, finds packages in the repository root and build/, and fails when there are none. Lintian runs on each package unless lintian: false. The zero-package failure and the build/ lookup address the silent pass from issue 32 for callers of the new workflow; the legacy workflows keep that behaviour until they are removed.
  • version-bump-check.yml: .coderabbit.yaml and .coderabbit.yml no longer count as package-affecting files. This was the only change in the hatlabs copy that halos-org lacked.
  • Examples: examples/deb and examples/npm pr.yml, each ending in a status job that needs every other job and passes only if all of them succeeded. They trigger on every pull_request without a branch filter, so a stacked PR is checked before it is retargeted to main.

build-deb.yml has no release mode yet; changelog generation, renaming and artifact upload come with the release pipeline (issue 52).

Verification

Canary PRs pointed pr.yml at this branch and were closed afterwards:

Canary Jobs Result
halos-browser-trust PR 3 (deb) checks (3 jobs), build-deb, status all pass; lintian checked ./halos-browser-trust_0.1.0_all.deb
skip-freeboard-panel PR 3 (npm) checks (3 jobs), status all pass; no deb jobs
same PR, extra job with if: false in status.needs never-runs skipped status fails
halos-browser-trust PR 4: valid package in build/, invalid bogus.deb in root build-deb, status both fail; lintian checked ./bogus.deb and build/halos-browser-trust_0.1.0_all.deb
same PR, only the valid package in build/ build-deb, status both pass
same PR, invalid bogus.deb with lintian: false build-deb, status both pass; lintian steps skipped
same PR, build action deletes its output build-deb, status both fail: "produced no .deb in the repository root or build/"
  • The nested uses: ./.github/workflows/version-bump-check.yml ran as version-bump-check.yml@refs/heads/feat/v1-pr-checks at the branch head, so a relative nested call follows the caller's pinned ref. This settles one of the plan's deferred questions for branch refs; the rc canaries confirm it for tags.
  • actionlint passes on workflows and examples; the status expression was also checked locally against success, skipped, failure and cancelled results.

Not exercised by a run: the action.yaml spelling of check-versions, a caller without a run-tests action, a non-default runs-on, a PR changing only .coderabbit.yaml, a cancelled upstream job in status (checked as a local expression test), and the nested call under a tag (rc canaries).

Closes #51


🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5d4c77ec-f546-46b5-90e9-a16f28bd6e97

📥 Commits

Reviewing files that changed from the base of the PR and between ac12e8a and 17fc5cf.

📒 Files selected for processing (5)
  • .github/workflows/build-deb.yml
  • .github/workflows/checks.yml
  • .github/workflows/version-bump-check.yml
  • examples/deb/.github/workflows/pr.yml
  • examples/npm/.github/workflows/pr.yml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mairas

mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Code Review Results

Scope: merge-base ac12e8afeat/v1-pr-checks (5 files, +179/−1)
Intent: Add the v1 PR-side reusable workflows (checks.yml, build-deb.yml), port the .coderabbit exclusion into version-bump-check.yml, and add example callers with a caller-owned status job. CodeRabbit was rate-limited on this PR, so this review replaces it.
Mode: interactive (PR-first)

Reviewers: correctness, testing, maintainability, project-standards, api-contract, adversarial

  • api-contract: two new workflow_call interfaces that become part of v1
  • adversarial: 179 lines of CI that gate branch protection and packaging

P2 -- Moderate

# File Issue Reviewer Confidence Route
1 .github/workflows/build-deb.yml:43 The two behaviours that fix issue 32 never ran: a package found only in build/, and failure when the build produces no package. The deb canary built into the repository root. testing, correctness, api-contract, adversarial 0.95 manual -> human (canary runs)
2 .github/workflows/build-deb.yml:60 The lintian failure path (failed=1 through the here-string loop) and lintian: false never ran. A bug in the loop would turn lintian into a pass-always check. testing, maintainability, correctness 0.90 manual -> human (canary runs)

P3 -- Low

# File Issue Reviewer Confidence Route
3 examples/deb/.github/workflows/pr.yml:5 branches: [main] means a PR stacked on a feature branch never runs, so after retargeting it has no status until a push. Downgraded from P2: callers' protection is strict, which forces that push anyway. Same in examples/npm. adversarial 0.70 safe_auto -> review-fixer
4 .github/workflows/version-bump-check.yml:2 Header still names only pr-checks.yml as a caller; checks.yml is now the v1 caller. api-contract, project-standards 0.80 safe_auto -> review-fixer
5 PR body Verification section reads as complete but several Unit 2 scenarios did not run: action.yaml detection, missing run-tests action, runs-on arm64, a .coderabbit.yaml-only PR, cancelled upstream job, nested call under a tag. testing, maintainability 0.75 advisory -> human

Coverage

  • Suppressed: 0 findings below 0.60.
  • Dismissed: "hatlabs repos should use hatlabs/shared-workflows" (adversarial, residual). The plan in issue 49 merges that copy into this repo; the workspace AGENTS.md is updated when it is retired.
  • Residual risks carried to later units:
    • Unit 3 (issue 52): release inputs on build-deb.yml must be optional with check mode as the default, or adding them breaks v1 callers; a partial set of release inputs must fail loudly; release mode must reuse the root + build/ lookup; check mode must not start generating a changelog within v1.
    • Unit 7 (issue 56): container-app packages have never been through lintian in CI. Run a canary on halos-marine-containers before migrating it, or set lintian: false explicitly.
    • checks.yml works only on pull_request events (nested version-bump-check reads the PR base ref), same as legacy. Document in the Unit 5 README.
    • The status gate is only as strong as the needs list in the PR head's pr.yml; edits to pr.yml are edits to the merge gate.
    • A duplicate .deb in both root and build/ is linted twice; harmless in check mode.
  • Testing gaps: covered by findings 1, 2 and 5.

Verdict: Ready with fixes

Reasoning: No defects found in the code. The two new failure paths that justify build-deb.yml and its lintian input have not run, and that must be shown before this becomes part of v1.

Fix order: canary runs for findings 1 and 2 → fixes 3 and 4 → PR body lists what remains unverified (finding 5)

mairas and others added 4 commits September 17, 2026 13:04
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mairas

mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Force-pushed review fixes, folded into their commits:

  • version-bump-check.yml header names checks.yml as a caller (finding 4).
  • Example pr.yml files drop branches: [main], so stacked PRs are checked before retargeting (finding 3).

Findings 1 and 2 are verified by canary runs on halos-browser-trust PR 4, now in the PR description table: a package in build/ is found and linted, a lintian failure fails build-deb and status, lintian: false skips both lintian steps, and a build with no package fails with the zero-package error. The scenarios still not exercised are listed in the description (finding 5). Unit 3 constraints went to issue 52, the container-repo lintian note to issue 56. actionlint passes.

@mairas
mairas merged commit 34e25f1 into main Sep 17, 2026
4 checks passed
@mairas
mairas deleted the feat/v1-pr-checks branch September 17, 2026 10:15
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.

v1 Unit 2: PR-side workflows — checks.yml, build-deb.yml check mode

1 participant