feat: add M5 test runner backend

[gnanam1990]

Summary

• Adds the M5 Zero test-runner backend for detecting workspace checks across Go, package-manager scripts, pytest, and Cargo projects.
• Parses runner output into structured test summaries with framework, pass/fail/skip counts, and failure details.
• Integrates those summaries into zero verify JSON and text output, with nested redaction for output summaries and failure details.

Validation

• git diff --check origin/main...HEAD
• go test -count=1 -p 1 ./...
• bun install --frozen-lockfile
• bun run typecheck
• bun test ./tests --timeout 15000 (217 pass, 0 fail)
• bun run build
• bun run smoke:build
• bun run smoke:go
• ./zero verify --json --only go.test --timeout-ms 120000 | rg '"testSummary"|"framework"|"total"|"passed"|"failed"' -n

Reviewers: @Vasanthdev2004 @anandh8x

Summary by CodeRabbit

• New Features
  • Verify now detects and parses test summaries across multiple ecosystems (Go, Node, Python, Rust, Bun) and shows structured totals and failure details in output.
• Bug Fixes
  • Redaction of sensitive workspace output improved and no longer mutates original results.
• Tests
  • Expanded test coverage for detection, parsing across runners, stable ordering, and redaction behavior.

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Typecheck: bun run typecheck
• [pass] Tests: bun run test
• [pass] Build: bun run build
• [pass] Smoke build: bun run smoke:build

Scope

Head: e12651711ea4
Changed files (6): internal/cli/workflow_test.go, internal/cli/workflows.go, internal/testrunner/testrunner.go, internal/testrunner/testrunner_test.go, internal/verify/verify.go, internal/verify/verify_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b72c9934-dac2-4c0f-8901-e5a54aaa9caa

📥 Commits

Reviewing files that changed from the base of the PR and between 491099f and e126517.

📒 Files selected for processing (3)

• internal/testrunner/testrunner.go
• internal/testrunner/testrunner_test.go
• internal/verify/verify_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/verify/verify_test.go
• internal/testrunner/testrunner_test.go
• internal/testrunner/testrunner.go

---

Walkthrough

Adds a new internal/testrunner package for workspace check detection and parsing (Go, Bun, Node, pytest, Cargo); integrates testrunner into internal/verify (Check metadata and conditional TestSummary parsing); updates CLI formatting and redaction to render and sanitize parsed test summaries; and adds tests for detection, parsing, integration, and redaction.

Changes

Test discovery and parsing

Layer / File(s)	Summary
Testrunner package and detection internal/testrunner/testrunner.go	Adds Kind/Framework enums, Check/Summary/Failure types, filesystem/package.json helpers, and Detect to find workspace checks across Go, Node/Bun, pytest, and Cargo.
Framework-specific output parsing internal/testrunner/testrunner.go	Implements ParseSummary and parsers for Go (with failure location association), Bun, Node TAP, pytest, and Cargo; includes normalization and utilities to merge textual counts into structured totals.
Testrunner tests internal/testrunner/testrunner_test.go	Tests Detect ordering and package-manager selection; tests ParseSummary behavior for Go (verbose/non-verbose/package-status) and common runner outputs (bun, node tap, pytest, cargo).

Verify system integration

Layer / File(s)	Summary
Verify model updates internal/verify/verify.go	Adds Kind and Framework to Check and TestSummary *testrunner.Summary to Result.
Plan detection and parsing wiring internal/verify/verify.go	Replaces bespoke detection with testrunner.Detect, adds shouldParseTestSummary heuristic and conditions ParseSummary to test-like checks, and removes legacy helpers.
Verify tests internal/verify/verify_test.go	Updates plan assertions to validate check metadata, asserts parsed TestSummary presence/redaction for test checks, and adds a test ensuring structured parsing is skipped for non-test checks.

CLI output formatting and redaction

Layer / File(s)	Summary
Output redaction and formatting internal/cli/workflows.go	redactVerifyReport now clones and deep-copies results before redacting nested OutputSummary and TestSummary; formatVerifyReport renders per-check test totals and failure lines via formatVerifyTestSummary.
CLI workflow tests internal/cli/workflow_test.go	Tests updated to include structured TestSummary in mocked results, tighten text-output assertions to include totals, and assert redaction markers appear in structured failure fields.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Gitlawb/zero#70: Prior work on verify-report detection and CLI formatting that this PR extends with structured TestSummary handling.
• Gitlawb/zero#72: Overlaps in verify loop/output summarization and CLI redaction/formatting changes.

Suggested reviewers

• anandh8x
• Vasanthdev2004

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add M5 test runner backend' directly and clearly summarizes the main change—introducing a new test runner backend for the M5 ecosystem with the core new module internal/testrunner.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/m5-test-runner-backend

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/testrunner/testrunner.go`:
- Around line 193-206: The detectPackageManager function currently defaults to
Bun which causes projects without lockfiles or a packageManager field to be
treated as bun; change the default return in detectPackageManager to
packageManager{Name: "npm", IDPrefix: "npm", Framework: FrameworkNode} so plain
Node workspaces fall back to npm (keep the existing declaredName handling and
other cases intact). Ensure the packageManager struct construction and Framework
references (FrameworkNode) are used in the default branch instead of Bun.
- Around line 224-260: parseGoSummary currently sets sawDetailedTest when
goFailPattern matches, which causes package-only "ok ..." lines to be ignored in
mixed verbose/non-verbose runs; update parseGoSummary so that encountering
goFailPattern does not alone set sawDetailedTest (or only sets it when a real
detailed test line is seen), and ensure goPackageOK matches (using goPackageOK)
still increments packagePasses and that summary.Passed is augmented by
packagePasses when detailed tests were not seen; modify the logic around the
goFailPattern case and the sawDetailedTest flag handling in parseGoSummary to
preserve counting of ok packages while still recording failures into
summary.Failures.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 681bf6d4-0c40-48c3-82aa-31f25a0371d3

📥 Commits

Reviewing files that changed from the base of the PR and between 4eacb5b and 491099f.

📒 Files selected for processing (6)

• internal/cli/workflow_test.go
• internal/cli/workflows.go
• internal/testrunner/testrunner.go
• internal/testrunner/testrunner_test.go
• internal/verify/verify.go
• internal/verify/verify_test.go

[anandh8x]

What's good

• internal/testrunner is the right shared home for detection and parsing. Detect returns checks in a stable, deterministic order (Go, then per-script package checks, then pytest, then cargo) and ParseSummary dispatches by framework. Pulling this out of internal/verify removes the Bun-only hardcoding in DetectPlan and unblocks pytest/cargo for zero verify.
• Framework coverage is pragmatic and reasonably tested. parseGoSummary handles verbose --- PASS/FAIL/SKIP, package-only ok, the mixed non-verbose+verbose case, and correctly avoids double-counting; parseBunSummary, parseNodeSummary, parsePytestSummary, and parseCargoSummary all sit on a shared mergeSummaryCounts helper so plain "N pass / N fail / N skip" summary lines contribute. Failure location extraction for Go uses the next line when present, plus a back-fill pass for the case where the location appears after a blank.
• Redaction order is now correct. verify.Run applies redaction.RedactString to stdout/stderr before ParseSummary sees them, so test summary counts and failure messages (name, file, message) operate on redacted content. redactVerifyReport in the CLI now copies Results first and copies nested slices (OutputSummary.Lines, TestSummary.Failures) before mutating, so a JSON report passed in is not mutated in place — fixes the in-place mutation concern from #59.
• CLI formatting shows the most useful information without being noisy. The new formatVerifyTestSummary produces a single "tests: N total, N passed, N failed" line (skip count when non-zero) and follows with per-failure failure: Name at File lines only when a name is present. JSON output gets the structured testSummary block.
• Detection defaults to npm for plain Node workspaces. detectPackageManager now falls through to npm/FrameworkNode when no lockfile and no declared manager are present; the prior Bun default is gone, and TestDetectDefaultsPlainNodeWorkspacesToNPM locks that behavior in.
• shouldParseTestSummary keeps parse-on-test intact even for legacy plans. New plans get a populated Kind, but for plans constructed by older code (or external callers) the kind fallbacks (id contains .test/pytest, or command contains test/pytest) are conservative and only match real test scripts.
• Defensive copies are consistent. Plan.Checks is built via make(..., 0, len(detected)) and each Command slice is copied with append([]string{}, check.Command...) before being passed to the runner and to ParseSummary. The testrunner Summary and Failure slices are also handled by reference-only, with redaction handled in the CLI layer.

Observations (non-blocking)

1. shouldParseTestSummary id-substring heuristic can false-positive. It checks strings.Contains(id, ".test") || strings.HasSuffix(id, "test") || strings.Contains(id, "pytest") and any command part equal to test or pytest. In current code Detect sets Kind explicitly, so this only matters for external callers. Still, ids like bun.pretest or a hypothetical e2e.attest script would be treated as a test. A Kind != "" short-circuit already short-circuits to true for KindTest; a Kind != "" short-circuit to false for other kinds is the missing twin. Right now a Kind == KindTypecheck check still falls through to the id heuristic — fine, but worth noting.

2. parseGoSummary second pass has a dead guard. if location == nil || index == 0 { continue } — index == 0 cannot occur because the first pass already established that lines is non-empty only when the input had content, and len(lines) > 0 is guaranteed. The guard is harmless but reads like it was guarding against a case that is no longer possible after splitLines filters empty lines.

3. parseNodeSummary has no Skipped tracking in the TAP path. ok N - name increments Passed, not ok N - name increments Failed and records the name, but # skip N summary lines are the only way skips enter via mergeSummaryCounts. A TAP test that prints # SKIP name (TAP "directives") is not captured, only aggregate counts are.

4. mergeSummaryCounts uses maxInt so later, larger counts win. That is the right choice for repeating summary lines, but it means a parser that prints both an old stale line and a new accurate one will keep the larger value. In practice summary lines appear once at the end, so this is fine; worth a comment in the helper.

5. Framework summary regex is greedy on whitespace boundaries. summaryCountPrefix matches (\d+)\s+(pass|passes|...); for an input like panic: 1 failed the 1 and failed are two tokens and the regex needs the \s+ between them — which exists, so it matches. That is correct, but the same pattern will also match a line like fatal: saw 0 passed assertions in some other tool's output. Not a real problem; just noting the parser is permissive on purpose.

6. redactVerifyReport walks the copied Results by index rather than range. Using for index := range report.Results and then report.Results[index] is fine because the loop is read-only, but a for _, result := range report.Results plus a small cloneResult helper would read more cleanly. Cosmetic.

7. No stdout/stderr size cap before ParseSummary. A pathological runner that emits megabytes of test output still gets fully redacted and parsed on the main goroutine. For local zero verify this is acceptable, but a future follow-up could cap the buffer fed to ParseSummary (e.g., last 1 MB) so a runaway test cannot stall the agent loop. The existing OutputSummary already truncates via summarizeOutput for display.

8. Test TestRunParsesStructuredFailureSummary redaction assertion is on the formatted text, not the JSON TestSummary. The new if got := report.Results[0].TestSummary.Failures[0]; !strings.Contains(got.Message, "[REDACTED]") assertion does cover the JSON, which is good. Worth a follow-up that the message stored on the TestSummary is itself redaction-safe even if a caller decides to re-redact via a different policy — currently it relies on RedactString being applied at the source.

9. Cargo test names with spaces are split by the regex. cargoTestLine matches test (.+) \.\.\. (ok|FAILED|ignored)$, so test runs::zero test ... ok becomes Name: "runs::zero test", which is fine; just noting that names can include spaces and that gets recorded verbatim into the Failure.Name and through the CLI line.

10. Pytest Message uses everything after -. pytestFailureLine ^FAILED\s+(\S+)(?:\s+-\s*(.*))?$ records the dash and tail as Message. For a FAILED tests/test_cli.py::test_stream - AssertionError: boom it stores AssertionError: boom, which is what you want, but the leading - is stripped by safeSubmatch's TrimSpace — the downstream Failure.Message will be AssertionError: boom (good).

11. parseGoSummary Total from summary.Total++ only on === RUN lines. For a non-verbose run that still records package status with go test -v disabled, summary.Total stays at 0 until normalizeTotals sets it to Passed + Failed + Skipped. That is correct, but means summary.Total reflects distinct pass/fail/skip sources; if a verbose run mixed with ok lines, summary.Total includes RUNs and summary.Passed is incremented per PASS plus package passes, which already-matches-a-verbose-test would not double-count because sawVerbosePerTestOutput is true. This is the intended invariant, but a regression test for Total == Passed + Failed in mixed scenarios is the one that would catch any future change.

12. No parallel test for TestRunSkipsStructuredSummaryForNonTestChecks via the JSON path. The test asserts report.Results[0].TestSummary == nil for a typecheck check, which is the right guard. A follow-up that feeds typecheck output with "tests:" in it (e.g., a tsc error that says "Failed 0 tests") would currently be ignored as expected. No action needed; just noting the shouldParseTestSummary kind-first rule is the right defense.

No blockers. The slice is well-scoped, tests cover the prior CodeRabbit nits (npm default, mixed non-verbose Go counting), the integration with zero verify is clean, and the nested redaction in redactVerifyReport is now mutation-safe. The two follow-ups I would prioritize next: (1) the shouldParseTestSummary id heuristic tightening once Kind is required for plans, and (2) a buffer cap before ParseSummary for long-running test output.