[joehan]

Resolves Buganizer b/566322255
Parent Goal: b/566303602

Proposed Improvement

• Started this.detectProjectSetup() in FirebaseMcpServer.prototype.start() instead of the constructor.
• When FirebaseMcpServer is instantiated directly (e.g. in unit tests), it no longer starts background asynchronous project detection, preventing leaked timers, background child process spawns (crossSpawn.sync("firebase --version")), filesystem globs, and event-loop stalls across unrelated test suites.
• For production CLI usage (firebase mcp in src/bin/mcp.ts), server.start() is called immediately after construction, so project detection still begins at startup and waits up to 2000 ms for initialized.
• Added unit tests in src/mcp/index.spec.ts verifying that detectProjectSetup is not called on construction and is invoked once when server.start() is called.

Verification

• npm run build: Clean compilation with TypeScript and Vite bundles.
• npm run lint:quiet: Clean linting and Prettier checks across the entire repo.
• Specific test suite:
  npx mocha src/mcp/index.spec.ts src/mcp/util/crashlytics/availability.spec.ts src/mcp/util/apptesting/availability.spec.ts src/mcp/tools/core/login.spec.ts src/mcp/tools/core/get_environment.spec.ts src/mcp/tools/core/deploy.spec.ts
  -> 53 passing in 2s (including 2 new regression tests).
• Before/after timing on the 5 leaking spec files (time npx mocha ...):
  • Pre-fix: 5m 26s (44 passing, 1 timed-out test failure due to event-loop stalls/contention from 45 leaked background tasks).
  • Post-fix: 27.18s wall time (45 passing in 1s).
• Full fast unit test suite:
  npm run mocha:fast
  -> 5568 passing, 11 pending.

[gemini-code-assist]

Code Review

This pull request defers the background project detection in FirebaseMcpServer from construction time to when the server is started, preventing side effects during construction, and adds corresponding unit tests. The feedback suggests catching and logging errors from the asynchronous detectProjectSetup call to prevent potential unhandled promise rejections and process crashes.

[gemini-code-assist]

Since detectProjectSetup runs asynchronously in the background and performs several complex operations (such as resolving options, checking authentication, and checking billing), any thrown error or rejection will result in an unhandled promise rejection. In modern Node.js environments, this can lead to process crashes. It is highly recommended to catch and log any errors from this background task.

    this.detectProjectSetup().catch((err) => {
      this.logger.error("Error detecting project setup: " + err);
    });

[joehan]

Addressed in commit 294160e with .catch((err: unknown) => { this.logger.debug([mcp] Background project detection failed: ${err}); }) and added test in src/mcp/index.spec.ts.

[ajperel]

testing some PR review skills and might as well share the feedback.

[ajperel]

🟡 [Lifecycle Resilience] Catch unhandled rejection on floating background detection promise

Rationale: this.detectProjectSetup() is an asynchronous function invoked in the background without await. If an unexpected error occurs during background initialization (such as an unreadable filesystem directory, misconfigured environment, or CLI preparation failure), this floating promise could trigger an unhandled promise rejection in Node.js. Adding a .catch(...) logger ensures fault-tolerant background execution.

Suggested Fix:

Suggested change

	void this.detectProjectSetup();
	// Background detection starts when the server is started, not when it is constructed, so constructing a server (as unit tests do) has no side effects.
	void this.detectProjectSetup().catch((err: unknown) => {
	this.logger.debug(`[mcp] Background project detection failed: ${err}`);
	});

[joehan]

Addressed in commit 294160e. Chained .catch((err: unknown) => { this.logger.debug([mcp] Background project detection failed: ${err}); }) to the background detectProjectSetup() call in start(), and added a regression test in src/mcp/index.spec.ts verifying that background rejections are caught and logged without unhandled promise rejections.