Skip to content

feat(cli): add a first class operator command to apply SchemaBot's own schema changes - #1391

Closed
aparajon wants to merge 5 commits into
mainfrom
armand/storage-diff-apply-commands
Closed

aparajon wants to merge 5 commits into
mainfrom
armand/storage-diff-apply-commands

Conversation

@aparajon

@aparajon aparajon commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Adds two operator commands for SchemaBot's own storage schema:

schemabot storage diff  --embedded|--release <tag>|--schema-dir <path>   # what is outstanding, read-only
schemabot storage apply [--embedded]                                     # converge it, with confirmation

Both take [--deployment <d> -e <env>] to reach a data plane's own storage.

Today that question is answered by checking out a release tag, extracting the embedded schema files by hand, and anti-joining the expected (table, column) and (table, index) pairs against information_schema.

diff always reads the live storage database. It never infers from a version pin: a release tag says what that release would converge to, not what the storage converged to, and the two diverge exactly when a deploy has failed, which is the only time anyone asks.

apply is the startup bootstrap, called unchanged: the same differ, the same destructive-statement refusal, and the same advisory lock, so two operators running it at once serialize the way two booting pods do. A pre-deploy convergence job is this command with nothing added.

The two sides of the diff

The live side is a read of one database. The desired side is schema files, and which files is the operator's to name — because the question a deploy asks is not "does this storage match the binary that is running" but "does this storage match the release I am about to roll", and the running binary cannot answer that from files it does not carry.

Naming it is required. There is no default: a diff whose desired side the operator did not choose is not a weaker answer, it is an unusable one. The same storage is converged against the release that is running and short of the release about to roll, and an operator who assumed the wrong one of those either rolls into a failing bootstrap or converges storage they did not mean to touch. Naming more than one selector is refused too, rather than resolved by precedence.

schemabot storage diff --deployment west -e production --release v1.4.0

  desired side: schema files              live side: the database
  ┌────────────────────────────────┐      ┌────────────────────────────────┐
  │ pkg/schema/mysql/*.sql at tag  │      │ the catalog of the storage     │
  │ v1.4.0, fetched by the CLI     │      │ database west owns, read by    │
  │ over the contents API          │      │ west itself: no DDL, no lock   │
  └───────────────┬────────────────┘      └───────────────┬────────────────┘
                  │                                       │
                  └───────────────────┬───────────────────┘
                                      ▼
                       the differ a boot of west runs
                                      │
                                      ▼
                  the statements, and the line saying which
                  database and which schema they came from
Desired schema Where the files come from
--embedded the files compiled into the binary that answers the request
--schema-dir <path> that directory's .sql files, read by the CLI
--release <tag> that tag's pkg/schema/<dialect>/ files, fetched by the CLI

--release learns the dialect by first asking the target — one extra read-only diff, paid only by this flag — so a PostgreSQL storage is never diffed against MySQL DDL published alongside it. --release-repo points at a fork or mirror, GITHUB_API_URL/GITHUB_TOKEN are honored, and a repository the CLI cannot read is an error naming the token to set and --schema-dir as the offline alternative. Naming both selectors is refused rather than resolved by precedence. Because the storage schema is declarative, one diff against the release being rolled to covers however many releases lie between.

apply takes neither file selector, by construction. The apply RPC has no schema field, ApplyStorageSchema has no source parameter, and the CLI accepts the two flags only to refuse them with the two real ways to converge a release. It does accept --embedded, which names what it already does, so an operator moving from a diff to an apply can carry the flag over and have it mean what it said. A convergence runs the schema embedded in the binary running it, so that it does exactly what that binary's next boot would do — which is what keeps AV-9 from turning into "an older binary can be handed newer schema to destroy".

The report names both ends, so two correct reports about the same database are tellable apart, and a schema the caller supplied is never relabelled as the answering binary's own:

$ schemabot storage diff --deployment west -e production --embedded
schemabot on db-1.example (mysql), deployment west in production is converged, against the schema embedded in v1.2.3.

$ schemabot storage diff --deployment west -e production --release v1.4.0
schemabot on db-1.example (mysql), deployment west in production needs 1 statement: 1 outstanding, against the schema files of release v1.4.0 in block/schemabot.

Reaching a data plane's storage

--deployment routes through the control plane rather than dialing the storage directly, because a data plane owns its own storage database and generally sits where a workstation cannot reach it.

schemabot storage diff --deployment west -e production
                      │
                      │ POST /api/storage/schema/diff
                      │   {"deployment":"west","environment":"production"}
                      ▼
┌──────────────────────────────────────────┐
│ control plane                            │
│                                          │
│   write-tier admin gate (AZ-2)           │
│   tern_deployments[west][production]     │
│     unconfigured ──▶ error naming it     │
│     in-process   ──▶ error naming it     │
└─────────────────────┬────────────────────┘
                      │ gRPC StorageSchemaDiff
                      │   (the request carries no target)
                      ▼
┌──────────────────────────────────────────┐
│ data plane "west"                        │
│                                          │
│   diffs the desired schema against its   │
│   own live storage database, with        │
│   catalog reads only: no DDL, no lock    │
└─────────────────────┬────────────────────┘
                      │
                      ▼
  the outstanding statements, stamped with the
  deployment and environment the operator asked for

Nothing falls back from one path to the other. An unconfigured deployment, or one whose route resolves to an in-process client, is an error naming the deployment and the config key it was looked up under, never a report about the control plane's own storage under the deployment's name. The endpoint is resolved from tern_deployments before the client is, so a deployment that shares a name with a local database cannot silently localize.

The direct path stays for the case the API cannot serve: the server is down, including down because its own schema bootstrap is failing.

Target Reads
no flags the storage of the server the CLI is pointed at
--deployment <name> -e <env> that data plane's own storage, over the gRPC connection that already exists
--dsn / --config the storage database this workstation opens itself

The two paths are mutually exclusive at the flag layer: --deployment with --dsn is refused rather than resolved by precedence, since dropping one would report the wrong database under the right name.

Authorization

Both routes are admin-only and both are admitted at the write tier, the read-only diff included: on a deployment configured with read and write groups only, the tier is the whole admin decision, so a read-tier diff would expose the internal shape of SchemaBot's bookkeeping database to everyone with read access.

The diff is a POST — it carries the desired schema files — so it takes the write tier by the default "anything not classified as a read is a write" rule. That retires the write-tier GET exception the tier classification carried: writePaths and its coverage test are deleted, and the remaining readPaths is the only list. Both routes appear in the route authorization sweep with a test that a scoped operator gets 403 and that neither route touches storage on the way to the denial.

A caller-supplied schema is validated before anything reads a database, because the failure it prevents is silent: a half-read file set diffs cleanly and reports the tables it is missing as surplus, which is a report full of statements that destroy the schema the operator was about to extend. Files with no description, a description with no files, path-like names, non-.sql names, empty contents, and an empty set are all refused.

Invariants

  • AV-9 (SchemaBot never destroys its own storage to start) — extends the enforcement to an operator-invoked convergence. ApplyStorageSchema calls EnsureSchema; the refusal still lives in the per-dialect bootstrappers, and the entry's rule and *Enforced:* line now say so. The convergence is also structurally unable to take a caller's schema, so the operator surface cannot hand an older binary newer schema to prune. A request may only widen the deployment's standing destructive policy, never narrow it: a deployment that already allows destructive storage changes converges the way its next boot would, so "apply is what a boot does" keeps holding there.
  • AZ-2 (authorization fails closed at every tier) — upholds, and simplifies the enforcement: with the write-tier GET exception gone, every route on this surface is classified by the default rule. New mutating endpoints, classified, swept, and tested for denial.
  • AV-10 (anything the PR can do, the CLI can do) — upholds. The new surface is CLI-first and the HTTP API is the same call.
  • UX-4 (a refusal says what to do next) — upholds. Every refusal names the deployment, the config key, or the flag that resolves it; a diff that finds work prints the storage apply command that converges it, without echoing a DSN that may carry credentials, and a diff against a named release deliberately does not print storage apply — it names the two ways to converge that release instead.

Both storage dialects are supported through the bootstrap's own per-dialect dispatch, and a dialect without a differ fails closed naming itself rather than running another family's catalog queries.

Exit status

diff exits 0 when converged, 2 when statements are outstanding, and 1 when the read itself failed. A pre-deploy gate needs those three apart: "converged" and "unreachable" call for opposite decisions, and a command that collapsed them would be unusable unattended.

Converged
$ schemabot storage diff --embedded
schemabot on db-1.example (mysql) is converged, against the schema embedded in v1.2.3.
$ echo $?
0
Statements outstanding
$ schemabot storage diff --embedded --deployment west -e production
schemabot on db-1.example (mysql), deployment west in production needs 3 statements: 3 outstanding, against the schema embedded in v1.2.3.

Outstanding, and run automatically on the next boot or apply (3):

ALTER TABLE `applies` ADD COLUMN `driver_note` varchar(255) NOT NULL DEFAULT '' AFTER `lease_owner`;
ALTER TABLE `checks` ADD COLUMN `blocked_reason` varchar(64) NOT NULL DEFAULT '' AFTER `state`;
CREATE TABLE `check_gate_audit` (
  `id` BIGINT UNSIGNED AUTO_INCREMENT,
  `check_id` BIGINT UNSIGNED NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;

Converge it with: schemabot storage apply --deployment west -e production
$ echo $?
2
Outstanding against a release the target is not running
$ schemabot storage diff --deployment west -e production --release v1.4.0
schemabot on db-1.example (mysql), deployment west in production needs 1 statement: 1 outstanding, against the schema files of release v1.4.0 in block/schemabot.

Outstanding, and run automatically on the next boot or apply (1):

ALTER TABLE `applies` ADD COLUMN `driver_note` varchar(255) NOT NULL DEFAULT '' AFTER `lease_owner`;

These are what schemabot on db-1.example (mysql), deployment west in production needs in order to match the schema files of release v1.4.0 in block/schemabot, not what its own next boot would run. To converge them, run that release's binary against this database — its container image is that release — or let the release's first boot converge them.
$ echo $?
2
Destructive statements refused
$ schemabot storage diff --embedded
schemabot on db-1.example (mysql) needs 1 statement: 1 destructive, against the schema embedded in v1.2.3.

Destructive, and refused; surplus state stays in place (1):

-- check_gate_audit: DROP TABLE destroys data
DROP TABLE `check_gate_audit`;

Converge it with: schemabot storage apply
$ echo $?
2
Converging, with confirmation
$ schemabot storage apply
schemabot on db-1.example (mysql) needs 1 statement: 1 outstanding, against the schema embedded in v1.2.3.

Outstanding, and run automatically on the next boot or apply (1):

ALTER TABLE `applies` ADD COLUMN `driver_note` varchar(255) NOT NULL DEFAULT '' AFTER `lease_owner`;

Run these statements against schemabot on db-1.example (mysql)? Only 'yes' will be accepted: yes
Ran 1 statement against schemabot on db-1.example (mysql).
schemabot on db-1.example (mysql) is converged.
No schema named, refused
$ schemabot storage diff --deployment west -e production
Error: missing flags: --embedded or --schema-dir=STRING or --release=STRING
$ echo $?
1
A release selector on `apply`, refused
$ schemabot storage apply --deployment west -e production --release v1.4.0
Error: --release cannot be used with a convergence: an apply runs the schema embedded in the binary running it, so that it converges exactly what that binary's next boot would. To converge a release's schema, run that release's binary — its container image is that release — or let the release's own first boot converge it. To see what it would do, use the same flag on `storage diff`
Needs manual remediation (PostgreSQL)
$ schemabot storage apply
schemabot on db-2.example (postgres) needs 1 statement: 1 needing manual remediation, against the schema embedded in v1.2.3.

Needs manual remediation before anything converges (1):

-- checks: column is NOT NULL without a DEFAULT
ALTER TABLE "checks" ADD COLUMN "head_sha" varchar(64) NOT NULL;

Error: refusing to converge storage schema on schemabot on db-2.example (postgres): 1 change(s) need manual remediation first (listed above)

Docs: a first-class guide

How SchemaBot's own storage schema changes is critical operator knowledge, and it was scattered across a configuration reference. This adds docs/storage-schema.md, an operator guide covering both paths a storage schema change can take:

the startup path          the CLI path
(every boot)              (an operator)
       │                        │
       └───────────┬────────────┘
                   ▼
              EnsureSchema
  the same differ, refusal, and lock

It covers, in the order an operator needs it: how a change reaches the database and why storage apply is what a boot does; what each dialect converges by itself; what is never automatic and why (including the asymmetry that bites — a pre-created table or column survives a boot of the earlier release, but an index does not, since dropping one destroys no data and so is not refused); the two commands; the deploy sequence; what to do when a pod will not start; the indexes worth pre-creating on a long-lived database; and what adding a storage schema change obliges a contributor to do.

The operator narrative moves out of configuration.md, which keeps allow_destructive_schema_changes and points at the guide. README.md and cli.md link it, release.md points at the command from the pre-tag storage schema check, and auth.md records why the read-only route sits at the write tier.


This PR was prepared by Claude Code (Claude Opus 5).

aparajon and others added 3 commits September 11, 2026 14:25
Adds `schemabot storage diff` and `schemabot storage apply` for SchemaBot's
own storage database. The diff reads the live database and compares it against
the embedded schema files of the binary that answers, never against a version
pin: a release tag says what that release would converge to, not what the
storage converged to, and the two diverge exactly when a deploy has failed.

`--deployment <d> -e <env>` routes through the control plane to the data
plane's own gRPC endpoint, because a data plane owns its storage database and
generally sits where a workstation cannot reach it. The direct `--dsn` /
`--config` path stays for when the server is down, including down because its
own bootstrap is failing. Nothing falls back between the two: an unreachable
deployment is an error naming it, never a report about another database.

Apply is the startup bootstrap called unchanged — same differ, same
destructive-statement refusal, same advisory lock — so it extends AV-9 to an
operator-invoked convergence rather than deciding policy again. Both routes are
admin-only and both sit at the write tier, the read-only diff included (AZ-2).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The commands take no schema directory and no flag to override one: the files
are embedded in whichever binary answers the request. That is the thing to keep
straight when deploying, because through the API the answer describes the
release currently running, which before a roll is the old one.

Documents the deploy sequence around a storage schema change, and the asymmetry
an operator pre-creating an index has to plan around: a table or column created
ahead of the roll survives a boot of the earlier release because dropping one is
refused as destructive, while an index does not, since dropping an index
destroys no data and falls outside that refusal. A storage schema integration
test pins it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A deploy asks whether the storage is ready for the release being rolled,
which the running binary cannot answer from files it does not carry. The
diff's desired side is now selectable: --schema-dir reads a checkout,
--release fetches a tag's schema files for the dialect the live storage
runs. The live side is still always a read of the database.

The report names both ends — the database and host it read, and the schema
it compared against — so two correct reports about the same database are
tellable apart, and a caller-supplied schema is never relabelled as the
answering binary's own.

`storage apply` takes neither selector: a convergence runs the schema
embedded in the binary running it, so it does exactly what that binary's
next boot would do (AV-9). The flags are refused with the two real ways to
converge a release rather than with an unknown-flag error.

The diff route moves to POST so the schema files can travel on it, which
also retires the write-tier GET exception in the tier classification —
both storage routes now take the write tier by the default rule (AZ-2).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 18:32
`storage diff` had no flag for the common case: with none given it compared
against the schema embedded in whichever binary answered. That default is
the one thing about this command an operator has to know, and it was the
one thing the command line did not say. Name it — `--embedded` for the
answering binary's own schema, `--release <tag>` or `--schema-dir <path>`
for another release's — or the command refuses before it reads anything.

`storage apply` accepts `--embedded` and still refuses the other two: a
convergence runs the schema of the binary running it (AV-9), so there is
nothing else for it to converge.

Adds docs/storage-schema.md, the operator guide: how a storage schema
change reaches the database through either path — every boot's EnsureSchema
and an operator's `storage apply`, which are the same code — what each
dialect converges by itself, what it refuses and why, the deploy and
crashloop playbooks, and the indexes worth pre-creating on a long-lived
database. The operator narrative moves out of the configuration reference,
which keeps the settings and points at the guide.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings remain in convergence safety, configuration parity, and CLI/API behavior.

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

Pull request overview

Adds CLI, HTTP, and gRPC workflows for inspecting and converging SchemaBot’s storage schema across direct and routed deployments.

Changes:

  • Adds storage diff and storage apply commands with schema sources, confirmation, JSON output, and exit statuses.
  • Adds routed storage-schema APIs, authorization, validation, and generated protocol bindings.
  • Reuses bootstrap behavior across MySQL/PostgreSQL with tests, helpers, and operator documentation.
File summaries
File Reviewed change
pkg/testutil/postgres.go PostgreSQL test column helper.
pkg/testutil/container.go MySQL test container/index helper.
pkg/tern/storage_schema.go Storage schema service contract.
pkg/tern/server.go gRPC storage schema server methods.
pkg/tern/grpc_client.go gRPC client forwarding.
pkg/serve/storage_schema.go Local storage schema service adapter.
pkg/serve/storage_schema_test.go Service configuration tests.
pkg/serve/serve.go Service wiring and registration.
pkg/serve/serve_build_test.go Serve build coverage.
pkg/proto/ternv1/tern.pb.gw.go Generated HTTP gateway bindings.
pkg/proto/ternv1/tern_grpc.pb.go Generated gRPC bindings.
pkg/proto/tern.proto Storage schema RPC definitions.
pkg/cmd/main.go CLI integration.
pkg/cmd/commands/storage.go Storage command registration and target handling.
pkg/cmd/commands/storage_schema_test.go CLI workflow and output tests.
pkg/cmd/commands/storage_schema_source.go Schema source and release loading.
pkg/cmd/commands/storage_schema_source_test.go Schema source tests.
pkg/cmd/commands/exit_code.go Storage diff exit statuses.
pkg/cmd/client/request.go Operator request support.
pkg/cmd/client/client.go Storage schema API client.
pkg/auth/tiers.go Route tier classification.
pkg/auth/tiers_test.go Authorization tier tests.
pkg/apitypes/storage_schema.go API report and payload types.
pkg/api/storage_schema_test.go API schema behavior tests.
pkg/api/storage_schema_source.go API schema source loading.
pkg/api/storage_schema_source_test.go API source validation tests.
pkg/api/storage_schema_integration_test.go Dialect integration tests.
pkg/api/storage_schema_handlers.go HTTP handlers and routing.
pkg/api/storage_schema_handlers_test.go Handler and routing tests.
pkg/api/service.go Route registration.
pkg/api/route_authorization_sweep_test.go Route authorization coverage.
pkg/api/ensure_schema.go Shared schema bootstrap behavior.
docs/release.md Release workflow documentation.
docs/invariants.md Invariant enforcement updates.
docs/configuration.md Storage operation documentation.
docs/auth.md Storage authorization documentation.
Review details

Suppressed comments (16)

pkg/api/storage_schema.go:190

  • DiffStorageSchema can report no statements while EnsureSchema would still find and clean stale Spirit internal tables such as _..._new and _..._checkpoint under the advisory lock. Returning early on planned.Converged() means storage apply leaves those interrupted-apply artifacts behind, so it does not execute the same startup bootstrap promised by AV-9. Include stale-table state in the plan or avoid this early return when cleanup is pending.
	if planned.Converged() {
		logger.Info("storage schema already converged; nothing to apply",
			"dialect", planned.Dialect, "database", planned.Database)
		return planned, planned, nil

pkg/api/storage_schema_handlers.go:220

  • The local-target path can invoke the handler with the storage service in-process, so no gRPC boundary sanitizes err; storage-driver and connection failures are returned verbatim in the HTTP response. The raw error is already logged above, so return a fixed message here (as the gRPC server does) to avoid exposing infrastructure details.
		s.writeError(w, http.StatusInternalServerError, fmt.Sprintf("storage schema apply failed: %v", err))

pkg/apitypes/storage_schema.go:20

  • The public report type is documented as always describing the instance's embedded schema, but SchemaSource and the diff API allow a directory or release's files to be the desired side. Consumers reading this type's documentation can therefore misinterpret a valid release comparison; describe the report as matching the named SchemaSource instead.
// StorageSchemaReport is what one SchemaBot instance's storage database needs
// in order to match that instance's embedded schema.
//
// The three statement lists are disjoint and have different dispositions:
// Outstanding runs, Destructive is refused unless destructive changes are
// allowed, and any Manual entry blocks the whole convergence.

pkg/cmd/commands/storage.go:322

  • strings.Contains treats these substrings as libpq keywords even when they occur inside a valid Go MySQL DSN username, password, or parameter value. For example, a MySQL password containing host= is classified as PostgreSQL and rejected unless --dialect is supplied. Detect actual keyword/value tokens with a structural parser rather than scanning the whole DSN for substrings.
	for _, keyword := range []string{"host=", "hostaddr=", "dbname=", "port=", "user=", "sslmode="} {
		if strings.Contains(lowered, keyword) {
			return true

pkg/cmd/commands/storage.go:201

  • On the --config path this target drops the config's effective storage policy and PostgreSQL statement budget, so direct diff/apply only use the CLI flag and the default timeout. A config with allow_destructive_schema_changes: true will therefore be reported/refused by the command even though the next boot would run those statements, and custom PostgreSQL statement timeouts are not honored. Carry these options through to both direct paths so operator convergence preserves AV-9's apply-is-boot behavior.
type storageTarget struct {
	dsn     string
	dialect schema.Dialect
	source  string

pkg/cmd/commands/storage_schema.go:324

  • For a PostgreSQL target selected through --config, this direct path also drops cfg.Postgres.StatementTimeout and falls back to the API default. A direct apply can therefore fail or wait differently from the configured startup bootstrap; propagate the configured timeout through the resolved target options and use it for the preview too.
		target, err := resolveStorageTarget(cmd.DSN, cmd.Config, cmd.Dialect)
		if err != nil {
			return nil, nil, err

pkg/cmd/commands/storage_schema.go:163

  • Unlike the routed adapter, the direct diff does not apply StorageSchemaDiffTimeout here. A blocked MySQL catalog or metadata-lock read can therefore keep the CLI running without the 30-second bound documented by DiffStorageSchema; create a timeout context before this call.
		"source", target.source, "dialect", target.dialect, "schema_source", desired.Describe())
	report, err := api.DiffStorageSchema(ctx, target.dsn, desired, logger,
		api.WithDialect(target.dialect),

pkg/cmd/commands/storage_schema.go:509

  • When the diff was run with --allow-destructive, this hint still prints an apply command without that flag. Following the suggested command then refuses the destructive statements that the report says are permitted, so include the opt-in when constructing the hint for each target form.
	if cmd.suppliesFiles() {
		// Naming `storage apply` here would be wrong: it converges the schema
		// of the binary that answers, which is not the schema this report is
		// about. The two ways to converge the release's schema are the release
		// itself, and this is where an operator is about to look for them.

pkg/cmd/commands/storage_schema.go:60

  • direct() trims the values before deciding which path is selected. With --dsn ' ' plus a valid deployment (or --config ' '), validate therefore treats the command as API mode and read can report a different database instead of rejecting the explicitly supplied direct source. Decide the path from whether the raw flag is non-empty, then let resolveStorageTarget report the blank-source error.
func (f *storageSchemaTargetFlags) direct() bool {
	return strings.TrimSpace(f.DSN) != "" || strings.TrimSpace(f.Config) != ""
}

pkg/cmd/commands/storage_schema.go:163

  • When the direct target comes from --config, resolveStorageTarget has already loaded StorageConfig.AllowDestructiveSchemaChanges, but this call drops that policy and uses only the CLI flag. A config with destructive storage changes enabled therefore makes storage diff --config ... report them as refused even though the next boot and the corresponding apply would run them, violating the documented policy-widening behavior and AV-9. Carry the config policy through the resolved target and OR it with the request flag.
		"source", target.source, "dialect", target.dialect, "schema_source", desired.Describe())
	report, err := api.DiffStorageSchema(ctx, target.dsn, desired, logger,
		api.WithDialect(target.dialect),

pkg/cmd/commands/storage_schema.go:274

  • --json is advertised as machine-readable, but a non-auto-approved apply writes the human preview to stdout first; if the preview is already converged it returns before emitting JSON, and if confirmed it concatenates text and JSON. This makes the result unusable for parsers. Reject this flag combination or send all interactive preview/prompt output to stderr and emit exactly one JSON document.

	// Confirm against a fresh read rather than against a description of the
	// command: an operator approving DDL on SchemaBot's own storage should see
	// the statements, and the read is free of side effects. With --auto-approve
	// the convergence's own planned report says what ran, so there is nothing
	// this preview would add.
	if !cmd.AutoApprove {
		preview := &StorageDiffCmd{
			storageSchemaTargetFlags: cmd.storageSchemaTargetFlags,
			storageSchemaSourceFlags: storageSchemaSourceFlags{Embedded: true},

pkg/cmd/commands/storage_schema.go:329

  • The direct apply path stamps only Version, leaving SchemaSource as the generic "the schema embedded in this binary". The API path and direct diff call AttributeTo, so the same binary reports a precise embedded version everywhere else. Use the same attribution here so direct apply JSON and follow-up output identify the exact schema source.
			"source", target.source, "dialect", target.dialect, "allow_destructive", cmd.AllowDestructive)
		plannedReport, remainingReport, err := api.ApplyStorageSchema(ctx, target.dsn, logger,

pkg/cmd/commands/storage_schema_source.go:228

  • http.Client.Timeout applies to each request, not to the listing plus the loop that fetches every file. A repository that takes 30 seconds per response can therefore keep this command busy for roughly 30 seconds per file (up to the 256-file limit), despite the comment promising a whole-fetch bound. Put one context.WithTimeout around the listing and all file fetches so the complete release read is bounded.
	// storageSchemaFetchTimeout bounds the whole fetch. A diff is run at a
	// terminal during a deploy, so a repository that does not answer has to
	// fail rather than hang: the operator still has --schema-dir, which needs
	// no network at all.
	storageSchemaFetchTimeout = 30 * time.Second
	// storageSchemaMaxFileBytes and storageSchemaMaxFiles bound what a fetch
	// will read. The storage schema is a few dozen small files; anything past
	// these bounds means the path being fetched is not a storage schema

pkg/proto/tern.proto:234

  • This RPC comment says the answer always comes from the serving binary's embedded files, but StorageSchemaDiffRequest also intentionally accepts caller-supplied release files and the implementation uses them. That makes the public proto documentation contradict the release-diff behavior; document the embedded schema as the default and the request files as the override.
    pkg/serve/storage_schema.go:119
  • Both EnsureSchema bootstrappers replace their input with a context.Background()-based timeout, so the caller context passed here cannot cut the convergence short despite the comment above. A disconnected or canceled API/CLI request can therefore continue executing storage DDL for the full bootstrap timeout while the operator believes it stopped. Thread cancellation through the bootstrap or make the operation explicitly asynchronous instead of claiming request cancellation is honored.
    pkg/tern/storage_schema.go:27
  • This interface comment says every diff comes from the serving binary's embedded files, but the same RPC deliberately accepts SchemaFiles for comparing a deployment's storage with a release the data plane is not running. State that embedded files are the default and request files replace them for a read-only comparison; otherwise the public service contract contradicts its request type.
  • Files reviewed: 43/43 changed files
  • Comments generated: 5
  • Review effort level: Lite

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

Comment thread pkg/cmd/commands/storage_schema.go
Comment thread pkg/api/storage_schema.go
Comment thread pkg/api/storage_schema_handlers.go
Comment thread pkg/cmd/commands/storage_schema.go
Comment thread pkg/cmd/commands/storage_schema.go
@aparajon aparajon changed the title feat(cli): ask what storage DDL is outstanding, and converge it feat(cli): say what storage DDL a release still needs, and converge it Sep 11, 2026
@aparajon aparajon changed the title feat(cli): say what storage DDL a release still needs, and converge it feat(cli): diff and converge SchemaBot's own storage schema against any release Sep 11, 2026
The refusal when no desired schema is named is now one line naming the
three flags, and the parser refuses two selectors at once. The reasoning
for having no default lives in the flag help and in the storage schema
guide, where an operator reads it before they are blocked by it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@aparajon aparajon changed the title feat(cli): diff and converge SchemaBot's own storage schema against any release feat(cli): add a first class operator command to apply SchemaBot's own schema changes Sep 11, 2026
@aparajon aparajon closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants