Skip to content

Wait for green CI before a job is accepted (publish.ci) - #12

Merged
Steel-tech merged 1 commit into
mainfrom
claude/overview-use-cases-t3fj4h
Sep 28, 2026
Merged

Steel-tech merged 1 commit into
mainfrom
claude/overview-use-cases-t3fj4h

Conversation

@Steel-tech

@Steel-tech Steel-tech commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

What

Opt-in CI babysitting, part A of two. This part watches CI and makes green CI a condition of accepted. The auto-fix loop comes in a follow-up PR.

publish:
  hold_when: "risk != low"
  ci: {wait: true, timeout: 30m}   # default 30m, allowed 1m–6h
  • A fourth fenced publish step, ci, runs after push → pull_request → proof.
    • The worker polls check runs and commit statuses on the branch's current remote head with gh api --paginate --jq.
    • Each signal is normalized to pending/pass/fail. Neutral and skipped count as pass.
  • Outcomes:
    • All green: ci is recorded and the job is accepted.
    • Red: fails fast as ci_failed, naming the failing checks.
    • Still pending past the timeout: ci_timeout.
    • No checks after a 2-minute registration grace: pass.
    • Three reads in a row fail: ci_unavailable.
    • Cancel: ci_wait_cancelled.
    • Every failure ends accepted_unpublished. The branch and PR stay in place, and ci_ref/ci_failures are in the result.
  • The control plane enforces it. CompleteAttempt returns publish_ci_required when the run's frozen definition waits and the ledger has no ci record. It mirrors the proof rule and does not rely on what the worker reports.
  • The publish-only retry now carries the run snapshot, so it applies the same policy. It judges the branch's current head, which means a fix a person pushes to the branch counts.
  • Migration 004 rebuilds publish_records to add 'ci' to the step CHECK. Rows are copied across, and a test covers that.
  • factory.yaml opts in. Definitions without ci behave as before.

Trade-offs

  • The wait holds a worker slot for up to the timeout. That's why it is opt-in and bounded.
  • Passing a head with no checks after 2 minutes assumes check runs register within that window. GitHub Actions normally registers them within seconds.
  • CI is judged on the branch head, which may be a commit a person pushed rather than jig's commit. The proof record keeps jig's own commit, so worktree cleanup is unchanged.

Tests

  • Protocol: opt-in behaviour and timeout bounds.
  • Migration: rows are preserved and the step vocabulary is widened.
  • Ledger: step order and SHA requirement for ci, the accepted gate with and without opt-in, and the retry carrying the snapshot.
  • Worker, against a real bare remote with scripted CI:
    • pending → green gives accepted;
    • red fails fast, then a person's fix plus retry gives accepted on the new head;
    • the no-checks grace;
    • timeout, cancel, repeated transient errors, and a single transient error.
  • Parsing of gh api output.

format-check, vet, boundary, definitions, build, test and -race are all green. The exceptions are two TestAManifestWriteFailure… tests that fail only as root in the cloud container: they fail the same way on unchanged main and pass as a non-root user.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LwarQhEaL1ygzyMLvaXvZH


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Publishing can optionally wait for pull-request CI checks, with a configurable timeout from 1 minute to 6 hours (30 minutes by default).
    • Successful or neutral checks allow publishing to complete. Failed or timed-out checks leave the job accepted but unpublished, with the branch and pull request available.
    • Retrying publication checks CI against the branch’s latest commit. If no checks appear within two minutes, the wait passes.
  • Documentation
    • Updated the authoring guide, quickstart, and example to explain CI-gated publishing and retries.

A definition can now opt into `publish: {ci: {wait: true, timeout: 30m}}`.
After push, pull request, and proof, publish runs a fourth fenced step: it
polls the check runs and commit statuses on the branch's current remote
head via `gh api`, and records `ci` on the ledger only when every signal is
green. Red fails fast, pending past the timeout fails, a head with no checks
passes after a two-minute registration grace, three consecutive read errors
end the wait, and a cancel stops it. Every failure ends the job
accepted_unpublished with the branch and pull request in place and the red
checks named in the result.

The control plane enforces the rule itself: CompleteAttempt refuses
`accepted` without a ci record when the run's frozen definition waits for
CI (publish_ci_required), mirroring the proof requirement. The publish-only
retry now carries the run snapshot, so it re-applies the same policy and
judges whatever the branch head is by then, which lets a person's fix count.

Migration 004 rebuilds publish_records to widen its step CHECK with 'ci'.
factory.yaml opts in; definitions without `ci` are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwarQhEaL1ygzyMLvaXvZH
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ba9727c8-e881-4bed-9178-bc49d04d9f0e

📥 Commits

Reviewing files that changed from the base of the PR and between 88835e7 and be9b795.

📒 Files selected for processing (15)
  • README.md
  • docs/quickstart.md
  • examples/definitions/factory.yaml
  • internal/controlplane/migrate_test.go
  • internal/controlplane/publish_ledger.go
  • internal/controlplane/publish_ledger_test.go
  • internal/controlplane/store.go
  • internal/protocol/definition.go
  • internal/protocol/definition_test.go
  • internal/protocol/publish.go
  • internal/worker/publish.go
  • internal/worker/publish_ci_test.go
  • internal/worker/publish_test.go
  • internal/worker/worker_integration_test.go
  • migrations/004_publish_ci_step.sql

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Publishing can optionally wait for pull request CI. The worker polls checks on the current branch head and records the CI result. The control plane requires that result before accepting runs whose frozen definition enables CI waiting.

Changes

CI-gated publishing

Layer / File(s) Summary
CI policy and publish-step contract
internal/protocol/definition.go, internal/protocol/definition_test.go, internal/protocol/publish.go, README.md, docs/quickstart.md, examples/definitions/factory.yaml
Definitions can enable CI waiting and set a timeout from one minute to six hours, with a 30-minute default. The publish-step contract and example documentation describe the CI gate and retry behavior.
Worker CI polling and retry
internal/worker/publish.go, internal/worker/publish_ci_test.go, internal/worker/publish_test.go, internal/worker/worker_integration_test.go
The worker polls check runs and commit statuses for the branch’s current head. It handles pass, failure, timeout, cancellation, and transient read errors; retries evaluate the current head.
CI ledger and acceptance enforcement
migrations/004_publish_ci_step.sql, internal/controlplane/publish_ledger.go, internal/controlplane/store.go, internal/controlplane/publish_ledger_test.go, internal/controlplane/migrate_test.go
The ledger accepts CI records with SHA validation. Acceptance requires a CI record when the frozen definition enables waiting. Migration 004 adds ci to the stored publish-step values and copies existing records.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Worker
  participant GitHubCLIGateway
  participant GitHubAPI
  Worker->>GitHubCLIGateway: Request checks for current branch head
  GitHubCLIGateway->>GitHubAPI: Read check runs and commit statuses through gh api
  GitHubAPI-->>GitHubCLIGateway: Return check runs and statuses
  GitHubCLIGateway-->>Worker: Return normalized check verdicts
  Worker->>GitHubCLIGateway: Poll again while checks are pending
Loading

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to be9b7

The one-minute timeout behaves as a hard limit, and no merge-blocking issue remains after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to be9b7

The optional CI gate adds a useful acceptance control, but a job can be accepted after checks pass on a head that is no longer current, or after no checks appear during the registration grace period. Exposure depends on who can change the attempt branch and how its CI is configured.

Retained concerns

  • Medium · security · inferred: A passing CI record can authorize accepted after the attempt branch advances: polling returns an observed SHA, while ledger recording and acceptance do not establish that it remains the branch head. This matters where another actor can update that branch during the transition.
  • Medium · security · observed: The new CI gate treats an empty check inventory as success after the registration grace period. An opted-in job can therefore be accepted without a passing check if no signals are reported; whether a branch writer can induce that condition depends on repository CI controls.
Security review details

Security Blast Radius

  • inferred — The directly evidenced exposure is an opted-in attempt and its published branch and pull request. Its practical attackability depends on branch-write authority and CI configuration, neither of which is established here.

Security Findings and Attack Paths

  • inferred — If an actor can advance the attempt branch after a passing poll but before completion, accepted can refer to CI observed on the previous head. Separately, a head for which no checks are reported passes after the grace period without a passing CI signal.

Trust Boundaries and Controls

  • observed — Worker observations cross into control-plane authority through lease-fenced, attempt-scoped ledger records. SHA shape, branch identity, prerequisite ordering, and the frozen-definition check constrain that boundary, but do not attest that CI ran on the head current at acceptance.

Resilience and Maintainability Implications

  • observed — CI read failures, failed checks, and timeout do not produce a successful CI step in the worker's wait path. A retry preserves the attempt's policy and ledger history, while acceptance updates remain transactional.

Hardening Proposals

  • proposed — Define an acceptance-time relationship between the CI-attested SHA and the branch head, while continuing to permit an intentionally updated head on retry; make that relationship explicit in the ledger or completion contract.
  • proposed — For deployments using this gate as a security assurance, require an explicitly configured set of checks or make zero-check success a separate policy choice rather than treating absence as green.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 11 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: jobs with opt-in publish.ci wait for green CI before acceptance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 11 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@Steel-tech
Steel-tech merged commit b2b6325 into main Sep 28, 2026
5 checks passed
@Steel-tech
Steel-tech deleted the claude/overview-use-cases-t3fj4h branch September 28, 2026 05:25
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.

2 participants