-
Notifications
You must be signed in to change notification settings - Fork 965
feat(host-service): host-side agent launches in workspace.create (PR4) #3990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7cbd388
30eb85a
86d6970
2b84dca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -132,6 +132,43 @@ function seedDefaultsIfEmpty(db: HostDb): HostAgentConfigRow[] { | |||||||||||||||
| return listOrdered(db); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Find an agent config row by `presetId`, seeding defaults first if | ||||||||||||||||
| * the table is empty and creating the row from the preset definition | ||||||||||||||||
| * if the user picked a builtin that wasn't part of the default seed | ||||||||||||||||
| * set (e.g. mastracode, opencode, pi). Returns null only when | ||||||||||||||||
| * `presetId` doesn't match any known preset. | ||||||||||||||||
| * | ||||||||||||||||
| * Used by the launches/ wiring in `create.ts`: a user picking any of | ||||||||||||||||
| * the 9 builtin agents gets a row materialized on first use. | ||||||||||||||||
| */ | ||||||||||||||||
| 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 nextOrder = | ||||||||||||||||
| seeded.length === 0 | ||||||||||||||||
| ? 0 | ||||||||||||||||
| : Math.max(...seeded.map((row) => row.displayOrder)) + 1; | ||||||||||||||||
| const insert = rowFromPreset(preset, nextOrder); | ||||||||||||||||
| db.insert(hostAgentConfigs).values(insert).run(); | ||||||||||||||||
| const created = db | ||||||||||||||||
| .select() | ||||||||||||||||
| .from(hostAgentConfigs) | ||||||||||||||||
| .where(eq(hostAgentConfigs.id, insert.id)) | ||||||||||||||||
| .get(); | ||||||||||||||||
| return created ? toOutput(created) : null; | ||||||||||||||||
|
Comment on lines
+162
to
+167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't hide a persistence failure behind 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 🔧 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Do not return Prompt for AI agents
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| export type { HostAgentConfigOutput }; | ||||||||||||||||
|
|
||||||||||||||||
| const updatePatchSchema = z | ||||||||||||||||
| .object({ | ||||||||||||||||
| label: z.string().trim().min(1).optional(), | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,16 +9,25 @@ import { | |
| resolveDefaultBranchName, | ||
| resolveUpstream, | ||
| } from "../../../../runtime/git/refs"; | ||
| import type { HostServiceContext } from "../../../../types"; | ||
| import { protectedProcedure } from "../../../index"; | ||
| import { gitConfigWrite } from "../../git/utils/config-write"; | ||
| import { ensureMainWorkspace } from "../../project/utils/ensure-main-workspace"; | ||
| import { findOrCreateHostPresetByPresetId } from "../../settings/agent-configs"; | ||
| import { createInputSchema } from "../schemas"; | ||
| import { enablePushAutoSetupRemote } from "../shared/git-config"; | ||
| import { | ||
| buildAgentLaunch, | ||
| buildHostResolveCtx, | ||
| resolveAttachmentFiles, | ||
| startTerminalLaunch, | ||
| writeAttachmentsToWorktree, | ||
| } from "../shared/launches"; | ||
| import { requireLocalProject } from "../shared/local-project"; | ||
| import { clearProgress, setProgress } from "../shared/progress-store"; | ||
| import { startSetupTerminalIfPresent } from "../shared/setup-terminal"; | ||
| import { buildStartPointFromHint } from "../shared/start-point"; | ||
| import type { TerminalDescriptor } from "../shared/types"; | ||
| import type { LaunchDescriptor, TerminalDescriptor } from "../shared/types"; | ||
| import { safeResolveWorktreePath } from "../shared/worktree-paths"; | ||
| import { applyAiWorkspaceRename } from "../utils/ai-workspace-names"; | ||
| import { listBranchNames } from "../utils/list-branch-names"; | ||
|
|
@@ -333,6 +342,7 @@ export const create = protectedProcedure | |
| } | ||
|
|
||
| const terminals: TerminalDescriptor[] = []; | ||
| const launches: LaunchDescriptor[] = []; | ||
| const warnings: string[] = []; | ||
|
|
||
| if (input.composer.runSetupScript) { | ||
|
|
@@ -349,14 +359,118 @@ export const create = protectedProcedure | |
| } | ||
| } | ||
|
|
||
| // PR4: agent launch. Optional — runs only when the renderer | ||
| // passed `composer.agentId` (the picker selected an agent). | ||
| // Failures here surface as warnings rather than aborting create | ||
| // — the workspace is already on disk and registered, so a | ||
| // missing preset / spawn failure shouldn't unwind that work. | ||
| if (input.composer.agentId) { | ||
| try { | ||
| await runAgentLaunch({ | ||
| ctx, | ||
| workspaceId: cloudRow.id, | ||
| projectId: input.projectId, | ||
| worktreePath, | ||
| agentPresetId: input.composer.agentId, | ||
| prompt: input.composer.prompt, | ||
| githubIssueUrls: input.linkedContext?.githubIssueUrls ?? [], | ||
| internalTaskIds: input.linkedContext?.internalIssueIds ?? [], | ||
| linkedPrUrl: input.linkedContext?.linkedPrUrl, | ||
| attachmentIds: input.linkedContext?.attachmentIds ?? [], | ||
| launches, | ||
| warnings, | ||
| }); | ||
| } catch (err) { | ||
| console.warn( | ||
| "[workspaceCreation.create] agent launch failed; continuing without", | ||
| err, | ||
| ); | ||
| warnings.push( | ||
| `Agent launch failed: ${err instanceof Error ? err.message : String(err)}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| clearProgress(input.pendingId); | ||
|
|
||
| return { | ||
| workspace: cloudRow, | ||
| terminals, | ||
| launches, | ||
| warnings, | ||
| }; | ||
| } finally { | ||
| clearProgress(input.pendingId); | ||
| } | ||
| }); | ||
|
|
||
| interface RunAgentLaunchInput { | ||
| ctx: HostServiceContext; | ||
| workspaceId: string; | ||
| projectId: string; | ||
| worktreePath: string; | ||
| agentPresetId: string; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm what is this? should this be moved to launches? |
||
| prompt?: string; | ||
| githubIssueUrls: string[]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| internalTaskIds: string[]; | ||
| linkedPrUrl?: string; | ||
| attachmentIds: string[]; | ||
| launches: LaunchDescriptor[]; | ||
| warnings: string[]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are warnings part of the input out of curiosity? seems like a smell |
||
| } | ||
|
|
||
| /** | ||
| * Resolves the host preset row, builds an agent launch plan, writes | ||
| * any attachments to the worktree, and spawns the terminal session. | ||
| * Pushes the resulting terminalId+label onto `launches`. Soft-fails: | ||
| * an unknown presetId or empty plan logs a warning and returns | ||
| * without throwing — the workspace is already created and shouldn't | ||
| * unwind on a launch-only failure. | ||
| */ | ||
| async function runAgentLaunch(input: RunAgentLaunchInput): Promise<void> { | ||
| const presetRow = findOrCreateHostPresetByPresetId( | ||
| input.ctx.db, | ||
| input.agentPresetId, | ||
| ); | ||
| if (!presetRow) { | ||
| input.warnings.push(`Unknown agentId: ${input.agentPresetId}`); | ||
| return; | ||
| } | ||
|
|
||
| const attachments = resolveAttachmentFiles(input.attachmentIds); | ||
| const resolveCtx = buildHostResolveCtx({ | ||
| ctx: input.ctx, | ||
| projectId: input.projectId, | ||
| githubIssueUrls: input.githubIssueUrls, | ||
| linkedPrUrl: input.linkedPrUrl, | ||
| }); | ||
|
|
||
| const plan = await buildAgentLaunch({ | ||
| projectId: input.projectId, | ||
| preset: presetRow, | ||
| prompt: input.prompt, | ||
| internalTaskIds: input.internalTaskIds, | ||
| githubIssueUrls: input.githubIssueUrls, | ||
| linkedPrUrl: input.linkedPrUrl, | ||
| attachments, | ||
| resolveCtx, | ||
| }); | ||
| if (!plan) return; | ||
|
|
||
| writeAttachmentsToWorktree(input.worktreePath, plan.attachmentsToWrite); | ||
|
|
||
| const result = await startTerminalLaunch({ | ||
| ctx: input.ctx, | ||
| workspaceId: input.workspaceId, | ||
| plan, | ||
| }); | ||
| if ("error" in result) { | ||
| input.warnings.push(`Failed to start agent terminal: ${result.error}`); | ||
| return; | ||
| } | ||
| input.launches.push({ | ||
| kind: "terminal", | ||
| terminalId: result.terminalId, | ||
| label: result.label, | ||
| }); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no return value from here? |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the preset before seeding defaults.
A typo currently seeds the bundled defaults even though the helper ultimately returns
null. ValidatepresetIdfirst 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