docs(plan): standing artifact-provenance audit for M2–M5 upstream bumps - #298
Conversation
M0 Phase 0 closed #295's mechanism (make build now runs npm ci) but not the class: a bundle can be correct-by-construction yet behaviorally regressive -- an upstream client regression, or a server<->client wire mismatch. No install-time check sees that, and CI does not run the live UI, since it excludes the e2e suite twice over. Each of M2-M5 pins a new upstream release per convention 11 and so regenerates the artifact -- M5 certainly, as it ships a client-side feature. M1 does not, which is what makes deferring this reasonable rather than negligent. Adds a pre-bump checklist where the executing session will actually read it, since M2 will be a different session without this context: verify direction rather than mere difference, match livetemplate's declared ClientVersion rather than npm latest, run the local suite under GOWORK=off, and treat a checkbox-toggle failure as a bundle problem until proven otherwise -- it mimics the flakiness #292/#293 fixed and the tell is 100% reproducibility. Also records the explicit decision point for #297 (browser e2e smoke subset in CI), deferred out of M0 on purpose: the natural smoke subset is the checkbox suite that #292/#293 had to stabilize, so that call is better made with M1's stability learnings in hand. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
ReviewDocs-only change (
One accuracy concern worth a look before this lands: The "M2–M5 regenerates this artifact" claim overstates M4. The new section says "each of M2–M5 pins a new upstream release (convention 11) and therefore regenerates this artifact" — but per the Roadmap outline just above it, M4 bumps Minor/non-blocking:
Nothing else stood out; the cross-link into § Risks and the M0 Phase 0 Learn tie-back are accurate and well-scoped. |
Review-bot catch on #298, verified: the checklist said "each of M2-M5 pins a new upstream release and therefore regenerates this artifact", which is wrong for M4. github.com/livetemplate/lvt/components is a server-side Go module consumed by internal/server/websocket.go and internal/runtime/state.go; it appears nowhere in client/src or client/package.json, so bumping it does not touch the browser bundle. As written, a session executing M4 would have been sent through npm ci, a marker check and checkbox-focused e2e for an artifact its change never regenerates -- wasted work that also teaches the wrong mental model. The real trigger is a bump to @livetemplate/client specifically. Replaces the blanket claim with a per-milestone table: M1 no; M2/M3 yes but transitively, since ClientVersion is a wire contract with no runtime handshake so a server bump obliges matching the client; M4 no; M5 certainly, as it ships a client-side feature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
|
Good catch — verified and fixed in the follow-up commit. You're right that Replaced the blanket claim with a per-milestone table naming the real trigger (a bump to I took the table over your 'scope to M2/M3/M5' option because the M2/M3 case is indirect — the bundle isn't what's being bumped, the wire contract is what drags it along — and a bare milestone list would have lost that reasoning. |
ReviewDocs-only change to
This is a good example of self-correcting process: the second commit ( Nits (non-blocking)
Overall: accurate, well-scoped, and appropriately deferential to #297 rather than scope-creeping browser e2e into this PR. No code changes to review for bugs/security/perf, and no tests are applicable to a docs-only change. |
Second review-bot catch on #298, and the same class of error as the first: "Yes, transitively" for M2/M3 read as an unconditional rule, when what actually governs is whether the release moves ClientVersion. A livetemplate release touching only server-side APIs may leave it unchanged -- Validate() is arguably one -- making M2 legitimately a No. Reworded both rows to "Check ClientVersion" with the counter-case spelled out. The distinction is the reason the checklist exists, so stating it as a rule to look up rather than a conclusion to inherit is the difference between a session verifying and a session assuming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
|
Taken — same class of error as your first catch, so worth fixing rather than landing. Reworded both M2/M3 rows from "Yes, transitively" to "Check That's the right framing anyway — the distinction is the whole reason the checklist exists, and stating it as a constant to look up rather than a conclusion to inherit is the difference between a future session verifying and assuming. Landing after this goes green. |
ReviewDocs-only change (plan file), and unusually well fact-checked for a PR like this — I verified the claims against the actual repo rather than just reading the prose, and everything checks out:
Nice to see the commit history shows this already went through two rounds of self-correction from review-bot catches (the M4 blanket-claim bug, then the M2/M3 hard-yes-vs-check distinction) — both fixes are reflected correctly in the final diff. Minor nits (non-blocking)
No code changes, so no functional bugs, performance, or security concerns to flag, and test coverage is N/A for a docs-only PR. Good change — the "why" section in the description (why #297 was deferred rather than pre-empted) is also a useful bit of context to have preserved in the plan itself rather than only in the PR body. |
Follow-up to #296, and the plan-side half of #297.
M0 Phase 0 closed #295's mechanism —
make buildnow runsnpm ci, so the committed client bundle can no longer be built from stalenode_modules. It did not close the class: a bundle can be correct-by-construction yet behaviorally regressive (an upstream client regression, or a server↔client wire mismatch — livetemplate exportsClientVersionprecisely because there is no runtime handshake). No install-time check sees that, and CI does not run the live UI, excluding the e2e suite twice over (-tags=ciand-skip='E2E|e2e').Every M2–M5 milestone pins a new upstream release per convention 11 and therefore regenerates this artifact — M5 certainly, since it ships a client-side feature. M1 does not, which is what makes deferring the CI work reasonable rather than negligent.
What this adds
A standing pre-bump checklist in the M2–M5 section, placed where convention 10 guarantees the executing session reads it — M2 will be a different session without this context, and a risk-section entry alone is too easy to skim past:
ctrlKey1→5 check was a proxy for one fix, not whole-diff verification.ClientVersion, not to npmlatest.GOWORK=offwith./tinkerdownpre-built.Also cross-links #297 from the § Risks entry.
Why #297 was deferred rather than done now
The natural CI smoke subset is the checkbox suite that #292/#293 had to stabilize. Putting headless Chrome into CI to guard a risk that doesn't bite until M2 risks trading a quiet problem for a flaky one, on the critical path of the demo milestone. Better made with M1's test-stability learnings in hand — so this records the decision point rather than pre-empting it.
Docs-only; no code changes.
🤖 Generated with Claude Code
https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h