feat(workspace): add workspace provider for remote development environments#1292
feat(workspace): add workspace provider for remote development environments#1292arnestrickmann merged 21 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…s EMDASH_TASK_ID to teardown
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brings in 280 commits from main including MCP servers, TaskScopeProvider, gitlab/plain/forgejo integrations, preflight worktree checks, and more. All workspace provider functionality preserved. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryAdds a "Bring Your Own Infrastructure" workspace provider system that lets users define custom shell scripts (via
Key issues found:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/main/services/WorkspaceProviderService.ts | Core workspace lifecycle service (~458 lines). Well-structured with clear separation of provision/terminate/query flows. Issues: fire-and-forget error handling in provision() doesn't update DB status on outer catch; potential unique index violation in createSshConnection name field. |
| src/main/db/schema.ts | Adds workspace_instances table with proper foreign keys (cascade delete on task, set null on SSH connection), indexes, and Drizzle relations. Schema is clean and consistent with existing patterns. |
| src/main/services/ssh/SshService.ts | Improves SSH config alias resolution in buildConnectConfig by resolving host/port/username from ~/.ssh/config. Good enhancement that enables SSH aliases for workspace connections. Does not handle wildcard Host * entries (pre-existing parser limitation). |
| src/main/services/fsIpc.ts | Adds fs:ensureGitignore IPC handler to idempotently add patterns to .gitignore. Minor edge case with negation patterns but overall solid implementation. |
| src/main/services/ptyIpc.ts | Forces tmux for workspace-provisioned connections via connectionId.startsWith('workspace-') string convention. Works but couples behavior to a naming convention rather than a more explicit flag. |
| src/renderer/components/WorkspaceProvisioningOverlay.tsx | Overlay component for provisioning progress/error UI. Clean event-driven architecture. Issue: unbounded log line accumulation could cause memory pressure with verbose scripts. |
| src/renderer/hooks/useWorkspaceConnection.ts | Hook to resolve SSH connection info for workspace tasks. Issue: uses object reference (task?.metadata?.workspace) in useEffect dependency array, causing unnecessary re-runs on each render. |
| src/renderer/lib/taskCreationService.ts | Extends task creation to support remote workspace provisioning. Properly stores workspace config in metadata for teardown, and fires provisioning as background fire-and-forget with error logging. |
| src/renderer/components/ChatInterface.tsx | Integrates workspace connection overrides into terminal and remote settings. Correctly gates PTY start on workspace readiness to prevent launching terminals against localhost when a remote workspace is expected. |
| src/renderer/hooks/useTaskManagement.ts | Adds workspace termination to task delete and archive flows. Best-effort teardown with toast notifications. Also extends handleCreateTask signature with workspace parameters. |
Sequence Diagram
sequenceDiagram
participant User
participant TaskModal
participant taskCreationService
participant IPC as IPC (preload)
participant WorkspaceIpc
participant WPS as WorkspaceProviderService
participant Script as Provision Script
participant DB as SQLite DB
participant Overlay as WorkspaceProvisioningOverlay
participant ChatInterface
User->>TaskModal: Create task (Remote workspace)
TaskModal->>taskCreationService: createTask(useRemoteWorkspace=true)
taskCreationService->>DB: Save task with workspace metadata
taskCreationService->>IPC: workspaceProvision(...)
IPC->>WorkspaceIpc: workspace:provision
WorkspaceIpc->>WPS: provision(config)
WPS->>DB: INSERT workspace_instances (provisioning)
WPS-->>WorkspaceIpc: instanceId
WorkspaceIpc-->>IPC: { success, instanceId }
WPS->>Script: spawn bash -c provisionCommand
loop stderr lines
Script-->>WPS: stderr output
WPS-->>Overlay: provision-progress event
end
Script-->>WPS: JSON stdout + exit 0
WPS->>DB: INSERT ssh_connections
WPS->>DB: UPDATE workspace_instances (ready)
WPS-->>Overlay: provision-complete (ready)
Overlay-->>ChatInterface: Overlay unmounts
ChatInterface->>IPC: workspaceStatus(taskId)
ChatInterface->>ChatInterface: Start PTY with workspace connection
Last reviewed commit: ca11343
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| cancelled = true; | ||
| unsubComplete(); | ||
| }; | ||
| }, [task?.id, task?.metadata?.workspace]); |
There was a problem hiding this comment.
Object reference in dependency array
task?.metadata?.workspace is an object, so React's reference equality check will cause this effect to re-run on every render where the task's metadata is a new object (which is common after deserialization from the DB). This triggers repeated IPC calls and event re-subscriptions on each re-render.
Consider using a primitive derived value instead:
| }, [task?.id, task?.metadata?.workspace]); | |
| }, [task?.id, !!task?.metadata?.workspace]); |
| useEffect(() => { | ||
| const unsubProgress = window.electronAPI.onWorkspaceProvisionProgress( | ||
| (data: { instanceId: string; line: string }) => { | ||
| // Only handle events for our task's workspace instance | ||
| if (instanceIdRef.current && data.instanceId !== instanceIdRef.current) return; | ||
| setLines((prev) => [...prev, data.line]); | ||
| } | ||
| ); | ||
|
|
||
| const unsubComplete = window.electronAPI.onWorkspaceProvisionComplete( | ||
| (data: { instanceId: string; status: string; error?: string }) => { | ||
| if (instanceIdRef.current && data.instanceId !== instanceIdRef.current) return; | ||
| if (data.status === 'ready') { | ||
| setStatus('ready'); | ||
| toast({ title: 'Workspace connected', description: 'Remote workspace is ready.' }); | ||
| } else { | ||
| setStatus('error'); | ||
| setErrorMessage(data.error || 'Workspace provisioning failed.'); | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| return () => { | ||
| unsubProgress(); | ||
| unsubComplete(); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Unbounded log accumulation may cause memory pressure
The setLines((prev) => [...prev, data.line]) pattern at line 68 appends to the array indefinitely. A long-running provision script that emits a lot of stderr output (e.g., apt-get install or docker pull output) could accumulate thousands of lines, creating memory pressure and making the scroll container sluggish.
Consider capping the array size, for example:
setLines((prev) => {
const next = [...prev, data.line];
return next.length > 500 ? next.slice(-500) : next;
});
| private async createSshConnection(instanceId: string, output: ProvisionOutput): Promise<string> { | ||
| const connectionId = `workspace-${instanceId}`; | ||
| const { db } = await getDrizzleClient(); | ||
| const now = new Date().toISOString(); | ||
|
|
||
| await db.insert(sshConnections).values({ | ||
| id: connectionId, | ||
| name: `workspace-${instanceId.slice(0, 8)}-${output.host}`, | ||
| host: output.host, | ||
| port: output.port ?? 22, | ||
| username: output.username ?? process.env.USER ?? 'root', | ||
| authType: 'agent', | ||
| useAgent: 1, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
| }); | ||
|
|
||
| return connectionId; | ||
| } |
There was a problem hiding this comment.
Unique index violation on SSH connection name
The ssh_connections table has a unique index on name (uniqueIndex('idx_ssh_connections_name')). The name here is workspace-${instanceId.slice(0, 8)}-${output.host}. While UUID collisions in the first 8 characters are rare, if a user retries provisioning for the same host, or if two workspaces target the same host, the combination instanceId.slice(0, 8) + host could theoretically collide (since only 8 hex chars = 4 bytes of entropy).
More practically, if provision errors and the user clicks "Retry" in the overlay, a new instanceId is generated (via randomUUID()), but the old SSH connection row from the failed attempt is never cleaned up. The new insert could collide with the old row's name if the 8-char prefix happens to match.
Consider using the full instanceId in the name, or adding ON CONFLICT handling to the insert.
| status: 'provisioning', | ||
| createdAt: Date.now(), | ||
| }); | ||
|
|
||
| // Fire and forget — the caller listens for events. | ||
| this.runProvision(instanceId, config).catch((err) => { | ||
| log.error('[WorkspaceProvider] Unhandled provision error', { instanceId, error: err }); | ||
| }); |
There was a problem hiding this comment.
Provision race: DB row can be left as "provisioning" if runProvision rejects before spawning
The catch in .catch((err) => { log.error(...) }) on line 86 only logs the error — it does not update the DB row to 'error' status. If runProvision throws synchronously before reaching the internal try/catch (e.g., getDrizzleClient() fails during the SSH connection creation), the workspace instance row will remain stuck in provisioning status until the next app restart calls reconcileOnStartup.
This matters because getActiveInstance filters for 'provisioning' or 'ready', so the stuck row prevents future provisions for that task.
Consider updating the status to 'error' in the outer catch:
| status: 'provisioning', | |
| createdAt: Date.now(), | |
| }); | |
| // Fire and forget — the caller listens for events. | |
| this.runProvision(instanceId, config).catch((err) => { | |
| log.error('[WorkspaceProvider] Unhandled provision error', { instanceId, error: err }); | |
| }); | |
| this.runProvision(instanceId, config).catch(async (err) => { | |
| log.error('[WorkspaceProvider] Unhandled provision error', { instanceId, error: err }); | |
| await this.updateStatus(instanceId, 'error').catch(() => {}); | |
| }); |
| content = fs.readFileSync(gitignorePath, 'utf8'); | ||
| } | ||
|
|
||
| const existingLines = new Set(content.split('\n').map((line) => line.trim())); |
There was a problem hiding this comment.
Gitignore line matching ignores comments and negation patterns
The duplicate check trims each existing line and compares directly. This means if a pattern like ./scripts/create-workspace.sh is already present but commented out (e.g., # ./scripts/create-workspace.sh), the trimmed comment line won't match the pattern (correct behavior). However, .gitignore also supports negation (!pattern). If a user has !./scripts/create-workspace.sh in their gitignore, the existingLines set will contain !./scripts/create-workspace.sh, which won't match ./scripts/create-workspace.sh, so it will be added — creating a contradictory gitignore where the pattern is both negated and included. This is an edge case, but worth noting.
1 participant
Summary
WorkspaceProviderServiceto provision and manage remote workspace instances (e.g., Codespaces, Devboxes) for tasksworkspace_instancesdatabase table and migration to persist workspace stateuseWorkspaceConnectionhook for managing workspace connection state in the rendererSshServiceandsshConfigParserKey Changes
WorkspaceProviderService(~465 lines) handles workspace lifecycle — create, start, stop, delete, and status pollingworkspace_instancestable with schema and migration (0010_add_workspace_instances.sql)workspaceIpc.tswith handlers for CRUD operations, exposed viapreload.tsWorkspaceProvisioningOverlaycomponent, updates toTaskModal,TaskAdvancedSettings,ConfigEditorModal,RightSidebar, andChatInterfaceuseTaskManagementandtaskCreationServiceupdated to integrate workspace provisioningHostaliases when looking up connection detailsTest Plan
WorkspaceProviderService.test.tswith 479 lines of unit tests covering service operationspnpm exec vitest run)