test(cli): fmt.test.ts asserts canonical=null without distinguishing failure modes

[antnewman]

Summary

packages/cli/tests/fmt.test.ts asserts that toCanonical() returns null for "file without YAML frontmatter" — but doesn't have companion tests for other input shapes that could plausibly return null. A caller looking at this test sees "returns null on missing frontmatter" and may write code that treats every null from toCanonical as "no frontmatter", when it could equally mean malformed YAML, unbalanced delimiters, or something the parser silently couldn't read.

The test codifies a silent-null-return without distinguishing failure modes — the test discipline anti-pattern flagged by the silent-failure audit lens.

Where

packages/cli/tests/fmt.test.ts:203-207:

it("should handle file without YAML frontmatter", () => {
    const content = "# Just markdown\n\nNo YAML frontmatter here.";
    const canonical = toCanonical(content);
    expect(canonical).toBeNull();
});

There is no companion test for:

• Input has frontmatter delimiters (---) but the YAML between them is malformed
• Input has unbalanced delimiters (--- at top, no closing ---)
• Input has frontmatter but unparseable YAML (e.g. tabs where spaces required)

All three could plausibly return null from toCanonical. A caller can't tell the test apart from the underlying behaviour, and the underlying behaviour isn't pinned for the other shapes.

Why it matters

This is the same anti-pattern the canaries (PR #54) were designed to address at the compiler level — tests that assert silent-null without distinguishing failure modes codify the bug rather than catch it. A future refactor that changes toCanonical to return null on malformed YAML (because the parser threw and the function ate the error) would pass this test, and any caller depending on "null only means missing frontmatter" would silently break.

Suggested fix

Quick: Add three companion tests, each asserting a distinct outcome:

it("should handle malformed YAML in frontmatter", () => {
    const content = "---\nkey: value\n  bad: indent\n---\n# body";
    expect(() => toCanonical(content)).toThrow();  // or assert specific return shape
});

it("should handle unbalanced frontmatter delimiters", () => {
    const content = "---\nkey: value\n# missing close";
    // assert specific shape
});

The specific assertions depend on what toCanonical's contract actually is — which is itself worth pinning explicitly.

Better: Change toCanonical's return signature to a discriminated union:

type CanonicalResult =
    | { ok: true; value: string }
    | { ok: false; reason: "no-frontmatter" | "malformed-yaml" | "unbalanced-delimiters" };

Forces callers to handle each case at the type level.

Related

• Surfaced by the silent-failure audit's Pass B (test discipline). Companion to the broader observation that the codebase has a history of silent-null-return bugs (Issue @logic-md/core: dist/schema.json is in the published tarball but not produced by npm run build — CI fails 69 tests as a result #1, the cross-step risk_id finding on docs(examples): add public-sector risk-register assurance worked example #45) — and tests should be auditing for that pattern, not codifying it.

Offer

Happy to PR Option A (additional companion tests pinning current behaviour). Option B is a bigger change touching the function's signature; recommend doing it as a separate refactor if the type-discriminated approach feels right.