diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 459fde5..ed1501a 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -33,22 +33,86 @@ permissions: pull-requests: write env: - # The server is built from source because releases are cut from conductor-oss - # main and no artifact is published from it: Maven's newest is a tagged RC and - # the S3 'latest' jar is months stale. Building means the CLI is tested against - # what will actually ship. - # - # TODO(#105): revisit pinning to the newest published RC instead. RCs are cut - # from main, so an RC is a main snapshot, and pulling a cached jar is faster and - # decouples this repo's CI from the server repo's build health. See the tracking - # issue linked in ADR-0001. - CONDUCTOR_SERVER_REF: 'main' # Scratch directory the local server is started from. `conductor server start` # writes its SQLite database relative to the working directory with no flag to # override it (see #104), so it must not run from the repo root. SERVER_WORKDIR: /tmp/conductor-e2e + # Bucket `conductor server start --version` downloads from. Named here so the + # preflight can check a pin without reimplementing the CLI's URL construction. + SERVER_JAR_BUCKET: https://conductor-server.s3.us-east-2.amazonaws.com jobs: + # --------------------------------------------------------------------------- + # Declares which published Server Versions the server-backed jobs pin, and + # proves each one is downloadable before those jobs do any real work. + # + # The bucket carries only a subset of the server repo's tags, so a version + # existing in conductor-oss/conductor does not mean it can be pinned here. + # Without this check a bad pin surfaces minutes into a job as an opaque + # download failure. + # --------------------------------------------------------------------------- + server-versions: + name: Pinned Server Versions + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.verify.outputs.matrix }} + blocking: ${{ steps.verify.outputs.blocking }} + steps: + - name: Declare the pinned Server Versions + run: | + # THE place to bump a pin. One entry per Server Version to test, at the + # newest that line has actually published. Exactly one entry is blocking + # — the current release, which is what users are running. + # + # Only the current release is pinned. An older line can be added as an + # entry with "blocking": false, which reports against that line without + # gating a merge; ADR-0006 records why none is pinned today. + # + # Pin a fixed Server Version, never `latest`: `latest` is mutable + # content, which loses reproducibility and poisons a cache keyed on it. + cat > pins.json <<'JSON' + [ + { "version": "3.32.0", "blocking": true } + ] + JSON + jq -e 'map(select(.blocking)) | length == 1' pins.json > /dev/null || { + echo "::error::exactly one pinned Server Version must be blocking" + exit 1 + } + + - name: Verify every pinned Server Version is published + id: verify + run: | + # The URL is built the same way cmd/server.go builds it. That is a second + # copy of the shape, accepted so the check can run before the CLI is even + # built; if the template there changes, change it here too. + # + # A missing *blocking* pin is fatal. A missing older pin only drops its + # own leg, with a warning — failing the whole preflight would skip the + # blocking leg too, and an older-version problem must never gate a merge. + available='[]' + fatal=0 + while read -r row; do + version=$(jq -r '.version' <<<"$row") + blocking=$(jq -r '.blocking' <<<"$row") + url="$SERVER_JAR_BUCKET/conductor-server-$version.jar" + # -L because the bucket may redirect; %{http_code} is 000 on a network + # error, which fails the comparison just as a 403 does. + code=$(curl -sSL -o /dev/null -w '%{http_code}' -I "$url" || true) + if [ "$code" = "200" ]; then + echo "ok $version $url" + available=$(jq -c --argjson row "$row" '. + [$row]' <<<"$available") + elif [ "$blocking" = "true" ]; then + echo "::error::Blocking Server Version $version is not published: HEAD $url returned $code. The bucket carries a subset of conductor-oss/conductor's tags, so a git tag is not enough — check with a HEAD before pinning." + fatal=1 + else + echo "::warning::Non-blocking Server Version $version is not published (HEAD $url returned $code), so its leg is dropped from this run. Pin a version the bucket carries, or remove the entry." + fi + done < <(jq -c '.[]' pins.json) + [ "$fatal" -eq 0 ] || exit 1 + echo "matrix=$available" >> "$GITHUB_OUTPUT" + echo "blocking=$(jq -r 'map(select(.blocking))[0].version' <<<"$available")" >> "$GITHUB_OUTPUT" + # --------------------------------------------------------------------------- # Remote Enterprise server. Covers the Orkes-only surface (secret, webhook, # api-gateway) that a local OSS server cannot exercise. @@ -56,6 +120,22 @@ jobs: e2e-enterprise: name: E2E (Enterprise) runs-on: ubuntu-latest + # This job addresses a single shared remote server, and the suites create fixtures + # under fixed global names (`e2e_test_secret`, `e2e_test_service`, …) whose + # `setup_file` deletes leftovers before running. Two overlapping runs therefore + # delete each other's fixtures mid-test. The group is a constant rather than keyed + # on the ref, because the contention is between *different* pull requests, not + # between pushes to one. + # + # A stopgap, not a fix. While one run holds the slot another can sit pending and be + # superseded by a newer one, so an overlapping run may report cancelled rather than + # green — which is at least honest, unlike the interleaved corruption it replaces. + # The real fix is per-run fixture namespacing, which removes the contention instead + # of queueing around it. The OSS matrix needs none of this: every leg gets its own + # private server. + concurrency: + group: e2e-enterprise-shared-server + cancel-in-progress: false if: >- github.event_name != 'workflow_dispatch' || inputs.run_enterprise @@ -121,16 +201,38 @@ jobs: if-no-files-found: ignore # --------------------------------------------------------------------------- - # Local OSS server built from conductor-oss main. Covers the OSS code paths, the - # `server` command, and validates the CLI against the code that will ship — - # none of which the Enterprise job can do. + # Local OSS server at a pinned, published Server Version, started through the + # CLI's own `server start --version`. Covers the OSS code paths and the + # `server` command group, neither of which the Enterprise job can reach. + # + # One leg per pinned version. Today that is the current release alone, which + # gates a merge. A leg pinned with "blocking": false reports without gating, + # so version skew that predates a change cannot block it. + # + # Starting through the CLI is also what gives server.bats a CLI-managed Local + # server to assert against, so its six server-dependent tests run rather than + # skip. # --------------------------------------------------------------------------- e2e-local-server: - name: E2E (local OSS server) + # Only a non-blocking leg is labelled, so the check name carries the caveat + # exactly when there is one; with a single pin there is nothing to contrast + # "blocking" against and the suffix would just raise the question. + # The condition is inverted deliberately: `x && '' || y` always yields y, + # because the empty string is falsy in a GitHub Actions expression. + name: E2E (local OSS server ${{ matrix.server.version }}${{ !matrix.server.blocking && ', non-blocking' || '' }}) runs-on: ubuntu-latest + needs: server-versions if: >- github.event_name != 'workflow_dispatch' || inputs.run_local_server + # A non-blocking leg reports without gating. Which legs exist, and which one + # gates, is a one-line change in the pins above. + continue-on-error: ${{ !matrix.server.blocking }} + strategy: + # Every leg's result is wanted; a failure on one must not cancel the rest. + fail-fast: false + matrix: + server: ${{ fromJSON(needs.server-versions.outputs.matrix) }} steps: - uses: actions/checkout@v4 @@ -154,23 +256,14 @@ jobs: chmod +x conductor ./conductor --version - - name: Check out Conductor server source - uses: actions/checkout@v4 + - name: Cache Conductor server jar + uses: actions/cache@v4 with: - repository: conductor-oss/conductor - ref: ${{ env.CONDUCTOR_SERVER_REF }} - path: conductor-server-src - - - name: Set up Gradle - uses: gradle/actions/setup-gradle@v4 - - - name: Build Conductor server from source - working-directory: conductor-server-src - run: | - # Module names are prefixed in settings.gradle, so the task is - # :conductor-server:bootJar, not :server:bootJar. - ./gradlew :conductor-server:bootJar -x test --console=plain - ls -la server/build/libs/*-boot.jar + # ~450 MB download. Cached at the per-version directory the CLI already + # downloads into, keyed on the version, so adding a leg costs one + # download once. Pins are immutable, so the key never needs busting. + path: ~/.conductor-cli/server/oss/${{ matrix.server.version }} + key: conductor-server-oss-${{ matrix.server.version }} - name: Setup bats uses: bats-core/bats-action@2.0.0 @@ -184,28 +277,20 @@ jobs: } - name: Start local Conductor server + env: + SERVER_VERSION: ${{ matrix.server.version }} run: | mkdir -p "$SERVER_WORKDIR" - JAR=$(ls "$GITHUB_WORKSPACE"/conductor-server-src/server/build/libs/*-boot.jar | head -1) - echo "starting $JAR" - # Run from the scratch dir so the SQLite database is not written into the - # repository (see #104). The AI flags mirror what `conductor server start` - # passes, so agent workflows behave the same as under the CLI. - # - # NOTE: a source-built jar cannot be launched via `conductor server start`, - # which only downloads published versions. Consequently no CLI-managed pid - # file exists and the 6 server-dependent tests in server.bats skip rather - # than run. They skip loudly with a reason; see the tracking issue in - # ADR-0001 for closing that gap. + # Started from the scratch dir so the SQLite database is not written into + # the repository (see #104). `server start` passes the local server + # defaults itself, so this job does not restate them. cd "$SERVER_WORKDIR" - nohup java -jar "$JAR" \ - --conductor.integrations.ai.enabled=true \ - --agentspan.embedded=true \ - > /tmp/conductor-server.log 2>&1 & - echo $! > /tmp/conductor-server.pid + "$GITHUB_WORKSPACE/conductor" server start --version "$SERVER_VERSION" - name: Wait for server health run: | + # `server start` waits for readiness but only warns on timeout, so assert + # health here rather than trusting its exit status. for i in $(seq 1 60); do if curl -sf http://localhost:8080/health | grep -q '"healthy":true'; then echo "server healthy after ${i}s" @@ -214,7 +299,7 @@ jobs: sleep 1 done echo "::error::server did not become healthy within 60s" - tail -n 100 /tmp/conductor-server.log || true + ./conductor server logs -n 100 || true exit 1 - name: Run OSS E2E tests @@ -222,30 +307,51 @@ jobs: CONDUCTOR_SERVER_URL: http://localhost:8080/api CONDUCTOR_SERVER_TYPE: OSS run: | + # pipefail so tee does not mask a bats failure. + set -o pipefail # Excludes orkes-only; includes oss-only (the server suite). - bats --filter-tags 'tier:pr,!orkes-only' test/e2e/ --show-output-of-passing-tests + bats --filter-tags 'tier:pr,!orkes-only' test/e2e/ --show-output-of-passing-tests \ + | tee /tmp/bats-oss.txt + + - name: Report skips and guard the server-dependent tests + if: always() + env: + SERVER_VERSION: ${{ matrix.server.version }} + run: | + [ -f /tmp/bats-oss.txt ] || { echo "no bats output to inspect"; exit 0; } + skips=$(grep -c '# skip' /tmp/bats-oss.txt || true) + echo "::notice::OSS-safe tier:pr on ${SERVER_VERSION}: ${skips} skipped" + grep '# skip' /tmp/bats-oss.txt || echo "(nothing skipped)" + # Pinning exists so a CLI-managed Local server is available. If these skip + # again, the launch path regressed and the coverage silently vanished — + # which is exactly what this change was made to stop. + if grep -q 'no CLI-managed local server is running' /tmp/bats-oss.txt; then + echo "::error::server.bats skipped for want of a CLI-managed Local server. The server was not started through 'conductor server start', so the six server-dependent tests did not run." + exit 1 + fi - name: Dump server logs on failure if: failure() - run: tail -n 200 /tmp/conductor-server.log || true + run: ./conductor server logs -n 200 || true - name: Collect server log for artifacts if: always() # upload-artifact does not expand '~', so copy the log into the workspace. run: | mkdir -p e2e-logs - cp /tmp/conductor-server.log e2e-logs/ 2>/dev/null || true + cp ~/.conductor-cli/server/conductor.log e2e-logs/ 2>/dev/null || true - name: Stop local Conductor server if: always() - run: | - [ -f /tmp/conductor-server.pid ] && kill "$(cat /tmp/conductor-server.pid)" 2>/dev/null || true + run: ./conductor server stop || true - name: Upload test artifacts if: always() uses: actions/upload-artifact@v4 with: - name: e2e-artifacts-local-server + # Artifact names must be unique across a workflow run, so each leg + # carries its Server Version. + name: e2e-artifacts-local-server-${{ matrix.server.version }} path: | test/e2e/*.log test/e2e/*.json @@ -257,6 +363,10 @@ jobs: # the PR path because it spends provider tokens, depends on PyPI, and is # non-deterministic by nature. # + # Runs against the blocking Server Version only, however many are pinned: its + # suites depend on capabilities older lines lack, and each extra leg would + # spend model tokens to tell us so. + # # Currently manual-only: there is no `schedule:` trigger, so this runs solely # when someone dispatches the workflow with run_nightly=true. The tier is still # named "nightly" because that is its intended cadence once someone owns the @@ -265,6 +375,7 @@ jobs: e2e-nightly: name: E2E (nightly tier - LLM + deploy, manual) runs-on: ubuntu-latest + needs: server-versions if: >- github.event_name == 'workflow_dispatch' && inputs.run_nightly env: @@ -272,6 +383,8 @@ jobs: # job-level `env`. Reduce the secret to a boolean here so steps can gate on # it without leaking the value. HAS_LLM_KEY: ${{ secrets.ANTHROPIC_API_KEY != '' }} + # Same pin as the blocking leg, taken from the same declaration. + CONDUCTOR_SERVER_VERSION: ${{ needs.server-versions.outputs.blocking }} steps: - uses: actions/checkout@v4 @@ -300,21 +413,11 @@ jobs: chmod +x conductor ./conductor --version - - name: Check out Conductor server source - uses: actions/checkout@v4 + - name: Cache Conductor server jar + uses: actions/cache@v4 with: - repository: conductor-oss/conductor - ref: ${{ env.CONDUCTOR_SERVER_REF }} - path: conductor-server-src - - - name: Set up Gradle - uses: gradle/actions/setup-gradle@v4 - - - name: Build Conductor server from source - working-directory: conductor-server-src - run: | - ./gradlew :conductor-server:bootJar -x test --console=plain - ls -la server/build/libs/*-boot.jar + path: ~/.conductor-cli/server/oss/${{ env.CONDUCTOR_SERVER_VERSION }} + key: conductor-server-oss-${{ env.CONDUCTOR_SERVER_VERSION }} - name: Setup bats uses: bats-core/bats-action@2.0.0 @@ -328,13 +431,8 @@ jobs: ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} run: | mkdir -p "$SERVER_WORKDIR" - JAR=$(ls "$GITHUB_WORKSPACE"/conductor-server-src/server/build/libs/*-boot.jar | head -1) cd "$SERVER_WORKDIR" - nohup java -jar "$JAR" \ - --conductor.integrations.ai.enabled=true \ - --agentspan.embedded=true \ - > /tmp/conductor-server.log 2>&1 & - echo $! > /tmp/conductor-server.pid + "$GITHUB_WORKSPACE/conductor" server start --version "$CONDUCTOR_SERVER_VERSION" - name: Wait for server health run: | @@ -346,6 +444,7 @@ jobs: sleep 1 done echo "::error::server did not become healthy within 60s" + ./conductor server logs -n 100 || true exit 1 - name: Run agentspan deploy tests @@ -373,19 +472,18 @@ jobs: - name: Dump server logs on failure if: failure() - run: tail -n 200 /tmp/conductor-server.log || true + run: ./conductor server logs -n 200 || true - name: Collect server log for artifacts if: always() # upload-artifact does not expand '~', so copy the log into the workspace. run: | mkdir -p e2e-logs - cp /tmp/conductor-server.log e2e-logs/ 2>/dev/null || true + cp ~/.conductor-cli/server/conductor.log e2e-logs/ 2>/dev/null || true - name: Stop local Conductor server if: always() - run: | - [ -f /tmp/conductor-server.pid ] && kill "$(cat /tmp/conductor-server.pid)" 2>/dev/null || true + run: ./conductor server stop || true - name: Upload test artifacts if: always() diff --git a/CLAUDE.md b/CLAUDE.md index dded6db..51f3ae2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,7 +134,7 @@ background process on port 8080. **Flags:** - `--port` - Port to run the server on (default: 8080) - `--foreground`, `-f` - Run in the foreground instead of daemonizing -- `--version` - Server version to download and run (default: `latest`, e.g. `3.21.23`) +- `--version` - Server version to download and run (default: `latest`, e.g. `3.32.0`) - `--oss` - Use the open-source Conductor server (default) - `--orkes` - Use the Orkes Conductor server (coming soon) - `--follow`, `-f` - Follow log output like `tail -f` (logs command) diff --git a/CONTEXT.md b/CONTEXT.md index 5cd83ff..bddcc65 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -35,6 +35,14 @@ A Conductor server that the CLI downloads and runs as a background process on th user's machine for development and testing. _Avoid_: embedded server, dev server, test server +**Server Version**: +Which published Conductor artifact a Local server runs, named by a version string +or by the floating `latest` tag. Orthogonal to Server type: it selects an artifact, +not a distribution. Only versions the download bucket actually carries can be named +— it holds a subset of the server repo's tags. +_Avoid_: version (unqualified — the CLI has its own), distribution (already names +OSS versus Orkes), server build + ### Configuration **Profile**: diff --git a/README.md b/README.md index 205f823..ec29966 100644 --- a/README.md +++ b/README.md @@ -526,7 +526,7 @@ conductor server [flags] conductor server start # Start specific version on custom port -conductor server start --version 3.21.23 --port 9090 +conductor server start --version 3.32.0 --port 9090 # Run in foreground conductor server start -f diff --git a/cmd/server.go b/cmd/server.go index fc5c214..9619b27 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -35,7 +35,8 @@ import ( const ( // OSS Conductor server download URL template - // Version is substituted: "latest" or specific version like "3.21.23" + // Version is substituted: "latest" or specific version like "3.32.0". The bucket + // carries a subset of the server repo's tags, so not every tag resolves here. ossJarURLTemplate = "https://conductor-server.s3.us-east-2.amazonaws.com/conductor-server-%s.jar" // Orkes Conductor server download URL template (TBD) @@ -93,7 +94,7 @@ Examples: conductor server start # Start with specific version - conductor server start --version 3.21.23 + conductor server start --version 3.32.0 # Start on a different port conductor server start --port 9090 @@ -143,7 +144,7 @@ Examples: conductor server update # Update a specific version - conductor server update --version 3.21.23`, + conductor server update --version 3.32.0`, RunE: updateServer, SilenceUsage: true, } @@ -1039,7 +1040,7 @@ func init() { // Start command flags serverStartCmd.Flags().Int("port", defaultPort, "Port to run the server on") serverStartCmd.Flags().BoolP("foreground", "f", false, "Run server in foreground (don't daemonize)") - serverStartCmd.Flags().String("version", "latest", "Server version to download and run (e.g., 'latest', '3.21.23')") + serverStartCmd.Flags().String("version", "latest", "Server version to download and run (e.g., 'latest', '3.32.0')") serverStartCmd.Flags().Bool("oss", false, "Use open-source Conductor server (default)") serverStartCmd.Flags().Bool("orkes", false, "Use Orkes Conductor server (coming soon)") serverStartCmd.MarkFlagsMutuallyExclusive("oss", "orkes") @@ -1049,7 +1050,7 @@ func init() { serverLogsCmd.Flags().IntP("lines", "n", 50, "Number of lines to show") // Update command flags - serverUpdateCmd.Flags().String("version", "latest", "Server version to update (e.g., 'latest', '3.21.23')") + serverUpdateCmd.Flags().String("version", "latest", "Server version to update (e.g., 'latest', '3.32.0')") serverUpdateCmd.Flags().Bool("oss", false, "Use open-source Conductor server (default)") serverUpdateCmd.Flags().Bool("orkes", false, "Use Orkes Conductor server (coming soon)") serverUpdateCmd.MarkFlagsMutuallyExclusive("oss", "orkes") diff --git a/docs/adr/0002-e2e-builds-the-conductor-server-from-source.md b/docs/adr/0002-e2e-builds-the-conductor-server-from-source.md index 9e15977..5cae2d3 100644 --- a/docs/adr/0002-e2e-builds-the-conductor-server-from-source.md +++ b/docs/adr/0002-e2e-builds-the-conductor-server-from-source.md @@ -1,9 +1,17 @@ --- -status: accepted +status: superseded --- # E2E builds the Conductor server from source rather than pinning a published version +> **Superseded by +> [ADR-0006](./0006-e2e-pins-a-published-server-version.md).** The blocker +> below — that no artifact is published from `main` — expired when `3.32.0` GA shipped +> and the S3 `latest` jar was republished to match it. E2E now pins a published Server +> Version and runs the OSS-safe suite against it. Read this ADR as the recorded case +> for source-building, not as current policy; ADR-0006 explains how to recover the +> arrangement it describes. + The E2E jobs check out `conductor-oss/conductor` at `main` and run `:conductor-server:bootJar`, instead of pinning a published version via `conductor server start --version `. We chose this because releases are cut from @@ -69,8 +77,21 @@ Any of these should prompt switching back to a pin: The workflow keeps a single knob for this: `CONDUCTOR_SERVER_REF`. Reverting means replacing the checkout-and-build steps with `conductor server start --version `, -which the earlier revision of #106 already implemented, so the change is recoverable -from git history rather than needing redesign. +which commit `cf9d6b8c` already implements — `CONDUCTOR_SERVER_VERSION`, an +`actions/cache` keyed on it, and the `server start` invocation, for both server-backed +jobs. + +That commit is **not reachable from `main`**: #106 was squash-merged, so +`git log -S'server start --version'` finds nothing. Fetch it with +`git fetch origin refs/pull/106/head`, or read the file directly: + +``` +gh api "repos/conductor-oss/conductor-cli/contents/.github/workflows/e2e.yml?ref=cf9d6b8c" +``` + +Pinning also restores coverage rather than only saving time: with `conductor server +start` managing the server there is a CLI-managed pid file again, so the six +`server.bats` tests that currently skip will run. ## Alternatives considered diff --git a/docs/adr/0006-e2e-pins-a-published-server-version.md b/docs/adr/0006-e2e-pins-a-published-server-version.md new file mode 100644 index 0000000..5f6a3e7 --- /dev/null +++ b/docs/adr/0006-e2e-pins-a-published-server-version.md @@ -0,0 +1,263 @@ +--- +status: accepted +--- + +# E2E pins a published Server Version rather than building the server from source + +The server-backed E2E jobs no longer check out `conductor-oss/conductor` and build it +with Gradle. They start a Local server through the CLI's own +`conductor server start --version `, and the OSS-safe job runs that suite +against the current release, which gates a merge. + +The job is shaped as a matrix over a declared list of pins, but the list holds one +entry. Testing older lines was considered and deliberately not adopted — see +[why the older lines are not pinned](#why-the-older-lines-are-not-pinned). + +This reverses [ADR-0002](./0002-e2e-builds-the-conductor-server-from-source.md), and it +is the arrangement `@mp-orkes` asked for in review of +[#106](https://github.com/conductor-oss/conductor-cli/pull/106). Pinning was always the +better default on speed, reproducibility and CI isolation; the single thing blocking it +was that no artifact was published from `main`, so a pin could not validate the code +about to ship. + +## Why now + +That blocker expired. `3.32.0` GA is published, and the floating `latest` jar — which +ADR-0002 recorded as months stale and apparently abandoned — was republished to match +it: both URLs return the same 455,946,354-byte artifact. Release validation is +therefore no longer scoped to an unreleased `main`, which was the second of ADR-0002's +three stated revisit triggers. + +## What pinning buys, and why the shape is still a matrix + +ADR-0002 tested one moving target, and that target was not a version anyone had +released. Pinning the current release means CI tests the version users actually have, +byte-identically between runs. + +The job is nonetheless written as a matrix over a declared list of `{version, blocking}` +objects, with `continue-on-error` on any entry that is not blocking. That machinery +costs nothing at one entry and makes testing an additional line a one-line edit rather +than a redesign — which matters, because the reason no older line is pinned today is a +server-side one that may well be resolved (below). Exactly one entry must be blocking, +and the preflight refuses to start otherwise. + +The nightly tier stays on the blocking version alone. Its suites depend on +capabilities older lines lack, and an extra leg would spend model tokens per run to +discover that. + +The Orkes-facing job is untouched. It addresses a remote server whose Server Version is +neither known nor selectable, so a pin cannot reach it. + +## Why the older lines are not pinned + +`3.31.0` and `3.30.2` were pinned as non-blocking legs in an earlier revision of this +change and run in CI. What follows is the measurement, and why the legs were then +dropped rather than kept. + +All three Server Versions were run before this landed, each downloaded through the CLI +and started against its own fresh database, so the older ones are a measurement rather +than an expectation. These counts are from a **local macOS run**; one skip on every line +is the macOS-only GNU `timeout(1)` guard, which does not skip on a Linux runner, so +expect one fewer of each in CI: + +| Server Version | tests | skips | failures | +|---|---|---|---| +| `3.32.0` (pinned, blocking) | 116 | 2 | 0 | +| `3.31.0` (not pinned) | 116 | 11 | 3 | +| `3.30.2` (not pinned) | 116 | 11 | 3 | + +The six recovered `server.bats` tests pass on **all three**, so the launch path itself is +not version-sensitive — the result that actually mattered for this change. The older +lines are otherwise *not* clean. + +The mechanism worked as designed: the preflight passed, the matrix expanded to the three +expected legs, the `3.32.0` leg and the Orkes-facing job passed, both older legs failed, +and the run's overall conclusion was still **success**. + +**It was not the mechanism that failed, it was the reading cost.** Two permanently-red +checks on every pull request oblige every reviewer to learn which reds are load-bearing, +and standing red is indistinguishable at a glance from a CI system that is merely broken. +The failures are known, understood, upstream, and not the CLI's; that is a fact worth +recording once, here, rather than re-asserting on every run through a check that readers +must be told to ignore. Nobody asked for the older lines when the choice was put to +review, so nothing is being given up that anyone had claimed. + +**Three failures on both older lines, one root cause — and it is already fixed upstream.** +`GET /api/scheduler/schedules/` returns `HTTP 200` with an empty body for a +schedule that does not exist. That is +[conductor-oss/conductor#1357](https://github.com/conductor-oss/conductor/pull/1357), +merged 2026-07-19 as `c08d60c8a`, which makes the endpoint throw `NotFoundException` +instead. By ancestry the fix is in `v3.32.0` and its RCs from `rc.14` on, and in neither +`v3.31.0` nor `v3.30.2` — so the older lines were not reporting a new defect, they were +reporting a fix they predate. Confirmed against `3.32.0`, which returns `404` with an +error body both for a name that never existed and after a delete. + +Note that `schedule delete` works on the older lines: the schedule leaves `schedule list` +and the underlying list API. It is only the single-schedule read that disagrees. + +That one behaviour produces all three failures, once directly and twice through a +helper. `schedule.bats` test 7 asserts a deleted schedule can no longer be fetched, and +fails outright. The suite's `ensure_schedule` helper decides whether to create a schedule +by calling `schedule get`, so it concludes the schedule already exists, skips creating +it, and leaves tests 12 and 19 asserting against a schedule that is not there. +Pre-existing server-side skew, unrelated to the CLI and unrelated to pinning — so there +is nothing to file, and making these lines green would mean asking for a backport of +#1357 to the 3.31 and 3.30 lines. That is a support-policy decision rather than a bug +report, and it is the decision that would have to be taken before pinning them is worth +doing. + +**The nine extra skips are the Agents API.** Neither older line serves it, so nine +`agent.bats` tests skip on their runtime guard rather than fail. That guard exists +because not every deployment serves those endpoints, so this is intended behaviour, not +a gap. + +Neither finding is a defect in this change. Together they are the case for not pinning +these lines: the suite cannot be green against either one for reasons that live outside +this repository, and a check that can only be red reports nothing a reader can act on. + +**What would change this.** If #1357 is backported to the 3.31 or 3.30 line, or the +project decides it wants a supported-line signal enough to tag around the three +`schedule.bats` failures, add the entry back with `"blocking": false`. The machinery is +still there; the argument above is what would need to have changed, not the workflow. + +One caveat for anyone reproducing this locally: run each version against a *fresh* +working directory. Pointing two Server Versions at one `c123.db` produced an extra, +spurious `workflow create --force` failure on `3.30.2` that disappears on a clean +database. CI is unaffected — each matrix leg is its own runner, and the jar cache holds +only the jar, not the database or the server state. + +## Published availability is the constraint, not the tag list + +The S3 bucket the CLI downloads from carries a *subset* of the server repo's tags. +Verified by `HEAD` at the time of writing: `3.32.0`, `3.31.0`, `3.30.2` and `latest` +return 200, while `3.31.2` and `3.21.23` return 403. A pin must therefore be chosen +against bucket contents rather than against `git tag` — the 3.31 line, had it been +pinned, would have had to sit at `3.31.0` rather than at its newest patch. + +Because "someone pins a tag the bucket does not carry" is a live failure mode rather +than a hypothetical, a preflight job `HEAD`s every pin before any job does real work, +naming the version, the URL and the status code. Without it a bad pin surfaces minutes +into a job as an opaque download error. + +The preflight is asymmetric, and deliberately so. An unavailable **blocking** pin fails +it outright — there is nothing meaningful left to run. An unavailable **non-blocking** +pin only drops its own leg, with a warning naming what was dropped, because failing the +whole preflight would skip the blocking leg along with it and let an older-version +problem gate a merge through the back door. The preflight therefore emits the matrix it +verified rather than the matrix it was given. With a single blocking pin the second +branch is currently unexercised; it is kept because it is what makes adding a line safe. + +`latest` is deliberately not pinned, even though it currently resolves to the GA +release. It reintroduces the irreproducibility this change removes, and it is mutable +content under a stable cache key — a jar cache keyed on `latest` would serve stale +bytes indefinitely. + +## Consequences + +**Six tests stopped skipping.** `server start` can only launch a published version, so +under ADR-0002 the source-built jar had to be launched with `java -jar`, leaving no +CLI-managed Local server and six `server.bats` tests skipping on a runtime condition — +including the `server start` and `server update` mutual-exclusion guards. A downloaded +version is started by the CLI, so those tests run. Un-skipping them is the +fix-verification step, in the Known-broken guard sense. + +Measured against `3.32.0` by launching the same jar both ways, changing nothing but the +launch path: the OSS-safe `tier:pr` selection is 116 tests, and the skip count goes from +**8 to 2**, with all six recovered tests passing, including both mutual-exclusion guards. +The two that remain are unrelated and expected — the `#103` Known-broken guard, and one +macOS-only skip for absent GNU `timeout(1)` which does not skip on a Linux runner, so CI +should report 1. + +To keep that from silently regressing, the job fails the leg if any test skips for want +of a CLI-managed Local server, and reports the skip count as a run annotation either +way. A number nobody looks at is not a check. + +**Runs are reproducible.** A pinned Server Version is byte-identical between runs, so a +failure seen yesterday reproduces today. + +**CI no longer depends on the server repo's build health.** A broken `main` in +`conductor-oss/conductor` can no longer redden an unrelated conductor-cli pull request. +This was ADR-0002's most significant cost. + +**The Gradle build is gone from the pull-request path**, along with a build time that +would have grown with the server. + +**Fidelity against unreleased `main` is given up.** This is the real cost, and it is +the trade ADR-0002 made in the other direction. A CLI-facing server change now shows up +in E2E only once it is published. The mitigation is that the pin is one line, so +tracking a new release or an RC during a release cycle is a one-line change. + +**The pin needs bumping.** A release nobody pins is a release nobody tests, and with a +single pin that is the whole of the version signal. The pin lives in one declaration in +the workflow, and the preflight refuses a version the bucket does not carry, so a bump +either works or fails immediately and legibly. + +**Nothing tests the older supported lines.** This is the cost of the section above, and +it is real: a break against 3.31 or 3.30 will now first be reported by a user. It was +accepted because the alternative on offer was not a working signal but a permanently +red one. + +## Recovering the source build + +If fidelity against unreleased `main` becomes the priority again — a release cycle +scoped to `main`, or a CLI-facing server change that must be validated before it ships +— the source build is recoverable from commit +[`c7eb744`](https://github.com/conductor-oss/conductor-cli/commit/c7eb744), which +carries `CONDUCTOR_SERVER_REF`, the `conductor-oss/conductor` checkout, the Gradle setup +and the `:conductor-server:bootJar` build, for both server-backed jobs: + +``` +git show c7eb744:.github/workflows/e2e.yml +``` + +That commit is deliberately one that is **reachable from `main`**, which is the mistake +ADR-0002 made: it named `cf9d6b8c` from a squash-merged pull request, so the commit was +unreachable and `git log -S` could not find it. If this change is itself squash-merged, +the pointer still holds, and it stays discoverable without reading this file, because +the string it searches for is one this change removes: + +``` +git log -S'CONDUCTOR_SERVER_REF' -- .github/workflows/e2e.yml +``` + +Note that recovering it re-skips the six `server.bats` tests, for the reason above. +Restoring both fidelity *and* that coverage needs `conductor server start --jar `, +option A of [#105](https://github.com/conductor-oss/conductor-cli/issues/105), filed +separately as [#118](https://github.com/conductor-oss/conductor-cli/issues/118). + +## The seam this leaves for the capabilities matrix + +Testing older lines at all — let alone gating on them — is deliberately not attempted +here. Doing it properly requires the suites to know which capabilities exist in which +Server Version, so that a line which simply lacks an endpoint skips rather than fails. +That is the cross-version capabilities matrix, and it is separate work. + +The seam that work builds on is the pin declaration in the `server-versions` job: it +emits a JSON array of `{version, blocking}` objects that the OSS-safe job consumes as +its matrix, and each leg exports its version to the suite through +`conductor server start --version`. Adding a per-version capability dimension means +extending those objects and having the suites select on them, not inventing a +version-parameterised run from scratch. + +## Alternatives considered + +**Keep 3.31.0 and 3.30.2 as non-blocking legs.** Implemented, run in CI, then rejected — +see [why the older lines are not pinned](#why-the-older-lines-are-not-pinned). The signal +they carried was one standing fact, already recorded here; the cost was two permanently +red checks on every pull request and a reviewer convention for ignoring them. Put to +reviewers in the pull request that introduced them, with no reply either way. + +**Keep both launch paths behind a flag.** Rejected. It doubles the workflow's surface +to hedge a fidelity gap that is currently small, and an untaken path in CI rots +unnoticed. Documented recovery is cheaper than live redundancy. + +**Pin an RC instead of the GA release.** Rejected. RCs are cut from `main`, so an RC is +a closer proxy for unreleased code, but it is not what users run, and the bucket's RC +coverage is patchy — several RCs return 403. Pinning an RC during a release cycle +remains a reasonable one-line change when fidelity matters more than fidelity to users. + +**Ask for `main`-tracking snapshots to be published.** ADR-0002 called this the best +outcome, and [#105](https://github.com/conductor-oss/conductor-cli/issues/105) raised it +partly because `latest` looked abandoned. `latest` is demonstrably alive, so the request +is no longer needed to unblock pinning. It would still make the fidelity trade above +disappear, and remains worth having. diff --git a/test/e2e/README.md b/test/e2e/README.md index 79966cb..b2f00be 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -19,27 +19,31 @@ Run from the **repository root**, not from this directory. Against a local OSS server: ```bash -conductor server start --version 3.32.0-rc.23 # from a scratch dir, see note below +conductor server start --version 3.32.0 # from a scratch dir, see note below export CONDUCTOR_SERVER_URL=http://localhost:8080/api export CONDUCTOR_SERVER_TYPE=OSS bats --filter-tags 'tier:pr,!orkes-only' test/e2e/ ``` -To test against the code that will actually ship, build the server from source -instead — releases are cut from `conductor-oss/conductor` `main`, and no artifact is -published from it: +This is what CI does, and starting the server through the CLI rather than with +`java -jar` is what gives `server.bats` a CLI-managed Local server to assert against. + +To reproduce the CI leg, pass its Server Version — the checks list names the version in +the job title. CI pins one, the current release, declared in a single place: the +`server-versions` job in [`e2e.yml`](../../.github/workflows/e2e.yml). + +Pin a version the download bucket actually carries: it holds a subset of the server +repo's tags, so several tagged versions return 403. Check before pinning, which is also +what CI's preflight does: ```bash -cd ../conductor && git checkout main && git pull -./gradlew :conductor-server:bootJar -x test # note the conductor- prefix -mkdir -p /tmp/conductor-e2e && cd /tmp/conductor-e2e -java -jar ../../conductor/server/build/libs/*-boot.jar \ - --conductor.integrations.ai.enabled=true --agentspan.embedded=true +curl -sI https://conductor-server.s3.us-east-2.amazonaws.com/conductor-server-3.32.0.jar | head -1 ``` -This is what CI does. Because `conductor server start` can only download published -versions, a source-built jar has no CLI-managed pid file, so the six server-dependent -tests in `server.bats` skip. See #105. +See [ADR-0006](../../docs/adr/0006-e2e-pins-a-published-server-version.md) for why E2E +pins a published version instead of building the server from source, why no older line +is pinned, and how to recover the source build if fidelity against unreleased `main` is +needed. Against an Orkes server: @@ -70,11 +74,100 @@ bats test/e2e/agent.bats bats --count --filter-tags 'tier:pr,!orkes-only' test/e2e/ ``` +### What the OSS-safe run should report + +Against the pinned Server Version: 116 tests and **1 skip** on Linux, the `#103` +Known-broken guard. On macOS expect a second, for absent GNU `timeout(1)`. + +Against an older line, expect **more** skips and some failures — neither 3.31 nor 3.30 +serves the Agents API, so nine `agent.bats` tests skip, and both fail three +`schedule.bats` tests on pre-existing server-side skew. CI does not run them for that +reason; [ADR-0006](../../docs/adr/0006-e2e-pins-a-published-server-version.md) records +the root cause and what would have to change to pin one. + +Anything skipping with *"no CLI-managed local server is running"* means the server was +not started through `conductor server start`, so the six `server.bats` tests did not +run. CI fails the leg on that rather than letting the coverage vanish quietly: + +```bash +bats --filter-tags 'tier:pr,!orkes-only' test/e2e/ | grep '# skip' +``` + > Start the local server from a scratch directory (e.g. `/tmp/conductor-e2e`). > `conductor server start` writes its SQLite database relative to the working > directory with no flag to override it, so starting it from the repo drops a > multi-hundred-MB `c123.db` here. See issue #104. +## Changing which Server Versions CI tests + +The OSS-safe suite runs once per pinned Server Version. All the pins live in one place — +the `server-versions` job in [`e2e.yml`](../../.github/workflows/e2e.yml): + +```json +[ + { "version": "3.32.0", "blocking": true } +] +``` + +`blocking: true` means that leg can fail a merge. Exactly one entry must be blocking, and +the job refuses to start otherwise. Any further entry must therefore be +`"blocking": false`: it runs on the same pull requests and reports in the same checks +list under a job name marked *non-blocking*, but cannot gate a merge, so version skew +that predates your change can't block it. + +Only the current release is pinned today. Older lines were tried and dropped — +[ADR-0006](../../docs/adr/0006-e2e-pins-a-published-server-version.md) explains why, and +what would make pinning one worthwhile again. + +### Before adding a version, check it exists + +The jar bucket carries only a *subset* of the server repo's tags — some tagged versions +are simply absent. Check with a `HEAD` first: + +```bash +V=3.31.0 +curl -sI "https://conductor-server.s3.us-east-2.amazonaws.com/conductor-server-$V.jar" | head -1 +``` + +`200` means you can pin it. `403` means you can't, whatever `git tag` says in the server +repo — the 3.31 line, for instance, is only available at `3.31.0` and not at its later +patches. + +CI checks this too, and treats the two cases differently: an unavailable **blocking** pin +fails the preflight outright, while an unavailable **non-blocking** pin only drops its own +leg, with a warning saying so. Failing the whole preflight would skip the blocking leg as +well, which would let an older version gate a merge through the back door. + +### The four changes you're likely to make + +| Goal | Change | +|---|---| +| Bump the blocking pin for a new release | Edit the `version` of the `blocking: true` entry | +| Add a line to the matrix | Add an entry with `"blocking": false` | +| Stop testing a line | Delete its entry | +| Make an older line gate merges | Flip its `blocking` to `true` and the old one to `false` | + +Nothing else needs touching: the job name, the jar cache key, and the nightly job's +version all derive from these entries. Adding a leg costs one jar download the first +time and is cache-hit afterwards, because pins are immutable. + +Promoting an older line to blocking is a one-line edit but a real commitment — it means +the suite must pass against that line, and today the older lines fail tests for reasons +that are server-side, not CLI-side. Expect to fix or tag around those first. + +### Check your change before pushing + +```bash +# what each leg will run +bats --count --filter-tags 'tier:pr,!orkes-only' test/e2e/ + +# reproduce a leg locally, from a scratch dir, on a fresh database +conductor server start --version 3.32.0 +``` + +Run each version against its own working directory. Pointing two Server Versions at one +`c123.db` produces spurious failures that have nothing to do with the version. + ## Tags | Tag | Meaning |