feat(desktop): Add GitHub issue attachment to new workspace modal#2678
Merged
Conversation
- Add backend tRPC procedures to list and fetch GitHub issues via gh CLI - Create IssueIcon component for open/closed issue states - Implement GitHubIssueLinkCommand with fuzzy search by number/title - Add LinkedGitHubIssuePill component for displaying linked issues - Extend LinkedIssue type to support both GitHub and internal issues - Fetch and attach full issue content as markdown files to agent context - Support URL pasting for quick issue linking - Gracefully handle errors without blocking workspace creation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds GitHub issue integration: two tRPC procedures to list and fetch issue content (via the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as PromptGroup / Command
participant Client as tRPC Client
participant Server as tRPC Server
participant GH as GitHub CLI (gh)
User->>UI: Open issue link popover / type query
UI->>Client: listIssues(projectId)
Client->>Server: tRPC request
Server->>GH: run `gh issue list --json ...`
GH-->>Server: JSON stdout
Server->>Server: Parse & validate (Zod), normalize states
Server-->>Client: issue array
Client-->>UI: update results
UI->>UI: Fuse.js search/filter
User->>UI: Select issue
UI->>Client: getIssueContent(projectId, issueNumber)
Client->>Server: tRPC request
Server->>GH: run `gh issue view <n> --json ...`
GH-->>Server: JSON stdout
Server->>Server: Parse & validate (Zod), normalize fields
Server-->>Client: issue content
Client-->>UI: issue content returned
UI->>UI: sanitize/convert to attachment and add pill
sequenceDiagram
participant User
participant PromptGroup
participant Client as tRPC Client
participant Server as tRPC Server
participant GH as GitHub CLI (gh)
User->>PromptGroup: Click "Create Workspace"
PromptGroup->>PromptGroup: Filter linkedIssues for GitHub ones
loop per GitHub issue
PromptGroup->>Client: getIssueContent(projectId, number)
Client->>Server: tRPC request
Server->>GH: `gh issue view`
GH-->>Server: JSON
Server->>Server: Parse & validate
Server-->>Client: issue content
Client-->>PromptGroup: issue data
PromptGroup->>PromptGroup: sanitize, truncate, convert to base64
PromptGroup->>PromptGroup: append convertedFiles
end
PromptGroup->>Server: create workspace (with attachments if any)
Server-->>PromptGroup: workspace created
PromptGroup-->>User: workspace ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Add runtime validation with zod schema for GitHub issue API responses - Add input validation for issueNumber (must be positive integer) - Sanitize user-generated content (title, author, body) to prevent XSS - Fix memory leak in useCallback dependencies (remove unstable references)
Security & Validation: - Complete HTML entity sanitization (& < > " ') to prevent XSS - URL validation to prevent javascript: and other malicious protocols - Add Zod validation to listIssues for consistency - Normalize issue state types with runtime validation Performance & Reliability: - Add 10-second timeout per issue fetch to prevent hanging - Limit issue body size to 50KB to prevent memory issues - Use URL-based duplicate detection (handles cross-repo issues) Type Safety: - Normalize IssueState throughout the codebase - Proper type guards for state coercion - Consistent "open" | "closed" typing in LinkedIssue Code Quality: - Remove unused keyboard handlers from pill component - Simplify accessibility implementation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx:906">
P2: GitHub issue identity is inconsistent: items are deduped by URL but keyed/removed by numeric slug, so same-number issues from different repos collide.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| state.toLowerCase() === "closed" ? "closed" : "open"; | ||
|
|
||
| const issue = { | ||
| slug: `#${issueNumber}`, |
Contributor
There was a problem hiding this comment.
P2: GitHub issue identity is inconsistent: items are deduped by URL but keyed/removed by numeric slug, so same-number issues from different repos collide.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx, line 906:
<comment>GitHub issue identity is inconsistent: items are deduped by URL but keyed/removed by numeric slug, so same-number issues from different repos collide.</comment>
<file context>
@@ -762,6 +892,29 @@ function PromptGroupInner({
+ state.toLowerCase() === "closed" ? "closed" : "open";
+
+ const issue = {
+ slug: `#${issueNumber}`,
+ title,
+ source: "github" as const,
</file context>
![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
🤖 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/lib/trpc/routers/projects/projects.ts`:
- Around line 433-435: Replace the generic Error throws in getIssueContent with
TRPCError instances so tRPC clients receive structured errors; specifically,
change the Project-not-found throw in getIssueContent (currently "throw new
Error(`Project ${input.projectId} not found`)") to throw new TRPCError with an
appropriate code (e.g., 'NOT_FOUND') and a clear message, and likewise replace
the other generic throws around lines 480–482 with TRPCError using suitable
codes (e.g., 'NOT_FOUND' or 'INTERNAL') so error handling is consistent; locate
these in the getIssueContent resolver and update the throw sites to use
TRPCError({ code: ..., message: ... }) instead of new Error(...).
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx`:
- Around line 79-88: The exact URL match against searchQuery can fail if the
user pasted a URL with leading/trailing whitespace; trim searchQuery before
checking urlMatch. In the GitHubIssueLinkCommand component, normalize the query
by calling trim() (e.g., const normalizedQuery = searchQuery.trim()) and use
normalizedQuery for the urlMatch lookup and for the issueFuse.search call while
preserving the early-return for empty queries (use normalizedQuery when testing
emptiness and matching against issuesWithSearchField and issueFuse, keeping
MAX_RESULTS unchanged).
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/LinkedGitHubIssuePill/LinkedGitHubIssuePill.tsx`:
- Around line 33-38: The remove-button is only revealed for pointer hover via
the "group-hover" utilities, which breaks keyboard accessibility; update the
Button rendered in LinkedGitHubIssuePill (the Button with aria-label "Remove
linked issue" and its className) to also include the matching
"group-focus-within" utilities (e.g., add group-focus-within:opacity-100 and
group-focus-within:pointer-events-auto alongside the existing
group-hover:opacity-100 and group-hover:pointer-events-auto) so the control
becomes visible and interactive when its parent group receives keyboard focus.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 905-915: The slug currently set to `#${issueNumber}` collapses
issues from different repos; change the slug on the created `issue` object to a
repo-scoped identifier (e.g. use `url` or `\`${owner}/${repo}#${issueNumber}\``)
instead of `#${issueNumber}` so it’s unique per repo, and update any logic that
relies on `issue.slug` (the linkedIssues list, the remove-by-id handler, and the
React key usage that references `issue.slug`) to use this new repo-scoped id so
rendering and deletion remain correct; ensure `updateDraft({ linkedIssues:
[...linkedIssues, issue] })` still pushes the new issue with the updated slug.
- Around line 683-691: The code re-resolves issues using { projectId,
issue.number } which can return wrong or 404 for cross-repo links; instead parse
and use the repo-scoped identifier from the stored issue (e.g. issue.url or
stored owner/repo) and call the repo-scoped API rather than
utils.client.projects.getIssueContent.query with projectId. Concretely: in the
githubIssues loop (githubIssues, issue.url, issue.number, fetchWithTimeout,
utils.client.projects.getIssueContent.query), extract owner and repo from
issue.url (or use the persisted owner/repo field), then call the repo-specific
endpoint (pass owner, repo, issueNumber) to fetch issue content; add a safe
fallback/log and timeout handling if parsing fails. Ensure you stop using
projectId for resolving cross-repo issue content.
- Around line 667-680: The renderer adds a 10s Promise.race timeout
(fetchWithTimeout) but the backend call using execWithShellEnv still runs gh
issue view with no child-process timeout; update the backend execWithShellEnv
implementation to accept an optional timeout parameter and pass it through to
Node's execFile/child_process invocation (use the execFile timeout option) so
the spawned gh process is killed if it exceeds the timeout, then update the
caller that runs the "gh issue view" command to supply the same 10_000ms timeout
when invoking execWithShellEnv.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36648d10-2913-4f67-9799-a6f915a39930
📒 Files selected for processing (9)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/GitHubIssueLinkCommand/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/LinkedGitHubIssuePill/LinkedGitHubIssuePill.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/LinkedGitHubIssuePill/index.tsapps/desktop/src/renderer/screens/main/components/IssueIcon/IssueIcon.tsxapps/desktop/src/renderer/screens/main/components/IssueIcon/index.ts
Comment on lines
+667
to
+680
| // Helper to add timeout to promises | ||
| const fetchWithTimeout = <T,>( | ||
| promise: Promise<T>, | ||
| timeoutMs: number, | ||
| ): Promise<T> => { | ||
| return Promise.race([ | ||
| promise, | ||
| new Promise<T>((_, reject) => | ||
| setTimeout( | ||
| () => reject(new Error("Request timeout")), | ||
| timeoutMs, | ||
| ), | ||
| ), | ||
| ]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd 'projects\.ts$' | xargs -r sed -n '/getIssueContent/,/^[[:space:]]*}),/p'
echo '----'
rg -n --type ts --type tsx -C3 'getIssueContent|gh issue view|timeout|AbortSignal|abort|kill|execFile|spawn'Repository: superset-sh/superset
Length of output: 497
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find and read the full getIssueContent implementation
echo "=== Searching for getIssueContent implementation ==="
find . -name "*.ts" -o -name "*.tsx" | xargs rg -l "getIssueContent" | head -5
echo ""
echo "=== Full getIssueContent procedure ==="
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec rg -A 50 "getIssueContent.*publicProcedure" {} + | head -100
echo ""
echo "=== Searching for gh issue view calls ==="
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec rg -B 5 -A 10 "gh issue view" {} +
echo ""
echo "=== Searching for timeout/AbortSignal patterns ==="
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec rg -B 3 -A 5 "timeout|AbortSignal|AbortController" {} + | head -80Repository: superset-sh/superset
Length of output: 271
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the projects.ts file to find getIssueContent implementation
echo "=== Full projects.ts content (or relevant sections) ==="
wc -l ./apps/desktop/src/lib/trpc/routers/projects/projects.ts
echo ""
echo "=== getIssueContent procedure with full context ==="
rg -B 5 -A 80 "getIssueContent.*publicProcedure" ./apps/desktop/src/lib/trpc/routers/projects/projects.tsRepository: superset-sh/superset
Length of output: 2483
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for execWithShellEnv definition ==="
rg -B 3 -A 30 "^.*execWithShellEnv.*=|function execWithShellEnv|const execWithShellEnv" --type ts --type tsx | head -100
echo ""
echo "=== Check where execWithShellEnv is imported from ==="
rg "from.*execWithShellEnv|import.*execWithShellEnv" ./apps/desktop/src/lib/trpc/routers/projects/projects.tsRepository: superset-sh/superset
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check imports in projects.ts ==="
head -50 ./apps/desktop/src/lib/trpc/routers/projects/projects.ts | grep -i "execWithShellEnv\|import"
echo ""
echo "=== Search for execWithShellEnv definition ==="
rg -B 3 -A 30 "execWithShellEnv" ./apps/desktop/src/lib/trpc/routers/projects/projects.ts | head -20
echo ""
echo "=== Find where execWithShellEnv might be defined ==="
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "function execWithShellEnv\|const execWithShellEnv\|export.*execWithShellEnv" | head -10Repository: superset-sh/superset
Length of output: 2115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read shell-env.ts file ==="
wc -l ./apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts
echo ""
echo "=== Full execWithShellEnv implementation ==="
cat ./apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.tsRepository: superset-sh/superset
Length of output: 8705
Add timeout enforcement to the backend execWithShellEnv call.
The 10s timeout via Promise.race() in the renderer only bounds how long the renderer waits. The backend's gh issue view call via execWithShellEnv has no timeout, so slow GitHub API lookups will keep the process running in the main thread even after workspace creation continues. Node.js execFile supports a timeout option—pass it through execWithShellEnv to terminate the child process if it exceeds the timeout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 667 - 680, The renderer adds a 10s Promise.race timeout
(fetchWithTimeout) but the backend call using execWithShellEnv still runs gh
issue view with no child-process timeout; update the backend execWithShellEnv
implementation to accept an optional timeout parameter and pass it through to
Node's execFile/child_process invocation (use the execFile timeout option) so
the spawned gh process is killed if it exceeds the timeout, then update the
caller that runs the "gh issue view" command to supply the same 10_000ms timeout
when invoking execWithShellEnv.
Comment on lines
+683
to
+691
| const issueContents = await Promise.all( | ||
| githubIssues.map(async (issue) => { | ||
| try { | ||
| const content = await fetchWithTimeout( | ||
| utils.client.projects.getIssueContent.query({ | ||
| projectId, | ||
| issueNumber: issue.number, | ||
| }), | ||
| 10000, // 10 second timeout per issue |
Contributor
There was a problem hiding this comment.
Resolve GitHub issue attachments from the issue's repository, not projectId.
GitHub issue numbers are only unique within a repo. This lookup ignores the stored issue.url and re-resolves { projectId, issueNumber }, so a pasted cross-repo URL—or simply changing projects after linking—can attach the wrong issue body or 404. Persist a repo-scoped identifier (owner/repo or the full URL) and use that here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 683 - 691, The code re-resolves issues using { projectId,
issue.number } which can return wrong or 404 for cross-repo links; instead parse
and use the repo-scoped identifier from the stored issue (e.g. issue.url or
stored owner/repo) and call the repo-scoped API rather than
utils.client.projects.getIssueContent.query with projectId. Concretely: in the
githubIssues loop (githubIssues, issue.url, issue.number, fetchWithTimeout,
utils.client.projects.getIssueContent.query), extract owner and repo from
issue.url (or use the persisted owner/repo field), then call the repo-specific
endpoint (pass owner, repo, issueNumber) to fetch issue content; add a safe
fallback/log and timeout handling if parsing fails. Ensure you stop using
projectId for resolving cross-repo issue content.
Comment on lines
+905
to
+915
| const issue = { | ||
| slug: `#${issueNumber}`, | ||
| title, | ||
| source: "github" as const, | ||
| url, | ||
| number: issueNumber, | ||
| state: normalizedState, | ||
| }; | ||
| // Check for duplicates by URL to handle same issue numbers from different repos | ||
| if (linkedIssues.some((i) => i.url === url)) return; | ||
| updateDraft({ linkedIssues: [...linkedIssues, issue] }); |
Contributor
There was a problem hiding this comment.
Use a repo-scoped id instead of #${issueNumber} for GitHub issues.
The duplicate guard intentionally allows same-number issues from different repos via distinct URLs, but this slug collapses them back together. Line 1010 uses it as the React key, and Lines 918-920 remove by it, so two different repo-specific #123s will render and delete incorrectly. Use url or owner/repo#number as the stable identifier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 905 - 915, The slug currently set to `#${issueNumber}` collapses
issues from different repos; change the slug on the created `issue` object to a
repo-scoped identifier (e.g. use `url` or `\`${owner}/${repo}#${issueNumber}\``)
instead of `#${issueNumber}` so it’s unique per repo, and update any logic that
relies on `issue.slug` (the linkedIssues list, the remove-by-id handler, and the
React key usage that references `issue.slug`) to use this new repo-scoped id so
rendering and deletion remain correct; ensure `updateDraft({ linkedIssues:
[...linkedIssues, issue] })` still pushes the new issue with the updated slug.
- Use TRPCError with proper error codes instead of plain Error - Add 10s timeout to gh CLI calls to prevent hanging - Trim whitespace from search query for URL matching
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
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
Adds the ability to link and attach GitHub issues when creating new workspaces, with full issue content included as context for the AI agent.
Changes
Backend
listIssuestRPC procedure to fetch open GitHub issues viagh issue listgetIssueContentprocedure to fetch full issue details viagh issue viewFrontend Components
LinkedIssuetype to support both GitHub and internal Superset issuesIntegration
.superset/attachments/github-issue-{number}.mdTesting
Summary by cubic
Adds GitHub issue linking to the new workspace modal and attaches full issue content as a markdown file to the agent context. Improves validation, sanitization, and error handling for safer, more reliable linking.
New Features
listIssuesandgetIssueContenttRPCprocedures viaghCLI, scoped to the repo.IssueIconandLinkedGitHubIssuePill(stored asLinkedIssuewithgithubsource).github-issue-{number}.md; includes per-issue 10s timeout and 50KB body truncation; failures don’t block.Bug Fixes
zodonlistIssues/getIssueContent, positiveissueNumber, complete HTML entity sanitization, and URL protocol checks; normalize issue state to"open" | "closed".TRPCErrorwith proper codes, add 10s timeouts toghCLI calls, trim whitespace for URL-based matching, URL-based duplicate detection, fix memory leak inuseCallbackdeps, and minor accessibility polish.Written for commit c8d3b83. Summary will update on new commits.
Summary by CodeRabbit