M5: save & share the captured skill (capture completeness + gallery + writable-WASM CLI parity)

[adnaan]

M5 — Save & share the captured skill (spine-only)

Makes the ephemeral-UI model's "keep it, the need recurs" path real and honest: a saved skill re-runs on-brand and self-contained, saved workflows have a discoverable home, and writable WASM has CLI parity. Scope was settled at kickoff as tinkerdown-only, spine-only — save is real; share = git portability (commit the skill dir, a teammate pulls and re-runs). No hosted service, share-link subsystem, or collaborative review — those stay deferred (#282, #249, #223, #222).

Phase 1 — Capture completeness: the house style travels

/tinkerdown:save now captures the manifest whole — the styling block (theme + design tokens) and any generation.style_guide → style-guide.md — not trimmed to sources + actions. So a saved workflow re-runs on-brand instead of on bare defaults, closing the M4 Phase 2 feed-forward.

Honest framing: a committed point-capture already loads its whole tinkerdown.yaml on serve, so house style already travels there — the real gap this closes is the bundle case (a hand-assembled throwaway skill dir that trims the yaml or drops the separate style file). The deliverable is a prose instruction whose future-LLM effect is untestable; the guards target what's real — the style-guide.md ref travels, and the instruction survives.

Phase 2 — Saved-skills gallery (examples/gallery/)

A discoverable home for captured workflows — itself a plain Tinkerdown app (lvt-source over a read-only CSV, no custom JS).

STOP-gate decision (Audit): no source type reads a directory of SKILL.md frontmatter (no ReadDir/glob in internal/source, no frontmatter parsing), so a dynamic directory index isn't buildable in-grain without exec (friction on a listing page). tinkerdown build is an app→binary compiler, not an index generator. This was a material divergence from the plan's imagined dynamic index, so it was put to the operator, who chose the minimal static-index fallback: the gallery renders a committed skills.csv, kept in sync with the captured skills on disk by TestSavedSkillsGalleryInSync (skills/ dirs minus the explicit framework set {tinkerdown, tinkerdown-save} — a new capture fails the test until listed; no silent drift).

Note: a skill's Location renders as displayed text (skills/pii-access-approval), a discoverable pointer — not a clickable link.

Phase 3 — Writable-WASM CLI parity + honest #216 disposition

tinkerdown cli now constructs wasm sources (previously "unsupported source type for CLI"), so a WASM source has the same CRUD surface on the CLI as under serve: list for any module, add/update/delete when the module exports write, and a read-only module refuses a write with a clear error. exec stays out of CLI CRUD (read-oriented, --allow-exec-gated), now documented in the switch rather than a silent fallthrough. Also fixed a load-breaking wasm doc/template bug: module:→path: and config:→options: (the docs used keys that don't exist), plus a Read-only vs. writable doc section.

#216 stays OPEN (not closed by this PR). "Verify, don't infer" found the plan's premise wrong: the only committed .wasm (quotes.wasm) is read-only, there's no TinyGo here to build a write-exporting fixture, and a committed mystery binary would be the #295/#297 provenance trap. So a successful write has zero verified coverage in any transport (WS or CLI). What is verified running: the wasm read path via CLI, and the readonly guard (WriteItem refuses; cli add returns "source is read-only"). A follow-up issue will track the reproducible TinyGo fixture + write-landing test.

Verification

• Full integrated suite green on the final commit: 17/17 packages ok, full root E2E (117 tests) + the new four-channel gallery E2E ok … 870s, EXIT=0 (GOWORK=off go test ./...).
• Per-phase: unit + real-state round-trips; chromedp four-channel E2E (browser console, server log, WS frames, rendered HTML, screenshot); real-binary CLI checks.
• /simplify run on each phase's diff.

Plan

docs/plans/2026-07-09-ephemeral-ui-reframe.md § M5 — every phase's Audit/Implementation/Acceptance/Learn is filled in, including the STOP-gate and the write-verification blocker, per the drift-in-same-commit convention.

🤖 Generated with Claude Code

https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h

[claude]

Review: PR 310 -- M5 save and share the captured skill

Verified against the actual code (not just the PR description) -- the claims all check out.

Spot checks performed

• createCLISource wasm parity (cmd/tinkerdown/commands/cli.go:238-243): wasm.NewWasmSource(name, cfg.Path, siteDir, cfg.Options) matches the real NewWasmSource(name, path, siteDir, initConfig) signature and config.SourceConfig.Path/.Options fields -- same pattern already used in internal/source/source.go:247. exec's omission is now a documented comment instead of a silent default: fallthrough. Good.
• wasm docs/template fix: confirmed module:/config: are gone from docs/sources/wasm.md and the wasm-source template -- path:/options: are the real keys per config.go:307. This was a genuine load-breaking doc bug; good catch.
• Gallery template field binding: skills.csv headers are lowercase (name,description,triggers,path) but index.md binds {{.Name}}/{{.Description}}/{{.Triggers}}/{{.Path}}. I initially suspected a case-mismatch bug (the CSV source builds its row map with the literal lowercase header as key), but internal/runtime/state.go's snakeToPascal transform adds a PascalCase alias for every field, so this resolves correctly -- same convention as the existing lvt-source-file-test example (id,name,price CSV maps to {{.Id}}/{{.Name}}/{{.Price}}). No bug.
• index.md vs app.md/README.md naming: confirmed in internal/site/manager.go -- index.md is explicitly special-cased to /, while README.md maps to /README. The Phase 2 Learn note about the discovery-order-luck footgun and the fix to name the gallery's entry point index.md is accurate and well-reasoned.
• Manifest round-trip claims (captured_skills_test.go): confirmed examples/pii-access-approval/tinkerdown.yaml really does carry styling.tokens plus generation.style_guide: style-guide.md, and style-guide.md exists on disk -- the new TestCapturedSkillsWellFormed assertions check real state, not re-assert SKILL.md prose.
• Build-tag footgun caught in this PR: nice catch renaming cli_wasm_test.go to cli_wasmsource_test.go -- a file matching *_wasm_test.go is an implicit GOARCH=wasm build constraint, so the original name would have silently excluded the test everywhere except wasm builds.

Code quality

• The createCLISource switch addition is minimal and consistent with the existing internal/source/source.go dispatch -- good reuse rather than reinventing wiring.
• cliOptions struct whitespace-only realignment is a harmless gofmt fix, correctly called out as incidental in the Learn notes rather than hidden in the diff.
• The gallery is genuinely dogfooded (lvt-source over a read-only csv, no custom JS) -- consistent with the project's own convention of not reaching for exec/new primitives when an existing source type suffices. The STOP-gate reasoning for why a dynamic frontmatter-scanning index isn't in-grain is sound and well-documented.
• TestSavedSkillsGalleryInSync's explicit frameworkSkills allowlist (rather than a magic marker/heuristic) is a good call -- forces a conscious decision on every new capture instead of silent drift.

Honesty of scope (a strength, not a gap)

The PR is unusually candid about what it does not verify: issue 216 (writable WASM) is deliberately left open because there's no TinyGo available to build a write-exporting fixture, and the PR doesn't fake coverage by mocking one. The only committed .wasm (quotes.wasm) is read-only, and the new TestCLIWasmSourceReadParityAndReadonlyGuard only asserts what's actually exercisable (read path plus readonly guard rejection). This is the right call -- a committed mystery writable binary would reintroduce the artifact-provenance risk called out from issues 295/297.

Minor observations (non-blocking)

• examples/gallery/tinkerdown.yaml sets readonly: true on the csv source, but CSVFileSource has no write path at all (no IsReadonly()/WriteItem implementation) -- the flag is inert for this source type today. Harmless (it documents intent and matches SourceConfig.IsReadonly()'s own safe-by-default), just noting it's belt-and-suspenders rather than load-bearing.
• docs/sources/csv.md still documents the CSV source's file key as path:, while the actual dispatch (internal/source/source.go:229, cfg.File) and this PR's own examples/gallery/tinkerdown.yaml (and the older lvt-source-file-test example) use file:. Pre-existing doc drift, not introduced by this PR, but since this PR is on a streak of fixing exactly this class of doc bug for wasm.md, it might be worth a quick follow-up pass on csv.md too.

Test coverage

Strong -- unit (TestCapturedSkillsWellFormed extended, TestSaveSkillCapturesHouseStyle, TestSavedSkillsGalleryInSync, TestSavedSkillsGalleryManifestLoads, TestCLIWasmSourceReadParityAndReadonlyGuard) and E2E (TestSavedSkillsGalleryRenders, four-channel capture) are all real-state assertions rather than prose scraping, with one explicit, well-justified exception (the SKILL.md instruction-pin, which the PR itself flags as unable to prove future-LLM compliance).

Overall: solid, well-scoped PR with honest treatment of what remains unverified. No blocking issues found.

[adnaan]

Thanks for the thorough, code-verified review. On the two non-blocking observations:

1. readonly: true on the gallery's csv source is inert at the csv runtime — agreed, and intentional. It's config-level intent (config.SourceConfig.IsReadonly() reads it, and TestSavedSkillsGalleryManifestLoads asserts it), plus belt-and-suspenders as you say: a CSV source is inherently read-only at runtime regardless. Keeping it as a declaration that the gallery lists, never mutates.

2. docs/sources/csv.md uses path: where the real key is file: — real bug, and it's broader: json.md and markdown.md have the identical issue (all three read cfg.File; only wasm reads cfg.Path, which M5: save & share the captured skill (capture completeness + gallery + writable-WASM CLI parity) #310 already corrected). Rather than expand this M5 PR beyond its save/share scope with three unrelated doc files, I've tracked it as a follow-up: File-source docs use path: but the real config key is file: (csv/json/markdown) #312. (Same call as leaving feat: writable WASM sources (Phase C2) #216 open with Verify a writable-WASM write actually lands (reproducible TinyGo fixture + write-landing test) #311 for the write-landing fixture — keeping each PR honest to its scope.)

No code changes to #310 from these; both are addressed (kept-with-rationale / tracked-as-follow-up).