Skip to content

test: E2E coverage for agent, deploy, doctor, config, server; tag-selected suites and a local-OSS-server CI job - #106

Merged
ambiorix2099 merged 8 commits into
mainfrom
feature/e2e-test-coverage
Aug 5, 2026
Merged

test: E2E coverage for agent, deploy, doctor, config, server; tag-selected suites and a local-OSS-server CI job#106
ambiorix2099 merged 8 commits into
mainfrom
feature/e2e-test-coverage

Conversation

@ambiorix2099

@ambiorix2099 ambiorix2099 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe): tests + documentation

NOTE: ./gradlew spotlessApply is not applicable — this repository is Go. go build,
go test ./... and gofmt are clean; no Go sources are modified by this PR.

Changes in this PR

Adds E2E coverage for five commands that had none, selects the suite by tag instead of by
hand-maintained file lists, and adds a CI job that tests against a locally built OSS server.

Coverage: 129 → 177 tests. Per-PR test executions: 111 → 266.

New suite Tests Covers
agent.bats 17 scaffolding, CRUD, execution search, live runs
config.bats 9 profile save/list/delete, --profile precedence
doctor.bats 7 runtime, server and AI provider reporting
deploy.bats 7 Python agent discovery and deployment
server.bats 8 local server status/logs and mutual-exclusion guards

Brings api_gateway.bats back into a job. Its 18 tests appeared in no job's file list, so they
ran nowhere. Tag selection picks them up automatically, and future suites cannot go missing the same
way.

An early revision of this PR described that as recovering tests orphaned by oversight. The first CI
run suggests otherwise: the Enterprise server CI targets does not have API Gateway enabled, and all
18 fail there with 404 No static resource api/gateway/.... They were most likely dropped from the
file list deliberately. Re-including them without a guard simply reinstated 15 failures.

They are now guarded: the suite runs wherever API Gateway exists and skips with a stated reason
where it does not. That is better than being silently absent from every job — the gap is now visible
in the run output rather than invisible in a file list — but it is a skip on this server, not
recovered coverage.

New CI shape. Three jobs replace one:

Job Trigger Selection Tests
e2e-enterprise PR unauthenticated 5
tier:pr,!oss-only,!unauthenticated 152
e2e-local-server PR tier:pr,!orkes-only 109
e2e-nightly manual tier:nightly,needs:agentspan / needs:llm 7 / 5

e2e-local-server closes gaps the Enterprise job could not, even in principle: CI pinned
CONDUCTOR_SERVER_TYPE=Enterprise, so OSS code paths were never exercised — the OSS schedule
support added in #86 was untested anywhere — and it tested a server of unknown version rather than
what is being released.

The server is built from conductor-oss/conductor main, because releases are cut from main and
no artifact is published from it: Maven Central carries only tagged RCs and the S3 latest jar has
not moved since 3 June. The trade-offs (a Gradle build per run; this repo's CI becoming sensitive to
the server repo's build health) are tracked in #105.

Defects found by this work, all reproduced against a main build, not just a tagged RC:

Also re-confirmed #96, #97 and #98 still reproduce on main. main resolves none of them — the
rc.23→main diff touches zero scheduler files, and SchedulerResource still declares @PutMapping.
Worth stating because main does contain fix: 404 on SSE stream for nonexistent execution IDs,
which reads like #102 but is already in rc.23 and addresses a different case.

Corrects venue-naive tests. auth.bats is Orkes-only by design — it asserts that
unauthenticated calls fail, which is untrue against OSS where anonymous access is legitimate — and
task.bats tests 9, 10, 13 and 14 exercise task signal, which OSS refuses outright. These
previously would have failed against any OSS server.

Documentation. Adds CONTEXT.md (the repo had none, though docs/agents/domain.md says it
should), docs/adr/0001-... recording the testing decisions, and test/e2e/README.md with the
runnable commands. Also drops conductor code from CLAUDE.md, which #95 removed without removing
its docs, and corrects the help-group listing against actual --help output.

Issue #
Found #101, #102, #103, #104; follow-up tracked in #105. Recovers coverage relevant to #86.
Re-verified #96, #97, #98 as still reproducing.

Alternatives considered

k6. Explicitly proposed and rejected on capability rather than preference: k6's JavaScript
runtime has no subprocess API — no child_process, no os.exec — so it cannot invoke a CLI binary
at all. It would need a custom build with xk6-exec to drive a shell from inside a load-testing VM.
k6 remains right for load-testing the Conductor server's HTTP API, which is a different project.

Go integration tests via os/exec. Genuinely attractive: one toolchain, structured JSON
assertions, idiomatic gating via build tags. Rejected because 129 bats tests already existed and
worked, so this would mean two E2E idioms to maintain and teach for a real but not decisive benefit.

Positive tagging. Rejected after finding that bats file_tags can be added to by test_tags
but never subtracted from. Expressing "valid everywhere except these two tests" positively would
mean tagging ~150 tests explicitly. Only exceptions are tagged, and selection uses negation.

Pinning a published RC instead of building from source. RCs are cut from main, so an RC is a main
snapshot — main was 8 commits ahead of rc.23 when this was written, none of them CLI-facing. Faster,
cached and reproducible, and it isolates this repo from the server repo's build health. Deferred
rather than rejected: the ask was to test what will actually ship. Tracked in #105.

Scheduling the nightly tier. Deliberately not enabled. An unattended cron that spends model
tokens and reports to nobody is worse than no coverage. The cron line is present but commented.

Covering worker and code. Both were excluded because #92 and #93 proposed reworking and
removing them. Both have since merged (#94, #95): code is gone, so its exclusion is now permanent
and correct, but worker is converged and stable, so it is worth covering in a follow-up.

Known limitations

  • The CI has now run (it had not when this was first opened). All three jobs are green: test,
    E2E (Enterprise), E2E (local OSS server); the nightly job correctly skips on pull requests.
    Building the server from source took 3m56s including a cold Gradle cache.
  • 6 of server.bats's 8 tests skip in CI. conductor server start can only download published
    versions, so a source-built jar must be launched with java -jar; with no CLI-managed pid file
    those tests skip with a stated reason rather than failing. Revisit building the Conductor server from source in E2E CI (and the 6 server.bats tests it skips) #105 proposes a --jar flag to close it.
  • 20 tests skip on the Enterprise server, which has neither the Agents API nor API Gateway
    enabled: agent (5) and api_gateway (15). Green on that job therefore means "nothing failing",
    not "everything exercised". Both are covered on the local OSS server, which has both capabilities.
  • The first CI run of this PR failed with exactly those 20 tests before the capability guards were
    added; the CLI's own error messages were correct throughout.
  • worker, code and TypeScript deploy are not covered — see above.
  • The nightly tier runs only on manual dispatch, so deploy coverage is "when someone remembers".

🤖 Generated with Claude Code

ambiorix2099 and others added 7 commits August 5, 2026 12:43
…tag all suites by tier

Adds 48 E2E tests (129 -> 177) covering five commands that previously had no E2E
coverage at all, and introduces a bats tag scheme so the suite can be selected by
tier and by server venue.

New suites:
- agent.bats (17)  definition scaffolding, CRUD, execution search, live runs
- config.bats (9)  profile save/list/delete and --profile precedence
- doctor.bats (7)  runtime, server and AI provider reporting
- deploy.bats (7)  Python agent discovery and deployment
- server.bats (8)  local server status/logs and mutual-exclusion guards

Tagging uses negative selection: suites carry a tier, and only the exceptions are
marked. bats file_tags can be added to by test_tags but never subtracted, so
tagging every test positively would have been unmaintainable.

  tier:pr | tier:nightly    when it runs
  orkes-only               requires Orkes/Enterprise
  oss-only                 requires a local OSS server
  needs:llm                requires a provider credential
  needs:agentspan          requires the agentspan Python package
  needs:timeout            requires GNU timeout(1)

Selections:
  tier:pr,!orkes-only   109   OSS venue
  tier:pr,!oss-only     157   Enterprise venue
  tier:nightly           12   5 needs:llm + 7 needs:agentspan

Correctly classifies tests that were previously venue-naive. auth.bats is
Orkes-only by design: it asserts that unauthenticated calls fail, which is not
true against OSS where anonymous access is legitimate. task.bats tests 9, 10, 13
and 14 exercise task signal/signal-sync, which OSS rejects outright.

Known-broken behaviour is encoded as a test of the *correct* behaviour plus a
skip naming the issue, so the gap is executable documentation and un-skipping is
a one-line change: #96 (agent compile), #98 (default profile), #101 (schedule
pause/resume on OSS), #103 (stale model strings). #97 and #102 are covered the
same way in the nightly tier.

server.bats deliberately omits the start/stop lifecycle: the CLI tracks a single
instance in server-state.json, so starting or stopping there would clobber the
server the rest of the run depends on. It asserts the read-only commands and the
guards instead, which also avoids triggering a 435 MB download.

LLM assertions are structural only -- execution ids and terminal status, never
generated text -- so provider variance cannot cause flakes.

Verified against server 3.32.0-rc.23 on local OSS: 109 passed, 0 failed, 6
skipped, every skip carrying a reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ateway tests

Replaces the single e2e-test job with three tag-selected jobs, so the suite is
chosen by tier and server venue rather than by a hand-maintained file list.

  e2e-enterprise     PR   remote Enterprise; Orkes-only surface
  e2e-local-server   PR   pinned OSS jar; OSS paths, `server`, and the RC server
  e2e-nightly        cron live LLM runs + agentspan deploy tests

Per-PR test executions go from 111 to 266.

Recovers api_gateway.bats. Its 18 tests existed but appeared in no job's file
list, so they ran nowhere; tag selection picks them up automatically, and future
suites will be too.

e2e-local-server is the job that closes the structural gaps the Enterprise job
cannot: it exercises the OSS code paths (CONDUCTOR_SERVER_TYPE was pinned to
Enterprise everywhere, so the OSS schedule support added in #86 was untested),
covers the `server` command, and validates the CLI against the pinned
release-candidate server rather than a remote of unknown version.

Details worth noting:

- The server is started from a scratch directory, not the repo, because
  `server start` writes its SQLite database relative to the working directory
  with no flag to override (see #104).
- The 435 MB jar is cached on the pinned version, so only the first run per bump
  pays the download.
- Agent executions run inside the server, not the CLI, so ANTHROPIC_API_KEY is
  set on the server-start step in the nightly job. Setting it only on the bats
  step would leave the server unable to reach the provider.
- The `secrets` context is unavailable in step-level `if`, so the LLM gate reduces
  the secret to a boolean in job-level env and tests that. Missing credentials
  produce a warning annotation, not a failure, so forks do not report spurious
  red.
- auth.bats gains an `unauthenticated` tag. It needs a secured server reached
  *without* credentials, so it must be excluded from the authenticated run or its
  "should fail" assertions fail for the opposite reason.
- Both server-backed jobs assert that bats supports --filter-tags (>= 1.8.0)
  before running, since silently ignoring the filter would run the wrong tests.
- upload-artifact does not expand '~', so the server log is copied into the
  workspace before upload.

workflow_dispatch inputs toggle each job independently; the nightly tier is off by
default on manual runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Creates the repo's domain glossary and records the testing decisions taken while
building out E2E coverage.

CONTEXT.md pins vocabulary that was previously used loosely, including two
distinctions that caused real confusion in this work:

- Definition vs Execution — the CLI separates registered templates from their
  running instances, and the command surface mirrors that split.
- Agent vs Worker — an Agent's turns are executed by the *server*, a Worker runs
  on the machine that polls. This is why the server, not the CLI, needs the model
  provider credential; getting it backwards produces provider auth failures that
  look like CLI bugs.

It also fixes the testing terms coined here: tier (when a test runs), venue (which
distribution it is valid against), OSS-safe, and known-broken guard. Venue is
deliberately defined as validity rather than location — a test asserting that
anonymous access is refused is not merely misplaced against OSS, it is wrong.

ADR-0001 records why bats with tag selection, and why not the alternatives. k6 was
explicitly proposed and is rejected on capability, not taste: its JS runtime has no
subprocess API, so it cannot invoke a CLI at all. Worth recording because it will
otherwise be suggested again. Also records why tagging is negative rather than
positive — bats file_tags can be added to but never subtracted, so tagging every
test positively would have been unmaintainable.

test/e2e/README.md gives the runnable form: prerequisites, the exact selection
commands per venue and tier, the tag table, and the conventions for adding suites.
All four selection counts in it were verified against the suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the cron trigger. The tier:nightly suites spend model tokens on every run,
and an unattended schedule reporting to nobody is worse than no coverage — it
accrues cost and trains people to ignore a red job.

The job now runs only on an explicit workflow_dispatch with run_nightly=true
(default false). The cron line is kept as a comment alongside the one other change
needed to enable it, so turning it on later is a two-line edit rather than
archaeology.

Keeps the tag name tier:nightly. It states the intended cadence, and renaming it to
tier:manual would churn 12 tests and three documents to describe a temporary state.
CONTEXT.md, ADR-0001 and the E2E README now say explicitly that the name is intent
rather than current wiring, so the gap between the two is documented rather than
misleading.

Nothing about test selection changes: 109 OSS, 12 nightly, unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…inned RC

Releases are cut from conductor-oss/conductor main, so the CLI should be validated
against main rather than a tagged release candidate. No artifact is published from
main — Maven Central carries only tagged RCs and the S3 'latest' jar has not moved
since 3 June — so the E2E jobs now check out the server repo and run
`:conductor-server:bootJar`.

Verified locally before switching: built main (8 commits ahead of v3.32.0-rc.23) and
re-ran the OSS selection against it. 109 passed, 0 failed. The one CLI-facing change
in those 8 commits, aggregate token usage on the execution payload, does not affect
CLI parsing.

Also re-verified every filed defect against a main build. None is resolved by main:

  #101 schedule pause/resume  405; probe confirms GET->405, PUT->200
  #96  agent compile          400 agentConfig is required
  #97  execution --since      returns nothing while --name returns the row
  #98  config save default    profile name is required
  #102 agent stream           still running at 20s

That is worth recording because main *does* contain "fix: 404 on SSE stream for
nonexistent execution IDs", which reads like #102 but is already in rc.23 and
addresses nonexistent rather than terminal executions. The scheduler is untouched
between rc.23 and main — zero files — and SchedulerResource still declares
@PutMapping, so #101 cannot have been fixed there.

Consequence to be aware of: `conductor server start` can only download published
versions, so a source-built jar must be launched with `java -jar`. There is then no
CLI-managed pid file and the six server-dependent tests in server.bats skip rather
than run. They skip with a stated reason, and the guard behaving this way is why the
run stayed green rather than reporting six false failures.

The costs of building — a Gradle build per PR run, and this repo's CI becoming
sensitive to the server repo's build health — are tracked in #105 along with the
server.bats gap and the apparently stalled S3 'latest' publish. The env var is now a
git ref rather than a version, with a TODO pointing at that issue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #95 removed the `code` command but left its documentation behind, so CLAUDE.md
still described a command that now returns `unknown command "code"`. Removes the
table rows and the flag list.

Also corrects the help-group listing, which is now verified against `--help` output
rather than asserted: `worker` belongs to Development only, not Conductor Management,
and `skill` is gone. The Conductor Management line had carried `worker` incorrectly
since the section was written.

Noticed while rebasing onto main after #94, #95 and #100 merged. Small and adjacent
to this branch's other doc work, but a separable concern — happy to split it out if
preferred.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…capabilities

First CI run surfaced 20 failures in the Enterprise job, all from two deployment
capability gaps rather than CLI defects:

  agent (5)        "Agents API is not available on this Conductor server"
  api_gateway (15) 404 "No static resource api/gateway/config/auth"

The Enterprise server CI targets has neither the Agents API nor API Gateway enabled.
The CLI reports both clearly and exits non-zero, which is correct behaviour; the tests
were simply assuming capabilities that deployment does not have.

Adds capability guards that skip with a stated reason, matching the pattern already
used by require_running_server, require_agentspan and require_llm. Both were verified
not to over-skip: on the local OSS server, which has both capabilities, all 109 tests
still run and pass.

api_gateway's guard goes in setup() so it covers all 18 tests in one place. agent's is
per test, so the three offline `agent init` tests keep running even where the server
has no Agents API.

This also corrects an assumption in the previous commit. api_gateway.bats was
described as orphaned by oversight; it now looks likely it was dropped from the CI
list deliberately, because it fails against this server. Recovering it into a
tag-selected run reintroduced those failures. The guard is the right fix either way —
the suite now runs wherever API Gateway exists and skips loudly where it does not,
rather than being silently absent from every job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/e2e.yml
@mp-orkes
mp-orkes requested a review from bradyyie August 5, 2026 21:27
Captures the decision @mp-orkes raised in review of #106: why the E2E jobs build
conductor-oss/conductor from main rather than pinning a published version with
`conductor server start --version <x>`.

States plainly that pinning was the preferred option and the original plan, and that
it was not taken for one narrow factual reason: no artifact is published from main.
Maven Central carries tagged releases only, and the S3 'latest' jar has not moved
since 3 June, so a pinned version cannot validate the code the release is cut from.

Also records that in this instance pinning would have been adequate — main was 8
commits ahead of rc.23, none CLI-facing — but that this is a property of that
particular gap rather than a guarantee, and only knowable after the fact.

Consequences are listed without softening: a Gradle build per PR, CI becoming
sensitive to the server repo's build health, non-reproducible runs, and six server.bats
tests skipping because `server start` cannot launch a source-built jar. Includes
explicit triggers for reverting to a pin, and notes the earlier revision of #106
already implemented the pinned form, so reverting is recoverable from history rather
than a redesign.

ADR-0001 now points here instead of summarising the trade-off inline.

Tracked for revisit in #105.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ambiorix2099

Copy link
Copy Markdown
Contributor Author

@mp-orkes as promised, added ADR-0002 in 9cabb23 recording this decision.

It states plainly that pinning was the preferred option and the original plan, and that it was not taken for one narrow factual reason: nothing is published from main — Maven Central carries tagged releases only, and the S3 latest jar has not moved since 3 June. So a pinned version cannot validate the code the release is cut from.

It also records that in this instance pinning would have been adequate — main was 8 commits ahead of rc.23, all UI/CI/token-usage, nothing CLI-facing — but that this is a property of that particular gap, not a guarantee, and only knowable after the fact.

Consequences are listed without softening, including the two you'd care about most: a Gradle build on every PR run, and this repo's CI becoming sensitive to the server repo's build health. Plus explicit triggers for switching back, the first being "a red CI run traced to the server build rather than the CLI".

One practical note: the earlier revision of this PR already implemented your pinned form with a CI parameter, so reverting is recoverable from git history rather than a redesign. CONDUCTOR_SERVER_REF is the single knob.

Tracked for revisit in #105.

@ambiorix2099
ambiorix2099 merged commit c7eb744 into main Aug 5, 2026
4 checks passed
@ambiorix2099
ambiorix2099 deleted the feature/e2e-test-coverage branch August 6, 2026 04: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.

3 participants