Repository navigation
Run the database provider tests in CI against PostgreSQL - #409
Merged
Merged
Conversation
5 tasks
The DatabaseIntegration category covers the hand-written per-provider SQL: the atomic dispatch claim, the consolidated baseline migration, startup recovery and the dashboard duration trend. Every one of those tests skips itself without a server, and CI set none, so the PostgreSQL branch - the production default - was executed by nothing until somebody remembered to start Docker locally. The Windows runner ships PostgreSQL installed but stopped. The new job starts it and points NODEPILOT_TEST_POSTGRES at it. It is a job of its own so a database that fails to start reports as a database failure rather than hiding inside the coverage run. SQL Server is not on that image, so its cases keep skipping there and stay a local concern. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sev7eNup
force-pushed
the
ci/database-provider-tests
branch
from
September 22, 2026 21:56
8769f15 to
e9e04f4
Compare
This was referenced Sep 23, 2026
Sev7eNup
added a commit
that referenced
this pull request
Sep 27, 2026
…eck) Automated documentation-drift sync. Both claims were verified against the checked-out code on main, not inferred from a diff. CLAUDE.md - markdown exception lists the skipped CI jobs (from #409) The sentence enumerates which jobs a markdown-only PR skips, and named Frontend, Desktop and E2E. #409 added the `database` job, gated on the same `needs.changes.outputs.code == 'true'` as those three, so it skips with them. Database is added to the list. docs/claude-reference.md - RestApi:AllowedHosts address rule (from #406) The row is specifically about targets that go through a proxy, and stated that one usable address suffices when a host resolves to several, citing NetworkGuard.AssertAnyAddressUsable and the per-address re-check in EnforceConnect. #406 inverted exactly that case: the method is now AssertAddressesUsable, and for a proxied destination every resolved address must pass, because the proxy resolves the destination DNS itself and EnforceConnect never sees those addresses. A host whose DNS mixes a routable record with a link-local one is now rejected when proxied, so the row told an operator the opposite of what the guard does. The one-address rule is scoped to direct targets and the proxied rule spelled out, naming the requireAllAddresses flag and RestApiHttpClientProvider.UsesProxyForDestination. Text only; no code, config or test changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122Ukd5gDBxgLcUPeCPQn7W
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the CI half of #379.
The
DatabaseIntegrationcategory covers the hand-written per-provider SQL — the atomic dispatch claim (FOR UPDATE SKIP LOCKED/UPDLOCK,READPAST,READCOMMITTEDLOCK), the consolidatedInitialBaselinemigration, startup recovery, and the dashboard duration trend with its three dialects. Each of those tests callsAssert.Skipwithout a server, and CI set neitherNODEPILOT_TEST_POSTGRESnorNODEPILOT_TEST_SQLSERVER. So the PostgreSQL branch, which is the production default, was executed by nothing unless somebody remembered to start Docker locally.The new
databasejob starts the PostgreSQL the Windows runner already ships (installed, stopped, passwordroot) and points the environment variable at it. Each fixture still creates its ownnodepilot_test_<guid>database, so the admin connection only ever creates and drops those.A job of its own rather than a step inside
backend, so a database that fails to start reports as a database failure instead of hiding in the coverage run. It is wired intoci-gateand follows the samechanges.codecondition as the other code jobs.SQL Server is not on the Windows image, so its
InlineDatacases keep skipping and stay a local concern throughscripts/Test-DatabaseProviders.ps1.CONTRIBUTING.mdsays so where the script is documented.