Agent-First PSQL v0.8.2: A Test That Poisoned Its Neighbours

by Agent-First Kit Contributors

Two tests mutated PGHOST and AFPSQL_DSN_SECRET inside the shared library test binary while unlocked readers ran in parallel, so five unrelated connection tests failed depending on thread scheduling. Both moved into a dedicated integration binary that owns process-environment mutation outright. Also picks up Agent-First Data 0.26.2.

resolve_pg_config reads environment variables directly — AFPSQL_DSN_SECRET, PGHOST, PGPORT, PGPASSWORD and friends — because libpq-style fallbacks are part of the connection contract. That makes any test which sets those variables a hazard to any test which reads them, and in Rust both run as threads inside one process.

Two tests did exactly that. Each politely took a shared env_lock() before mutating and restored the previous values afterward. The trouble is that the lock only helps if the readers take it too, and the ordinary connection tests — resolve_conn_from_conninfo, resolve_conn_from_discrete_fields, and three others — never did. They had no reason to: they pass explicit hosts and usernames and assert on the result.

So the mutation window was real, and anything scheduled inside it saw a hostile environment. The failure looked like this, arriving in the middle of a release:

thread 'conn::tests::resolve_conn_from_conninfo' panicked at unit_conn.rs:27:5:
assertion `left == right` failed

Five failures, none of them in the code under test. The same suite had passed twice locally an hour earlier.

The fix is about ownership, not locking

The tempting repair is to make the readers take the lock too. That works until someone adds the sixth reader and forgets, which is the same bug with a longer fuse.

Instead, tests/connection_env.rs is now the only place in the project allowed to mutate the process environment for connection resolution. It is a separate integration binary, so it gets its own process; within it, a single with_connection_env helper clears every connection variable, applies the ones a test wants, runs the body, and restores. The library test binary now contains no environment mutation at all — there is nothing left to race with.

That relocation also carries the v0.8.0 locked-profile test, which was the noisier of the two offenders: it sets AFPSQL_DSN_SECRET, and that variable short-circuits before host and port are ever consulted, so it redirected far more than the variable’s name suggests.

Keeping the test honest

The locked-profile test asserts that an administrator-pinned endpoint ignores the environment. Written carelessly, that assertion passes when the environment was never set — which is exactly how a check quietly stops checking. So it also resolves an unpinned session under the same hostile variables and asserts that one does get redirected to loot:

assert_eq!(
    unpinned.get_dbname(),
    Some("loot"),
    "the environment was not actually in effect, so the pinned case proves nothing"
);

Same variables, same call, opposite outcomes — the difference is the pin, which is the only thing the test is entitled to conclude.

Upgrading

v0.8.2 is a drop-in for v0.8.1: no CLI, protocol, or connection behavior changes. It moves tests and picks up Agent-First Data 0.26.2, whose spec amendment names the consumption-mode rule that v0.8.0 implemented for --mode pipe and --stream-rows.