Add preset-backed workspace run#4335
Merged
Merged
Conversation
Contributor
|
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 (5)
📝 WalkthroughWalkthroughWorkspace run definitions now resolve from terminal presets (flagged by ChangesWorkspace Run Definition System
Host Terminal Runtime
V2 Dashboard Runtime
Preset & Config UI
V1 & Cache Wiring
Tests & Documentation
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR wires up a unified workspace run system across v1 and v2 workspaces, backed by either a project config
Confidence Score: 3/5The v2 run hook has two issues that could manifest in real use: broad error string matching could silently swallow non-terminal errors and mark a running process as stopped, and the working directory from presets/config is never forwarded to the terminal launcher, diverging from v1 behavior. The core resolver logic and schema changes are solid, but useV2WorkspaceRun.ts deserves the most scrutiny — the error matching logic and the cwd omission are in the hot path for every stop and start operation.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts | New v2 workspace run hook — contains overly broad error matching in isTerminalGoneError, silently drops definition.cwd when launching, and loses Force Stop capability after optimistic Ctrl-C stop. |
| apps/desktop/src/shared/workspace-run-definition.ts | New shared resolver for workspace run definitions — clean three-level priority (project-targeted preset → config → global preset) with well-typed discriminated union return. |
| packages/host-service/src/terminal/terminal.ts | Adds writeInputToSession for Ctrl-C support and fixes a timestamp inconsistency in the terminal exit handler — both changes are correct. |
| packages/host-service/src/trpc/router/config/config.ts | Makes setup/teardown optional in updateConfig and adds run support; adds getWorkspaceRunDefinition query — backwards compatible and well-tested. |
| apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts | Refactors getResolvedRunCommands to use shared resolver and adds getWorkspaceRunDefinition endpoint — logic is correct, though getResolvedRunCommands now silently expands to include preset commands. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/V2WorkspaceRunButton.tsx | New v2 run button component — mirrors v1 structure with correct disabled/pending states and configure routing based on definition source. |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts | Adds workspaceRunTerminals map to workspace local state schema with full lifecycle state enum — well-structured with proper Zod defaults and heal logic. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/useWorkspaceRunCommand.ts | Updates v1 hook to use new getWorkspaceRunDefinition endpoint and correctly passes runDefinition?.cwd as initialCwd — parity gap with v2 noted. |
| packages/host-service/src/trpc/router/terminal/terminal.ts | Adds writeInput mutation that delegates to writeInputToSession and converts errors to TRPC NOT_FOUND — straightforward and safe. |
| packages/local-db/src/schema/zod.ts | Adds optional useAsWorkspaceRun field to the terminal preset schema — non-breaking additive change. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Run Button Clicked] --> B{v1 or v2?}
B -->|v1| C[electronTrpc.workspaces\n.getWorkspaceRunDefinition]
B -->|v2| D[workspaceTrpc.config\n.getWorkspaceRunDefinition\n+ client presets]
C --> E[selectWorkspaceRunDefinition\nserver-side]
D --> F[selectWorkspaceRunDefinition\nclient-side]
E --> G{Priority Resolution}
F --> G
G -->|1 - project-targeted preset| H[Terminal Preset\nWorkspaceRunDefinition]
G -->|2 - config run| I[Project Config\nWorkspaceRunDefinition]
G -->|3 - global preset| J[Global Preset\nWorkspaceRunDefinition]
G -->|4 - none| K[Toast: No run command]
H --> L[Build command]
I --> L
J --> L
L -->|v1| M[addTab + launchWorkspaceRunInPane\npane metadata tracking]
L -->|v2| N[launcher.create\nnew tab + pane\nworkspaceRunTerminals state]
N --> O{Stop requested?}
O -->|Ctrl-C| P[writeInput TRPC\nmark stopped-by-user]
O -->|Force Stop| Q[killSession TRPC\nmark stopped-by-user]
R[terminal:lifecycle exit event] --> S[updateWorkspaceRunTerminals\nmark stopped-by-exit]
Comments Outside Diff (4)
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts, line 668-676 (link)isTerminalGoneErroroverly broad "not found" matchmessage.includes("not found")will match any TRPC 404 or networking error whose message contains that substring, not just terminal-specific errors. For example, a transient "Resource not found" proxy error or a future endpoint renaming could satisfy this check, silently eating the exception and marking the run asstopped-by-usereven though the terminal process is still alive.The actual errors returned by
writeInputToSessionare:"Terminal session not found","Terminal session does not belong to this workspace", and"Terminal session has exited". The second one does NOT contain "not found", so it would fall through and show a toast instead. The pattern is both over-broad (catches unrelated errors) and under-specific (misses the workspace-ownership error). The"not alive"branch also has no matching error string in the currentwriteInputToSessionimplementation.Consider checking the TRPC error code instead of the message, or matching only the exact strings produced by
writeInputToSession.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts Line: 668-676 Comment: **`isTerminalGoneError` overly broad "not found" match** `message.includes("not found")` will match any TRPC 404 or networking error whose message contains that substring, not just terminal-specific errors. For example, a transient "Resource not found" proxy error or a future endpoint renaming could satisfy this check, silently eating the exception and marking the run as `stopped-by-user` even though the terminal process is still alive. The actual errors returned by `writeInputToSession` are: `"Terminal session not found"`, `"Terminal session does not belong to this workspace"`, and `"Terminal session has exited"`. The second one does NOT contain "not found", so it would fall through and show a toast instead. The pattern is both over-broad (catches unrelated errors) and under-specific (misses the workspace-ownership error). The `"not alive"` branch also has no matching error string in the current `writeInputToSession` implementation. Consider checking the TRPC error code instead of the message, or matching only the exact strings produced by `writeInputToSession`. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts, line 793-833 (link)definition.cwdsilently dropped on v2 startstartWorkspaceRuncallslauncher.create({ command })but never passesdefinition?.cwd. BothWorkspaceRunDefinitionsources —project-config(viaconfigRunToWorkspaceRun) andterminal-preset(viapresetToWorkspaceRun) — carry an optionalcwdfield that will be ignored here.The v1 counterpart in
useWorkspaceRunCommand.tsexplicitly usesrunDefinition?.cwdasinitialCwdbefore creating the tab. Any preset or config that sets acwdwill behave correctly in v1 but silently start in the default directory in v2, creating a hard-to-debug discrepancy.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts Line: 793-833 Comment: **`definition.cwd` silently dropped on v2 start** `startWorkspaceRun` calls `launcher.create({ command })` but never passes `definition?.cwd`. Both `WorkspaceRunDefinition` sources — `project-config` (via `configRunToWorkspaceRun`) and `terminal-preset` (via `presetToWorkspaceRun`) — carry an optional `cwd` field that will be ignored here. The v1 counterpart in `useWorkspaceRunCommand.ts` explicitly uses `runDefinition?.cwd` as `initialCwd` before creating the tab. Any preset or config that sets a `cwd` will behave correctly in v1 but silently start in the default directory in v2, creating a hard-to-debug discrepancy. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts, line 835-879 (link)Force Stop disappears after optimistic Ctrl-C
stopWorkspaceRunimmediately marks the terminal state asstopped-by-userafter writing the Ctrl-C byte, before the process actually exits. BecausecanForceStop = Boolean(runningState)andrunningStateonly includes entries withstate === "running", the "Force Stop" menu item vanishes the moment the Ctrl-C is written.If the process ignores SIGINT (e.g., a process that traps or blocks Ctrl-C), the terminal is still running but the UI shows it as stopped and the "Force Stop" option is gone. The user would have to navigate to the terminal pane manually to kill it. Consider keeping a brief intermediate
"stopping"state (or leavingcanForceStopavailable for a terminal that is still alive) until anexitevent confirms the process actually exited.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts Line: 835-879 Comment: **Force Stop disappears after optimistic Ctrl-C** `stopWorkspaceRun` immediately marks the terminal state as `stopped-by-user` after writing the Ctrl-C byte, before the process actually exits. Because `canForceStop = Boolean(runningState)` and `runningState` only includes entries with `state === "running"`, the "Force Stop" menu item vanishes the moment the Ctrl-C is written. If the process ignores SIGINT (e.g., a process that traps or blocks Ctrl-C), the terminal is still running but the UI shows it as stopped and the "Force Stop" option is gone. The user would have to navigate to the terminal pane manually to kill it. Consider keeping a brief intermediate `"stopping"` state (or leaving `canForceStop` available for a terminal that is still alive) until an `exit` event confirms the process actually exited. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts, line 752-772 (link)configRunDefinition.cwdalso dropped when building client-side definitionselectWorkspaceRunDefinitionis called withconfigRunCommands: configRunDefinition?.commandsbut thecwdcarried by the config definition is not forwarded asconfigCwd. Even if the host-service starts returningcwdfor config-backed runs, the v2 flow would drop it. Since v1 (useWorkspaceRunCommand.ts) usesrunDefinition?.cwd, this is a persistent parity gap.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts Line: 752-772 Comment: **`configRunDefinition.cwd` also dropped when building client-side definition** `selectWorkspaceRunDefinition` is called with `configRunCommands: configRunDefinition?.commands` but the `cwd` carried by the config definition is not forwarded as `configCwd`. Even if the host-service starts returning `cwd` for config-backed runs, the v2 flow would drop it. Since v1 (`useWorkspaceRunCommand.ts`) uses `runDefinition?.cwd`, this is a persistent parity gap. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:668-676
**`isTerminalGoneError` overly broad "not found" match**
`message.includes("not found")` will match any TRPC 404 or networking error whose message contains that substring, not just terminal-specific errors. For example, a transient "Resource not found" proxy error or a future endpoint renaming could satisfy this check, silently eating the exception and marking the run as `stopped-by-user` even though the terminal process is still alive.
The actual errors returned by `writeInputToSession` are: `"Terminal session not found"`, `"Terminal session does not belong to this workspace"`, and `"Terminal session has exited"`. The second one does NOT contain "not found", so it would fall through and show a toast instead. The pattern is both over-broad (catches unrelated errors) and under-specific (misses the workspace-ownership error). The `"not alive"` branch also has no matching error string in the current `writeInputToSession` implementation.
Consider checking the TRPC error code instead of the message, or matching only the exact strings produced by `writeInputToSession`.
### Issue 2 of 4
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:793-833
**`definition.cwd` silently dropped on v2 start**
`startWorkspaceRun` calls `launcher.create({ command })` but never passes `definition?.cwd`. Both `WorkspaceRunDefinition` sources — `project-config` (via `configRunToWorkspaceRun`) and `terminal-preset` (via `presetToWorkspaceRun`) — carry an optional `cwd` field that will be ignored here.
The v1 counterpart in `useWorkspaceRunCommand.ts` explicitly uses `runDefinition?.cwd` as `initialCwd` before creating the tab. Any preset or config that sets a `cwd` will behave correctly in v1 but silently start in the default directory in v2, creating a hard-to-debug discrepancy.
### Issue 3 of 4
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:835-879
**Force Stop disappears after optimistic Ctrl-C**
`stopWorkspaceRun` immediately marks the terminal state as `stopped-by-user` after writing the Ctrl-C byte, before the process actually exits. Because `canForceStop = Boolean(runningState)` and `runningState` only includes entries with `state === "running"`, the "Force Stop" menu item vanishes the moment the Ctrl-C is written.
If the process ignores SIGINT (e.g., a process that traps or blocks Ctrl-C), the terminal is still running but the UI shows it as stopped and the "Force Stop" option is gone. The user would have to navigate to the terminal pane manually to kill it. Consider keeping a brief intermediate `"stopping"` state (or leaving `canForceStop` available for a terminal that is still alive) until an `exit` event confirms the process actually exited.
### Issue 4 of 4
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:752-772
**`configRunDefinition.cwd` also dropped when building client-side definition**
`selectWorkspaceRunDefinition` is called with `configRunCommands: configRunDefinition?.commands` but the `cwd` carried by the config definition is not forwarded as `configCwd`. Even if the host-service starts returning `cwd` for config-backed runs, the v2 flow would drop it. Since v1 (`useWorkspaceRunCommand.ts`) uses `runDefinition?.cwd`, this is a persistent parity gap.
Reviews (1): Last reviewed commit: "Add preset-backed workspace run" | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/lib/trpc/routers/settings/index.ts (1)
410-420:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep
useAsWorkspaceRununique per resolution scope.Both create and update can leave multiple presets marked as workspace-run candidates. The resolver then picks the first matching preset by stored order, so turning a later preset on can be a no-op from the user's perspective. Please clear conflicting flags here, or reject the write when another preset in the same project/global scope is already enabled.
Also applies to: 441-468
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/lib/trpc/routers/settings/index.ts` around lines 410 - 420, When a preset is created or updated with useAsWorkspaceRun=true, ensure uniqueness per resolution scope by finding other TerminalPreset entries that share the same scope (use normalizePresetProjectIds(input.projectIds) to determine project/global scope) and either (a) clear their useAsWorkspaceRun flags before saving this preset or (b) reject the write with a clear error; implement this logic inside the create mutation (the mutation building TerminalPreset with crypto.randomUUID) and the corresponding update mutation (the one around lines 441-468) so that only one preset per scope can have useAsWorkspaceRun=true.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/config/config.ts (1)
137-139: ⚡ Quick winNormalize
runcommands before returning them.Line 137-139 currently filters by
trim()but returns untrimmed strings. Trimming here prevents whitespace-only drift from leaking into UI/execution paths.Suggested patch
- const commands = (config?.run ?? []).filter( - (command) => command.trim().length > 0, - ); + const commands = (config?.run ?? []) + .map((command) => command.trim()) + .filter((command) => command.length > 0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/config/config.ts` around lines 137 - 139, The current construction creates `commands` from `config?.run` but only filters by trimmed length and returns untrimmed strings; change the creation of `commands` (the variable in this module) to first normalize each entry by trimming whitespace (e.g., map each run string to its trimmed form) and then filter out empty strings, so that the `commands` array contains no leading/trailing whitespace and no blank entries when returned from the router function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts`:
- Around line 70-80: The call to selectWorkspaceRunDefinition is dropping the
configured run working directory; update the call site after loadSetupConfig so
you pass the configured cwd into selectWorkspaceRunDefinition via its configCwd
parameter (e.g. use config?.run?.cwd or the project-level cwd from
loadSetupConfig) in addition to the existing configRunCommands, keeping the
existing presets (getTerminalPresetsForWorkspaceRun()) and projectId arguments;
locate this in the code around loadSetupConfig and selectWorkspaceRunDefinition
to add the configCwd argument.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WorkspaceRunButton/WorkspaceRunButton.tsx`:
- Around line 75-90: handleConfigureClick currently returns early if projectId
is falsy, preventing the terminal-preset branch from navigating to the preset
editor; change the logic so the early return happens only for the project
settings path. Specifically, in handleConfigureClick inspect
runDefinition?.source first and, if it's "terminal-preset", call navigate to
"/settings/terminal" with search { editPresetId: runDefinition.presetId }
regardless of projectId; otherwise (non-preset)
setSettingsSearchQuery("scripts") and then require projectId before calling
navigate to "/settings/projects/$projectId" with params { projectId } so
projectId is only enforced for the project-config route.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:
- Around line 147-178: startWorkspaceRun currently creates a terminal with
launcher.create({ command }) but never passes the resolved working directory
from the selected definition; update startWorkspaceRun to include the
definition's resolved cwd when creating the terminal (e.g. call
launcher.create({ command, cwd: definition.cwd || resolvedCwd }) so
preset-backed runs and project-config cwd are honored). Locate startWorkspaceRun
and the launcher.create call and thread the appropriate cwd value from the
definition (or the upstream resolved workspace cwd) into the create options;
keep existing behavior if cwd is undefined.
- Around line 106-126: The current call to selectWorkspaceRunDefinition only
forwards configRunDefinition?.commands and drops the configured working
directory; update the call to pass the config's working directory as well (e.g.
include configRunDefinition?.cwd or the run-directory field returned by the
host-service) so selectWorkspaceRunDefinition receives both configRunCommands
and the config cwd. Locate the invocation of selectWorkspaceRunDefinition (using
symbols selectWorkspaceRunDefinition, configRunDefinition,
resolvedMatchedPresets, projectId) and add the configRunDefinition's
cwd/run-directory property to the argument object so the v2 run definition
preserves the configured working directory.
---
Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/settings/index.ts`:
- Around line 410-420: When a preset is created or updated with
useAsWorkspaceRun=true, ensure uniqueness per resolution scope by finding other
TerminalPreset entries that share the same scope (use
normalizePresetProjectIds(input.projectIds) to determine project/global scope)
and either (a) clear their useAsWorkspaceRun flags before saving this preset or
(b) reject the write with a clear error; implement this logic inside the create
mutation (the mutation building TerminalPreset with crypto.randomUUID) and the
corresponding update mutation (the one around lines 441-468) so that only one
preset per scope can have useAsWorkspaceRun=true.
---
Nitpick comments:
In `@packages/host-service/src/trpc/router/config/config.ts`:
- Around line 137-139: The current construction creates `commands` from
`config?.run` but only filters by trimmed length and returns untrimmed strings;
change the creation of `commands` (the variable in this module) to first
normalize each entry by trimming whitespace (e.g., map each run string to its
trimmed form) and then filter out empty strings, so that the `commands` array
contains no leading/trailing whitespace and no blank entries when returned from
the router function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99745e8e-b838-4ada-8c90-17d197f5efcf
📒 Files selected for processing (34)
apps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/lib/project-scripts.tsapps/desktop/src/renderer/react-query/presets/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WorkspaceRunButton/WorkspaceRunButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2PresetsBar/V2PresetsBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/V2WorkspaceRunButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/useWorkspaceRunCommand.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.tsapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/PresetsSection.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/V2ProjectSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/components/V2ScriptsEditor/V2ScriptsEditor.tsxapps/desktop/src/shared/workspace-run-definition.test.tsapps/desktop/src/shared/workspace-run-definition.tspackages/host-service/src/runtime/setup/config.test.tspackages/host-service/src/terminal/terminal.tspackages/host-service/src/trpc/router/config/config.test.tspackages/host-service/src/trpc/router/config/config.tspackages/host-service/src/trpc/router/terminal/terminal.tspackages/local-db/src/schema/zod.tsplans/done/20260509-run-script-presets-design.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 34 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts:199">
P1: Don’t mark the run as stopped immediately after writing Ctrl-C; wait for terminal exit (or terminal-gone error) to avoid false stopped state and duplicate concurrent runs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| workspaceId, | ||
| data: CTRL_C_INPUT, | ||
| }); | ||
| const stoppedAt = Date.now(); |
Contributor
There was a problem hiding this comment.
P1: Don’t mark the run as stopped immediately after writing Ctrl-C; wait for terminal exit (or terminal-gone error) to avoid false stopped state and duplicate concurrent runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts, line 199:
<comment>Don’t mark the run as stopped immediately after writing Ctrl-C; wait for terminal exit (or terminal-gone error) to avoid false stopped state and duplicate concurrent runs.</comment>
<file context>
@@ -0,0 +1,306 @@
+ workspaceId,
+ data: CTRL_C_INPUT,
+ });
+ const stoppedAt = Date.now();
+ updateWorkspaceRunTerminals((states) => {
+ const state = states[runningState.terminalId];
</file context>
Kitenite
force-pushed
the
run-script-presets-design
branch
from
2dd387b to
599839a
Compare
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/shared/workspace-run-definition.test.ts (1)
75-75: 💤 Low valueConsider consistent representation of global presets.
Test 2 (line 37) explicitly sets
projectIds: nullfor global presets, while this test omits the field. Both likely work, but using a consistent representation (preferably explicitprojectIds: null) would improve readability.♻️ Suggested consistency fix
{ id: "preset-global", name: "Global dev", commands: ["npm run dev"], + projectIds: null, useAsWorkspaceRun: true, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/shared/workspace-run-definition.test.ts` at line 75, The test omits the explicit projectIds field for the global preset object (it currently has useAsWorkspaceRun: true); add projectIds: null to that object to match the other test's representation of global presets for consistency—locate the object literal in workspace-run-definition.test.ts where useAsWorkspaceRun: true is set and add projectIds: null alongside it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/host-service/src/runtime/setup/config.ts`:
- Around line 59-67: The cwd validation currently only checks type but allows
empty or whitespace-only strings; update the check in the block that inspects
obj.cwd to reject strings where obj.cwd.trim().length === 0 (use the existing
nonEmptyStrings behavior as reference), log the same error when
blank/whitespace-only, and assign a trimmed value to result.cwd (e.g.,
result.cwd = obj.cwd.trim()) so stored cwd is non-empty and normalized.
---
Nitpick comments:
In `@apps/desktop/src/shared/workspace-run-definition.test.ts`:
- Line 75: The test omits the explicit projectIds field for the global preset
object (it currently has useAsWorkspaceRun: true); add projectIds: null to that
object to match the other test's representation of global presets for
consistency—locate the object literal in workspace-run-definition.test.ts where
useAsWorkspaceRun: true is set and add projectIds: null alongside it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 417ac689-3433-492d-8157-75e85ef9affb
📒 Files selected for processing (38)
apps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/setup.tsapps/desktop/src/renderer/lib/project-scripts.tsapps/desktop/src/renderer/react-query/presets/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WorkspaceRunButton/WorkspaceRunButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2PresetsBar/V2PresetsBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/V2WorkspaceRunButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2TerminalLauncher/useV2TerminalLauncher.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/useWorkspaceRunCommand.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.tsapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/PresetsSection.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsxapps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/V2ProjectSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/components/V2ScriptsEditor/V2ScriptsEditor.tsxapps/desktop/src/shared/types/config.tsapps/desktop/src/shared/workspace-run-definition.test.tsapps/desktop/src/shared/workspace-run-definition.tspackages/host-service/src/runtime/setup/config.test.tspackages/host-service/src/runtime/setup/config.tspackages/host-service/src/terminal/terminal.tspackages/host-service/src/trpc/router/config/config.test.tspackages/host-service/src/trpc/router/config/config.tspackages/host-service/src/trpc/router/terminal/terminal.tspackages/local-db/src/schema/zod.tsplans/done/20260509-run-script-presets-design.md
✅ Files skipped from review due to trivial changes (9)
- apps/desktop/src/shared/types/config.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/index.ts
- packages/local-db/src/schema/zod.ts
- packages/host-service/src/runtime/setup/config.test.ts
- apps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/V2ProjectSettings.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/index.ts
- packages/host-service/src/trpc/router/config/config.test.ts
- plans/done/20260509-run-script-presets-design.md
- apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2PresetsBar/V2PresetsBar.tsx
- apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
- apps/desktop/src/renderer/lib/project-scripts.ts
- packages/host-service/src/trpc/router/terminal/terminal.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx
- apps/desktop/src/lib/trpc/routers/settings/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
- packages/host-service/src/trpc/router/config/config.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WorkspaceRunButton/WorkspaceRunButton.tsx
- apps/desktop/src/renderer/react-query/presets/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/useWorkspaceRunCommand.ts
- apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx
- apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/components/PresetEditorDialog/PresetEditorDialog.tsx
- apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetsSection/PresetsSection.tsx
- apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/V2PresetsSection/V2PresetsSection.tsx
- apps/desktop/src/shared/workspace-run-definition.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
- apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.ts
- apps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/components/V2ScriptsEditor/V2ScriptsEditor.tsx
- apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2WorkspaceRunButton/V2WorkspaceRunButton.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspaceRun/useV2WorkspaceRun.ts
- apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts
Move V2WorkspaceRunButton from the leading slot of V2PresetsBar to the trailing end via ml-auto, and refine the bar, preset items, and run button toward Linear's flatter, lower-chroma aesthetic — softer borders, muted defaults, fill-on-hover.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
runscripts and terminal presets marked as workspace runValidation
bun run lint:fixbun run lintbun run typecheckinapps/desktopbun run typecheckinpackages/host-servicebun test apps/desktop/src/shared/workspace-run-definition.test.ts apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.ts apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts packages/host-service/src/trpc/router/config/config.test.tsSummary by cubic
Adds a shared resolver for the workspace Run button that selects a run command from project config or a preset marked “Use as workspace run,” with validated
cwdsupport. Updates v1 and v2 to start runs in a fresh terminal, handle Ctrl‑C and Force Stop, deep‑link to the right editor (preset or project), and place the v2 Run button at the end of the presets bar with a muted style.New Features
run› global preset; ignores empty commands; carriescwd. Implemented inshared/workspace-run-definitionwith tests.config.updateConfigaccepts optionalsetup/teardown/runand preserves omitted keys; newconfig.getWorkspaceRunDefinition.terminal.writeInputsends Ctrl‑C. Terminal session creation acceptscwd(absolute or worktree‑relative), and exit events include precise timestamps.workspaces.getWorkspaceRunDefinition;getResolvedRunCommandsnow uses it. Query invalidations added for preset and script changes. Local workspace state storesworkspaceRunTerminalskeyed byterminalId.Migration
runscript (and optional absolute or worktree‑relativecwd) in.superset/config.json. No breaking changes.Written for commit d8bd4bd. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation