Run the integration tests against the published conductor image - #7
Merged
Conversation
The tier-3 harness required an explicit conductor source and, in practice, a local checkout: no published tag carried the v2 API, so CI gated the whole integration job on a CONDUCTOR_IMAGE repo variable that was never set. The job has therefore never run. conductor 0.17.0 is the first release cut from a main that includes controllers/apiv2 and the openapi subcommand, and it is on Docker Hub as a multi-arch image, so latest now serves an API this CLI can talk to. Default to it: - conductortest picks its source in precedence order — CONDUCTOR_DIR (build from a checkout, the escape hatch for unreleased conductor changes), then CONDUCTOR_IMAGE (a specific tag, used as-is so a locally built one works), then dbosdev/conductor:latest. Nothing to configure for the normal path. - The default sets AlwaysPullImage: the tag moves, and testcontainers would otherwise reuse whatever copy a machine had cached and quietly test an old conductor. It is scoped to the default so an explicit CONDUCTOR_IMAGE that no registry serves still works. - The CI integration job loses its `if:` gate. It now needs only Docker; the license key still gates the tier at runtime, so fork PRs (no secret) skip the tests and the job stays green. CONDUCTOR_IMAGE is still passed through, so the repo variable can pin a tag without a code change. Tracking a moving tag means a conductor release can turn this suite red with no commit here. That is the intent — an API break should surface the day it lands — and the usual cause is the vendored spec drifting from the released API, which AGENTS.md now documents how to re-vendor straight from the image.
`workflow fork` is the first command TestWorkflowMutationsIntegration has conductor dispatch to the executor, and it ran once with no retry. Executor registration arrives over the WebSocket asynchronously after Launch, so the fork can beat it and fail with "no healthy executors available" — observed once in ten runs against the published image. Poll it for 30s, matching what E1/E3/E4/D4 already do with their first dispatched command, and keep the last error so a real failure still reports the cause rather than an empty fork ID.
GitHub is deprecating the Node 20 runtime: checkout@v4 and setup-go@v5 target it and are already being force-run on Node 24, with a warning on every run. Bump to the current majors, both of which declare node24. Neither major carries a breaking change that reaches us: setup-go v6/v7 are dependency and ESM migrations, and checkout v6 moves credentials to a separate file while v7 blocks fork checkouts for pull_request_target and workflow_run — this workflow triggers on pull_request.
Every push to a PR branch started a full parallel run, including the ~2-minute container-backed integration job, while the run it obsoleted kept burning a runner to report on code nobody would merge. Group runs per PR and cancel in progress. Scoped to pull_request via the cancel-in-progress expression: pushes to main keep every run, so main keeps an unbroken per-commit history to bisect. The group key falls back to github.ref for push events, where head_ref is empty.
The checks list says pass/fail and nothing else, which is too coarse here: the conductor tier skips wholesale when the license key is absent, so a job that ran nothing looks exactly like a job that ran everything. This PR's own first CI run was that case — green, with every conductor test skipped. Route the test targets through gotestsum (added as a module tool, like oapi-codegen) when JUNIT names an output path, and have both CI jobs publish the XML two ways: test-summary/action for a summary on the run page, and upload-artifact for the raw file. Both if: always() — a failing run is when the report matters most. `show: "fail, skip"` surfaces the skips with their reason, which is the part the checks list cannot show. Without JUNIT the targets stay a plain `go test`, so local runs are unchanged and build no extra tooling. Matches the Java repo's setup.
kraftp
approved these changes
Aug 13, 2026
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
2 participants
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.
The tier-3 (conductor + Postgres container) tests have never run in CI. The harness demanded an explicit conductor source and, in practice, a local checkout — no published tag carried the v2 API — so the integration job was gated on a
CONDUCTOR_IMAGErepo variable that was never set.conductor
0.17.0is the first release cut from amaincontainingcontrollers/apiv2and theopenapisubcommand, sodbosdev/conductor:latestnow serves an API this CLI can talk to. Default to it.Changes
internal/conductortest— conductor source resolves in precedence order:CONDUCTOR_DIR(build from a checkout; the escape hatch for unreleased conductor changes) →CONDUCTOR_IMAGE(a specific tag, used as-is so a locally built one works) →dbosdev/conductor:latest. Nothing to configure for the normal path; the old "fail if neither is set" branch is gone.AlwaysPullImageon the default only. The tag moves, and testcontainers otherwise reuses whatever copy a machine cached — a developer who pulledlatestonce would quietly keep testing an old conductor. Scoping it to the default keeps an explicitCONDUCTOR_IMAGEthat no registry serves working.if:gate. It needs only Docker now. The license key still gates the tier at runtime, so fork PRs (no secret) skip the tests and the job stays green.vars.CONDUCTOR_IMAGEis still passed through, so a tag can be pinned without a code change.workflow forkis the first commandTestWorkflowMutationsIntegrationhas conductor dispatch to the executor, and it ran once with no retry. Executor registration arrives over the WebSocket asynchronously afterLaunch, so the fork could beat it and fail withno healthy executors available(hit once in ten runs). It now polls for 30s like E1/E3/E4/D4 already do, keeping the last error so a genuine failure still reports its cause..env.exampleand AGENTS.md rewritten for the image-by-default model, including how to re-vendor the spec straight from the image (make specstill needs a checkout).Verification
Locally, with
CONDUCTOR_DIRandCONDUCTOR_IMAGEboth unset — the exact path CI takes — the full suite is green in ~70s, with no image build. I deleted the locallatesttag first, so the pull path is exercised rather than just declared.make lint,go vet -tags integration, and the unit tests pass. The flaky test was then run 5×, all green.The trade-off
Tracking a moving tag means a conductor release can turn this job red with no commit in this repo. That is the intent — an API break surfaces the day it ships rather than at the next deliberate bump — but it is a real change in what a red CI means here. The usual cause will be the vendored spec drifting from the released API; set the
CONDUCTOR_IMAGErepo variable to an older tag to confirm a failure arrived with a release.Note that
main's vendored spec is already one release behind the image: it still marksGET /v2/orgs/{orgName}/permissionsas OAuth-gated, which conductor dropped in dbos-inc/conductor#177. No integration test hits that route, so the suite passes either way, and the in-flightupdate-openapi-specbranch fixes it — its vendored spec is byte-identical to what the0.17.0image emits.🤖 Generated with Claude Code