feat(desktop): v2 launch context composition (steps 1-5)#3467
Merged
Conversation
Replace initial draft with V2-greenfield architecture: structured AgentLaunchSpec (system/user/attachments ContentPart[]), per-agent contextPromptTemplate on ResolvedAgentConfig, Uint8Array over IPC, vendor-aligned (AI SDK v3, Anthropic cache_control, Continue.dev contributor metadata). CLI agents keep disk + path-ref pattern; chat agents get structured passthrough with Files API as phase 6.
Step 1 of the v2 launch-context composition plan. Defines the core discriminated types (LaunchSource, ContextSection, LaunchContext, AgentLaunchSpec, ContentPart with Uint8Array data) and the canonical multi-source + prompt-only fixtures that the composer and buildLaunchSpec tests will share. No runtime code yet — types and fixtures only.
…erance Step 2 of the v2 launch-context composition plan. buildLaunchContext parallel-resolves sources via a contributor registry, dedups URL/id-kinds (attachments never dedup), preserves input order within a kind, applies the default kind group order at the end, tolerates per-source failures (populated on failures[]), and enforces a 10s per-contributor timeout. taskSlug derivation: first internal-task section wins, falling back to first github-issue. 12 tests pass.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a V2 workspace context composition system that orchestrates heterogeneous inputs (user prompts, GitHub issues/PRs, internal tasks, file attachments) through a contributor-based architecture, deduplicates sources, resolves sections with timeout handling, and aggregates results with deterministic ordering and failure capture while extending agent definitions to support context prompt templates. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Composer as buildLaunchContext
participant DedupeSort as Dedup & Sort
participant ContribReg as Contributor<br/>Registry
participant Contributors as Contributors<br/>(5 types)
participant ResolveCtx as ResolveCtx<br/>(Fetch Handlers)
App->>Composer: inputs, deps<br/>(sources, contributors,<br/>resolveCtx, timeout)
Composer->>DedupeSort: Deduplicate sources<br/>(by URL, excluding<br/>attachments)
DedupeSort-->>Composer: deduplicated sources
Composer->>Composer: For each source<br/>get kind & resolve
loop For each source
Composer->>ContribReg: Lookup contributor<br/>by source.kind
ContribReg-->>Composer: contributor handler
Composer->>Contributors: resolve(source, ctx)<br/>with timeout wrapper
Contributors->>ResolveCtx: Fetch remote data<br/>(issue, PR, task)
ResolveCtx-->>Contributors: typed content
Contributors->>Contributors: Format & transform<br/>to ContentPart[]
Contributors-->>Composer: ContextSection<br/>or null or error
Composer->>Composer: Collect result<br/>(success or failure)
end
Composer->>DedupeSort: Sort sections<br/>by kindRank
DedupeSort-->>Composer: sorted sections
Composer->>Composer: Derive taskSlug<br/>(first task or issue)
Composer-->>App: LaunchContext<br/>(sources, sections,<br/>failures, taskSlug)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 introduces the V2 context composition layer for the desktop app: a type-safe Key changes:
Confidence Score: 4/5Safe to merge — well-tested, purely additive infrastructure with no runtime paths wired up yet. The composer logic is correct and the test suite is comprehensive (12 cases covering dedup, ordering, failure isolation, timeout, null-drop, taskSlug derivation). The two P2s — returning un-deduped sources in the output and the AbortSignal not being honoured at the outer Promise.all level — are real footguns for future consumers but won't cause failures today since no production caller exists yet. The plan file placement is a policy nit. None of these are blockers. apps/desktop/src/shared/context/composer.ts — the sources/deduped inconsistency and missing abort-signal integration are worth resolving before callers are wired up. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["BuildLaunchContextInputs\n(projectId, sources[], agent)"] --> B["dedupeSources()\nkey: kind + url/id/path\nattachments: never dedup"]
B --> C["Promise.all — resolveOne() per source\n(concurrent, up to timeoutMs each)"]
C --> D{"ResolveResult per source"}
D -->|"kind: section, section != null"| E["sections[]"]
D -->|"kind: section, section == null"| F["silently dropped"]
D -->|"kind: error (throw or timeout)"| G["failures[]"]
E --> H["sections.sort() by KIND_ORDER\nuser-prompt → internal-task → github-issue\n→ github-pr → attachment → agent-instructions"]
H --> I["deriveTaskSlug(sections)\n1st internal-task → 1st github-issue → undefined"]
I --> J["LaunchContext\n(projectId, sources, sections, failures, taskSlug, agent)"]
G --> J
subgraph resolveOne["resolveOne()"]
R1["contributors[source.kind].resolve(source, resolveCtx)"]
R1 -->|"Promise.race"| R2["withTimeout(promise, timeoutMs)\nclears timer in .finally()"]
end
C --> resolveOne
Reviews (1): Last reviewed commit: "feat(desktop/context): add composer with..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+59
to
+66
| return { | ||
| projectId: inputs.projectId, | ||
| sources: inputs.sources, | ||
| sections, | ||
| failures, | ||
| taskSlug: deriveTaskSlug(sections), | ||
| agent: inputs.agent, | ||
| }; |
There was a problem hiding this comment.
sources returns un-deduped input while sections/failures reflect deduped processing
sources: inputs.sources is returned as-is (potentially containing duplicate entries), but sections and failures are both built from deduped. A consumer that tries to reconcile sources against sections + failures (e.g. to show a "resolved / failed / skipped" breakdown in the UI) will see a count mismatch whenever the input had duplicates — the duplicates simply disappear without appearing in either sections or failures.
Consider returning deduped instead:
Suggested change
| return { | |
| projectId: inputs.projectId, | |
| sources: inputs.sources, | |
| sections, | |
| failures, | |
| taskSlug: deriveTaskSlug(sections), | |
| agent: inputs.agent, | |
| }; | |
| return { | |
| projectId: inputs.projectId, | |
| sources: deduped, | |
| sections, | |
| failures, | |
| taskSlug: deriveTaskSlug(sections), | |
| agent: inputs.agent, | |
| }; |
If preserving the original user-intent list is important, a separate rawSources / dedupedSources field would make the split explicit rather than silent.
| @@ -0,0 +1,243 @@ | |||
| # V2 Workspace Launch Context — Composition | |||
There was a problem hiding this comment.
plans/done/ per AGENTS.md
Per the repo's agent rules, "shipped plans move to plans/done/". This plan covers Gaps 3–5 (types, composer, fixtures), all of which are implemented in this PR. Placing the plan in plans/ rather than plans/done/ leaves it looking like work still in progress and makes the plans/ directory harder to scan for active work.
Step 3 of the v2 launch-context composition plan. One contributor per LaunchSource kind, each with Continue.dev-style metadata (displayName, description, requiresQuery), its own co-located test file, and a consistent 404 -> null (non-fatal) pattern for fetch-based kinds: - userPrompt -- trims, returns null on empty - attachment -- file or image ContentPart by mediaType - agentInstructions -- system-scoped, cacheControl: ephemeral - githubIssue -- title + body markdown, meta.taskSlug from slug - githubPr -- title + branch + body markdown - internalTask -- title + description, meta.taskSlug Also adds composer.integration.test.ts covering the real registry end-to-end against the multi-source fixture. 41 tests green.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![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
🧹 Nitpick comments (5)
apps/desktop/src/shared/context/composer.test.ts (1)
120-149: Add dedupe coverage for the other keyed source kinds.This only protects
github-issue, but the launch contract dedupes keyed URL/id sources more broadly.github-prandinternal-taskcan still regress without a failing test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/context/composer.test.ts` around lines 120 - 149, Add tests mirroring the existing "dedups github-issue sources by url before dispatch" case for the other keyed source kinds so regressions are caught; specifically, create two additional tests (or extend the test) that call buildLaunchContext with duplicate sources for "github-pr" and "internal-task", register contributors via the same registry(...) pattern using makeContributor("github-pr", ...) and makeContributor("internal-task", ...) that increment a local calls counter, then assert calls === 1 and ctx.sections length is 1 for each kind to verify deduplication; use the same structure and assertions as the existing test so you exercise the launch contract dedupe logic for those keyed kinds.apps/desktop/src/shared/context/__fixtures__/launchContext.multi-source.ts (2)
32-85: Build section labels from the imported fixture fields.These labels retype the issue/task/PR numbers and titles that already exist in the source fixtures, so this canonical expected output can drift silently when fixture data changes.
♻️ Proposed cleanup
- label: "Task TASK-42 — Refactor auth middleware", + label: `Task ${internalTaskRefactorAuth.id} — ${internalTaskRefactorAuth.title}`, ... - label: "Issue `#123` — Auth middleware stores tokens in plaintext", + label: `Issue #${githubIssueAuthMiddleware.number} — ${githubIssueAuthMiddleware.title}`, ... - label: "Issue `#124` — Rotate session tokens on password change", + label: `Issue #${githubIssueTokenRotation.number} — ${githubIssueTokenRotation.title}`, ... - label: "PR `#200` — Rewrite auth middleware", + label: `PR #${githubPrAuthRewrite.number} — ${githubPrAuthRewrite.title}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/context/__fixtures__/launchContext.multi-source.ts` around lines 32 - 85, The fixture currently hardcodes section labels (e.g., "Task TASK-42 — Refactor auth middleware", "Issue `#123` — Auth middleware stores tokens in plaintext", "PR `#200` — Rewrite auth middleware"); update these to derive their labels from the imported fixture objects instead of duplicating values so they stay canonical when source fixtures change—use the existing variables internalTaskRefactorAuth, githubIssueAuthMiddleware, githubIssueTokenRotation, and githubPrAuthRewrite and build labels from their fields (e.g., use internalTaskRefactorAuth.slug or .title, githubIssueAuthMiddleware.number and .title, githubPrAuthRewrite.number/.title) when constructing the objects with ids like `task:${...}`, `issue:${...}`, and `pr:${...}`. Ensure every label string concatenation references the fixture fields rather than hardcoded text so the fixture stays in sync.
114-123: Make theagent-instructionsfixture look like an actualAGENTS.md.This sample currently models a tooling preference, not agent responsibilities/capabilities/interaction patterns. Since it is the canonical
agent-instructionsexample, the content should mirror the shape consumers are expected to handle.Based on learnings, "Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/context/__fixtures__/launchContext.multi-source.ts` around lines 114 - 123, The fixture object with id "agent-instructions:/worktree/AGENTS.md" and kind "agent-instructions" currently contains a tooling preference instead of real agent guidance; replace the content array so AGENTS.md models agent responsibilities, capabilities, and interaction patterns (e.g., sections for Purpose, Responsibilities, Capabilities, Limits, Typical Interaction Examples and Example Prompts) so consumers of the "agent-instructions" kind can parse and render expected fields; update the content property (the object with type "text") to contain those sections in markdown form and keep cacheControl "ephemeral" and label "AGENTS.md" unchanged.apps/desktop/src/shared/context/composer.ts (1)
101-112: Timeout handling should cancel contributor work, not only race it.
Promise.racehere returns early but does not stop in-flight contributor operations, which can continue consuming resources after timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/context/composer.ts` around lines 101 - 112, The withTimeout function currently uses Promise.race which doesn't stop in-flight work; change it to create an AbortController and race the original promise against a timeout that calls controller.abort() on expiry, pass the controller.signal into contributor operations (making contributor functions accept an AbortSignal), and reject with a clear AbortError message; ensure the timer is cleared in finally and that withTimeout returns the original promise (or a wrapper) that respects signal-based cancellation so contributors can clean up when aborted.apps/desktop/src/shared/context/types.ts (1)
147-152: DefineAttachmentParttype to enforce file/image-only invariant.
AgentLaunchSpec.attachmentscurrently usesContentPart[], which permits text content despite this field being intended exclusively for files and images. Create aAttachmentParttype and narrow the field to strengthen the type system:Suggested implementation
export type ContentPart = | { type: "text"; text: string } | { type: "file"; data: Uint8Array; mediaType: string; filename?: string; } | { type: "image"; data: Uint8Array; mediaType: string }; + +export type AttachmentPart = Extract<ContentPart, { type: "file" | "image" }>; ... export interface AgentLaunchSpec { agentId: AgentDefinitionId; system: ContentPart[]; user: ContentPart[]; - attachments: ContentPart[]; + attachments: AttachmentPart[]; taskSlug?: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/context/types.ts` around lines 147 - 152, Add a new AttachmentPart type that narrows ContentPart to only the file/image variants (e.g., enforce kind/type/mediadata fields used for files/images) and replace AgentLaunchSpec.attachments: ContentPart[] with attachments: AttachmentPart[]; update any code creating or consuming AgentLaunchSpec (constructors, factories, serializers) to build/accept AttachmentPart instances or convert/validate ContentPart -> AttachmentPart where appropriate so the type invariant (files/images only) is enforced across AgentLaunchSpec usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/shared/context/composer.ts`:
- Around line 125-127: The "user-prompt" case in the identity switch in
composer.ts currently returns the constant "user-prompt", collapsing all
distinct prompts; change that branch to return a unique identity derived from
the prompt payload (e.g., use an existing message.id, payload.promptId,
prompt.content hash or timestamp) instead of the constant string so each user
prompt remains distinct; update the "user-prompt" case to build and return a
stable unique key (like `user-prompt:{id}` or a content-based hash) and fall
back to a generated UUID only if no identifying fields exist.
- Around line 88-90: Remove the unsafe cast "source as any" when calling
contributor.resolve; instead implement a typed helper (e.g., resolveByKind) that
does a discriminated switch on source.kind and calls contributor.resolve with
the correctly-typed source for each case, passing resolveCtx and timeoutMs
through; replace the direct call in Composer (the call site using
contributor.resolve(..., resolveCtx)) with a call to resolveByKind(source,
contributor, resolveCtx, timeoutMs) so the compiler can verify
source/contributor compatibility without using any.
In `@plans/v2-workspace-context-composition.md`:
- Around line 31-33: The markdown has untyped fenced code blocks (e.g., the
sequence "Inputs → resolve sources → LaunchContext → buildLaunchSpec →
executeAgentLaunch" and the file-list block showing
"apps/desktop/src/shared/context/ ...
consumers/{branchName,preview,createFromPr}.ts") which trigger markdownlint
MD040; update each triple-backtick fence to include a language identifier (e.g.,
```text) for those blocks (lines around the sequence and the file-list block
referenced in the comment) so all fenced code blocks are typed and linting
errors are resolved.
- Line 96: Update the spec text that currently reads “Dedup by `source.id`
pre-dispatch” to describe deduplication by a kind-specific source identity
instead of only `source.id`; e.g., state that pre-dispatch deduplication should
use the canonical identity derived from the source kind (examples: `source.id`
when available, GitHub repo/PR URL for GitHub sources, instructions file path
for local instructions, etc.), and add a short note listing these example
identity types so implementers use the appropriate canonical key for each source
kind.
---
Nitpick comments:
In `@apps/desktop/src/shared/context/__fixtures__/launchContext.multi-source.ts`:
- Around line 32-85: The fixture currently hardcodes section labels (e.g., "Task
TASK-42 — Refactor auth middleware", "Issue `#123` — Auth middleware stores tokens
in plaintext", "PR `#200` — Rewrite auth middleware"); update these to derive
their labels from the imported fixture objects instead of duplicating values so
they stay canonical when source fixtures change—use the existing variables
internalTaskRefactorAuth, githubIssueAuthMiddleware, githubIssueTokenRotation,
and githubPrAuthRewrite and build labels from their fields (e.g., use
internalTaskRefactorAuth.slug or .title, githubIssueAuthMiddleware.number and
.title, githubPrAuthRewrite.number/.title) when constructing the objects with
ids like `task:${...}`, `issue:${...}`, and `pr:${...}`. Ensure every label
string concatenation references the fixture fields rather than hardcoded text so
the fixture stays in sync.
- Around line 114-123: The fixture object with id
"agent-instructions:/worktree/AGENTS.md" and kind "agent-instructions" currently
contains a tooling preference instead of real agent guidance; replace the
content array so AGENTS.md models agent responsibilities, capabilities, and
interaction patterns (e.g., sections for Purpose, Responsibilities,
Capabilities, Limits, Typical Interaction Examples and Example Prompts) so
consumers of the "agent-instructions" kind can parse and render expected fields;
update the content property (the object with type "text") to contain those
sections in markdown form and keep cacheControl "ephemeral" and label
"AGENTS.md" unchanged.
In `@apps/desktop/src/shared/context/composer.test.ts`:
- Around line 120-149: Add tests mirroring the existing "dedups github-issue
sources by url before dispatch" case for the other keyed source kinds so
regressions are caught; specifically, create two additional tests (or extend the
test) that call buildLaunchContext with duplicate sources for "github-pr" and
"internal-task", register contributors via the same registry(...) pattern using
makeContributor("github-pr", ...) and makeContributor("internal-task", ...) that
increment a local calls counter, then assert calls === 1 and ctx.sections length
is 1 for each kind to verify deduplication; use the same structure and
assertions as the existing test so you exercise the launch contract dedupe logic
for those keyed kinds.
In `@apps/desktop/src/shared/context/composer.ts`:
- Around line 101-112: The withTimeout function currently uses Promise.race
which doesn't stop in-flight work; change it to create an AbortController and
race the original promise against a timeout that calls controller.abort() on
expiry, pass the controller.signal into contributor operations (making
contributor functions accept an AbortSignal), and reject with a clear AbortError
message; ensure the timer is cleared in finally and that withTimeout returns the
original promise (or a wrapper) that respects signal-based cancellation so
contributors can clean up when aborted.
In `@apps/desktop/src/shared/context/types.ts`:
- Around line 147-152: Add a new AttachmentPart type that narrows ContentPart to
only the file/image variants (e.g., enforce kind/type/mediadata fields used for
files/images) and replace AgentLaunchSpec.attachments: ContentPart[] with
attachments: AttachmentPart[]; update any code creating or consuming
AgentLaunchSpec (constructors, factories, serializers) to build/accept
AttachmentPart instances or convert/validate ContentPart -> AttachmentPart where
appropriate so the type invariant (files/images only) is enforced across
AgentLaunchSpec usage.
🪄 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: 04c65f1f-856a-4d09-8888-c6c1d2c24514
📒 Files selected for processing (11)
apps/desktop/src/shared/context/__fixtures__/attachment.logs-txt.tsapps/desktop/src/shared/context/__fixtures__/githubIssue.auth-middleware.tsapps/desktop/src/shared/context/__fixtures__/githubPr.auth-rewrite.tsapps/desktop/src/shared/context/__fixtures__/index.tsapps/desktop/src/shared/context/__fixtures__/internalTask.refactor-auth.tsapps/desktop/src/shared/context/__fixtures__/launchContext.multi-source.tsapps/desktop/src/shared/context/__fixtures__/launchContext.prompt-only.tsapps/desktop/src/shared/context/composer.test.tsapps/desktop/src/shared/context/composer.tsapps/desktop/src/shared/context/types.tsplans/v2-workspace-context-composition.md
Step 4 of the v2 launch-context composition plan. - Extract renderPromptTemplate(template, Record<string, string>) as the generic primitive; existing renderTaskPromptTemplate is now a shim (same API, same behavior — callers unchanged). - Add AGENT_CONTEXT_PROMPT_VARIABLES (userPrompt, tasks, issues, prs, attachments, agentInstructions) + getSupportedContextPromptVariables + validateContextPromptTemplate. - Ship default context templates for markdown (codex/cursor/custom) and Claude (XML-wrapped user-request) — both for system + user scopes. - Collapse runs of 3+ newlines to 2 so empty variables produce clean output. Empty-string values substitute in (not treated as missing). 16 tests green; no consumer breakage.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/shared/context/contributors/githubIssue.ts`:
- Around line 21-23: The untyped variable "issue" should be explicitly typed as
GitHubIssueContent to avoid implicit any; replace the declaration "let issue;"
with a typed declaration (e.g., "let issue: GitHubIssueContent;") immediately
before awaiting ctx.fetchIssue(source.url) in the code that calls fetchIssue,
and apply the same change pattern to the analogous variables in internalTask.ts
and githubPr.ts so each variable using the ResolveCtx.fetchIssue/resolve return
types is strongly typed rather than implicit any.
In `@apps/desktop/src/shared/context/contributors/githubPr.ts`:
- Around line 21-23: Declare the variables with explicit types instead of
implicit any: change "let pr;" to "let pr: ReturnType<typeof
ctx.fetchPullRequest> | undefined" or, preferably, the concrete PullRequest type
imported from your GitHub types (e.g., PullRequest | undefined) and update the
try/catch usage accordingly; do the analogous fixes in githubIssue.ts (change
"let issue;" to an explicit Issue type matching ctx.fetchIssue or your Issue
model) and internalTask.ts (change "let task;" to the explicit Task type used by
ctx.fetchTask/internal task model). Ensure you import the correct type symbols
and use them in the variable declarations where ctx.fetchPullRequest,
ctx.fetchIssue, and ctx.fetchTask are used.
In `@apps/desktop/src/shared/context/contributors/internalTask.ts`:
- Around line 21-23: The variable task is declared without a type (implicit
any); update its declaration to an explicit type to restore type safety: replace
the current untyped let task; with a typed declaration using the known return
type of ctx.fetchInternalTask—either let task: InternalTaskContent; (importing
InternalTaskContent from types.ts if needed) or let task:
Awaited<ReturnType<typeof ctx.fetchInternalTask>>;—so subsequent property access
on task (e.g., .id, .title, .slug, .description) is correctly typed.
🪄 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: 2661d5df-9a1f-4627-bf28-1a28093807c4
📒 Files selected for processing (14)
apps/desktop/src/shared/context/composer.integration.test.tsapps/desktop/src/shared/context/contributors/agentInstructions.test.tsapps/desktop/src/shared/context/contributors/agentInstructions.tsapps/desktop/src/shared/context/contributors/attachment.test.tsapps/desktop/src/shared/context/contributors/attachment.tsapps/desktop/src/shared/context/contributors/githubIssue.test.tsapps/desktop/src/shared/context/contributors/githubIssue.tsapps/desktop/src/shared/context/contributors/githubPr.test.tsapps/desktop/src/shared/context/contributors/githubPr.tsapps/desktop/src/shared/context/contributors/index.tsapps/desktop/src/shared/context/contributors/internalTask.test.tsapps/desktop/src/shared/context/contributors/internalTask.tsapps/desktop/src/shared/context/contributors/userPrompt.test.tsapps/desktop/src/shared/context/contributors/userPrompt.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/shared/context/contributors/internalTask.test.ts
Step 5 of the v2 launch-context composition plan. Extends the agent config surface so V2 launches can render structured context into per-agent system/user templates: - packages/shared/agent-definition: required contextPromptTemplateSystem and contextPromptTemplateUser fields on BaseAgentDefinition; createTerminalAgentDefinition fills defaults with the markdown templates from step 4. - packages/shared/builtin-terminal-agents: Claude terminal ships the Claude-XML defaults; other builtins inherit markdown defaults. - packages/shared/agent-catalog: BUILTIN_CHAT_AGENT (Claude-based superset-chat) ships the Claude-XML defaults. - packages/local-db/schema/zod: add both fields to AGENT_PRESET_FIELDS, agentPresetOverrideSchema, agentCustomDefinitionSchema (optional). - apps/desktop/shared/utils/agent-settings: thread through TERMINAL_OVERRIDE_FIELDS, CHAT_OVERRIDE_FIELDS, AgentPresetPatch, CustomAgentDefinitionPatch, resolveAgentConfig (both branches), applyCustomAgentDefinitionPatch, createOverrideEnvelopeWithPatch. - apps/desktop/test-setup: update the mocked @superset/local-db schema (the Bun test workaround for drizzle-orm/sqlite-core) so tests see the same shape as runtime. New tests: contextPromptTemplate resolution for claude terminal, codex markdown defaults, superset-chat claude defaults, terminal and chat override replacement, custom terminal fallback to markdown. 113 tests green across context + agent-settings suites.
Post-phase-1 reference: what shipped, manual + automated test plan, known gaps, prioritized follow-ups, and a file-layout map. Lives in apps/desktop/docs/ per AGENTS.md rule 7 (architecture docs). The original plan stays in plans/ since phases 2-6 are still unshipped.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
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/pending/$pendingId/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx:110">
P2: The V2 fork launch enqueue is incorrectly gated on `agentPresetsQuery.data` being preloaded, which can race and permanently skip enqueue for successful workspace creation.</violation>
</file>
<file name="apps/desktop/docs/V2_LAUNCH_CONTEXT.md">
<violation number="1" location="apps/desktop/docs/V2_LAUNCH_CONTEXT.md:10">
P3: The phase-status summary contradicts the later “Known phase 1 gaps” section about Gap 4, so the document currently reports an incorrect completion state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Temporary logs for manual testing: - pending page: what buildForkAgentLaunch returned + enqueue inputs. - useEnqueueAgentLaunch: stash / null-short-circuit. - WorkspaceInitEffects: every handleTerminalSetup + dispatch branch, launchAgentViaOrchestrator invocation. Grep devtools console on "[v2-launch]" to trace a full submit. Remove or soften once the dispatch path is dialed in.
Contributor
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
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/screens/main/components/WorkspaceInitEffects.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx:153">
P3: Remove this verbose debug `console.log`; it will spam runtime logs in a hot setup flow and is not tied to an error path.
(Based on your team's feedback about keeping console/debug logging lean in frequently executed paths.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/hooks/useEnqueueAgentLaunch/useEnqueueAgentLaunch.ts">
<violation number="1" location="apps/desktop/src/renderer/hooks/useEnqueueAgentLaunch/useEnqueueAgentLaunch.ts:57">
P2: Remove the unconditional debug `console.log` from this hot path to avoid noisy runtime logs.
(Based on your team's feedback about removing excessive debug logs.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
V2 must own its own launch dispatch. V1's WorkspaceInitEffects → orchestrator → terminal-adapter path writes panes into V1's useTabsStore, which V2 doesn't render from, so launches dispatched through V1 land invisibly for V2 workspaces. Documents the replacement: pending-row-as-bus. Pending page produces terminalLaunch / chatLaunch fields on the collection-backed pending row; V2 workspace page mount-effect consumes them, opens a pane in the @superset/panes store, and wires PTY via workspaceTrpc. This mirrors the pattern V2 preset execution already uses (useV2PresetExecution): live-query a record, open a pane, call workspaceTrpc.terminal.ensureSession. Zero V1 primitives, zero new host-service work, and leaves a clean migration path to host-owned terminal launch when phase 5 ships. Adds a blocking follow-up (#0) for the dispatch rewrite; marks useEnqueueAgentLaunch + buildAgentLaunchRequest for removal.
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
The original step-8/9 wire-up stashed an AgentLaunchRequest in V1's useWorkspaceInitStore, expecting V1's WorkspaceInitEffects to dispatch. V1's orchestrator writes panes into useTabsStore — which V2 never renders from — so launches landed invisibly for V2 workspaces. This rewrite keeps V2 self-contained. After host-service.create resolves, the pending page runs the composer pipeline and stashes a terminalLaunch or chatLaunch on the pending row. The V2 workspace page's new useConsumePendingLaunch mount-effect live-queries that row, opens a pane in @superset/panes, and drives PTY via workspaceTrpc. Same pattern as useV2PresetExecution. Changes: - Schema: pendingWorkspaceSchema gains optional terminalLaunch and chatLaunch fields, cleared to null once consumed. - buildForkAgentLaunch returns a PendingLaunchBuild union (terminal with attachmentsToWrite / chat with inline initialFiles) instead of the V1 AgentLaunchRequest shape. - dispatchForkLaunch: new pending-page helper that runs the composer, writes attachments to .superset/attachments/ via workspaceTrpc .filesystem.writeFile for the terminal path, and applies the launch field to the pending row. - useConsumePendingLaunch: new V2-workspace-page mount effect. Reads row by workspaceId, opens pane in V2 store, calls workspaceTrpc .terminal.ensureSession with initialCommand for terminal launches, clears the field. - ChatPaneData gains a transient launchConfig slot. ChatPane and WorkspaceChatInterface thread initialLaunchConfig + onConsumeLaunchConfig through. After the V2 chat runtime auto-sends the initial message, it clears the pane's launchConfig. - Rip out useEnqueueAgentLaunch hook, buildAgentLaunchRequest, and the debug logs in WorkspaceInitEffects. 23 tests green for buildForkAgentLaunch / buildLaunchSourcesFromPending; type-check clean in the touched surface area. See apps/desktop/docs/V2_LAUNCH_CONTEXT.md "Dispatch architecture".
Structured manual test checklist for the V2 launch dispatch pipeline: terminal + chat happy paths, pending-row lifecycle, failure paths, source-mapping edge cases, custom agents, cross-pane behavior, V1 regression. Paired with copy-pasteable fixtures on ~/Desktop/v2-launch-test-artifacts/ (trace.log, notes.md, sample.png, prompts.txt, README) for drag-and- drop testing.
Logs the blob/data URLs we get from the PromptInput provider at submit time, then does a fetch() probe on each URL before storeAttachments runs. Lets us see whether the URL is already dead when useSubmitWorkspace fires — which would confirm a pre-submit revocation (as opposed to a race inside storeAttachments itself). Not a fix. Remove once the root cause is nailed down.
Root cause of the "Failed to fetch" attachment toast: the
PromptInput library calls clearComposer() before invoking onSubmit,
which revokes all blob: URLs stored in the provider. Our
useSubmitWorkspace was reading attachments back from the provider
via takeFiles() after that — so it got file entries whose URLs had
just been invalidated.
The library already does the blob→data-URL conversion itself and
passes the converted files into onSubmit's message arg. Use them
directly:
- useSubmitWorkspace now takes `files: SubmitAttachment[]` as an
explicit argument. Drops the `useProviderAttachments()` dependency.
- handlePromptSubmit receives `{text, files}` from PromptInput and
forwards the files.
- The existing Cmd+Enter keyboard fallback calls handleCreate()
without files (unchanged behavior for the no-attachments path; the
PromptInput's own Enter handler takes the file-carrying path).
The prior hand-rolled IDB wrapper had two transaction-lifecycle bugs:
1. storeAttachments opened a readwrite transaction, then awaited
fetch() on each file before calling store.put() — IDB auto-commits
when the event loop yields with no pending requests, so the first
put() fired against a finished transaction ("The transaction has
finished.").
2. The same file (150+ lines of raw IDB callback plumbing) is exactly
the shape of code where this class of bug keeps reappearing as
the flow evolves.
Swap to Dexie 4 — the de-facto IndexedDB wrapper for apps (~11.9k⭐,
actively maintained, typed, handles transaction lifecycle correctly).
- storeAttachments: resolve blobs async outside any tx, then
bulkPut() in one shot.
- loadAttachments / clearAttachments: where("key").startsWith(prefix).
- File collapses from ~150 to ~90 lines, no raw transactions, no
cursor dance.
Behavior is identical from the caller's side. Schema version 1;
Dexie will open the existing database transparently (same DB name).
Traces: - dispatchForkLaunch start / built / chatLaunch-applied / terminalLaunch-applied - useConsumePendingLaunch tick (live-query fires) + whether terminalKey / chatKey are already consumed - consumeTerminalLaunch ensureSession + addTab + clear - consumeChatLaunch addTab + clear Grep devtools on "[v2-launch]" through the full submit -> open-workspace flow. Lets us pin where dispatch stalls when no pane appears. Temporary — remove once the end-to-end flow is nailed down.
Electron renderer doesn't expose Node's `Buffer` global (nodeIntegration
off). The fork-launch dispatch path and buildForkAgentLaunch were both
using `Buffer.from(...).toString("base64")` / `Buffer.from(base64, "base64")`
for binary <-> base64 conversion, which ReferenceError'd at runtime.
Swap to standards-based `btoa` / `atob` + a small byte <-> binary-string
helper. Works in renderer and Bun alike.
Applies to:
- dataUrlAttachmentToBytes (buildForkAgentLaunch.ts) — decode
attachment data URL into Uint8Array.
- toBase64DataUrl (buildForkAgentLaunch.ts) — encode chat-bound files
for ChatLaunchConfig.initialFiles.
- writeAttachmentsToWorktree (dispatchForkLaunch.ts) — encode bytes
for host-service filesystem.writeFile's base64 content variant.
Seven items we caught during manual testing and intentionally deferred: 1. Deep solve for binary transport (blob URL / base64 fragility) 2. Reload-mid-launch spawns duplicate PTY (key terminalId off pending row) 3. Silent failure in consume hook — add toast 4. joinPath assumes POSIX — breaks for Windows hosts (phase 5) 5. Dexie schema coupling with pre-existing IDB store 6. PendingTerminalLaunch.attachmentNames unused by consumer 7. Remove [v2-launch] debug logs once flow is stable Tracked in V2_LAUNCH_CONTEXT.md "Known footguns to revisit". None are blocking phase-1 behavior; all have notes on the proper fix.
Seven silent swallow points across the launch path now surface a toast so the user knows why the agent didn't auto-launch instead of seeing "nothing happened": - dispatchForkLaunch: buildForkAgentLaunch throw -> "Couldn't prepare agent launch" (description = error message). - dispatchForkLaunch: buildForkAgentLaunch returned null AND user gave meaningful input -> warning "Workspace created but no agent launched" with hint to enable one in settings. Silent for the "fresh empty workspace, no agent configured" case (expected). - dispatchForkLaunch: host-service URL not resolved -> "Couldn't reach host service". - dispatchForkLaunch: writeAttachmentsToWorktree throw -> warning "Attachments didn't save to the workspace; agent will launch without files". - writeAttachmentsToWorktree: missing worktreePath -> throw instead of silent return so the outer catch's toast fires. - consumeTerminalLaunch: defensive bail -> "Couldn't open agent pane" (shouldn't happen, but defensive). - consumeTerminalLaunch: ensureSession throw -> "Couldn't start agent terminal" with error message. - pending page: loadAttachments throw in fork intent -> warning "Couldn't load saved attachments" (non-fatal, workspace still creates). All keep their [v2-launch] console.warn/log so trace survives alongside the toast.
Addresses the non-stale, non-debatable feedback from review bots:
- Prototype-chain substitution in prompt templates (agent-prompt-
template.ts + buildLaunchSpec.ts): {{toString}} and similar now
stay intact. Use Object.hasOwn() instead of `variables[key] ??`.
- renderTaskPromptTemplate no longer picks up generic 3+-newline
collapsing — task-flow output matches V1 exactly: own-property
substitution + trim only.
- buildLaunchSpec.renderUserTemplate tolerates whitespace in the
placeholder: {{ userPrompt }} / {{userPrompt}} / {{ userPrompt }}
all match.
- Pending page's fork dispatch fetches agent configs imperatively
via trpcUtils.settings.getAgentPresets.fetch() instead of reading
from a useQuery hook — eliminates the race where a not-yet-
resolved query silently skipped the dispatch and lost the launch
for a successful workspace create.
- Drop ContextSection.scope field. It was never read (buildLaunchSpec
ignored it); no contributor populated anything but "user" after we
removed agent-instructions. Cleaner type + future re-introduction
when a real system-scope consumer lands (phase 6 host-side
instructions injection).
Tests: 54 context-suite passing, 14 shared-suite passing; desktop
typecheck clean in touched areas.
Claude currently sees title-only for linked issues / PRs / tasks — no bodies. Documents the gap, what V1 did (Electron IPC to projects.getIssueContent), why we can't reuse it for V2 (no Electron in V2 rule), and proposes the host-service procedures + stub swap. Also covers: - Empty `Branch:` in PR block — pending-row schema doesn't carry branch; fix via getPullRequestContent body fetch. - Sanitization helpers to extract from V1 into a shared util. - Attach-as-file vs inline-in-prompt decision (V1 attached, current V2 inlines — keeping inline for phase 1). Ordered work plan at the bottom: getIssueContent first, then PR, then internal-task (requires scoping). Acceptance criteria shows the expected prompt shape after the fixes land.
…rite
Prompt refinements from manual testing:
- buildLaunchSpec {{attachments}} block now includes a short framing
header: "Attached files — read them to understand the request."
Cues the agent to actually use the files rather than treating them
as passive metadata. Only appears when there are files.
- githubPr contributor says "Branch `X` is checked out in this
workspace — commits you make continue this PR." Confirms to the
agent that the worktree is on the PR's branch, so it shouldn't
create a new branch or open a new PR.
- V2_LAUNCH_CONTEXT_GAPS.md rewritten with locked design decisions:
bodies inline in prompt (no file writes for linked context), no
truncation, no sanitization, PR checkout is true. Work plan:
host-service getIssueContent → getPullRequestContent → task body
API → swap stubs. Target prompt shape included.
54 tests green; 2 snapshots updated for new PR format.
Agents shouldn't guess whether a section is a task, issue, or PR from context clues. Each contributor now prefixes its heading with the kind: - `# GitHub Issue #123 — Auth middleware stores tokens in plaintext` - `# Task TASK-42 — Refactor auth middleware` - `# PR #200 — Rewrite auth middleware` PR phrasing also clarified: "This PR is checked out in this workspace on branch `fix/auth-encryption`. Commits you make here will be added to this PR." 54 tests green; 2 snapshots updated.
…cloud API The launch prompt now includes full bodies for linked GitHub issues, PRs, and internal tasks instead of title-only stubs. Host-service (packages/host-service): - getGitHubPullRequestContent: new procedure wrapping octokit.pulls.get. Returns body, branch, baseBranch, author, isDraft, timestamps. (getGitHubIssueContent already existed.) Client (apps/desktop pending page): - buildForkAgentLaunch accepts an optional hostServiceClient. When provided, the issue + PR resolvers call getGitHubIssueContent / getGitHubPullRequestContent for full bodies. Falls back to pending-row title-only if the call fails (non-fatal). - Task resolver calls apiTrpcClient.task.byId (Superset cloud API, same source as the task view) for description. Falls back to title-only on failure. - dispatchForkLaunch threads the host-service client through. Contributors (already landed earlier this session): - GitHub issue header: `# GitHub Issue #N — Title` - PR header: `# PR #N — Title` + "This PR is checked out in this workspace on branch `X`. Commits you make here will be added to this PR." - Task header: `# Task ID — Title` - Attachments block: framing header cueing the agent to read the files. 77 tests green. Typecheck clean.
- internalTask.test.ts: replace `TASK.description!` non-null assertion with `if (TASK.description)` guard (biome lint/style/noNonNullAssertion). - buildForkAgentLaunch.ts: update stale docstring that claimed bodies aren't fetched yet — they are, via host-service and the cloud task API. 77 tests green, biome clean, typecheck clean.
host-service's octokit path needs a GitHub token from
providers.credentials.getToken("github.com") — which most users don't
have set up (requires GITHUB_TOKEN env or git credential helper config
for github.com). Result: getGitHubIssueContent / getGitHubPullRequestContent
silently 500'd, buildForkAgentLaunch fell back to title-only, and the
agent received empty bodies for linked issues/PRs.
V1's projects.getIssueContent shells out to `gh issue view` via the
user's `gh auth login` — that works out of the box.
Port the same approach:
- New packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts
— promisified execFile("gh", ...) with user shell env so PATH
resolves on macOS GUI contexts.
- getGitHubIssueContent now calls `gh issue view <n> --repo owner/name
--json number,title,body,url,state,author,createdAt,updatedAt`.
- getGitHubPullRequestContent calls `gh pr view <n> --repo owner/name
--json number,title,body,url,state,author,headRefName,baseRefName,isDraft,...`.
- Zod-validate the JSON output before returning.
- Normalize state to lowercase (gh returns "OPEN"/"CLOSED" uppercase).
Drops the Octokit dependency on these two procedures. Other host-service
paths that still use ctx.github() unchanged.
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
Groundwork for V2 workspace-launch context composition (closes Gaps 3, 4, 5 in
apps/desktop/V2_WORKSPACE_MODAL_GAPS.md). Plan:plans/v2-workspace-context-composition.md.Ships steps 1–5 of the plan — the agent-agnostic composer, six contributors, new prompt-template primitives, and the
ResolvedAgentConfig.contextPromptTemplateextension. No UI yet; no consumer wires this in. That's step 6+.apps/desktop/src/shared/context/types.ts):LaunchSourcediscriminated union,ContextSection(withscope: "system"|"user"+cacheControl),ContentPartwithUint8Arraydata (AI SDK v3 aligned, no base64 internally),AgentLaunchSpecreplacing V1's flatinitialPrompt: stringboundary.composer.ts): parallel source resolution, dedup by kind identity (attachments never dedup), default kind group order,failures[]surfaced explicitly, 10s per-contributor timeout.displayName/description/requiresQuery). Fetch-based kinds (issue/PR/task) all use404 → nullnon-fatal pattern.renderPromptTemplate+ newAGENT_CONTEXT_PROMPT_VARIABLES(userPrompt/tasks/issues/prs/attachments/agentInstructions) + default markdown and Claude-XML templates.renderTaskPromptTemplatepreserved as a shim — no task-flow regression.ResolvedAgentConfig.contextPromptTemplate{System,User}threaded throughagent-definition.ts,agent-catalog.ts,builtin-terminal-agents.ts, Zod schemas, andagent-settings.tsresolver/override/patch.V1 agent-launch code is untouched. V2 duplicates the pending-setup store in step 8 (separate spec shape).
Test plan
bun test src/shared/context/— 41 green (composer dedup/ordering/failure, 6 contributors, integration end-to-end with default registry)bun test src/shared/utils/agent-settings.test.ts— 15 green (including 6 new forcontextPromptTemplateresolution on terminal / chat / custom agents)bun test packages/shared/src/agent-prompt-template.test.ts— 16 greentsc --noEmitclean for all touched areas (pre-existing route-tree errors remain; unrelated)Follow-up (steps 6–11)
buildLaunchSpec(ctx, agentConfig)— render per-kind markdown sub-blocks + fill agent template intoAgentLaunchSpec.executeAgentLaunch— chat structured passthrough; terminal flattens via existingbuildPromptCommandFromAgentConfig+ writes attachments via newfilesystem.writeFile({kind: "bytes"}).useEnqueueAgentLaunchhook (V2-owned pending-setup store).useSubmitWorkspace— closes Gaps 4, 5.Summary by CodeRabbit