From fd545d1495e2aa6c9ceac637c4cb51ebc4264e75 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Sun, 11 Oct 2026 13:11:06 +0100 Subject: [PATCH 1/2] Fix MongoDB Atlas Local container reporting ready before mongod restarts The Atlas Local image restarts mongod at the end of its first-run initialisation, and `runner healthcheck` can pass on the mongod that is about to be stopped. A client connecting straight after `start()` then reached a server that was shutting down. Run the healthcheck twice in the wait strategy: the second check fails or blocks until the final mongod is up. Also await the mongoose initial connection in the MongoDB tests, so a failed first connect fails the test instead of surfacing as an unhandled rejection that Vitest retries cannot absorb. --- .../src/mongodb-atlas-local-container.test.ts | 36 +++++++++++++++---- .../src/mongodb-atlas-local-container.ts | 4 ++- .../mongodb/src/mongodb-container.test.ts | 4 +-- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts b/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts index fa6dd7a3e..18ac7f1bf 100644 --- a/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts +++ b/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts @@ -58,10 +58,30 @@ describe("MongoDBAtlasLocalContainer", { timeout: 240_000 }, () => { ); }); + it("should not be ready when the healthcheck passes just before mongod restarts", async () => { + // The image restarts mongod at the end of its initialisation, and its healthcheck can pass on the + // mongod that is about to be stopped. Simulate that with a `runner` earlier on the PATH whose first + // healthcheck passes while the container is still initialising. + const runner = `#!/bin/sh +if [ "$1" = healthcheck ] && [ ! -e /tmp/healthcheck-passed-early ]; then + touch /tmp/healthcheck-passed-early + exit 0 +fi +exec /usr/local/bin/runner "$@" +`; + + await using container = await new MongoDBAtlasLocalContainer(IMAGE) + .withCopyContentToContainer([{ content: runner, target: "/usr/local/sbin/runner", mode: 0o755 }]) + .start(); + + expect((await container.exec(["test", "-e", "/tmp/healthcheck-passed-early"])).exitCode).toBe(0); + expect((await container.exec(["/usr/local/bin/runner", "healthcheck"])).exitCode).toBe(0); + }); + it("should connect to mongodb atlas local", async () => { await using container = await new MongoDBAtlasLocalContainer(IMAGE).start(); - const db = mongoose.createConnection(container.getConnectionString(), { directConnection: true }); + const db = await mongoose.createConnection(container.getConnectionString(), { directConnection: true }).asPromise(); const obj = { value: 1 }; const collection = db.collection("test"); @@ -79,7 +99,9 @@ describe("MongoDBAtlasLocalContainer", { timeout: 240_000 }, () => { .withPassword("customPassword") .start(); - const db = mongoose.createConnection(container.getDatabaseConnectionString(), { directConnection: true }); + const db = await mongoose + .createConnection(container.getDatabaseConnectionString(), { directConnection: true }) + .asPromise(); const obj = { value: 1 }; const collection = db.collection("test"); @@ -95,10 +117,12 @@ describe("MongoDBAtlasLocalContainer", { timeout: 240_000 }, () => { // createAtlasIndexAndSearchIt { await using atlasLocalContainer = await new MongoDBAtlasLocalContainer(IMAGE).start(); - const db = mongoose.createConnection(atlasLocalContainer.getConnectionString(), { - dbName: "test", - directConnection: true, - }); + const db = await mongoose + .createConnection(atlasLocalContainer.getConnectionString(), { + dbName: "test", + directConnection: true, + }) + .asPromise(); try { const collection = db.collection("test"); diff --git a/packages/modules/mongodb/src/mongodb-atlas-local-container.ts b/packages/modules/mongodb/src/mongodb-atlas-local-container.ts index 36031ce85..b0c2d2463 100644 --- a/packages/modules/mongodb/src/mongodb-atlas-local-container.ts +++ b/packages/modules/mongodb/src/mongodb-atlas-local-container.ts @@ -9,8 +9,10 @@ export class MongoDBAtlasLocalContainer extends GenericContainer { constructor(image: string) { super(image); + // The image restarts mongod once it has finished initialising, and the healthcheck can pass on the mongod + // that is about to be stopped. A second healthcheck fails or blocks until the final mongod is up. this.withExposedPorts(MONGODB_PORT) - .withWaitStrategy(Wait.forSuccessfulCommand("runner healthcheck")) + .withWaitStrategy(Wait.forSuccessfulCommand("runner healthcheck && runner healthcheck")) .withStartupTimeout(120_000); } diff --git a/packages/modules/mongodb/src/mongodb-container.test.ts b/packages/modules/mongodb/src/mongodb-container.test.ts index e6c014f40..d44864e9e 100644 --- a/packages/modules/mongodb/src/mongodb-container.test.ts +++ b/packages/modules/mongodb/src/mongodb-container.test.ts @@ -9,7 +9,7 @@ describe("MongoDBContainer", { timeout: 240_000 }, () => { // connectMongo { await using container = await new MongoDBContainer(image).start(); - const db = mongoose.createConnection(container.getConnectionString(), { directConnection: true }); + const db = await mongoose.createConnection(container.getConnectionString(), { directConnection: true }).asPromise(); const obj = { value: 1 }; const collection = db.collection("test"); @@ -30,7 +30,7 @@ describe("MongoDBContainer", { timeout: 240_000 }, () => { .start(); // } - const db = mongoose.createConnection(container.getConnectionString(), { directConnection: true }); + const db = await mongoose.createConnection(container.getConnectionString(), { directConnection: true }).asPromise(); const result = await db.collection("test").insertOne({ title: "test" }); const resultId = result.insertedId.toString(); From 52410293e2f86d88e8694ac635b2abdc4e1943a5 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Sun, 11 Oct 2026 19:05:43 +0100 Subject: [PATCH 2/2] Drop the repeated healthcheck and keep only the awaited connection Running `runner healthcheck` twice only made the early-ready race in the Atlas Local image less likely, so revert the wait strategy change and its simulated regression test. The image exposes no deterministic signal for its final mongod. What remains is the test change: awaiting the mongoose initial connection, so a failed first connect fails the test instead of surfacing as an unhandled rejection. Record in AGENTS.md that odds-reducing changes are not fixes. --- AGENTS.md | 3 +++ .../src/mongodb-atlas-local-container.test.ts | 20 ------------------- .../src/mongodb-atlas-local-container.ts | 4 +--- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 043db2c2d..7fa3f435c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,6 +26,9 @@ It captures practical rules that prevent avoidable CI and PR churn. - If new types are made part of the public API, export them from the package's `index.ts` in the same PR. - If new learnings or misunderstandings are discovered, propose an `AGENTS.md` update in the same PR. - In docs Markdown, keep `` blocks tight with no blank lines between the markers and the include line, or the rendered snippet will contain blank lines between code lines. +- Fix the root cause. A change that only makes a failure less likely is not a fix. + - Arbitrary delays (`sleep`), repeating a check, or adding retries do not remove a race; they hide it. + - For readiness problems, wait on a deterministic signal of the state you need. If no such signal exists, say so rather than shipping a mitigation as a fix. - Tests should verify observable behavior changes, not only internal/config state. - Example: for a security option, assert a real secure/insecure behavior difference. - When adding a regression test for a bug fix, follow a red-green-refactor workflow. diff --git a/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts b/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts index 18ac7f1bf..44d0a4960 100644 --- a/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts +++ b/packages/modules/mongodb/src/mongodb-atlas-local-container.test.ts @@ -58,26 +58,6 @@ describe("MongoDBAtlasLocalContainer", { timeout: 240_000 }, () => { ); }); - it("should not be ready when the healthcheck passes just before mongod restarts", async () => { - // The image restarts mongod at the end of its initialisation, and its healthcheck can pass on the - // mongod that is about to be stopped. Simulate that with a `runner` earlier on the PATH whose first - // healthcheck passes while the container is still initialising. - const runner = `#!/bin/sh -if [ "$1" = healthcheck ] && [ ! -e /tmp/healthcheck-passed-early ]; then - touch /tmp/healthcheck-passed-early - exit 0 -fi -exec /usr/local/bin/runner "$@" -`; - - await using container = await new MongoDBAtlasLocalContainer(IMAGE) - .withCopyContentToContainer([{ content: runner, target: "/usr/local/sbin/runner", mode: 0o755 }]) - .start(); - - expect((await container.exec(["test", "-e", "/tmp/healthcheck-passed-early"])).exitCode).toBe(0); - expect((await container.exec(["/usr/local/bin/runner", "healthcheck"])).exitCode).toBe(0); - }); - it("should connect to mongodb atlas local", async () => { await using container = await new MongoDBAtlasLocalContainer(IMAGE).start(); diff --git a/packages/modules/mongodb/src/mongodb-atlas-local-container.ts b/packages/modules/mongodb/src/mongodb-atlas-local-container.ts index b0c2d2463..36031ce85 100644 --- a/packages/modules/mongodb/src/mongodb-atlas-local-container.ts +++ b/packages/modules/mongodb/src/mongodb-atlas-local-container.ts @@ -9,10 +9,8 @@ export class MongoDBAtlasLocalContainer extends GenericContainer { constructor(image: string) { super(image); - // The image restarts mongod once it has finished initialising, and the healthcheck can pass on the mongod - // that is about to be stopped. A second healthcheck fails or blocks until the final mongod is up. this.withExposedPorts(MONGODB_PORT) - .withWaitStrategy(Wait.forSuccessfulCommand("runner healthcheck && runner healthcheck")) + .withWaitStrategy(Wait.forSuccessfulCommand("runner healthcheck")) .withStartupTimeout(120_000); }