test(engines): binary-gated smoke/contract tier across every engine adapter (#915) - #1014
Conversation
…dapter (#915) Every external-engine adapter test patches shutil.which and Popen and replays the output the adapter's author expected. That is the mechanism by which three engines shipped with invocations that could not do any work, each behind a green suite: #913 (opencode's non-existent --non-interactive), #914 (codex's invented app-server handshake), #1012 (kilocode's non-existent `run`). This tier never mocks. Two legs, gated differently: - contract: the CLI exists, responds, and still documents the entry point the adapter depends on, asserted against the CLI's own --help rather than our code. No credentials, seconds to run, so it can gate PRs. - task: one trivial task per engine driven through adapter.run() to a terminal state with a real file written — never "exit 0", since every bug above produced a clean-looking exit having done nothing. Needs credentials and costs money, so it additionally requires CODEFRAME_ENGINE_SMOKE=1. An upstream provider fault is distinguished from an adapter defect by a deliberately narrow marker list, and reports "NO COVERAGE" rather than a quiet pass — a broad skip here would hide exactly what the tier exists to catch. Workflow runs the contract leg on PRs touching adapter paths (the job always runs and filters internally: a path-filtered required check reports "skipped" and blocks the merge forever) and the full tier weekly with the staging credentials. Verified locally against all four real CLIs: contract leg 12 passed in 2.4s; task leg drove claude-code, codex and kilocode to completion with real files written, opencode skipped on its backend's UnknownError outage.
codex review [P2]: the best-effort `npm install ... || echo` left a failed install as an absent binary, the tier skipped that engine, and pytest still exited 0 — a required check green while covering nothing. That is precisely the failure mode this tier exists to end, reintroduced in its own workflow. Installs now fail the step, and an explicit verification step fails the job naming any missing CLI. Applied to the scheduled task job too: a canary that silently stops canarying is worse than no canary.
|
Claude finished @frankbria's task in 5m 41s —— View job GLM bug-hunting review — doneScope: concrete defects only (logic errors, null/error paths, security, data loss, contract breaks, resource leaks). Style/coverage/architecture are covered by the general review and intentionally excluded.
Verified clean (no defect):
Net: 1 minor finding, no major/critical. The tier's core guarantee (no false-green) is intact — the gap only makes codex over-report failures on transient provider slowness. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…g on it (#915, #1015) The tier caught real upstream drift on its own first CI run: @kilocode/cli went 0.22.0 (2026-01, what the adapter targets) -> 7.4.17 (2026-07, what CI installs). 7.x reinstated a `run` subcommand and renamed --workspace to --dir, so the adapter is stale against a current install. Filed as #1015. That is a true positive, so it is not silenced — but it must not block every future adapter PR on an unrelated migration either. The kilocode contract expectation now carries a `known_drift` reason and xfails (non-strict) when the installed CLI lacks the flags, naming #1015 in the report. It passes normally against 0.22.0, and will flip to a pass by itself once the adapter is migrated, at which point the marker must be removed. Verified both directions: 12 passed against local 0.22.0; xfail with the drift reason against a 7.4.17 install.
The contract leg caught real drift on its own first CI run
I flagged the version gap as a risk in the PR body; it turned out to be an actual break. Investigated against a real 7.4.17 install rather than guessing:
So the adapter targets a CLI six months out of date. Filed as #1015 (P0.30) with the full comparison. On #1012, plainly#1012 was not wrong — How this PR handles it (f1b98af)Not silenced, not blocking. The kilocode contract expectation carries a Verified both directions: The scheduled Engine Task Smoke job is deliberately left to fail loudly for kilocode against 7.x — it is not a required check, and a red canary on a genuinely broken engine is the correct signal until #1015 lands. |
|
Claude finished @frankbria's task in 2m 12s —— View job GLM bug-hunting re-review — synchronize (
|
| severity | file:line | finding |
|---|---|---|
| — | — | no new findings this commit |
Net: no new defects in f1b98af. The tier's no-false-green guarantee is intact.
Still open from the earlier review (f1b98af did not touch it, so not re-posted as an inline): minor — tests/core/adapters/test_engine_smoke_tier.py:48-51 _TIMEOUT_PREFIXES omits CodexAdapter's Turn timeout / Stall timeout / Codex app-server timed out strings (codex.py:229/345/351), so a codex run exceeding the 240s turn_timeout_ms fails red instead of skipping as "ran out of time locally". Safe direction (loud, not a false green); the suggestion block is in the prior inline comment.
…al (#915) GLM review [minor]: _TIMEOUT_PREFIXES only knew SubprocessAdapter's wording, so a codex run exceeding turn_timeout_ms failed red instead of reporting NO COVERAGE. Safe direction, but it made the tier's environmental-vs-defect distinction inconsistent for exactly one engine. Added codex.py's three distinct timeout paths (:229 handshake, :345 stall, :351 turn). Enumerated rather than pattern-matched on the word 'timeout' — a loose match would start swallowing the real defects this tier exists to catch.
|
Claude finished @frankbria's task in 2m 31s —— View job GLM bug-hunting re-review — synchronize (
|
| prefix | emitted at | path into result.error |
startswith |
|---|---|---|---|
"Codex app-server timed out" |
codex.py:229 f"Codex app-server timed out responding to '{method}'" |
raises _ProtocolError → caught codex.py:196 → AgentResult(error=str(exc)) |
✓ |
"Stall timeout:" |
codex.py:345 f"Stall timeout: no events for {…}ms" |
direct AgentResult(status="failed", error=…) return from _stream_turn |
✓ |
"Turn timeout:" |
codex.py:351 f"Turn timeout: exceeded {…}ms" |
direct AgentResult(status="failed", error=…) return from _stream_turn |
✓ |
Cross-checked the surrounding logic for new defects:
- The stderr-append at
codex.py:204-205(result.error = f"{result.error}\nstderr: …") preserves the original string as the prefix, sostartswithstill matches after the append. ✓ - No overlap with
_UPSTREAM_FAULT_MARKERS(test_engine_smoke_tier.py:64-69); the timeout branch (:81) is checked first and is the more specific classification regardless. ✓ - Engine-agnostic prefix list is safe across the other three adapters: SubprocessAdapter emits
"Process timed out"(already listed);"Turn timeout:"/"Stall timeout:"are codex-specific JSON-RPC concepts no other adapter produces, and even a hypothetical collision would only produce a skip ("NO COVERAGE"), never a false pass. ✓ - No false-green introduced: none of the three is emitted in the [P0.19] Invoke
opencode runand require file changes for the OpenCode engine #913/[P0.20] Rewrite the Codex adapter against the real app-server protocol #914/[P0.29] Kilocode adapter invokes arunsubcommand that does not exist — the CLI opens the TUI and the task hangs #1012 false-completion shape — those produced a clean-looking exit having done no work (status="completed"/clean exit), not a timeout failure. A timeout skip is genuinely "ran out of time locally," never a pass.
The comment's cited line numbers (codex.py:229/345/351) are accurate.
| severity | file:line | finding |
|---|---|---|
| — | — | no new findings this commit |
Net: no new defects in d02f98f. The change correctly closes the prior minor finding, and the tier's no-false-green guarantee is intact.
1 participant
Closes #915.
Why this matters
Every external-engine adapter test patches
shutil.whichandsubprocess.Popenand replays the output the adapter's author expected. That is not a hygiene problem — it is the mechanism by which three of the four shipped engines went out with invocations that could not do any work, each behind a fully green suite:opencode --non-interactiveinitializekilo run <prompt>All three were found by pointing the real binary at the adapter's own
build_command. This tier makes that the standing check rather than a thing someone happens to try.Two legs, gated differently on purpose
Contract leg — the CLI exists, responds, and still documents the entry point the adapter depends on. Asserted against the CLI's own
--help, not against our code, so an upstream rename fails here instead of silently producing no-op runs. No credentials, no model calls, ~2s. Cheap enough to gate PRs.Task leg — one trivial task per engine driven through
adapter.run()to a terminal state with a real file written. Never "exit 0" and never "reached a terminal state" alone: every bug above produced a clean-looking exit having done nothing, so the file on disk is the only evidence that survives. Needs credentials, costs money, takes minutes — additionally gated onCODEFRAME_ENGINE_SMOKE=1(the opt-in established in #1012) and run on a schedule.Demo — all four real CLIs, locally
Contract leg, no credentials:
Task leg,
CODEFRAME_ENGINE_SMOKE=1, real model calls:Three engines drove a real task to completion and wrote real files. opencode's skip is honest, not convenient: its backend is returning
UnknownError / "Unexpected server error"with a server-side ref id, reproducible from a bareopencode runoutside the adapter, on both the positional and stdin transports. The adapter is fine; the provider is down.That distinction is the one place this tier could quietly rot, so it is deliberately narrow: a small list of the engines' own server-error envelopes, and the skip reports
NO COVERAGE — upstream provider faultrather than passing quietly. A broad "if it failed, skip it" would hide exactly what the tier exists to catch.Acceptance criteria
--help/version and drives one trivial task to a terminal state per enginetest_engine_smoke_tier.py, demoed above against all four installed CLIsgenerate-json-schemafixturefixtures/codex_app_server/+TestCodexSchemaContractfileno()-less fake_PipeStdoutoveros.pipe(); theselectorspath is gone entirely (reader thread + queue).github/workflows/engine-smoke.yml— contract leg on PRs, full tier weeklyThe workflow
The contract job always runs and filters internally rather than using
paths:. A path-filtered required check reports "skipped", which GitHub treats as never-satisfied — it would block every unrelated PR forever.Review
codex review --base main(cross-family, pre-PR) raised one [P2], and it was the sharpest possible critique: mynpm install … || echo "unavailable"meant a failed install left the binary absent, the tier skipped that engine, and pytest still exited 0 — a required check green while covering nothing. That is the exact failure mode this tier exists to end, reintroduced in its own workflow.Fixed in d03a81f: installs now fail the step, and an explicit verification step fails the job naming any missing CLI. Applied to the scheduled job too — a canary that silently stops canarying is worse than no canary.
Tests
tests/core/adapters/— 214 passed, 13 skipped (default, no credentials).ruff checkclean. Workflow YAML and everyrun:block parse-checked.Known limitations
main's protection currently requires onlyCode QualityandBackend Unit Tests. AddingEngine CLI Contractto the required contexts is a one-line settings change and is the last step of AC4.@anthropic-ai/claude-code,@openai/codex,opencode-ai,@kilocode/cli), but CI installs latest while my localkilois@kilocode/cli@0.22.0and the registry is at 7.4.17. If that major jump changed the CLI surface, the contract leg is what will say so — which is the point.