Feature/saved prompt snippets#3062
Conversation
# Conflicts: # .env.example # .github/workflows/release.yml # apps/desktop/package.json # apps/desktop/src/electron/ElectronMenu.ts # apps/desktop/src/window/DesktopWindow.test.ts # apps/marketing/src/pages/download.astro # apps/marketing/src/pages/index.astro # apps/mobile/src/features/cloud/publicConfig.ts # apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts # apps/web/index.html # apps/web/src/components/ChatView.tsx # apps/web/src/components/Sidebar.tsx # apps/web/src/components/chat/ChatHeader.tsx # apps/web/src/components/cloud/RelayClientInstallDialog.tsx # apps/web/src/components/settings/CloudSettings.tsx # apps/web/src/index.css # apps/web/src/main.tsx # docs/cloud/t3-code-connect-auth-flow.html # docs/operations/release.md # infra/relay/README.md # infra/relay/scripts/deploy.ts # pnpm-lock.yaml # scripts/lib/public-config.test.ts # scripts/lib/public-config.ts
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5d6cb5. Configure here.
| body, | ||
| }); | ||
| const nextMap: SnippetMap = { ...currentSnippets, [updated.id]: updated }; | ||
| updateSettings({ promptSnippets: nextMap }); |
There was a problem hiding this comment.
Edit ignores trigger changes
Medium Severity
In the edit-snippet dialog, the Trigger field stays editable, but saving only updates title, description, and body from the original snippet. A changed trigger is not applied, so the slash menu and stored id stay on the old value while the UI suggests the trigger was updated.
Reviewed by Cursor Bugbot for commit b5d6cb5. Configure here.
| }; | ||
| const nextSnippet = newSnippetFromDraft(draft); | ||
| const nextMap: SnippetMap = { ...currentSnippets, [nextSnippet.id]: nextSnippet }; | ||
| updateSettings({ promptSnippets: nextMap }); |
There was a problem hiding this comment.
Duplicate triggers overwrite snippets
Medium Severity
Adding a snippet does not check whether the trigger id already exists. Saving with a duplicate trigger replaces the existing entry in promptSnippets with no warning, which removes the prior snippet from settings and the slash menu.
Reviewed by Cursor Bugbot for commit b5d6cb5. Configure here.
| query, | ||
| ); | ||
| const rankedSnippets = searchSavedSnippetItems(savedSnippetItems, query); | ||
| return [...builtInAndProvider, ...rankedSnippets]; |
There was a problem hiding this comment.
Provider commands beat snippets
Medium Severity
When filtering the slash menu with a query, built-in and provider slash commands are listed before saved snippets. Enter or Tab uses the first menu item, so a matching provider command can be applied instead of the saved snippet the user typed toward.
Reviewed by Cursor Bugbot for commit b5d6cb5. Configure here.
| } else if (!isValidSnippetId(id)) { | ||
| errors.push({ | ||
| field: "id", | ||
| message: | ||
| "Trigger must start with a letter or digit and contain only lowercase letters, digits, and dashes.", | ||
| }); | ||
| } else if (id.length > MAX_SNIPPET_ID_LENGTH) { | ||
| errors.push({ | ||
| field: "id", | ||
| message: `Trigger must be ${MAX_SNIPPET_ID_LENGTH} characters or fewer.`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟢 Low settings/SnippetsSettings.logic.ts:88
The else if (id.length > MAX_SNIPPET_ID_LENGTH) branch on line 94 is unreachable because isValidSnippetId already returns false when the length exceeds MAX_SNIPPET_ID_LENGTH. An over-length ID therefore hits the pattern-error branch at line 88 and produces the misleading message about allowed characters instead of the intended length-specific message. Swap the two else if branches so the length check runs first.
| } else if (!isValidSnippetId(id)) { | |
| errors.push({ | |
| field: "id", | |
| message: | |
| "Trigger must start with a letter or digit and contain only lowercase letters, digits, and dashes.", | |
| }); | |
| } else if (id.length > MAX_SNIPPET_ID_LENGTH) { | |
| errors.push({ | |
| field: "id", | |
| message: `Trigger must be ${MAX_SNIPPET_ID_LENGTH} characters or fewer.`, | |
| }); | |
| } | |
| } else if (id.length > MAX_SNIPPET_ID_LENGTH) { | |
| errors.push({ | |
| field: "id", | |
| message: `Trigger must be ${MAX_SNIPPET_ID_LENGTH} characters or fewer.`, | |
| }); | |
| } else if (!isValidSnippetId(id)) { | |
| errors.push({ | |
| field: "id", | |
| message: | |
| "Trigger must start with a letter or digit and contain only lowercase letters, digits, and dashes.", | |
| }); | |
| } |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/SnippetsSettings.logic.ts around lines 88-99:
The `else if (id.length > MAX_SNIPPET_ID_LENGTH)` branch on line 94 is unreachable because `isValidSnippetId` already returns `false` when the length exceeds `MAX_SNIPPET_ID_LENGTH`. An over-length ID therefore hits the pattern-error branch at line 88 and produces the misleading message about allowed characters instead of the intended length-specific message. Swap the two `else if` branches so the length check runs first.
Evidence trail:
apps/web/src/components/settings/SnippetsSettings.logic.ts lines 77-80 (isValidSnippetId checks length > MAX_SNIPPET_ID_LENGTH and returns false), lines 86-99 (validateSnippetDraft: line 88 checks !isValidSnippetId which catches over-length IDs before line 94's length check can run)
| @@ -254,15 +253,11 @@ export const make = Effect.fn("effect-codex-app-server/CodexAppServerClient.make | |||
| export const layerChildProcess = ( | |||
There was a problem hiding this comment.
🟠 High src/client.ts:253
The layerChildProcess refactoring removed the forkScoped drain of handle.stderr. When the child process writes to stderr and the OS pipe buffer fills, the child blocks; the parent then deadlocks waiting for stdout output while the child waits for stderr buffer space. Consider restoring the Stream.runDrain(handle.stderr).pipe(Effect.ignore, Effect.forkScoped) call before constructing the client.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/effect-codex-app-server/src/client.ts around line 253:
The `layerChildProcess` refactoring removed the `forkScoped` drain of `handle.stderr`. When the child process writes to stderr and the OS pipe buffer fills, the child blocks; the parent then deadlocks waiting for stdout output while the child waits for stderr buffer space. Consider restoring the `Stream.runDrain(handle.stderr).pipe(Effect.ignore, Effect.forkScoped)` call before constructing the client.
Evidence trail:
1. PR diff at `packages/effect-codex-app-server/src/client.ts`: git_diff MERGE_BASE..REVIEWED_COMMIT shows removal of `yield* Stream.runDrain(handle.stderr).pipe(Effect.ignore, Effect.forkScoped)` from `makeChildProcessClient` (which was deleted entirely).
2. New `layerChildProcess` at line 253-260 (REVIEWED_COMMIT) calls `makeChildStdio(handle)` and `make(...)` without any stderr drain.
3. `packages/effect-codex-app-server/src/_internal/stdio.ts` lines 13-22 (REVIEWED_COMMIT): `makeChildStdio` maps `stdin: handle.stdout`, `stdout: () => Sink.mapInput(handle.stdin, ...)`, `stderr: () => Sink.drain` — but `handle.stderr` is never referenced.
4. git_grep for `handle.stderr` in `packages/effect-codex-app-server/**` returns zero results at REVIEWED_COMMIT, confirming no code consumes the child process's stderr stream.
| @@ -1129,49 +550,11 @@ function ChatMarkdown({ | |||
| li({ node: _node, children, ...props }) { | |||
| return <li {...props}>{renderSkillInlineMarkdownChildren(children, skills)}</li>; | |||
| }, | |||
| a({ node, href, children, ...props }) { | |||
| a({ node: _node, href, ...props }) { | |||
There was a problem hiding this comment.
🟡 Medium components/ChatMarkdown.tsx:553
Same-document anchor links (e.g., #section) incorrectly receive target="_blank", causing them to open in a new tab instead of navigating within the document. The previous implementation checked href?.startsWith("#") to avoid adding target="_blank" to fragment links, but now all non-file links unconditionally get target="_blank" rel="noopener noreferrer". Consider restoring the fragment link check to preserve in-document navigation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 553:
Same-document anchor links (e.g., `#section`) incorrectly receive `target="_blank"`, causing them to open in a new tab instead of navigating within the document. The previous implementation checked `href?.startsWith("#")` to avoid adding `target="_blank"` to fragment links, but now all non-file links unconditionally get `target="_blank" rel="noopener noreferrer"`. Consider restoring the fragment link check to preserve in-document navigation.
Evidence trail:
apps/web/src/components/ChatMarkdown.tsx line 553-557 at REVIEWED_COMMIT: unconditional `target="_blank"` for all non-file links. Old code (visible in diff at MERGE_BASE): `const isSameDocumentLink = href?.startsWith("#") ?? false;` with conditional target/rel, plus `handleMarkdownFragmentClick` handler. git_diff base=MERGE_BASE head=REVIEWED_COMMIT path=apps/web/src/components/ChatMarkdown.tsx shows removal of fragment-link check and associated functions (findMarkdownFragmentTarget, decodeMarkdownFragmentId, normalizeSanitizedFragmentId, handleMarkdownFragmentClick).
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |


What Changed
Added saved prompts support.
Why
UI Changes
Checklist
Note
High Risk
Deletes the entire desktop distribution and production release/deploy automation; shipping and install paths change materially until the new Linux workflow fully replaces them.
Overview
This PR removes the Electron desktop application (
apps/desktopand related scripts, tests, and IPC/backend wiring) and drops the largerelease.ymlpipeline that built signed desktop artifacts, published the CLI to npm, created GitHub releases, deployed hosted web to Vercel, and announced on Discord.CI and release are narrowed:
ci.ymlno longer installs Electron, builds desktop, or verifies the preload bundle; a newlinux-packages.ymlworkflow builds headless.deb/.rpmonv*.*.*tags (viavp run dist:linux) and uploads them to releases.Product/docs positioning shifts from T3 Code to more Code (README, AGENTS, plans, devcontainer name) and de-emphasizes the desktop app in user-facing docs—README drops desktop install paths and points at headless Linux packages; issue templates remove
apps/desktopand desktop-specific environment hints. CONTRIBUTING is shortened (less “we’re not taking PRs” / vouch wording).Misc:
.gitignoredrops Electron build paths; addsignored/.Reviewed by Cursor Bugbot for commit b5d6cb5. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add saved prompt snippets to the composer and rebrand 'T3 Code' to 'more Code'
Snippet/SnippetMapcontract types (snippets.ts), a/settings/snippetsUI, and composer slash-command search that ranks results by trigger slug, title, and descriptionT3CODE_*environment variables toMORECODE_T3CODE_*, renames thet3 connectCLI subcommand tot3 cloud, and moves server HTTP endpoints from/api/connect/*to/api/cloud/*sectionproject kind with shared context markdown and credentials; section context is injected as a<task_section_context>block into the first user turn sent to providers/runsroute withRunsViewlisting agent runs with status filtering and the ability to interrupt active runsclaude-fable-5model from built-in drivers; adds asemi-sandboxedruntime mode mapping toneverapprovals andworkspace-writesandbox policypackaging/linux/T3CODE_*environment variables, thet3 connectCLI, and/api/connect/*HTTP routes are renamed — existing server configurations, deployment scripts, and mobile build environments require updates📊 Macroscope summarized b5d6cb5. 93 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.