From 4ec62cd4e79497289c6277b9cc8cbe5db93d9328 Mon Sep 17 00:00:00 2001 From: ioplex Date: Fri, 23 Oct 2026 11:42:37 -0400 Subject: [PATCH 01/12] fix: address findings from the full Docker evaluation of main Critical and high: - repos: unlinking a local repo removes only the Manager reference and alias, never the repository files; confirm button reads "Unlink" - auth: rate-limit by the socket address; trust proxy headers only when AUTH_TRUSTED_PROXY_HEADERS is set (documented and forwarded in compose) - composer: per-session in-memory drafts (text, bash mode, attachments); Escape no longer clears the prompt - url params: compose same-tick updates so schedule dialogs stay closed after create/delete; delete dialog names the job Medium: - tool rows show failures (non-zero shell exit, failed exploration members) - voice conversation shows the specific error and a starting state - settings: Escape in dropdowns and the shortcut recorder no longer closes the dialog; mobile deep links open the requested section; one content tree per platform - closing pushed sheets, settings, tool panels and schedule dialogs goes back in history - login returns to the requested page; signed-out pages make no authenticated requests - action terminals keep their output (Manager-side output capture and replay) - unpushed-commit detection, commit message kept on failure - binary files with unknown extensions are detected and not editable - tool panel resizes when docked and floats as a dismissible sheet when the page is too narrow - Docker image runs tini as PID 1 Low: not-found states, add-repo and branch validation errors (4xx), search-aware empty states, mobile repo sort/filter/select cues, sidebar resize affordance and collapse shortcut, mobile unread badge, preview port filtering, terminal close handling, walkthrough empty state and helper session filtering, placeholder contrast, permission labels and focus, TTS task-list sanitizing, git identity email validation, quieter git logging, hermetic worktree tests, and accessible names for icon-only controls. --- .env.example | 8 + Dockerfile | 3 +- backend/src/auth/client-ip.ts | 44 +++ backend/src/auth/index.ts | 4 + backend/src/db/change-walkthroughs.ts | 7 + backend/src/routes/auth.ts | 6 +- backend/src/routes/change-walkthroughs.ts | 4 + backend/src/routes/repos.ts | 2 +- backend/src/routes/session-sandbox.ts | 21 +- backend/src/services/change-walkthroughs.ts | 8 + backend/src/services/files.ts | 47 ++- backend/src/services/git/GitService.ts | 22 +- backend/src/services/preview/ports.ts | 2 +- backend/src/services/repo.ts | 104 ++++-- backend/src/services/terminal-hub.ts | 327 ++++++++++++++++++ backend/src/services/terminal.ts | 65 +++- backend/src/utils/process.ts | 10 +- backend/test/auth/client-ip.test.ts | 233 +++++++++++++ backend/test/auth/index.test.ts | 2 + backend/test/routes/auth.test.ts | 2 + .../test/routes/change-walkthroughs.test.ts | 13 +- backend/test/routes/repos.test.ts | 37 ++ backend/test/routes/session-sandbox.test.ts | 39 ++- backend/test/routes/settings.test.ts | 53 +++ backend/test/scripts/docker-config.test.ts | 14 + .../test/services/change-walkthroughs.test.ts | 17 + backend/test/services/files.test.ts | 44 +++ .../test/services/git/GitService.real.test.ts | 118 ++++++- backend/test/services/preview/ports.test.ts | 20 ++ backend/test/services/repo-git.test.ts | 100 +++++- backend/test/services/repo-worktree.test.ts | 8 +- .../test/services/schedule-worktree.test.ts | 6 +- backend/test/services/terminal-hub.test.ts | 167 +++++++++ backend/test/services/terminal.test.ts | 156 ++++++++- backend/test/utils/process.test.ts | 20 +- docker-compose.release.yml | 1 + docker-compose.yml | 1 + e2e/fixtures.ts | 13 - frontend/src/App.tsx | 207 ++++++----- frontend/src/api/changeWalkthroughs.ts | 7 + frontend/src/api/repos.ts | 1 + .../components/file-browser/FileBrowser.tsx | 4 +- .../components/file-browser/FileDiffView.tsx | 1 + .../file-browser/FileOperations.tsx | 6 +- .../file-browser/FilePreview.test.tsx | 8 + .../components/file-browser/FilePreview.tsx | 8 +- .../components/file-browser/FileTree.test.tsx | 31 ++ .../src/components/file-browser/FileTree.tsx | 3 +- .../message/ExplorationGroup.test.tsx | 85 +++++ .../components/message/ExplorationGroup.tsx | 11 +- .../message/PromptInput.command.test.tsx | 6 +- .../message/PromptInput.composer.test.tsx | 2 + .../message/PromptInput.draft.test.tsx | 258 ++++++++++++++ .../message/PromptInput.goal.test.tsx | 2 + .../message/PromptInput.mention.test.tsx | 29 +- .../message/PromptInput.stt.test.tsx | 3 + .../src/components/message/PromptInput.tsx | 47 ++- .../src/components/message/TextPart.test.tsx | 15 + frontend/src/components/message/TextPart.tsx | 2 +- .../components/message/ToolCallPart.test.tsx | 72 ++++ .../src/components/message/ToolCallPart.tsx | 12 +- .../message/VoiceConversationBar.test.tsx | 16 + .../message/VoiceConversationBar.tsx | 9 +- .../src/components/model/ModelQuickSelect.tsx | 2 +- .../navigation/MobileTabBar.test.tsx | 43 +++ .../components/navigation/MobileTabBar.tsx | 7 +- .../components/navigation/MoreDrawer.test.tsx | 7 +- .../src/components/navigation/MoreDrawer.tsx | 2 +- .../navigation/RepoQuickSwitchSheet.test.tsx | 40 ++- .../navigation/RepoQuickSwitchSheet.tsx | 6 +- .../navigation/ToolSidePanel.test.tsx | 142 ++++++++ .../components/navigation/ToolSidePanel.tsx | 239 +++++++++++-- .../components/repo/AddRepoDialog.test.tsx | 83 +++++ .../src/components/repo/AddRepoDialog.tsx | 74 ++-- .../components/repo/MultiRunSheet.test.tsx | 19 + .../src/components/repo/MultiRunSheet.tsx | 24 +- .../src/components/repo/RepoList.test.tsx | 116 ++++++- frontend/src/components/repo/RepoList.tsx | 13 +- .../components/repo/RepoListControls.test.tsx | 59 ++++ .../src/components/repo/RepoListControls.tsx | 55 ++- .../src/components/repo/RepoRowActions.tsx | 5 +- .../components/repo/repo-list-state.test.ts | 19 + .../src/components/repo/repo-list-state.ts | 8 + .../src/components/schedules/GeneralTab.tsx | 20 +- .../schedules/RepoSchedulesContent.test.tsx | 174 ++++++++++ .../schedules/RepoSchedulesContent.tsx | 18 +- .../ScheduleJobDialog.permissions.test.tsx | 44 +++ .../schedules/ScheduleJobDialog.tsx | 21 +- .../session/ChangesWalkthroughSheet.test.tsx | 33 +- .../session/ChangesWalkthroughSheet.tsx | 31 +- .../session/PermissionModeToggle.test.tsx | 8 +- .../session/PermissionRequestDialog.test.tsx | 8 + .../session/PermissionRequestDialog.tsx | 12 +- .../components/session/SessionList.test.tsx | 4 + .../components/settings/AccountSettings.tsx | 2 +- .../components/settings/GitSettings.test.tsx | 18 + .../src/components/settings/GitSettings.tsx | 22 +- .../settings/KeyboardShortcuts.test.tsx | 74 +++- .../components/settings/KeyboardShortcuts.tsx | 52 ++- .../settings/ManagerTokenSettings.tsx | 3 + .../settings/OpenCodeServerAuthSettings.tsx | 1 + .../src/components/settings/STTSettings.tsx | 4 +- .../settings/SettingsDialog.test.tsx | 209 +++++++++-- .../components/settings/SettingsDialog.tsx | 57 ++- .../settings/SettingsDialogLogs.test.tsx | 4 +- .../settings/SettingsDisclosure.tsx | 2 +- .../components/settings/TTSSettings.test.tsx | 31 +- .../src/components/settings/TTSSettings.tsx | 11 +- .../source-control/ChangesTab.test.tsx | 91 +++++ .../components/source-control/ChangesTab.tsx | 6 +- .../source-control/GitFlatFileItem.test.tsx | 117 +++++++ .../source-control/GitFlatFileItem.tsx | 21 +- .../terminal/TerminalPanel.test.tsx | 103 +++++- .../src/components/terminal/TerminalPanel.tsx | 52 ++- frontend/src/components/ui/combobox.tsx | 2 +- .../ui/confirm-destructive-dialog.tsx | 2 +- frontend/src/components/ui/delete-dialog.tsx | 8 +- frontend/src/components/ui/dialog.test.tsx | 19 + frontend/src/components/ui/dialog.tsx | 9 +- frontend/src/components/ui/input.tsx | 2 +- frontend/src/components/ui/multi-select.tsx | 2 +- .../src/components/ui/not-found-state.tsx | 23 ++ .../ui/route-error-boundary.test.tsx | 67 ++++ .../components/ui/route-error-boundary.tsx | 5 + frontend/src/components/ui/sidebar.test.tsx | 15 + frontend/src/components/ui/sidebar.tsx | 44 +-- frontend/src/components/ui/textarea.tsx | 2 +- frontend/src/contexts/AuthContext.test.tsx | 126 +++++++ frontend/src/contexts/AuthContext.tsx | 23 +- frontend/src/contexts/EventContext.test.tsx | 13 +- frontend/src/contexts/EventContext.tsx | 7 +- frontend/src/contexts/TTSContext.test.tsx | 4 + frontend/src/contexts/TTSContext.tsx | 4 +- frontend/src/contexts/ThemeContext.tsx | 4 +- .../__tests__/useScheduleUrlState.test.tsx | 84 ++++- .../src/hooks/useChangeWalkthrough.test.tsx | 2 +- frontend/src/hooks/useChangeWalkthrough.ts | 17 +- frontend/src/hooks/useDialogParam.test.tsx | 55 ++- frontend/src/hooks/useDialogParam.ts | 8 +- .../hooks/useGlobalShortcutActions.test.tsx | 31 +- .../src/hooks/useGlobalShortcutActions.ts | 37 +- frontend/src/hooks/useMobileTabBar.test.tsx | 55 ++- frontend/src/hooks/useMobileTabBar.ts | 6 +- frontend/src/hooks/useOpenCode.ts | 3 +- frontend/src/hooks/usePermissionModeToggle.ts | 4 +- frontend/src/hooks/useSTT.test.tsx | 27 ++ frontend/src/hooks/useSTT.ts | 11 +- frontend/src/hooks/useScheduleUrlState.ts | 10 +- frontend/src/hooks/useSessionSearch.test.tsx | 21 ++ frontend/src/hooks/useSessionSearch.ts | 6 +- frontend/src/hooks/useSessionTranscript.ts | 4 + frontend/src/hooks/useSettings.ts | 3 +- frontend/src/hooks/useSettingsDialog.test.tsx | 77 ++++- frontend/src/hooks/useSettingsDialog.ts | 25 +- .../src/hooks/useSidebarCollapsed.test.tsx | 15 + frontend/src/hooks/useSidebarCollapsed.ts | 53 ++- frontend/src/hooks/useTheme.test.tsx | 4 + frontend/src/hooks/useToolPanel.test.ts | 105 +++++- frontend/src/hooks/useToolPanel.ts | 58 +++- .../src/hooks/useToolPanelLayout.test.tsx | 115 ++++++ frontend/src/hooks/useToolPanelLayout.ts | 118 +++++++ frontend/src/hooks/useUrlParams.test.tsx | 89 ++++- frontend/src/hooks/useUrlParams.ts | 73 +++- .../src/hooks/useVoiceConversation.test.tsx | 64 +++- frontend/src/hooks/useVoiceConversation.ts | 11 +- frontend/src/index.css | 2 + frontend/src/lib/auth-loaders.test.ts | 104 ++++++ frontend/src/lib/auth-loaders.ts | 40 ++- frontend/src/lib/explorationGroups.test.ts | 30 +- frontend/src/lib/explorationGroups.ts | 11 +- frontend/src/lib/repoQuery.ts | 15 + frontend/src/lib/tuiKeybindImport.test.ts | 23 ++ frontend/src/lib/tuiKeybindImport.ts | 22 ++ frontend/src/lib/utils.test.ts | 13 + frontend/src/lib/utils.ts | 6 +- frontend/src/lib/voiceConversation.test.ts | 72 +++- frontend/src/pages/AssistantRedirect.tsx | 8 +- frontend/src/pages/RemoteProject.tsx | 8 +- frontend/src/pages/RepoDetail.tsx | 60 +--- frontend/src/pages/Repos.tsx | 8 +- frontend/src/pages/Schedules.tsx | 2 +- frontend/src/pages/SessionDetail.tsx | 44 ++- .../__tests__/RepoDetail.not-found.test.tsx | 132 +++++++ .../__tests__/SessionDetail.commands.test.tsx | 24 ++ .../SessionDetail.not-found.test.tsx | 225 ++++++++++++ frontend/src/stores/composerDraftStore.ts | 39 +++ shared/src/config/env.ts | 1 + shared/src/opencode/index.ts | 1 + shared/src/opencode/voiceConversation.ts | 75 +++- shared/src/schemas/change-walkthroughs.ts | 1 + shared/src/schemas/settings.ts | 13 +- 191 files changed, 6669 insertions(+), 691 deletions(-) create mode 100644 backend/src/auth/client-ip.ts create mode 100644 backend/src/services/terminal-hub.ts create mode 100644 backend/test/auth/client-ip.test.ts create mode 100644 backend/test/services/terminal-hub.test.ts create mode 100644 frontend/src/components/file-browser/FileTree.test.tsx create mode 100644 frontend/src/components/message/ExplorationGroup.test.tsx create mode 100644 frontend/src/components/message/PromptInput.draft.test.tsx create mode 100644 frontend/src/components/message/TextPart.test.tsx create mode 100644 frontend/src/components/repo/AddRepoDialog.test.tsx create mode 100644 frontend/src/components/schedules/RepoSchedulesContent.test.tsx create mode 100644 frontend/src/components/source-control/ChangesTab.test.tsx create mode 100644 frontend/src/components/source-control/GitFlatFileItem.test.tsx create mode 100644 frontend/src/components/ui/not-found-state.tsx create mode 100644 frontend/src/components/ui/route-error-boundary.test.tsx create mode 100644 frontend/src/contexts/AuthContext.test.tsx create mode 100644 frontend/src/hooks/useToolPanelLayout.test.tsx create mode 100644 frontend/src/hooks/useToolPanelLayout.ts create mode 100644 frontend/src/lib/auth-loaders.test.ts create mode 100644 frontend/src/lib/repoQuery.ts create mode 100644 frontend/src/pages/__tests__/RepoDetail.not-found.test.tsx create mode 100644 frontend/src/pages/__tests__/SessionDetail.not-found.test.tsx create mode 100644 frontend/src/stores/composerDraftStore.ts diff --git a/.env.example b/.env.example index f2a5598a8..804a9fc0b 100644 --- a/.env.example +++ b/.env.example @@ -114,6 +114,14 @@ DEBUG=false # so set true explicitly for HTTPS. # AUTH_SECURE_COOKIES=true +# Trusted proxy headers for the client IP used by auth rate limiting and session records +# (comma-separated header names, for example x-forwarded-for or cf-connecting-ip). +# Unset by default: the Manager uses the real TCP socket address and ignores any +# client-supplied forwarding header, so a header cannot be rotated to bypass the +# login rate limit. Set this only when the Manager sits behind a reverse proxy or +# tunnel that overwrites the header, and list only headers that proxy sets. +# AUTH_TRUSTED_PROXY_HEADERS=x-forwarded-for + # OAuth Providers (optional - enable by providing client ID and secret) # GITHUB_CLIENT_ID= # GITHUB_CLIENT_SECRET= diff --git a/Dockerfile b/Dockerfile index 2dad4aa2c..34ad338f4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,6 +19,7 @@ RUN apt-get update && apt-get install -y \ python3 \ python3-pip \ python3-venv \ + tini \ && rm -rf /var/lib/apt/lists/* RUN curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg \ @@ -155,5 +156,5 @@ EXPOSE 5003 5004 5100 5101 5102 5103 HEALTHCHECK --interval=30s --timeout=3s --start-period=40s --retries=3 \ CMD curl -f http://localhost:5003/api/health || exit 1 -ENTRYPOINT ["/docker-entrypoint.sh"] +ENTRYPOINT ["/usr/bin/tini", "--", "/docker-entrypoint.sh"] CMD ["bun", "backend/src/index.ts"] diff --git a/backend/src/auth/client-ip.ts b/backend/src/auth/client-ip.ts new file mode 100644 index 000000000..63c34097e --- /dev/null +++ b/backend/src/auth/client-ip.ts @@ -0,0 +1,44 @@ +import type { Context } from 'hono' +import { getConnInfo } from '@hono/node-server/conninfo' +import { ENV } from '@opencode-manager/shared/config/env' + +export const AUTH_SOCKET_IP_HEADER = 'x-ocm-auth-socket-ip' + +const DEFAULT_PROXY_HEADERS = ['x-forwarded-for'] +const HEADER_NAME_PATTERN = /^[a-z0-9!#$%&'*+.^_`|~-]+$/ + +export function getTrustedProxyHeaders(): string[] { + return ENV.AUTH.TRUSTED_PROXY_HEADERS + .split(',') + .map((header) => header.trim().toLowerCase()) + .filter((header) => HEADER_NAME_PATTERN.test(header)) +} + +export function getClientIpHeaders(): string[] { + return [...getTrustedProxyHeaders(), AUTH_SOCKET_IP_HEADER] +} + +export function resolveSocketIp(c: Context): string | undefined { + try { + return getConnInfo(c).remote.address ?? undefined + } catch { + return undefined + } +} + +export function applyClientIpHeaders(headers: Headers, socketIp: string | undefined): Headers { + const trustedProxyHeaders = getTrustedProxyHeaders() + + for (const header of DEFAULT_PROXY_HEADERS) { + if (!trustedProxyHeaders.includes(header)) { + headers.delete(header) + } + } + + headers.delete(AUTH_SOCKET_IP_HEADER) + if (socketIp) { + headers.set(AUTH_SOCKET_IP_HEADER, socketIp) + } + + return headers +} diff --git a/backend/src/auth/index.ts b/backend/src/auth/index.ts index 161527f2c..078072b91 100644 --- a/backend/src/auth/index.ts +++ b/backend/src/auth/index.ts @@ -4,6 +4,7 @@ import { passkey } from '@better-auth/passkey' import { Database } from 'bun:sqlite' import { ENV, getTrustedOrigins } from '@opencode-manager/shared/config/env' import { AUTH_COOKIE_PREFIX } from './cookies' +import { getClientIpHeaders } from './client-ip' export type AuthInstance = ReturnType @@ -90,6 +91,9 @@ export function createAuth(db: Database) { advanced: { cookiePrefix: AUTH_COOKIE_PREFIX, useSecureCookies: ENV.AUTH.SECURE_COOKIES, + ipAddress: { + ipAddressHeaders: getClientIpHeaders(), + }, }, }) diff --git a/backend/src/db/change-walkthroughs.ts b/backend/src/db/change-walkthroughs.ts index bca5b5f7a..0e9276593 100644 --- a/backend/src/db/change-walkthroughs.ts +++ b/backend/src/db/change-walkthroughs.ts @@ -222,6 +222,13 @@ export function updateChangeWalkthrough( return getChangeWalkthrough(db, sessionId, sourceKey) } +export function listChangeWalkthroughSessionIds(db: Database): string[] { + const rows = db + .prepare('SELECT DISTINCT walkthrough_session_id FROM change_walkthroughs WHERE walkthrough_session_id IS NOT NULL') + .all() as Array<{ walkthrough_session_id: string }> + return rows.map((row) => row.walkthrough_session_id) +} + export function listChangeWalkthroughs(db: Database, sessionId: string): ChangeWalkthrough[] { const rows = db .prepare(`SELECT ${ROW_COLUMNS} FROM change_walkthroughs WHERE session_id = ?`) diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts index 57e267d82..1f73ede93 100644 --- a/backend/src/routes/auth.ts +++ b/backend/src/routes/auth.ts @@ -1,5 +1,6 @@ import { Hono } from 'hono' import { isAdminConfigured, type AuthInstance } from '../auth' +import { applyClientIpHeaders, resolveSocketIp } from '../auth/client-ip' import { Database } from 'bun:sqlite' import { ENV } from '@opencode-manager/shared/config/env' import { logger } from '../utils/logger' @@ -8,7 +9,10 @@ import { hashPassword } from 'better-auth/crypto' export function createAuthRoutes(auth: AuthInstance): Hono { const app = new Hono() - app.all('/*', (c) => auth.handler(c.req.raw)) + app.all('/*', (c) => { + applyClientIpHeaders(c.req.raw.headers, resolveSocketIp(c)) + return auth.handler(c.req.raw) + }) return app } diff --git a/backend/src/routes/change-walkthroughs.ts b/backend/src/routes/change-walkthroughs.ts index add6c10eb..3e20ea28c 100644 --- a/backend/src/routes/change-walkthroughs.ts +++ b/backend/src/routes/change-walkthroughs.ts @@ -12,6 +12,10 @@ import { ServiceError } from '../utils/service-error' export function createChangeWalkthroughRoutes(service: ChangeWalkthroughService) { const app = new Hono() + app.get('/helper-sessions', (c) => { + return c.json({ sessionIds: service.listHelperSessionIds() }) + }) + app.get('/:sessionId', async (c) => { const sourceParam = c.req.query('source') const source = sourceParam === undefined ? DEFAULT_WALKTHROUGH_SOURCE : parseWalkthroughSourceKey(sourceParam) diff --git a/backend/src/routes/repos.ts b/backend/src/routes/repos.ts index 96f295a42..6e384aa22 100644 --- a/backend/src/routes/repos.ts +++ b/backend/src/routes/repos.ts @@ -498,7 +498,7 @@ app.get('/', async (c) => { return c.json({ ...updatedRepo, currentBranch }) } catch (error: unknown) { logger.error('Failed to create branch:', error) - return c.json({ error: getErrorMessage(error) }, 500) + return c.json({ error: getErrorMessage(error) }, getStatusCode(error) as ContentfulStatusCode) } }) diff --git a/backend/src/routes/session-sandbox.ts b/backend/src/routes/session-sandbox.ts index cdbc559ca..ec7ccc3ac 100644 --- a/backend/src/routes/session-sandbox.ts +++ b/backend/src/routes/session-sandbox.ts @@ -1,8 +1,23 @@ -import { Hono } from 'hono' +import { Hono, type Context } from 'hono' +import type { ContentfulStatusCode } from 'hono/utils/http-status' import { SetSessionSandboxRequestSchema } from '@opencode-manager/shared/schemas' +import { openCodeErrorStatus } from '@opencode-manager/shared/opencode' import { SessionSandboxError, type SessionSandboxService } from '../services/session-sandbox' +import { getErrorMessage } from '../utils/error-utils' import { handleServiceError, parseJsonBody } from '../utils/route-helpers' +/** + * Maps both sandbox errors and the OpenCode errors raised while resolving a session (for example + * `SessionNotFoundError` or `InvalidRequestError` for a malformed id) to their real status codes. + */ +function handleSessionSandboxError(c: Context, error: unknown, fallback: string) { + const status = openCodeErrorStatus(error) + if (status !== undefined) { + return c.json({ error: getErrorMessage(error) || fallback }, status as ContentfulStatusCode) + } + return handleServiceError(c, error, fallback, SessionSandboxError) +} + export function createSessionSandboxRoutes(service: SessionSandboxService) { const app = new Hono() @@ -11,7 +26,7 @@ export function createSessionSandboxRoutes(service: SessionSandboxService) { const state = await service.getState(c.req.param('sessionId')) return c.json(state) } catch (error) { - return handleServiceError(c, error, 'Failed to read session sandbox state', SessionSandboxError) + return handleSessionSandboxError(c, error, 'Failed to read session sandbox state') } }) @@ -25,7 +40,7 @@ export function createSessionSandboxRoutes(service: SessionSandboxService) { const state = await service.update(c.req.param('sessionId'), parsed.data) return c.json(state) } catch (error) { - return handleServiceError(c, error, 'Failed to set session sandbox state', SessionSandboxError) + return handleSessionSandboxError(c, error, 'Failed to set session sandbox state') } }) diff --git a/backend/src/services/change-walkthroughs.ts b/backend/src/services/change-walkthroughs.ts index 7a4d9fa55..b30b773f4 100644 --- a/backend/src/services/change-walkthroughs.ts +++ b/backend/src/services/change-walkthroughs.ts @@ -31,6 +31,7 @@ import { deleteChangeWalkthroughs, getChangeWalkthrough, getChangeWalkthroughByWalkthroughSession, + listChangeWalkthroughSessionIds, listChangeWalkthroughs, updateChangeWalkthrough, type ChangeWalkthroughPatch, @@ -56,6 +57,7 @@ import type { SettingsService } from './settings' import type { SSEEvent } from './sse-aggregator' const WALKTHROUGH_TIMEOUT_MESSAGE = 'The walkthrough session did not publish within 15 minutes' +const EMPTY_CHANGES_HASH = computeChangesHash([]) export interface ChangeWalkthroughServiceOptions { generationTimeoutMs?: number @@ -170,6 +172,11 @@ export class ChangeWalkthroughService { this.walkthroughsRoot = options.walkthroughsRoot ?? getWalkthroughsPath() } + /** Sessions the walkthrough runs were created in, which are helper sessions rather than user sessions. */ + listHelperSessionIds(): string[] { + return listChangeWalkthroughSessionIds(this.db) + } + async getState( sessionId: string, source: WalkthroughSource = DEFAULT_WALKTHROUGH_SOURCE, @@ -187,6 +194,7 @@ export class ChangeWalkthroughService { return { walkthrough, currentDiffHash, + hasChanges: currentDiffHash === null || currentDiffHash !== EMPTY_CHANGES_HASH, stale: walkthrough !== null && currentDiffHash !== null && walkthrough.diffHash !== currentDiffHash, generating: walkthrough?.status === 'generating', running: walkthrough !== null && walkthrough.executionStartedAt !== null, diff --git a/backend/src/services/files.ts b/backend/src/services/files.ts index 2a8d6c9ed..d230b5f26 100644 --- a/backend/src/services/files.ts +++ b/backend/src/services/files.ts @@ -40,9 +40,15 @@ interface FileUploadResult { mimeType: string } +const BINARY_MIME_TYPE = 'application/octet-stream' +const SNIFF_BYTE_COUNT = 8192 +const MAX_INVALID_UTF8_RATIO = 0.1 + +type DetectedMimeType = AllowedMimeType | typeof BINARY_MIME_TYPE + export interface RawFileMetadata { size: number - mimeType: AllowedMimeType + mimeType: DetectedMimeType lastModified: Date } @@ -85,7 +91,7 @@ export async function getRawFileMetadata(userPath: string): Promise { } else { // It's a file - get content let content = '' - const mimeType = getMimeType(validatedPath) + const mimeType = await detectMimeType(validatedPath) if (stats.size < FILE_LIMITS.MAX_SIZE_BYTES) { try { - const mimeType = getMimeType(validatedPath) - if (mimeType.startsWith('image/') || !mimeType.startsWith('text/')) { content = await readFileAsBase64(validatedPath) } else { @@ -179,7 +183,7 @@ export async function uploadFile(userPath: string, file: File, relativePath?: st throw new Error('File too large') } - const mimeType = (file.type || getMimeType(file.name)) as AllowedMimeType + const mimeType = (file.type || getKnownMimeType(file.name) || 'text/plain') as AllowedMimeType if (!ALLOWED_MIME_TYPES.includes(mimeType) && !mimeType.startsWith('text/')) { throw new Error('File type not allowed') } @@ -288,7 +292,7 @@ function validatePath(userPath: string): string { return resolved } -function getMimeType(filePath: string): AllowedMimeType { +function getKnownMimeType(filePath: string): AllowedMimeType | undefined { const ext = path.extname(filePath).toLowerCase() const mimeTypes: Record = { @@ -315,7 +319,32 @@ function getMimeType(filePath: string): AllowedMimeType { '.mdx': 'text/markdown', } - return mimeTypes[ext] || 'text/plain' + return mimeTypes[ext] +} + +async function detectMimeType(filePath: string): Promise { + const known = getKnownMimeType(filePath) + if (known) return known + return isBinaryContent(await readFilePrefix(filePath)) ? BINARY_MIME_TYPE : 'text/plain' +} + +async function readFilePrefix(filePath: string): Promise { + const handle = await fs.open(filePath, 'r') + try { + const buffer = Buffer.alloc(SNIFF_BYTE_COUNT) + const { bytesRead } = await handle.read(buffer, 0, SNIFF_BYTE_COUNT, 0) + return buffer.subarray(0, bytesRead) + } finally { + await handle.close() + } +} + +function isBinaryContent(buffer: Buffer): boolean { + if (buffer.length === 0) return false + if (buffer.includes(0)) return true + const decoded = buffer.toString('utf8') + const invalidCharacters = decoded.split('\uFFFD').length - 1 + return invalidCharacters / decoded.length > MAX_INVALID_UTF8_RATIO } async function readFileLinesAndCount( @@ -356,7 +385,7 @@ export async function getFileRange(userPath: string, startLine: number, endLine: const { lines, totalLines } = await readFileLinesAndCount(validatedPath, startLine, endLine) const clampedEnd = Math.min(endLine, totalLines) - const mimeType = getMimeType(validatedPath) + const mimeType = await detectMimeType(validatedPath) return { name: path.basename(validatedPath), diff --git a/backend/src/services/git/GitService.ts b/backend/src/services/git/GitService.ts index 9d2ff5749..1bf8860b6 100644 --- a/backend/src/services/git/GitService.ts +++ b/backend/src/services/git/GitService.ts @@ -176,12 +176,13 @@ export class GitService { } const repoPath = path.resolve(repo.fullPath) + const revision = branch?.trim() || 'HEAD' const logArgs = [ 'git', '-C', repoPath, 'log', - branch?.trim() || 'HEAD', + revision, `-n`, String(limit), '--format=%H|%an|%ae|%at|%s' @@ -210,7 +211,7 @@ export class GitService { } } - const unpushedCommits = await this.getUnpushedCommitHashes(repoPath, logEnv) + const unpushedCommits = await this.getUnpushedCommitHashes(repoPath, revision, logEnv, limit) return commits.map(commit => ({ ...commit, @@ -858,6 +859,9 @@ export class GitService { return { ahead: ahead || 0, behind: behind || 0 } } catch (error) { + if (isNoUpstreamError(error as Error)) { + return { ahead: 0, behind: 0 } + } logger.warn(`Could not get branch status for repo ${repoId}, returning zeros:`, error) return { ahead: 0, behind: 0 } } @@ -1515,16 +1519,18 @@ export class GitService { } private async isCommitUnpushed(repoPath: string, commitHash: string, env: Record): Promise { - const unpushedHashes = await this.getUnpushedCommitHashes(repoPath, env) + const unpushedHashes = await this.getUnpushedCommitHashes(repoPath, 'HEAD', env) return unpushedHashes.has(commitHash) } - private async getUnpushedCommitHashes(repoPath: string, env: Record): Promise> { + private async getUnpushedCommitHashes(repoPath: string, revision: string, env: Record, limit?: number): Promise> { try { - const output = await executeCommand( - ['git', '-C', repoPath, 'log', '--not', '--remotes', '--format=%H'], - { env, silent: true } - ) + const args = ['git', '-C', repoPath, 'log', revision, '--not', '--remotes'] + if (limit !== undefined) { + args.push('-n', String(limit)) + } + args.push('--format=%H') + const output = await executeCommand(args, { env, silent: true }) const hashes = output.trim().split('\n').filter(Boolean) return new Set(hashes) } catch { diff --git a/backend/src/services/preview/ports.ts b/backend/src/services/preview/ports.ts index b0027af02..aacd21c7e 100644 --- a/backend/src/services/preview/ports.ts +++ b/backend/src/services/preview/ports.ts @@ -96,7 +96,7 @@ function isAllowedAddress(address: string): boolean { address === WILDCARD_V4 || address === WILDCARD_V6 || address === '::1' || - address.startsWith('127.') + address === '127.0.0.1' ) } diff --git a/backend/src/services/repo.ts b/backend/src/services/repo.ts index d85acf058..ed36f9c55 100644 --- a/backend/src/services/repo.ts +++ b/backend/src/services/repo.ts @@ -27,30 +27,52 @@ const GIT_CLONE_TIMEOUT = 300000 const DEFAULT_DISCOVERY_MAX_DEPTH = 4 const DISCOVERY_SKIP_DIRECTORIES = new Set(['.git', 'node_modules']) -function enhanceCloneError(error: unknown, repoUrl: string, originalMessage: string): Error { +export class RepoInputError extends Error { + readonly statusCode: 400 + + constructor(message: string) { + super(message) + this.name = 'RepoInputError' + this.statusCode = 400 + } +} + +function notFoundOrDeniedMessage(repoUrl: string): string { + return `Repository not found or access denied: ${repoUrl}. Check the URL and add credentials in Settings > Git Credentials if it is private.` +} + +function enhanceCloneError(repoUrl: string, originalMessage: string): RepoInputError { const message = originalMessage.toLowerCase() - - if (message.includes('authentication failed') || message.includes('could not authenticate') || message.includes('invalid credentials')) { - return new Error(`Authentication failed for ${repoUrl}. Please add your credentials in Settings > Git Credentials.`) + + if (message.includes('timed out')) { + return new RepoInputError(`Clone timed out for ${repoUrl}. The repository might be too large or there could be network issues.`) } - - if (message.includes('repository not found') || message.includes('404')) { - return new Error(`Repository not found: ${repoUrl}. Check the URL and ensure you have access to it.`) + + if (message.includes('could not resolve host') || message.includes('network is unreachable')) { + return new RepoInputError(`Repository not reachable: ${repoUrl}. Check the URL and your network connection.`) } - - if (isSSHUrl(repoUrl) && message.includes('permission denied')) { - return new Error(`Access denied to ${repoUrl}. Please add your SSH credentials in Settings > Git Credentials and ensure your SSH key has access to this repository.`) + + if (message.includes('repository not found') || message.includes('not found') || message.includes('does not exist') || message.includes('404')) { + return new RepoInputError(notFoundOrDeniedMessage(repoUrl)) } - - if (isGitHubHttpsUrl(repoUrl) && (message.includes('permission denied') || message.includes('fatal'))) { - return new Error(`Access denied to ${repoUrl}. Please add your credentials in Settings > Git Credentials and ensure you have proper access.`) + + if (message.includes('permission denied')) { + return isSSHUrl(repoUrl) + ? new RepoInputError(`Access denied to ${repoUrl}. Add your SSH credentials in Settings > Git Credentials and ensure your SSH key has access.`) + : new RepoInputError(notFoundOrDeniedMessage(repoUrl)) } - - if (message.includes('timed out')) { - return new Error(`Clone timed out for ${repoUrl}. The repository might be too large or there could be network issues. Try again or verify the repository exists.`) + + if (message.includes('authentication failed') || message.includes('could not authenticate') || message.includes('invalid credentials') || message.includes('could not read username')) { + return isGitHubHttpsUrl(repoUrl) + ? new RepoInputError(notFoundOrDeniedMessage(repoUrl)) + : new RepoInputError(`Authentication failed for ${repoUrl}. Check your credentials in Settings > Git Credentials.`) } - - return error instanceof Error ? error : new Error(originalMessage) + + if (message.includes('not a git repository') || message.includes('fatal:')) { + return new RepoInputError(notFoundOrDeniedMessage(repoUrl)) + } + + return new RepoInputError(`Could not clone ${repoUrl}. Check the URL and your access to it.`) } async function hasCommits(repoPath: string, env: Record): Promise { @@ -258,12 +280,12 @@ async function registerExistingLocalRepo( const exists = await pathExists(normalizedSourcePath) if (!exists) { - throw new Error(`No such file or directory: '${normalizedSourcePath}'`) + throw new RepoInputError(`No such directory: '${normalizedSourcePath}'`) } const isGitRepo = await isValidGitRepo(normalizedSourcePath, env) if (!isGitRepo) { - throw new Error(`Directory exists but is not a valid Git repository. Use folder discovery to scan nested repositories.`) + throw new RepoInputError(`Directory is not a valid Git repository: '${normalizedSourcePath}'.`) } if (branch) { @@ -317,11 +339,11 @@ export async function discoverLocalRepos( }> { const normalizedRootPath = normalizeAbsolutePath(rootPath) const rootStats = await fs.stat(normalizedRootPath).catch((error: unknown) => { - throw new Error(`Failed to access '${normalizedRootPath}': ${getErrorMessage(error)}`) + throw new RepoInputError(`Failed to access '${normalizedRootPath}': ${getErrorMessage(error)}`) }) if (!rootStats.isDirectory()) { - throw new Error(`Path is not a directory: '${normalizedRootPath}'`) + throw new RepoInputError(`Path is not a directory: '${normalizedRootPath}'`) } const repoPaths: string[] = [] @@ -687,7 +709,7 @@ export async function cloneRepo( try { await executeCommand(['git', 'clone', normalizedRepoUrl, worktreeDirName], { cwd: getReposPath(), env, timeout: GIT_CLONE_TIMEOUT }) } catch (cloneError: unknown) { - throw enhanceCloneError(cloneError, normalizedRepoUrl, getErrorMessage(cloneError)) + throw enhanceCloneError(normalizedRepoUrl, getErrorMessage(cloneError)) } let localBranchExists = 'missing' @@ -704,7 +726,7 @@ export async function cloneRepo( await executeCommand(['git', '-C', path.resolve(getReposPath(), worktreeDirName), 'checkout', branch]) } } else { - throw enhanceCloneError(error, normalizedRepoUrl, getErrorMessage(error)) + throw enhanceCloneError(normalizedRepoUrl, getErrorMessage(error)) } } } else { @@ -801,7 +823,7 @@ export async function cloneRepo( try { await executeCommand(['git', 'clone', normalizedRepoUrl, worktreeDirName], { cwd: getReposPath(), env, timeout: GIT_CLONE_TIMEOUT }) } catch (cloneError: unknown) { - throw enhanceCloneError(cloneError, normalizedRepoUrl, getErrorMessage(cloneError)) + throw enhanceCloneError(normalizedRepoUrl, getErrorMessage(cloneError)) } let localBranchExists = 'missing' @@ -818,7 +840,7 @@ export async function cloneRepo( await executeCommand(['git', '-C', path.resolve(getReposPath(), worktreeDirName), 'checkout', branch]) } } else { - throw enhanceCloneError(error, normalizedRepoUrl, getErrorMessage(error)) + throw enhanceCloneError(normalizedRepoUrl, getErrorMessage(error)) } } } @@ -876,6 +898,14 @@ export async function switchBranch( } } +async function assertValidBranchName(branch: string): Promise { + try { + await executeCommand(['git', 'check-ref-format', '--branch', branch], { silent: true }) + } catch { + throw new RepoInputError(`Invalid branch name: '${branch}'.`) + } +} + export async function createBranch(database: Database, gitAuthService: GitAuthService, repoId: number, branch: string): Promise { const repo = getRepoById(database, repoId) if (!repo) { @@ -891,6 +921,8 @@ export async function createBranch(database: Database, gitAuthService: GitAuthSe .replace(/^refs\/remotes\//, '') .replace(/^origin\//, '') + await assertValidBranchName(sanitizedBranch) + logger.info(`Creating new branch: ${sanitizedBranch} in ${repo.localPath}`) await executeCommand(['git', '-C', repoPath, 'checkout', '-b', sanitizedBranch], { env }) logger.info(`Successfully created and switched to branch: ${sanitizedBranch}`) @@ -944,6 +976,19 @@ export async function resolveMainCheckoutPath(worktreePath: string): Promise { + try { + const stats = await fs.lstat(aliasPath) + if (stats.isSymbolicLink()) { + await fs.unlink(aliasPath) + } + } catch (error: unknown) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error + } + } +} + export async function deleteRepoFiles(database: Database, repoId: number): Promise { const repo = getRepoById(database, repoId) if (!repo) { @@ -959,9 +1004,14 @@ export async function deleteRepoFiles(database: Database, repoId: number): Promi if (baseRepoPath) { await removeWorktree(baseRepoPath, fullPath) } + + await executeCommand(['rm', '-rf', repo.localPath], getReposPath()) + } else if (repo.isLocal) { + await removeWorkspaceAlias(fullPath) + } else { + await executeCommand(['rm', '-rf', repo.localPath], getReposPath()) } - await executeCommand(['rm', '-rf', repo.localPath], getReposPath()) deleteRepo(database, repoId) } diff --git a/backend/src/services/terminal-hub.ts b/backend/src/services/terminal-hub.ts new file mode 100644 index 000000000..e530cd47b --- /dev/null +++ b/backend/src/services/terminal-hub.ts @@ -0,0 +1,327 @@ +import { + MAX_PEER_BUFFERED_BYTES, + MAX_UPSTREAM_BUFFERED_BYTES, + type WebSocketBridge, + type WebSocketPeer, +} from '../utils/websocket-bridge' + +const BUFFER_LIMIT_CHARS = 256 * 1024 +const REPLAY_CHUNK_CHARS = 64 * 1024 +const MAX_CONNECT_ATTEMPTS = 3 +const RETRY_DELAY_MS = 100 +const META_FRAME_PREFIX = 0 +const MAX_REGISTERED_HUBS = 64 +const WEBSOCKET_OPEN = 1 +const WEBSOCKET_CLOSING = 2 +const WEBSOCKET_CLOSED = 3 + +type HubState = 'connecting' | 'open' | 'closed' | 'failed' + +const META_DECODER = new TextDecoder() +const META_ENCODER = new TextEncoder() + +export function encodeTerminalMetaFrame(cursor: number): Uint8Array { + const payload = META_ENCODER.encode(JSON.stringify({ cursor })) + const frame = new Uint8Array(new ArrayBuffer(payload.length + 1)) + frame[0] = META_FRAME_PREFIX + frame.set(payload, 1) + return frame +} + +export interface TerminalHubOptions { + openUpstream: () => Promise +} + +export class TerminalHub { + private readonly decoder = new TextDecoder() + private readonly subscribers = new Set() + private readonly pendingInput: Array> = [] + private socket: WebSocket | null = null + private state: HubState = 'connecting' + private attempts = 0 + private retryTimer: ReturnType | undefined + private pendingInputBytes = 0 + private buffer = '' + private bufferStart = 0 + private cursor = 0 + + constructor(private readonly options: TerminalHubOptions) {} + + start(): Promise { + return this.connect() + } + + isAvailable(): boolean { + return this.state !== 'failed' + } + + isEnded(): boolean { + return this.state === 'closed' || this.state === 'failed' + } + + attach(peer: WebSocketPeer, cursor: number | undefined): WebSocketBridge { + this.replay(peer, cursor) + if (this.state === 'closed') { + this.closePeer(peer, 1000) + return { send: () => {}, close: () => {} } + } + this.subscribers.add(peer) + return { + send: (data) => this.write(data), + close: () => { + this.subscribers.delete(peer) + }, + } + } + + dispose(): void { + this.clearRetry() + if (this.state !== 'failed') this.state = 'closed' + const socket = this.socket + this.socket = null + closeSocketQuietly(socket) + const peers = [...this.subscribers] + this.subscribers.clear() + for (const peer of peers) this.closePeer(peer, 1000) + } + + private async connect(): Promise { + if (this.state !== 'connecting') return + this.attempts += 1 + let socket: WebSocket + try { + socket = await this.options.openUpstream() + } catch { + this.retryOrFail() + return + } + if (this.state !== 'connecting') { + closeSocketQuietly(socket) + return + } + this.socket = socket + socket.binaryType = 'arraybuffer' + socket.addEventListener('open', () => this.handleOpen(socket)) + socket.addEventListener('message', (event) => this.handleMessage(event as MessageEvent)) + socket.addEventListener('close', () => this.handleUpstreamClosed(socket)) + socket.addEventListener('error', () => this.handleUpstreamError(socket)) + const readyState = (socket as { readyState?: number }).readyState + if (readyState === WEBSOCKET_OPEN) { + this.handleOpen(socket) + return + } + if (readyState === WEBSOCKET_CLOSING || readyState === WEBSOCKET_CLOSED) { + this.socket = null + this.retryOrFail() + } + } + + private handleOpen(socket: WebSocket): void { + if (this.socket !== socket || this.state !== 'connecting') return + this.state = 'open' + while (this.pendingInput.length > 0) { + const item = this.pendingInput.shift()! + this.pendingInputBytes -= byteLengthOf(item) + try { + socket.send(item) + } catch { + this.endStream() + return + } + } + } + + private handleMessage(event: MessageEvent): void { + const data: unknown = event.data + if (typeof data === 'string') { + this.appendOutput(data) + return + } + let bytes: Uint8Array + if (data instanceof ArrayBuffer) { + bytes = new Uint8Array(data) + } else if (data instanceof Uint8Array) { + bytes = data + } else { + return + } + if (bytes.length > 0 && bytes[0] === META_FRAME_PREFIX) { + this.alignToCursor(bytes.subarray(1)) + return + } + this.appendOutput(this.decoder.decode(bytes, { stream: true })) + } + + private handleUpstreamError(socket: WebSocket): void { + if (this.socket !== socket || this.state !== 'connecting') return + this.socket = null + closeSocketQuietly(socket) + this.retryOrFail() + } + + private handleUpstreamClosed(socket: WebSocket): void { + if (this.socket !== socket) return + this.socket = null + if (this.state === 'open') { + this.endStream() + return + } + if (this.state === 'connecting') this.retryOrFail() + } + + private retryOrFail(): void { + if (this.state !== 'connecting') return + if (this.attempts >= MAX_CONNECT_ATTEMPTS) { + this.state = 'failed' + return + } + this.clearRetry() + this.retryTimer = setTimeout(() => { + this.retryTimer = undefined + void this.connect() + }, RETRY_DELAY_MS) + } + + private clearRetry(): void { + if (this.retryTimer === undefined) return + clearTimeout(this.retryTimer) + this.retryTimer = undefined + } + + private endStream(): void { + if (this.state === 'closed') return + this.state = 'closed' + const peers = [...this.subscribers] + this.subscribers.clear() + for (const peer of peers) this.closePeer(peer, 1000) + } + + private alignToCursor(payload: Uint8Array): void { + try { + const parsed = JSON.parse(META_DECODER.decode(payload)) as { cursor?: unknown } + if (typeof parsed.cursor !== 'number' || !Number.isSafeInteger(parsed.cursor)) return + if (parsed.cursor < this.cursor) return + this.cursor = parsed.cursor + this.bufferStart = Math.max(0, parsed.cursor - this.buffer.length) + } catch { + return + } + } + + private appendOutput(chunk: string): void { + if (chunk.length === 0) return + this.cursor += chunk.length + this.buffer += chunk + const excess = this.buffer.length - BUFFER_LIMIT_CHARS + if (excess > 0) { + this.buffer = this.buffer.slice(excess) + this.bufferStart += excess + } + for (const peer of [...this.subscribers]) this.deliver(peer, chunk) + } + + private replay(peer: WebSocketPeer, cursor: number | undefined): void { + const from = this.replayStart(cursor) + const offset = Math.max(0, from - this.bufferStart) + const replay = offset < this.buffer.length ? this.buffer.slice(offset) : '' + for (let index = 0; index < replay.length; index += REPLAY_CHUNK_CHARS) { + this.deliver(peer, replay.slice(index, index + REPLAY_CHUNK_CHARS)) + } + this.deliver(peer, encodeTerminalMetaFrame(this.cursor)) + } + + private replayStart(cursor: number | undefined): number { + if (cursor === -1) return this.cursor + if (cursor === undefined) return this.bufferStart + return Math.max(cursor, this.bufferStart) + } + + private write(data: string | ArrayBuffer | Uint8Array): void { + if (this.state === 'closed' || this.state === 'failed') return + const payload: string | Uint8Array = + typeof data === 'string' ? data : data instanceof Uint8Array ? data : new Uint8Array(data) + if (this.state === 'open' && this.socket) { + try { + this.socket.send(payload) + } catch { + this.endStream() + } + return + } + this.pendingInput.push(payload) + this.pendingInputBytes += byteLengthOf(payload) + while (this.pendingInputBytes > MAX_UPSTREAM_BUFFERED_BYTES && this.pendingInput.length > 0) { + const dropped = this.pendingInput.shift()! + this.pendingInputBytes -= byteLengthOf(dropped) + } + } + + private deliver(peer: WebSocketPeer, data: string | Uint8Array): void { + try { + peer.send(data) + } catch { + this.subscribers.delete(peer) + return + } + if (peer.bufferedAmount() > MAX_PEER_BUFFERED_BYTES) { + this.subscribers.delete(peer) + this.closePeer(peer, 1013, 'Client too slow') + } + } + + private closePeer(peer: WebSocketPeer, code: number, reason?: string): void { + try { + peer.close(code, reason) + } catch { + return + } + } +} + +function closeSocketQuietly(socket: { close: () => void } | null | undefined): void { + if (!socket) return + try { + socket.close() + } catch { + return + } +} + +function byteLengthOf(data: string | Uint8Array): number { + return typeof data === 'string' ? Buffer.byteLength(data) : data.byteLength +} + +export class TerminalHubRegistry { + private readonly hubs = new Map() + + get(key: string): TerminalHub | undefined { + return this.hubs.get(key) + } + + register(key: string, hub: TerminalHub): void { + if (this.hubs.has(key)) { + hub.dispose() + return + } + if (this.hubs.size >= MAX_REGISTERED_HUBS && !this.evict()) { + hub.dispose() + return + } + this.hubs.set(key, hub) + } + + dispose(key: string): void { + const hub = this.hubs.get(key) + if (!hub) return + this.hubs.delete(key) + hub.dispose() + } + + private evict(): boolean { + const target = [...this.hubs.entries()].find(([, hub]) => hub.isEnded()) + if (!target) return false + this.hubs.delete(target[0]) + target[1].dispose() + return true + } +} diff --git a/backend/src/services/terminal.ts b/backend/src/services/terminal.ts index d96def17e..e2ab93925 100644 --- a/backend/src/services/terminal.ts +++ b/backend/src/services/terminal.ts @@ -5,6 +5,7 @@ import type { OpenCodeClient } from './opencode/client' import type { CredentialProvider } from './credential-provider' import { toWebSocketUrl } from './opencode/upstream' import { bridgeWebSocket, type WebSocketBridge, type WebSocketPeer } from '../utils/websocket-bridge' +import { TerminalHub, TerminalHubRegistry } from './terminal-hub' type OpenCodePty = Awaited>['data'][number] type OpenCodePersistentPty = Awaited>[number] @@ -36,15 +37,28 @@ export class TerminalNotFoundError extends Error { } } +const DEFERRED_START_SHELL = '/bin/sh' +const DEFERRED_START_SCRIPT = 'sleep 0.3; exec "$@"' +const DEFERRED_START_HANDLE = 'ocm-terminal' +const DEFERRED_START_KINDS: ReadonlySet = new Set(['action', 'setup']) + +function deferCommandStart(command: string, args: readonly string[]): { command: string; args: string[] } { + return { + command: DEFERRED_START_SHELL, + args: ['-c', DEFERRED_START_SCRIPT, DEFERRED_START_HANDLE, command, ...args], + } +} + export class TerminalService { constructor( private readonly target: TerminalTarget, private readonly credentialProvider: CredentialProvider, private readonly openSocket: (url: string) => WebSocket = (url) => new WebSocket(url), + private readonly hubs: TerminalHubRegistry = new TerminalHubRegistry(), ) {} forTarget(target: TerminalTarget): TerminalService { - return new TerminalService(target, this.credentialProvider, this.openSocket) + return new TerminalService(target, this.credentialProvider, this.openSocket, this.hubs) } async list(directory: string): Promise { @@ -53,17 +67,39 @@ export class TerminalService { } async create(directory: string, input: CreateTerminalInput): Promise { + const deferred = + input.command !== undefined && DEFERRED_START_KINDS.has(input.kind) && process.platform !== 'win32' + ? deferCommandStart(input.command, input.args ?? []) + : undefined const result = await this.target.client.api.pty.create({ location: { directory }, title: formatTerminalTitle({ kind: input.kind, name: input.name, actionId: input.actionId }), - ...(input.command !== undefined ? { command: input.command } : {}), - ...(input.args !== undefined ? { args: input.args } : {}), + ...(deferred + ? { command: deferred.command, args: deferred.args } + : { + ...(input.command !== undefined ? { command: input.command } : {}), + ...(input.args !== undefined ? { args: input.args } : {}), + }), env: { ...(this.target.includeLocalCredentials ? this.credentialProvider.getGhCliEnv({ cwd: directory }) : {}), ...input.env, }, }) - return this.toTerminalInfo(result.data) + const terminal = this.toTerminalInfo(result.data) + this.recordTerminal(directory, terminal.id) + return terminal + } + + private recordTerminal(directory: string, ptyID: string): void { + const key = this.hubKey(directory, ptyID) + if (this.hubs.get(key)) return + const hub = new TerminalHub({ openUpstream: () => this.openUpstreamSocket(directory, ptyID, 0) }) + this.hubs.register(key, hub) + void hub.start() + } + + private hubKey(directory: string, ptyID: string): string { + return `${this.target.baseUrl()}\u0000${directory}\u0000${ptyID}` } async requireTerminal(directory: string, ptyID: string): Promise { @@ -80,7 +116,11 @@ export class TerminalService { } async remove(directory: string, ptyID: string): Promise { - await this.target.client.api.pty.remove({ ptyID, location: { directory } }) + try { + await this.target.client.api.pty.remove({ ptyID, location: { directory } }) + } finally { + this.hubs.dispose(this.hubKey(directory, ptyID)) + } } async removeAll(directory: string): Promise { @@ -104,6 +144,19 @@ export class TerminalService { ): Promise { await this.requireTerminal(directory, ptyID) + const key = this.hubKey(directory, ptyID) + const hub = this.hubs.get(key) + if (hub?.isAvailable()) return hub.attach(peer, cursor) + if (hub) this.hubs.dispose(key) + + return bridgeWebSocket(await this.openUpstreamSocket(directory, ptyID, cursor), peer) + } + + private async openUpstreamSocket( + directory: string, + ptyID: string, + cursor: number | undefined, + ): Promise { const result = await this.target.client.api.pty.connect.token({ ptyID, location: { directory }, @@ -117,7 +170,7 @@ export class TerminalService { socketUrl.searchParams.set('location[directory]', directory) if (cursor !== undefined) socketUrl.searchParams.set('cursor', String(cursor)) - return bridgeWebSocket(this.openSocket(toWebSocketUrl(socketUrl.toString())), peer) + return this.openSocket(toWebSocketUrl(socketUrl.toString())) } private async removePersistentTerminals(directory: string): Promise { diff --git a/backend/src/utils/process.ts b/backend/src/utils/process.ts index d6f84173a..7c8f8499b 100644 --- a/backend/src/utils/process.ts +++ b/backend/src/utils/process.ts @@ -32,14 +32,10 @@ export async function executeCommand ({ + ENV: { + SERVER: { PORT: 5003, HOST: '0.0.0.0', NODE_ENV: 'test' }, + AUTH: { + SECRET: 'test-secret-for-encryption-key-32c', + TRUSTED_ORIGINS: 'http://localhost:5173,http://localhost:5003', + SECURE_COOKIES: false, + TRUSTED_PROXY_HEADERS: '', + ADMIN_EMAIL: undefined as string | undefined, + ADMIN_PASSWORD: undefined as string | undefined, + ADMIN_PASSWORD_RESET: false, + GITHUB_CLIENT_ID: undefined as string | undefined, + GITHUB_CLIENT_SECRET: undefined as string | undefined, + GOOGLE_CLIENT_ID: undefined as string | undefined, + GOOGLE_CLIENT_SECRET: undefined as string | undefined, + DISCORD_CLIENT_ID: undefined as string | undefined, + DISCORD_CLIENT_SECRET: undefined as string | undefined, + PASSKEY_RP_ID: 'localhost', + PASSKEY_RP_NAME: 'OpenCode Manager', + PASSKEY_ORIGIN: 'http://localhost:5003', + }, + LOGGING: { DEBUG: false, LOG_LEVEL: 'info' }, + }, +})) + +vi.mock('@opencode-manager/shared/config/env', () => ({ + ENV, + getTrustedOrigins: () => ENV.AUTH.TRUSTED_ORIGINS + .split(',') + .map((origin: string) => origin.trim()) + .filter((origin: string) => origin.length > 0), +})) + +vi.mock('../../src/utils/logger', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})) + +function socketEnv(address: string | undefined) { + return { + incoming: { + socket: { + remoteAddress: address, + remotePort: 54321, + remoteFamily: address?.includes(':') ? 'IPv6' : 'IPv4', + }, + }, + } +} + +function createAuthDatabase(): Database { + const db = new DatabaseSync(':memory:') + const compatible = Object.assign(db, { + run: (sql: string, ...params: (string | number | null)[]) => db.prepare(sql).run(...params), + }) as unknown as Database + migrate(compatible, allMigrations) + return compatible +} + +function signUpRequest(headers: Record): Request { + return new Request('http://localhost:5003/api/auth/sign-up/email', { + method: 'POST', + headers: { 'content-type': 'application/json', origin: 'http://localhost:5003', ...headers }, + body: JSON.stringify({ email: 'spoof@example.com', password: 'test-password', name: 'Spoof' }), + }) +} + +function readSessionIpAddress(db: Database): string { + const row = db.prepare('SELECT ipAddress FROM "session" LIMIT 1').get() as { ipAddress: string } + return row.ipAddress +} + +describe('auth client ip headers', () => { + beforeEach(() => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = '' + }) + + it('trusts no proxy headers by default and resolves the socket address', () => { + expect(getTrustedProxyHeaders()).toEqual([]) + expect(getClientIpHeaders()).toEqual([AUTH_SOCKET_IP_HEADER]) + }) + + it('reads configured proxy headers in order before the socket address', () => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = ' CF-Connecting-IP , X-Forwarded-For ' + + expect(getTrustedProxyHeaders()).toEqual(['cf-connecting-ip', 'x-forwarded-for']) + expect(getClientIpHeaders()).toEqual(['cf-connecting-ip', 'x-forwarded-for', AUTH_SOCKET_IP_HEADER]) + }) + + it('ignores empty, malformed and duplicate header names', () => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = 'x-real-ip,,X-Real-IP,not a header' + + expect(getTrustedProxyHeaders()).toEqual(['x-real-ip', 'x-real-ip']) + }) + + it('drops a client-supplied forwarded header and uses the socket address by default', () => { + const headers = new Headers({ 'x-forwarded-for': '198.51.100.9', [AUTH_SOCKET_IP_HEADER]: '6.6.6.6' }) + + const result = applyClientIpHeaders(headers, '203.0.113.7') + + expect(result.get('x-forwarded-for')).toBeNull() + expect(result.get(AUTH_SOCKET_IP_HEADER)).toBe('203.0.113.7') + }) + + it('keeps the configured proxy header and still forwards the socket address', () => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = 'x-forwarded-for' + const headers = new Headers({ 'x-forwarded-for': '198.51.100.9' }) + + const result = applyClientIpHeaders(headers, '203.0.113.7') + + expect(result.get('x-forwarded-for')).toBe('198.51.100.9') + expect(result.get(AUTH_SOCKET_IP_HEADER)).toBe('203.0.113.7') + }) + + it('clears the socket header when the address cannot be resolved', () => { + const headers = new Headers({ [AUTH_SOCKET_IP_HEADER]: '6.6.6.6' }) + + const result = applyClientIpHeaders(headers, undefined) + + expect(result.get(AUTH_SOCKET_IP_HEADER)).toBeNull() + }) +}) + +describe('createAuthRoutes client ip forwarding', () => { + beforeEach(() => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = '' + }) + + it('passes the socket address to the auth handler and strips a spoofed forwarded header', async () => { + const received: Request[] = [] + const handler = vi.fn(async (request: Request) => { + received.push(request) + return new Response('ok') + }) + const app: Hono = createAuthRoutes({ handler } as unknown as ReturnType) + + await app.fetch( + new Request('http://localhost:5003/api/auth/get-session', { + headers: { 'x-forwarded-for': '198.51.100.9' }, + }), + socketEnv('203.0.113.7') + ) + + expect(received[0]?.headers.get(AUTH_SOCKET_IP_HEADER)).toBe('203.0.113.7') + expect(received[0]?.headers.get('x-forwarded-for')).toBeNull() + }) + + it('forwards the configured proxy header when trusted', async () => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = 'x-forwarded-for' + const received: Request[] = [] + const handler = vi.fn(async (request: Request) => { + received.push(request) + return new Response('ok') + }) + const app: Hono = createAuthRoutes({ handler } as unknown as ReturnType) + + await app.fetch( + new Request('http://localhost:5003/api/auth/get-session', { + headers: { 'x-forwarded-for': '198.51.100.9' }, + }), + socketEnv('203.0.113.7') + ) + + expect(received[0]?.headers.get('x-forwarded-for')).toBe('198.51.100.9') + expect(received[0]?.headers.get(AUTH_SOCKET_IP_HEADER)).toBe('203.0.113.7') + }) + + it('omits the socket header when the connection info is unavailable', async () => { + const received: Request[] = [] + const handler = vi.fn(async (request: Request) => { + received.push(request) + return new Response('ok') + }) + const app: Hono = createAuthRoutes({ handler } as unknown as ReturnType) + + await app.fetch(new Request('http://localhost:5003/api/auth/get-session', { + headers: { [AUTH_SOCKET_IP_HEADER]: '6.6.6.6' }, + })) + + expect(received[0]?.headers.get(AUTH_SOCKET_IP_HEADER)).toBeNull() + }) +}) + +describe('createAuth client ip resolution', () => { + beforeEach(() => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = '' + }) + + it('records the socket address on a new session and ignores a spoofed forwarded header', async () => { + const db = createAuthDatabase() + const app: Hono = createAuthRoutes(createAuth(db)) + + try { + const response = await app.fetch( + signUpRequest({ 'x-forwarded-for': '198.51.100.9' }), + socketEnv('203.0.113.7') + ) + + expect(response.status).toBe(200) + expect(readSessionIpAddress(db)).toBe('203.0.113.7') + } finally { + db.close() + } + }, 30000) + + it('records the trusted forwarded address when proxy headers are configured', async () => { + ENV.AUTH.TRUSTED_PROXY_HEADERS = 'x-forwarded-for' + const db = createAuthDatabase() + const app: Hono = createAuthRoutes(createAuth(db)) + + try { + const response = await app.fetch( + signUpRequest({ 'x-forwarded-for': '198.51.100.9' }), + socketEnv('203.0.113.7') + ) + + expect(response.status).toBe(200) + expect(readSessionIpAddress(db)).toBe('198.51.100.9') + } finally { + db.close() + } + }, 30000) +}) diff --git a/backend/test/auth/index.test.ts b/backend/test/auth/index.test.ts index 70af8bf33..9826f9122 100644 --- a/backend/test/auth/index.test.ts +++ b/backend/test/auth/index.test.ts @@ -13,6 +13,7 @@ const { ENV } = vi.hoisted(() => ({ SECRET: 'test-secret-for-encryption-key-32c', TRUSTED_ORIGINS: 'http://localhost:5173,http://localhost:5003', SECURE_COOKIES: false, + TRUSTED_PROXY_HEADERS: '', ADMIN_EMAIL: undefined as string | undefined, ADMIN_PASSWORD: undefined as string | undefined, ADMIN_PASSWORD_RESET: false, @@ -46,6 +47,7 @@ function resetEnv(): void { ENV.SERVER.PORT = 5003 ENV.AUTH.TRUSTED_ORIGINS = 'http://localhost:5173,http://localhost:5003' ENV.AUTH.SECURE_COOKIES = false + ENV.AUTH.TRUSTED_PROXY_HEADERS = '' ENV.AUTH.ADMIN_EMAIL = undefined ENV.AUTH.ADMIN_PASSWORD = undefined ENV.AUTH.GITHUB_CLIENT_ID = undefined diff --git a/backend/test/routes/auth.test.ts b/backend/test/routes/auth.test.ts index 00ddc9c3a..4af03e64c 100644 --- a/backend/test/routes/auth.test.ts +++ b/backend/test/routes/auth.test.ts @@ -10,6 +10,7 @@ const { ENV } = vi.hoisted(() => ({ SECRET: 'test-secret-for-encryption-key-32c', TRUSTED_ORIGINS: 'http://localhost:5173,http://localhost:5003', SECURE_COOKIES: false, + TRUSTED_PROXY_HEADERS: '', ADMIN_EMAIL: undefined as string | undefined, ADMIN_PASSWORD: undefined as string | undefined, ADMIN_PASSWORD_RESET: false, @@ -49,6 +50,7 @@ const mockLoggerDebug = vi.mocked(logger.debug) function resetEnv(): void { ENV.AUTH.TRUSTED_ORIGINS = 'http://localhost:5173,http://localhost:5003' ENV.AUTH.SECURE_COOKIES = false + ENV.AUTH.TRUSTED_PROXY_HEADERS = '' ENV.AUTH.ADMIN_EMAIL = undefined ENV.AUTH.ADMIN_PASSWORD = undefined ENV.AUTH.ADMIN_PASSWORD_RESET = false diff --git a/backend/test/routes/change-walkthroughs.test.ts b/backend/test/routes/change-walkthroughs.test.ts index 10adb63bc..8cf73cefd 100644 --- a/backend/test/routes/change-walkthroughs.test.ts +++ b/backend/test/routes/change-walkthroughs.test.ts @@ -11,6 +11,7 @@ function state(overrides: Partial = {}): ChangeWalkthrou return { walkthrough: null, currentDiffHash: 'hash', + hasChanges: true, stale: false, generating: false, running: false, @@ -24,6 +25,7 @@ describe('change walkthrough routes', () => { let startGeneration: ReturnType let cancelGeneration: ReturnType let refine: ReturnType + let listHelperSessionIds: ReturnType let app: Hono beforeEach(() => { @@ -31,13 +33,22 @@ describe('change walkthrough routes', () => { startGeneration = vi.fn(async () => state({ generating: true })) cancelGeneration = vi.fn(async () => true) refine = vi.fn(async () => state({ generating: true })) + listHelperSessionIds = vi.fn(() => ['ses_helper']) app = new Hono() app.route( '/change-walkthroughs', - createChangeWalkthroughRoutes({ getState, startGeneration, cancelGeneration, refine } as unknown as ChangeWalkthroughService), + createChangeWalkthroughRoutes({ getState, startGeneration, cancelGeneration, refine, listHelperSessionIds } as unknown as ChangeWalkthroughService), ) }) + it('GET helper-sessions lists walkthrough helper sessions', async () => { + const res = await app.request('/change-walkthroughs/helper-sessions') + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ sessionIds: ['ses_helper'] }) + expect(getState).not.toHaveBeenCalled() + }) + it('GET returns the state for the default source', async () => { const res = await app.request(`/change-walkthroughs/${SESSION_ID}`) diff --git a/backend/test/routes/repos.test.ts b/backend/test/routes/repos.test.ts index f2a56ce4e..5d853999a 100644 --- a/backend/test/routes/repos.test.ts +++ b/backend/test/routes/repos.test.ts @@ -494,6 +494,24 @@ describe('Repo Routes', () => { const body = await res.json() as { error: string } expect(body.error).toBe('init failed') }) + + it('should return the client status for user-input repo failures', async () => { + vi.mocked(repoService.cloneRepo).mockRejectedValue(Object.assign( + new Error('Repository not found or access denied: https://github.com/test/missing.'), + { statusCode: 400 }, + )) + + const app = createTestRoutes() + const res = await app.request('/', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ repoUrl: 'https://github.com/test/missing' }), + }) + + expect(res.status).toBe(400) + const body = await res.json() as { error: string } + expect(body.error).toBe('Repository not found or access denied: https://github.com/test/missing.') + }) }) describe('POST /:id/workspaces', () => { @@ -1108,6 +1126,25 @@ describe('Repo Routes', () => { const body = await res.json() as { error: string } expect(body.error).toBe('create failed') }) + + it('should return 400 when the branch name is invalid', async () => { + vi.mocked(db.getRepoById).mockReturnValue(createMockRepo({ id: 1 })) + vi.mocked(repoService.createBranch).mockRejectedValue(Object.assign( + new Error("Invalid branch name: 'bad name..x'."), + { statusCode: 400 }, + )) + + const app = createTestRoutes() + const res = await app.request('/1/branch/create', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ branch: 'bad name..x' }), + }) + + expect(res.status).toBe(400) + const body = await res.json() as { error: string } + expect(body.error).toBe("Invalid branch name: 'bad name..x'.") + }) }) describe('GET /:id/download', () => { diff --git a/backend/test/routes/session-sandbox.test.ts b/backend/test/routes/session-sandbox.test.ts index 4e1d5f8fc..57c6e5e91 100644 --- a/backend/test/routes/session-sandbox.test.ts +++ b/backend/test/routes/session-sandbox.test.ts @@ -22,13 +22,20 @@ function createTestDb(): Database { function createTestApp( db: Database, - options: { parents?: Record; sandboxEnabled?: boolean } = {}, + options: { parents?: Record; sandboxEnabled?: boolean; sessionGetError?: Error } = {}, ): Hono { const settingsService = new SettingsService(db) if (options.sandboxEnabled) { settingsService.updateSettings({ sandbox: { enabled: true } }) } - const client = createFakeSessionPermissionClient({ parents: options.parents ?? {} }) + const client = createFakeSessionPermissionClient({ + parents: options.parents ?? {}, + getSession: options.sessionGetError + ? async () => { + throw options.sessionGetError + } + : undefined, + }) const service = new SessionSandboxService(db, new SessionAncestry(client), settingsService, { removeSessionSandbox: async () => undefined, }) @@ -96,6 +103,34 @@ describe('session sandbox routes', () => { }) }) + it('GET returns 404 when the session does not exist on OpenCode', async () => { + const notFound = Object.assign(new Error('Session not found: ses_missing'), { _tag: 'SessionNotFoundError' }) + const app = createTestApp(db, { sessionGetError: notFound }) + + const res = await app.request('/session-sandbox/ses_missing') + + expect(res.status).toBe(404) + await expect(res.json()).resolves.toMatchObject({ error: 'Session not found: ses_missing' }) + }) + + it('GET returns 400 when the session id is malformed', async () => { + const invalid = Object.assign(new Error('Expected a string starting with "ses"'), { _tag: 'InvalidRequestError' }) + const app = createTestApp(db, { sessionGetError: invalid }) + + const res = await app.request('/session-sandbox/nope') + + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ error: 'Expected a string starting with "ses"' }) + }) + + it('GET keeps returning 500 for unexpected upstream failures', async () => { + const app = createTestApp(db, { sessionGetError: new Error('upstream unavailable') }) + + const res = await app.request('/session-sandbox/ses_root') + + expect(res.status).toBe(500) + }) + it('PUT stores a value and GET reads it back', async () => { const app = createTestApp(db, { sandboxEnabled: true }) diff --git a/backend/test/routes/settings.test.ts b/backend/test/routes/settings.test.ts index bc9d2f3ee..45c411480 100644 --- a/backend/test/routes/settings.test.ts +++ b/backend/test/routes/settings.test.ts @@ -1554,6 +1554,59 @@ describe('Settings Routes - OpenCode Upgrade', () => { }) }) + describe('PATCH / - git identity validation', () => { + it.each(['not-an-email', 'missing@tld', 'two words@example.com'])('rejects the email %s with 400', async (email) => { + const res = await settingsApp.fetch(new Request('http://localhost/', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ preferences: { gitIdentity: { name: 'Agent', email } } }), + })) + const json = await res.json() as { error: string } + + expect(res.status).toBe(400) + expect(json.error).toBe('Invalid settings data') + expect(mockUpdateSettings).not.toHaveBeenCalled() + }) + + it('accepts an empty email and a valid email', async () => { + mockGetSettings.mockReturnValue({ + preferences: { gitIdentity: { name: 'Agent', email: '' } }, + updatedAt: 1, + }) + mockUpdateSettings.mockReturnValue({ + preferences: { gitIdentity: { name: 'Agent', email: 'agent@example.com' } }, + updatedAt: 2, + }) + + const emptyRes = await settingsApp.fetch(new Request('http://localhost/', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ preferences: { gitIdentity: { name: 'Agent', email: '' } } }), + })) + expect(emptyRes.status).toBe(200) + + const validRes = await settingsApp.fetch(new Request('http://localhost/', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ preferences: { gitIdentity: { name: 'Agent', email: 'agent@example.com' } } }), + })) + expect(validRes.status).toBe(200) + }) + + it('rejects an invalid email in a saved identity profile', async () => { + const res = await settingsApp.fetch(new Request('http://localhost/', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + preferences: { gitIdentities: [{ id: 'a', name: 'Alpha', email: 'not-an-email' }] }, + }), + })) + + expect(res.status).toBe(400) + expect(mockUpdateSettings).not.toHaveBeenCalled() + }) + }) + describe('PATCH / - sandbox enable guard', () => { afterEach(() => { forceProcessAttestation(null) diff --git a/backend/test/scripts/docker-config.test.ts b/backend/test/scripts/docker-config.test.ts index 20cd381b0..01c6aee4d 100644 --- a/backend/test/scripts/docker-config.test.ts +++ b/backend/test/scripts/docker-config.test.ts @@ -78,6 +78,13 @@ describe('entrypoint library wiring', () => { const dockerfile = read(dockerfilePath) expect(dockerfile).not.toMatch(/chmod \+x \/usr\/local\/lib\/ocm\/container-user\.sh/) }) + + it('runs the entrypoint under tini so orphaned processes are reaped', () => { + const dockerfile = read(dockerfilePath) + expect(dockerfile).toMatch(/^\s+tini \\$/m) + expect(dockerfile).toContain('ENTRYPOINT ["/usr/bin/tini", "--", "/docker-entrypoint.sh"]') + expect(read(composePath)).not.toContain('init: true') + }) }) describe('microsandbox runtime install', () => { @@ -237,6 +244,13 @@ describe('workspace ownership configuration', () => { expect(envExample).toContain('# PUID=1000') expect(envExample).toContain('# PGID=1000') }) + + it('forwards the auth trusted proxy header opt-in and documents it', () => { + expect(read(composePath)).toContain('- AUTH_TRUSTED_PROXY_HEADERS=${AUTH_TRUSTED_PROXY_HEADERS:-}') + expect(read(join(repoRoot, 'docker-compose.release.yml'))) + .toContain('- AUTH_TRUSTED_PROXY_HEADERS=${AUTH_TRUSTED_PROXY_HEADERS:-}') + expect(read(envExamplePath)).toContain('# AUTH_TRUSTED_PROXY_HEADERS=x-forwarded-for') + }) }) describe('compose image source', () => { diff --git a/backend/test/services/change-walkthroughs.test.ts b/backend/test/services/change-walkthroughs.test.ts index 0acb59131..858f50aaf 100644 --- a/backend/test/services/change-walkthroughs.test.ts +++ b/backend/test/services/change-walkthroughs.test.ts @@ -265,10 +265,26 @@ describe('ChangeWalkthroughService', () => { expect(state.walkthrough).toBeNull() expect(state.stale).toBe(false) expect(state.currentDiffHash).toBe(computeChangesHash(THREE_HUNKS)) + expect(state.hasChanges).toBe(true) expect(state.generating).toBe(false) expect(state.error).toBeNull() }) + it('reports no changes for a session with an empty diff', async () => { + sessions[SESSION_ID]!.changes = [] + + const state = await service.getState(SESSION_ID) + + expect(state.currentDiffHash).toBe(computeChangesHash([])) + expect(state.hasChanges).toBe(false) + }) + + it('lists the helper sessions of existing walkthroughs', async () => { + await service.startGeneration(SESSION_ID, {}) + + expect(service.listHelperSessionIds()).toEqual(['ses_walk_1']) + }) + it('creates exactly one walkthrough session and returns generating', async () => { const state = await service.startGeneration(SESSION_ID, {}) @@ -479,6 +495,7 @@ describe('ChangeWalkthroughService', () => { const state = await service.getState(SESSION_ID) expect(state.currentDiffHash).toBeNull() + expect(state.hasChanges).toBe(true) expect(state.stale).toBe(false) }) diff --git a/backend/test/services/files.test.ts b/backend/test/services/files.test.ts index 5797672f4..92d0938bf 100644 --- a/backend/test/services/files.test.ts +++ b/backend/test/services/files.test.ts @@ -93,6 +93,15 @@ describe('files service', () => { expect(await getRawFileMetadata(`${relativeRoot}/meta-dir`)).toBeNull() }) + it('sniffs an unknown extension and reports binary content', async () => { + const bytes = Buffer.from(Array.from({ length: 512 }, (_, index) => (index * 37 + 11) % 256)) + await writeFile(resolveTempPath(`${relativeRoot}/payload.unknownext`), bytes) + + const metadata = await getRawFileMetadata(`${relativeRoot}/payload.unknownext`) + + expect(metadata?.mimeType).toBe('application/octet-stream') + }) + it('rejects with 404 when the file is missing', async () => { await expect(getRawFileMetadata(`${relativeRoot}/missing.htm`)).rejects.toEqual({ message: 'File or directory not found', @@ -134,6 +143,32 @@ describe('files service', () => { expect(Buffer.from(result.content ?? '', 'base64').toString('utf8')).toBe('mystery') }) + it('reports an unknown extension with text content as text', async () => { + await writeLines(`${relativeRoot}/plain.foo`, 'still text') + + const result = await getFile(`${relativeRoot}/plain.foo`) + + expect(result.mimeType).toBe('text/plain') + }) + + it('reports an unknown extension with binary content as binary', async () => { + const bytes = Buffer.from(Array.from({ length: 2930 }, (_, index) => (index * 37 + 11) % 256)) + await writeFile(resolveTempPath(`${relativeRoot}/blob.bin`), bytes) + + const result = await getFile(`${relativeRoot}/blob.bin`) + + expect(result.mimeType).toBe('application/octet-stream') + expect(result.content).toBe(bytes.toString('base64')) + }) + + it('reports an empty unknown extension as text', async () => { + await writeLines(`${relativeRoot}/empty.foo`, '') + + const result = await getFile(`${relativeRoot}/empty.foo`) + + expect(result.mimeType).toBe('text/plain') + }) + it('maps .htm files to text/html', async () => { await writeLines(`${relativeRoot}/page.htm`, '

hi

') @@ -326,6 +361,15 @@ describe('files service', () => { expect(result.hasMore).toBe(false) }) + it('reports binary content for an unknown extension', async () => { + const bytes = Buffer.from(Array.from({ length: 512 }, (_, index) => (index * 37 + 11) % 256)) + await writeFile(resolveTempPath(`${relativeRoot}/blob.unknownext`), bytes) + + const result = await getFileRange(`${relativeRoot}/blob.unknownext`, 0, 5) + + expect(result.mimeType).toBe('application/octet-stream') + }) + it('rejects with 404 when the file is missing', async () => { await expect(getFileRange(`${relativeRoot}/missing.txt`, 0, 5)).rejects.toEqual({ message: 'File does not exist', diff --git a/backend/test/services/git/GitService.real.test.ts b/backend/test/services/git/GitService.real.test.ts index bc63233f1..0166fab31 100644 --- a/backend/test/services/git/GitService.real.test.ts +++ b/backend/test/services/git/GitService.real.test.ts @@ -1,7 +1,7 @@ import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import path from 'node:path' -import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { Hono } from 'hono' import { Database } from 'bun:sqlite' import { migrate } from '../../../src/db/migration-runner' @@ -15,6 +15,7 @@ import { createRepoRoutes } from '../../../src/routes/repos' import { createStubOpenCodeClient } from '../../helpers/stub-opencode-client' import { cloneOrigin, createCommittedRepo, createGitAuthService, createOrigin, git, uniqueName } from '../../helpers/git-fixtures' import { GitOperationError, parseGitError } from '../../../src/utils/git-errors' +import { logger } from '../../../src/utils/logger' import type { GitAuthService } from '../../../src/services/git-auth' import type { Repo } from '../../../src/types/repo' import type { TerminalService } from '../../../src/services/terminal' @@ -103,6 +104,121 @@ describe('GitService real git', () => { }) }) + describe('getLog unpushed commits', () => { + it('flags only the commits that are not on a remote', async () => { + const origin = path.join(workspaceRoot, uniqueName('unpushed-origin.git')) + const work = path.join(workspaceRoot, uniqueName('unpushed-work')) + createOrigin(origin, work) + const repoPath = path.join(reposPath, uniqueName('unpushed-clone')) + cloneOrigin(origin, repoPath) + const repo = registerClone(origin, repoPath) + + writeFileSync(path.join(repoPath, 'pushed.txt'), 'pushed\n') + git(['add', 'pushed.txt'], repoPath) + git(['commit', '-m', 'pushed commit'], repoPath) + git(['push', 'origin', 'main'], repoPath) + writeFileSync(path.join(repoPath, 'local.txt'), 'local\n') + git(['add', 'local.txt'], repoPath) + git(['commit', '-m', 'local commit'], repoPath) + + const commits = await service.getLog(repo.id, db, 10) + + expect(commits.map((commit) => [commit.message, commit.unpushed])).toEqual([ + ['local commit', true], + ['pushed commit', false], + ['init', false], + ]) + }) + + it('flags the listed branch rather than HEAD', async () => { + const origin = path.join(workspaceRoot, uniqueName('unpushed-branch-origin.git')) + const work = path.join(workspaceRoot, uniqueName('unpushed-branch-work')) + createOrigin(origin, work) + const repoPath = path.join(reposPath, uniqueName('unpushed-branch-clone')) + cloneOrigin(origin, repoPath) + const repo = registerClone(origin, repoPath) + + git(['checkout', '-b', 'feature'], repoPath) + writeFileSync(path.join(repoPath, 'feature.txt'), 'feature\n') + git(['add', 'feature.txt'], repoPath) + git(['commit', '-m', 'feature commit'], repoPath) + git(['checkout', 'main'], repoPath) + + const commits = await service.getLog(repo.id, db, 10, 'feature') + + expect(commits.map((commit) => [commit.message, commit.unpushed])).toEqual([ + ['feature commit', true], + ['init', false], + ]) + }) + + it('flags every commit as unpushed when the repository has no remote', async () => { + const repoPath = path.join(reposPath, uniqueName('unpushed-local')) + createCommittedRepo(repoPath) + git(['commit', '--allow-empty', '-m', 'local only'], repoPath) + const repo = createRepo(db, { + isLocal: true, + localPath: path.basename(repoPath), + sourcePath: repoPath, + branch: 'main', + defaultBranch: 'main', + cloneStatus: 'ready', + clonedAt: Date.now(), + }) + + const commits = await service.getLog(repo.id, db, 10) + + expect(commits.map((commit) => commit.message)).toEqual(['local only', 'init']) + expect(commits.every((commit) => commit.unpushed)).toBe(true) + }) + }) + + describe('getBranchStatus', () => { + it('returns zeros without logging when the branch has no upstream', async () => { + const repoPath = path.join(reposPath, uniqueName('status-no-upstream')) + createCommittedRepo(repoPath) + const repo = createRepo(db, { + isLocal: true, + localPath: path.basename(repoPath), + sourcePath: repoPath, + branch: 'main', + defaultBranch: 'main', + cloneStatus: 'ready', + clonedAt: Date.now(), + }) + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + try { + await expect(service.getBranchStatus(repo.id, db)).resolves.toEqual({ ahead: 0, behind: 0 }) + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('still logs a real failure', async () => { + const notARepoPath = path.join(reposPath, uniqueName('status-not-a-repo')) + mkdirSync(notARepoPath, { recursive: true }) + const repo = createRepo(db, { + isLocal: true, + localPath: path.basename(notARepoPath), + sourcePath: notARepoPath, + branch: 'main', + defaultBranch: 'main', + cloneStatus: 'ready', + clonedAt: Date.now(), + }) + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + try { + await expect(service.getBranchStatus(repo.id, db)).resolves.toEqual({ ahead: 0, behind: 0 }) + expect(warnSpy).toHaveBeenCalledTimes(1) + } finally { + warnSpy.mockRestore() + } + }) + }) + describe('renameBranch', () => { it('renames the checked-out branch and updates the repo row', async () => { const origin = path.join(workspaceRoot, uniqueName('rename-origin.git')) diff --git a/backend/test/services/preview/ports.test.ts b/backend/test/services/preview/ports.test.ts index dd19abfb4..812708f9a 100644 --- a/backend/test/services/preview/ports.test.ts +++ b/backend/test/services/preview/ports.test.ts @@ -21,6 +21,7 @@ const IPV4_FIXTURE = [ procRow('0100007F:1F90', '00000000:0000', '0A', '22222'), procRow('0501A8C0:1F90', '00000000:0000', '0A', '33333'), procRow('0100007F:1435', '0100007F:9C40', '01', '44444'), + procRow('0B00007F:8F71', '00000000:0000', '0A', '88888'), ].join('\n') const IPV6_FIXTURE = [ @@ -70,6 +71,12 @@ describe('parseProcNetTcp', () => { ]) }) + it('drops loopback aliases such as Docker embedded DNS', () => { + const sockets = parseProcNetTcp(IPV4_FIXTURE, 4) + + expect(sockets.map((socket) => socket.address)).not.toContain('127.0.0.11') + }) + it('decodes little-endian IPv6 addresses and keeps loopback and wildcard', () => { const sockets = parseProcNetTcp(IPV6_FIXTURE, 6) @@ -252,6 +259,19 @@ describe('listListeningPorts', () => { expect(cwdCalls).toEqual([[777, 888]]) }) + it('drops loopback aliases from lsof listeners', async () => { + const deps = createDeps( + { files: {}, links: {}, dirs: {} }, + { + platform: 'darwin', + runLsof: async () => ['p999', 'cdockerd', 'n127.0.0.11:36721'].join('\n'), + runLsofCwd: async () => '', + }, + ) + + expect(await listListeningPorts(deps)).toEqual([]) + }) + it('keeps cwd null when the cwd lsof call rejects but still lists the ports', async () => { const deps = createDeps( { files: {}, links: {}, dirs: {} }, diff --git a/backend/test/services/repo-git.test.ts b/backend/test/services/repo-git.test.ts index 1d77238e1..5948bad60 100644 --- a/backend/test/services/repo-git.test.ts +++ b/backend/test/services/repo-git.test.ts @@ -196,9 +196,13 @@ describe('repo service real git', () => { }) it('rejects a missing absolute path', async () => { - const { initLocalRepo } = await import('../../src/services/repo') + const { initLocalRepo, RepoInputError } = await import('../../src/services/repo') + + const error = await initLocalRepo(db, gitAuth, path.join(projectsRoot, 'missing-absolute')).catch((thrown: unknown) => thrown) - await expect(initLocalRepo(db, gitAuth, path.join(projectsRoot, 'missing-absolute'))).rejects.toThrow(/No such file or directory/) + expect(error).toBeInstanceOf(RepoInputError) + expect((error as { statusCode: number }).statusCode).toBe(400) + expect((error as Error).message).toMatch(/No such directory/) }) it('rejects the repos root itself as a source', async () => { @@ -397,6 +401,19 @@ describe('repo service real git', () => { await expect(createBranch(db, gitAuth, 9999, 'main')).rejects.toThrow(/Repo not found/) }) + + it('rejects an invalid branch name as a repo input error', async () => { + const { createBranch, RepoInputError } = await import('../../src/services/repo') + const repoPath = path.join(reposPath, uniqueName('create-bad-branch')) + createCommittedRepo(repoPath) + const repo = registerLocalRepo(path.basename(repoPath)) + + const error = await createBranch(db, gitAuth, repo.id, 'bad name..x').catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(RepoInputError) + expect((error as Error).message).toBe("Invalid branch name: 'bad name..x'.") + expect(git(['rev-parse', '--abbrev-ref', 'HEAD'], repoPath)).toBe('main') + }) }) describe('pullRepo', () => { @@ -458,14 +475,66 @@ describe('repo service real git', () => { }) describe('deleteRepoFiles', () => { - it('deletes a local repo directory and its row', async () => { + it('keeps the files of a local repo inside the repos directory and deletes the row', async () => { const { deleteRepoFiles } = await import('../../src/services/repo') - const repoPath = path.join(reposPath, uniqueName('delete-local')) + const repoPath = path.join(reposPath, uniqueName('delete-local-inside')) createCommittedRepo(repoPath) + writeFileSync(path.join(repoPath, 'important.txt'), 'keep me') const repo = registerLocalRepo(path.basename(repoPath)) await deleteRepoFiles(db, repo.id) + expect(existsSync(path.join(repoPath, 'important.txt'))).toBe(true) + expect(existsSync(path.join(repoPath, '.git'))).toBe(true) + expect(getRepoById(db, repo.id)).toBeNull() + }) + + it('removes only the workspace alias of a local repo outside the repos directory', async () => { + const { initLocalRepo, deleteRepoFiles } = await import('../../src/services/repo') + const sourcePath = path.join(projectsRoot, uniqueName('delete-local-outside')) + createCommittedRepo(sourcePath) + writeFileSync(path.join(sourcePath, 'important.txt'), 'keep me') + const repo = await initLocalRepo(db, gitAuth, sourcePath) + const aliasPath = path.join(reposPath, repo.localPath) + + expect(lstatSync(aliasPath).isSymbolicLink()).toBe(true) + + await deleteRepoFiles(db, repo.id) + + expect(existsSync(aliasPath)).toBe(false) + expect(existsSync(path.join(sourcePath, 'important.txt'))).toBe(true) + expect(getRepoById(db, repo.id)).toBeNull() + }) + + it('keeps local repo files when the alias is missing', async () => { + const { initLocalRepo, deleteRepoFiles } = await import('../../src/services/repo') + const sourcePath = path.join(projectsRoot, uniqueName('delete-local-alias-gone')) + createCommittedRepo(sourcePath) + const repo = await initLocalRepo(db, gitAuth, sourcePath) + rmSync(path.join(reposPath, repo.localPath), { force: true }) + + await deleteRepoFiles(db, repo.id) + + expect(existsSync(path.join(sourcePath, '.git'))).toBe(true) + expect(getRepoById(db, repo.id)).toBeNull() + }) + + it('deletes a cloned repo directory and its row', async () => { + const { deleteRepoFiles } = await import('../../src/services/repo') + const baseName = uniqueName('delete-clone') + const repoPath = path.join(reposPath, baseName) + createCommittedRepo(repoPath) + const repo = createRepo(db, { + repoUrl: `https://github.com/example/${baseName}.git`, + localPath: baseName, + branch: 'main', + defaultBranch: 'main', + cloneStatus: 'ready', + clonedAt: Date.now(), + }) + + await deleteRepoFiles(db, repo.id) + expect(existsSync(repoPath)).toBe(false) expect(getRepoById(db, repo.id)).toBeNull() }) @@ -653,10 +722,29 @@ describe('repo service real git', () => { }) it('enhances a missing local clone error and rolls back the row', async () => { - const { cloneRepo } = await import('../../src/services/repo') + const { cloneRepo, RepoInputError } = await import('../../src/services/repo') const name = uniqueName('clone-missing-source') - await expect(cloneRepo(db, gitAuth, path.join(workspaceRoot, 'no-such-origin'), { directoryName: name })).rejects.toThrow(/does not exist/) + const error = await cloneRepo(db, gitAuth, path.join(workspaceRoot, 'no-such-origin'), { directoryName: name }) + .catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(RepoInputError) + expect((error as Error).message).toMatch(/Repository not found or access denied/) + expect((error as { statusCode: number }).statusCode).toBe(400) + expect(getRepoByLocalPath(db, name)).toBeNull() + }) + + it('reports an unparseable clone URL as a repo input error', async () => { + const { cloneRepo, RepoInputError } = await import('../../src/services/repo') + const name = uniqueName('clone-bad-url') + + const error = await cloneRepo(db, gitAuth, 'not a url !!', { directoryName: name }) + .catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(RepoInputError) + expect((error as Error).message).not.toMatch(/Command failed/) + expect((error as Error).message).toMatch(/Repository not found or access denied/) + expect((error as { statusCode: number }).statusCode).toBe(400) expect(getRepoByLocalPath(db, name)).toBeNull() }) diff --git a/backend/test/services/repo-worktree.test.ts b/backend/test/services/repo-worktree.test.ts index 5b26ecc65..35de7adcc 100644 --- a/backend/test/services/repo-worktree.test.ts +++ b/backend/test/services/repo-worktree.test.ts @@ -21,17 +21,17 @@ describe('repo worktree helpers', () => { worktreePath = path.join(tmpDir, 'feature-x') // Init bare origin - execSync(`git init --bare "${originRepoPath}"`, { env }) + execSync(`git init --bare -b main "${originRepoPath}"`, { env }) - // Clone origin to get a working base repo (default branch is "master" in bare repos) + // Clone origin to get a working base repo (default branch is "main") execSync(`git clone "${originRepoPath}" "${baseRepoPath}"`, { env }) // Set git config for commits execSync(`git -C "${baseRepoPath}" config user.email test@test.com`, { env }) execSync(`git -C "${baseRepoPath}" config user.name Test`, { env }) - // Rename default branch to "main" and push - execSync(`git -C "${baseRepoPath}" branch -m master main`, { env }) + // Ensure the branch is "main" and push + execSync(`git -C "${baseRepoPath}" branch -M main`, { env }) execSync(`git -C "${baseRepoPath}" commit --allow-empty -m "Initial commit"`, { env }) execSync(`git -C "${baseRepoPath}" push origin main`, { env }) diff --git a/backend/test/services/schedule-worktree.test.ts b/backend/test/services/schedule-worktree.test.ts index b5727356d..36764df16 100644 --- a/backend/test/services/schedule-worktree.test.ts +++ b/backend/test/services/schedule-worktree.test.ts @@ -72,12 +72,12 @@ describe('ScheduleWorktreeManager', () => { nonGitDir = path.join(tmpDir, 'non-git-dir') mkdirSync(nonGitDir, { recursive: true }) - // Init bare origin and clone a working base repo - execSync(`git init --bare "${originRepoPath}"`, { env }) + // Init bare origin on main and clone a working base repo + execSync(`git init --bare -b main "${originRepoPath}"`, { env }) execSync(`git clone "${originRepoPath}" "${baseRepoPath}"`, { env }) execSync(`git -C "${baseRepoPath}" config user.email test@test.com`, { env }) execSync(`git -C "${baseRepoPath}" config user.name Test`, { env }) - execSync(`git -C "${baseRepoPath}" branch -m master main`, { env }) + execSync(`git -C "${baseRepoPath}" branch -M main`, { env }) execSync(`git -C "${baseRepoPath}" commit --allow-empty -m "Initial commit"`, { env }) execSync(`git -C "${baseRepoPath}" push origin main`, { env }) execSync(`git -C "${originRepoPath}" symbolic-ref HEAD refs/heads/main`, { env }) diff --git a/backend/test/services/terminal-hub.test.ts b/backend/test/services/terminal-hub.test.ts new file mode 100644 index 000000000..a093a931c --- /dev/null +++ b/backend/test/services/terminal-hub.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it, vi } from 'vitest' +import { TerminalHub } from '../../src/services/terminal-hub' +import type { WebSocketPeer } from '../../src/utils/websocket-bridge' + +class FakeSocket { + binaryType: 'blob' | 'arraybuffer' = 'blob' + readyState = 0 + readonly sent: Array = [] + close = vi.fn() + private readonly listeners = new Map void>>() + + addEventListener(type: string, listener: (event: unknown) => void): void { + const set = this.listeners.get(type) ?? new Set<(event: unknown) => void>() + set.add(listener) + this.listeners.set(type, set) + } + + send(data: string | Uint8Array): void { + this.sent.push(data) + } + + emit(type: string, event: unknown = {}): void { + for (const listener of this.listeners.get(type) ?? []) listener(event) + } +} + +interface FakePeer extends WebSocketPeer { + sent: Array + closeCalls: Array<{ code?: number; reason?: string }> +} + +function createPeer(): FakePeer { + const sent: Array = [] + const closeCalls: Array<{ code?: number; reason?: string }> = [] + return { + sent, + closeCalls, + send(data) { + sent.push(data) + }, + close(code, reason) { + closeCalls.push({ code, reason }) + }, + bufferedAmount() { + return 0 + }, + } +} + +function decodeMeta(data: string | Uint8Array): number { + expect(data).toBeInstanceOf(Uint8Array) + const bytes = data as Uint8Array + expect(bytes[0]).toBe(0) + return (JSON.parse(new TextDecoder().decode(bytes.subarray(1))) as { cursor: number }).cursor +} + +async function createOpenHub(): Promise<{ hub: TerminalHub; socket: FakeSocket }> { + const socket = new FakeSocket() + const hub = new TerminalHub({ openUpstream: async () => socket as unknown as WebSocket }) + await hub.start() + socket.readyState = 1 + socket.emit('open') + return { hub, socket } +} + +describe('TerminalHub', () => { + it('sends retained output, then the cursor, then live output to a late client', async () => { + const { hub, socket } = await createOpenHub() + socket.emit('message', { data: 'hi\r\n' }) + + const peer = createPeer() + hub.attach(peer, undefined) + socket.emit('message', { data: 'more' }) + + expect(peer.sent[0]).toBe('hi\r\n') + expect(decodeMeta(peer.sent[1]!)).toBe(4) + expect(peer.sent[2]).toBe('more') + expect(peer.closeCalls).toEqual([]) + }) + + it('replays an exited terminal and then closes the client', async () => { + const { hub, socket } = await createOpenHub() + socket.emit('message', { data: 'hi' }) + socket.emit('close', { code: 1000 }) + + const peer = createPeer() + hub.attach(peer, undefined) + + expect(peer.sent[0]).toBe('hi') + expect(decodeMeta(peer.sent[1]!)).toBe(2) + expect(peer.closeCalls).toEqual([{ code: 1000, reason: undefined }]) + expect(hub.isEnded()).toBe(true) + }) + + it('replays only the output after the cursor a reconnecting client sends back', async () => { + const { hub, socket } = await createOpenHub() + socket.emit('message', { data: 'first' }) + socket.emit('message', { data: 'second' }) + + const client = createPeer() + hub.attach(client, undefined) + expect(client.sent[1]).toBeInstanceOf(Uint8Array) + hub.attach(client, decodeMeta(client.sent[1]!)) + + const reconnected = createPeer() + hub.attach(reconnected, 5) + + expect(reconnected.sent[0]).toBe('second') + expect(decodeMeta(reconnected.sent[1]!)).toBe(11) + }) + + it('tails an exited terminal without replaying its buffer for cursor -1', async () => { + const { hub, socket } = await createOpenHub() + socket.emit('message', { data: 'hello' }) + socket.emit('close', {}) + + const peer = createPeer() + hub.attach(peer, -1) + + expect(peer.sent).toHaveLength(1) + expect(decodeMeta(peer.sent[0]!)).toBe(5) + expect(peer.closeCalls).toEqual([{ code: 1000, reason: undefined }]) + }) + + it('keeps the retained buffer bounded while the cursor keeps counting', async () => { + const { hub, socket } = await createOpenHub() + socket.emit('message', { data: 'a'.repeat(300 * 1024) }) + + const peer = createPeer() + hub.attach(peer, undefined) + + const replay = peer.sent + .slice(0, -1) + .map((chunk) => chunk as string) + .join('') + expect(replay).toHaveLength(256 * 1024) + expect(decodeMeta(peer.sent[peer.sent.length - 1]!)).toBe(300 * 1024) + }) + + it('queues client input until the upstream opens and then forwards it', async () => { + const socket = new FakeSocket() + const hub = new TerminalHub({ openUpstream: async () => socket as unknown as WebSocket }) + await hub.start() + + const peer = createPeer() + const bridge = hub.attach(peer, undefined) + bridge.send('typed') + expect(socket.sent).toEqual([]) + + socket.readyState = 1 + socket.emit('open') + bridge.send('later') + + expect(socket.sent).toEqual(['typed', 'later']) + }) + + it('stops delivering live output once a client detaches', async () => { + const { hub, socket } = await createOpenHub() + const peer = createPeer() + const bridge = hub.attach(peer, undefined) + bridge.close() + + socket.emit('message', { data: 'after close' }) + + expect(peer.sent.some((chunk) => chunk === 'after close')).toBe(false) + }) +}) diff --git a/backend/test/services/terminal.test.ts b/backend/test/services/terminal.test.ts index ebc610b29..e0128c3f4 100644 --- a/backend/test/services/terminal.test.ts +++ b/backend/test/services/terminal.test.ts @@ -19,7 +19,20 @@ function createPty(overrides: Partial = {}): PtyRecord { } function createClient(pty: Record, api: Record = {}): OpenCodeClient { - return { api: { pty, ...api } } as unknown as OpenCodeClient + const hubToken = vi.fn(async () => ({ data: { ticket: 'hub-ticket', expires_in: 60 } })) + return { api: { pty: { connect: { token: hubToken }, ...pty }, ...api } } as unknown as OpenCodeClient +} + +function createSocketFactory(): { openSocket: (url: string) => WebSocket; sockets: FakeUpstreamSocket[] } { + const sockets: FakeUpstreamSocket[] = [] + return { + sockets, + openSocket: (url) => { + const socket = new FakeUpstreamSocket(url) + sockets.push(socket) + return socket as unknown as WebSocket + }, + } } function createCredentialProvider(shellEnv: Record = {}): CredentialProvider { @@ -29,7 +42,7 @@ function createCredentialProvider(shellEnv: Record = {}): Creden function createTerminalService( client: OpenCodeClient, provider: CredentialProvider, - openSocket?: (url: string) => WebSocket, + openSocket: (url: string) => WebSocket = createSocketFactory().openSocket, baseUrl = 'http://127.0.0.1:5551', includeLocalCredentials = true, ): TerminalService { @@ -139,7 +152,7 @@ describe('TerminalService', () => { location: { directory: '/repo' }, title: 'ocm:action:dev:Dev server', command: '/bin/sh', - args: ['-c', 'npm run dev'], + args: ['-c', 'sleep 0.3; exec "$@"', 'ocm-terminal', '/bin/sh', '-c', 'npm run dev'], env: { GH_TOKEN: 'gh', FOO: 'bar' }, }) expect(result).toEqual({ @@ -170,6 +183,143 @@ describe('TerminalService', () => { expect(captured).not.toHaveProperty('args') expect(captured?.env).toEqual({}) }) + + it('defers a setup command until the manager has attached to the new PTY', async () => { + const create = vi.fn(async () => ({ + location: { directory: '/repo' }, + data: createPty({ id: 'pty-setup', title: 'ocm:setup:Worktree setup' }), + })) + const service = createTerminalService(createClient({ create }), createCredentialProvider()) + + await service.create('/repo', { + kind: 'setup', + name: 'Worktree setup', + command: '/bin/sh', + args: ['-c', 'set -e\npnpm install'], + }) + + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ + command: '/bin/sh', + args: ['-c', 'sleep 0.3; exec "$@"', 'ocm-terminal', '/bin/sh', '-c', 'set -e\npnpm install'], + }), + ) + }) + + it('keeps a plain shell command without deferring it', async () => { + let captured: Record | undefined + const create = vi.fn(async (input: Record) => { + captured = input + return { location: { directory: '/repo' }, data: createPty({ id: 'pty-1', title: 'Terminal' }) } + }) + const service = createTerminalService(createClient({ create }), createCredentialProvider()) + + await service.create('/repo', { kind: 'shell', name: 'Terminal', command: '/bin/zsh', args: ['-l'] }) + + expect(captured).toMatchObject({ command: '/bin/zsh', args: ['-l'] }) + }) + }) + + describe('output capture', () => { + function createCapturingService(actionTerminal = createPty({ id: 'pty-action', title: 'ocm:action:say:Say hi' })) { + const create = vi.fn(async () => ({ location: { directory: '/repo' }, data: actionTerminal })) + const list = vi.fn(async () => ({ location: { directory: '/repo' }, data: [actionTerminal] })) + const factory = createSocketFactory() + const service = createTerminalService( + createClient({ create, list }), + createCredentialProvider(), + factory.openSocket, + ) + return { service, sockets: factory.sockets } + } + + async function openHub(sockets: FakeUpstreamSocket[]): Promise { + await vi.waitFor(() => expect(sockets).toHaveLength(1)) + sockets[0]!.emit('open', {}) + return sockets[0]! + } + + it('opens the upstream at creation so output is captured before the first client attaches', async () => { + const { service, sockets } = createCapturingService() + + await service.create('/repo', { kind: 'shell', name: 'Terminal' }) + const socket = await openHub(sockets) + + const url = new URL(socket.url) + expect(url.pathname).toBe('/api/pty/pty-action/connect') + expect(url.searchParams.get('cursor')).toBe('0') + }) + + it('replays captured output and the exit close frame to a late client', async () => { + const exited = createPty({ id: 'pty-action', title: 'ocm:action:say:Say hi', status: 'exited', exitCode: 0 }) + const { service, sockets } = createCapturingService(exited) + + await service.create('/repo', { kind: 'action', actionId: 'say', name: 'Say hi', command: '/bin/sh', args: ['-c', 'echo hi'] }) + const socket = await openHub(sockets) + socket.emit('message', { data: 'hi\r\n' }) + socket.emit('close', { code: 1000 }) + + const peer = createPeer() + await service.connect('/repo', 'pty-action', undefined, peer) + + expect(peer.sent[0]).toBe('hi\r\n') + expect(peer.sent[1]).toBeInstanceOf(Uint8Array) + const meta = peer.sent[1] as Uint8Array + expect(meta[0]).toBe(0) + expect(JSON.parse(new TextDecoder().decode(meta.subarray(1)))).toEqual({ cursor: 4 }) + expect(peer.closeCalls).toEqual([{ code: 1000, reason: undefined }]) + expect(sockets).toHaveLength(1) + }) + + it('forwards late-client input through the captured upstream', async () => { + const { service, sockets } = createCapturingService() + + await service.create('/repo', { kind: 'shell', name: 'Terminal' }) + const socket = await openHub(sockets) + const peer = createPeer() + const bridge = await service.connect('/repo', 'pty-action', undefined, peer) + + bridge.send('ls\r') + + expect(socket.sent).toEqual(['ls\r']) + }) + + it('streams live output to an attached client without duplicating the replay', async () => { + const { service, sockets } = createCapturingService() + + await service.create('/repo', { kind: 'shell', name: 'Terminal' }) + const socket = await openHub(sockets) + socket.emit('message', { data: 'old' }) + const peer = createPeer() + await service.connect('/repo', 'pty-action', undefined, peer) + + socket.emit('message', { data: 'new' }) + + expect(peer.sent[0]).toBe('old') + expect(peer.sent[2]).toBe('new') + }) + + it('drops the captured upstream when the terminal is removed', async () => { + const removed = createPty({ id: 'pty-action', title: 'Terminal', status: 'exited', exitCode: 0 }) + const remove = vi.fn(async () => undefined) + const factory = createSocketFactory() + const service = createTerminalService( + createClient({ create: vi.fn(async () => ({ data: removed })), list: vi.fn(async () => ({ data: [removed] })), remove }), + createCredentialProvider(), + factory.openSocket, + ) + + await service.create('/repo', { kind: 'shell', name: 'Terminal' }) + await openHub(factory.sockets) + await service.remove('/repo', 'pty-action') + + const peer = createPeer() + await service.connect('/repo', 'pty-action', undefined, peer) + + expect(remove).toHaveBeenCalledWith({ ptyID: 'pty-action', location: { directory: '/repo' } }) + expect(factory.sockets).toHaveLength(2) + expect(new URL(factory.sockets[1]!.url).searchParams.has('cursor')).toBe(false) + }) }) describe('remote target', () => { diff --git a/backend/test/utils/process.test.ts b/backend/test/utils/process.test.ts index d284473f8..657284bf7 100644 --- a/backend/test/utils/process.test.ts +++ b/backend/test/utils/process.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { executeCommand } from '../../src/utils/process' +import { logger } from '../../src/utils/logger' describe('executeCommand signal handling', () => { it('reports a signal-terminated child as a non-zero exit code when exit codes are ignored', async () => { @@ -50,4 +51,21 @@ describe('executeCommand signal handling', () => { expect(result).toBe('hello') }) + + it('logs a git command as one debug line and never at info level', async () => { + const infoSpy = vi.spyOn(logger, 'info').mockImplementation(() => {}) + const debugSpy = vi.spyOn(logger, 'debug').mockImplementation(() => {}) + + try { + await executeCommand(['git', '--version'], { silent: true }) + + expect(infoSpy).not.toHaveBeenCalled() + expect(debugSpy).toHaveBeenCalledTimes(1) + expect(debugSpy.mock.calls[0]).toHaveLength(1) + expect(debugSpy.mock.calls[0]?.[0]).toContain('git --version') + } finally { + infoSpy.mockRestore() + debugSpy.mockRestore() + } + }) }) diff --git a/docker-compose.release.yml b/docker-compose.release.yml index 2ff780114..277607e0e 100644 --- a/docker-compose.release.yml +++ b/docker-compose.release.yml @@ -43,6 +43,7 @@ services: - DISCORD_CLIENT_ID=${DISCORD_CLIENT_ID:-} - DISCORD_CLIENT_SECRET=${DISCORD_CLIENT_SECRET:-} - AUTH_SECURE_COOKIES=${AUTH_SECURE_COOKIES:-false} + - AUTH_TRUSTED_PROXY_HEADERS=${AUTH_TRUSTED_PROXY_HEADERS:-} - PASSKEY_RP_ID=${PASSKEY_RP_ID:-localhost} - PASSKEY_RP_NAME=${PASSKEY_RP_NAME:-OpenCode Manager} - PASSKEY_ORIGIN=${PASSKEY_ORIGIN:-http://localhost:5003} diff --git a/docker-compose.yml b/docker-compose.yml index 31104d2ea..ed4a847ef 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,7 @@ services: - DISCORD_CLIENT_ID=${DISCORD_CLIENT_ID:-} - DISCORD_CLIENT_SECRET=${DISCORD_CLIENT_SECRET:-} - AUTH_SECURE_COOKIES=${AUTH_SECURE_COOKIES:-false} + - AUTH_TRUSTED_PROXY_HEADERS=${AUTH_TRUSTED_PROXY_HEADERS:-} - PASSKEY_RP_ID=${PASSKEY_RP_ID:-localhost} - PASSKEY_RP_NAME=${PASSKEY_RP_NAME:-OpenCode Manager} - PASSKEY_ORIGIN=${PASSKEY_ORIGIN:-http://localhost:5003} diff --git a/e2e/fixtures.ts b/e2e/fixtures.ts index a1ccc54ad..56589d0b1 100644 --- a/e2e/fixtures.ts +++ b/e2e/fixtures.ts @@ -4,18 +4,6 @@ import path from 'node:path' import { test as base, expect, request as playwrightRequest, type Page } from '@playwright/test' import { BASE_URL, E2E_TMP, STORAGE_STATE } from './env' -const UNAUTHENTICATED_BOOTSTRAP_ENDPOINTS = [ - '/api/settings', - '/api/schedules/all/runs/unread', - '/api/repos', - '/api/sse/stream', -] - -function isIgnoredConsoleError(text: string): boolean { - if (!text.includes('401 (Unauthorized)')) return false - return UNAUTHENTICATED_BOOTSTRAP_ENDPOINTS.some((endpoint) => text.includes(endpoint)) -} - export interface RepoFixture { id: number name: string @@ -48,7 +36,6 @@ export const test = base.extend<{ page: Page }, WorkerFixtures>({ if (message.type() !== 'error') return const url = message.location().url const text = `${message.text()}${url ? ` (${url})` : ''}` - if (isIgnoredConsoleError(text)) return errors.push(`console.error: ${text}`) }) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index e3f049fae..6453076d7 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -21,6 +21,7 @@ import { PwaUpdatePrompt } from '@/components/PwaUpdatePrompt' import { MobileTabBar } from '@/components/navigation/MobileTabBar' import { MobileSheetHost } from '@/components/navigation/MobileSheetHost' import { RouteErrorBoundary } from '@/components/ui/route-error-boundary' +import { NotFoundState } from '@/components/ui/not-found-state' import { DesktopSidebar } from '@/components/navigation/DesktopSidebar' import { useRightEdgeSwipe, useSwipeBack } from './hooks/useMobile' import { useMobileTabBar } from '@/hooks/useMobileTabBar' @@ -164,6 +165,19 @@ function ServerScope({ children }: { children: ReactNode }) { return {children} } +function RootLayout() { + return ( + + + + + + + + + ) +} + function AppShell() { const navigate = useNavigate() const location = useLocation() @@ -233,41 +247,36 @@ function AppShell() { ) return ( - - - - - - - -
- -
- -
- -
-
-
- - - - - - - - - - - - - + + + + + +
+ +
+ +
+ +
+
+
+ + + + + + + + + + ) } const router = createBrowserRouter([ { - element: , + element: , errorElement: , children: [ { @@ -286,62 +295,80 @@ const router = createBrowserRouter([ loader: setupLoader, }, { - path: '/', - element: , - loader: protectedLoader, - }, - { - path: '/assistant', - element: , - loader: protectedLoader, - }, - { - path: '/repos/:id', - element: , - loader: protectedLoader, - }, - { - path: '/repos/:id/assistant', - element: , - loader: protectedLoader, - }, - { - path: '/repos/:id/sessions/:sessionId', - element: , - loader: protectedLoader, - }, - { - path: '/repos/:id/schedules', - element: , - loader: protectedLoader, - }, - { - path: '/schedules', - element: , - loader: protectedLoader, - }, - { - path: '/servers/:serverId', - element: , - loader: protectedLoader, - }, - { - path: '/servers/:serverId/projects', - element: ( - - - - ), - loader: protectedLoader, - }, - { - path: '/servers/:serverId/sessions/:sessionId', - element: ( - - - - ), - loader: protectedLoader, + element: , + errorElement: , + children: [ + { + path: '/', + element: , + loader: protectedLoader, + }, + { + path: '/assistant', + element: , + loader: protectedLoader, + }, + { + path: '/repos/:id', + element: , + loader: protectedLoader, + }, + { + path: '/repos/:id/assistant', + element: , + loader: protectedLoader, + }, + { + path: '/repos/:id/sessions/:sessionId', + element: , + loader: protectedLoader, + }, + { + path: '/repos/:id/schedules', + element: , + loader: protectedLoader, + }, + { + path: '/schedules', + element: , + loader: protectedLoader, + }, + { + path: '/servers/:serverId', + element: , + loader: protectedLoader, + }, + { + path: '/servers/:serverId/projects', + element: ( + + + + ), + loader: protectedLoader, + }, + { + path: '/servers/:serverId/sessions/:sessionId', + element: ( + + + + ), + loader: protectedLoader, + }, + { + path: '*', + element: ( + + ), + loader: protectedLoader, + }, + ], }, ], }, @@ -350,11 +377,9 @@ const router = createBrowserRouter([ function App() { return ( - - - - - + + + ) } diff --git a/frontend/src/api/changeWalkthroughs.ts b/frontend/src/api/changeWalkthroughs.ts index 0d8c18498..3ab8d6132 100644 --- a/frontend/src/api/changeWalkthroughs.ts +++ b/frontend/src/api/changeWalkthroughs.ts @@ -12,6 +12,13 @@ function walkthroughUrl(sessionId: string): string { return `${API_BASE_URL}/api/change-walkthroughs/${encodeURIComponent(sessionId)}` } +const HELPER_SESSIONS_URL = `${API_BASE_URL}/api/change-walkthroughs/helper-sessions` + +export async function getWalkthroughHelperSessionIds(): Promise { + const response = await fetchWrapper<{ sessionIds: string[] }>(HELPER_SESSIONS_URL) + return response.sessionIds +} + export async function getChangeWalkthrough( sessionId: string, source: WalkthroughSource, diff --git a/frontend/src/api/repos.ts b/frontend/src/api/repos.ts index 0c529f7c3..541798933 100644 --- a/frontend/src/api/repos.ts +++ b/frontend/src/api/repos.ts @@ -42,6 +42,7 @@ export async function getRepo(id: number): Promise { return fetchWrapper(`${API_BASE_URL}/api/repos/${id}`) } + export function workspaceLabel(workspace: RepoSibling): string { return workspace.currentBranch || workspace.branch || workspace.localPath || 'worktree' } diff --git a/frontend/src/components/file-browser/FileBrowser.tsx b/frontend/src/components/file-browser/FileBrowser.tsx index 835aaae89..eaa8c8d83 100644 --- a/frontend/src/components/file-browser/FileBrowser.tsx +++ b/frontend/src/components/file-browser/FileBrowser.tsx @@ -615,7 +615,7 @@ useEffect(() => { onChange={(e) => setSearchQuery(e.target.value)} className="flex-1" /> - { File Browser - diff --git a/frontend/src/components/file-browser/FileDiffView.tsx b/frontend/src/components/file-browser/FileDiffView.tsx index 481b02cfc..fa29ae260 100644 --- a/frontend/src/components/file-browser/FileDiffView.tsx +++ b/frontend/src/components/file-browser/FileDiffView.tsx @@ -117,6 +117,7 @@ export function FileDiffContent({ size="sm" onClick={onBack} className="h-7 w-7 p-0 mr-1" + aria-label="Back to file list" > diff --git a/frontend/src/components/file-browser/FileOperations.tsx b/frontend/src/components/file-browser/FileOperations.tsx index 577aacab9..f8785e1a0 100644 --- a/frontend/src/components/file-browser/FileOperations.tsx +++ b/frontend/src/components/file-browser/FileOperations.tsx @@ -57,7 +57,7 @@ export const FileOperations = memo(function FileOperations({ onUpload, onCreate {isMobile ? ( - @@ -73,7 +73,7 @@ export const FileOperations = memo(function FileOperations({ onUpload, onCreate ) : ( - @@ -81,7 +81,7 @@ export const FileOperations = memo(function FileOperations({ onUpload, onCreate - diff --git a/frontend/src/components/file-browser/FilePreview.test.tsx b/frontend/src/components/file-browser/FilePreview.test.tsx index 22feb0842..d3c2bf06f 100644 --- a/frontend/src/components/file-browser/FilePreview.test.tsx +++ b/frontend/src/components/file-browser/FilePreview.test.tsx @@ -105,6 +105,14 @@ describe('FilePreview header buttons', () => { expect(external).toHaveAttribute('rel', 'noopener noreferrer') }) + it('shows the binary message and no edit action for non-text files', () => { + render() + + expect(screen.getByText('Binary file - download to view content')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Edit file' })).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Download file' })).toBeInTheDocument() + }) + it('keeps the success and destructive tints on the edit actions in dark mode', () => { render() diff --git a/frontend/src/components/file-browser/FilePreview.tsx b/frontend/src/components/file-browser/FilePreview.tsx index 70ee4879d..3234ff0fd 100644 --- a/frontend/src/components/file-browser/FilePreview.tsx +++ b/frontend/src/components/file-browser/FilePreview.tsx @@ -426,25 +426,25 @@ export const FilePreview = memo(function FilePreview({ file, hideHeader = false, )} {isTextFile && viewMode !== 'edit' && ( - )} {showSaveButton && ( - )} {showCancelButton && ( - )} {viewMode !== 'edit' && ( - )} diff --git a/frontend/src/components/file-browser/FileTree.test.tsx b/frontend/src/components/file-browser/FileTree.test.tsx new file mode 100644 index 000000000..94b5f2315 --- /dev/null +++ b/frontend/src/components/file-browser/FileTree.test.tsx @@ -0,0 +1,31 @@ +import { describe, expect, it, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import type { FileInfo } from '@/types/files' +import { FileTree } from './FileTree' + +const file: FileInfo = { + name: 'notes.txt', + path: 'docker/notes.txt', + isDirectory: false, + size: 5, + mimeType: 'text/plain', + content: '', + lastModified: new Date('2026-07-28T09:12:40Z'), +} + +describe('FileTree row actions', () => { + it('names the per-row actions trigger after the file', () => { + render( + , + ) + + expect(screen.getByRole('button', { name: 'Actions for notes.txt' })).toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/file-browser/FileTree.tsx b/frontend/src/components/file-browser/FileTree.tsx index 394e8a215..54404652a 100644 --- a/frontend/src/components/file-browser/FileTree.tsx +++ b/frontend/src/components/file-browser/FileTree.tsx @@ -181,7 +181,8 @@ function TreeNode({ file, level, onFileSelect, onDirectoryClick, selectedFile, o diff --git a/frontend/src/components/message/ExplorationGroup.test.tsx b/frontend/src/components/message/ExplorationGroup.test.tsx new file mode 100644 index 000000000..39add4ba7 --- /dev/null +++ b/frontend/src/components/message/ExplorationGroup.test.tsx @@ -0,0 +1,85 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen } from '@testing-library/react' +import type { SessionMessageAssistantTool } from '@opencode-manager/shared/opencode' +import { ExplorationGroup } from './ExplorationGroup' + +const mocks = vi.hoisted(() => ({ + useSettings: vi.fn(), + useToolCallPermission: vi.fn(), +})) + +vi.mock('@/hooks/useSettings', () => ({ + useSettings: mocks.useSettings, +})) + +vi.mock('@/contexts/EventContext', () => ({ + useToolCallPermission: mocks.useToolCallPermission, +})) + +const readTool = (id: string, status: 'completed' | 'running' | 'error'): SessionMessageAssistantTool => { + if (status === 'completed') { + return { + type: 'tool', + id, + name: 'read', + state: { status, input: { filePath: '/repo/a.ts' }, content: [], metadata: {} }, + time: { created: 0, ran: 1, completed: 2 }, + } + } + if (status === 'error') { + return { + type: 'tool', + id, + name: 'read', + state: { status, input: { filePath: '/repo/missing.ts' }, error: { type: 'tool.failed', message: 'File not found' }, metadata: {} }, + time: { created: 0, ran: 1, completed: 2 }, + } + } + return { + type: 'tool', + id, + name: 'read', + state: { status, input: { filePath: '/repo/a.ts' } }, + time: { created: 0, ran: 1 }, + } +} + +describe('ExplorationGroup', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.useSettings.mockReturnValue({ + preferences: { expandToolCalls: false }, + isLoading: false, + updateSettings: vi.fn(), + isUpdating: false, + }) + mocks.useToolCallPermission.mockReturnValue(null) + }) + + it('summarises a successful run and keeps the arrow marker', () => { + render() + + const summary = screen.getByRole('button', { name: /Explored: 2 reads/ }) + expect(summary).toHaveTextContent('→') + expect(summary).not.toHaveTextContent('✗') + }) + + it('marks a run with a failed member and counts it in the label', () => { + render() + + const summary = screen.getByRole('button', { name: /Explored: 2 reads, 1 failed/ }) + const marker = screen.getByText('✗') + expect(marker).toHaveClass('text-destructive') + expect(summary).not.toHaveTextContent('→') + }) + + it('keeps the spinner while an exploration run is still in progress', () => { + const { container } = render( + , + ) + + expect(screen.getByRole('button', { name: /Exploring: 2 reads/ })).toBeInTheDocument() + expect(screen.queryByText('✗')).not.toBeInTheDocument() + expect(container.querySelector('.animate-spin')).not.toBeNull() + }) +}) diff --git a/frontend/src/components/message/ExplorationGroup.tsx b/frontend/src/components/message/ExplorationGroup.tsx index c66886047..e4f9197ec 100644 --- a/frontend/src/components/message/ExplorationGroup.tsx +++ b/frontend/src/components/message/ExplorationGroup.tsx @@ -3,7 +3,7 @@ import { ChevronRight, Loader2 } from 'lucide-react' import type { SessionMessageAssistantTool } from '@opencode-manager/shared/opencode' import { useSettings } from '@/hooks/useSettings' import { useToolCallPermission } from '@/contexts/EventContext' -import { explorationLabel, isExplorationComplete } from '@/lib/explorationGroups' +import { explorationFailureCount, explorationLabel, isExplorationComplete } from '@/lib/explorationGroups' import type { WalkthroughSource } from '@opencode-manager/shared/schemas' import { MessagePart } from './MessagePart' @@ -31,6 +31,7 @@ export const ExplorationGroup = memo(function ExplorationGroup({ parts, ...membe const { preferences } = useSettings() const [expanded, setExpanded] = useState(preferences?.expandToolCalls ?? false) const completed = isExplorationComplete(parts) + const failed = completed && explorationFailureCount(parts) > 0 return (
@@ -40,7 +41,13 @@ export const ExplorationGroup = memo(function ExplorationGroup({ parts, ...membe aria-expanded={expanded} className="group flex w-full min-w-0 items-center gap-2 py-0.5 text-left text-sm text-muted-foreground hover:text-foreground transition-colors" > - {completed ? → : } + {failed ? ( + ✗ + ) : completed ? ( + → + ) : ( + + )} {explorationLabel(parts)} diff --git a/frontend/src/components/message/PromptInput.command.test.tsx b/frontend/src/components/message/PromptInput.command.test.tsx index 3438631be..a57767dd7 100644 --- a/frontend/src/components/message/PromptInput.command.test.tsx +++ b/frontend/src/components/message/PromptInput.command.test.tsx @@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { render, screen, fireEvent, act, waitFor } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { PromptInput, type PromptInputHandle } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { BUILTIN_COMMANDS } from '@/lib/builtinCommands' import { createCommandActionsMock, stubMatchMedia, createVoiceConversationControllerMock } from '@/test/test-utils' @@ -174,6 +175,7 @@ describe('PromptInput command submission', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useVoiceConversation.mockReturnValue(createVoiceConversationControllerMock()) mocks.runCommand.mockResolvedValue(undefined) mocks.switchSessionModel.mockResolvedValue(undefined) @@ -496,7 +498,7 @@ describe('PromptInput command submission', () => { expect(screen.queryByTestId('composer-mode-badge')).not.toBeInTheDocument() }) - it('clears attachments and exits bash mode on Escape while idle', async () => { + it('exits bash mode on Escape while idle and keeps the attachments', async () => { const { container } = renderComponent() const input = await screen.findByPlaceholderText('Send a message...') @@ -510,7 +512,7 @@ describe('PromptInput command submission', () => { expect(input).toHaveValue('') expect(screen.queryByTestId('composer-mode-badge')).not.toBeInTheDocument() - expect(screen.queryByText('pic.png')).not.toBeInTheDocument() + expect(screen.getByText('pic.png')).toBeInTheDocument() }) it('keeps attachments and clears only the text when a built-in command is submitted', async () => { diff --git a/frontend/src/components/message/PromptInput.composer.test.tsx b/frontend/src/components/message/PromptInput.composer.test.tsx index e291a6221..7b57c6f6f 100644 --- a/frontend/src/components/message/PromptInput.composer.test.tsx +++ b/frontend/src/components/message/PromptInput.composer.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor, within, act } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { PromptInput } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { useSessionStatus } from '@/stores/sessionStatusStore' import { createCommandActionsMock, createVoiceConversationControllerMock } from '@/test/test-utils' @@ -116,6 +117,7 @@ describe('PromptInput composer layout', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useVoiceConversation.mockReturnValue(createVoiceConversationControllerMock()) mocks.hasPendingPermission.mockReturnValue(false) mocks.useMobile.mockReturnValue(false) diff --git a/frontend/src/components/message/PromptInput.draft.test.tsx b/frontend/src/components/message/PromptInput.draft.test.tsx new file mode 100644 index 000000000..b8b3f4d1c --- /dev/null +++ b/frontend/src/components/message/PromptInput.draft.test.tsx @@ -0,0 +1,258 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { PromptInput } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' +import { createCommandActionsMock, createVoiceConversationControllerMock } from '@/test/test-utils' + +const mocks = vi.hoisted(() => ({ + useSTT: vi.fn(), + useVoiceConversation: vi.fn(), + useMobile: vi.fn(), + useCommands: vi.fn(), + useCommandHandler: vi.fn(), + useFileSearch: vi.fn(), + useModelSelection: vi.fn(), + useVariants: vi.fn(), + useSessionAgent: vi.fn(), + useUserBash: vi.fn(), + useSessionAgentStore: vi.fn(), + useSendErrorStore: vi.fn(), + useHasBackgroundActivity: vi.fn(), + useSendPrompt: vi.fn(), + hasPendingPermission: vi.fn(() => false), + setShowDialog: vi.fn(), +})) + +vi.mock('@/hooks/useSTT', () => ({ + useSTT: mocks.useSTT, + useSttAvailability: () => ({ isEnabled: true, isSupported: true }), +})) +vi.mock('@/hooks/useVoiceConversation', () => ({ useVoiceConversation: mocks.useVoiceConversation })) +vi.mock('@/hooks/useMobile', () => ({ useMobile: mocks.useMobile })) +vi.mock('@/hooks/useCommands', () => ({ useCommands: mocks.useCommands })) +vi.mock('@/hooks/useCommandHandler', () => ({ useCommandHandler: mocks.useCommandHandler })) +vi.mock('@/hooks/useFileSearch', () => ({ useFileSearch: mocks.useFileSearch })) +vi.mock('@/hooks/useModelSelection', () => ({ useModelSelection: mocks.useModelSelection })) +vi.mock('@/hooks/useVariants', () => ({ useVariants: mocks.useVariants })) +vi.mock('@/hooks/useSessionAgent', () => ({ useSessionAgent: mocks.useSessionAgent })) +vi.mock('@/stores/userBashStore', () => ({ useUserBash: mocks.useUserBash })) +vi.mock('@/stores/sessionAgentStore', () => ({ useSessionAgentStore: mocks.useSessionAgentStore })) +vi.mock('@/stores/sendErrorStore', () => ({ useSendErrorStore: mocks.useSendErrorStore })) +vi.mock('@/stores/sessionStatusStore', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useHasBackgroundActivity: mocks.useHasBackgroundActivity } +}) +vi.mock('@/lib/toast', () => ({ + showToast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn(), loading: vi.fn(), promise: vi.fn(), dismiss: vi.fn() }, +})) + +vi.mock('@/hooks/useOpenCode', () => ({ + useSendPrompt: () => ({ mutate: mocks.useSendPrompt, isPending: false }), + useInterruptSession: () => ({ mutate: vi.fn() }), + useSendShell: () => ({ mutate: vi.fn(), isPending: false }), + useAgents: () => ({ data: [] }), + useSkills: () => ({ data: [] }), +})) + +vi.mock('@/hooks/useProviders', () => ({ useProviders: () => ({ data: undefined }) })) + +vi.mock('@/contexts/EventContext', () => ({ + usePermissions: () => ({ + hasForSession: mocks.hasPendingPermission, + setShowDialog: mocks.setShowDialog, + }), +})) + +vi.mock('@/components/agent/AgentQuickSelect', () => ({ + AgentQuickSelect: () =>
, +})) + +vi.mock('@/components/model/ModelQuickSelect', () => ({ + ModelQuickSelect: ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ), +})) + +vi.mock('@/components/session/PermissionModeToggle', () => ({ PermissionModeToggle: () => null })) +vi.mock('@/components/session/SessionSandboxMenu', () => ({ SessionSandboxMenu: () => null })) +vi.mock('./ComposerToolsMenu', () => ({ ComposerToolsMenu: () => null })) +vi.mock('./PromptSuggestions', () => ({ PromptSuggestions: () => null })) +vi.mock('@/hooks/useSessionGoals', () => ({ + useSessionGoal: () => ({ data: undefined }), + useStartSessionGoal: () => ({ mutateAsync: vi.fn(), isPending: false }), +})) +vi.mock('@/hooks/useSessionPermissionMode', () => ({ useSessionPermissionMode: () => ({ data: undefined }) })) + +const createTestQueryClient = () => new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, +}) + +const defaultProps = { + directory: '/test', + sessionID: 'session-a', + showScrollButton: false, + isSessionActive: false, + isStreamingResponse: false, + onScrollToBottom: vi.fn(), + commandActions: createCommandActionsMock(), + onPromptChange: vi.fn(), +} + +describe('PromptInput per-session composer draft', () => { + const harness = (sessionID: string, overrides: Partial = {}) => ( + + + + ) + + const renderSession = (sessionID: string, overrides: Partial = {}) => + render(harness(sessionID, overrides)) + + const promptInput = () => screen.getByPlaceholderText(/Send a message\.\.\.|Enter bash command\.\.\./) + + const attachImage = async (container: HTMLElement) => { + const fileInput = container.querySelector('input[type="file"]') as HTMLInputElement + fireEvent.change(fileInput, { target: { files: [new File(['img'], 'pic.png', { type: 'image/png' })] } }) + await screen.findByText('pic.png') + } + + beforeEach(() => { + vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) + mocks.useSendPrompt.mockImplementation((_variables, options) => { + options?.onSuccess?.() + }) + mocks.useVoiceConversation.mockReturnValue(createVoiceConversationControllerMock()) + mocks.hasPendingPermission.mockReturnValue(false) + mocks.useMobile.mockReturnValue(false) + mocks.useSTT.mockReturnValue({ + isRecording: false, + isProcessing: false, + isSupported: false, + isEnabled: false, + interimTranscript: '', + transcript: '', + startRecording: vi.fn(), + stopRecording: vi.fn(), + abortRecording: vi.fn(), + reset: vi.fn(), + clear: vi.fn(), + }) + mocks.useCommands.mockReturnValue({ searchCommands: () => [], findCommand: () => undefined, recentNames: [] }) + mocks.useCommandHandler.mockReturnValue({ executeCommand: vi.fn() }) + mocks.useFileSearch.mockReturnValue({ files: [] }) + mocks.useModelSelection.mockReturnValue({ + model: null, + modelString: 'test-model', + setModel: vi.fn(), + setActiveAgent: vi.fn(), + recentModels: [], + favoriteModels: [], + toggleFavorite: vi.fn(), + isModelReady: true, + }) + mocks.useVariants.mockReturnValue({ hasVariants: false, currentVariant: null, cycleVariant: vi.fn() }) + mocks.useSessionAgent.mockReturnValue({ agent: 'default' }) + mocks.useUserBash.mockImplementation((selector: (state: unknown) => unknown) => selector({ addUserBashCommand: vi.fn() })) + mocks.useSessionAgentStore.mockImplementation((selector: (state: unknown) => unknown) => selector({ setAgent: vi.fn() })) + mocks.useSendErrorStore.mockImplementation((selector: (state: unknown) => unknown) => selector({ errors: {} })) + mocks.useHasBackgroundActivity.mockReturnValue(false) + }) + + it('keeps the typed draft of each session', () => { + const { rerender } = renderSession('session-a') + + fireEvent.change(promptInput(), { target: { value: 'draft for A' } }) + + rerender(harness('session-b')) + expect(promptInput()).toHaveValue('') + + fireEvent.change(promptInput(), { target: { value: 'draft for B' } }) + + rerender(harness('session-a')) + expect(promptInput()).toHaveValue('draft for A') + + rerender(harness('session-b')) + expect(promptInput()).toHaveValue('draft for B') + }) + + it('keeps bash mode and attachments of each session', async () => { + const { container, rerender } = renderSession('session-a') + + fireEvent.change(promptInput(), { target: { value: '!' } }) + await attachImage(container) + expect(screen.getByTestId('composer-mode-badge')).toHaveTextContent('BASH') + + rerender(harness('session-b')) + expect(promptInput()).toHaveValue('') + expect(screen.queryByTestId('composer-mode-badge')).not.toBeInTheDocument() + expect(screen.queryByText('pic.png')).not.toBeInTheDocument() + + rerender(harness('session-a')) + expect(promptInput()).toHaveValue('!') + expect(screen.getByPlaceholderText('Enter bash command...')).toBeInTheDocument() + expect(screen.getByTestId('composer-mode-badge')).toHaveTextContent('BASH') + expect(screen.getByText('pic.png')).toBeInTheDocument() + }) + + it('sends the restored draft to the session that owns it and clears only that draft', () => { + const { rerender } = renderSession('session-a') + + fireEvent.change(promptInput(), { target: { value: 'draft for A' } }) + + rerender(harness('session-b')) + fireEvent.change(promptInput(), { target: { value: 'draft for B' } }) + + rerender(harness('session-a')) + fireEvent.click(screen.getByTitle('Send')) + + expect(mocks.useSendPrompt).toHaveBeenCalledWith( + expect.objectContaining({ sessionID: 'session-a', text: 'draft for A' }), + expect.anything(), + ) + expect(promptInput()).toHaveValue('') + + rerender(harness('session-b')) + expect(promptInput()).toHaveValue('draft for B') + }) + + it('keeps the draft when Escape is pressed while the session is idle', () => { + renderSession('session-a') + + fireEvent.change(promptInput(), { target: { value: 'an important draft' } }) + fireEvent.keyDown(promptInput(), { key: 'Escape' }) + + expect(promptInput()).toHaveValue('an important draft') + }) + + it('reports suggestion list changes instead of clearing on Escape', async () => { + const onSuggestionsOpenChange = vi.fn() + mocks.useCommands.mockReturnValue({ + searchCommands: () => [{ item: { name: 'alpha', description: 'Alpha' }, ranges: [] }], + findCommand: () => undefined, + recentNames: [], + }) + + renderSession('session-a', { onSuggestionsOpenChange }) + expect(onSuggestionsOpenChange).toHaveBeenLastCalledWith(false) + + fireEvent.change(promptInput(), { target: { value: '/al', selectionStart: 3 } }) + + await waitFor(() => expect(onSuggestionsOpenChange).toHaveBeenLastCalledWith(true)) + expect(promptInput()).toHaveValue('/al') + + fireEvent.keyDown(promptInput(), { key: 'Escape' }) + + await waitFor(() => expect(onSuggestionsOpenChange).toHaveBeenLastCalledWith(false)) + expect(promptInput()).toHaveValue('/al') + }) + + it('labels the attachment remove button with the filename', async () => { + const { container } = renderSession('session-a') + + await attachImage(container) + + expect(screen.getByRole('button', { name: 'Remove pic.png' })).toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/message/PromptInput.goal.test.tsx b/frontend/src/components/message/PromptInput.goal.test.tsx index da4a8efdb..e4d80e490 100644 --- a/frontend/src/components/message/PromptInput.goal.test.tsx +++ b/frontend/src/components/message/PromptInput.goal.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { render, screen, fireEvent, waitFor, act } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { PromptInput } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { createCommandActionsMock, stubMatchMedia, createVoiceConversationControllerMock } from '@/test/test-utils' const mocks = vi.hoisted(() => ({ @@ -164,6 +165,7 @@ describe('PromptInput goal mode', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useVoiceConversation.mockReturnValue(createVoiceConversationControllerMock()) mocks.useSessionGoal.mockReturnValue({ data: null }) mocks.useSessionPermissionMode.mockReturnValue({ data: undefined }) diff --git a/frontend/src/components/message/PromptInput.mention.test.tsx b/frontend/src/components/message/PromptInput.mention.test.tsx index fa7ae0ed7..b686b7bbe 100644 --- a/frontend/src/components/message/PromptInput.mention.test.tsx +++ b/frontend/src/components/message/PromptInput.mention.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { PromptInput } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { createCommandActionsMock, createVoiceConversationControllerMock } from '@/test/test-utils' const mocks = vi.hoisted(() => ({ @@ -9,7 +10,7 @@ const mocks = vi.hoisted(() => ({ switchSessionModel: vi.fn(), switchSessionAgent: vi.fn(), runCommand: vi.fn(), - agents: [] as Array<{ name: string; description?: string }>, + agents: [] as Array<{ name: string; description?: string; mode?: string; hidden?: boolean }>, skills: [] as Array<{ id: string; name: string; description?: string }>, useSTT: vi.fn(), useVoiceConversation: vi.fn(), @@ -24,7 +25,7 @@ const mocks = vi.hoisted(() => ({ useSendErrorStore: vi.fn(), })) -const suggestionProps = vi.hoisted(() => ({ isOpen: false, selectedIndex: 0 })) +const suggestionProps = vi.hoisted(() => ({ isOpen: false, selectedIndex: 0, labels: [] as string[] })) vi.mock('@/api/opencode', async () => { const actual = await vi.importActual('@/api/opencode') @@ -95,10 +96,11 @@ vi.mock('@/hooks/useSessionPermissionMode', () => ({ })) vi.mock('./PromptSuggestions', () => ({ - PromptSuggestions: (props: { isOpen: boolean; selectedIndex: number }) => { + PromptSuggestions: (props: { isOpen: boolean; selectedIndex: number; items: Array<{ label: string }> }) => { if (props.isOpen) { suggestionProps.isOpen = true suggestionProps.selectedIndex = props.selectedIndex + suggestionProps.labels = props.items.map((item) => item.label) } return null }, @@ -153,6 +155,7 @@ describe('PromptInput agent mention submission', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useVoiceConversation.mockReturnValue(createVoiceConversationControllerMock()) mocks.sendPrompt.mockResolvedValue({ id: 'inbox_1', @@ -199,6 +202,7 @@ describe('PromptInput agent mention submission', () => { mocks.useSessionAgentStore.mockImplementation((selector: (state: unknown) => unknown) => selector({ setAgent: vi.fn() })) suggestionProps.isOpen = false suggestionProps.selectedIndex = 0 + suggestionProps.labels = [] }) it('sends an @agent mention as an attachment without switching the session agent', async () => { @@ -311,4 +315,23 @@ describe('PromptInput agent mention submission', () => { fireEvent.keyDown(input, { key: 'ArrowDown' }) await waitFor(() => expect(suggestionProps.selectedIndex).toBe(1)) }) + + it('offers only mentionable subagents in the @ list', async () => { + mocks.agents = [ + { name: 'build', description: 'Build', mode: 'primary', hidden: false }, + { name: 'plan', description: 'Plan', mode: 'primary', hidden: false }, + { name: 'explore', description: 'Explore', mode: 'subagent', hidden: false }, + { name: 'general', description: 'General', mode: 'all', hidden: false }, + { name: 'Compaction', description: 'Compaction', mode: 'primary', hidden: true }, + { name: 'Title', description: 'Title', mode: 'primary', hidden: true }, + ] + mocks.skills = [] + renderComponent() + + const input = await screen.findByPlaceholderText('Send a message...') + fireEvent.change(input, { target: { value: '@', selectionStart: 1 } }) + + await waitFor(() => expect(suggestionProps.isOpen).toBe(true)) + expect(suggestionProps.labels).toEqual(['explore', 'general']) + }) }) diff --git a/frontend/src/components/message/PromptInput.stt.test.tsx b/frontend/src/components/message/PromptInput.stt.test.tsx index ae5078ce8..8a77fb363 100644 --- a/frontend/src/components/message/PromptInput.stt.test.tsx +++ b/frontend/src/components/message/PromptInput.stt.test.tsx @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { PromptInput } from './PromptInput' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { SessionTranscriptContext, type SessionTranscriptSnapshot, @@ -220,6 +221,7 @@ describe('PromptInput voice conversation', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useSendPromptMutate.mockImplementation((_variables, options) => { options?.onSuccess?.() }) @@ -423,6 +425,7 @@ describe('PromptInput submission', () => { beforeEach(() => { vi.clearAllMocks() + useComposerDraftStore.setState({ drafts: {} }) mocks.useSendPromptMutate.mockImplementation((_variables, options) => { options?.onSuccess?.() }) diff --git a/frontend/src/components/message/PromptInput.tsx b/frontend/src/components/message/PromptInput.tsx index e416b0a7e..22bdd9364 100644 --- a/frontend/src/components/message/PromptInput.tsx +++ b/frontend/src/components/message/PromptInput.tsx @@ -11,6 +11,7 @@ import { useSttAvailability } from '@/hooks/useSTT' import { useUserBash } from '@/stores/userBashStore' import { useSessionAgentStore } from '@/stores/sessionAgentStore' import { useSendErrorStore } from '@/stores/sendErrorStore' +import { useComposerDraftStore } from '@/stores/composerDraftStore' import { useRecentCommandsStore } from '@/stores/recentCommandsStore' import { useHasBackgroundActivity } from '@/stores/sessionStatusStore' import { useTouchTapSelect } from '@/hooks/useTouchTapSelect' @@ -119,6 +120,7 @@ interface PromptInputProps { directory?: string sessionID: string onVoiceConversationActiveChange?: (active: boolean) => void + onSuggestionsOpenChange?: (open: boolean) => void showScrollButton?: boolean isSessionActive?: boolean isStreamingResponse?: boolean @@ -131,6 +133,7 @@ export const PromptInput = memo(forwardRef( directory, sessionID, onVoiceConversationActiveChange, + onSuggestionsOpenChange, showScrollButton, isSessionActive = false, isStreamingResponse = false, @@ -138,13 +141,14 @@ export const PromptInput = memo(forwardRef( commandActions, onPromptChange }, ref) { - const [prompt, setPrompt] = useState('') - const [isBashMode, setIsBashMode] = useState(false) + const [restoredDraft] = useState(() => useComposerDraftStore.getState().getDraft(sessionID)) + const [prompt, setPrompt] = useState(restoredDraft.prompt) + const [isBashMode, setIsBashMode] = useState(restoredDraft.isBashMode) const [isGoalArmed, setIsGoalArmed] = useState(false) const [showSuggestions, setShowSuggestions] = useState(false) const [suggestionQuery, setSuggestionQuery] = useState('') - const [attachedFiles, setAttachedFiles] = useState(new Map()) - const [imageAttachments, setImageAttachments] = useState([]) + const [attachedFiles, setAttachedFiles] = useState(restoredDraft.attachedFiles) + const [imageAttachments, setImageAttachments] = useState(restoredDraft.imageAttachments) const [isDragging, setIsDragging] = useState(false) const [isModelPickerOpen, setIsModelPickerOpen] = useState(false) const [showMentionSuggestions, setShowMentionSuggestions] = useState(false) @@ -183,6 +187,17 @@ export const PromptInput = memo(forwardRef( imageAttachmentsRef.current = imageAttachments }, [attachedFiles, imageAttachments, prompt]) + const saveDraft = useComposerDraftStore((state) => state.saveDraft) + const clearDraft = useComposerDraftStore((state) => state.clearDraft) + + useEffect(() => { + if (prompt === '' && !isBashMode && attachedFiles.size === 0 && imageAttachments.length === 0) { + clearDraft(sessionID) + return + } + saveDraft(sessionID, { prompt, isBashMode, attachedFiles, imageAttachments }) + }, [sessionID, prompt, isBashMode, attachedFiles, imageAttachments, saveDraft, clearDraft]) + const clearSubmittedPrompt = useCallback((submittedPrompt: string, submittedAttachedFiles: Map, submittedImageAttachments: ImageAttachment[], options?: { keepAttachments?: boolean }) => { if ( promptRef.current !== submittedPrompt || @@ -246,6 +261,10 @@ export const PromptInput = memo(forwardRef( const { data: agents = [] } = useAgents(directory) const { data: skills = [] } = useSkills(directory) + const mentionableAgents = useMemo( + () => agents.filter((agent) => !agent.hidden && agent.mode !== 'primary'), + [agents], + ) const agentNames = useMemo(() => agents.map((agent) => agent.name), [agents]) const skillIds = useMemo(() => skills.map((skill) => skill.id), [skills]) const failedPrompt = useSendErrorStore((state) => state.errors[sessionID]?.failedPrompt) @@ -269,7 +288,7 @@ export const PromptInput = memo(forwardRef( }, [failedPrompt]) const mentionSuggestions = useMemo((): PromptSuggestion[] => { - const agentMatches = rankByMatch(agents, mentionQuery, { + const agentMatches = rankByMatch(mentionableAgents, mentionQuery, { getName: (agent) => agent.name, getDescription: (agent) => agent.description, }).map(({ item, tier, ranges }) => ({ @@ -321,7 +340,7 @@ export const PromptInput = memo(forwardRef( return [...agentMatches, ...skillMatches, ...fileMatches] .sort((a, b) => a.tier - b.tier) .map(({ suggestion }) => suggestion) - }, [agents, skills, searchResults, mentionQuery]) + }, [mentionableAgents, skills, searchResults, mentionQuery]) const commandSuggestions = useMemo((): PromptSuggestion[] => searchCommands(suggestionQuery).map(({ item, ranges }) => ({ @@ -735,7 +754,10 @@ if (isIOS && isSecureContext && navigator.clipboard && navigator.clipboard.read) const handleKeyDown = (e: KeyboardEvent) => { if (isBashMode && e.key === 'Escape') { e.preventDefault() - resetPrompt() + if (prompt === '!' || prompt === '') { + setIsBashMode(false) + setPrompt('') + } return } @@ -815,8 +837,6 @@ if (isIOS && isSecureContext && navigator.clipboard && navigator.clipboard.read) textareaRef.current?.blur() } handleSubmit() - } else if (e.key === 'Escape' && !isSessionActive) { - resetPrompt() } } @@ -1063,6 +1083,12 @@ if (isIOS && isSecureContext && navigator.clipboard && navigator.clipboard.read) }) const isCommandListOpen = showSuggestions && commandSuggestions.length > 0 const isMentionListOpen = showMentionSuggestions && mentionSuggestions.length > 0 + const isSuggestionListOpen = isCommandListOpen || isMentionListOpen + + useEffect(() => { + onSuggestionsOpenChange?.(isSuggestionListOpen) + }, [isSuggestionListOpen, onSuggestionsOpenChange]) + const composerMode: ComposerMode | null = isBashMode ? 'bash' : isMentionListOpen @@ -1071,7 +1097,7 @@ if (isIOS && isSecureContext && navigator.clipboard && navigator.clipboard.read) ? 'command' : null const composerModeStyle = composerMode ? COMPOSER_MODE_STYLES[composerMode] : null - const isTakeoverOpen = isMobile && (isCommandListOpen || isMentionListOpen) + const isTakeoverOpen = isMobile && isSuggestionListOpen const showStopButton = isSessionActive const showMobileScrollButton = isMobile && showScrollButton const hasOpenGoal = isOpenSessionGoal(sessionGoal) @@ -1279,6 +1305,7 @@ return ( {attachment.filename}