Summary
The tests check shipped in the AIR / SPIR / ASPIR protocol skeletons hardcodes:
npm test -- --exclude='**/e2e/**'
This assumes npm test forwards extra args straight to the test runner (e.g. vitest,
where --exclude is a valid flag). That assumption breaks in any project whose test
script is a wrapper — most notably a Turborepo monorepo where "test": "turbo test".
There, the single -- only forwards the flag across the first boundary (npm → turbo),
and turbo grabs --exclude as its own argument and rejects it before any test runs:
ERROR unexpected argument '--exclude' found
tip: to pass '--exclude' as a value, use '-- --exclude'
Usage: turbo [OPTIONS] [TASKS]... [-- <PASS_THROUGH_ARGS>...]
The check fails in ~0.1s (nothing executed), so porch check, porch approve <id> pr,
and porch done <id> all report tests ✗ even though the tests pass. It's an
argument-forwarding failure, not a test failure.
Affected
@cluesmith/codev 3.2.2
skeleton/protocols/air/protocol.json
skeleton/protocols/spir/protocol.json
skeleton/protocols/aspir/protocol.json
(all three carry the identical "command": "npm test -- --exclude='**/e2e/**'")
Reproduction
- Any project whose root
test script wraps the runner in another CLI, e.g. Turborepo:
package.json → "test": "turbo test".
- Run an AIR/SPIR/ASPIR project through to the build/PR checks (or just
porch check <id>).
- The
tests check errors with unexpected argument '--exclude' found; the underlying
unit tests pass fine when run directly (pnpm --filter <pkg> test / vitest run).
Root cause
The flag must cross two CLI boundaries to reach the runner in a wrapped setup
(npm → turbo → vitest), but only one -- is supplied, so it stops at the wrapper. The
command is written for the unwrapped case (test = the runner itself) and silently
mis-targets otherwise.
Impact
- False-negative gate: blocks
porch approve pr / porch done, so a correct,
green PR can't be advanced without a manual workaround (porch done <id> --merged <PR#>).
- Worse for autonomous protocols (AIR/ASPIR): the whole point is unattended
implement-and-review; a spurious red check undermines the gate's trust signal.
- The
**/e2e/** exclusion is also a no-op for the (common) case of a project with no
e2e/ tests — so the check adds risk with no benefit there.
Proposed fix (keep it build-tool-agnostic)
The core issue is the skeleton assuming a specific arg-passthrough shape. Please avoid
swapping one assumption (plain npm test) for another (e.g. hardcoding
turbo test -- --exclude=…) — projects may or may not use turbo. Options, roughly in
order of preference:
- Drop the
--exclude from the default and make the skeleton tests check a plain
npm test. Projects that genuinely need to exclude e2e can override the check command
via their own codev/protocols/<name>/protocol.json (tier-2). This removes the
passthrough assumption entirely and works for every project out of the box.
- Make the test command a first-class, documented config value (e.g. in
.codev/config.json) with npm test as the default, so wrapper-based repos can point
it at the right invocation without shadowing the whole protocol file.
- If an exclusion default must stay, at least document the wrapper caveat in the
skeleton and the check-failure output (surface the "your test script may be a
wrapper; the flag needs a second --" hint).
Workarounds (for anyone hitting this now)
- Verify tests directly:
pnpm --filter <pkg> test (or vitest run).
- Record a merged PR without the check-gated advance:
porch done <id> --merged <PR#>.
- Or add a tier-2 override at
codev/protocols/<name>/protocol.json with a
wrapper-correct command (e.g. turbo test -- --exclude='**/e2e/**').
Summary
The
testscheck shipped in the AIR / SPIR / ASPIR protocol skeletons hardcodes:This assumes
npm testforwards extra args straight to the test runner (e.g. vitest,where
--excludeis a valid flag). That assumption breaks in any project whosetestscript is a wrapper — most notably a Turborepo monorepo where
"test": "turbo test".There, the single
--only forwards the flag across the first boundary (npm → turbo),and turbo grabs
--excludeas its own argument and rejects it before any test runs:The check fails in ~0.1s (nothing executed), so
porch check,porch approve <id> pr,and
porch done <id>all report tests ✗ even though the tests pass. It's anargument-forwarding failure, not a test failure.
Affected
@cluesmith/codev3.2.2skeleton/protocols/air/protocol.jsonskeleton/protocols/spir/protocol.jsonskeleton/protocols/aspir/protocol.json(all three carry the identical
"command": "npm test -- --exclude='**/e2e/**'")Reproduction
testscript wraps the runner in another CLI, e.g. Turborepo:package.json→"test": "turbo test".porch check <id>).testscheck errors withunexpected argument '--exclude' found; the underlyingunit tests pass fine when run directly (
pnpm --filter <pkg> test/vitest run).Root cause
The flag must cross two CLI boundaries to reach the runner in a wrapped setup
(
npm → turbo → vitest), but only one--is supplied, so it stops at the wrapper. Thecommand is written for the unwrapped case (
test= the runner itself) and silentlymis-targets otherwise.
Impact
porch approve pr/porch done, so a correct,green PR can't be advanced without a manual workaround (
porch done <id> --merged <PR#>).implement-and-review; a spurious red check undermines the gate's trust signal.
**/e2e/**exclusion is also a no-op for the (common) case of a project with noe2e/tests — so the check adds risk with no benefit there.Proposed fix (keep it build-tool-agnostic)
The core issue is the skeleton assuming a specific arg-passthrough shape. Please avoid
swapping one assumption (plain
npm test) for another (e.g. hardcodingturbo test -- --exclude=…) — projects may or may not use turbo. Options, roughly inorder of preference:
--excludefrom the default and make the skeletontestscheck a plainnpm test. Projects that genuinely need to exclude e2e can override the check commandvia their own
codev/protocols/<name>/protocol.json(tier-2). This removes thepassthrough assumption entirely and works for every project out of the box.
.codev/config.json) withnpm testas the default, so wrapper-based repos can pointit at the right invocation without shadowing the whole protocol file.
skeleton and the check-failure output (surface the "your
testscript may be awrapper; the flag needs a second
--" hint).Workarounds (for anyone hitting this now)
pnpm --filter <pkg> test(orvitest run).porch done <id> --merged <PR#>.codev/protocols/<name>/protocol.jsonwith awrapper-correct command (e.g.
turbo test -- --exclude='**/e2e/**').