M1 Phase 1: approved-surface manifest + precedence pinning

[adnaan]

First phase of M1 in the ephemeral-UI reframe plan, following M0 (#296, #299, #300).

Adds the generation: block that turns a tinkerdown.yaml into a manifest — the set of sources and actions an LLM may wire up when generating an app — plus describes: metadata carrying no runtime behavior, so an operator reviewing a generated app reads "the pending PII access requests queue" rather than a table name.

Approval is a precedence tier, not a prohibition

The rejected alternative was "when a manifest is present, frontmatter may not declare sources:/actions: at all." A conditional ban is confusing and forces per-field tracking of what's required versus forbidden. Instead the documented two-tier rule gains a top tier:

1	manifest-approved — pinned
2	frontmatter
3	config defaults

Projects with no generation: block behave exactly as before.

Why pinning is necessary: a page's frontmatter can declare its own sources and actions. Without it, a generated page could reference an approved name while defining that name as something the operator never approved — defeating any check that reasons about names alone, since the name really is in the approved set. Pinning makes an approved name mean one thing regardless of what the page says. Attempts are logged, not silently dropped, so a generating agent sees why its definition had no effect.

The action side needed two changes, and one was a missing capability

Sources and actions resolve through different code paths, so each needed its own fix — and actions needed more than a guard:

• Reachability. Actions never had the site-config fallback sources have; a page could only invoke actions declared in its own frontmatter. That left generation.actions inert — naming a surface no generated page could reach, forcing it to declare those actions itself, which is precisely what approval exists to prevent. Since all the privilege lives in actions (scoped export, approve, deny, audit append), the manifest would have governed only the harmless half of the system.
• The pin. Same rule as sources.

The fallback is deliberately limited to approved actions. Mirroring sources exactly would have made every action in tinkerdown.yaml — including ones written for schedules or webhooks — callable from any page: a privilege expansion arriving as a side effect of a manifest feature. A test pins that projects without a generation block see no such leak.

Verification

• TestGetEffectiveSourcePrecedence (6 cases) and TestGetPageActionsPrecedence (5 cases) cover both resolution paths
• Each was verified to fail with the implementation neutralised, not merely to pass with it
• Full GOWORK=off go test ./... green including the root package with all 32 //go:build !ci e2e files (821s)

One acceptance criterion is recorded as NOT met. I wrote an E2E criterion during the Audit precisely because a runtime demo was missing, so quietly ticking it would be the failure this plan keeps documenting. The unit tests call getEffectiveSource/getPageActions directly — the real resolution functions, not reimplementations — so the logic is proven where it runs. Not proven: that a served page reaches them with a shadowing frontmatter and renders approved data. Carried to Phase 4, which builds the fixture it needs.

Deferrals (operator decision)

• Manifest accessor → Phase 2. The config already exposes approved sources, actions and style guide directly; building the struct now means guessing a shape and maintaining a parallel representation. Phase 2 defines it against a real consumer or strikes it.
• Demo fixture manifest → Phase 4, which builds the PII console fixtures. Seeding it here means inventing sources Phase 4 would rewrite.

Also recorded for Phase 4

Action.Statement is a single statement — nothing splits on ;. Phase 4 specifies Approve as "runs the scoped export and appends an audit record": two operations. Split into two actions, an approve can succeed while its audit append fails, destroying the durable-audit guarantee that is the demo's justification. Flagged as a deliberate Phase 4 decision rather than something to discover mid-wiring.

🤖 Generated with Claude Code

https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h

[claude]

Review: M1 Phase 1 — approved-surface manifest + precedence pinning

Solid, well-scoped change. The precedence design (manifest-approved > frontmatter > config defaults) is sound, the write-up in the plan doc is unusually thorough about what was and wasn't verified, and both resolution paths (getEffectiveSource/getPageActions) are nil-safe and covered by tests that were confirmed to fail with the pin removed — that's the right bar.

A few things worth a look:

1. Doc comment overstates enforcement (internal/config/config.go)

The GenerationConfig field comments say:

Sources names the sources a generated app may bind to. Each must exist in Config.Sources; naming an undefined source is a config error, not a silent skip.

but I don't see anywhere that this is actually enforced — no Validate* function checks generation.sources/generation.actions against the declared Sources/Actions maps (confirmed via grep -rn Generation internal/ cmd/, only config.go and websocket.go reference it). In practice today, an approved name that doesn't exist:

• for sources: silently falls through to tiers 2/3 in getEffectiveSource (never errors)
• for actions: silently skipped in approvedSiteActions's continue

This matches the test names ("approving a name the project never defined does not invent a source" / "...does not synthesise one"), which correctly describe a silent skip — so the test and the comment actually disagree with each other. Since Phase 2 is where the validate lint lands, might be worth either softening the comment now (e.g. "Phase 2's validate lint will report a name that doesn't exist; today it is silently inert") or, if it's cheap, adding the existence check to config loading in this phase so the comment is true on merge.

2. Possibly broken anchor link (docs/reference/config.md)

A project that declares a [generation block](#generation-approved-surface-for-llm-generated-apps) gains a third, highest tier:

targets the heading:

## `generation:` — approved surface for LLM-generated apps

GitHub's heading-slug algorithm strips the colon and em-dash but doesn't collapse the resulting adjacent whitespace, so the actual generated anchor likely has a double hyphen after "generation" (#generation--approved-surface-for-llm-generated-apps) rather than the single hyphen used in the link. Worth checking in the rendered preview — low severity (docs-only) but a easy one-character fix if confirmed.

3. Minor: no YAML round-trip test for the new block

All the new tests (source_precedence_test.go) construct config.Config/GenerationConfig directly in Go, and config_test.go doesn't exercise yaml.Unmarshal for generation:/describes: either. That means a yaml:"..." tag typo on GenerationConfig or the new Describes fields wouldn't be caught by any test today. This seems consistent with how sparsely other config fields are round-trip tested in this repo, so take it as a nice-to-have rather than a blocker.

What looks right

• getEffectiveSource's tier-1 branch reads from h.config.Sources (already config.SourceConfig), correctly skipping the parser→config struct conversion needed for the frontmatter tier — no accidental double-conversion bug.
• The action-reachability fix is properly scoped to approved actions only (approvedSiteActions), not all of config.Actions — avoids the privilege-expansion trap called out in the PR description (schedule/webhook-only actions staying unreachable from pages). Verified server.go/webhook.go's scheduled/webhook action execution paths are untouched and don't share this new reachability.
• Nil-safety throughout (ApprovedSource/ApprovedAction/IsManifest all check c == nil first) means getEffectiveSource/getPageActions don't need extra guards before calling them.
• Shadowing attempts are logged unconditionally (not gated behind h.debug), which fits the "visible, not silent" design goal, and call frequency (once per source-block init) shouldn't make this noisy.

Nothing here blocks merging — #1 is the only one I'd actually want addressed (or at least the comment softened) before this ships, since it's a claim about behavior that isn't true yet.

[adnaan]

All three addressed — the first was a genuine contradiction I shipped, and it was worse than a doc nit.

1. The comment claimed enforcement that didn't exist, and my own tests asserted the opposite. You're right that they disagreed. I made the behavior true rather than softening the comment, because the silent version has a real consequence: approval is what pins a name against redefinition, so an approved name with nothing behind it never pins anything. generation: sources: [requets] would leave requests shadowable — the typo removes a protection while appearing to add one. That's the worst shape a config error can take, so it can't be quiet.

ValidateGeneration is wired into Load, deliberately not following the ValidateWebhooks/ValidateOutputs pattern — those have no callers anywhere in the repo, so copying them would have produced a validator that never runs. Worth flagging as a separate latent issue: two validators exist that nothing invokes.

Note the resolution-layer tests you spotted ("does not invent a source" / "does not synthesise one") are unchanged and still correct — those assert defense-in-depth at the lookup layer. Config load now rejects the typo earlier; the lookup still refuses to invent anything if one slips through.

2. Anchor — fixed by simplifying the heading to ## The \generation:` blockrather than encoding the fragile--` GitHub generates from the em dash.

3. YAML round-trip — added, and you were right that it's a real gap rather than a nicety: every other test builds Config structs directly in Go, so a mistyped tag would have been invisible to all of them. The block would parse into zero values and approval would silently do nothing — the same silent-inertness failure as #1, arriving by a different route.

Full suite green at 843s with the validator now running on every config load in the repo.

[claude]

Review: M1 Phase 1 — approved-surface manifest + precedence pinning

Overall this is a well-scoped, carefully-documented change. The PR description and plan doc do a lot of the reviewer's work already (rejected alternatives, the MergeFromFrontmatter→getEffectiveSource correction, the "verified to fail before it passed" methodology for the precedence tests). A few notes below, nothing blocking.

Code quality

• getEffectiveSource/getPageActions/approvedSiteActions are all nil-receiver-safe (Config.IsManifest, ApprovedSource, ApprovedAction handle c == nil), which matches the existing style in this file and avoids a class of panics if h.config is ever unset.
• Minor, non-blocking: in getEffectiveSource the guard h.config.IsManifest() && h.config.ApprovedSource(name) is slightly redundant — ApprovedSource already returns false whenever Generation == nil, so the IsManifest() half of the check is always implied. Not a bug, just a spot /simplify could still trim.
• ValidateGeneration's error messages name the offending entry ("generation.sources: %q is approved but no such source is defined"), which is exactly what you want for a config that fails at load time — good.

Bugs / correctness

• Didn't find a way to make an approved name shadowable — traced both getEffectiveSource and getPageActions/approvedSiteActions and the pin is applied last in both (source: checked first and returned immediately; actions: site/approved copied into result after page actions, so it always overwrites). The new tests (TestGetEffectiveSourcePrecedence, TestGetPageActionsPrecedence) cover the shadow case, the no-manifest case, and the "approved name that resolves to nothing" case, and match what the code does.
• Load() calls ValidateGeneration() right after yaml.Unmarshal, and config.Load is the only call site in the repo (cmd/tinkerdown/commands/serve.go:88), so there's no path that loads a config from disk without the check running.
• One thing worth double-checking outside this diff: getPageActions/approvedSiteActions make an approved site action reachable from any page once it's named in generation.actions. That's called out explicitly in the PR description and is deliberately scoped to the approved set only (not all site actions) — the test "without a generation block site actions stay unreachable" pins that. Worth confirming in Phase 2/3 that nothing else assumes an approved action is not page-reachable — ValidateGeneration won't catch "approved something that was only meant for a webhook," since that's a policy question, not a config-shape one.

Performance

No concerns — slices.Contains over what will realistically be small name lists, and the pin/log logic runs at block-init/action-lookup time (once per websocket handler / cached in lookupSource), not per source poll.

Security

This is fundamentally a privilege/precedence fix, and it's treated with the right amount of rigor:

• The core bug being closed (page frontmatter can shadow an approved name, defeating name-based approval) is real and the fix is correct.
• Good catch in the third commit: the doc comment claimed unapproved-name typos were "a config error, not a silent skip" while the tests asserted the opposite — that's exactly the kind of contradiction that turns into a real vulnerability (an approved name that silently pins nothing). Fixing the code to match the comment (fail at load) rather than softening the comment was the right call.
• The action-reachability fallback is intentionally not mirrored to "all site actions" to avoid a privilege-expansion side effect, with a test asserting no leak when no generation block is present.

Test coverage

Strong for what's in scope: TestGetEffectiveSourcePrecedence (6 cases) and TestGetPageActionsPrecedence (5 cases) exercise both resolution paths including the negative cases (approving something undefined doesn't synthesize an entry), plus a YAML round-trip test that would have caught a mistyped yaml tag (the other tests all build Config structs directly in Go, so that gap was real). The PR is upfront that the E2E path (a served page actually reaching these functions with shadowing frontmatter) isn't proven yet and defers it to Phase 4 rather than quietly checking the box — appreciate that this is called out rather than glossed over.

Docs / nit

• docs/reference/config.md and the plan doc are updated consistently with the code; the three-tier precedence table is clear.
• CHANGELOG.md isn't touched — since generation: is a new user-facing config block, might be worth a line under [Unreleased] at some point (not necessarily this PR, given Phase 1 has no runtime-demo consumer yet).

Nothing here blocks merging — nice work tying the runtime fix, the config validation, and the plan-doc audit trail together.

[adnaan]

Both taken.

Redundant guard — removed from getEffectiveSource; you're right that ApprovedSource already covers the nil-Generation case. Kept in approvedSiteActions, where it's load-bearing rather than stylistic: that function goes on to dereference Generation.Actions.

CHANGELOG — added under [Unreleased]. Agreed it's warranted: generation: is user-facing config, and the field list alone doesn't convey the three things a reader actually needs — that approval is a precedence tier rather than a prohibition (and why pinning is necessary at all), that approval is also what makes site-level actions reachable with the fallback deliberately limited so schedule/webhook actions stay out, and that an approved name referring to nothing fails at load because it would otherwise be silently inert.

Separately, flagging something I hit while wiring ValidateGeneration: ValidateWebhooks() and ValidateOutputs() have no callers anywhere in the repo. Two validators that never run, so webhook and output configs are accepted unvalidated today. I avoided copying that pattern here by wiring into Load, but the existing two look like a real latent issue — out of scope for this PR, worth its own.

[claude]

Review: M1 Phase 1 — approved-surface manifest + precedence pinning

Solid piece of work — the PR description and commit history already do most of the reviewer's job (the self-correction of the shadowing mechanism, the "actions were inert" finding, and the follow-up fixing the ValidateGeneration comment/test contradiction are all good signs of a careful process). I read the full diff, cross-checked the claims in the commit messages against the code, and traced the nil-safety chain. Notes below are mostly minor; nothing blocking.

Correctness

• getEffectiveSource/getPageActions/approvedSiteActions nil-safety checks out: Config.ApprovedSource/ApprovedAction/IsManifest all guard c == nil first, so h.config.ApprovedSource(name) is safe to call even when h.config is nil, and the short-circuit || in approvedSiteActions (!h.config.IsManifest() || h.config.Actions == nil) means h.config is only dereferenced once IsManifest() has already proven it's non-nil. No panic path found.
• The precedence pin is implemented independently for sources (getEffectiveSource) and actions (getPageActions), matching the PR's stated finding that they resolve through different code paths — confirmed by reading both call sites rather than taking it on faith.
• ValidateGeneration is correctly wired into Load (internal/config/config.go:884), and I confirmed ValidateWebhooks/ValidateOutputs genuinely have no callers anywhere in the repo, so the commit's justification for not following that pattern holds up.
• The doc anchor fix (## The generation: block → #the-generation-block) matches GitHub's slug algorithm correctly (backticks stripped, colon dropped, spaces to hyphens).
• Approved action pointers (approvedSiteActions) are shared directly from h.config.Actions rather than copied — consistent with how webhook.go and server.go already resolve site actions elsewhere in the codebase, so this isn't a new mutation-sharing risk.

Test coverage

• TestGetEffectiveSourcePrecedence (6 cases) and TestGetPageActionsPrecedence (5 cases) cover the tiers well, including the two edge cases that matter most for a security-relevant feature: approving a name the project never defined (must not synthesize a source/action), and no-generation-block behavior being byte-for-byte unchanged.
• The YAML round-trip test (TestGenerationYAMLRoundTrip) is a good catch on its own — every other test in this PR builds Config structs directly in Go, which would never catch a mistyped yaml tag.
• One gap: there's no test exercising getPageActions/getEffectiveSource through the actual Load() → ValidateGeneration() path together with a page that has a malformed frontmatter action of the same name (vs. a well-formed one). Low priority given the unit tests already hit the resolution functions directly.

Design

• The "precedence tier, not prohibition" choice, and confining the action fallback to approved actions only (rather than mirroring the source fallback exactly), are both good calls — the PR description's rationale (avoiding a privilege-expansion side effect for schedule/webhook-only actions) is directly reflected in approvedSiteActions' scoping and is explicitly tested ("without a generation block site actions stay unreachable").
• Logging shadow attempts instead of silently dropping them is a nice touch for a generating agent's feedback loop, though at log.Printf this is easy to miss in production logs — worth a note for Phase 2 whether validate should also surface this at generation time (already flagged in the plan doc's feed-forward).

Minor / non-blocking

• Action.Statement being a single statement (no atomic export+audit) is correctly flagged as a Phase 4 risk rather than silently punted — appreciated that this was caught now rather than mid-wiring later.
• Per this repo's CLAUDE.md, I checked whether this PR touches any lvt-* attributes that might already exist in livetemplate/client — it doesn't; this is config/server precedence logic only, so that check doesn't apply here.

Security

• The core security property (an approved name can't be silently hijacked by page frontmatter) is the right thing to guard, and the "approved name refers to nothing → fail at load" validation closes a real footgun (a typo silently disabling the pin) rather than just a style nit — good catch in the review-bot follow-up commit.
• No obvious injection/traversal concerns in this diff — style_guide is just a path string carried through config with no filesystem access added in this phase (deferred to the accessor).

Nice, well-documented change with good self-correction visible in the commit history. Approving from a code standpoint; the acknowledged E2E gap is reasonably deferred to Phase 4 per the plan doc.