feat(host-service): host-side agent launches in workspace.create (PR4)#3990
Open
Kitenite wants to merge 4 commits into
Open
feat(host-service): host-side agent launches in workspace.create (PR4)#3990Kitenite wants to merge 4 commits into
Kitenite wants to merge 4 commits into
Conversation
…a workspace package Moves the launch-context logic (buildLaunchSpec, composer, contributors, types) out of the desktop app's shared/ dir into a new @superset/launch-context package so host-service can consume it for the upcoming canonical workspace.create() flow (PR 4). Pure file move + import path update — no behavior change. 54 tests move with the source. Single consumer in the desktop app (buildForkAgentLaunch.ts) updated to the new package path. Adds plans/20260502-workspace-create-pr4.md documenting the broader PR 4 scope this extraction enables.
V1's mastracode used `argv` transport with `--prompt` and a `; mastracode` REPL-re-entry suffix. The new flow pipes the prompt to mastracode's stdin (`prompt | mastracode`), matching how amp already works. No `promptCommand` / suffix needed. This commit also locks in the design choice to NOT add per-preset template columns to `host_agent_configs`. PR4's host-side launch builder will synthesize a `ResolvedAgentConfig` inline using the default template constants from `@superset/shared`. Customization per preset can land in a separate storage flow when a product use case motivates it. - AGENT_PRESETS: mastracode promptTransport "argv" → "stdin", promptArgs ["--prompt"] → [] - Test: mastracode round-trips as a stdin agent - Plan: PR4 step 3 + decision 5 reflect the no-new-columns scope
Host-side port of the renderer's buildForkAgentLaunch flow, prepping for PR4's create-flow integration (#26). Terminal-only this PR; chat slice deferred to #29. Modules under .../workspace-creation/shared/launches/: - synth-agent-config: adapter from PR1 host preset row → minimal TerminalResolvedAgentConfig that buildLaunchSpec needs. Templates come from @superset/shared constants (no per-preset DB columns). - build-agent-launch: orchestrates buildLaunchContext + buildLaunchSpec and composes a TerminalLaunchPlan from PR1's argv-array model (`[command, ...args, ...promptArgs, ...(transport === "argv" ? [prompt] : [])]`). Stdin-transport prompts surface as `stdinPrompt`. No V1 escape hatches. - start-terminal-launch: thin wrapper over createTerminalSessionInternal. POSIX-quotes the argv into initialCommand. Stdin transport prepends `printf '%s' '<prompt>' | ` so the prompt reaches the spawned process via stdin. - write-attachments: writes pre-resolved AttachmentFile bytes to <worktree>/.superset/attachments/. Also adds readAttachment to attachments/storage.ts so the wiring layer (#26) can resolve attachmentIds → AttachmentFile[] before calling buildAgentLaunch. Tests for synth-agent-config, build-agent-launch, write-attachments; start-terminal-launch is covered by the create-flow integration test in #28.
Adds optional `composer.agentId` + `linkedContext.attachmentIds` to
the create input. When agentId is set, host-service:
1. Resolves the host preset row (auto-materializing on first use for
non-default builtins like mastracode).
2. Reads attachment bytes from the host attachment store.
3. Builds a host-side ResolveCtx that fetches issue/PR bodies via
the gh-CLI (cached) and internal task content via ctx.api.task.byId.
4. Calls buildAgentLaunch → writeAttachmentsToWorktree → startTerminalLaunch.
5. Pushes { kind: "terminal", terminalId, label } onto the new
`launches[]` field on the response.
Launch failures soft-fail: the workspace is already created and
shouldn't unwind, so errors surface as warnings.
Refactors the gh-CLI fetch logic out of the tRPC procedures into a
shared `github-content.ts` module so the launch path and the
existing tRPC procedures share the 30s PR cache.
Adds:
- findOrCreateHostPresetByPresetId helper on agent-configs (3 tests)
- launches/host-resolve-ctx.ts (host-side ResolveCtx factory)
- launches/resolve-attachment-files.ts (3 tests)
- LaunchDescriptor on workspace-creation/shared/types.ts
Contributor
📝 WalkthroughWalkthroughNew ChangesAgent Launch During Workspace Creation
Sequence DiagramsequenceDiagram
actor Client
participant Host Service
participant GitHub API
participant Attachment Store
participant Terminal
Client->>Host Service: workspace.create({ composer: { agentId }, attachmentIds, ... })
Host Service->>Host Service: Create workspace, register
alt composer.agentId provided
Host Service->>Host Service: findOrCreateHostPresetByPresetId(agentId)
Host Service->>Attachment Store: resolveAttachmentFiles(attachmentIds)
Attachment Store-->>Host Service: [AttachmentFile]
Host Service->>GitHub API: fetchGithubIssueContent(issueNumber)
GitHub API-->>Host Service: IssueContent
Host Service->>GitHub API: fetchGithubPullRequestContent(prNumber)
GitHub API-->>Host Service: PullRequestContent
Host Service->>Host Service: buildAgentLaunch(preset, prompt, issues, attachments)
Host Service->>Host Service: buildLaunchContext() + buildLaunchSpec()
Host Service->>Attachment Store: writeAttachmentsToWorktree(worktreePath, attachments)
Host Service->>Terminal: startTerminalLaunch(spawn args, stdinPrompt)
Terminal-->>Host Service: { terminalId, label }
Host Service->>Host Service: Append launch to launches[]
else Error in agent launch
Host Service->>Host Service: Append warning, continue
end
Host Service-->>Client: { workspace, launches, terminals, warnings }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. Review rate limit: 6/8 reviews remaining, refill in 13 minutes and 34 seconds.Comment |
Greptile SummaryThis PR moves the All three P2 findings are non-blocking: Confidence Score: 4/5Safe to merge — all findings are P2; soft-fail semantics ensure workspace creation is never broken by launch failures. No P0/P1 issues found. The soft-fail design means a launch failure produces warnings but never unwinds the already-created workspace. Three P2 issues (signal propagation, missing issue cache, silent env drop) cap the score at 4. packages/host-service/src/trpc/router/workspace-creation/shared/launches/host-resolve-ctx.ts and packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts merit a follow-up look before the renderer integration lands in #27.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/workspace-creation/procedures/create.ts | Adds the runAgentLaunch helper and wires it into the create flow behind composer.agentId; soft-fail semantics look correct; launches[] returned on response. |
| packages/host-service/src/trpc/router/workspace-creation/shared/launches/build-agent-launch.ts | New module: builds a TerminalLaunchPlan from preset row + launch context; assignFilenamesAndCollect and nextUniqueName collision logic are correct; stdinPrompt/argv branching matches test expectations. |
| packages/host-service/src/trpc/router/workspace-creation/shared/launches/host-resolve-ctx.ts | New module: builds a ResolveCtx backed by gh CLI + cloud API; signal stored in returned object but never forwarded to outbound async calls (P2). |
| packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts | Extracts issue/PR fetchers into a shared module; PR content cache migrated correctly; issue fetch has no cache (pre-existing, but the PR description implies shared caching); resolveGithubRepo awaited before cache lookup on every call. |
| packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts | New module: composes shell command from plan.spawn argv using POSIX shellQuote, handles stdin transport via `printf '%s' ... |
| packages/host-service/src/trpc/router/settings/agent-configs.ts | Adds findOrCreateHostPresetByPresetId — synchronous, correctly seeds defaults and materializes non-default builtins on demand; well-tested. |
| packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts | Writes pre-sanitized attachment bytes to <worktree>/.superset/attachments/; filenames already collision-safe from assignFilenamesAndCollect; no path traversal risk. |
| packages/host-service/src/trpc/router/settings/agent-presets.ts | Switches mastracode preset from argv/--prompt to stdin/[] transport — intentional change matching amp behavior. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts | Import paths updated from shared/context/... to @superset/launch-context; no logic changes. |
| packages/launch-context/src/index.ts | New package entry point re-exporting the moved buildLaunchContext, buildLaunchSpec, defaultContributorRegistry, and related types. |
Sequence Diagram
sequenceDiagram
participant Renderer
participant CreateProc as "create.ts (host-service)"
participant FindPreset as "findOrCreateHostPresetByPresetId"
participant ResolveFiles as "resolveAttachmentFiles"
participant BuildLaunch as "buildAgentLaunch"
participant GhContent as "fetchGithubIssueContent / fetchGithubPullRequestContent"
participant WriteAttach as "writeAttachmentsToWorktree"
participant StartTerm as "startTerminalLaunch"
Renderer->>CreateProc: "workspaceCreation.create(input)"
CreateProc->>CreateProc: "git clone / branch setup"
CreateProc->>FindPreset: "findOrCreateHostPresetByPresetId(db, agentId)"
FindPreset-->>CreateProc: "presetRow | null"
CreateProc->>ResolveFiles: "resolveAttachmentFiles(attachmentIds)"
ResolveFiles-->>CreateProc: "AttachmentFile[]"
CreateProc->>BuildLaunch: "buildAgentLaunch(preset, sources, resolveCtx)"
BuildLaunch->>GhContent: "fetchIssue / fetchPullRequest (via ResolveCtx)"
GhContent-->>BuildLaunch: "IssueContent / PullRequestContent"
BuildLaunch-->>CreateProc: "TerminalLaunchPlan | null"
CreateProc->>WriteAttach: "writeAttachmentsToWorktree(worktreePath, plan.attachmentsToWrite)"
CreateProc->>StartTerm: "startTerminalLaunch(ctx, workspaceId, plan)"
StartTerm-->>CreateProc: "{ terminalId, label } | { error }"
CreateProc-->>Renderer: "{ workspace, terminals, launches[], warnings[] }"
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
packages/host-service/src/trpc/router/workspace-creation/shared/launches/host-resolve-ctx.ts:22-30
**`signal` captured but never forwarded to async operations**
The `signal` from the caller (or the freshly-created `AbortController().signal`) is stored in the returned `ResolveCtx` object, but it is never passed to `fetchGithubIssueContent`, `fetchGithubPullRequestContent`, or `ctx.api.task.byId.query`. If workspace creation is cancelled mid-flight, these outbound `gh` CLI invocations and cloud-API calls will continue running until they complete on their own, making the signal a no-op.
### Issue 2 of 3
packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts:52-68
**`fetchGithubIssueContent` has no cache; `resolveGithubRepo` called on every PR cache hit**
Two minor inefficiencies introduced by this extraction:
1. `fetchGithubIssueContent` has no cache at all. The PR description says "shared 30s cache with existing tRPC procedures" but only `fetchGithubPullRequestContent` got the cache. Issue content is now fetched via the same `gh issue view` on every call from both the tRPC procedure and the launch builder.
2. `fetchGithubPullRequestContent` awaits `resolveGithubRepo` **before** the cache lookup (line 74), so even a cache hit still incurs a `resolveGithubRepo` round-trip. The old per-procedure cache had the same issue, but it is now in a shared helper called from more sites.
Neither is a correctness bug, but issue fetch is missing the caching mentioned in the PR description.
### Issue 3 of 3
packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts:26-30
**`plan.spawn.env` silently dropped with no runtime warning**
The comment documents that env vars are currently ignored, but if a preset row has a non-empty `env` map and someone adds it in future, there's no log or assertion to surface the omission. A guard like `if (Object.keys(plan.spawn.env).length > 0) console.warn(...)` would make the gap visible without requiring a code change to add env support.
Reviews (1): Last reviewed commit: "feat(host-service): wire agent launches ..." | Re-trigger Greptile
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+22
to
+30
| signal?: AbortSignal; | ||
| githubIssueUrls: string[]; | ||
| linkedPrUrl?: string; | ||
| }): ResolveCtx { | ||
| const { ctx, projectId, githubIssueUrls, linkedPrUrl } = input; | ||
| const signal = input.signal ?? new AbortController().signal; | ||
|
|
||
| return { | ||
| projectId, |
There was a problem hiding this comment.
signal captured but never forwarded to async operations
The signal from the caller (or the freshly-created AbortController().signal) is stored in the returned ResolveCtx object, but it is never passed to fetchGithubIssueContent, fetchGithubPullRequestContent, or ctx.api.task.byId.query. If workspace creation is cancelled mid-flight, these outbound gh CLI invocations and cloud-API calls will continue running until they complete on their own, making the signal a no-op.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/shared/launches/host-resolve-ctx.ts
Line: 22-30
Comment:
**`signal` captured but never forwarded to async operations**
The `signal` from the caller (or the freshly-created `AbortController().signal`) is stored in the returned `ResolveCtx` object, but it is never passed to `fetchGithubIssueContent`, `fetchGithubPullRequestContent`, or `ctx.api.task.byId.query`. If workspace creation is cancelled mid-flight, these outbound `gh` CLI invocations and cloud-API calls will continue running until they complete on their own, making the signal a no-op.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+52
to
+68
| ): Promise<IssueContent> { | ||
| const repo = await resolveGithubRepo(ctx, projectId); | ||
| const raw = await execGh([ | ||
| "issue", | ||
| "view", | ||
| String(issueNumber), | ||
| "--repo", | ||
| `${repo.owner}/${repo.name}`, | ||
| "--json", | ||
| "number,title,body,url,state,author,createdAt,updatedAt", | ||
| ]); | ||
| const data = issueContentSchema.parse(raw); | ||
| return { | ||
| number: data.number, | ||
| title: data.title, | ||
| body: data.body ?? "", | ||
| url: data.url, |
There was a problem hiding this comment.
fetchGithubIssueContent has no cache; resolveGithubRepo called on every PR cache hit
Two minor inefficiencies introduced by this extraction:
-
fetchGithubIssueContenthas no cache at all. The PR description says "shared 30s cache with existing tRPC procedures" but onlyfetchGithubPullRequestContentgot the cache. Issue content is now fetched via the samegh issue viewon every call from both the tRPC procedure and the launch builder. -
fetchGithubPullRequestContentawaitsresolveGithubRepobefore the cache lookup (line 74), so even a cache hit still incurs aresolveGithubReporound-trip. The old per-procedure cache had the same issue, but it is now in a shared helper called from more sites.
Neither is a correctness bug, but issue fetch is missing the caching mentioned in the PR description.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts
Line: 52-68
Comment:
**`fetchGithubIssueContent` has no cache; `resolveGithubRepo` called on every PR cache hit**
Two minor inefficiencies introduced by this extraction:
1. `fetchGithubIssueContent` has no cache at all. The PR description says "shared 30s cache with existing tRPC procedures" but only `fetchGithubPullRequestContent` got the cache. Issue content is now fetched via the same `gh issue view` on every call from both the tRPC procedure and the launch builder.
2. `fetchGithubPullRequestContent` awaits `resolveGithubRepo` **before** the cache lookup (line 74), so even a cache hit still incurs a `resolveGithubRepo` round-trip. The old per-procedure cache had the same issue, but it is now in a shared helper called from more sites.
Neither is a correctness bug, but issue fetch is missing the caching mentioned in the PR description.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+26
to
+30
| */ | ||
| export async function startTerminalLaunch( | ||
| input: StartTerminalLaunchInput, | ||
| ): Promise<StartedTerminalLaunch | { error: string }> { | ||
| const { ctx, workspaceId, plan } = input; |
There was a problem hiding this comment.
plan.spawn.env silently dropped with no runtime warning
The comment documents that env vars are currently ignored, but if a preset row has a non-empty env map and someone adds it in future, there's no log or assertion to surface the omission. A guard like if (Object.keys(plan.spawn.env).length > 0) console.warn(...) would make the gap visible without requiring a code change to add env support.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts
Line: 26-30
Comment:
**`plan.spawn.env` silently dropped with no runtime warning**
The comment documents that env vars are currently ignored, but if a preset row has a non-empty `env` map and someone adds it in future, there's no log or assertion to surface the omission. A guard like `if (Object.keys(plan.spawn.env).length > 0) console.warn(...)` would make the gap visible without requiring a code change to add env support.
How can I resolve this? If you propose a fix, please make it concise.
saddlepaddle
reviewed
May 2, 2026
| worktreePath: string; | ||
| agentPresetId: string; | ||
| prompt?: string; | ||
| githubIssueUrls: string[]; |
Collaborator
There was a problem hiding this comment.
should move all this to the UI imo, and build the prompt in the UI. it probably simplifies a lot of the api code / makes the endpoint way more accessible for consumers
saddlepaddle
reviewed
May 2, 2026
| workspaceId: string; | ||
| projectId: string; | ||
| worktreePath: string; | ||
| agentPresetId: string; |
Collaborator
There was a problem hiding this comment.
hmm what is this? should this be moved to launches?
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts (2)
426-465:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep fallback slugs synthesized.
The success path already backfills missing slugs with
slugifyTitle(...), but both fallback branches returnmatch.slugverbatim. If the host-service/cloud fetch fails and the pending row doesn't already have a slug, downstream launch composition can end up withundefined.♻️ Proposed fix
return { number: match.number ?? 0, url: match.url ?? url, title: match.title, body: "", - slug: match.slug, + slug: match.slug || slugifyTitle(match.title), }; @@ return { id, - slug: match.slug, + slug: match.slug || slugifyTitle(match.title), title: match.title, description: null, };Also applies to: 520-553
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/buildForkAgentLaunch.ts around lines 426 - 465, The fallback branches in fetchIssue return match.slug verbatim which can be undefined; update both fallback returns in the fetchIssue function so they synthesize a slug when missing by using slugifyTitle(match.title) (same logic used in the success path), i.e., for the final return and any other non-cloud/error fallback replace match.slug with (match.slug ?? slugifyTitle(match.title || "")) so downstream launch composition always receives a string slug; reference fetchIssue, match, slugifyTitle, and the client.workspaceCreation.getGitHubIssueContent.query path when applying the change.
399-407:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject malformed attachment payloads instead of emitting empty files.
match?.[1] ?? ""silently turns a bad data URL into a zero-byte attachment. That makes broken uploads look successful and can hide the real source of the failure.🛠️ Proposed fix
function dataUrlAttachmentToBytes(loaded: LoadedAttachment): AttachmentFile { const match = loaded.data.match(/^data:[^;]+;base64,(.+)$/); - const base64 = match?.[1] ?? ""; + if (!match) { + throw new Error(`Invalid attachment data URL: ${loaded.filename}`); + } + const base64 = match[1]; return { data: base64ToBytes(base64), mediaType: loaded.mediaType, filename: loaded.filename, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/buildForkAgentLaunch.ts around lines 399 - 407, The dataUrlAttachmentToBytes function currently converts a malformed data URL into an empty file by using match?.[1] ?? "", so update dataUrlAttachmentToBytes to validate the regex match for a base64 payload (the match variable) and reject malformed inputs instead of returning zero-byte data; specifically, if match is null or match[1] is falsy, throw a descriptive Error (or return a failure result) indicating "malformed data URL attachment" and include the original LoadedAttachment.id/filename for context, otherwise call base64ToBytes(match[1]) and return the AttachmentFile with mediaType and filename as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/src/trpc/router/settings/agent-configs.ts`:
- Around line 162-167: The current code masks a persistence/read failure by
returning null after inserting; change it to throw an explicit error when the
follow-up select against hostAgentConfigs (the variable created) returns falsy
so callers can distinguish a read failure from "unknown preset" — mirror the
behavior used in add(): after the
db.select().from(hostAgentConfigs).where(eq(hostAgentConfigs.id,
insert.id)).get() check, if created is falsy throw a descriptive Error (include
the insert id/context) instead of returning null, otherwise return
toOutput(created).
- Around line 145-154: The function findOrCreateHostPresetByPresetId currently
calls seedDefaultsIfEmpty before validating the presetId which can mutate state
for invalid input; change the flow to call getPresetById(presetId) first and if
it returns null immediately return null, then call seedDefaultsIfEmpty(db) and
search for the preset, using toOutput(existing) if found or creating as
needed—update references in this function (findOrCreateHostPresetByPresetId,
getPresetById, seedDefaultsIfEmpty, toOutput) so the host config is not seeded
when presetId is invalid.
In `@packages/host-service/src/trpc/router/workspace-creation/schemas.ts`:
- Around line 63-68: The schema currently allows empty strings for agentId
because it uses z.string().optional(); update the workspace creation schema for
the agentId field (the agentId symbol) to reject blank values at validation
time—e.g., require a non-empty string when present by replacing the current type
with a non-empty-string constraint such as z.string().min(1).optional() (or an
equivalent refine that disallows ""), so an empty "" will fail validation
instead of being treated as “no launch.”
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts`:
- Around line 89-127: The cache currently uses fetchedAt set before the async
fetch resolves so a slow `execGh` (> PR_CONTENT_CACHE_TTL_MS) lets a second
fetch start; change the logic in prContentCache handling in github-content.ts:
(1) when starting a fetch, immediately set prContentCache.set(cacheKey, {
promise, fetchedAt: 0 }) to ensure in-flight dedupe; (2) on promise resolution
set the entry's fetchedAt = Date.now() (and update the stored entry if needed);
(3) change the lookup to first return cached.promise if cached exists (to dedupe
in-flight) and only apply TTL logic when cached.fetchedAt is non-zero (e.g., if
(cached && cached.fetchedAt && Date.now() - cached.fetchedAt <
PR_CONTENT_CACHE_TTL_MS) return cached.promise). Reference symbols:
prContentCache, cacheKey, fetchedAt, promise, PR_CONTENT_CACHE_TTL_MS, execGh.
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts`:
- Around line 21-23: The loop in write-attachments.ts writes attachment.filename
verbatim with writeFileSync(join(dir, filename)), which allows path traversal
and uses a non-unique fallback "attachment"; sanitize or validate names by
normalizing and rejecting names containing path separators or traversal
sequences (e.g. ".." or absolute paths) and strip unsafe characters, or else
generate a safe unique filename (e.g. UUID or incremented counter with original
extension) when attachment.filename is missing or unsafe; apply this check
inside the for (const attachment of attachments) block before calling
writeFileSync so join(dir, safeFilename) is always within the attachments
directory.
In `@plans/20260502-workspace-create-pr4.md`:
- Around line 110-117: Two fenced code blocks are missing language tags causing
markdownlint MD040; update both fenced blocks shown (the directory tree block
containing launches/ with files like build-agent-launch.ts,
start-terminal-launch.ts, start-chat-launch.ts, write-attachments.ts, index.ts
and the flow diagram block starting with "ensureMainWorkspace" and containing
steps like writeAttachmentsToWorktree, buildAgentLaunch, startTerminalLaunch /
startChatLaunch) to include a language identifier (e.g., change ``` to ```text)
so markdownlint no longer flags them.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/buildForkAgentLaunch.ts:
- Around line 426-465: The fallback branches in fetchIssue return match.slug
verbatim which can be undefined; update both fallback returns in the fetchIssue
function so they synthesize a slug when missing by using
slugifyTitle(match.title) (same logic used in the success path), i.e., for the
final return and any other non-cloud/error fallback replace match.slug with
(match.slug ?? slugifyTitle(match.title || "")) so downstream launch composition
always receives a string slug; reference fetchIssue, match, slugifyTitle, and
the client.workspaceCreation.getGitHubIssueContent.query path when applying the
change.
- Around line 399-407: The dataUrlAttachmentToBytes function currently converts
a malformed data URL into an empty file by using match?.[1] ?? "", so update
dataUrlAttachmentToBytes to validate the regex match for a base64 payload (the
match variable) and reject malformed inputs instead of returning zero-byte data;
specifically, if match is null or match[1] is falsy, throw a descriptive Error
(or return a failure result) indicating "malformed data URL attachment" and
include the original LoadedAttachment.id/filename for context, otherwise call
base64ToBytes(match[1]) and return the AttachmentFile with mediaType and
filename as before.
🪄 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: 00237cfc-ccf8-4cf6-8c51-3c2e626a9035
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (52)
apps/desktop/package.jsonapps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.tspackages/host-service/package.jsonpackages/host-service/src/trpc/router/attachments/storage.tspackages/host-service/src/trpc/router/settings/agent-configs.test.tspackages/host-service/src/trpc/router/settings/agent-configs.tspackages/host-service/src/trpc/router/settings/agent-presets.tspackages/host-service/src/trpc/router/workspace-creation/procedures/create.tspackages/host-service/src/trpc/router/workspace-creation/procedures/get-github-issue-content.tspackages/host-service/src/trpc/router/workspace-creation/procedures/get-github-pull-request-content.tspackages/host-service/src/trpc/router/workspace-creation/schemas.tspackages/host-service/src/trpc/router/workspace-creation/shared/github-content.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/build-agent-launch.test.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/build-agent-launch.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/host-resolve-ctx.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/index.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/resolve-attachment-files.test.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/resolve-attachment-files.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/synth-agent-config.test.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/synth-agent-config.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.test.tspackages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.tspackages/host-service/src/trpc/router/workspace-creation/shared/types.tspackages/launch-context/package.jsonpackages/launch-context/src/__fixtures__/attachment.logs-txt.tspackages/launch-context/src/__fixtures__/githubIssue.auth-middleware.tspackages/launch-context/src/__fixtures__/githubPr.auth-rewrite.tspackages/launch-context/src/__fixtures__/index.tspackages/launch-context/src/__fixtures__/internalTask.refactor-auth.tspackages/launch-context/src/__fixtures__/launchContext.multi-source.tspackages/launch-context/src/__fixtures__/launchContext.prompt-only.tspackages/launch-context/src/buildLaunchSpec.test.tspackages/launch-context/src/buildLaunchSpec.tspackages/launch-context/src/composer.integration.test.tspackages/launch-context/src/composer.test.tspackages/launch-context/src/composer.tspackages/launch-context/src/contributors/attachment.test.tspackages/launch-context/src/contributors/attachment.tspackages/launch-context/src/contributors/githubIssue.test.tspackages/launch-context/src/contributors/githubIssue.tspackages/launch-context/src/contributors/githubPr.test.tspackages/launch-context/src/contributors/githubPr.tspackages/launch-context/src/contributors/index.tspackages/launch-context/src/contributors/internalTask.test.tspackages/launch-context/src/contributors/internalTask.tspackages/launch-context/src/contributors/userPrompt.test.tspackages/launch-context/src/contributors/userPrompt.tspackages/launch-context/src/index.tspackages/launch-context/src/types.tspackages/launch-context/tsconfig.jsonplans/20260502-workspace-create-pr4.md
Comment on lines
+145
to
+154
| export function findOrCreateHostPresetByPresetId( | ||
| db: HostDb, | ||
| presetId: string, | ||
| ): HostAgentConfigOutput | null { | ||
| const seeded = seedDefaultsIfEmpty(db); | ||
| const existing = seeded.find((row) => row.presetId === presetId); | ||
| if (existing) return toOutput(existing); | ||
|
|
||
| const preset = getPresetById(presetId); | ||
| if (!preset) return null; |
Contributor
There was a problem hiding this comment.
Check the preset before seeding defaults.
A typo currently seeds the bundled defaults even though the helper ultimately returns null. Validate presetId first so bad input doesn't mutate host config state.
♻️ Proposed fix
export function findOrCreateHostPresetByPresetId(
db: HostDb,
presetId: string,
): HostAgentConfigOutput | null {
- const seeded = seedDefaultsIfEmpty(db);
- const existing = seeded.find((row) => row.presetId === presetId);
- if (existing) return toOutput(existing);
-
const preset = getPresetById(presetId);
if (!preset) return null;
+
+ const seeded = seedDefaultsIfEmpty(db);
+ const existing = seeded.find((row) => row.presetId === presetId);
+ if (existing) return toOutput(existing);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/trpc/router/settings/agent-configs.ts` around lines
145 - 154, The function findOrCreateHostPresetByPresetId currently calls
seedDefaultsIfEmpty before validating the presetId which can mutate state for
invalid input; change the flow to call getPresetById(presetId) first and if it
returns null immediately return null, then call seedDefaultsIfEmpty(db) and
search for the preset, using toOutput(existing) if found or creating as
needed—update references in this function (findOrCreateHostPresetByPresetId,
getPresetById, seedDefaultsIfEmpty, toOutput) so the host config is not seeded
when presetId is invalid.
Comment on lines
+162
to
+167
| const created = db | ||
| .select() | ||
| .from(hostAgentConfigs) | ||
| .where(eq(hostAgentConfigs.id, insert.id)) | ||
| .get(); | ||
| return created ? toOutput(created) : null; |
Contributor
There was a problem hiding this comment.
Don't hide a persistence failure behind null.
If the insert succeeds but the follow-up read fails, the caller can't distinguish that from "unknown preset" and workspace creation will silently skip launch setup. Throw here instead, like add() does.
🔧 Proposed fix
const created = db
.select()
.from(hostAgentConfigs)
.where(eq(hostAgentConfigs.id, insert.id))
.get();
- return created ? toOutput(created) : null;
+ if (!created) {
+ throw new TRPCError({
+ code: "INTERNAL_SERVER_ERROR",
+ message: "Failed to read back inserted host agent config",
+ });
+ }
+ return toOutput(created);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/trpc/router/settings/agent-configs.ts` around lines
162 - 167, The current code masks a persistence/read failure by returning null
after inserting; change it to throw an explicit error when the follow-up select
against hostAgentConfigs (the variable created) returns falsy so callers can
distinguish a read failure from "unknown preset" — mirror the behavior used in
add(): after the
db.select().from(hostAgentConfigs).where(eq(hostAgentConfigs.id,
insert.id)).get() check, if created is falsy throw a descriptive Error (include
the insert id/context) instead of returning null, otherwise return
toOutput(created).
Comment on lines
+63
to
+68
| // PR4: presetId of the host_agent_configs row to launch in the | ||
| // new workspace. When set, host-service builds an | ||
| // AgentLaunchSpec, writes attachments, and spawns the agent | ||
| // terminal as part of create. Returned in `launches[]` on the | ||
| // response. | ||
| agentId: z.string().optional(), |
Contributor
There was a problem hiding this comment.
Reject blank agent IDs at the schema boundary.
z.string().optional() still accepts "", but the create flow treats falsy agentId as “no launch,” so this will silently skip the launch path instead of failing validation.
🔧 Suggested fix
- agentId: z.string().optional(),
+ agentId: z.string().min(1).optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // PR4: presetId of the host_agent_configs row to launch in the | |
| // new workspace. When set, host-service builds an | |
| // AgentLaunchSpec, writes attachments, and spawns the agent | |
| // terminal as part of create. Returned in `launches[]` on the | |
| // response. | |
| agentId: z.string().optional(), | |
| // PR4: presetId of the host_agent_configs row to launch in the | |
| // new workspace. When set, host-service builds an | |
| // AgentLaunchSpec, writes attachments, and spawns the agent | |
| // terminal as part of create. Returned in `launches[]` on the | |
| // response. | |
| agentId: z.string().min(1).optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/trpc/router/workspace-creation/schemas.ts` around
lines 63 - 68, The schema currently allows empty strings for agentId because it
uses z.string().optional(); update the workspace creation schema for the agentId
field (the agentId symbol) to reject blank values at validation time—e.g.,
require a non-empty string when present by replacing the current type with a
non-empty-string constraint such as z.string().min(1).optional() (or an
equivalent refine that disallows ""), so an empty "" will fail validation
instead of being treated as “no launch.”
Comment on lines
+89
to
+127
| if (cached && Date.now() - cached.fetchedAt < PR_CONTENT_CACHE_TTL_MS) { | ||
| return cached.promise; | ||
| } | ||
|
|
||
| const fetchedAt = Date.now(); | ||
| const promise = (async (): Promise<PullRequestContent> => { | ||
| const raw = await execGh([ | ||
| "pr", | ||
| "view", | ||
| String(prNumber), | ||
| "--repo", | ||
| `${repo.owner}/${repo.name}`, | ||
| "--json", | ||
| "number,title,body,url,state,author,headRefName,baseRefName,headRepositoryOwner,isCrossRepository,isDraft,createdAt,updatedAt", | ||
| ]); | ||
| const data = pullRequestContentSchema.parse(raw); | ||
| return { | ||
| number: data.number, | ||
| title: data.title, | ||
| body: data.body ?? "", | ||
| url: data.url, | ||
| state: data.state.toLowerCase(), | ||
| branch: data.headRefName, | ||
| baseBranch: data.baseRefName, | ||
| headRepositoryOwner: data.headRepositoryOwner?.login ?? null, | ||
| isCrossRepository: data.isCrossRepository, | ||
| author: data.author?.login ?? null, | ||
| isDraft: data.isDraft, | ||
| createdAt: data.createdAt, | ||
| updatedAt: data.updatedAt, | ||
| }; | ||
| })(); | ||
| promise.catch(() => { | ||
| if (prContentCache.get(cacheKey)?.promise === promise) { | ||
| prContentCache.delete(cacheKey); | ||
| } | ||
| }); | ||
| prContentCache.set(cacheKey, { promise, fetchedAt }); | ||
| return promise; |
Contributor
There was a problem hiding this comment.
PR cache can miss in-flight dedupe when fetch latency exceeds TTL.
fetchedAt is set before the async fetch resolves. If a gh pr view call takes longer than 30s, subsequent requests can start a second fetch despite an in-flight promise, defeating the token-bucket protection goal.
💡 Suggested fix
const cached = prContentCache.get(cacheKey);
-if (cached && Date.now() - cached.fetchedAt < PR_CONTENT_CACHE_TTL_MS) {
- return cached.promise;
+if (
+ cached &&
+ (cached.fetchedAt === 0 ||
+ Date.now() - cached.fetchedAt < PR_CONTENT_CACHE_TTL_MS)
+) {
+ return cached.promise;
}
-const fetchedAt = Date.now();
const promise = (async (): Promise<PullRequestContent> => {
// ...
})();
+const entry = { promise, fetchedAt: 0 };
+prContentCache.set(cacheKey, entry);
+
+promise.then(() => {
+ entry.fetchedAt = Date.now();
+});
promise.catch(() => {
if (prContentCache.get(cacheKey)?.promise === promise) {
prContentCache.delete(cacheKey);
}
});
-prContentCache.set(cacheKey, { promise, fetchedAt });
return promise;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (cached && Date.now() - cached.fetchedAt < PR_CONTENT_CACHE_TTL_MS) { | |
| return cached.promise; | |
| } | |
| const fetchedAt = Date.now(); | |
| const promise = (async (): Promise<PullRequestContent> => { | |
| const raw = await execGh([ | |
| "pr", | |
| "view", | |
| String(prNumber), | |
| "--repo", | |
| `${repo.owner}/${repo.name}`, | |
| "--json", | |
| "number,title,body,url,state,author,headRefName,baseRefName,headRepositoryOwner,isCrossRepository,isDraft,createdAt,updatedAt", | |
| ]); | |
| const data = pullRequestContentSchema.parse(raw); | |
| return { | |
| number: data.number, | |
| title: data.title, | |
| body: data.body ?? "", | |
| url: data.url, | |
| state: data.state.toLowerCase(), | |
| branch: data.headRefName, | |
| baseBranch: data.baseRefName, | |
| headRepositoryOwner: data.headRepositoryOwner?.login ?? null, | |
| isCrossRepository: data.isCrossRepository, | |
| author: data.author?.login ?? null, | |
| isDraft: data.isDraft, | |
| createdAt: data.createdAt, | |
| updatedAt: data.updatedAt, | |
| }; | |
| })(); | |
| promise.catch(() => { | |
| if (prContentCache.get(cacheKey)?.promise === promise) { | |
| prContentCache.delete(cacheKey); | |
| } | |
| }); | |
| prContentCache.set(cacheKey, { promise, fetchedAt }); | |
| return promise; | |
| if ( | |
| cached && | |
| (cached.fetchedAt === 0 || | |
| Date.now() - cached.fetchedAt < PR_CONTENT_CACHE_TTL_MS) | |
| ) { | |
| return cached.promise; | |
| } | |
| const promise = (async (): Promise<PullRequestContent> => { | |
| const raw = await execGh([ | |
| "pr", | |
| "view", | |
| String(prNumber), | |
| "--repo", | |
| `${repo.owner}/${repo.name}`, | |
| "--json", | |
| "number,title,body,url,state,author,headRefName,baseRefName,headRepositoryOwner,isCrossRepository,isDraft,createdAt,updatedAt", | |
| ]); | |
| const data = pullRequestContentSchema.parse(raw); | |
| return { | |
| number: data.number, | |
| title: data.title, | |
| body: data.body ?? "", | |
| url: data.url, | |
| state: data.state.toLowerCase(), | |
| branch: data.headRefName, | |
| baseBranch: data.baseRefName, | |
| headRepositoryOwner: data.headRepositoryOwner?.login ?? null, | |
| isCrossRepository: data.isCrossRepository, | |
| author: data.author?.login ?? null, | |
| isDraft: data.isDraft, | |
| createdAt: data.createdAt, | |
| updatedAt: data.updatedAt, | |
| }; | |
| })(); | |
| const entry = { promise, fetchedAt: 0 }; | |
| prContentCache.set(cacheKey, entry); | |
| promise.then(() => { | |
| entry.fetchedAt = Date.now(); | |
| }); | |
| promise.catch(() => { | |
| if (prContentCache.get(cacheKey)?.promise === promise) { | |
| prContentCache.delete(cacheKey); | |
| } | |
| }); | |
| return promise; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts`
around lines 89 - 127, The cache currently uses fetchedAt set before the async
fetch resolves so a slow `execGh` (> PR_CONTENT_CACHE_TTL_MS) lets a second
fetch start; change the logic in prContentCache handling in github-content.ts:
(1) when starting a fetch, immediately set prContentCache.set(cacheKey, {
promise, fetchedAt: 0 }) to ensure in-flight dedupe; (2) on promise resolution
set the entry's fetchedAt = Date.now() (and update the stored entry if needed);
(3) change the lookup to first return cached.promise if cached exists (to dedupe
in-flight) and only apply TTL logic when cached.fetchedAt is non-zero (e.g., if
(cached && cached.fetchedAt && Date.now() - cached.fetchedAt <
PR_CONTENT_CACHE_TTL_MS) return cached.promise). Reference symbols:
prContentCache, cacheKey, fetchedAt, promise, PR_CONTENT_CACHE_TTL_MS, execGh.
Comment on lines
+21
to
+23
| for (const attachment of attachments) { | ||
| const filename = attachment.filename ?? "attachment"; | ||
| writeFileSync(join(dir, filename), attachment.data); |
Contributor
There was a problem hiding this comment.
Sanitize attachment filenames before writing them.
This trusts attachment.filename verbatim. A crafted path can escape .superset/attachments, and the current "attachment" fallback will also overwrite every unnamed attachment. Normalize or reject unsafe names and make the fallback unique.
🔧 Suggested fix
- for (const attachment of attachments) {
- const filename = attachment.filename ?? "attachment";
- writeFileSync(join(dir, filename), attachment.data);
+ for (const [index, attachment] of attachments.entries()) {
+ const filename =
+ attachment.filename?.trim().replace(/[\\/]/g, "_").replace(/^\.+$/, "_") ||
+ `attachment-${index}`;
+ writeFileSync(join(dir, filename), attachment.data);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const attachment of attachments) { | |
| const filename = attachment.filename ?? "attachment"; | |
| writeFileSync(join(dir, filename), attachment.data); | |
| for (const [index, attachment] of attachments.entries()) { | |
| const filename = | |
| attachment.filename?.trim().replace(/[\\/]/g, "_").replace(/^\.+$/, "_") || | |
| `attachment-${index}`; | |
| writeFileSync(join(dir, filename), attachment.data); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts`
around lines 21 - 23, The loop in write-attachments.ts writes
attachment.filename verbatim with writeFileSync(join(dir, filename)), which
allows path traversal and uses a non-unique fallback "attachment"; sanitize or
validate names by normalizing and rejecting names containing path separators or
traversal sequences (e.g. ".." or absolute paths) and strip unsafe characters,
or else generate a safe unique filename (e.g. UUID or incremented counter with
original extension) when attachment.filename is missing or unsafe; apply this
check inside the for (const attachment of attachments) block before calling
writeFileSync so join(dir, safeFilename) is always within the attachments
directory.
Comment on lines
+110
to
+117
| ``` | ||
| launches/ | ||
| ├── build-agent-launch.ts # agentId + composer → command/chat config | ||
| ├── start-terminal-launch.ts # spawn PTY, return terminalId | ||
| ├── start-chat-launch.ts # create chat session, return sessionId | ||
| ├── write-attachments.ts # resolve attachment IDs → worktree files | ||
| └── index.ts | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
Two fenced blocks are missing language tags, which triggers markdownlint warnings.
Suggested patch
-```
+```text
launches/
├── build-agent-launch.ts # agentId + composer → command/chat config
├── start-terminal-launch.ts # spawn PTY, return terminalId
├── start-chat-launch.ts # create chat session, return sessionId
├── write-attachments.ts # resolve attachment IDs → worktree files
└── index.ts- +text
ensureMainWorkspace
↓
deduplicateBranchName + worktree add
↓
register cloud row + local workspaces row
↓
fire-and-forget AI rename
↓
[NEW] writeAttachmentsToWorktree (if linkedContext.attachmentIds)
↓
[NEW] buildAgentLaunch (if composer.agentId)
↓
[NEW] startTerminalLaunch / startChatLaunch
↓
[KEPT] startSetupTerminalIfPresent (if runSetupScript) — also pushed
into launches[]
↓
return { workspace, launches, terminals, warnings }
Also applies to: 133-152
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plans/20260502-workspace-create-pr4.md` around lines 110 - 117, Two fenced
code blocks are missing language tags causing markdownlint MD040; update both
fenced blocks shown (the directory tree block containing launches/ with files
like build-agent-launch.ts, start-terminal-launch.ts, start-chat-launch.ts,
write-attachments.ts, index.ts and the flow diagram block starting with
"ensureMainWorkspace" and containing steps like writeAttachmentsToWorktree,
buildAgentLaunch, startTerminalLaunch / startChatLaunch) to include a language
identifier (e.g., change ``` to ```text) so markdownlint no longer flags them.
saddlepaddle
reviewed
May 2, 2026
| linkedPrUrl?: string; | ||
| attachmentIds: string[]; | ||
| launches: LaunchDescriptor[]; | ||
| warnings: string[]; |
Collaborator
There was a problem hiding this comment.
why are warnings part of the input out of curiosity? seems like a smell
saddlepaddle
reviewed
May 2, 2026
| kind: "terminal", | ||
| terminalId: result.terminalId, | ||
| label: result.label, | ||
| }); |
Collaborator
There was a problem hiding this comment.
no return value from here?
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 53 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="packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-pull-request-content.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-pull-request-content.ts:15">
P2: Do not wrap existing `TRPCError`s into `INTERNAL_SERVER_ERROR`; this masks client-meaningful error codes like `BAD_REQUEST`.</violation>
</file>
<file name="plans/20260502-workspace-create-pr4.md">
<violation number="1" location="plans/20260502-workspace-create-pr4.md:300">
P2: Decision 1 contradicts Decision 5 about whether PR4 adds template columns, which can lead to implementing the wrong schema changes.</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-issue-content.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-issue-content.ts:14">
P2: Preserve existing `TRPCError` codes here; wrapping all helper errors as `INTERNAL_SERVER_ERROR` turns expected client errors (like missing linked repo) into 500s.</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts:22">
P2: Validate attachment filenames before writing; dot-segment names like `.`/`..` can resolve outside the intended file target and cause write failures.</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts:39">
P2: Agent preset environment variables are silently ignored during workspace-create launches, so configured runtime env (for example API keys or flags) is never applied to the spawned agent process.</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts:48">
P3: Add the same short-lived in-memory cache used for PR content to issue content fetches; otherwise repeated issue lookups trigger redundant `gh issue view` calls in both tRPC and launch-building paths.</violation>
</file>
<file name="packages/host-service/src/trpc/router/settings/agent-configs.ts">
<violation number="1" location="packages/host-service/src/trpc/router/settings/agent-configs.ts:167">
P2: Do not return `null` on insert readback failure; throw an error so `null` remains reserved for unknown preset IDs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+15
to
20
| } catch (err) { | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: `Failed to fetch PR #${input.prNumber}: ${err instanceof Error ? err.message : String(err)}`, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Do not wrap existing TRPCErrors into INTERNAL_SERVER_ERROR; this masks client-meaningful error codes like BAD_REQUEST.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-pull-request-content.ts, line 15:
<comment>Do not wrap existing `TRPCError`s into `INTERNAL_SERVER_ERROR`; this masks client-meaningful error codes like `BAD_REQUEST`.</comment>
<file context>
@@ -1,92 +1,21 @@
+ input.projectId,
+ input.prNumber,
+ );
+ } catch (err) {
+ throw new TRPCError({
+ code: "INTERNAL_SERVER_ERROR",
</file context>
Suggested change
| } catch (err) { | |
| throw new TRPCError({ | |
| code: "INTERNAL_SERVER_ERROR", | |
| message: `Failed to fetch PR #${input.prNumber}: ${err instanceof Error ? err.message : String(err)}`, | |
| }); | |
| } | |
| } catch (err) { | |
| if (err instanceof TRPCError) { | |
| throw err; | |
| } | |
| throw new TRPCError({ | |
| code: "INTERNAL_SERVER_ERROR", | |
| message: `Failed to fetch PR #${input.prNumber}: ${err instanceof Error ? err.message : String(err)}`, | |
| }); | |
| } |
| 1. **Agent preset selection.** Renderer passes `composer.agentId`; | ||
| host resolves the preset from PR 1's `host_agent_configs` table. | ||
| Future CLI / automation callers don't need to know how presets | ||
| work. **PR 4 extends PR 1's schema with template columns** (see |
Contributor
There was a problem hiding this comment.
P2: Decision 1 contradicts Decision 5 about whether PR4 adds template columns, which can lead to implementing the wrong schema changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plans/20260502-workspace-create-pr4.md, line 300:
<comment>Decision 1 contradicts Decision 5 about whether PR4 adds template columns, which can lead to implementing the wrong schema changes.</comment>
<file context>
@@ -0,0 +1,370 @@
+1. **Agent preset selection.** Renderer passes `composer.agentId`;
+ host resolves the preset from PR 1's `host_agent_configs` table.
+ Future CLI / automation callers don't need to know how presets
+ work. **PR 4 extends PR 1's schema with template columns** (see
+ decision 5) so the host can build the full prompt itself, not
+ just `[command, ...args, prompt]`.
</file context>
Comment on lines
+14
to
21
| return await fetchGithubIssueContent( | ||
| ctx, | ||
| input.projectId, | ||
| input.issueNumber, | ||
| ); | ||
| } catch (err) { | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", |
Contributor
There was a problem hiding this comment.
P2: Preserve existing TRPCError codes here; wrapping all helper errors as INTERNAL_SERVER_ERROR turns expected client errors (like missing linked repo) into 500s.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/procedures/get-github-issue-content.ts, line 14:
<comment>Preserve existing `TRPCError` codes here; wrapping all helper errors as `INTERNAL_SERVER_ERROR` turns expected client errors (like missing linked repo) into 500s.</comment>
<file context>
@@ -11,28 +10,12 @@ import { execGh } from "../utils/exec-gh";
- createdAt: data.createdAt,
- updatedAt: data.updatedAt,
- };
+ return await fetchGithubIssueContent(
+ ctx,
+ input.projectId,
</file context>
| const dir = join(worktreePath, WORKTREE_ATTACHMENTS_SUBDIR); | ||
| mkdirSync(dir, { recursive: true }); | ||
| for (const attachment of attachments) { | ||
| const filename = attachment.filename ?? "attachment"; |
Contributor
There was a problem hiding this comment.
P2: Validate attachment filenames before writing; dot-segment names like ./.. can resolve outside the intended file target and cause write failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/shared/launches/write-attachments.ts, line 22:
<comment>Validate attachment filenames before writing; dot-segment names like `.`/`..` can resolve outside the intended file target and cause write failures.</comment>
<file context>
@@ -0,0 +1,25 @@
+ const dir = join(worktreePath, WORKTREE_ATTACHMENTS_SUBDIR);
+ mkdirSync(dir, { recursive: true });
+ for (const attachment of attachments) {
+ const filename = attachment.filename ?? "attachment";
+ writeFileSync(join(dir, filename), attachment.data);
+ }
</file context>
| } | ||
|
|
||
| const terminalId = crypto.randomUUID(); | ||
| const result = await createTerminalSessionInternal({ |
Contributor
There was a problem hiding this comment.
P2: Agent preset environment variables are silently ignored during workspace-create launches, so configured runtime env (for example API keys or flags) is never applied to the spawned agent process.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/shared/launches/start-terminal-launch.ts, line 39:
<comment>Agent preset environment variables are silently ignored during workspace-create launches, so configured runtime env (for example API keys or flags) is never applied to the spawned agent process.</comment>
<file context>
@@ -0,0 +1,54 @@
+ }
+
+ const terminalId = crypto.randomUUID();
+ const result = await createTerminalSessionInternal({
+ terminalId,
+ workspaceId,
</file context>
| .from(hostAgentConfigs) | ||
| .where(eq(hostAgentConfigs.id, insert.id)) | ||
| .get(); | ||
| return created ? toOutput(created) : null; |
Contributor
There was a problem hiding this comment.
P2: Do not return null on insert readback failure; throw an error so null remains reserved for unknown preset IDs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/settings/agent-configs.ts, line 167:
<comment>Do not return `null` on insert readback failure; throw an error so `null` remains reserved for unknown preset IDs.</comment>
<file context>
@@ -132,6 +132,43 @@ function seedDefaultsIfEmpty(db: HostDb): HostAgentConfigRow[] {
+ .from(hostAgentConfigs)
+ .where(eq(hostAgentConfigs.id, insert.id))
+ .get();
+ return created ? toOutput(created) : null;
+}
+
</file context>
Suggested change
| return created ? toOutput(created) : null; | |
| if (!created) { | |
| throw new Error( | |
| `Failed to read back inserted host agent config for presetId: ${presetId}`, | |
| ); | |
| } | |
| return toOutput(created); |
| * inline issue bodies into the agent prompt without going through | ||
| * tRPC. | ||
| */ | ||
| export async function fetchGithubIssueContent( |
Contributor
There was a problem hiding this comment.
P3: Add the same short-lived in-memory cache used for PR content to issue content fetches; otherwise repeated issue lookups trigger redundant gh issue view calls in both tRPC and launch-building paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/shared/github-content.ts, line 48:
<comment>Add the same short-lived in-memory cache used for PR content to issue content fetches; otherwise repeated issue lookups trigger redundant `gh issue view` calls in both tRPC and launch-building paths.</comment>
<file context>
@@ -0,0 +1,128 @@
+ * inline issue bodies into the agent prompt without going through
+ * tRPC.
+ */
+export async function fetchGithubIssueContent(
+ ctx: HostServiceContext,
+ projectId: string,
</file context>
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
2 participants
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
PR4 of the canonical
workspace.create()flow: moves the renderer'sbuildForkAgentLaunchpipeline onto host-service so workspace creation can spawn the terminal agent server-side.apps/desktop/src/shared/context→ new@superset/launch-contextpackage so host-service can consumebuildLaunchSpec/ composer / contributors. Pure move, 54 tests follow.workspace-creation/shared/launches/module:synth-agent-config,build-agent-launch,start-terminal-launch,write-attachments,host-resolve-ctx,resolve-attachment-files. Argv-array model from PR1; no V1 escape hatches.create.ts: whencomposer.agentIdis set, host resolves the preset row (auto-materializing builtins), reads attachment bytes, fetches issue/PR bodies viagh(shared 30s cache with existing tRPC procedures), then runsbuildAgentLaunch → writeAttachmentsToWorktree → startTerminalLaunchand returnslaunches[]. Launch failures soft-fail — the workspace is already created.stdintransport (matches amp). Locks in the decision not to add per-preset template columns; templates come from@superset/sharedconstants.Renderer integration (#27) is the remaining piece — backend is done.
Plan:
plans/20260502-workspace-create-pr4.md.Test plan
bun testinpackages/host-service(build-agent-launch, write-attachments, synth-agent-config, resolve-attachment-files, agent-configs, create-flow integration)bun testinpackages/launch-context(54 moved tests still pass)bun run typecheck<worktree>/.superset/attachments/and issue body is included in the launch contextSummary by cubic
Host-service now builds and starts terminal agents during
workspace.create(), returninglaunches[]so panes open without a follow-up dispatch. Launch-context moved to@superset/launch-context, and themastracodepreset now uses stdin transport.New Features
@superset/launch-context, read attachment bytes, write to<worktree>/.superset/attachments/, and spawn the terminal; failures add warnings and don’t abort create.createnow returnslaunches[](plus existing fields). Presets auto-materialize on first use; GitHub issue/PR content fetched viaghwith a 30s cache.readAttachmentto the host attachment store.Migration
composer.agentIdandlinkedContext.attachmentIdsto trigger a server-side agent launch; inlinelinkedContext.attachmentsstays for one release.@superset/launch-contextforbuildLaunchSpec, composer, and contributors (imports updated).Written for commit 2b84dca. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements