feat(config): gate experimental Strata behind server opt-in

[aparajon]

Strata is experimental and should not appear as an available integration in a default installation. Add the server-only experimental-strata-enabled setting, defaulting to false, to gate registrations and setup guidance.

Disabled servers reject Strata registrations during configuration validation and server startup. Enabled servers offer it with an experimental label. Existing Strata deployments must explicitly enable the setting before upgrading; repository configuration cannot opt in.

Keep the existing OSS documentation and template examples. Add a brief notice that the Strata integration is not yet available for open-source use and is disabled by default.

Validation: focused configuration, startup, and comment-rendering tests passed; affected-package race tests and lint passed. Regenerated templates and documentation checks passed.

Generated with Codex

[Copilot]

🟡 Changes recommended

The updated setup guidance has a formatting issue and also omits still-supported Vitess configuration in a way that can mislead users during onboarding.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR gates the experimental Strata integration behind a server-only opt-in (experimental-strata-enabled, default false) so Strata doesn’t appear in default OSS setup guidance and cannot be enabled via repository configuration alone.

Changes:

• Add experimental-strata-enabled to server config and enforce it during config validation and server startup.
• Update error-comment setup guidance to conditionally offer Strata (labeled experimental) only when the server flag is enabled.
• Remove Strata onboarding/docs examples and the Strata template snapshot sections, replacing with an availability notice.

File summaries

File	Description
TEMPLATES.md	Regenerated template catalog; removes Strata sharded-apply examples and updates setup guidance text.
scripts/update-templates.sh	Stops rendering the “Sharded Apply” (Strata) preview section into the snapshot.
pkg/webhook/templates/errors.go	Adds server-driven Strata gating to setup guidance rendering via ExperimentalStrataEnabled + DatabaseTypeOptions().
pkg/webhook/templates/errors_test.go	Adds coverage ensuring setup guidance includes/excludes Strata depending on the server flag.
pkg/webhook/plan.go	Plumbs ExperimentalStrataEnabled from server config into error comment rendering data.
pkg/serve/serve.go	Validates Strata opt-in before server startup (Build).
pkg/serve/experimental_strata_test.go	Verifies Build rejects Strata config unless the opt-in is enabled.
pkg/cmd/commands/unlock.go	Updates CLI help text to avoid enumerating database types explicitly.
pkg/api/experimental_strata_test.go	Adds config parsing/validation tests for the new server-only opt-in setting.
pkg/api/config.go	Introduces ExperimentalStrataEnabled and validates all Strata registration paths require server opt-in.
docs/strata-engine.md	Replaces Strata engine doc with an experimental/availability notice.
docs/github-app-setup.md	Removes Strata from setup guidance and narrows stated type options in schemabot.yaml.
docs/direct-execution.md	Removes Strata-specific direct execution notes.
docs/configuration.md	Documents the new experimental-strata-enabled setting and its behavior.
docs/.toc-manifest	Removes Strata engine doc from TOC manifest.

Review details

Suppressed comments (1)

pkg/webhook/templates/errors.go:115

• The invalid-config setup guidance line has an extra trailing backtick, which will render a malformed inline-code span in the PR comment output.

- **type** (required): {{.DatabaseTypeOptions}}`

• Files reviewed: 11/11 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1338, bbb9d5f.

Verdict: 8 findings — 2 blocking (a casing bypass of the gate itself; the comment-rendering half of the feature is unpinned), 5 non-blocking (docs contradict the gate, two messages became dead ends), 1 suggestion bundle. The gate's core placement is sound: it is the first statement of serve.Build, before the storage check, and no repo-side config can mint a database type the server did not register.

Blocking

1. The gate compares an un-canonicalized type, so type: Strata walks straight through it. config.go:1633 tests databaseType == storage.DatabaseTypeStrata against the raw YAML value, but static targets are canonicalized only downstream — static.go:197 applies canonicalDatabaseType, which is strings.ToLower(strings.TrimSpace(...)) at :259. A target_resolver.targets entry spelled Strata, STRATA, or " strata" passes ValidateExperimentalStrata, passes serve.go:291, and then resolves to strata — and config.go:1664 exempts a resolver-only data plane from the databases block, so nothing else catches it. OSS builds still fail closed at first request (local_client.go:307, "no engine registered for database type"), but a deployment that registers a Strata engine executes the apply with no opt-in; the fix is to run check on canonicalDatabaseType(target.DatabaseType).

2. Deleting the config→comment wiring leaves the entire repo-wide unit suite green. Removing data.ExperimentalStrataEnabled = config.ExperimentalStrataEnabled at plan.go:568 survives go test ./... — no test connects ServerConfig.ExperimentalStrataEnabled to a rendered comment, so DatabaseTypeOptions() is tested only in isolation. The inverse regression is the one that matters: that single assignment is the only thing stopping a non-opted-in server from advertising strata to repo owners, which is the disclosure this PR exists to prevent, and a refactor could drop it invisibly. handleSchemaRequestError is already reachable in tests (handler_test.go:332), so a handler-level assertion that the rendered body flips with the flag is cheap — and AGENTS.md:158 asks for exactly "a test that would fail if it stopped holding".

Non-blocking

3. The onboarding doc still instructs operators into a boot failure. strata-engine.md:121 says flipping type: mysql to type: strata "requires no other SchemaBot config change", and the example at :112 shows type: strata with no opt-in. The new banner at :17 names no setting and does not link back to the new configuration section — the link runs one way only, from configuration.md:1705. Doc-only: the startup error itself names the setting.

4. invalid type lost its vocabulary and no doc replaces it. config.go:1745 now says "choose a database type supported by this server" where it used to enumerate the four types, while the switch one line above still accepts exactly those four. Nothing documents the server-side databases.<name>.type vocabulary — github-app-setup.md:194 is the repo-side schemabot.yaml, and configuration.md:669 is the storage dialect — so the refusal points nowhere against UX-4 (invariants.md:1065). Commit bbb9d5fb restored the list for the repo-side templates but not here, which reads as missed rather than decided.

5. strata is still offered to the audience the gate hides it from. github/config.go:124 ("type must be 'vitess', 'mysql', 'strata', or 'postgres'", and :119 for the required case) is repo-owner-visible, not log-only: it flows through FindConfigsForPRFiles → handleSchemaRequestError's fall-through to RenderGenericError, which posts {{.ErrorDetail}} verbatim into a PR comment. github-app-setup.md:194 likewise tabulates "strata" as a plain valid value. Commit 01b5761e re-widened that row deliberately, and no path actually breaks — so this is the PR's goal being half met, worth a conscious decision rather than a silent asymmetry.

6. Validate's doc comment now documents the wrong function. config.go:1626 — "Validate checks the configuration for required fields and consistency." — sits above ValidateExperimentalStrata, which got its own line too, leaving Validate() at :1656 undocumented. go doc now renders the one-flag helper as the whole-config validator. The new function was inserted between the original comment and its body.

7. "Gate runs first" is only incidentally pinned. Moving the ValidateExperimentalStrata() call to the last statement of Validate() is caught by one sub-case, and only because that fixture has no environments; a Strata config that is otherwise valid produces the identical error either way. So an operator with a typo'd environment name gets the typo error first, fixes it, restarts, and only then learns Strata is gated — two restart cycles. Either pin the ordering with a valid-except-for-Strata config or stop relying on it.

General suggestions

• config.go:47 yaml:"experimental-strata-enabled,omitempty" is the only kebab-case key among the 28 top-level ServerConfig fields (kebab appears only inside the nested github: block), and dec.KnownFields(true) at :1576 turns the natural experimental_strata_enabled guess into a hard parse error. Rename now while nothing depends on the spelling. omitempty on the bool is inert — ServerConfig is never marshalled — but would drop an explicit false if a config-dump surface is ever added.
• config.go:1649 passes resolver.DatabaseType as both the identity and the type, rendering the tautology resolver "strata": Strata is experimental…. Harmless — target_resolver_factory.go:60-67 enforces database_type unique per resolver, so it does identify the entry — but the repo's own precedent for these elements is the target_resolver.etre: config path.
• config.go:996 — "Type is the database type: "mysql", "vitess", or "strata"" — omits postgres and no longer flags Strata as gated. Pre-existing rot from add support for strata engine #245, but this PR is the natural place to fix the field it now gates.
• experimental_strata_test.go:20's require.False(t, cfg.ExperimentalStrataEnabled) asserts a zero value on a literal that never sets the field, and the enabled == false half of TestExperimentalStrataServerSetting cannot distinguish a decoded false from an undecoded one. Also, all six api/serve assertions are substring matches on the message text — one reword kills every one of them at once — and none of the four new tests carries the scenario comment AGENTS.md:286 asks for.

The one thing that could have broken, verified

A server-side gate is worthless if the data plane can mint a gated type at request time, so I traced every path that sets ExecutionTarget.DatabaseType. The repo-side route is genuinely closed: ResolveDatabaseTargets (config.go:2611-2670) sets the type from dbConfig.Type on all three return paths, plan_handlers.go:635 and :259 reject a repo declaring a type that disagrees with the server, no persisted databases table exists (database_type appears in pkg/schema/mysql/ only on in-flight rows), and serve.Build is the sole construction seam — Run:143, RunLocal:82, cmd/commands/serve.go:31 and local.go:47 all route through it, so an upgrade without the opt-in fail-closes at boot as the PR claims. The bypass I did find is the sibling path, not this one: static targets reach the engine registry with a type the gate never canonicalized, which is finding 1.

Verified correct

• TEMPLATES.md is not drifted — the hand-edit matches what the generator produces, and make check-templates reports "TEMPLATES.md is current".
• Both branches of DatabaseTypeOptions() are asserted directly, including exact-line equality on the rendered - **type** line, so a regression there does fail unit tests.
• All three new test files run on every PR: test.yaml:147 → make test-unit → go test -race ./.... Baseline is green, gofmt/go vet clean, integration build tags included.
• The gate precedes the storage-DSN check — mutating it away yields "storage DSN not configured", proving the ordering.
• No docs/invariants.md entry is owed: AGENTS.md:169-171 forbids engine-named entries, and the PR does all three prescribed alternatives instead (docs, a field comment, a validation test).
• The docs/configuration.md "Experimental integrations" section needs no .toc-manifest change and its #experimental-integrations anchor resolves.
• plan.go:568's h.config() != nil guard is load-bearing — removing it panics an existing handler test.

This review was generated by Claude Code (claude-opus-5).