Skip to content

fix(remote): report what pg_restore printed when schema sync fails - #215

Merged
veksen merged 1 commit into
mainfrom
fix-restore-failure-diagnostics
Aug 4, 2026
Merged

fix(remote): report what pg_restore printed when schema sync fails#215
veksen merged 1 commit into
mainfrom
fix-restore-failure-diagnostics

Conversation

@veksen

@veksen veksen commented Aug 4, 2026

Copy link
Copy Markdown
Member

Goal

A failed schema sync should say what failed. It reported an exit code, and the explanation it had already captured went to a websocket that CI does not subscribe to. Closes Query-Doctor/Site#3836.

Found by putting four open-source repositories through the agent onboarding flow. Two of them, NetBox and Immich, failed here.

What

Before:

Error: Schema sync failed: Error: Restore failed with status 1

That was the only diagnostic in a 1,981-line CI log. Not one line of pg_restore output appeared anywhere.

After:

Error: Schema sync failed: Error: pg_restore failed with status 1:
pg_restore: connecting to database for restore
pg_restore: creating TABLE "public.testing"
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 217; 1259 16385 TABLE testing test
pg_restore: error: could not execute query: ERROR:  relation "testing" already exists

Both blocks above are real output from the same scenario run against this branch and against main, not illustrations.

NetBox cost two 11-minute CI runs and was still unexplained afterwards. The cause turned out to be Query-Doctor/Site#2762, open since April: NetBox declares an ICU collation and the analyzer's Postgres is built --without-icu. One line of the discarded output would have identified it on the first run.

How

Read command-failure.ts first, then the pipeSchema change in remote.ts.

pipeTo already reads the restore's stderr chunk by chunk and re-emits each one. The only subscriber forwards them to a websocket, which the live UI attaches to and a CI job does not, so the lines were produced and then dropped. pipeSchema now keeps them as it forwards them, and hands them to describeCommandFailure when the command reports failure.

describeCommandFailure keeps the end of the output rather than the start. pg_restore prints one line per object it could not create and then a summary, so truncating from the end would drop the line that says how the run finished. The cap is 2000 characters, marked when it applies.

Both the dump and the restore go through it, since a failing dump had the same hole.

Tests

command-failure.test.ts covers the message: it carries the captured output, it still names the command and code when nothing was captured, it keeps the tail of a long output and stays bounded, and it handles a process that exited on a signal rather than a code. The first case uses the exact pg_restore text from the Immich failure. I wrote these before the module existed and confirmed them red.

End-to-end, against real Postgres. I drove Remote.syncFrom through a genuinely failing restore: a source container holding a table, and a target whose template1 already holds the same table, so the freshly created optimizing database collides on it. On main that produced Restore failed with status 1. On this branch it produced the block quoted above, naming the object and the reason. Same script, same containers, one file changed between the two runs.

npm run typecheck is clean and src/sync passes at 42 across 8 files.

What is not here

That end-to-end check is a script, not a committed test. I tried three times to express it as a vitest case in remote.test.ts and it hangs on container startup every time, while the identical sequence completes in about two seconds outside vitest. The neighbouring syncs correctly test starts two containers the same way and passes, so this is something specific to that file's harness that I could not pin down in reasonable time. I removed the test rather than commit one that hangs, and I would rather flag the gap than leave a 120-second timeout in the suite.

I also confirmed one thing the original bug report got wrong: RestoreCommand already passes --no-owner --no-acl, so object ownership is not a failure mode here. The ownership theory in the Site issue's evidence came from a hand-rolled pg_restore that did not use the flags this code uses.

pipeTo already reads the restore's stderr and re-emits it, but the only
subscriber forwards to a websocket that CI has no client for. A failed job
therefore reported an exit code while pg_restore had already named the
extension or collation it could not create.

Closes Query-Doctor/Site#3836

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Query Doctor — 6 successful checks

 Cost regression — No query went up more than 5%
 Untested data access — No changed data-access file without a test
 New query — No new queries
 New query with index recommendation — No new query ships an index recommendation
 Schema drift — No schema changes
 High-value nudge — No index or rewrite past the threshold


More details via MCP → get_ci_run({ runId: "019fcae8-3895-7485-b878-a1363143dc62" }) · view run · docs
3 queries read against main on assumed statistics of 10,000,000 rows per table. Sync production stats for costs measured against your real data.

@veksen
veksen merged commit f4e4039 into main Aug 4, 2026
6 checks passed
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.

1 participant