feat(#63): test-generation layer

[sugat009]

Summary

Implements the test-generation layer (#63). After code generation, the development supervisor generates tests for the changed code through a dedicated layer, persists and displays them alongside the code, and is non-fatal on failure.

Based off 62-integrate-code-gen-layer and targets it, not main, since 62 is not yet merged.

What it adds

• A TestGenerationResult type and test-generation phase, and a generateTests node wired after validateImpl, outside the refinement loop.
• A TestGenerationAgent wrapping a test-gen module registry, with a buildTestGenModuleInput adapter mirroring code-gen.
• The claude-api test-gen module producing test files in-memory; output flows through staging and the HC2 approval gate.
• A semantic-correctness gate (planted-bug fixtures) that spawn-executes an emitted spec to prove it fails on buggy source and passes on fixed source.
• Removal of the legacy test-environment agent.

Design notes for reviewers

• Tool use is gated on a provider capability, not the env. LLMProvider carries honorsCustomTools (Anthropic true, claude-cli false). Test-gen Phase-2 passes disableTools on providers that do not honor custom tools, so the CLI returns capturable text and its --disallowedTools deny-list applies. This keeps the CLI from running its own tool loop and writing outside the approval gate; the API path keeps tools.
• Generation is contained on the CLI path. TestGenerationAgent.generate snapshots the cht-core tree and rolls back any out-of-band write, capturing the in-memory result before rollback and degrading safely when the path is not a git repo. The API path writes nothing to disk and is not wrapped.
• The content gate parses the emitted spec. A syntactically-invalid emit (e.g. a leaked reasoning preamble) is rejected and fed back into the regenerate loop, not just checked for the presence of describe/it/require.
• Test-gen output never feeds the validation score or the refinement loop, and a test-gen failure is non-fatal to the run.

Known limitation

A generated test can over-reach into untouched pre-existing code with a wrong assumption (e.g. an assertion about compare() semver semantics outside a ticket's scope). This is inherent LLM-output-quality risk; the durable mitigation (running the emitted test and feeding failures back) is queued behind the dual-path claude-code-cli test-gen module. Generated tests should be human-reviewed before merge.

Gates

npm run build, npm run lint, npm test (718 passing) all green; coverage statements 84.39% (floors 78/60/72/78). SonarCloud Quality Gate runs on this PR.

[Hareet]

Reviewed on 6-27-26. If you want to address H1-H3, otherwise let's get it in there and iterate on it

Used:

• comprehensive-review:code-reviewer agent — full quality/correctness pass over all 12 source files.
• comprehensive-review:security-auditor agent — tool-gating seam, file writes, traversal, prompt injection.
• Ultracode workflow — 7 finder dimensions, every finding adversarially verified (38 confirmed, 3 refuted/invalid out of 41 raised; many claimed severities were downgraded on verification).
• My own read of the highest-risk files (content gate, tool-gating types, core module parse/retry, graph wiring).

Solid, defensively-engineered PR. No blockers. Recommend: address the High items, then merge.

The architecture is right and matches both the design intent and the #115 recommendation doc:

• Separate layer from code-gen (src/layers/test-gen/), running as a terminal LangGraph node (generateTests) reached only off validateImpl's terminal branch, outside the refinement loop — so test output cannot influence the validation score. ✅ Verified.
• Non-fatal contract — test-gen failures never halt the run or corrupt validation metrics.
• Content/parse gate that does not execute generated code at runtime (ts.createSourceFile parses; no eval/vm/spawn). ✅ Verified clean by the security track.
• Tool-gating seam (LLMProvider.honorsCustomTools) forces text-only output on the CLI provider.
• Genuinely good test coverage — ~1,900 test lines for ~3,000 source lines, including the tool-gating seam spec, the semantic-correctness gate (spawns real mocha against planted-bug fixtures), and containment/rollback tests.

The recurring weaknesses are concentrated in two places: (a) the content gate is weaker than it looks — several checks are near no-ops or trivially satisfiable, so "passed the gate" overstates quality; and (b) several validation results are computed and then discarded (Zod schemas run but their verdicts are ignored), making the type-safety guarantees partly decorative. Plus a cluster of observability gaps where failures look identical to success to the human reviewer, and duplication of the code-gen engine that should largely be shared.

High — address before merge

H1 🔁 — claude-cli test-gen alias resolves to a module that is never registered

src/layers/test-gen/registry.ts (PROVIDER_ALIAS_MAP + createDefaultTestGenRegistry)
'claude-cli' → 'claude-code-cli', but the default registry only registers claude-api. So getActiveModule('claude-cli') / TEST_GEN_MODULE=claude-cli throws. The registry spec masks this by manually registering a claude-code-cli module the default registry never has. claude-cli is a first-class provider in this project, so a CLI user hits a hard throw (caught by the non-fatal wrapper → silently zero test files).
Fix: point the alias at claude-api (which runs correctly under a CLI LLM provider via the containment path), or drop the alias until a real claude-code-cli test-gen module exists.

H2 🔁 — hasAssertions is a near no-op; the content gate certifies empty shells as valid

src/layers/test-gen/schemas.ts:38-45, 65-74 (and validateTestFile, 141-149)
hasAssertions tests against TEST_STRUCTURE_PATTERNS, which includes /describe\s*\(/ and /it\s*\(/. So any file with a describe/it block "has assertions" — even with zero expect/assert. Combined: a syntactically-valid empty-shell stub (describe('x', () => { it('works', () => {}); })) passes the entire gate (parsesAsCode ✓, isNotPlaintext ✓, hasProperImports ✓ if any import line exists, hasTestStructure ✓, hasAssertions ✓). The gate's name promises more than it delivers — "passed" ≠ "is a real test." the content-gate track (verifier kept this at high).
Fix: make hasAssertions use an assertion-only pattern set (expect|assert|should|chai|sinon.assert), excluding describe/it. Optionally require ≥1 assertion inside an it body. This is the single highest-leverage gate fix.

H3 🔁 — LLM-controlled path reaches the filesystem with no traversal guard

src/layers/test-gen/modules/claude-api/index.ts:77 (+84-94) (tool handler) → src/utils/staging.ts:349-355,360-375 (readFromChtCore/listChtCoreDirectory); write side via staging.ts:132,156
The tool handler casts toolInput.path as string (:77) and passes it straight to path.join(chtCorePath, relativePath) with no normalization or containment check. sanitizePath (code-gen/lib/plan.ts:15-17) only strips backticks and trims — it does not reject ../, absolute, ~, or symlink paths despite its name. A prompt-injected source file under test could steer a read_file call to ../../../../etc/passwd, or a plan filePath could escape the tree on write.
Re-verify found it worse than first documented: (1) the tool handler at :77 doesn't even call sanitizePath — the raw cast string flows directly to readFile/listDirectory (sanitizePath is only applied to plan-parsed paths at :319-320, and wouldn't help anyway). So the LLM read/list primitive has zero path filtering. (2) That read primitive fires during generate() — before the HC2 checkpoint — and in Anthropic mode (honorsCustomTools=true), so arbitrary file read is reachable pre-approval. ../ segments fully escape; a leading / is contained by path.join semantics.
Bounding context: the module doesn't write at runtime; writes are HC2-gated and the diff shows the raw ../ path. The read side, however, is not HC2-gated. Shared with code-gen, but the PR widens the surface with a second independently-reachable instance.
Fix: add const full = path.resolve(base, rel); if (full !== base && !full.startsWith(base + path.sep)) reject; in the staging read/write helpers; reject absolute/.. paths; guard the as string cast (typeof toolInput.path !== 'string' currently flows undefined into path.join → unhandled TypeError).

---

Medium

M1 🔁 — Several Zod validations are computed and then discarded ("validate-and-ignore")

• index.ts:298-305 (generateTestPlan) — TestPlanSchema.safeParse runs, logs issues on failure, then returns the raw unvalidated plan anyway. The filePath regex (.spec.*/.test.*) and description ≥ 10 invariants don't gate anything; N LLM calls get spent on items the schema flagged.
• index.ts:695-700 (parseRequirementsChecklist) — on safeParse failure it falls through to return parsed.checklist (raw, unvalidated, cast to TestScenario[]); the catch swallows JSON errors with a bare // Fall through. Malformed scenarios (missing name/scenarios, bad type enum) flow to the human display (development-workflow.ts:102 does s.scenarios.map(...) → can TypeError on a fallback item missing scenarios).
  Fix: make schemas authoritative — filter the plan to items passing TestPlanItemSchema; on checklist safeParse failure log and return [] (it's cosmetic/non-fatal, so strictness loses nothing). At minimum, surface the dropped items on output.warnings (the field exists and is displayed) so they're visible at HC2 rather than console-only.

M2 🔁 — Non-fatal test-gen failure is reported to the human as a successful 0-file run

development-supervisor.ts:497-501 (catch in runTestGenWithFallback) + :509-516 (finishTestGeneration) + development-workflow.ts:83-108
On a thrown generation error the catch (:497-501) logs a console.warn and routes through finishTestGeneration, which unconditionally todos.complete() + printSummary() (:513-515, designed to read "3/3"). The failure path never calls todos.fail() — contrast the validation node at :393-395. So "generation crashed," "all files dropped by the gate," and "no tests needed" are indistinguishable to the reviewer — a uniformly green summary (printSummary counts only failed-status todos in its "N failed" column). Re-verify note: even legitimate no-op skips (shutdown :458, missing data :465, no code :469) also route to complete(), not just the crash path. Harm is bounded (observability, not outcome — the human still reviews real code diffs), but a reviewer may approve unaware tests were silently skipped.
Fix: on the failure catch, populate result.warnings (e.g. "test generation failed: <msg>") and call todos.fail(); reserve complete() for genuine success / intentional no-op. (Note: TodoStatus has no skipped state — pending|in_progress|completed|failed — so distinguishing "skipped" needs an enum addition or reusing fail with a benign message.)

M3 🔁 — Continuation/truncation handling has three real seam bugs

The truncated-file → continuation path (index.ts:479-500, 612-664) is the most complex logic in the PR and is entirely untested. Three confirmed issues:

• Continuation chunks are re-run through stripReasoningPreamble (index.ts:660 → output-parsing.ts:206). A mid-it('...') resumption reads as prose on line 1, so the preamble-stripper deletes the first line of real continuation code, leaving an unterminated string. Verifier reproduced it; downgraded high→medium because CHT test titles are overwhelmingly lowercase (714 lowercase vs 12 capitalized it( in the repo) and the corruption usually breaks parsing → caught → expensive regenerate rather than shipped.
• A failed continuation API call is treated as a successful (still-truncated) file (index.ts:627-635, 612-616). runOneContinuation returns false on a thrown invoke without setting acc.stopReason, so stillTruncated = (undefined === 'max_tokens') = false → overBudget:false → the original truncated file is returned as success. parsesAsCode catches many truncations, but not all.
• A truncated markdown-fenced first response leaves an unstrippable opening ``` fence in the assembled file (index.ts:541,547).
  Fix: skip stripReasoningPreamble on continuation chunks (fence-only strip, or an isContinuation flag); propagate continuation failure as over-budget / file:null; and add tests for the continuation path (the biggest coverage gap). At minimum attach a "machine-stitched, review me" warning when continuations were used.

M4 🔁 — honorsCustomTools conflates two orthogonal concerns, and the two claude-api modules gate tools differently

src/llm/types.ts:138, consumed at index.ts:567 and test-generation-agent.ts:168
The single boolean drives both "does the provider run the custom tool loop" and "does this path need filesystem containment." These are independent; a future provider that honors tools but still needs containment (or vice-versa) can't be expressed. Separately, the test-gen and code-gen claude-api modules consume the flag with divergent strategies (test-gen index.ts:567-569 vs code-gen index.ts:678) — a latent regression trap where fixing one won't fix the other. Verifier kept the conflation at medium, downgraded the divergence to low.
Fix: split into honorsCustomTools + a separate requiresContainment (or derive containment from config.mode === 'cli'). Factor the shared gating into one helper both modules call.

M5 🔁 — extractCodeContent is dead production code kept alive only by its own test

index.ts:959-986 — the JSDoc admits it exists so its spec keeps passing; the real path uses parseSingleFileContent. It's a second, divergent parser (no preamble protection) on the public surface, inviting drift/accidental use.
Fix: delete it and its describe('extractCodeContent') block, or make it delegate to libParseSingleFileContent. A test should track real behavior, not pin an inferior duplicate.

M6 🔁 — Dead input surface: existingTestExamples + additionalContext read but never populated

test-generation-agent.ts:61-72 (buildTestGenModuleInput) vs index.ts:710,717-723,754,837
existingTestExamples ("existing test patterns for style matching") is read in the plan prompt and buildPatternContext (:837) but never set by buildTestGenModuleInput. So the contained/CLI path (tools disabled) has no example-pattern context — exactly the path that most needs style anchoring. The spec feeds it by hand, hiding the dead-at-runtime status. Re-verify correction: it's 2 read-but-unpopulated fields, not 3 — existingTestExamples and additionalContext (:754, iteration feedback). additionalContext is doubly lost: the builder routes feedback into contextFiles, which no test-gen module reads. (directoryListing at interface.ts:25 is dead-declared — only code-gen reads it.)
Fix: populate existingTestExamples (glob a few *.spec.* near the target via the staging helpers); wire additionalContext from the feedback the builder currently drops into contextFiles; or remove the unused fields/branches.

M7 — Parse gate catches syntax only, not types/semantics (false confidence)

schemas.ts:126-139 — parsesAsCode reads sourceFile.parseDiagnostics, which catches syntax errors but not type errors, unresolved imports, or wrong API usage (there's no tsc in the runtime path — by design). The doc comment is honest about catching leaked prose, but "passed the gate" can still mean a test that won't compile against cht-core. Pairs with H2.
Fix: document the gate's limit explicitly at the call site; longer-term this is what the deferred compile/coverage gate (per the #115 doc) is for.

M8 — parseRequirementsChecklist greedy {...} regex mis-captures

index.ts:686 — /\{[\s\S]*\}/ greedily grabs to the last brace; trailing prose containing } makes it over-capture → JSON.parse throws → silent []. Cosmetic (checklist only), but combined with M1's swallowed catch it hides real parse failures.

---

Low / Nits (grouped)

Content gate robustness (all schemas.ts): hasTestStructure false-positives on describe/it appearing in strings/comments and false-negatives on describe.skip/it.each (M, :49-63); isNotPlaintext inspects only the first 5 non-comment lines against 4 narrow English phrases (:87-116); hasProperImports regex is order-dependent and trivially spoofable (:76-85). Net: the gate is a syntactic smoke test, not a quality gate — fine if labeled as such.

Maintainability: the test-gen claude-api module duplicates the code-gen module's entire generation engine (index.ts:46-405 and throughout) rather than sharing it — verifier's top maintainability finding (high→medium); also validateAgainstManifest reimplemented inline despite a shared lib (:1167), getProvider silently diverges from code-gen (:421-424), duplicated unvalidated env-int parsing (:884-895), and the registry's register() diverges (throws on duplicate vs code-gen). Comment density is high against the project's stated minimal-comments preference — many JSDoc blocks restate signatures or narrate iteration history ("iter8 Fix 2a", "R17 v7" tags); keep only the why comments (the honorsCustomTools rationale is a good keep).

Tool-gating residuals: CLI text-only deny-list is a blocklist, not an allowlist and does not deny mcp__* tools (claude-cli.ts:50-53) — with --dangerously-skip-permissions defaulting on (:87), any non-listed/MCP tool runs uncontained if a caller forgets disableTools. The test-gen path is correct today (sets it on every call) but the safety is by-convention, not enforced by the provider. Anthropic provider silently ignores disableTools (untyped provider-specific contract, :166-188); CLI provider misreports providerType: 'anthropic' (:378-382).

Resilience residuals: no timeout/AbortSignal on any LLM invoke — a hung call stalls the terminal node (shutdown checks only fire between files); parseTestPlan (\S+) silently drops paths with spaces / splits target path at an embedded hyphen when spacing is missing.

Type safety: parseDiagnostics read via as unknown as cast on a TS-internal field (schemas.ts:134-135) — refuted as a fail-open security risk because the schema spec's known-broken snippet would turn RED on TS API drift, but it's a real internal-API coupling nit (swap for ts.transpileModule({reportDiagnostics:true})).

Graph-wiring nit: the "currently inert" comments (development-supervisor.ts:240 etc.) now contradict actual behavior — the node does run the LLM agent. Refresh the stale comment/log strings.

Recommended merge path

1. Fix H1, H2, H3 (alias, hasAssertions, path containment) — small, high-leverage, close the real gaps.
2. Fix M1 + M2 (authoritative validation + honest failure reporting) — restores the type-safety and observability the design promises; both are small.
3. Add tests for the continuation/truncation path (M3) and apply the stripReasoningPreamble-on-continuation fix — biggest coverage gap and a genuine correctness bug.
4. Defer the deduplication of the code-gen/test-gen engines (M5/Low maintainability) to a dedicated refactor PR — it's the largest change and lowest risk if left for now.
5. Comment-density trim and stale-comment refresh can ride along or wait.