feat(schema): support ignore_namespaces in schemabot.yaml

[aparajon]

Why this matters

Some schema roots carry a namespace that must never be reconciled against a live database — the common case is a namespace that exists only to back local test infrastructure. Today every plan diffs it like any other namespace and proposes creating its tables in real environments, and the only way out is restructuring the schema root. This PR adds ignore_namespaces to schemabot.yaml so a repo declares those namespaces once and plans, applies, and merge-gate checks never see them.

What it does

database: commerce
type: vitess
ignore_namespaces:
  - commerce_test

• Ignored namespaces are dropped at the single grouping choke point (schema.GroupFilesByNamespace), so every consumer — GitHub PR flow and CLI, plan and apply and checks — sees the same filtered view.
• Filtering happens after grouping, so layout validation (mixed flat/subdirectory rejection) still sees the full tree.
• Entries get the same $ENV substitution as directory names, and are validated at config load to be bare namespace names, not paths.
• The webhook flow logs the exclusion with repo/PR/database identifiers, and the "no schema files found" error says when ignores emptied the result.
• onboard rewrites preserve an existing config's ignore_namespaces.

Two safety properties worth calling out:

• Fail closed on the DSN shape that can't honor the exclusion. The excluded namespaces travel with the plan request (CLI and webhook → proto), and the server refuses the one MySQL shape where the exclusion is unenforceable:

plan with ignored namespaces
  ├─ namespace-free MySQL DSN   → per-namespace diff; ignored namespaces never touched
  └─ database-scoped MySQL DSN  → whole-DB diff would plan the ignored namespace's
                                  live tables as DROP TABLE → refused at plan time

• Exclusions are disclosed where reviewers look. The plan comment renders a disclosure line under the plan summary — on changes, no-changes, and multi-env paths (environments whose exclusions differ no longer deduplicate into one section) — and the CLI prints the same line for plan/apply. A withheld namespace is distinguishable from an unchanged one, and a PR that introduces an entry is visible in review:

  📋 Plan: 2 tables to create, 1 table to alter

  ℹ️ Namespaces excluded from this plan by ignore_namespaces: local_fixtures

  Only namespaces actually removed are disclosed; configured entries that matched nothing (typo, case mismatch, stale entry) produce a warning instead of silently reconciling the namespace they were meant to exclude, and ignoring every namespace is an error.

Docs: new "Ignoring Namespaces" section in docs/namespaces.md (including disclosure behavior and the MySQL DSN requirement) and a field-table row in docs/github-app-setup.md.

How it moves us toward the northstar

Git as the interface only works if the schema root can be the single source of truth for every repo layout that exists in the wild — and real repos share their schema root with local test harnesses. Without a first-class exclusion those repos either can't onboard or must accept plans that propose test fixtures in production. This removes that adoption blocker declaratively, in the same config file that already defines the database.

Opened by Claude (Fable 5).

[Copilot]

Pull request overview

Adds ignore_namespaces support to schemabot.yaml so repository-only namespace directories (commonly local-test Vitess keyspaces) are excluded at the namespace-grouping choke point, preventing them from affecting plans, applies, and merge-gate checks in both the GitHub webhook flow and the CLI.

Changes:

• Extend namespace grouping to accept and apply an ignore_namespaces list (with $ENV substitution) and validate the config entries at load time.
• Wire ignore_namespaces through the GitHub schema fetch/group path and the CLI plan/apply path.
• Add integration/unit tests plus documentation for the new configuration field.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/webhook/webhook_integration_test.go	Enhances plan test fixture handling to allow multiple namespace subdirectories under schema/.
pkg/webhook/plan_integration_test.go	Adds E2E webhook plan test asserting ignored namespaces don’t appear in comments/storage.
pkg/schema/namespace.go	Updates grouping API to support ignore_namespaces and adds validation helper.
pkg/schema/namespace_test.go	Updates existing tests for new signature; adds tests for ignore behavior + validation.
pkg/github/schema.go	Threads ignore list into schema grouping; logs exclusion; improves empty-result error messaging.
pkg/github/schema_test.go	Updates grouping tests for new ignore list parameter.
pkg/github/config.go	Adds ignore_namespaces to parsed config and validates entries during fetch.
pkg/github/config_test.go	Adds YAML parse test for ignore_namespaces.
pkg/github/client_test.go	Updates grouping call for new signature.
pkg/cmd/commands/plan.go	Passes ignore list from CLI config into plan API call.
pkg/cmd/commands/onboard.go	Updates plan API call signature usage (passes nil ignores).
pkg/cmd/commands/common.go	Extends CLI config to read/validate ignore_namespaces.
pkg/cmd/commands/common_test.go	Adds CLI config load tests for ignore list parsing/validation.
pkg/cmd/commands/apply.go	Passes ignore list from CLI config into plan call used by apply.
pkg/cmd/client/client.go	Extends plan/read-schema APIs to accept ignore list and pass it into grouping.
pkg/cmd/client/client_test.go	Updates existing tests; adds ReadSchemaFiles ignore coverage.
e2e/testutil/apply.go	Updates helper call for new namespace grouping signature.
e2e/local/vitess_test.go	Updates plan API call sites for new signature (passes nil ignores).
docs/namespaces.md	Documents the new “Ignoring Namespaces” feature and rules.
docs/github-app-setup.md	Adds ignore_namespaces row to schemabot.yaml field table.

---

💡 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/1073, 3a73422.

Verdict: 9 findings — 3 blocking (on one engine shape "ignore" means "propose dropping", the config is read from the PR branch so a PR can unmanage a namespace itself, and a non-matching entry is a silent no-op that logs as a success), 4 non-blocking, 2 suggestions.

Blocking

1. On MySQL with a database-scoped target DSN, ignoring a namespace turns its live tables into DROP candidates — the exact inverse of the documented behavior. That shape routes every namespace to one Engine.Plan (local_client.go:1625), where fetchCurrentSchema loads every table in the database while desiredSchemas is built only from the post-filter req.SchemaFiles (spirit.go:412-441) — so the declarative diff emits DROP TABLE for the ignored namespace. It fails closed at apply (RequiresUnsafeOptIn() is true for any drop), but the DROP still lands in the plan, the PR comment and the merge-gate check, and an operator opting into allow_unsafe=true for an unrelated intentional drop takes the ignored namespace's tables with it. Vitess/PlanetScale and namespace-free MySQL DSNs are safe (both diff strictly per namespace); this needs either a per-engine guard or an honest doc caveat, because namespaces.md:253 currently promises "excluded everywhere… never see them".

2. ignore_namespaces is read from the PR head SHA, so a pull request can unmanage a namespace with no operator involvement — and the guard built for exactly this does not fire. Config discovery passes prInfo.HeadSHA straight into FetchConfig (config.go:482, :508), so a PR adding ignore_namespaces: [payments] removes payments from its own plan, comment and check. The repo already defends the neighbouring case — "Dropping the config must not silently unmanage a server-owned schema directory" (pull_request.go:446) — but schemaFileCoveredByConfig only checks that a config is an ancestor, and here the config still exists, so the fail-closed path is skipped. Compounded with finding 1, merging an ignore-only PR can make the next plan propose dropping that namespace's tables. Reading the ignore list from the base ref, or extending the unmanaged-dir guard to cover it, would close this.

3. An entry that matches nothing is a silent no-op, and the log and error both report it as though it worked. delete(result, ignored) on an absent key does nothing (namespace.go:91), and ValidateIgnoreNamespaces checks shape only — blank, whitespace-padded, or containing //\ — never existence, so a typo, a case mismatch (matching is exact while sibling database lookups use strings.EqualFold), or the very plausible *_test glob all leave the namespace fully reconciled. Meanwhile schema.go:88-97 logs "excluding ignored namespaces…" and can blame the ignore list in the "no schema files found… after excluding %v" error, both keyed on len(config.IgnoreNamespaces) > 0 rather than on anything actually removed. ResolveIgnoreNamespaces's own doc comment claims it returns "the namespace keys that are actually excluded" — it returns every configured entry; comparing against the pre-delete keys and warning on non-matches is a few lines.

Non-blocking

4. The base-schema freshness gate is namespace-blind, so a base commit touching only an ignored namespace blocks every open PR's apply. schema_freshness.go:104 passes schemaPaths := []string{schema.SchemaPath} — the schema root — and SchemaPathsChangedSinceMergeBase compares that directory's tree OID, which any blob under any subdirectory changes. A fixture-only merge to main therefore rejects every open PR's apply with "schema path changed on base branch after PR divergence", which is precisely the churn the feature exists to remove. Same class: HasSchemaInputFiles treats any .sql change as a schema input, so a fixture-only push re-posts a byte-identical plan comment.

5. The CLI half of the feature has no test at all — nil'ing cfg.IgnoreNamespaces at both CLI call sites leaves the whole suite green. Two tests exist but never meet: one proves the YAML parses into cfg.IgnoreNamespaces, the other proves ReadSchemaFiles filters when handed a list; nothing proves plan.go:86 and apply.go:107 pass the former to the latter. The webhook equivalent is covered — the same mutation at schema.go:84 is killed by TestE2EPlanExcludesIgnoredNamespaces — so this is an asymmetry, not an oversight of the whole feature. docs/namespaces.md:61 promises the CLI explicitly.

6. The exclusion is disclosed only in a server-side log the PR author cannot read. ic.logger.Info at schema.go:90 is the only report; SchemaRequestResult carries no field for it, so no comment, check summary, or stored plan can render it, and the new integration test pins the absence (assert.NotContains(t, body, "local_fixtures")). A reviewer looking at a green plan cannot distinguish "this namespace has no changes" from "this namespace was withheld" — which is the signal that would have made finding 2 visible.

7. onboard --force silently erases ignore_namespaces. onboard.go:188 rewrites schemabot.yaml from a two-field template (database: + type:), and checkConflicts returns early when force is set — lossless before this PR, lossy now. verifyOnboardPlan compounds it by hardcoding nil for the ignore list, so even a preserved config is verified unfiltered.

General suggestions

8. Several new branches are dead to the test suite. The entire "no schema files after ignores" block can be deleted green — both the improved message and, more seriously, the len(schemaFiles) == 0 fail-closed guard itself (mutating it to if false && … stays green). FetchConfig's new ValidateIgnoreNamespaces call is uncovered because FetchConfig has no test and the new config test hand-rolls its own decoder; the CLI twin is covered. Also unpinned: the layout-validation ordering both the code comment and the docs promise (the mixed-layout test passes nil), case-sensitivity (flipping to EqualFold survives), and the \ half of the path-separator class.

9. Three smaller ones. Filtering happens after the GitHub fetch, so an ignored namespace's files are still listed, still fetched, and still counted against the fail-closed 1000-entry Contents API cap — a large enough fixture directory fails every plan closed via ErrDirListingCapped, blocked by a namespace the config says to ignore. The CLI's empty-result error still says "no .sql files found in %s" while the webhook path got the disambiguating "after excluding ignored namespaces %v" — the two entry points now disagree about the same condition. And in a flat layout the sole namespace key is the directory's base name, so ignoring it validates fine and turns every plan into a hard error rather than a no-op.

The one thing that could have broken, verified

The PR's central claim is that GroupFilesByNamespace is the single choke point, so filtering there covers everything. For schema content it holds: there are exactly two production callers (github/schema.go:84, cmd/client/client.go:352), both wired to the real config, and the data plane never enumerates namespaces from the live target — pkg/tern and pkg/engine are driven entirely by the already-filtered SchemaFiles map, so an ignored namespace cannot re-enter through gRPC. Where it fails is everything that reasons about schema at the path level rather than the content level: the freshness gate (finding 4), the unmanaged-dir guard (finding 2), HasSchemaInputFiles, and the Contents-API listing cap (finding 9) all bypass the choke point entirely and none were threaded. And the deepest gap is that "filtered out of the desired state" and "invisible" are the same thing only for engines that diff per namespace — verified safe for Vitess/PlanetScale and namespace-free MySQL, unsafe for database-scoped MySQL (finding 1).

Verified correct

• $ENV handling is symmetric: namespace keys and ignore entries both skip substitution when environment == "", so an entry can never collapse to "" or a partial name.
• Ordering is as documented — the mixed-layout rejection precedes the deletes, so layout validation still sees ignored directories.
• Config is threaded as a parameter, never ambient; findNearestConfig resolves each file to its own nearest config, so one root's ignores cannot leak into another's grouping.
• No assertion was bent: all 11 edited test expectations in pkg/github/schema_test.go and pkg/cmd/client/client_test.go are pure signature updates adding , nil, with every require.Len/Contains value byte-identical.
• TestE2EPlanExcludesIgnoredNamespaces is a genuine end-to-end test — it killed the no-op, inverted-filter and webhook-call-site mutations and asserts both directions.
• Drops remain fail-closed at apply: RequiresUnsafeOptIn() is true for every drop, and rejectUnsafeDDLChangesWithoutOptIn refuses without allow_unsafe=true.
• make docs-toc produces no diff in docs/namespaces.md; the agent-authorship line is at the bottom of the PR body, which is what this repo's AGENTS.md requires.
• go build, go vet, and the touched-package suites all pass; CI fully green across every E2E, Vitess, MySQL, gRPC, K8s and integration job.

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

[aparajon]

🤖 All findings addressed in de5c018 (plus 3a73422 earlier for the CLI apply-gate fix):

• 1 (DROP hazard on database-scoped MySQL DSNs) — fail-closed guard: the excluded namespaces now travel with the plan request (CLI and webhook → proto), and planWithEngine refuses ignore_namespaces on a DSN that names a database, with the DROP rationale in the error. Documented in namespaces.md.
• 2 + 6 (self-unmanage invisible / disclosure only in server logs) — kept head-ref semantics, made the exclusion visible instead: the plan comment renders an "excluded by ignore_namespaces" line on changes, no-changes, and multi-env paths (envs with differing exclusions no longer dedupe into one section), and the CLI prints the same disclosure for plan/apply. New TEMPLATES.md scenario.
• 3 (resolved-vs-actual conflation) — GroupFilesByNamespace now returns the namespaces actually removed; logs/comments report those, and UnmatchedIgnoreEntries drives a webhook warn + CLI warning for entries that matched nothing.
• 5 + 8 (coverage) — CLI wiring test (httptest asserting ignored_namespaces in the request body and the namespace absent from schema_files), case-sensitivity and \-separator pins, guard unit test, empty-after-ignore webhook e2e, onboard preservation tests.
• 7 (onboard --force erases the config) — preservedIgnoreNamespaces carries an existing config's entries through rewrites, and verification plans with them.
• 9 (three smaller) — CLI error parity ("after excluding ignored namespaces") and the flat-layout sole-namespace case now fails with that error; the pre-fetch/Contents-cap half is deferred.

Deferred as tracked follow-ups rather than folded in here: finding 4 (namespace-scoped freshness gate) and finding 9's pre-fetch filtering.

Reply by Claude (Fable 5).