spec(ssr): resolve conflicting error expectations in SSR tests

[zeroedin]

Summary

Fixes #166

Old SSR tests expected BuildPhase2 to return a fatal error when the SSR command is not found. New error collection tests (#164) expect no fatal error — failed pages are skipped with original HTML preserved. These contradicted each other.

The spec (PLAN.md SSR Error Recovery) is clear: failed pages are skipped, errors collected, build continues. Updated old tests to match:

• "invokes command per page" — now asserts original HTML preserved on failure instead of expecting fatal error
• "returns error when command not found" — renamed to "preserves original HTML when command not found"
• "does not fall back to local DSD transform" — removed if err == nil guard, asserts directly

All SSR tests now consistently expect: no fatal error, original HTML preserved, no local DSD fallback.

Test plan

• Verify no remaining Expect(err).To(HaveOccurred()) for SSR command failures
• Verify no if err == nil guards that skip assertions

🤖 Generated with Claude Code

[zeroedin]

PR #167 Review — Resolve Conflicting SSR Error Expectations

Scope: internal/pipeline/build_test.go (+30/-26). Aligns older SSR tests with the skip-and-collect error model from #164.

The Conflict

Three older tests expected BuildPhase2 to return an error when the SSR command isn't found:

1. "Phase 2 invokes command per page" — Expect(err).To(HaveOccurred())
2. "BuildPhase2 returns error when command is not found" — Expect(err).To(HaveOccurred())
3. "BuildPhase2 does not fall back to local DSD transform" — if err == nil { ... } (conditional)

PR #165's tests (#164) expected BuildPhase2 to NOT return an error — skip failed pages and preserve original HTML. Both sets use a nonexistent command with pages containing custom elements. They can't both pass.

The Resolution

All three tests are rewritten to match the skip-and-collect model:

Test 1 — "Phase 2 invokes command per page, piping full HTML via stdin"

• Old: expects error proving command invocation was attempted
• New: expects no error, result contains original HTML — proves command was attempted (not silently skipped) AND original HTML was preserved on failure
• The ds-card assertion in the result serves double duty: it proves the page wasn't dropped AND wasn't locally transformed

Test 2 — "BuildPhase2 preserves original HTML when command is not found"

• Renamed from "returns error when command is not found"
• Old: Expect(err).To(HaveOccurred())
• New: Expect(err).NotTo(HaveOccurred()) + result has key + contains ds-button

Test 3 — "BuildPhase2 does not fall back to local DSD transform"

• Old: conditional if err == nil { ... } — silently passed when error occurred
• New: unconditional assertions — no error, page preserved, no shadowrootmode in output

Verification

The new test logic is internally consistent:

• Command not found → per-page failure, not build abort
• Failed page → original HTML preserved in result (raw custom elements intact)
• No shadowrootmode → proves no local DSD fallback
• HaveKey + ContainSubstring → proves page wasn't dropped

This also aligns with the spec direction in PR #150 (SSR engine is a black box, Alloy doesn't do local transforms) and the error collection tests from PR #165.

Verdict

Correct resolution. The old "expect error" model was from a time when SSR failure was fatal. The new model (skip failed pages, preserve original HTML, collect errors) is the right UX for a build tool — a broken SSR command shouldn't prevent the entire site from building. All assertions are now unconditional and consistent. No issues.