Skip to content

A test file falls back to a database name that does not exist, so it fails when run from server/ #233

Description

@zopeVaibhav

Found while running the server suite from inside server/ rather than from the repository root. server/tests/server-side-tools.integration.test.ts falls back to a database name that no longer exists anywhere in the project, so the file fails for anyone who runs it without DATABASE_URL already exported, and it fails on a credential error that says nothing about the real cause.

What it looks like

$ cd server && bun test tests/server-side-tools.integration.test.ts
bun test v1.3.14 (0d9b296a)

PostgresError: password authentication failed for user "openkai"
    errno: "28P01",
 severity: "FATAL",
     file: "auth.c",
  routine: "auth_failed",
     code: "ERR_POSTGRES_SERVER_ERROR"

 0 pass
 2 fail

The same failure also surfaces as a Drizzle query error from the afterAll cleanup, which is the line most people will read first and which points at the wrong thing entirely:

error: Failed query: delete from "plugin_grants" where ("plugin_grants"."kind" = $1 and "plugin_grants"."ref" = $2)
params: skill,notes_6bc5a497/search_notes

The suite is green from the repository root, so this does not show up in CI and does not show up for anyone whose habit is to run everything from the top.

Why it happens

server/tests/server-side-tools.integration.test.ts:34

const database = createDatabase(
  process.env.DATABASE_URL ??
    "postgres://openkai:openkai@localhost:5432/openkai",
  TEST_POOL,
);

No user, password or database named openkai exists in docker-compose.yml or in .env.example, and the string appears nowhere else in the tree.

Twenty-six test files under server/tests carry their own copy of this DSN. Twenty-five of them say openbot. This one is the odd one out, which is why it is the only one that fails.

The reason the root and the server/ runs disagree is Bun's .env loading: it reads .env from the working directory. DATABASE_URL is set in the repository's root .env, and there is no server/.env, so the variable is present for bun test at the root and absent for bun test inside server/. Every other file's fallback happens to be correct, so the absence is invisible everywhere except here.

Reproduction

  1. Bring up the stack, or at minimum postgres, so that postgres://openbot:openbot@localhost:5432/openbot is reachable.
  2. Make sure DATABASE_URL is not exported in the shell. An unset variable is the default; nothing in the setup instructions asks for it to be exported.
  3. cd server
  4. bun test tests/server-side-tools.integration.test.ts

Both tests fail with password authentication failed for user "openkai".

For contrast, from the repository root bun test server/tests/server-side-tools.integration.test.ts passes, and every other integration file in that directory passes from either location.

Why it matters

Running a single test file from the workspace it belongs to is an ordinary thing to do, and it is what the failure message will push a contributor toward once they see a database error in a file full of database calls.

What they get is a credential error naming a user they have never heard of, in a project where nothing else mentions openkai. The plausible readings are all wrong: that their .env is broken, that docker-compose.yml provisions a second role, that the schema is half-migrated. The actual answer — one stale string in one test file — is not reachable from anything the error says.

Nothing in the product is broken by it. It costs a newcomer an hour and it will keep doing so until the string is gone.

What a fix probably has to do

The one-line change is to make line 34 say openbot like its twenty-five siblings. That removes the failure.

It does not remove the shape that produced it. The DSN is written out twenty-six times, so any future change to it has twenty-six chances to leave one behind, and the one it leaves behind fails only under a working directory nobody checks.

server/tests/support/ already exists for exactly this kind of thing: database.ts owns TEST_POOL, and environment.ts owns the minimum boot environment and states in its own header that it exists because five files each carried their own copy and all five broke together. The same argument applies here. A testDatabaseUrl() — or a testDatabase() that pairs it with TEST_POOL, since the two are always used together — would give the value one home and make any future change a single edit.

Whether that consolidation belongs in the same change as the one-line fix is a maintainer call. The fix is safe on its own and I am happy to send either.

Severity

Low. No product behaviour is affected and CI is unaffected. It is a newcomer trap with a misleading error, in a file that is otherwise correct.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions