BOHICA-LABS / BOHICA-LABS/vsdd-factory

Phase 3: Red Gate tests cannot land on main — CI rejects expected-red and idiomatic test-code lints

Open
#357 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2
Forks
1
Avg merge
6h 43m
Merged PRs (30d)
29

Description

## Symptom

VSDD per-story-delivery.md step (b) instructs test-writer to author failing tests for a wave's stubs, and step (c) instructs implementer to make them pass. The implicit assumption is that step (b)'s output lands on `main` as the executable spec (Red Gate per BC-5.38.001), then implementer turns red → green.

In practice, the plugin pack's shipped CI workflow makes this impossible:

1. **`cargo test --workspace` exit-101 = CI failure.** No envelope distinguishes "expected red per BC-5.38.001" from "regression red". Every test-only PR fails the Test check by design.
2. **`-D warnings` denies test-code idioms.** Shipped clippy config denies `clippy::unwrap_used`, `clippy::expect_used`, `clippy::doc_markdown` (transitively via `-D warnings`). These are standard, idiomatic in test code (`result.expect("test setup")` etc).
3. **Required-checks branch protection blocks merge** until both Test and Clippy pass, which they never will for a tests-only PR.

## Reproduction (concrete, from ArcavenAE/akey)

- Phase 3 Wave 1 stubs merged via PR #8 (todo!() bodies).
- test-writer authored 70 tests across 3 stories per spec (BC-traced names, etc).
- PR #11 opened with the test commit on top of post-PR-#8 main.
- CI: Format check ✓ / Build ✓ / cargo-deny ✓ / Harden-Runner ✓ / **Test ✗ (59 todo!() panics) / Clippy ✗ (30+ lint denials in test code)**.
- `mergeStateStatus: BLOCKED`.

## Forced workaround

The orchestrator must collapse steps (b) + (c) per story into one PR (tests + impl land together, branch goes red → green internally, never visits main red). This contradicts per-story-delivery.md's stated step ordering and removes the audit trail of "the tests existed before any implementation" that Red Gate is meant to provide.

## Root-cause hypotheses (engine should pick)

- **Workflow gating:** ci.yml should partition checks by stage. Test runs that include known-red BCs should not block merge; an explicit "expected-red" allowlist (e.g., a YAML in `.factory/code-delivery/wave-N-expected-red.yaml` consumed by the test job) lets the CI distinguish.
- **Clippy scope:** the shipped `clippy.toml` / workflow should NOT apply `unwrap_used`/`expect_used` to `#[cfg(test)]` code and `tests/` integration files. These are idiomatic; denying them in tests forces ceremonial `#[allow]` items on every test file.
- **doc_markdown on test files:** test docstrings (`/// BC-x.y.z: ...`) trip `doc_markdown` because of bare identifiers; the lint should be allow-by-default in test contexts.
- **per-story-delivery.md ambiguity:** the skill should explicitly state whether step (b)'s output is supposed to land on main separately or whether it always rides with step (c). The wording today ambiguous; orchestrators currently guess.

## Suggested fix shape

A combination of:
1. Plugin pack updates clippy lint config to scope test-code-idiomatic lints to non-test code only (`avoid-breaking-exported-api = false` + per-crate `#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used, clippy::doc_markdown))]` in the lib root, OR a workflow-side `--no-deps -- -D warnings -A clippy::unwrap_used` style invocation that respects test context).
2. ci.yml "test" job gains a "phase-3-expected-red" mode (env-gated or branch-name-gated) that converts panics-from-BC-5.38.001 into pass while preserving real-error visibility.
3. per-story-delivery.md updated to state the canonical ordering explicitly. Either:
- **Option A:** Step (b) lands on a per-story branch directly; implementer commits to the same branch in step (c); single per-story PR. This is what akey is now forced into; document it as canonical.
- **Option B:** Step (b) lands on main behind an "expected red" CI envelope (requires the workflow changes above). Preserves audit trail of "tests existed first".

## Severity

**P1** — blocks every Phase 3 wave for every factory-bootstrapped project the moment test-writer runs. Forced workaround works but degrades the per-story-delivery skill's auditability claim.

## Refs

- ArcavenAE/akey PR #11 (closed — this defect)
- ArcavenAE/akey PR #10 (closed — stale base, unrelated)
- ArcavenAE/akey PR #8 (Phase 3 Wave 1 stubs merged `c8f1ddd`)
- vsdd-factory#349 (related: CI/protection name mismatch)
- vsdd-factory#351 (related: pack CI never green-tested against empty product)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.