Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions .github/workflows/node.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,15 @@ jobs:
run: pnpm -r build

- name: Unit tests
# pnpm -r runs jest in every workspace package, sequentially (--runInBand
# so per-package fixtures don't contend on ports/files — CLAUDE.md guidance).
# Open-handle leaks that previously prevented clean Jest exit were fixed at
# the source: SingleServerProvider now cancels its connection-timeout and
# reconnect timers on close(), mcp-server tests mock WebSocket so no real
# undici handles outlive each test, and pino disables the thread-stream
# transport in test environments, so the run exits on its own with no
# force-exit stopgap.
# pnpm -r runs jest in every workspace package (--runInBand so per-package
# fixtures don't contend on ports/files — CLAUDE.md guidance). Open-handle
# leaks that previously prevented clean Jest exit were fixed at the source:
# SingleServerProvider unref()s its connection-timeout and reconnect timers
# so an unclosed client can never keep the worker alive, mcp-server tests
# mock WebSocket, and pino disables the thread-stream transport in tests —
# so the run exits on its own with no force-exit stopgap. CLI tests that
# invoke the foreground `dev` server force an absent binary so they never
# boot a real server that would block execSync.
run: pnpm -r exec jest --runInBand --passWithNoTests

- name: Lint (eslint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,17 @@
// BetterAuth not available or failed to initialize — fallback tests cover this case.
// The ESM-only nature of better-auth may prevent dynamic import() in some Jest CJS configs.
betterAuthAvailable = false;
console.log('Note: auth.api tests skipped:', String(_e).split('\n')[0]);

Check warning on line 152 in packages/adapter-better-auth/src/__tests__/TopGunAdapter.integration.test.ts

View workflow job for this annotation

GitHub Actions / Build · Test · Lint

Unexpected console statement
}
});

afterAll(async () => {
// Dispose the started client — its SingleServerProvider keeps a live
// heartbeat/reconnect timer that otherwise outlives the suite and keeps
// Jest's worker alive past the last expect() (hangs CI without --forceExit).
await client?.close();
});

// Conditionally skip tests when BetterAuth is not available (ESM-only package
// cannot be dynamically imported in all Jest CJS configs). Using a conditional
// wrapper so Jest reports these as "skipped" rather than false-positive "passed".
Expand Down Expand Up @@ -257,6 +264,12 @@
adapter = topGunAdapter({ client })({} as BetterAuthOptions);
});

afterEach(async () => {
// Each test starts a fresh client; dispose it so its heartbeat/reconnect
// timer does not leak across tests and keep Jest's worker alive at exit.
await client?.close();
});

it('creates and retrieves a user record', async () => {
const userId = `user-${Date.now()}`;
const userData = {
Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/__tests__/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ describe('topgun dev', () => {
encoding: 'utf8',
cwd: tempDir,
stdio: 'pipe',
// Force the server binary to be absent so `dev` deterministically hits
// the "binary not found" exit (code 1). The binary is resolved from the
// installed @topgunbuild/server package, NOT from cwd — so without this
// override, on a machine where the binary IS present (CI) `dev` boots a
// real foreground server that never returns, hanging execSync and the
// whole job. The timeout is a belt-and-suspenders guard.
env: { ...process.env, TOPGUN_SERVER_BINARY: '/nonexistent/topgun-server' },
timeout: 30000,
});
} catch (error: unknown) {
const err = error as NodeJS.ErrnoException & {
Expand Down
39 changes: 17 additions & 22 deletions packages/cli/src/__tests__/doctor.test.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,38 @@
import { execSync } from 'child_process';
import path from 'path';
import { runCli } from './test-utils';

const CLI_PATH = path.join(__dirname, '../../dist/topgun.js');
// Run from the repo root so doctor finds the server binary + .env
const CLI_CWD = path.join(__dirname, '../../../..');

describe('topgun doctor', () => {
// `doctor` intentionally exits non-zero when an optional dependency is absent
// (e.g. the Rust toolchain, which the Node CI runner does not install), so the
// raw execSync would throw before any assertion. runCli captures stdout/stderr
// regardless of exit code; the environment-check report still prints in full.
it('should run doctor command', () => {
// Pipe stderr to avoid propagation of docker-not-found shell errors to the test runner
const output = execSync(`node ${CLI_PATH} doctor`, {
encoding: 'utf8',
cwd: CLI_CWD,
stdio: ['pipe', 'pipe', 'pipe'],
});
const { stdout, stderr } = runCli(['doctor'], CLI_CWD);
const out = stdout + stderr;

expect(output).toContain('TopGun Environment Check');
expect(output).toContain('Node.js');
expect(output).toContain('pnpm');
expect(out).toContain('TopGun Environment Check');
expect(out).toContain('Node.js');
expect(out).toContain('pnpm');
});

it('should detect Node.js version', () => {
const output = execSync(`node ${CLI_PATH} doctor`, {
encoding: 'utf8',
cwd: CLI_CWD,
stdio: ['pipe', 'pipe', 'pipe'],
});
const { stdout, stderr } = runCli(['doctor'], CLI_CWD);
const out = stdout + stderr;

expect(output).toMatch(/Node\.js.*v\d+/);
expect(output).toContain('✓');
expect(out).toMatch(/Node\.js.*v\d+/);
expect(out).toContain('✓');
});

it('should check for dependencies', () => {
const output = execSync(`node ${CLI_PATH} doctor`, {
encoding: 'utf8',
cwd: CLI_CWD,
stdio: ['pipe', 'pipe', 'pipe'],
});
const { stdout, stderr } = runCli(['doctor'], CLI_CWD);
const out = stdout + stderr;

expect(output).toContain('Dependencies');
expect(out).toContain('Dependencies');
});
});

Expand Down
9 changes: 6 additions & 3 deletions packages/client/src/TopGunClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,12 @@ export class TopGunClient<TSchema extends Record<string, any> = any> {
// Awaiting the provider directly here ensures all reconnect timers are cleared
// before close() returns — otherwise pending connectionTimeoutId / reconnectTimer
// handles keep the Jest event loop alive after every test that creates a client.
await this.syncEngine.getConnectionProvider().close().catch(() => {
// Error already logged inside provider.close()
});
await this.syncEngine
.getConnectionProvider()
.close()
.catch(() => {
// Error already logged inside provider.close()
});
}

// ============================================
Expand Down
12 changes: 12 additions & 0 deletions packages/client/src/__tests__/ORMapPersistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ afterAll(() => {
describe('ORMap Integration & Persistence', () => {
let storage: MemoryStorageAdapter;
let client: TopGunClient;
// Track clients constructed inside individual tests so afterEach can dispose
// them too. An unclosed client keeps a live SingleServerProvider whose
// heartbeat fails (the mock never pongs) and falls into a reconnect loop;
// once afterAll restores the real WebSocket, that loop hits real undici and
// keeps Jest's worker alive indefinitely past the last expect().
const extraClients: TopGunClient[] = [];

beforeEach(() => {
storage = new MemoryStorageAdapter();
Expand All @@ -115,6 +121,10 @@ describe('ORMap Integration & Persistence', () => {
// installed above) the 5s connection-timeout. Without this, each test
// leaks resources that keep Jest's worker alive past the last expect().
await client.close();
for (const c of extraClients) {
await c.close();
}
extraClients.length = 0;
});

test('should persist added items to storage', async () => {
Expand Down Expand Up @@ -174,6 +184,7 @@ describe('ORMap Integration & Persistence', () => {
serverUrl: 'ws://localhost:1234',
storage,
});
extraClients.push(newClient);

const map = newClient.getORMap<string, string>('tags');

Expand All @@ -200,6 +211,7 @@ describe('ORMap Integration & Persistence', () => {
serverUrl: 'ws://localhost:1234',
storage,
});
extraClients.push(newClient);

const map = newClient.getORMap<string, string>('tags');
await new Promise((resolve) => setTimeout(resolve, 50));
Expand Down
12 changes: 12 additions & 0 deletions packages/client/src/connection/SingleServerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ const DEFAULT_CONFIG = {
maxReconnectDelayMs: 30000,
};

/**
* Detach a background timer from the host event loop. A pending reconnect or
* connection-timeout is background machinery — it must never be the sole reason
* a Node process (or a Jest worker) stays alive. Node timers expose unref();
* in browsers setTimeout returns a number with no unref(), so this is a no-op there.
*/
function unrefTimer(timer: ReturnType<typeof setTimeout> | null): void {
(timer as unknown as { unref?: () => void } | null)?.unref?.();
}

/**
* SingleServerProvider implements IConnectionProvider for single-server mode.
*
Expand Down Expand Up @@ -123,6 +133,7 @@ export class SingleServerProvider implements IConnectionProvider {
reject(new Error(`Connection timeout to ${this.url}`));
}
}, this.config.reconnectDelayMs * 5); // 5x initial delay as connection timeout
unrefTimer(this.connectionTimeoutId);

// Clear timeout on successful connection
const originalOnOpen = this.ws.onopen;
Expand Down Expand Up @@ -321,6 +332,7 @@ export class SingleServerProvider implements IConnectionProvider {
this.scheduleReconnect();
}
}, delay);
unrefTimer(this.reconnectTimer);
}

/**
Expand Down
Loading