Skip to content

ci: run python SDK agent e2e against a SQLite server built from source - #1351

Open
ling-senpeng13 wants to merge 41 commits into
mainfrom
ci/python-sdk-e2e-sqlite
Open

ci: run python SDK agent e2e against a SQLite server built from source#1351
ling-senpeng13 wants to merge 41 commits into
mainfrom
ci/python-sdk-e2e-sqlite

Conversation

@ling-senpeng13

@ling-senpeng13 ling-senpeng13 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Adds a gating python-sdk-e2e job to CI that boots the conductor server built from this commit in SQLite mode (the default persistence — no external DB) and runs the python SDK agent e2e suite against it, so a server change can't break the SDK end-to-end before a release.

How

  • From-source server, SQLite: builds :conductor-server:bootJar and boots it on :8080. SQLite is the default (conductor.db.type=sqlite / queue.type=sqlite / indexing.type=sqlite), so no Postgres/Redis/ES services are needed.
  • openai auto-config: the server reads conductor.ai.openai.api-key=${OPENAI_API_KEY:}, so setting the env var is enough — no manual integration registration.
  • The suite runs in place from the SDK repo. conductor-oss/python-sdk is checked out at the pinned ref into .github/agent-e2e/src-python, and .github/agent-e2e/run-suite.sh runs its e2e/ against that repo's own build files — the same entrypoint python-sdk's own agent-e2e.yml uses. No dependency set is restated on our side, so an upstream dep change or version bump can't leave us testing something stale; if upstream breaks its own entrypoint, its CI goes red before ours does. Mirrors orkes-io/orkes-conductor#3888.
  • The SDK is built from source, not resolved from PyPI. Nothing has to be published for a ref to work, so the pin can be any tag, branch or SHA — set the CONDUCTOR_PYTHON_E2E_BUNDLE_VERSION repo/org variable to point the lane at unreleased SDK work without a commit here. (The _BUNDLE_ in the name is vestigial; it matches the variable already configured in settings.) This lane tests the SDK at that ref rather than its packaged artifact — packaging is the SDK repo's own release CI to cover.
  • Scheduler wired up: SCHEDULER_CONDUCTOR_URL points at the same server. Suite21 skips itself unless GET {url}/scheduler/schedules answers 200, and its default is port 8089; conductor-oss serves the scheduler itself (conductor.scheduler.enabled=true, SchedulerResource at /api/scheduler).
  • Cost control: runs on push/dispatch/schedule, and on PRs only when agent-relevant paths change (ai/, agentspan/, server/, core/, sqlite-persistence/, .github/agent-e2e/, the workflow) via a detect-changes agent filter.
  • Fork PRs are skipped. GitHub withholds secrets from fork-originated runs, so OPENAI_API_KEY would be empty and the LLM suites would fail for a reason an outside contributor cannot fix. The fork test is nested inside the pull_request branch of the if — as a top-level AND it would also disable push/dispatch/schedule, where github.event.pull_request is null.
  • Passed-count floor. A pytest skip is not a failure, so a server change that breaks a test's setup silently converts pass → skip and the lane stays green while covering less. After the run, results/junit-e2e.xml is parsed and the job fails if fewer than E2E_MIN_PASSED (135) tests passed, or if the file is absent. -rs is passed to pytest so every skip reason appears in the log.
  • Known failures are xfail-ed via .github/agent-e2e/known_failures_plugin.py, loaded as a trailing -p known_failures_plugin arg so the upstream suite is never modified. It reads known-failures-python.json, reports each entry's match count, and warns when an entry matches 0 or >1 tests so a stale or over-broad key is visible rather than silent.
  • mcp-testkit is pinned to 1.0.3 with mcp<2: it declares mcp[cli]>=1.0.0 unbounded, and mcp 2.0.0 dropped mcp.server.fastmcp which the testkit imports at start-up — unpinned, the testkit exits immediately and every MCP suite skips. Same pin as fix(test): pin mcp<2 and gate mcp-testkit readiness #1408, which fixed the equivalent install in the test-harness.

Test results

145 collected → 135 passed, 7 skipped, 3 xfailed, 0 failed (~2m30s).

10 of 145 tests do not assert anything. Full breakdown:

xfail (3)

Test Reason
test_suite14 :: test_stateful_swarm_handoff_completes Stateful swarm times out. A tool worker on a swarm member registers with domain=None while the stateful swarm makes the server domain-route its task, so nothing polls it (pollCount=0) and the FORK's JOIN never satisfies. Fixed by conductor-oss/python-sdk#459 (unmerged). Listed run: false — it costs ~5min per attempt, ~15min with reruns.
test_suite21 :: test_get_after_delete_returns_none get_schedule() on a deleted schedule is expected to return None; the server 404s and OrkesSchedulerClient propagates ApiException(404). Server/SDK contract mismatch.
test_suite7 :: test_image_openai Carries its own @pytest.mark.xfail upstream — not listed by us.

skipped (7)

Count Reason Fixable here?
2 server secret store is read-only (env-backed) No — conductor-oss serves secrets from the server process env
1 jupyter_client not installed Yes, not yet wired
1 GOOGLE_AI_API_KEY not set Needs a key
1 DG skill not installed Local-developer path, N/A on CI
1 Server does not attach media to the Anthropic provider request Server gap
1 Could not extract PDF URL from task output Worth a look — reads like a real breakage wearing a skip

⚠️ Requires a repo secret

The job needs OPENAI_API_KEY (and optionally ANTHROPIC_API_KEY) as a repository/organization secret. The keys are deliberately not read from vars — GitHub variables are unmasked in logs and exposed to fork PRs, which would undermine the fork guard above.

Known limitation

run-suite.sh pip-installs at run time from the SDK's own package metadata, which bounds only its direct deps — pytest, langgraph and the provider SDKs all float. An upstream release can therefore redden this lane with no change to this repo, and it will present as a server regression. If the lane fails for no apparent reason, check the pip install output before hunting a server cause. Pinning the transitive set via PIP_CONSTRAINT works but needs the lockfile regenerated on every SDK bump — judged not worth the upkeep.

@ling-senpeng13

ling-senpeng13 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Validated green ✅ (and the gate already earned its keep)

Ran on an internal branch so the org OPENAI_API_KEY / ANTHROPIC_API_KEY secrets are available (fork PRs can't see them — that's why the earlier fork draft #1350 was superseded by this one).

Final result: python-sdk-e2e job success0 failures / 154 tests / 26 skipped against a from-source server booted in SQLite mode (conductor-ai-e2e-python-2.0.0-rc2 bundle, real openai/gpt-4o-mini).

The gate surfaced a real server gap

While bringing this up, the job caught a genuine discrepancy (not infra):

agentspan skill registerPOST /api/skills/register returns HTTP 404 "No static resource api/skills/register" on the server built from main.

That endpoint exists in the released 3.32.0-rc.8 (which conductor-oss/python-sdk's own agent-e2e.yml pins, so it's green there) but is absent from a from-source main build. Three Suite16 cli-skills tests hit it:

  • test_cli_skill_register_list_get_pull_and_delete
  • test_registered_cross_skill_dependency_versions_are_pinned
  • test_cli_skill_run_registered_executes_downloaded_script_worker

They're xfail-ed (with the reason above) in .github/agent-e2e/known-failures-python.json so the lane gates green today. Worth a maintainer's look: either the skills-register endpoint needs to land on main, or it's expected to be absent and stays skip-listed. When it lands, those tests XPASS — the signal to delete the entries.

The other 151 tests — LLM chat/complete, tools, MCP, guardrails, termination, and the credential-lifecycle suites — all pass.

How the skip support works (kept even though ~green)

  • .github/agent-e2e/known_failures_plugin.py — external pytest plugin (-p known_failures_plugin) that xfails any node-id listed in E2E_KNOWN_FAILURES. Tests still run; a fixed bug XPASSes (cleanup signal); a stale/mismatched entry is a harmless no-op that can't hide a regression.
  • .github/agent-e2e/known-failures-python.json — the conductor-oss list (the 3 above).

Cost control

Runs on push/dispatch and on PRs only when agent-relevant paths change (ai/, conductor-agentspan/, server/, core/, sqlite-persistence/, the workflow) via the detect-changes agent filter — unrelated PRs don't spend LLM budget.

@ling-senpeng13

Copy link
Copy Markdown
Contributor Author

Correction on the cli-skills failures — root cause + design decision

My earlier note said the skills-register endpoint was "missing on main." That was wrongSkillController exists; it's gated:

@ConditionalOnProperty(name = {"agentspan.embedded", "agentspan.skills.enabled"}, havingValue = "true")

agentspan.embedded currently resolves true in OSS (via conductor.integrations.ai.enabled=true), but agentspan.skills.enabled is unset, so POST /api/skills/register → 404.

Design decision (per maintainers): agentspan.embedded is intended to be true only for orkes-conductor and false for conductor-oss, and that design may change. So we will not force-enable skills on boot for OSS — the skills API isn't a settled OSS surface. The 3 Suite16 cli-skills tests stay xfail-ed with an accurate reason, tracked in #1353; they'll XPASS (and the entries get removed) once the OSS skills story is settled.

Everything else is unchanged: 0 real failures / 154 tests, gate green.

@mp-orkes mp-orkes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking

1. Fork PRs are unhandled. Public repo, and GitHub doesn't pass secrets to forks → OPENAI_API_KEY is empty, but the job runs anyway. Every outside contributor who touches core/** gets either a red X they can't fix, or a green check that tested nothing. Fix:

if: >-
  (github.event_name != 'pull_request' ||
   needs.detect-changes.outputs.agent == 'true') &&
  github.event.pull_request.head.repo.full_name == github.repository

2. Tests that quietly stop running don't turn the gate red. A pytest test ends in one of three ways: pass (checked something, it worked), fail (checked something, it broke), or skip — checked nothing at all. This suite's tests skip themselves whenever they can't set up (missing binary, missing API key, endpoint that doesn't answer). CI only goes red on fail, so skips are free. The green run was 154 tests: 127 checked something, 27 checked nothing, ✅ with no mention of it.

The danger: a server change that removes something a test needs in its setup makes that test skip rather than fail — coverage silently drops to zero and the gate stays green. The lane built to catch the break becomes the thing hiding it. Both flags on the report step (fail_on_failure: false, require_tests: false) mean there's no backstop either; even an empty results file renders green.

Fix: after the run step, parse results/junit-e2e.xml and fail if the passed count drops below a floor (127 today, bumped deliberately). Add -rs to the pytest args so skip reasons show up in the log.

3. Finding #2 has already bitten this PR. Commit 8ca8ffe added the agentspan CLI download specifically so the credential suites would run. They still skip — suite2/3/4/5 @credentials and both suite26 tests, plus all 10 of test_suite21_scheduling.py. CI was green, the PR body lists them as passing, and nobody noticed. Either wire them up or correct the claims; the CLI download step may be dead weight as written.

4. _matches() can silently xfail unrelated tests. known_failures_plugin.py:58 — the unanchored nid.endswith(suf) arm means a key like _completes matches every test whose name ends that way, which is exactly the "silently hide a regression" case the docstring says can't happen. Drop that arm; keep the exact and ::-anchored matches.

5. The plugin's only diagnostic never prints. [known-failures] xfail-marked N item(s) is absent from the run log: under xdist, collection happens in workers, where terminalreporter is None. So there's no visibility into whether entries matched — and combined with #4, a typo'd key and an over-broad key are equally invisible. Log per-key match counts and warn on zero.

Comment thread .github/workflows/ci.yml Outdated

@mp-orkes mp-orkes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some feedback - please address it before requesting another review.

@ling-senpeng13

Copy link
Copy Markdown
Contributor Author

Thanks — all five addressed. Commit per item below. Two of them I implemented differently from the suggested fix, and I've said why rather than quietly diverging.


1. Fork PRs are unhandled — b5f91207f

Agreed and fixed. I did change the condition, because the suggested form also disables the lane on push, workflow_dispatch and schedule: on those events github.event.pull_request is null, so .head.repo.full_name == github.repository is false, and a top-level AND fails the whole expression. Nesting the fork test inside the pull_request branch keeps the intent without taking out the lane's main coverage:

if: >-
  github.event_name != 'pull_request' ||
  (needs.detect-changes.outputs.agent == 'true' &&
   github.event.pull_request.head.repo.full_name == github.repository)
Scenario suggested applied
push / dispatch / schedule ❌ skipped ✅ runs
PR same-repo, agent paths ✅ runs ✅ runs
PR from fork ❌ skipped ❌ skipped

On the two outcomes named: the realistic one is the red X. The server boots fine without a provider key (it just logs cannot init ...AIConfiguration), and nothing in conftest.py skips on "no model available" — it only skips on "server unreachable" — so the suites fail rather than false-green.

2. Skips don't turn the gate red — b89ba0e10, floor raised in 7b1b23687

Implemented as described: -rs added, and a step after the run parses results/junit-e2e.xml and fails below a floor (also fails if the file is absent). Both flags you flagged on the report step are indeed no backstop — confirmed.

Floor is 137, not 127: item 3 recovered 10 tests. One subtlety worth recording — junit folds xfail into skipped, so passed has to be computed as tests - errors - failures - skipped. Verified against the real file and against CI, both giving passed=137.

3. Already bitten — 7b1b23687 (+ 2c41926a4 → reverted by 10d77edf2), PR body rewritten

Correct, and it split three ways:

Scheduling — wired up, +10 tests. Not a credential issue at all. Suite21 probes {SCHEDULER_CONDUCTOR_URL}/scheduler/schedules and defaults to port 8089; the lane's server is on 8080, so the probe never matched. conductor-oss serves the scheduler itself (conductor.scheduler.enabled=true by default, SchedulerResource at /api/scheduler) — verified 200 against the lane's own server. Pointing the var there: 10 pass, 1 fails for a real reason (get_schedule() on a deleted schedule 404s instead of returning None), now listed with that description rather than swept up. It's 11 tests in that file, not 10.

Credential suites — cannot be wired up here. They skip on server secret store is read-only (env-backed), a genuine conductor-oss limitation; suite26's own comment says so. Those 5 will not run on this flavor.

"CLI download may be dead weight" — tested, and it is not. I removed it on exactly that reasoning and the run turned 3 clean skips into hard failures in suites 2, 4 and 5. The credential suites reach their skip through the CLI: CredentialsCLI.set() shells out, the server rejects the write as read-only, and that stderr is what triggers pytest.skip. subprocess.run has no try/except there, so with no binary they raise FileNotFoundError. Reverted in 10d77edf2 and the reason is now recorded at the step so it doesn't get removed again.

PR body rewritten with a "Coverage, honestly" section: 154 collected → 137 passed, 11 skipped, 6 xfailed, every skip tabled with its reason and whether it's fixable here.

4. _matches() can silently xfail unrelated tests — f954b9e3c

Real bug, confirmed: _completes does match test_stateful_swarm_handoff_completes via the unanchored arm.

Dropping that arm would have broken the lane, though. Node-ids are e2e/<file>.py::<Class>::<test> while the list's keys are <file>.py::<Class>::<test>, so neither the exact nor the ::-anchored arm matches them — the unanchored arm is the only thing making today's 5 entries work. Removing it would have un-xfail-ed every known failure and turned the lane red.

Anchored on / instead, which is the same guarantee:

if nid == suf or nid.endswith("::" + suf) or nid.endswith("/" + suf):

Verified: each of the 5 entries matches exactly one node-id, no node-id matches two keys, and _completes / completes / returns_none / and_delete / e now match nothing (all previously leaked).

5. Diagnostic never prints — 10dbead16

Confirmed before fixing — zero occurrences of [known-failures] in either full CI-shaped run. (Careful if you re-test: it does print under --collect-only -n, because xdist collects on the controller there.)

Per-key match counts now reported over two channels, since neither alone reaches the log: warnings for the 0-match and >1-match cases (xdist forwards worker warnings to the controller), and terminal lines for the full table, written inline without xdist or from pytest_testnodedown with it. Counting is per key rather than per first-match, so an over-broad key stays visible even when another key claimed the item first.

[known-failures] xfail-marked 5 item(s) from .../known-failures-python.json
[known-failures]   1x  test_suite14_..._handoff_completes
[known-failures]   1x  test_suite16_..._register_list_get_pull_and_delete
[known-failures]   1x  test_suite16_..._run_registered_executes_downloaded_script_worker
[known-failures]   1x  test_suite16_..._cross_skill_dependency_versions_are_pinned
[known-failures]   1x  test_suite21_..._get_after_delete_returns_none

An injected bogus key reports 0x ... <-- MATCHED NOTHING plus a warning.


Also fixed from a self-review pass

  • 44b8fc621 — stale plugin docstring ("empty when the suite is green" against a 5-entry list); a missing E2E_KNOWN_FAILURES path loading nothing and returning before any reporting, so every forgiven failure reported as real with no explanation; and the mcp-testkit readiness probe, which used curl -sf against a root path that returns 404 — it never succeeded, always burned all 15 iterations, and printed "started" whether or not anything was listening.
  • 6935ea940setup-java v4 → v5 in this job, matching every other job.
  • 137428dfd — reverted a transitive-dependency lockfile I'd added. It worked, but had to be regenerated on every bundle bump, and it's the wrong repo for it: the floating deps are declared in python-sdk (pyproject.toml + the bundle's generated requirements.txt), not here. Replaced with a comment recording the risk and, more usefully, that a mystery failure here should be triaged as a possible dependency bump before hunting a server cause.

Current state

137 passed, 11 skipped, 6 xfailed, 0 failed — matching locally and on CI.

Known-not-covered, all listed in the PR body: 5 credential tests (read-only secret store), 3 cli-skills (server-side skills API unsupported — agentspan.skills.enabled is off by default and set in exactly one place repo-wide), suite14's deterministic hang (#1363), and the scheduler 404/None mismatch. Two remaining skips are fixable and not yet wired (GITHUB_TOKEN, jupyter_client), and one — Could not extract PDF URL from task output — reads like a real breakage wearing a skip and deserves a look.

@ling-senpeng13

Copy link
Copy Markdown
Contributor Author

Blocking

1. Fork PRs are unhandled. Public repo, and GitHub doesn't pass secrets to forks → OPENAI_API_KEY is empty, but the job runs anyway. Every outside contributor who touches core/** gets either a red X they can't fix, or a green check that tested nothing. Fix:

if: >-
  (github.event_name != 'pull_request' ||
   needs.detect-changes.outputs.agent == 'true') &&
  github.event.pull_request.head.repo.full_name == github.repository

2. Tests that quietly stop running don't turn the gate red. A pytest test ends in one of three ways: pass (checked something, it worked), fail (checked something, it broke), or skip — checked nothing at all. This suite's tests skip themselves whenever they can't set up (missing binary, missing API key, endpoint that doesn't answer). CI only goes red on fail, so skips are free. The green run was 154 tests: 127 checked something, 27 checked nothing, ✅ with no mention of it.

The danger: a server change that removes something a test needs in its setup makes that test skip rather than fail — coverage silently drops to zero and the gate stays green. The lane built to catch the break becomes the thing hiding it. Both flags on the report step (fail_on_failure: false, require_tests: false) mean there's no backstop either; even an empty results file renders green.

Fix: after the run step, parse results/junit-e2e.xml and fail if the passed count drops below a floor (127 today, bumped deliberately). Add -rs to the pytest args so skip reasons show up in the log.

3. Finding #2 has already bitten this PR. Commit 8ca8ffe added the agentspan CLI download specifically so the credential suites would run. They still skip — suite2/3/4/5 @credentials and both suite26 tests, plus all 10 of test_suite21_scheduling.py. CI was green, the PR body lists them as passing, and nobody noticed. Either wire them up or correct the claims; the CLI download step may be dead weight as written.

4. _matches() can silently xfail unrelated tests. known_failures_plugin.py:58 — the unanchored nid.endswith(suf) arm means a key like _completes matches every test whose name ends that way, which is exactly the "silently hide a regression" case the docstring says can't happen. Drop that arm; keep the exact and ::-anchored matches.

5. The plugin's only diagnostic never prints. [known-failures] xfail-marked N item(s) is absent from the run log: under xdist, collection happens in workers, where terminalreporter is None. So there's no visibility into whether entries matched — and combined with #4, a typo'd key and an over-broad key are equally invisible. Log per-key match counts and warn on zero.

Thanks for the review. I have fixed these findings. Please see #1351 (comment) for details

@ling-senpeng13
ling-senpeng13 marked this pull request as draft July 30, 2026 16:54
ling-senpeng13 and others added 11 commits July 30, 2026 10:21
Adds a `python-sdk-e2e` job to CI that boots the conductor server built from
the current commit in SQLite mode (the default persistence — no external DB)
and runs the released python SDK agent e2e suite against it, so a server
change can't silently break the SDK before a release.

- The suite + bundle come from conductor-oss/python-sdk
  (conductor-ai-e2e-python-<version>, pinned); fetched at runtime, sha256-verified.
- The server auto-configures the openai provider from OPENAI_API_KEY
  (conductor.ai.openai.api-key), so no manual integration setup is needed.
- Known failures are xfail-ed via an external pytest plugin
  (.github/agent-e2e/known_failures_plugin.py loaded with -p) + a per-repo
  list (known-failures-python.json). The suite is green today so the list is
  empty; the mechanism stays so the lane can gate while any future gap is
  fixed (a fixed bug XPASSes; a stale entry is a harmless no-op).
- Gating. Runs on push/dispatch and on PRs touching agent-relevant paths
  (ai/, conductor-agentspan/, server/, core/, sqlite-persistence/, the
  workflow) via a detect-changes `agent` filter, to avoid spending LLM
  budget on unrelated PRs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The credential-lifecycle suites (Suite2/4/5 @credentials) shell out to the
`agentspan` binary and errored with FileNotFoundError instead of skipping,
so the first run showed 3 failed / 126 passed. Download the pinned agentspan
CLI release (mirrors conductor-oss/python-sdk agent-e2e.yml) and point
AGENTSPAN_CLI_PATH at it so those suites can run. No product issue — the
server + LLM path were already green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The re-run failed on infra, not the e2e (which never ran):
- `gh release download --output agentspan` collided with the repo's existing
  `agentspan/` module dir ("already exists"). Download the CLI to
  `agentspan-cli` instead (+ --clobber) via AGENTSPAN_CLI_PATH.
- The junit report step couldn't create its check ("Resource not accessible
  by integration") — the workflow had no `checks: write`. Add a job
  permissions block, and mark the report step continue-on-error so gating
  stays purely the e2e run step's exit code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The python-sdk-e2e gate correctly surfaced a real server gap: the CLI's
`skill register` calls POST /api/skills/register, which the server built
from main returns 404 for ("No static resource api/skills/register"). The
endpoint exists in the released 3.32.0-rc.8 that python-sdk's own CI pins,
so it's green there but not against a from-source main build.

Add the 3 affected Suite16 cli-skills tests to the conductor-oss
known-failures list so the lane gates green while the skills-register API
gap is investigated (an XPASS will flag it once the endpoint lands). The
other 151 tests — LLM, tools, MCP, guardrails, credential lifecycle — pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ted (#1353)

Earlier reason ("skills-register endpoint missing on main") was wrong: the
SkillController exists but is gated on agentspan.embedded (intended
orkes-only for OSS) AND agentspan.skills.enabled, so POST /api/skills/register
returns 404 on conductor-oss. Per design direction, agentspan.embedded should
be false for conductor-oss (the design may change), so the skills API isn't a
settled OSS surface — do not force-enable it on boot.

Keep the 3 Suite16 cli-skills tests xfail-ed with the accurate reason + link
to the tracking issue #1353. They XPASS (remove the
entries) once the OSS skills story is settled.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sharpen the 3 Suite16 cli-skills reasons with the traced root cause: #1288
(rc.9) changed the SkillController gate from agentspan.embedded [1 prop] to
agentspan.embedded + agentspan.skills.enabled [2 props], flipping the skills
API off-by-default on main (served on rc.8, which python-sdk pins). Add a
shared _CONTEXT_cli_skills note (ignored by the plugin) with the full trace
and the #1353 decision. No behavior change — same 3 node-ids xfail-ed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion (#1363)

The python-sdk-e2e gate caught a real, deterministic regression on main:
test_stateful_swarm_handoff_completes hangs (workflow stuck RUNNING ~908s,
2/2 runs). Root cause: #1356 ("Enhances A2A/AgentSpan execution") stopped
registering the swarm transfer/handoff task defs (now compiler-owned INLINE),
but the pinned SDK bundle (2.0.0-rc2, the latest release) still PUTs them ->
updateTaskDef NotFound -> agents can't hand off -> workflow never completes.
Passed pre-#1356 (2026-07-17). Server-side fix owned by #1356; no newer SDK
to bump to. Tracked in #1363.

xfail it so the gate goes green; it XPASSes (remove the entry) once #1363 is
fixed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The stateful-swarm-handoff xfail (#1363) is a deterministic ~908s hang that
would burn ~15 min of CI every run. Add per-entry run control to the plugin:
a JSON value may now be a reason string (run=True, XPASSes when fixed) or an
object {"reason":..., "run":false} to xfail WITHOUT executing.

Set the swarm entry to run:false so it's skipped (marked xfailed [NOTRUN], no
hang); the 3 cli-skills entries stay run=True. Un-list the swarm entry
manually when #1363 is fixed (a non-run xfail can't auto-XPASS).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The released python e2e bundle reads CONDUCTOR_SERVER_URL and
CONDUCTOR_AGENT_LLM_MODEL; the AGENTSPAN_* names were ignored, so the
suite fell back to defaults instead of the CI server and pinned model.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Read CONDUCTOR_PY_E2E_BUNDLE_VERSION from `vars` and fall back to the
2.0.0-rc2 pin, so a bundle can be trialled without a commit. An unset
variable is the empty string (falsy), so the fallback applies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ling-senpeng13 and others added 9 commits July 30, 2026 10:21
Public repo, so GitHub withholds secrets from fork-originated runs:
OPENAI_API_KEY arrives empty, the server's openai provider fails to
initialise, and the LLM suites fail for a reason an outside contributor
cannot fix. Skip rather than run a lane that cannot pass.

The fork test is nested inside the pull_request branch on purpose. As a
top-level AND it would also disable push, workflow_dispatch and schedule,
where github.event.pull_request is null so the comparison is false —
which would silently turn off the lane's primary coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pytest skip is not a failure, so CI never reddens on one. A server
change that breaks a test's setup therefore converts pass -> skip and the
lane stays green while covering less — the job built to catch the break
becomes the thing hiding it. The report step is no backstop either:
fail_on_failure and require_tests are both false, so even an empty results
file renders green.

Parse results/junit-e2e.xml after the run and fail if fewer than
E2E_MIN_PASSED tests actually passed, or if the file is absent. Floor is
127, observed identically on two CI runs (30398185123, 30400603109) and
locally; junit folds xfail into skipped, which the arithmetic accounts for.

Also adds -rs so skip reasons appear in the log — without it a test that
quietly stops running leaves no trace in the output.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_suite21_scheduling.py skips all 11 of its tests unless
GET {SCHEDULER_CONDUCTOR_URL}/scheduler/schedules answers 200. Its default
is port 8089 and the lane's server is on 8080, so the probe never matched
and the suite skipped silently on every run — no failure, no signal.

conductor-oss serves the scheduler itself (conductor.scheduler.enabled=true
by default, SchedulerResource at /api/scheduler); the endpoint was verified
to return 200 on the lane's own server. Pointing the var there runs the
suite: 10 pass, 1 fails.

That one failure is real and now recorded rather than hidden:
get_schedule() on a deleted schedule is expected to return None, but the
server 404s and OrkesSchedulerClient propagates ApiException(404).

Floor raised 127 -> 137 to lock the recovered coverage in. Verified locally:
137 passed, 11 skipped, 6 xfailed, floor check green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_matches() had a bare `nid.endswith(suf)` arm, so a key like "_completes"
matched every test whose name ended that way and silently xfail-ed
unrelated tests — precisely the hide-a-regression failure the module
docstring claims is impossible.

Anchor on "/" rather than dropping the arm. Node-ids are
"e2e/<file>.py::<Class>::<test>" while the list's keys are
"<file>.py::<Class>::<test>", so neither the exact nor the "::"-anchored
arm matches them; removing the unanchored arm outright would have stopped
all five current entries from matching, un-xfail-ing known failures and
reddening the lane.

Verified: each of the 5 entries still matches exactly one node-id, no
node-id matches two keys, and the bogus keys "_completes" / "completes" /
"returns_none" / "and_delete" / "e" now match nothing. Real collection
still reports "xfail-marked 5 item(s)" over 154 collected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plugin's only diagnostic never reached the log. run.sh runs pytest
with -n 3, collection happens in xdist workers, and a worker has no
terminalreporter — so the write was dropped. Confirmed: zero occurrences
of "[known-failures]" in either full CI-shaped run, while a non-xdist run
prints it. With matching unverifiable, a typo'd key and an over-broad key
looked identical to a correct one.

Report per-key match counts over two channels, since neither alone
suffices: warnings (xdist forwards worker warnings to the controller) for
the actionable 0-match and >1-match cases, and terminal lines for the full
table, written inline without xdist or from pytest_testnodedown with it.
Counting is per key rather than per first-match, so an over-broad key is
still visible when another key claimed the item first.

Verified under -n 3: full suite reports all 5 entries at 1x each with no
warnings; an injected bogus key reports 0x and warns. pytest_testnodedown
is marked optionalhook so the plugin still loads without xdist.

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

Three review findings on the e2e lane.

1. Stale docs. The plugin docstring said the known-failures list is "empty
   when the suite is green" — it has 5 entries and the suite is green, so
   the parenthetical read as a contradiction. Reworded, and the JSON
   _README now also documents the new match-count warnings and the fact
   that adding an entry requires lowering E2E_MIN_PASSED in the same
   commit.

2. Silent misconfiguration. A missing/typo'd E2E_KNOWN_FAILURES path loaded
   nothing and returned early before any reporting, so every forgiven
   failure reported as real with no explanation — the one case the new
   diagnostics still could not see. Now warns, and distinguishes that from
   the legitimate "no list configured" case, which stays silent.

3. Readiness probe. `curl -sf http://localhost:3001/` treats mcp-testkit's
   404-on-root as failure, so it never succeeded: the loop burned all 15
   iterations and printed "started" whether or not anything was listening.
   Check for any HTTP status instead, warn and tail the log on timeout.

Verified: bad path warns / unset path silent / good path reports 5x1 with
no warnings; probe detects a live mcp-testkit (HTTP 404) and warns when
nothing listens, without aborting under bash -e.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bundle ships prebuilt and its run.sh does a plain
`pip install -r requirements.txt` at run time. That requirements.txt pins
only conductor-python — pytest, langgraph, mcp-testkit and the provider
SDKs all float, so an upstream release can redden this gating lane with no
change to this repo, presenting as a server regression. Already real: a
langgraph/pydantic interaction with typing.TypedDict breaks suite11 on
Python < 3.12.

Add a full transitive pin set and point PIP_CONSTRAINT at it. pip honours
that env var for every install in the job, including the one inside the
bundle, so the bundle gets pinned without being modified. Constraints only
bound versions, they install nothing, and a conflict with a future bundle
surfaces as a loud resolver error — regenerate rather than delete, per the
header.

Generated with `uv pip compile --python-platform x86_64-unknown-linux-gnu
--python-version 3.12` to target the runner rather than a dev machine; the
CI log was not usable as a source because the earlier --quiet install
swallowed most of the closure (73 of 108 packages).

Verified: PIP_CONSTRAINT demonstrably rebinds resolution (forced pytest
8.3.4 / langgraph 0.6.7 against newer available), and resolving the real
requirements.txt under this file yields 108 packages with all 108 pins
satisfied and no conflict.

Also bumps this job's setup-java v4 -> v5, matching every other job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 108-pin constraints file worked, but it had to be regenerated on every
CONDUCTOR_PY_E2E_BUNDLE_VERSION bump or pip would fail on the conflict —
too much standing upkeep for the protection it bought on a CI lane.

Drop the file and PIP_CONSTRAINT. Replace with a comment recording that
transitive deps float, that an upstream release can redden the lane with
no change here, and — the useful part — that it will look like a server
regression, so check the pip install output before hunting a server cause.

setup-java v5 from the previous commit is unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entry blamed #1356 for dropping the swarm transfer task defs, giving
updateTaskDef NotFound so "agents can't hand off". Reproducing it locally
showed that is wrong on every link: the SDK registers all six
*_transfer_to_* defs successfully, the "No such task by name" server lines
are its benign lookup-before-create, and the handoff itself succeeds —
agent 0's sub-workflow reaches COMPLETED including transfer_msg.

What actually hangs is a tool worker. _register_workers reads
`agent.stateful` on the immediate agent only, but the test hangs swarm_tool
off the swarm MEMBERS, which are not themselves stateful — so the worker
registers with domain=None while the stateful swarm makes the server
domain-route the task. pollCount=0, startTime=0, the FORK's JOIN never
satisfies, workflow times out.

A second, independent bug sits behind it: the test's _find_tasks_by_type
matches only taskDefName, but handoff_check is an INLINE task whose
taskDefName is literally "INLINE", so the assertion fails even though 20
handoff_check tasks exist and are COMPLETED.

Both fixes belong to python-sdk. Verified against the pinned 2.0.0-rc2:
stock FAILED (RUNNING, 304s); SDK fix alone reached COMPLETED but failed
the handoff_check assert; both fixes PASSED in 118s.

Also corrects the timing: one attempt is ~304s, not ~908s — that figure was
three attempts under conftest's unconditional flaky(reruns=2).

Refs #1363

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ling-senpeng13
ling-senpeng13 force-pushed the ci/python-sdk-e2e-sqlite branch from f69fd19 to c680fd4 Compare July 30, 2026 17:22
ling-senpeng13 and others added 3 commits July 30, 2026 10:24
rc4 is the release that lands the changes several deferred items were
waiting on, so they all come due together:

* env names. rc4's conftest migrated to CONDUCTOR_SERVER_URL /
  CONDUCTOR_AGENT_LLM_MODEL, so the rename TODO is resolved and the vars
  are live rather than inert. Noted in-file that they must go back if the
  pin is ever moved down to rc2 or earlier, which read only AGENTSPAN_*.

* agentspan CLI, removed. rc4 dropped it entirely — no CredentialsCLI, no
  CLI_PATH, and test_suite16_cli_skills.py is gone — so provisioning it
  buys nothing. Recorded that it WAS load-bearing under rc2, where the
  credential suites reached their read-only skip by invoking it and
  removing it turned clean skips into FileNotFoundError failures, so it is
  not restored on stale reasoning.

* the three Suite16 cli-skills known failures, removed. Their file no
  longer exists, and a key matching nothing now raises a
  KnownFailuresWarning, so leaving them would add noise every run. History
  kept in _HOWTO.

* floor 137 -> 135. This is a coverage LOSS, not a fix: rc4 deleted
  test_suite16_cli_skills.py, and while three of its tests were xfail-ed
  here, two were genuinely passing (load_serve_and_run_by_name,
  run_ephemeral_executes_script_worker). Real coverage of the
  ephemeral/by-name CLI skill paths went away with the file, invisibly in
  pass/fail terms. Spelled out at the constant so the -2 is not read as
  routine.

Verified locally against a server built from this branch: 135 passed,
7 skipped, 3 xfailed, 0 failed; floor gate green; plugin reports both
remaining entries matching exactly one test with no MATCHED NOTHING.

suite14's entry stays — its fixes are in conductor-oss/python-sdk#455,
still unmerged, so rc4 does not carry them.

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

Spell out "PYTHON" to match the lane's name and the other CONDUCTOR_*
variables. Covers all three roles: the job env key, the vars.* lookup for
the commit-free override, and the shell consumer in the fetch step.

Note the vars.* name changes with it, so a repo/org variable set under the
old name stops being honoured and the lane falls back to the in-file pin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entries had grown into full root-cause write-ups — 2.2KB for one
reason string. That analysis belongs in the tracking issue and the fixing
PR, where it can be discussed and closed out; duplicated here it just goes
stale silently, which is exactly what happened to the suite14 entry.

Each value is now a short statement of what fails plus a tracking link.
_README says to keep it that way. File is 2227 bytes, down from 4781.

Kept the operational parts, which are not history and prevent real
mistakes: the E2E_MIN_PASSED coupling, and the warning that a run:false
xfail can never XPASS so it must be un-listed by hand.

Also repoints suite14 at conductor-oss/python-sdk#459, which supersedes
the now-closed #455.

Verified: plugin still reports both entries matching exactly one test,
no MATCHED NOTHING.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ling-senpeng13
ling-senpeng13 requested a review from mp-orkes July 30, 2026 17:57
@ling-senpeng13
ling-senpeng13 marked this pull request as ready for review July 30, 2026 18:03
ling-senpeng13 and others added 11 commits July 31, 2026 09:29
Mirrors orkes-io/orkes-conductor#3888. Instead of downloading the released
conductor-ai-e2e-python-<version> bundle, check out conductor-oss/python-sdk
at the pinned ref and run its e2e/ in place through run-suite.sh — the same
entrypoint that repo's own agent-e2e.yml uses.

Why: the bundle restated the suite's dependency set on our side, so an
upstream dep change or version bump could leave us testing something stale,
and the bundle had to exist as a published release artifact before we could
point at it. Running in place removes both. The SDK is now built from the
checked-out source rather than resolved from PyPI, so the pin can be any tag,
branch or SHA — set the repo variable to test unreleased SDK work without a
commit here. If upstream breaks its own entrypoint, its CI goes red before
ours does.

run-suite.sh keeps require_path guards on e2e/ and setup.py: that layout is
the only coupling left after dropping the generated manifest, so it fails with
a named cause instead of a confusing pip or pytest error.

Unchanged: the version env var (the _BUNDLE_ in its name is now vestigial but
matches the configured repository variable), known-failures passed through as
trailing pytest args, the passed-count floor, JUnit publishing and artifact
upload.

Also pins mcp-testkit, which is broken today independently of this change:
it declares `mcp[cli]>=1.0.0` unbounded, and mcp 2.0.0 dropped
mcp.server.fastmcp which the testkit imports at start-up, so the unpinned
install yields a testkit that exits immediately and every MCP suite skips.
Verified locally. #1408 already made this fix for the test-harness install;
this job predates it on this branch and never got it.

Verified against a server built from this branch: 135 passed, 7 skipped,
3 xfailed, 0 failed — identical to the bundle-based run, so the floor stays
at 135.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four comment blocks had accumulated the story of how the lane got here —
the rc2-to-rc4 migration, the agentspan CLI that used to be load-bearing,
the floor's 127/137/135 progression with CI run IDs, a cross-reference to
the PR that first pinned mcp. That belongs in git history and the PR, not
in a workflow someone reads to understand the current configuration.

Each block keeps the part that prevents a wrong edit and loses the part
that only records what happened:

* env names — keep "verify against the pinned ref before renaming, a name
  the suite does not read falls through to defaults silently"; drop the
  AGENTSPAN_* migration account.
* SCHEDULER_CONDUCTOR_URL — keep the probe and the port-8089 default that
  makes it necessary; drop "10 of the 11 then pass".
* E2E_MIN_PASSED — keep why the floor exists and the raise/lower rule, and
  add that moving the pin can change the test set so the number must be
  re-measured; drop the progression log.
* mcp pin — keep the unbounded-dependency mechanism; drop the #1408
  cross-reference.

Comments in the job: 69 lines -> 52. No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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