feat(pg-delta): add statement-level debugging to schema apply#343
feat(pg-delta): add statement-level debugging to schema apply#343avallete wants to merge 11 commits into
Conversation
…ging Regression coverage for statement-level debugging in `schema apply`: apply()'s new onEvent observer (tests/apply-observer.test.ts) and the `schema apply --dry-run`/`--verbose`/`--out-plan` CLI flags (tests/cli.test.ts, "CLI: schema apply debugging"). Both fail today — onEvent does not exist on ApplyOptions yet, and the three flags are unrecognized by parseFlags. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
…d an apply onEvent observer
`schema apply` today only prints action counts; the SQL it actually runs is
visible only when a statement fails. Since the executed statements are
planner-rendered atomic DDL derived from the catalog diff — not the authored
declarative SQL — there was no way to inspect what would run, or watch it run,
without instrumenting the process externally.
Library (src/apply/apply.ts): ApplyOptions gains an optional `onEvent`
observer fired at segment/action boundaries (ApplyEvent: segmentStart,
actionStart, actionEnd, segmentEnd, and control — the latter covering every
non-action statement sent on the wire: BEGIN, preamble SET/SET LOCAL, COMMIT,
ROLLBACK, RESET ALL). Every emission is wrapped in try/catch so a throwing
observer can never change apply()'s control flow, action statuses, or the
returned report — purely additive.
CLI (src/cli/commands/schema.ts, src/cli/main.ts): `schema apply` gains:
--dry-run print the exact executable script (via the `render` command's
renderer, so it splits on the same segment boundaries apply()
executes) to stdout instead of applying; no fingerprint gate
runs. A stderr summary reports the action count and flags
destructive actions.
--verbose stream the onEvent trace to stderr during a real apply:
segment start/end, each action's SQL and ok/FAILED outcome
with timing, and every control statement (prefixed " ; ").
--out-plan write the plan artifact right after planning, before apply
(or the --dry-run script).
RED (tests/apply-observer.test.ts, tests/cli.test.ts "CLI: schema apply
debugging", captured before this commit):
error: expect(received).toBeGreaterThan(expected)
Expected: > 0
Received: 0
(fail) apply() onEvent observer > happy path: segmentStart, actionStart/
actionEnd(ok:true) pairs in plan order, segmentEnd committed
error: expect(received).toMatchObject(expected)
{ - "code": 0, + "code": 2, + "stderr": "Unknown flag: --dry-run ..." }
(fail) CLI: schema apply debugging > --dry-run prints the executable
script to stdout and applies nothing
(fail) CLI: schema apply debugging > --dry-run --out-plan writes a plan
artifact that parses with actions
(fail) CLI: schema apply debugging > --verbose logs per-statement progress
to stderr during a real apply
(a follow-up spec addendum requiring `control` events for BEGIN/preamble
SET/COMMIT also went RED first: "Expected: true / Received: false" on
`controlEvents.some((e) => e.sql === "BEGIN")`.)
GREEN: all of the above pass, plus `bun test src/` (710 pass) and the full
existing tests/cli.test.ts suite (44 pass) show no regressions.
Changeset: .changeset/schema-apply-statement-debugging.md (minor).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
…path Review findings 1-3 on the schema-apply debugging feature: the trace lies exactly on the CREATE INDEX CONCURRENTLY path it exists to debug. RED (against 1cfcfbc): - success path: preamble SET is emitted AFTER actionStart expect(pos).toBeLessThan(actionStartPos) error: expect(received).toBeLessThan(expected) - failure path: control(RESET ALL) is emitted AFTER segmentEnd Expected: "segmentEnd" Received: "control" - preamble failure: action events emitted for an action that never ran expect(events.some((e) => e.kind === "actionStart")).toBe(false) Expected: false Received: true Also replaces the dead interleaving assertion in the happy-path test with real actionStart/actionEnd pairing checks (finding 3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
… non-transactional path Review follow-ups on the schema-apply debugging feature (findings 1-7): - non-transactional segments now emit preamble control events BEFORE actionStart, emit actionEnd as the action settles (so ms measures only the action's round-trip, not RESET ALL), emit NO action events when the preamble itself fails, and always emit segmentEnd as the segment's terminal event (after the RESET ALL control) on success and failure. The ApplyReport is unchanged on every path. - schema apply --dry-run now warns on stderr when --unsafe-show-secrets is set (the script is as persistent as the --out-plan artifact). - destructive-action predicate shared via isDestructiveAction() in cli/render.ts instead of being duplicated in the --dry-run summary. - docs no longer overstate --dry-run as 'that exact script' (transaction framing is shown by --verbose only) and note that --verbose has no effect under --dry-run. - cli.test.ts pins the ' ; ' control-line prefix for BEGIN/COMMIT. RED (captured before this fix, tests in the previous commit): expect(pos).toBeLessThan(actionStartPos) // SET emitted after actionStart Expected: "segmentEnd" Received: "control" // RESET ALL after segmentEnd Expected: false Received: true // action events without an action Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
…vent export Final-review findings N1/N2 on the schema-apply debugging feature. RED (against 43fac76): - a dir exported with --unsafe-show-secrets (manifest redactSecrets:false) re-applied WITHOUT the flag gets no UNREDACTED warning on --dry-run / --out-plan: expect(warnings.length).toBe(2) Expected: 2 Received: 0 - ApplyEvent is advertised as library API by the changeset but not exported from the package entry: tests/apply-observer.test.ts(16,15): error TS2305: Module '"../src/index.ts"' has no exported member 'ApplyEvent'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
…and export ApplyEvent Final-review findings N1/N2: - the --dry-run / --out-plan UNREDACTED warnings now gate on the effective redaction mode (manifest?.redactSecrets ?? !flag) instead of the flag alone, so a dir exported with --unsafe-show-secrets warns when re-applied without the flag, and a redacted-manifest dir no longer false-warns. - ApplyEvent is exported from the package entry (src/index.ts), matching the changeset's advertised library API. RED (captured before this fix, tests in the previous commit): expect(warnings.length).toBe(2) — Expected: 2 Received: 0 TS2305: Module '"../src/index.ts"' has no exported member 'ApplyEvent'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
🦋 Changeset detectedLatest commit: 96c9502 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
avallete
changed the title
schema apply…lan-y2k6hg Base refactored the CLI (commands throw CliExit instead of process.exit; schema apply planning moved into the planSchemaFiles frontend; renderPlan moved to frontends/render-plan-files.ts with cli/render.ts as a re-export shim). Resolution: - schema.ts: dropped the now-unused planning imports (plan/resolveView/ projectManagementScope) superseded by planSchemaFiles; kept the feature's serializePlan/ApplyEvent/renderPlan/isDestructiveAction imports and the --dry-run/--verbose/--out-plan usage line. The feature blocks auto-merged into the refactored body (out-plan + dry-run after planning, onEvent on the apply call) with redactSecrets still the manifest-aware const. - render.ts: took the base's shim and moved isDestructiveAction into the canonical frontends/render-plan-files.ts (used by the render gate there), re-exported via the shim and both public barrels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b42ee070cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…jection safety Codex review findings on PR #343 (both P2, verified real). RED (against 2ac8de7): - the --out-plan artifact omits the redaction mode it was fingerprinted under (apply --plan then re-extracts redacted and trips the gate): expect(parsed.redactSecrets).toBe(true) Expected: true Received: undefined expect(parsed.redactSecrets).toBe(false) Expected: false Received: undefined - an async onEvent observer's rejection escapes emit()'s synchronous try/catch as an unhandled rejection: error: async observer boom # Unhandled error between tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
… async observer rejections Codex review findings on PR #343: - planSchemaFiles now passes the effective redactSecrets into plan(), so a plan archived via 'schema apply --out-plan' records the mode its fingerprint was taken under and 'apply --plan' re-extracts in that same mode — an unstamped unredacted plan read as redacted and the fingerprint gate spuriously rejected an unchanged target. - emit() consumes a thenable returned by the onEvent observer (TypeScript allows async functions for void callbacks), so an async observer's rejection can no longer escape as an unhandled rejection and take down the process mid-apply. RED (captured before this fix, tests in the previous commit): expect(parsed.redactSecrets).toBe(false) Expected: false Received: undefined error: async observer boom / # Unhandled error between tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c27dc3a535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "actionStart": | ||
| process.stderr.write( | ||
| `[${event.actionIndex + 1}/${thePlan.actions.length}] ${event.sql}\n`, |
There was a problem hiding this comment.
When schema apply --verbose runs with effective redactSecrets === false (via --unsafe-show-secrets or an unredacted export manifest), this writes every planner-rendered action to stderr, but the new unredacted warnings only cover --out-plan and --dry-run. Secret-bearing actions such as CREATE USER MAPPING ... OPTIONS (...) or CREATE SUBSCRIPTION ... CONNECTION ... can therefore put credentials in logs without the loud warning the other persistent output paths get; emit the same effective-mode warning before enabling the verbose trace.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c27dc3a535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| process.stderr.write( | ||
| `[${event.actionIndex + 1}/${thePlan.actions.length}] ${event.sql}\n`, | ||
| ); |
There was a problem hiding this comment.
When applying a directory whose manifest has redactSecrets:false (for example, it was exported with --unsafe-show-secrets) or when the flag is passed explicitly, schema apply --verbose now writes each full planner action to stderr, so FDW options, user mappings, or subscription conninfo can be copied into CI/service logs even if the operator did not re-pass the unsafe flag. The new dry-run/out-plan paths add manifest-aware UNREDACTED warnings, but this verbose path has no equivalent guard; either warn before starting the trace or redact the logged SQL.
Useful? React with 👍 / 👎.
| schema apply --dir <dir> --shadow <pg-url> --target <pg-url> | ||
| [--renames auto|prompt|off] [--force] | ||
| [--accept-rename <from>=<to>] ... [--no-reorder] | ||
| [--dry-run] [--verbose] [--out-plan <plan.json>] |
There was a problem hiding this comment.
This updates the top-level pgdelta --help usage, but the separate SCHEMA_USAGE string printed by pgdelta schema --help still lists schema apply only through --no-reorder, so the new --dry-run, --verbose, and --out-plan options remain hidden in the schema-specific help path. Please mirror these flags in that help text too so users looking at the schema subcommand discover them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96c95023f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| })\n`; | ||
| return `${header}${file.contents}`; | ||
| }) | ||
| .join("\n"); |
There was a problem hiding this comment.
When a user executes the advertised --dry-run output (for example, pgdelta schema apply ... --dry-run | psql), flattening the renderer's files into one stream discards the transaction context that renderPlanFiles() assumes its transactional files receive from a migration runner. Ordinary plans always emit SET LOCAL search_path and SET LOCAL check_function_bodies, but without BEGIN these settings have no effect, while wrapping the entire stream externally breaks plans containing non-transactional segments. Emit explicit BEGIN/COMMIT around each transactional segment so the stdout script is actually executable with the same segment semantics as apply().
Useful? React with 👍 / 👎.
| const actionStartedAt = Date.now(); | ||
| emit(onEvent, { kind: "actionStart", actionIndex: i, sql: action.sql }); |
There was a problem hiding this comment.
When an onEvent observer performs synchronous work—such as the verbose CLI writing a large SQL statement to a blocking stderr—the timer starts before the actionStart callback runs, so the reported ms includes observer latency even though the API documents it as the action query's round-trip time. The non-transactional path has the same ordering; record the start time after emitting actionStart and immediately before client.query().
Useful? React with 👍 / 👎.
2 participants
Summary
Adds observability into exactly what
schema applyexecutes by introducing:--dry-run: Prints the executable script (segment-split, mirroring execution) to stdout without applying anything or running the fingerprint gate. Useful for inspecting what will be applied before committing.--verbose: Streams per-statement progress to stderr during a real apply, showing every statement sent on the connection — planner-rendered actions, transaction framing (BEGIN/COMMIT/ROLLBACK), preamble SETs, and RESET ALL — with wall-clock timing per action.--out-plan: Writes the plan artifact (same format asplan --out) to a specified path right after planning, before apply or the dry-run script. Useful for inspecting/archiving the exact plan a run executed.ApplyEventobserver hook: New optionalonEventcallback inApplyOptionsthat fires at segment/action boundaries asapply()executes. Purely additive — no event may change apply's control flow, action statuses, or the returned report. Swallows any exceptions thrown by the observer to maintain isolation.The implementation adds comprehensive event types (
segmentStart,actionStart,actionEnd,segmentEnd,control) that capture the complete wire trace, including transaction framing and session-level settings. This enables--verboseto show exactly what ran, not just the planner's atomic DDL. The motivation: the statements applied to the target are planner-rendered atomic DDL re-derived from the catalog diff, never the authored declarative SQL — so debugging an apply requires visibility into what actually runs.The branch went through three internal review rounds; the trailing
test/fixcommit pairs are RED→GREEN remediations of those findings (non-transactional event ordering, manifest-aware secrets warnings, publicApplyEventexport).Linked issue
N/A — follow-up debugging tooling discussed during pg-delta-next development, no tracked issue.
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
tests/apply-observer.test.tscovering happy path, control events, failure paths, non-transactional segments, and observer isolationtests/cli.test.tsfor--dry-run,--verbose,--out-plan, and credential redaction warnings.changeset/schema-apply-statement-debugging.mdbun run format-and-lintandbun run check-typespass (full corpus: 626/626 on postgres:17-alpine)https://claude.ai/code/session_01JwpD8FPVBzVv2Qs8giVSBm