ci: make tests authoritative + wire Docker E2E (fix 形同虚设) - #1
Merged
Conversation
CI's `bun install --frozen-lockfile` was failing on every run because the repo tracked npm's package-lock.json instead of bun.lock. Bun found no bun.lock, migrated from the stale package-lock.json, detected drift vs package.json, and exited 1. This had been red for 5 consecutive pushes. Switch to bun.lock (the project is fully bun-based: CI uses setup-bun, all scripts use bun, runtime is bun). gitignore package-lock.json so a stray `npm install` can't reintroduce the drift. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
resolveBundledBin took a dev-repo shortcut (npm root -g) before checking AIO_PACKAGE_ROOT, so the env var was ignored when running from a git checkout. The test 'returns null when AIO_PACKAGE_ROOT points at an empty dir' relied on the override and was silently failing on dev machines + in CI — masked by ci.yml's `continue-on-error: true` on the test step (removed in the CI wiring change). AIO_PACKAGE_ROOT is documented as an override (preflight.ts:10); this makes the implementation match the docs. Explicit env now wins everywhere; the dev-repo fallback only runs when it's unset. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Two problems with the old ci.yml: 1. Toothless guards hid failures. `bun test` had `continue-on-error: true` and the CLI smoke had `|| true`, so CI was green-by-design even when tests failed. A unit test (preflight override) had been failing silently — now fixed, and the guard removed so it can't regress unnoticed again. 2. The 996-line Docker E2E (scripts/e2e-install.sh) never ran in CI — it was manual-only despite covering the full install/lifecycle/ migration/multi-daemon paths. Added an `e2e` job that installs opencode from the sst/opencode GitHub release (the runner has none; e2e-install.sh mounts the host binary into the container) and runs e2e-install.sh. Verified passing locally (all 8 phases green). Both jobs run on push + PR to master, in parallel. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The 'finds bundled ework-web' test deleted AIO_PACKAGE_ROOT and relied on the dev-repo fallback (npm root -g), which needs a global ework-aio install. CI has none (only local bun install), so the test returned null and failed — another failure hidden by the old continue-on-error guard. Point AIO_PACKAGE_ROOT at the checkout root instead: node_modules is populated by bun install in both local dev and CI. Deterministic. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
install.test.ts runs the real install flow, whose preflight requires opencode (REQUIRED_COMMANDS). The check job had none, so runInstall tests failed with 'Missing required commands on PATH: opencode' — also hidden by the old continue-on-error guard. Install opencode from the sst/opencode release, same as the e2e job. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The 'missing ework-web binary' test replaced PATH with a dev-machine-
specific string ('/home/dog/.local/bin:...'). CI's setup-bun installs bun
elsewhere, so the test failed with 'Missing required commands: bun'
instead of the expected 'ework-web not found'.
Filter the stub dir out of the real PATH instead — preserves bun/npm/
opencode wherever they live in dev or CI.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The CLI entry is bin/ework-aio (a bun shebang script), but ci.yml's 'Verify CLI entry' step and package.json's 'start' script both referenced bin/ework-aio.js — which doesn't exist. The CI step was masked by the old '|| true'. npm run start would also have failed if invoked. All three were buried under the toothless guards removed earlier. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit found the E2E Docker tests were 形同虚设 — the scripts are real (996-line `e2e-install.sh` with 8 phases of genuine assertions), but they never ran in CI, and the unit tests that did run had toothless guards that hid failures. CI was also 5× red and ignored.
Three fixes:
1. Fix red CI — lockfile drift (`52edbbb`)
`bun install --frozen-lockfile` failed every run: the repo tracked npm's `package-lock.json` instead of `bun.lock`. Bun migrated from the stale npm lockfile, detected drift vs `package.json`, exited 1. Switched to `bun.lock` (project is fully bun-based), gitignored `package-lock.json` to prevent recurrence.
2. Fix a silently-failing unit test (`08fa4c4`)
`resolveBundledBin` took a dev-repo shortcut (`npm root -g`) before checking `AIO_PACKAGE_ROOT`, so the env var was ignored from a git checkout. The test "returns null when AIO_PACKAGE_ROOT points at an empty dir" was failing on dev machines + CI — masked by `continue-on-error: true`. Made the explicit override win everywhere (matches the documented intent at `preflight.ts:10`). Test now passes (212/212).
3. Make CI authoritative + wire Docker E2E (`62e2076`)
Verification
The E2E scripts are not rotted — they pass end-to-end. They were 形同虚设 purely because nothing invoked them.
Files
```
bun.lock | new (lockfile CI actually uses)
package-lock.json | deleted (stale npm lockfile, source of drift)
.gitignore | + package-lock.json (prevent recurrence)
src/preflight.ts | resolveBundledBin override fix (8 lines)
.github/workflows/ci.yml | guards removed + e2e job added
```
Ultraworked with Sisyphus