Add cookie file reload and session warmup for proxy authenticationFix cookie warmup

[connelldave]

This PR addresses #314 and adds two features for cookie-based authentication (GITLAB_AUTH_COOKIE_PATH) when using a proxy that requires session establishment.

Feature 1: Session warmup

When accessing GitLab through a proxy that requires cookie-based authentication, the first request can fail because the proxy session hasn't been established yet.

This adds a warmup request to /api/v4/user before the first real request, which establishes the session with the proxy. Subsequent requests reuse the warmed session.

Feature 2: Cookie file reload on change

Cookies are loaded from file at MCP server startup. For authentication systems where cookies expire periodically (e.g., every 2 hours) and are refreshed by an external process, the MCP server previously required a restart to pick up new cookies.

Now the cookie file's mtime is checked before each request. If the file has been modified, cookies are reloaded and the session is re-warmed automatically.

Changes

// Resolve cookie path once at startup using os.homedir() for cross-platform support
const resolvedCookiePath = GITLAB_AUTH_COOKIE_PATH
  ? GITLAB_AUTH_COOKIE_PATH.startsWith("~/")
    ? path.join(os.homedir(), GITLAB_AUTH_COOKIE_PATH.slice(2))
    : GITLAB_AUTH_COOKIE_PATH
  : null;

// Cookie jar and fetch - reloaded when cookie file changes
let cookieJar: CookieJar | null = null;
let fetch: typeof nodeFetch = nodeFetch;
let lastCookieMtime = 0;
let cookieReloadLock: Promise<void> | null = null;
let initialSessionRequestMade = false;

// Cookie jar is loaded on first request via reloadCookiesIfChanged (lastCookieMtime=0 triggers load)

async function reloadCookiesIfChanged(): Promise<void> {
  if (!resolvedCookiePath) return;
  if (cookieReloadLock) return cookieReloadLock;

  cookieReloadLock = (async () => {
    try {
      const mtime = (await fs.promises.stat(resolvedCookiePath)).mtimeMs;
      if (mtime !== lastCookieMtime) {
        logger.info(
          { oldMtime: lastCookieMtime, newMtime: mtime },
          "Cookie file changed, reloading"
        );
        lastCookieMtime = mtime;
        const newJar = await createCookieJar();
        cookieJar = newJar;
        fetch = newJar ? fetchCookie(nodeFetch, newJar) : nodeFetch;
        initialSessionRequestMade = false;
      }
    } catch {
      // File deleted or inaccessible - clear cached cookies
      if (cookieJar) {
        logger.info("Cookie file removed, clearing cached cookies");
        cookieJar = null;
        fetch = nodeFetch;
        lastCookieMtime = 0;
        initialSessionRequestMade = false;
      }
    }
  })();

  try {
    await cookieReloadLock;
  } finally {
    cookieReloadLock = null;
  }
}

async function ensureSessionForRequest(): Promise<void> {
  if (!resolvedCookiePath) return;

  await reloadCookiesIfChanged();

  if (!cookieJar || initialSessionRequestMade) return;

  // Make warmup request to establish session cookies
  try {
    const response = await fetch(`${GITLAB_API_URL}/user`, {
      ...getFetchConfig(),
      redirect: "follow",
    });
    // Check if cookies were set by verifying we got a valid response
    initialSessionRequestMade = response.ok || response.status === 401;
  } catch {
    logger.debug("Session warmup request failed, will retry on next request");
  }
}

Testing

1. Start MCP server with GITLAB_AUTH_COOKIE_PATH pointing to a cookie file
2. Make a GitLab request (should succeed after warmup)
3. Modify the cookie file externally (e.g., touch ~/.cookies/gitlab)
4. Make another request - server should log "Cookie file changed, reloading" and re-warm
5. Delete the cookie file - server should log "Cookie file removed, clearing cached cookies" and fall back to standard fetch

Tradeoffs Considered

Approach	Pros	Cons
Reload cookies on every request	Always fresh	Doubles HTTP traffic + fs read per request
Reload on 401/403	Only reload when needed	False positives - auth errors happen for other reasons
Time-based re-warm	Simple	Arbitrary interval, doesn't align with actual cookie refresh
Mtime check per request ✓	Minimal overhead, reloads only when file changes	One stat() syscall per request when cookie auth is enabled (negligible)

Key Implementation Details

• Lazy loading: Cookies are loaded on first request (not at startup), avoiding race conditions with async initialization
• Cache invalidation: If the cookie file is deleted, cached cookies are cleared and fetch falls back to standard behavior
• Cross-platform: Uses os.homedir() instead of process.env.HOME for Windows compatibility
• Async I/O: Uses fs.promises for non-blocking file operations
• Concurrency safe: cookieReloadLock prevents parallel reload operations
• Clock-skew tolerant: Uses !== instead of > for mtime comparison
• No arbitrary delays: Removed 100ms sleep; fetch-cookie awaits setCookie before returning

Impact

• First request no longer fails when using proxy authentication
• Users with short-lived auth cookies no longer need to restart the MCP server when cookies are refreshed externally

[Copilot]

Pull request overview

This PR enhances cookie-based authentication for GitLab proxy scenarios by adding automatic session warmup and dynamic cookie file reloading capabilities. These changes address failures on the first request and eliminate the need for server restarts when cookies are refreshed externally.

Changes:

• Added session warmup functionality to establish proxy authentication before the first API request
• Implemented automatic cookie file reload based on modification time detection
• Refactored cookie path resolution to use os.homedir() for cross-platform compatibility

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

If createCookieJar() returns null due to an error reading the cookie file, the code sets cookieJar = null but leaves lastCookieMtime set to the new mtime. This means subsequent calls won't retry loading the file until the mtime changes again. Consider only updating lastCookieMtime if newJar is not null, or resetting it to 0 on failure.

Suggested change

	lastCookieMtime = mtime;
	const newJar = await createCookieJar();
	cookieJar = newJar;
	fetch = newJar ? fetchCookie(nodeFetch, newJar) : nodeFetch;
	const newJar = await createCookieJar();
	if (newJar) {
	cookieJar = newJar;
	fetch = fetchCookie(nodeFetch, newJar);
	lastCookieMtime = mtime;
	} else {
	cookieJar = null;
	fetch = nodeFetch;
	lastCookieMtime = 0;
	}

[connelldave]

If the cookie file exists but is malformed/unreadable, we should update lastCookieMtime - otherwise we'd spam createCookieJar() on every single request until the file changes. The current behavior is intentional: "I tried to load this version of the file, it failed, I'll try again when it changes."

[Copilot]

The warmup request uses the global GITLAB_API_URL instead of getEffectiveApiUrl(), which may not be correct in REMOTE_AUTHORIZATION mode with ENABLE_DYNAMIC_API_URL. This could cause the warmup to go to the wrong GitLab instance. Consider using getEffectiveApiUrl() to ensure consistency with other API calls.

Suggested change

	const response = await fetch(`${GITLAB_API_URL}/user`, {
	const response = await fetch(`${getEffectiveApiUrl()}/user`, {

[connelldave]

Addressed

[Copilot]

The log message 'Cookie file changed, reloading' is also emitted on the initial load when lastCookieMtime is 0, which could be misleading. Consider adding a conditional check to log a different message like 'Loading cookie file' on the first load (when lastCookieMtime === 0).

Suggested change

	logger.info(
	{ oldMtime: lastCookieMtime, newMtime: mtime },
	"Cookie file changed, reloading"
	const logMessage =
	lastCookieMtime === 0
	? "Loading cookie file"
	: "Cookie file changed, reloading";
	logger.info(
	{ oldMtime: lastCookieMtime, newMtime: mtime },
	logMessage

[connelldave]

Addressed

[Copilot]

The logic assumes that a 401 status means the session was established but authentication failed. However, other HTTP errors (like 403, 404, 500) also mean the request reached the server. Consider using a more inclusive check such as response.status >= 200 or documenting why only 401 and 2xx are considered successful for session establishment.

Suggested change

	// 401 means auth failed but the request completed - cookies were still exchanged
	initialSessionRequestMade = response.ok || response.status === 401;
	// Any HTTP response means the request completed and cookies were exchanged (or attempted)
	initialSessionRequestMade = response.status >= 200;

[connelldave]

A 500 error doesn't tell us cookies were properly exchanged with the auth layer - it could be a proxy error. The current logic is more conservative and correct.

[Copilot]

The catch block doesn't differentiate between file deletion and other errors (like permission issues). The comment says 'File deleted or inaccessible' which is accurate, but the code only clears cookies if cookieJar is truthy. This means if the file becomes inaccessible during initial load, cookies won't be cleared. Consider adding a comment explaining this behavior or handling initial load failures differently.

[connelldave]

🤷 I ignored this one