[codex] add web terminal presets#4653
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds host agent preset configuration and terminal launching. A new ChangesPreset terminal launcher integration
Sequence DiagramssequenceDiagram
participant WorkspacePage
participant listHostAgentConfigs
participant WebTerminalPresetsBar
participant runPreset
participant createHostTerminal
WorkspacePage->>listHostAgentConfigs: Fetch presets on mount
listHostAgentConfigs-->>WorkspacePage: HostAgentConfig[]
WorkspacePage->>WebTerminalPresetsBar: Render with presets, runningPresetId
WebTerminalPresetsBar-->>WorkspacePage: onRunPreset(preset)
WorkspacePage->>runPreset: User clicks preset button
runPreset->>runPreset: buildHostAgentLaunchCommand(preset)
runPreset->>createHostTerminal: Create terminal with initialCommand
createHostTerminal-->>runPreset: HostTerminalSession
runPreset->>WorkspacePage: Update activeTerminalId, clear runningPresetId
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit f9d0e16 on May 16, 2026 11:55pm UTC. |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR adds a web terminal presets bar that fetches host agent configs and lets users launch a preconfigured terminal session in one click. It also fixes the relay URL for browser-side host calls by writing
Confidence Score: 4/5Safe to merge; the shell-quoting helpers and state management changes are correct, with two areas that need clarification or a quick follow-up. The core plumbing — relay URL, parallel preset/terminal loading, derived active terminal, and per-preset launch — is sound. Two items warrant attention: the Settings gear button is visually interactive but wired to nothing, and buildHostAgentLaunchCommand silently ignores promptArgs/promptTransport, which may be intentional but could also mean presets launch the agent binary without the expected initial prompt. apps/web/src/trpc/host-client.ts (promptArgs unused) and apps/web/src/app/workspaces/[workspaceId]/components/WebTerminalPresetsBar/WebTerminalPresetsBar.tsx (Settings button no-op)
|
| Filename | Overview |
|---|---|
| apps/web/src/trpc/host-client.ts | Adds listHostAgentConfigs, extends createHostTerminal with initialCommand, and introduces shell-quoting helpers. The env key in envOverlayPrefix is unvalidated, and promptArgs/promptTransport go unused in buildHostAgentLaunchCommand. |
| apps/web/src/app/workspaces/[workspaceId]/page.tsx | Adds preset loading alongside terminals via Promise.allSettled, runPreset callback, and replaces the useEffect-driven selectedTerminalId with a derived activeTerminalId. Logic is sound with proper error handling. |
| apps/web/src/app/workspaces/[workspaceId]/components/WebTerminalPresetsBar/WebTerminalPresetsBar.tsx | New preset bar component with loading state, per-preset spinner, and icon rendering. The Manage Presets Settings button has no onClick handler and silently does nothing. |
| .superset/lib/setup/steps.sh | Adds NEXT_PUBLIC_RELAY_URL alongside existing RELAY_URL so the web app can reach the relay from the browser. |
| apps/web/src/types/svg.d.ts | Adds a module declaration for SVG imports, returning a string (URL), enabling preset icons to be imported. |
Sequence Diagram
sequenceDiagram
participant Page as WorkspaceTerminalPage
participant HC as host-client.ts
participant Relay as Relay / Host Service
participant Bar as WebTerminalPresetsBar
Page->>HC: listHostAgentConfigs(key) [parallel]
Page->>HC: listHostTerminals(key, workspaceId) [parallel]
HC->>Relay: "GET /hosts/{key}/trpc/settings.agentConfigs.list"
HC->>Relay: "GET /hosts/{key}/trpc/terminal.listSessions"
Relay-->>HC: HostAgentConfig[]
Relay-->>HC: HostTerminalSession[]
HC-->>Page: presets / terminals
Page->>Bar: render(presets, runningPresetId)
Bar->>Page: onRunPreset(preset)
Page->>HC: buildHostAgentLaunchCommand(preset)
HC-->>Page: shell string (env + argv)
Page->>HC: "createHostTerminal(key, workspaceId, {initialCommand})"
HC->>Relay: "POST /hosts/{key}/trpc/terminal.createSession"
Relay-->>HC: "{terminalId, status}"
HC-->>Page: created terminal
Page->>Page: setSelectedTerminalId(created.terminalId)
Page->>Bar: "runningPresetId = null"
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/web/src/app/workspaces/[workspaceId]/components/WebTerminalPresetsBar/WebTerminalPresetsBar.tsx:45-52
**Settings button is a no-op** — the "Manage presets" button renders with hover styling and an `aria-label`, giving users the impression it's actionable, but it has no `onClick` handler attached. Clicking it silently does nothing, which is confusing UX. Either wire it up to a handler or render it as `disabled` / remove it until the feature is ready.
### Issue 2 of 3
apps/web/src/trpc/host-client.ts:113-118
**Env variable keys are not shell-safe** — `envOverlayPrefix` interpolates the key directly into the shell assignment string (`${key}=…`) without quoting or validation. A key containing a space, `=`, or other shell metacharacter (e.g. `"FOO=BAR BAZ"`) would silently corrupt the generated command or, if the config data were ever untrusted, allow injection of additional assignments or arguments. Standard env-var names (letters, digits, underscore) are safe in practice, but an explicit validation (`/^[A-Za-z_][A-Za-z0-9_]*$/`) would make this defensive and prevent silent breakage from unexpected config values.
### Issue 3 of 3
apps/web/src/trpc/host-client.ts:120-129
**`promptArgs` and `promptTransport` are never used in the launch command** — `HostAgentConfig` exposes `promptTransport: "argv" | "stdin"` and `promptArgs: string[]`, but `buildHostAgentLaunchCommand` only concatenates `command` + `args` + `env`. If the host agent needs an initial prompt injected (e.g. via `promptArgs` appended when `promptTransport === "argv"`), the preset will launch the agent binary but not pass it any prompt, leaving it waiting for user input rather than executing a preset task.
Reviews (1): Last reviewed commit: "add web terminal presets" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <button | ||
| type="button" | ||
| title="Manage presets" | ||
| aria-label="Manage presets" | ||
| className="flex size-6 shrink-0 items-center justify-center rounded-md text-[#8f8983] transition-colors hover:bg-[#211d1b] hover:text-[#f3f0ed]" | ||
| > | ||
| <Settings className="size-4" /> | ||
| </button> |
There was a problem hiding this comment.
aria-label, giving users the impression it's actionable, but it has no onClick handler attached. Clicking it silently does nothing, which is confusing UX. Either wire it up to a handler or render it as disabled / remove it until the feature is ready.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/app/workspaces/[workspaceId]/components/WebTerminalPresetsBar/WebTerminalPresetsBar.tsx
Line: 45-52
Comment:
**Settings button is a no-op** — the "Manage presets" button renders with hover styling and an `aria-label`, giving users the impression it's actionable, but it has no `onClick` handler attached. Clicking it silently does nothing, which is confusing UX. Either wire it up to a handler or render it as `disabled` / remove it until the feature is ready.
How can I resolve this? If you propose a fix, please make it concise.| function envOverlayPrefix(env: Record<string, string>): string { | ||
| const assignments = Object.entries(env).map( | ||
| ([key, value]) => `${key}=${quoteSingleShell(value)}`, | ||
| ); | ||
| return assignments.length > 0 ? `${assignments.join(" ")} ` : ""; | ||
| } |
There was a problem hiding this comment.
envOverlayPrefix interpolates the key directly into the shell assignment string (${key}=…) without quoting or validation. A key containing a space, =, or other shell metacharacter (e.g. "FOO=BAR BAZ") would silently corrupt the generated command or, if the config data were ever untrusted, allow injection of additional assignments or arguments. Standard env-var names (letters, digits, underscore) are safe in practice, but an explicit validation (/^[A-Za-z_][A-Za-z0-9_]*$/) would make this defensive and prevent silent breakage from unexpected config values.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/trpc/host-client.ts
Line: 113-118
Comment:
**Env variable keys are not shell-safe** — `envOverlayPrefix` interpolates the key directly into the shell assignment string (`${key}=…`) without quoting or validation. A key containing a space, `=`, or other shell metacharacter (e.g. `"FOO=BAR BAZ"`) would silently corrupt the generated command or, if the config data were ever untrusted, allow injection of additional assignments or arguments. Standard env-var names (letters, digits, underscore) are safe in practice, but an explicit validation (`/^[A-Za-z_][A-Za-z0-9_]*$/`) would make this defensive and prevent silent breakage from unexpected config values.
How can I resolve this? If you propose a fix, please make it concise.| export function buildHostAgentLaunchCommand(config: { | ||
| command: string; | ||
| args: string[]; | ||
| env: Record<string, string>; | ||
| }) { | ||
| return `${envOverlayPrefix(config.env)}${buildArgvCommand([ | ||
| config.command, | ||
| ...config.args, | ||
| ])}`; | ||
| } |
There was a problem hiding this comment.
promptArgs and promptTransport are never used in the launch command — HostAgentConfig exposes promptTransport: "argv" | "stdin" and promptArgs: string[], but buildHostAgentLaunchCommand only concatenates command + args + env. If the host agent needs an initial prompt injected (e.g. via promptArgs appended when promptTransport === "argv"), the preset will launch the agent binary but not pass it any prompt, leaving it waiting for user input rather than executing a preset task. Is the intent for presets to only launch the agent binary (and let the user type the prompt), or should the preset also inject the promptArgs? If the latter, promptArgs/promptTransport need to be wired into buildHostAgentLaunchCommand.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/trpc/host-client.ts
Line: 120-129
Comment:
**`promptArgs` and `promptTransport` are never used in the launch command** — `HostAgentConfig` exposes `promptTransport: "argv" | "stdin"` and `promptArgs: string[]`, but `buildHostAgentLaunchCommand` only concatenates `command` + `args` + `env`. If the host agent needs an initial prompt injected (e.g. via `promptArgs` appended when `promptTransport === "argv"`), the preset will launch the agent binary but not pass it any prompt, leaving it waiting for user input rather than executing a preset task. Is the intent for presets to only launch the agent binary (and let the user type the prompt), or should the preset also inject the promptArgs? If the latter, promptArgs/promptTransport need to be wired into buildHostAgentLaunchCommand.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
Validation
Summary by cubic
Adds a web terminal presets bar powered by host agent configs, letting users launch a preconfigured terminal in one click. Also enables browser host-service calls via
NEXT_PUBLIC_RELAY_URLso the relay port is used correctly.New Features
settings.agentConfigs.listto fetch presets and a helper to build the launch command; supports GET requests without input.svg.d.tsto allow importing preset SVGs.Migration
NEXT_PUBLIC_RELAY_URLto the environment and restart the web app.Written for commit f9d0e16. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes