From 1df4247b24b9f4aa151875da1395269300cba95d Mon Sep 17 00:00:00 2001 From: Dennis Meister Date: Wed, 25 Feb 2026 14:22:52 +0100 Subject: [PATCH 1/5] feat: add kilo for slack gitlab support --- src/lib/slack-bot.ts | 154 ++++++++++++++---- .../gitlab-repository-context.test.ts | 95 +++++++++++ .../slack-bot/gitlab-repository-context.ts | 82 ++++++++++ 3 files changed, 299 insertions(+), 32 deletions(-) create mode 100644 src/lib/slack-bot/gitlab-repository-context.test.ts create mode 100644 src/lib/slack-bot/gitlab-repository-context.ts diff --git a/src/lib/slack-bot.ts b/src/lib/slack-bot.ts index db9cf99ed..ccfca3645 100644 --- a/src/lib/slack-bot.ts +++ b/src/lib/slack-bot.ts @@ -7,6 +7,13 @@ import { getGitHubTokenForUser, getGitHubTokenForOrganization, } from '@/lib/cloud-agent/github-integration-helpers'; +import { + getGitLabTokenForUser, + getGitLabTokenForOrganization, + getGitLabInstanceUrlForUser, + getGitLabInstanceUrlForOrganization, + buildGitLabCloneUrl, +} from '@/lib/cloud-agent/gitlab-integration-helpers'; import type OpenAI from 'openai'; import type { Owner } from '@/lib/integrations/core/types'; import { @@ -21,6 +28,10 @@ import { formatGitHubRepositoriesForPrompt, getGitHubRepositoryContext, } from '@/lib/slack-bot/github-repository-context'; +import { + formatGitLabRepositoriesForPrompt, + getGitLabRepositoryContext, +} from '@/lib/slack-bot/gitlab-repository-context'; import { formatSlackConversationContextForPrompt, getSlackConversationContext, @@ -66,23 +77,29 @@ const KILO_BOT_SYSTEM_PROMPT = `You are Kilo Bot, a helpful AI assistant integra Additional context may be appended to this prompt: - Slack conversation context (recent messages, thread context) - Available GitHub repositories for this Slack integration +- Available GitLab projects for this Slack integration -Treat this context as authoritative. Prefer selecting a repo from the provided repository list. If the user requests work on a repo that isn't in the list, ask them to confirm the exact owner/repo and ensure it's accessible to the integration. Never invent repository names. +Treat this context as authoritative. Prefer selecting a repo from the provided repository list. If the user requests work on a repo that isn't in the list, ask them to confirm the exact owner/repo (or group/project for GitLab) and ensure it's accessible to the integration. Never invent repository names. ## Tool: spawn_cloud_agent -You can call the tool "spawn_cloud_agent" to run a Cloud Agent session for coding work on a GitHub repository. +You can call the tool "spawn_cloud_agent" to run a Cloud Agent session for coding work on a GitHub repository or GitLab project. ### When to use it Use spawn_cloud_agent when the user asks you to: - change code, fix bugs, implement features, or refactor - review/analyze code in a repo beyond a quick, high-level answer -- do any task where you must inspect files, run tests, or open a PR +- do any task where you must inspect files, run tests, or open a PR/MR If the user is only asking a question you can answer directly (conceptual, small snippet, explanation), do not call the tool. ### How to use it -Provide: -- githubRepo: "owner/repo" +Provide exactly ONE of: +- githubRepo: "owner/repo" — for GitHub repositories +- gitlabProject: "group/project" or "group/subgroup/project" — for GitLab projects + +Determine which platform to use based on the repository context provided below. If the user mentions a repo that appears in the GitHub list, use githubRepo. If it appears in the GitLab list, use gitlabProject. + +Also provide: - mode: - code: implement changes - debug: investigate failures, flaky tests, production issues @@ -94,11 +111,11 @@ Provide: Your prompt to the agent should usually include: - the desired outcome (what "done" looks like) - any constraints (keep changes minimal, follow existing patterns, etc.) -- a request to open a PR and return the PR URL +- a request to open a PR (GitHub) or MR (GitLab) and return the URL ## Accuracy & safety -- Don't claim you ran tools, changed code, or created a PR unless the tool results confirm it. -- Don't fabricate links (including PR URLs). +- Don't claim you ran tools, changed code, or created a PR/MR unless the tool results confirm it. +- Don't fabricate links (including PR/MR URLs). - If you can't proceed (missing repo, missing details, permissions), say what's missing and what you need next.`; /** @@ -109,7 +126,7 @@ const SPAWN_CLOUD_AGENT_TOOL: OpenAI.Chat.Completions.ChatCompletionTool = { function: { name: 'spawn_cloud_agent', description: - 'Spawn a Cloud Agent session to perform coding tasks on a GitHub repository. The agent can make code changes, fix bugs, implement features, and more.', + 'Spawn a Cloud Agent session to perform coding tasks on a GitHub repository or GitLab project. Provide exactly one of githubRepo or gitlabProject.', parameters: { type: 'object', properties: { @@ -118,6 +135,12 @@ const SPAWN_CLOUD_AGENT_TOOL: OpenAI.Chat.Completions.ChatCompletionTool = { description: 'The GitHub repository in owner/repo format (e.g., "facebook/react")', pattern: '^[-a-zA-Z0-9_.]+/[-a-zA-Z0-9_.]+$', }, + gitlabProject: { + type: 'string', + description: + 'The GitLab project path in group/project format (e.g., "mygroup/myproject"). May include nested groups (e.g., "group/subgroup/project").', + pattern: '^[-a-zA-Z0-9_.]+(/[-a-zA-Z0-9_.]+)+$', + }, prompt: { type: 'string', description: @@ -131,7 +154,7 @@ const SPAWN_CLOUD_AGENT_TOOL: OpenAI.Chat.Completions.ChatCompletionTool = { default: 'code', }, }, - required: ['githubRepo', 'prompt'], + required: ['prompt'], }, }, }; @@ -214,11 +237,13 @@ async function getSlackRequesterInfo( /** * Spawn a Cloud Agent session and collect the results. + * Supports both GitHub (githubRepo) and GitLab (gitlabProject) repositories. * Delegates to the shared runSessionToCompletion helper. */ async function spawnCloudAgentSession( args: { - githubRepo: string; + githubRepo?: string; + gitlabProject?: string; prompt: string; mode?: string; }, @@ -231,37 +256,87 @@ async function spawnCloudAgentSession( console.log('[SlackBot] spawnCloudAgentSession called with args:', JSON.stringify(args, null, 2)); console.log('[SlackBot] Owner:', JSON.stringify(owner, null, 2)); - let githubToken: string | undefined; - let kilocodeOrganizationId: string | undefined; + if (args.githubRepo && args.gitlabProject) { + return { + response: 'Error: Both githubRepo and gitlabProject were specified. Provide exactly one.', + }; + } + + if (!args.githubRepo && !args.gitlabProject) { + return { + response: 'Error: No repository specified. Provide either githubRepo or gitlabProject.', + }; + } - // Handle organization-owned integrations + let kilocodeOrganizationId: string | undefined; if (owner.type === 'org') { - githubToken = await getGitHubTokenForOrganization(owner.id); kilocodeOrganizationId = owner.id; - } else { - githubToken = await getGitHubTokenForUser(owner.id); } - // Append PR signature to the prompt if we have requester info + // Append PR/MR signature to the prompt if we have requester info const promptWithSignature = requesterInfo ? args.prompt + buildPrSignature(requesterInfo) : args.prompt; - const result = await runSessionToCompletion({ - client: createCloudAgentNextClient(authToken, { skipBalanceCheck: true }), - prepareInput: { - githubRepo: args.githubRepo, + // Build platform-specific prepareInput and initiateInput + let prepareInput: PrepareSessionInput; + let initiateInput: { githubToken?: string; kilocodeOrganizationId?: string }; + + if (args.gitlabProject) { + // GitLab path: get token + instance URL, build clone URL, use gitUrl/gitToken + const gitlabToken = + owner.type === 'org' + ? await getGitLabTokenForOrganization(owner.id) + : await getGitLabTokenForUser(owner.id); + + const instanceUrl = + owner.type === 'org' + ? await getGitLabInstanceUrlForOrganization(owner.id) + : await getGitLabInstanceUrlForUser(owner.id); + + const gitUrl = buildGitLabCloneUrl(args.gitlabProject, instanceUrl); + + console.log( + '[SlackBot] GitLab session - project:', + args.gitlabProject, + 'instance:', + instanceUrl + ); + + prepareInput = { prompt: promptWithSignature, mode: (args.mode as PrepareSessionInput['mode']) || 'code', model, - githubToken, + gitUrl, + gitToken: gitlabToken, + platform: 'gitlab', kilocodeOrganizationId, createdOnPlatform: 'slack', - }, - initiateInput: { + }; + initiateInput = { kilocodeOrganizationId }; + } else { + // GitHub path: get token, use githubRepo/githubToken + const githubToken = + owner.type === 'org' + ? await getGitHubTokenForOrganization(owner.id) + : await getGitHubTokenForUser(owner.id); + + prepareInput = { + githubRepo: args.githubRepo, + prompt: promptWithSignature, + mode: (args.mode as PrepareSessionInput['mode']) || 'code', + model, githubToken, kilocodeOrganizationId, - }, + createdOnPlatform: 'slack', + }; + initiateInput = { githubToken, kilocodeOrganizationId }; + } + + const result = await runSessionToCompletion({ + client: createCloudAgentNextClient(authToken, { skipBalanceCheck: true }), + prepareInput, + initiateInput, ticketPayload: { userId: ticketUserId, organizationId: owner.type === 'org' ? owner.id : undefined, @@ -371,14 +446,29 @@ export async function processKiloBotMessage( ? await getSlackRequesterInfo(installation, slackEventContext) : undefined; - // Get repository context (no extra requests; uses the same integration row) - const repoContext = await getGitHubRepositoryContext(owner); - const repoCount = repoContext.repositories ? repoContext.repositories.length : 0; - console.log('[SlackBot] Found', repoCount, 'available repositories'); + // Get repository context (no extra requests; uses the same integration rows) + const githubRepoContext = await getGitHubRepositoryContext(owner); + const gitlabRepoContext = await getGitLabRepositoryContext(owner); + const githubRepoCount = githubRepoContext.repositories + ? githubRepoContext.repositories.length + : 0; + const gitlabRepoCount = gitlabRepoContext.repositories + ? gitlabRepoContext.repositories.length + : 0; + console.log( + '[SlackBot] Found', + githubRepoCount, + 'GitHub and', + gitlabRepoCount, + 'GitLab repositories' + ); - // Build system prompt with Slack context + repository context + // Build system prompt with Slack context + repository context for both platforms const systemPrompt = - KILO_BOT_SYSTEM_PROMPT + slackContextForPrompt + formatGitHubRepositoriesForPrompt(repoContext); + KILO_BOT_SYSTEM_PROMPT + + slackContextForPrompt + + formatGitHubRepositoriesForPrompt(githubRepoContext) + + formatGitLabRepositoriesForPrompt(gitlabRepoContext); const runResult = await runBot({ authToken, diff --git a/src/lib/slack-bot/gitlab-repository-context.test.ts b/src/lib/slack-bot/gitlab-repository-context.test.ts new file mode 100644 index 000000000..70284d155 --- /dev/null +++ b/src/lib/slack-bot/gitlab-repository-context.test.ts @@ -0,0 +1,95 @@ +import { + formatGitLabRepositoriesForPrompt, + type GitLabRepositoryContext, +} from './gitlab-repository-context'; + +describe('formatGitLabRepositoriesForPrompt', () => { + test('shows account, instance, and repository list when repos are available', () => { + const context: GitLabRepositoryContext = { + accountLogin: 'gitlab-user', + repositoryAccess: 'selected', + repositoriesSyncedAt: '2024-01-15T10:00:00Z', + instanceUrl: 'https://gitlab.com', + repositories: [ + { id: 1, name: 'project-a', full_name: 'mygroup/project-a', private: false }, + { id: 2, name: 'project-b', full_name: 'mygroup/subgroup/project-b', private: true }, + ], + }; + + const result = formatGitLabRepositoriesForPrompt(context); + + expect(result).toContain('GitLab repository context'); + expect(result).toContain('Account: gitlab-user'); + expect(result).toContain('Instance: https://gitlab.com'); + expect(result).toContain('Repository access: selected'); + expect(result).toContain('Repositories synced at: 2024-01-15T10:00:00Z'); + expect(result).toContain('mygroup/project-a [id: 1]'); + expect(result).toContain('mygroup/subgroup/project-b (private) [id: 2]'); + expect(result).toContain('nested groups'); + }); + + test('shows "all access" message when repositoryAccess is "all" and no repos listed', () => { + const context: GitLabRepositoryContext = { + accountLogin: 'gitlab-user', + repositoryAccess: 'all', + repositoriesSyncedAt: null, + instanceUrl: 'https://gitlab.com', + repositories: [], + }; + + const result = formatGitLabRepositoriesForPrompt(context); + + expect(result).toContain('not stored for "all" access'); + expect(result).toContain('group/project format'); + }); + + test('shows "no repos connected" when no integration repos and access is not "all"', () => { + const context: GitLabRepositoryContext = { + accountLogin: null, + repositoryAccess: null, + repositoriesSyncedAt: null, + instanceUrl: null, + repositories: null, + }; + + const result = formatGitLabRepositoriesForPrompt(context); + + expect(result).toContain('No GitLab repositories are currently connected'); + }); + + test('includes self-hosted instance URL', () => { + const context: GitLabRepositoryContext = { + accountLogin: 'admin', + repositoryAccess: 'selected', + repositoriesSyncedAt: null, + instanceUrl: 'https://gitlab.example.com', + repositories: [{ id: 10, name: 'internal', full_name: 'team/internal', private: true }], + }; + + const result = formatGitLabRepositoriesForPrompt(context); + + expect(result).toContain('Instance: https://gitlab.example.com'); + expect(result).toContain('team/internal (private) [id: 10]'); + }); + + test('handles null repositories the same as empty array', () => { + const contextNull: GitLabRepositoryContext = { + accountLogin: 'user', + repositoryAccess: 'selected', + repositoriesSyncedAt: null, + instanceUrl: 'https://gitlab.com', + repositories: null, + }; + + const contextEmpty: GitLabRepositoryContext = { + ...contextNull, + repositories: [], + }; + + const resultNull = formatGitLabRepositoriesForPrompt(contextNull); + const resultEmpty = formatGitLabRepositoriesForPrompt(contextEmpty); + + expect(resultNull).toContain('No GitLab repositories are currently connected'); + expect(resultEmpty).toContain('No GitLab repositories are currently connected'); + }); +}); diff --git a/src/lib/slack-bot/gitlab-repository-context.ts b/src/lib/slack-bot/gitlab-repository-context.ts new file mode 100644 index 000000000..eacf6d480 --- /dev/null +++ b/src/lib/slack-bot/gitlab-repository-context.ts @@ -0,0 +1,82 @@ +import type { Owner, PlatformRepository } from '@/lib/integrations/core/types'; +import { PLATFORM } from '@/lib/integrations/core/constants'; +import { getIntegrationForOwner } from '@/lib/integrations/db/platform-integrations'; + +export type GitLabRepositoryContext = { + accountLogin: string | null; + repositoryAccess: string | null; + repositoriesSyncedAt: string | null; + repositories: PlatformRepository[] | null; + instanceUrl: string | null; +}; + +/** + * Get GitLab repository context for an owner from their GitLab integration. + * This does not perform extra API requests; it uses data stored on the integration row. + */ +export async function getGitLabRepositoryContext(owner: Owner): Promise { + const integration = await getIntegrationForOwner(owner, PLATFORM.GITLAB); + if (!integration) { + return { + accountLogin: null, + repositoryAccess: null, + repositoriesSyncedAt: null, + repositories: null, + instanceUrl: null, + }; + } + + const repositories = integration.repositories ? integration.repositories : null; + const metadata = integration.metadata as { gitlab_instance_url?: string } | null; + const instanceUrl = metadata?.gitlab_instance_url || 'https://gitlab.com'; + + return { + accountLogin: integration.platform_account_login, + repositoryAccess: integration.repository_access, + repositoriesSyncedAt: integration.repositories_synced_at, + repositories, + instanceUrl, + }; +} + +export function formatGitLabRepositoriesForPrompt(context: GitLabRepositoryContext): string { + const headerLines: string[] = ['\n\nGitLab repository context for this workspace:']; + + if (context.accountLogin) { + headerLines.push(`- Account: ${context.accountLogin}`); + } + if (context.instanceUrl) { + headerLines.push(`- Instance: ${context.instanceUrl}`); + } + if (context.repositoryAccess) { + headerLines.push(`- Repository access: ${context.repositoryAccess}`); + } + if (context.repositoriesSyncedAt) { + headerLines.push(`- Repositories synced at: ${context.repositoriesSyncedAt}`); + } + + const header = headerLines.join('\n'); + + if (!context.repositories || context.repositories.length === 0) { + if (context.repositoryAccess === 'all') { + return `${header} +- Repository list: not stored for "all" access (no repo list to show without extra requests). + +When the user asks you to work on a GitLab project, ask them to specify the project path explicitly in group/project format.`; + } + + return `${header} +- No GitLab repositories are currently connected. The user will need to specify a project manually.`; + } + + const repoList = context.repositories + .map(repo => `- ${repo.full_name}${repo.private ? ' (private)' : ''} [id: ${repo.id}]`) + .join('\n'); + + return `${header} + +Available GitLab projects: +${repoList} + +When the user asks you to work on code without specifying a project, try to infer the correct project from context or ask them to clarify which project they want to use. GitLab project paths may have nested groups (e.g., group/subgroup/project).`; +} From f03fa7b6da99e2442104e520ff73b13bfdb8cf04 Mon Sep 17 00:00:00 2001 From: Dennis Meister Date: Wed, 25 Feb 2026 16:04:39 +0100 Subject: [PATCH 2/5] chore: review findings --- src/lib/slack-bot.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/lib/slack-bot.ts b/src/lib/slack-bot.ts index ccfca3645..97980a134 100644 --- a/src/lib/slack-bot.ts +++ b/src/lib/slack-bot.ts @@ -155,6 +155,7 @@ const SPAWN_CLOUD_AGENT_TOOL: OpenAI.Chat.Completions.ChatCompletionTool = { }, }, required: ['prompt'], + oneOf: [{ required: ['githubRepo'] }, { required: ['gitlabProject'] }], }, }, }; @@ -268,6 +269,14 @@ async function spawnCloudAgentSession( }; } + // Validate the repo identifier has at least "owner/repo" shape (non-empty segments around a slash) + const repoIdentifier = args.githubRepo ?? args.gitlabProject; + if (!repoIdentifier || !/\/./.test(repoIdentifier)) { + return { + response: `Error: Invalid repository identifier "${repoIdentifier ?? ''}". Expected format like "owner/repo".`, + }; + } + let kilocodeOrganizationId: string | undefined; if (owner.type === 'org') { kilocodeOrganizationId = owner.id; @@ -289,6 +298,13 @@ async function spawnCloudAgentSession( ? await getGitLabTokenForOrganization(owner.id) : await getGitLabTokenForUser(owner.id); + if (!gitlabToken) { + return { + response: + 'Error: No GitLab token available. Please ensure a GitLab integration is connected in your Kilo Code settings.', + }; + } + const instanceUrl = owner.type === 'org' ? await getGitLabInstanceUrlForOrganization(owner.id) @@ -296,11 +312,12 @@ async function spawnCloudAgentSession( const gitUrl = buildGitLabCloneUrl(args.gitlabProject, instanceUrl); + const isSelfHosted = !/^https?:\/\/(www\.)?gitlab\.com(\/|$)/i.test(instanceUrl); console.log( '[SlackBot] GitLab session - project:', args.gitlabProject, 'instance:', - instanceUrl + isSelfHosted ? 'self-hosted' : 'gitlab.com' ); prepareInput = { @@ -321,6 +338,13 @@ async function spawnCloudAgentSession( ? await getGitHubTokenForOrganization(owner.id) : await getGitHubTokenForUser(owner.id); + if (!githubToken) { + return { + response: + 'Error: No GitHub token available. Please ensure a GitHub integration is connected in your Kilo Code settings.', + }; + } + prepareInput = { githubRepo: args.githubRepo, prompt: promptWithSignature, From 7e04aa35868301abb1e329abf1d365a18c465964 Mon Sep 17 00:00:00 2001 From: Dennis Meister Date: Fri, 27 Feb 2026 11:53:32 +0100 Subject: [PATCH 3/5] fix: make it work again after rebase --- .../cloud-agent-next/cloud-agent-client.ts | 9 +- src/lib/cloud-agent-next/run-session.ts | 33 +++- src/lib/cloud-agent-next/websocket-manager.ts | 181 +++++++++++++----- src/lib/cloud-agent/websocket-manager.ts | 173 ++++++++++++----- src/lib/slack-bot.ts | 1 - .../gitlab-repository-context.test.ts | 7 +- .../slack-bot/gitlab-repository-context.ts | 13 +- 7 files changed, 302 insertions(+), 115 deletions(-) diff --git a/src/lib/cloud-agent-next/cloud-agent-client.ts b/src/lib/cloud-agent-next/cloud-agent-client.ts index 9060e8638..a2752c77d 100644 --- a/src/lib/cloud-agent-next/cloud-agent-client.ts +++ b/src/lib/cloud-agent-next/cloud-agent-client.ts @@ -13,11 +13,12 @@ import { INTERNAL_API_SECRET } from '@/lib/config.server'; * Client for the new cloud-agent-next worker that uses the V2 WebSocket-based API * with the new message format (Message + Part[]). * - * PLACEHOLDER: Update CLOUD_AGENT_NEXT_API_URL when the new worker is ready. + * Falls back to CLOUD_AGENT_API_URL (the original cloud-agent worker) when + * CLOUD_AGENT_NEXT_API_URL is not configured, since both workers expose + * compatible tRPC APIs. */ - -// TODO: Update this URL when the new cloud-agent-next worker is deployed -const CLOUD_AGENT_NEXT_API_URL = getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || ''; +const CLOUD_AGENT_NEXT_API_URL = + getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || getEnvVariable('CLOUD_AGENT_API_URL') || ''; // MCP server config types — CLI-native local/remote format type MCPLocalServerConfig = { diff --git a/src/lib/cloud-agent-next/run-session.ts b/src/lib/cloud-agent-next/run-session.ts index 3fd979720..268520d51 100644 --- a/src/lib/cloud-agent-next/run-session.ts +++ b/src/lib/cloud-agent-next/run-session.ts @@ -21,8 +21,11 @@ import type { PrepareSessionInput, InitiateFromPreparedSessionInput } from './cl // Constants // --------------------------------------------------------------------------- -const CLOUD_AGENT_NEXT_WS_URL = getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_NEXT_WS_URL'); -const CLOUD_AGENT_NEXT_API_URL = getEnvVariable('CLOUD_AGENT_NEXT_API_URL'); +const CLOUD_AGENT_NEXT_WS_URL = + getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_NEXT_WS_URL') || + getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_WS_URL'); +const CLOUD_AGENT_NEXT_API_URL = + getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || getEnvVariable('CLOUD_AGENT_API_URL'); const DEFAULT_STREAM_TIMEOUT_MS = 15 * 60 * 1000; // 15 minutes const COMPLETE_GRACE_MS = 1000; // Wait 1s after 'complete' for final events @@ -180,6 +183,7 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise { + console.log(`${logPrefix} WebSocket state: ${state.status}`, state); if (state.status === 'error') { hasError = true; errorMessage = state.error; @@ -332,7 +340,10 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise { @@ -345,7 +356,19 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise | null = null; + let connectionTimeoutId: ReturnType | null = null; let intentionalDisconnect = false; let currentTicket = config.ticket; let ticketRefreshAttempted = false; + /** Tracks whether onerror already triggered close handling (Node.js 22 workaround). */ + let errorHandledAsClose = false; function setState(newState: ConnectionState) { state = newState; config.onStateChange(state); } + function clearConnectionTimeout() { + if (connectionTimeoutId !== null) { + clearTimeout(connectionTimeoutId); + connectionTimeoutId = null; + } + } + function buildUrl(fromId?: number): string { const url = new URL(config.url); url.searchParams.set('ticket', currentTicket); @@ -109,6 +125,54 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { return url.toString(); } + /** + * Central close-handling logic, shared by onclose and the Node.js onerror fallback. + * @param code WebSocket close code (1006 when synthesised from onerror) + * @param reason Human-readable reason string + */ + function handleClose(code: number, reason: string) { + clearConnectionTimeout(); + + console.log('[WebSocketManager] handleClose', { + code, + reason, + intentionalDisconnect, + ticketRefreshAttempted, + currentState: state.status, + }); + + if (intentionalDisconnect) { + setState({ status: 'disconnected' }); + return; + } + + const isAuthFailure = + AUTH_FAILURE_CLOSE_CODES.includes(code as (typeof AUTH_FAILURE_CLOSE_CODES)[number]) || + AUTH_FAILURE_KEYWORDS.some(kw => reason.toLowerCase().includes(kw)); + + if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { + console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); + void refreshTicketAndReconnect(); + return; + } + + if (isAuthFailure && ticketRefreshAttempted) { + console.log('[WebSocketManager] Auth failure after ticket refresh - stopping retries'); + setState({ + status: 'error', + error: 'Authentication failed after ticket refresh. Check server configuration.', + retryable: false, + }); + return; + } + + if (state.status === 'connecting' || state.status === 'connected') { + scheduleReconnect(0); + } else if (state.status === 'reconnecting') { + scheduleReconnect(state.attempt); + } + } + async function refreshTicketAndReconnect() { console.log('[WebSocketManager] refreshTicketAndReconnect called', { hasRefreshHandler: !!config.onRefreshTicket, @@ -168,6 +232,8 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { } function connectInternal(_attempt = 0) { + clearConnectionTimeout(); + // Close existing socket if any - store reference so onclose handler can ignore it const oldWs = ws; if (oldWs !== null) { @@ -175,15 +241,52 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { oldWs.close(); } + // Reset per-connection flag + errorHandledAsClose = false; + // Always include lastEventId if we have one - this enables replay after any reconnection // (whether from network issues, ticket refresh, or other disconnects) const urlWithReplay = lastEventId ? buildUrl(lastEventId) : buildUrl(); setState({ status: 'connecting' }); - const newWs = new WebSocket(urlWithReplay); + let newWs: WebSocket; + try { + newWs = new WebSocket(urlWithReplay); + } catch (err) { + console.error('[WebSocketManager] Failed to create WebSocket:', err); + setState({ + status: 'error', + error: `WebSocket constructor error: ${err instanceof Error ? err.message : String(err)}`, + retryable: true, + }); + return; + } ws = newWs; + // Connection timeout — if we don't receive a message within CONNECTION_TIMEOUT_MS, + // treat it as a failed connection. This guards against Node.js WebSocket hanging + // without firing onclose. + connectionTimeoutId = setTimeout(() => { + connectionTimeoutId = null; + if (ws === newWs && state.status === 'connecting') { + console.log( + '[WebSocketManager] Connection timeout - no messages received within', + CONNECTION_TIMEOUT_MS, + 'ms' + ); + ws = null; + try { + newWs.close(); + } catch { + /* ignore */ + } + handleClose(1006, 'Connection timeout'); + } + }, CONNECTION_TIMEOUT_MS); + newWs.onmessage = (messageEvent: MessageEvent) => { + clearConnectionTimeout(); + const parsed = parseMessage(messageEvent.data); if (parsed === null) { return; @@ -208,72 +311,51 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { }; newWs.onerror = (errorEvent: Event) => { - // WebSocket errors during HTTP upgrade (like 401) may not give us useful close codes - // Log the error for debugging + const errorMessage = + 'message' in errorEvent + ? String((errorEvent as { message: unknown }).message) + : 'Unknown error'; console.log('[WebSocketManager] WebSocket error', { type: errorEvent.type, + message: errorMessage, + readyState: newWs.readyState, ticketRefreshAttempted, currentState: state.status, }); - // The actual handling happens in onclose which fires after onerror + + // Node.js 22 workaround: the built-in WebSocket fires onerror but NOT onclose + // when a connection fails (e.g., server unreachable or HTTP error before upgrade). + // readyState stays at CONNECTING (0). In browsers, onclose always fires after + // onerror, but Node.js 22's undici-based WebSocket doesn't follow this. + // Detect this case and trigger close handling directly. + if (newWs.readyState === WebSocket.CONNECTING || newWs.readyState === WebSocket.CLOSED) { + // Mark that we handled this error as a close so the real onclose (if it + // does eventually fire) won't double-process. + errorHandledAsClose = true; + ws = null; + clearConnectionTimeout(); + handleClose(1006, errorMessage); + } + // Otherwise, wait for the onclose event (browser behavior). }; newWs.onclose = (event: CloseEvent) => { // Ignore close events from replaced sockets - a new socket has already been created - if (ws !== newWs) { + if (ws !== newWs && !errorHandledAsClose) { console.log('[WebSocketManager] Ignoring close from replaced socket'); return; } - ws = null; - - console.log('[WebSocketManager] WebSocket closed', { - code: event.code, - reason: event.reason, - wasClean: event.wasClean, - intentionalDisconnect, - ticketRefreshAttempted, - currentState: state.status, - }); - - if (intentionalDisconnect) { - setState({ status: 'disconnected' }); - return; - } - - const isAuthFailure = isAuthFailureClose(event); - - console.log('[WebSocketManager] Auth failure check', { - isAuthFailure, - ticketRefreshAttempted, - hasRefreshHandler: !!config.onRefreshTicket, - willRefresh: isAuthFailure && !ticketRefreshAttempted && !!config.onRefreshTicket, - }); - - if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { - console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); - void refreshTicketAndReconnect(); - return; - } - // If we already tried refreshing the ticket and still getting auth failures, - // don't keep retrying - the issue is likely not the ticket - if (isAuthFailure && ticketRefreshAttempted) { + // If onerror already handled this as a close (Node.js 22 workaround), skip. + if (errorHandledAsClose) { console.log( - '[WebSocketManager] Auth failure after ticket refresh - stopping retries (likely origin/config issue)' + '[WebSocketManager] Ignoring onclose - already handled via onerror (Node.js 22 workaround)' ); - setState({ - status: 'error', - error: 'Authentication failed after ticket refresh. Check server configuration.', - retryable: false, - }); return; } - if (state.status === 'connecting' || state.status === 'connected') { - scheduleReconnect(0); - } else if (state.status === 'reconnecting') { - scheduleReconnect(state.attempt); - } + ws = null; + handleClose(event.code, event.reason ?? ''); }; } @@ -287,6 +369,7 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { function disconnect() { intentionalDisconnect = true; + clearConnectionTimeout(); if (reconnectTimeoutId !== null) { clearTimeout(reconnectTimeoutId); diff --git a/src/lib/cloud-agent/websocket-manager.ts b/src/lib/cloud-agent/websocket-manager.ts index 3bed0b2c1..2d0117dd2 100644 --- a/src/lib/cloud-agent/websocket-manager.ts +++ b/src/lib/cloud-agent/websocket-manager.ts @@ -44,6 +44,11 @@ function parseMessage(data: unknown): ParsedMessage | null { const MAX_RECONNECT_ATTEMPTS = 8; const BACKOFF_BASE_MS = 1000; const BACKOFF_CAP_MS = 30000; +/** + * Timeout for the initial WebSocket connection handshake. + * Guards against Node.js 22's built-in WebSocket silently hanging. + */ +const CONNECTION_TIMEOUT_MS = 30_000; /** * Calculate reconnect delay with exponential backoff and jitter. @@ -100,16 +105,26 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { let ws: WebSocket | null = null; let lastEventId = 0; let reconnectTimeoutId: ReturnType | null = null; + let connectionTimeoutId: ReturnType | null = null; let intentionalDisconnect = false; let currentTicket = config.ticket; let currentTicketExpiresAt = config.ticketExpiresAt; let ticketRefreshAttempted = false; + /** Tracks whether onerror already triggered close handling (Node.js 22 workaround). */ + let errorHandledAsClose = false; function setState(newState: ConnectionState) { state = newState; config.onStateChange(state); } + function clearConnectionTimeout() { + if (connectionTimeoutId !== null) { + clearTimeout(connectionTimeoutId); + connectionTimeoutId = null; + } + } + function buildUrl(fromId?: number): string { const url = new URL(config.url); url.searchParams.set('ticket', currentTicket); @@ -119,6 +134,52 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { return url.toString(); } + /** + * Central close-handling logic, shared by onclose and the Node.js onerror fallback. + */ + function handleClose(code: number, reason: string) { + clearConnectionTimeout(); + + console.log('[WebSocketManager] handleClose', { + code, + reason, + intentionalDisconnect, + ticketRefreshAttempted, + currentState: state.status, + }); + + if (intentionalDisconnect) { + setState({ status: 'disconnected' }); + return; + } + + const isAuthFailure = + AUTH_FAILURE_CLOSE_CODES.includes(code as (typeof AUTH_FAILURE_CLOSE_CODES)[number]) || + AUTH_FAILURE_KEYWORDS.some(kw => reason.toLowerCase().includes(kw)); + + if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { + console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); + void refreshTicketAndReconnect(); + return; + } + + if (isAuthFailure && ticketRefreshAttempted) { + console.log('[WebSocketManager] Auth failure after ticket refresh - stopping retries'); + setState({ + status: 'error', + error: 'Authentication failed after ticket refresh. Check server configuration.', + retryable: false, + }); + return; + } + + if (state.status === 'connecting' || state.status === 'connected') { + scheduleReconnect(0); + } else if (state.status === 'reconnecting') { + scheduleReconnect(state.attempt); + } + } + async function refreshTicketAndReconnect({ markAttempted = true } = {}) { console.log('[WebSocketManager] refreshTicketAndReconnect called', { hasRefreshHandler: !!config.onRefreshTicket, @@ -185,6 +246,8 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { } function connectInternal(_attempt = 0) { + clearConnectionTimeout(); + // Refresh ticket preemptively if it's expired/near-expiry before opening a new socket if ( isTicketExpiringSoon(currentTicketExpiresAt) && @@ -203,15 +266,50 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { oldWs.close(); } + // Reset per-connection flag + errorHandledAsClose = false; + // Always include lastEventId if we have one - this enables replay after any reconnection // (whether from network issues, ticket refresh, or other disconnects) const urlWithReplay = lastEventId ? buildUrl(lastEventId) : buildUrl(); setState({ status: 'connecting' }); - const newWs = new WebSocket(urlWithReplay); + let newWs: WebSocket; + try { + newWs = new WebSocket(urlWithReplay); + } catch (err) { + console.error('[WebSocketManager] Failed to create WebSocket:', err); + setState({ + status: 'error', + error: `WebSocket constructor error: ${err instanceof Error ? err.message : String(err)}`, + retryable: true, + }); + return; + } ws = newWs; + // Connection timeout — guards against Node.js WebSocket hanging + connectionTimeoutId = setTimeout(() => { + connectionTimeoutId = null; + if (ws === newWs && state.status === 'connecting') { + console.log( + '[WebSocketManager] Connection timeout - no messages received within', + CONNECTION_TIMEOUT_MS, + 'ms' + ); + ws = null; + try { + newWs.close(); + } catch { + /* ignore */ + } + handleClose(1006, 'Connection timeout'); + } + }, CONNECTION_TIMEOUT_MS); + newWs.onmessage = (messageEvent: MessageEvent) => { + clearConnectionTimeout(); + const parsed = parseMessage(messageEvent.data); if (parsed === null) { return; @@ -236,72 +334,42 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { }; newWs.onerror = (errorEvent: Event) => { - // WebSocket errors during HTTP upgrade (like 401) may not give us useful close codes - // Log the error for debugging + const errorMessage = + 'message' in errorEvent + ? String((errorEvent as { message: unknown }).message) + : 'Unknown error'; console.log('[WebSocketManager] WebSocket error', { type: errorEvent.type, + message: errorMessage, + readyState: newWs.readyState, ticketRefreshAttempted, currentState: state.status, }); - // The actual handling happens in onclose which fires after onerror + + // Node.js 22 workaround: fires onerror but NOT onclose on connection failure. + if (newWs.readyState === WebSocket.CONNECTING || newWs.readyState === WebSocket.CLOSED) { + errorHandledAsClose = true; + ws = null; + clearConnectionTimeout(); + handleClose(1006, errorMessage); + } }; newWs.onclose = (event: CloseEvent) => { - // Ignore close events from replaced sockets - a new socket has already been created - if (ws !== newWs) { + // Ignore close events from replaced sockets + if (ws !== newWs && !errorHandledAsClose) { console.log('[WebSocketManager] Ignoring close from replaced socket'); return; } - ws = null; - - console.log('[WebSocketManager] WebSocket closed', { - code: event.code, - reason: event.reason, - wasClean: event.wasClean, - intentionalDisconnect, - ticketRefreshAttempted, - currentState: state.status, - }); - if (intentionalDisconnect) { - setState({ status: 'disconnected' }); + // If onerror already handled this (Node.js 22 workaround), skip. + if (errorHandledAsClose) { + console.log('[WebSocketManager] Ignoring onclose - already handled via onerror'); return; } - const isAuthFailure = isAuthFailureClose(event); - - console.log('[WebSocketManager] Auth failure check', { - isAuthFailure, - ticketRefreshAttempted, - hasRefreshHandler: !!config.onRefreshTicket, - willRefresh: isAuthFailure && !ticketRefreshAttempted && !!config.onRefreshTicket, - }); - - if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { - console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); - void refreshTicketAndReconnect(); - return; - } - - // If we already tried refreshing the ticket and still getting auth failures, - // don't keep retrying - the issue is likely not the ticket - if (isAuthFailure && ticketRefreshAttempted) { - console.log( - '[WebSocketManager] Auth failure after ticket refresh - stopping retries (likely origin/config issue)' - ); - setState({ - status: 'error', - error: 'Authentication failed after ticket refresh. Check server configuration.', - retryable: false, - }); - return; - } - - if (state.status === 'connecting' || state.status === 'connected') { - scheduleReconnect(0); - } else if (state.status === 'reconnecting') { - scheduleReconnect(state.attempt); - } + ws = null; + handleClose(event.code, event.reason ?? ''); }; } @@ -315,6 +383,7 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { function disconnect() { intentionalDisconnect = true; + clearConnectionTimeout(); if (reconnectTimeoutId !== null) { clearTimeout(reconnectTimeoutId); diff --git a/src/lib/slack-bot.ts b/src/lib/slack-bot.ts index 97980a134..867b0b114 100644 --- a/src/lib/slack-bot.ts +++ b/src/lib/slack-bot.ts @@ -155,7 +155,6 @@ const SPAWN_CLOUD_AGENT_TOOL: OpenAI.Chat.Completions.ChatCompletionTool = { }, }, required: ['prompt'], - oneOf: [{ required: ['githubRepo'] }, { required: ['gitlabProject'] }], }, }, }; diff --git a/src/lib/slack-bot/gitlab-repository-context.test.ts b/src/lib/slack-bot/gitlab-repository-context.test.ts index 70284d155..a158bae82 100644 --- a/src/lib/slack-bot/gitlab-repository-context.test.ts +++ b/src/lib/slack-bot/gitlab-repository-context.test.ts @@ -20,7 +20,7 @@ describe('formatGitLabRepositoriesForPrompt', () => { expect(result).toContain('GitLab repository context'); expect(result).toContain('Account: gitlab-user'); - expect(result).toContain('Instance: https://gitlab.com'); + expect(result).toContain('Instance: gitlab.com'); expect(result).toContain('Repository access: selected'); expect(result).toContain('Repositories synced at: 2024-01-15T10:00:00Z'); expect(result).toContain('mygroup/project-a [id: 1]'); @@ -57,7 +57,7 @@ describe('formatGitLabRepositoriesForPrompt', () => { expect(result).toContain('No GitLab repositories are currently connected'); }); - test('includes self-hosted instance URL', () => { + test('redacts self-hosted instance URL to prevent leaking internal hostnames', () => { const context: GitLabRepositoryContext = { accountLogin: 'admin', repositoryAccess: 'selected', @@ -68,7 +68,8 @@ describe('formatGitLabRepositoriesForPrompt', () => { const result = formatGitLabRepositoriesForPrompt(context); - expect(result).toContain('Instance: https://gitlab.example.com'); + expect(result).toContain('Instance: self-hosted GitLab'); + expect(result).not.toContain('gitlab.example.com'); expect(result).toContain('team/internal (private) [id: 10]'); }); diff --git a/src/lib/slack-bot/gitlab-repository-context.ts b/src/lib/slack-bot/gitlab-repository-context.ts index eacf6d480..5d5e6a702 100644 --- a/src/lib/slack-bot/gitlab-repository-context.ts +++ b/src/lib/slack-bot/gitlab-repository-context.ts @@ -39,6 +39,17 @@ export async function getGitLabRepositoryContext(owner: Owner): Promise Date: Fri, 27 Feb 2026 12:11:21 +0100 Subject: [PATCH 4/5] chore: remove not needed changes --- src/lib/cloud-agent-next/websocket-manager.ts | 181 +++++------------- src/lib/cloud-agent/websocket-manager.ts | 173 +++++------------ 2 files changed, 101 insertions(+), 253 deletions(-) diff --git a/src/lib/cloud-agent-next/websocket-manager.ts b/src/lib/cloud-agent-next/websocket-manager.ts index 435496cfe..bd259ce12 100644 --- a/src/lib/cloud-agent-next/websocket-manager.ts +++ b/src/lib/cloud-agent-next/websocket-manager.ts @@ -49,12 +49,6 @@ function parseMessage(data: unknown): ParsedMessage | null { const MAX_RECONNECT_ATTEMPTS = 8; const BACKOFF_BASE_MS = 1000; const BACKOFF_CAP_MS = 30000; -/** - * Timeout for the initial WebSocket connection handshake. - * If the socket doesn't open within this window, we treat it as a connection failure. - * This guards against Node.js 22's built-in WebSocket silently hanging. - */ -const CONNECTION_TIMEOUT_MS = 30_000; /** * Calculate reconnect delay with exponential backoff and jitter. @@ -97,25 +91,15 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { let ws: WebSocket | null = null; let lastEventId = 0; let reconnectTimeoutId: ReturnType | null = null; - let connectionTimeoutId: ReturnType | null = null; let intentionalDisconnect = false; let currentTicket = config.ticket; let ticketRefreshAttempted = false; - /** Tracks whether onerror already triggered close handling (Node.js 22 workaround). */ - let errorHandledAsClose = false; function setState(newState: ConnectionState) { state = newState; config.onStateChange(state); } - function clearConnectionTimeout() { - if (connectionTimeoutId !== null) { - clearTimeout(connectionTimeoutId); - connectionTimeoutId = null; - } - } - function buildUrl(fromId?: number): string { const url = new URL(config.url); url.searchParams.set('ticket', currentTicket); @@ -125,54 +109,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { return url.toString(); } - /** - * Central close-handling logic, shared by onclose and the Node.js onerror fallback. - * @param code WebSocket close code (1006 when synthesised from onerror) - * @param reason Human-readable reason string - */ - function handleClose(code: number, reason: string) { - clearConnectionTimeout(); - - console.log('[WebSocketManager] handleClose', { - code, - reason, - intentionalDisconnect, - ticketRefreshAttempted, - currentState: state.status, - }); - - if (intentionalDisconnect) { - setState({ status: 'disconnected' }); - return; - } - - const isAuthFailure = - AUTH_FAILURE_CLOSE_CODES.includes(code as (typeof AUTH_FAILURE_CLOSE_CODES)[number]) || - AUTH_FAILURE_KEYWORDS.some(kw => reason.toLowerCase().includes(kw)); - - if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { - console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); - void refreshTicketAndReconnect(); - return; - } - - if (isAuthFailure && ticketRefreshAttempted) { - console.log('[WebSocketManager] Auth failure after ticket refresh - stopping retries'); - setState({ - status: 'error', - error: 'Authentication failed after ticket refresh. Check server configuration.', - retryable: false, - }); - return; - } - - if (state.status === 'connecting' || state.status === 'connected') { - scheduleReconnect(0); - } else if (state.status === 'reconnecting') { - scheduleReconnect(state.attempt); - } - } - async function refreshTicketAndReconnect() { console.log('[WebSocketManager] refreshTicketAndReconnect called', { hasRefreshHandler: !!config.onRefreshTicket, @@ -232,8 +168,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { } function connectInternal(_attempt = 0) { - clearConnectionTimeout(); - // Close existing socket if any - store reference so onclose handler can ignore it const oldWs = ws; if (oldWs !== null) { @@ -241,52 +175,15 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { oldWs.close(); } - // Reset per-connection flag - errorHandledAsClose = false; - // Always include lastEventId if we have one - this enables replay after any reconnection // (whether from network issues, ticket refresh, or other disconnects) const urlWithReplay = lastEventId ? buildUrl(lastEventId) : buildUrl(); setState({ status: 'connecting' }); - let newWs: WebSocket; - try { - newWs = new WebSocket(urlWithReplay); - } catch (err) { - console.error('[WebSocketManager] Failed to create WebSocket:', err); - setState({ - status: 'error', - error: `WebSocket constructor error: ${err instanceof Error ? err.message : String(err)}`, - retryable: true, - }); - return; - } + const newWs = new WebSocket(urlWithReplay); ws = newWs; - // Connection timeout — if we don't receive a message within CONNECTION_TIMEOUT_MS, - // treat it as a failed connection. This guards against Node.js WebSocket hanging - // without firing onclose. - connectionTimeoutId = setTimeout(() => { - connectionTimeoutId = null; - if (ws === newWs && state.status === 'connecting') { - console.log( - '[WebSocketManager] Connection timeout - no messages received within', - CONNECTION_TIMEOUT_MS, - 'ms' - ); - ws = null; - try { - newWs.close(); - } catch { - /* ignore */ - } - handleClose(1006, 'Connection timeout'); - } - }, CONNECTION_TIMEOUT_MS); - newWs.onmessage = (messageEvent: MessageEvent) => { - clearConnectionTimeout(); - const parsed = parseMessage(messageEvent.data); if (parsed === null) { return; @@ -311,51 +208,72 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { }; newWs.onerror = (errorEvent: Event) => { - const errorMessage = - 'message' in errorEvent - ? String((errorEvent as { message: unknown }).message) - : 'Unknown error'; + // WebSocket errors during HTTP upgrade (like 401) may not give us useful close codes + // Log the error for debugging console.log('[WebSocketManager] WebSocket error', { type: errorEvent.type, - message: errorMessage, - readyState: newWs.readyState, ticketRefreshAttempted, currentState: state.status, }); - - // Node.js 22 workaround: the built-in WebSocket fires onerror but NOT onclose - // when a connection fails (e.g., server unreachable or HTTP error before upgrade). - // readyState stays at CONNECTING (0). In browsers, onclose always fires after - // onerror, but Node.js 22's undici-based WebSocket doesn't follow this. - // Detect this case and trigger close handling directly. - if (newWs.readyState === WebSocket.CONNECTING || newWs.readyState === WebSocket.CLOSED) { - // Mark that we handled this error as a close so the real onclose (if it - // does eventually fire) won't double-process. - errorHandledAsClose = true; - ws = null; - clearConnectionTimeout(); - handleClose(1006, errorMessage); - } - // Otherwise, wait for the onclose event (browser behavior). + // The actual handling happens in onclose which fires after onerror }; newWs.onclose = (event: CloseEvent) => { // Ignore close events from replaced sockets - a new socket has already been created - if (ws !== newWs && !errorHandledAsClose) { + if (ws !== newWs) { console.log('[WebSocketManager] Ignoring close from replaced socket'); return; } + ws = null; + + console.log('[WebSocketManager] WebSocket closed', { + code: event.code, + reason: event.reason, + wasClean: event.wasClean, + intentionalDisconnect, + ticketRefreshAttempted, + currentState: state.status, + }); + + if (intentionalDisconnect) { + setState({ status: 'disconnected' }); + return; + } + + const isAuthFailure = isAuthFailureClose(event); + + console.log('[WebSocketManager] Auth failure check', { + isAuthFailure, + ticketRefreshAttempted, + hasRefreshHandler: !!config.onRefreshTicket, + willRefresh: isAuthFailure && !ticketRefreshAttempted && !!config.onRefreshTicket, + }); + + if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { + console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); + void refreshTicketAndReconnect(); + return; + } - // If onerror already handled this as a close (Node.js 22 workaround), skip. - if (errorHandledAsClose) { + // If we already tried refreshing the ticket and still getting auth failures, + // don't keep retrying - the issue is likely not the ticket + if (isAuthFailure && ticketRefreshAttempted) { console.log( - '[WebSocketManager] Ignoring onclose - already handled via onerror (Node.js 22 workaround)' + '[WebSocketManager] Auth failure after ticket refresh - stopping retries (likely origin/config issue)' ); + setState({ + status: 'error', + error: 'Authentication failed after ticket refresh. Check server configuration.', + retryable: false, + }); return; } - ws = null; - handleClose(event.code, event.reason ?? ''); + if (state.status === 'connecting' || state.status === 'connected') { + scheduleReconnect(0); + } else if (state.status === 'reconnecting') { + scheduleReconnect(state.attempt); + } }; } @@ -369,7 +287,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { function disconnect() { intentionalDisconnect = true; - clearConnectionTimeout(); if (reconnectTimeoutId !== null) { clearTimeout(reconnectTimeoutId); diff --git a/src/lib/cloud-agent/websocket-manager.ts b/src/lib/cloud-agent/websocket-manager.ts index 2d0117dd2..3bed0b2c1 100644 --- a/src/lib/cloud-agent/websocket-manager.ts +++ b/src/lib/cloud-agent/websocket-manager.ts @@ -44,11 +44,6 @@ function parseMessage(data: unknown): ParsedMessage | null { const MAX_RECONNECT_ATTEMPTS = 8; const BACKOFF_BASE_MS = 1000; const BACKOFF_CAP_MS = 30000; -/** - * Timeout for the initial WebSocket connection handshake. - * Guards against Node.js 22's built-in WebSocket silently hanging. - */ -const CONNECTION_TIMEOUT_MS = 30_000; /** * Calculate reconnect delay with exponential backoff and jitter. @@ -105,26 +100,16 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { let ws: WebSocket | null = null; let lastEventId = 0; let reconnectTimeoutId: ReturnType | null = null; - let connectionTimeoutId: ReturnType | null = null; let intentionalDisconnect = false; let currentTicket = config.ticket; let currentTicketExpiresAt = config.ticketExpiresAt; let ticketRefreshAttempted = false; - /** Tracks whether onerror already triggered close handling (Node.js 22 workaround). */ - let errorHandledAsClose = false; function setState(newState: ConnectionState) { state = newState; config.onStateChange(state); } - function clearConnectionTimeout() { - if (connectionTimeoutId !== null) { - clearTimeout(connectionTimeoutId); - connectionTimeoutId = null; - } - } - function buildUrl(fromId?: number): string { const url = new URL(config.url); url.searchParams.set('ticket', currentTicket); @@ -134,52 +119,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { return url.toString(); } - /** - * Central close-handling logic, shared by onclose and the Node.js onerror fallback. - */ - function handleClose(code: number, reason: string) { - clearConnectionTimeout(); - - console.log('[WebSocketManager] handleClose', { - code, - reason, - intentionalDisconnect, - ticketRefreshAttempted, - currentState: state.status, - }); - - if (intentionalDisconnect) { - setState({ status: 'disconnected' }); - return; - } - - const isAuthFailure = - AUTH_FAILURE_CLOSE_CODES.includes(code as (typeof AUTH_FAILURE_CLOSE_CODES)[number]) || - AUTH_FAILURE_KEYWORDS.some(kw => reason.toLowerCase().includes(kw)); - - if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { - console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); - void refreshTicketAndReconnect(); - return; - } - - if (isAuthFailure && ticketRefreshAttempted) { - console.log('[WebSocketManager] Auth failure after ticket refresh - stopping retries'); - setState({ - status: 'error', - error: 'Authentication failed after ticket refresh. Check server configuration.', - retryable: false, - }); - return; - } - - if (state.status === 'connecting' || state.status === 'connected') { - scheduleReconnect(0); - } else if (state.status === 'reconnecting') { - scheduleReconnect(state.attempt); - } - } - async function refreshTicketAndReconnect({ markAttempted = true } = {}) { console.log('[WebSocketManager] refreshTicketAndReconnect called', { hasRefreshHandler: !!config.onRefreshTicket, @@ -246,8 +185,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { } function connectInternal(_attempt = 0) { - clearConnectionTimeout(); - // Refresh ticket preemptively if it's expired/near-expiry before opening a new socket if ( isTicketExpiringSoon(currentTicketExpiresAt) && @@ -266,50 +203,15 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { oldWs.close(); } - // Reset per-connection flag - errorHandledAsClose = false; - // Always include lastEventId if we have one - this enables replay after any reconnection // (whether from network issues, ticket refresh, or other disconnects) const urlWithReplay = lastEventId ? buildUrl(lastEventId) : buildUrl(); setState({ status: 'connecting' }); - let newWs: WebSocket; - try { - newWs = new WebSocket(urlWithReplay); - } catch (err) { - console.error('[WebSocketManager] Failed to create WebSocket:', err); - setState({ - status: 'error', - error: `WebSocket constructor error: ${err instanceof Error ? err.message : String(err)}`, - retryable: true, - }); - return; - } + const newWs = new WebSocket(urlWithReplay); ws = newWs; - // Connection timeout — guards against Node.js WebSocket hanging - connectionTimeoutId = setTimeout(() => { - connectionTimeoutId = null; - if (ws === newWs && state.status === 'connecting') { - console.log( - '[WebSocketManager] Connection timeout - no messages received within', - CONNECTION_TIMEOUT_MS, - 'ms' - ); - ws = null; - try { - newWs.close(); - } catch { - /* ignore */ - } - handleClose(1006, 'Connection timeout'); - } - }, CONNECTION_TIMEOUT_MS); - newWs.onmessage = (messageEvent: MessageEvent) => { - clearConnectionTimeout(); - const parsed = parseMessage(messageEvent.data); if (parsed === null) { return; @@ -334,42 +236,72 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { }; newWs.onerror = (errorEvent: Event) => { - const errorMessage = - 'message' in errorEvent - ? String((errorEvent as { message: unknown }).message) - : 'Unknown error'; + // WebSocket errors during HTTP upgrade (like 401) may not give us useful close codes + // Log the error for debugging console.log('[WebSocketManager] WebSocket error', { type: errorEvent.type, - message: errorMessage, - readyState: newWs.readyState, ticketRefreshAttempted, currentState: state.status, }); - - // Node.js 22 workaround: fires onerror but NOT onclose on connection failure. - if (newWs.readyState === WebSocket.CONNECTING || newWs.readyState === WebSocket.CLOSED) { - errorHandledAsClose = true; - ws = null; - clearConnectionTimeout(); - handleClose(1006, errorMessage); - } + // The actual handling happens in onclose which fires after onerror }; newWs.onclose = (event: CloseEvent) => { - // Ignore close events from replaced sockets - if (ws !== newWs && !errorHandledAsClose) { + // Ignore close events from replaced sockets - a new socket has already been created + if (ws !== newWs) { console.log('[WebSocketManager] Ignoring close from replaced socket'); return; } + ws = null; + + console.log('[WebSocketManager] WebSocket closed', { + code: event.code, + reason: event.reason, + wasClean: event.wasClean, + intentionalDisconnect, + ticketRefreshAttempted, + currentState: state.status, + }); - // If onerror already handled this (Node.js 22 workaround), skip. - if (errorHandledAsClose) { - console.log('[WebSocketManager] Ignoring onclose - already handled via onerror'); + if (intentionalDisconnect) { + setState({ status: 'disconnected' }); return; } - ws = null; - handleClose(event.code, event.reason ?? ''); + const isAuthFailure = isAuthFailureClose(event); + + console.log('[WebSocketManager] Auth failure check', { + isAuthFailure, + ticketRefreshAttempted, + hasRefreshHandler: !!config.onRefreshTicket, + willRefresh: isAuthFailure && !ticketRefreshAttempted && !!config.onRefreshTicket, + }); + + if (isAuthFailure && !ticketRefreshAttempted && config.onRefreshTicket) { + console.log('[WebSocketManager] Auth failure detected, attempting ticket refresh'); + void refreshTicketAndReconnect(); + return; + } + + // If we already tried refreshing the ticket and still getting auth failures, + // don't keep retrying - the issue is likely not the ticket + if (isAuthFailure && ticketRefreshAttempted) { + console.log( + '[WebSocketManager] Auth failure after ticket refresh - stopping retries (likely origin/config issue)' + ); + setState({ + status: 'error', + error: 'Authentication failed after ticket refresh. Check server configuration.', + retryable: false, + }); + return; + } + + if (state.status === 'connecting' || state.status === 'connected') { + scheduleReconnect(0); + } else if (state.status === 'reconnecting') { + scheduleReconnect(state.attempt); + } }; } @@ -383,7 +315,6 @@ export function createWebSocketManager(config: WebSocketManagerConfig): { function disconnect() { intentionalDisconnect = true; - clearConnectionTimeout(); if (reconnectTimeoutId !== null) { clearTimeout(reconnectTimeoutId); From cf431b0ede03e73dd18d860a53e0254d8aa47ad9 Mon Sep 17 00:00:00 2001 From: Remon Oldenbeuving Date: Fri, 27 Feb 2026 13:28:29 +0100 Subject: [PATCH 5/5] Revert unrelated changes --- .../cloud-agent-next/cloud-agent-client.ts | 9 +++-- src/lib/cloud-agent-next/run-session.ts | 33 +++---------------- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/lib/cloud-agent-next/cloud-agent-client.ts b/src/lib/cloud-agent-next/cloud-agent-client.ts index a2752c77d..9060e8638 100644 --- a/src/lib/cloud-agent-next/cloud-agent-client.ts +++ b/src/lib/cloud-agent-next/cloud-agent-client.ts @@ -13,12 +13,11 @@ import { INTERNAL_API_SECRET } from '@/lib/config.server'; * Client for the new cloud-agent-next worker that uses the V2 WebSocket-based API * with the new message format (Message + Part[]). * - * Falls back to CLOUD_AGENT_API_URL (the original cloud-agent worker) when - * CLOUD_AGENT_NEXT_API_URL is not configured, since both workers expose - * compatible tRPC APIs. + * PLACEHOLDER: Update CLOUD_AGENT_NEXT_API_URL when the new worker is ready. */ -const CLOUD_AGENT_NEXT_API_URL = - getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || getEnvVariable('CLOUD_AGENT_API_URL') || ''; + +// TODO: Update this URL when the new cloud-agent-next worker is deployed +const CLOUD_AGENT_NEXT_API_URL = getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || ''; // MCP server config types — CLI-native local/remote format type MCPLocalServerConfig = { diff --git a/src/lib/cloud-agent-next/run-session.ts b/src/lib/cloud-agent-next/run-session.ts index 268520d51..3fd979720 100644 --- a/src/lib/cloud-agent-next/run-session.ts +++ b/src/lib/cloud-agent-next/run-session.ts @@ -21,11 +21,8 @@ import type { PrepareSessionInput, InitiateFromPreparedSessionInput } from './cl // Constants // --------------------------------------------------------------------------- -const CLOUD_AGENT_NEXT_WS_URL = - getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_NEXT_WS_URL') || - getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_WS_URL'); -const CLOUD_AGENT_NEXT_API_URL = - getEnvVariable('CLOUD_AGENT_NEXT_API_URL') || getEnvVariable('CLOUD_AGENT_API_URL'); +const CLOUD_AGENT_NEXT_WS_URL = getEnvVariable('NEXT_PUBLIC_CLOUD_AGENT_NEXT_WS_URL'); +const CLOUD_AGENT_NEXT_API_URL = getEnvVariable('CLOUD_AGENT_NEXT_API_URL'); const DEFAULT_STREAM_TIMEOUT_MS = 15 * 60 * 1000; // 15 minutes const COMPLETE_GRACE_MS = 1000; // Wait 1s after 'complete' for final events @@ -183,7 +180,6 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise { - console.log(`${logPrefix} WebSocket state: ${state.status}`, state); if (state.status === 'error') { hasError = true; errorMessage = state.error; @@ -340,10 +332,7 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise { @@ -356,19 +345,7 @@ export async function runSessionToCompletion(input: RunSessionInput): Promise