Repository navigation
feat(desktop): new-workspace screen experiment with sample prompts - #5907
Conversation
Test arm of PostHog experiment 387127 (flag new-workspace-screen): a page-based create surface for new users — Superset mark, "What should we build next?", three sample prompts, and a decluttered composer — replacing the dense modal. Control arm is untouched. - /new-workspace route; store opens redirect there in the test arm, and the empty dashboard renders the screen pinned as its zero state - eligibility gated in code by NEW_WORKSPACE_SCREEN_EXPERIMENT_START; variant evaluated only after flags load, only on surface reach, so exposure matches the experiment population; new-workspace-screen-override flag forces the UI for the team with zero experiment exposure - Enter submits (Shift+Enter newline), empty submit blocked, page-wide drag-drop with overlay, Codex-style attachment cards (image lightbox, reveal-in-Finder, saveToDownloads), linked issue/PR parity with the modal Claude-Session: https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
📝 WalkthroughWalkthroughAdds an experiment-gated full-screen new-workspace flow with project selection, prompt submission, attachments, image previews, Downloads persistence, and route-specific dashboard layout behavior. ChangesNew workspace flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant NewWorkspaceScreen
participant AttachmentCard
participant ImagePreviewOverlay
participant ExternalRouter
participant DownloadsFolder
User->>NewWorkspaceScreen: Add image attachment
NewWorkspaceScreen->>AttachmentCard: Render attachment
User->>AttachmentCard: Open image preview
AttachmentCard->>ImagePreviewOverlay: Show image
User->>ImagePreviewOverlay: Download image
ImagePreviewOverlay->>ExternalRouter: saveToDownloads(filename, dataBase64)
ExternalRouter->>DownloadsFolder: Write collision-safe file
DownloadsFolder-->>ExternalRouter: Return saved path
ExternalRouter-->>ImagePreviewOverlay: Return path
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/external/index.ts`:
- Around line 162-168: Update the download save logic around the target filename
loop to use exclusive file creation with the writeFile flag "wx" instead of
existsSync probing. Retry with the next suffixed filename only when the write
fails with EEXIST, and preserve the existing base name, extension, directory,
and base64 content behavior.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 364-372: Update the Enter submission condition in the custom
editor keydown handler to also require !event.isComposing before invoking
onEnterSubmitRef.current(), preserving submission for non-composition Enter
presses.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/page.tsx`:
- Around line 42-45: Update the workspace rendering flow around
useNewWorkspaceScreenVariant and the variant === "test" branch so that when
counts.all === 0 and variant is unresolved (null), it returns a neutral loading
state or null instead of rendering V2WorkspacesList. Preserve the existing
NewWorkspaceEmptyScreen behavior once the variant resolves to "test".
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsx`:
- Around line 120-127: Update the attachment upload failure path associated with
AttachmentCard so host-side upload errors are logged to main.log in addition to
displaying the existing errorMessage tooltip/toast. Add the logging at the
server or main-process boundary where the upload failure is detected, preserving
the current UI behavior.
🪄 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 Plus
Run ID: fe117ecb-85f4-439d-8318-0a1e99bdbe56
📒 Files selected for processing (20)
apps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/new-workspace/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/NewWorkspaceEmptyScreen/NewWorkspaceEmptyScreen.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/NewWorkspaceEmptyScreen/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/page.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/NewWorkspaceScreen.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/components/ImagePreviewOverlay/ImagePreviewOverlay.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/components/ImagePreviewOverlay/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/SamplePrompts.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/constants.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useNewWorkspaceScreenVariant/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useNewWorkspaceScreenVariant/useNewWorkspaceScreenVariant.tspackages/shared/src/constants.ts
| for (let i = 1; fs.existsSync(target); i++) { | ||
| target = nodePath.join(downloadsDir, `${name} (${i})${ext}`); | ||
| } | ||
| await fs.promises.writeFile( | ||
| target, | ||
| Buffer.from(input.dataBase64, "base64"), | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate file and inspect relevant sections"
fd -a 'index.ts' . | sed 's#^\./##' | rg 'apps/desktop/src/lib/trpc/routers/external/index.ts' || true
if [ -f apps/desktop/src/lib/trpc/routers/external/index.ts ]; then
wc -l apps/desktop/src/lib/trpc/routers/external/index.ts
sed -n '120,185p' apps/desktop/src/lib/trpc/routers/external/index.ts | cat -n
echo
echo "Search for procedure definitions/imports"
sed -n '1,120p' apps/desktop/src/lib/trpc/routers/external/index.ts | cat -n
fi
echo
echo "Search for callers/similar filename handling"
rg -n "input\.dataBase64|downloadsDir|base64|externalRouter|\.download|dataBase64" apps/desktop/src/lib/trpc apps/desktop/src -g '*.{ts,tsx}' || true
echo
echo "Node TOCTOU probe if node is available"
node - <<'JS'
const fs = require('fs');
const os = require('os');
const path = require('path');
const child = require('worker_threads').parentPort;
const shared = Buffer.alloc(4);
const { SharedArrayBuffer, Atomics } = require('worker_threads');
if (!child || !(shared instanceof SharedArrayBuffer)) {
console.log('node worker/shared-memory probe unavailable in this execution context');
process.exit(0);
}
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'node-tocouto-'));
const file = path.join(dir, 'same.txt');
(async () => {
await Promise.all([
new Promise(async (resolve) => {
const w = child.constructor;
const worker = new w(`
const { Atomics, SharedArrayBuffer, workerData } = require('worker_threads');
const shared = new SharedArrayBuffer(4);
const sharedView = new Int32Array(shared);
const file = workerData[0];
Atomics.store(sharedView, 0, 0);
Atomics.store(sharedView, 1, 1);
require('fs').existsSync(file);
Atomics.wait(sharedView, 0, 1);
const first = require('fs').existsSync(file);
try {
return require('fs').openSync(file, 'wx', 0o644);
} catch (error) {
return error && error.code;
}
`, { workerData: [file] });
worker.on('error', (err) => console.log('worker error', err.message));
worker.on('exit', () => resolve(Atomics.load(new Int32Array(shared), 1)));
Atomics.wait(new Int32Array(shared), 1, 1);
Atomics.wait(new Int32Array(shared), 0, 0);
}),
new Promise((resolve) => {
const w = child.constructor;
const worker = new w(`
const { Atomics, SharedArrayBuffer } = require('worker_threads');
const shared = new Int32Array(workerData[0]);
Atomics.store(shared, 0, 1);
const ready = Atomics.wait(shared, 0, 0);
return ready;
`, { workerData: Int32Array.buffer });
worker.on('error', (err) => console.log('worker error', err.message));
worker.on('exit', () => resolve());
}),
]);
fs.writeFileSync(file, 'lost');
await child.postMessage({ ok: 'done' });
})();
JSRepository: superset-sh/superset
Length of output: 24656
🌐 Web query:
Node.js fs.writeFile exclusive flag EEXIST race condition
💡 Result:
Yes, using the wx flag with fs.writeFile (or the corresponding file system method) is the correct, recommended way to prevent race conditions when creating files [1][2]. The 'wx' flag stands for "write" and "exclusive" (x) [3]. When this flag is used, the underlying operating system performs the check for the file's existence and the file creation as a single, atomic operation [1]. This atomicity is critical because it eliminates the "time-of-check to time-of-use" (TOCTOU) race condition that occurs if you attempt to manually check for a file's existence using methods like fs.exists (deprecated) or fs.stat before writing [4][1][5]. In those manual approaches, another process could potentially create the file in the small window of time between your check and your write command, leading to an accidental overwrite [1]. When you use the wx flag: 1. The operation will fail if the file already exists, and the error returned will have the code EEXIST [1][5]. 2. If the file does not exist, it will be created and written to atomically [1]. Important considerations: - Handle the error: Since 'wx' causes an error when the file exists, your code must explicitly catch and handle the EEXIST error code to respond appropriately (e.g., by logging a message or choosing a different filename) [1][5]. - Not a general lock: While the wx flag ensures atomic creation, it does not act as a general file lock for subsequent read/write operations [3]. If you need to perform multiple operations on the same file without other processes interfering, you should look into more robust file locking mechanisms or architectural patterns [3]. - Don't use existence checks first: Official Node.js documentation explicitly warns against using fs.access or similar checks to test for file accessibility before calling file system methods, as this introduces the very race condition that the flags are designed to avoid [4][5].
Citations:
- 1: https://stackoverflow.com/questions/45897074/using-fs-stat-and-fs-writefile
- 2: https://stackoverflow.com/questions/34187284/how-to-not-overwrite-file-in-node-js
- 3: https://stackoverflow.com/questions/12899061/creating-a-file-only-if-it-doesnt-exist-in-node-js
- 4: https://nodejs.org/api/fs.html
- 5: https://nodejs.org/docs/latest-v24.x/api/fs.html
Make collision handling atomic.
The existsSync(target) probe and subsequent writeFile are separate operations, so concurrent saves resolving to the same filename can race and one can be lost. Use an exclusive write (wx) loop that retries on EEXIST instead of probing before each write.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 164-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.promises.writeFile(
target,
Buffer.from(input.dataBase64, "base64"),
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 162 - 168,
Update the download save logic around the target filename loop to use exclusive
file creation with the writeFile flag "wx" instead of existsSync probing. Retry
with the next suffixed filename only when the write fails with EEXIST, and
preserve the existing base name, extension, directory, and base64 content
behavior.
Source: Linters/SAST tools
| if ( | ||
| onEnterSubmitRef.current && | ||
| event.key === "Enter" && | ||
| !event.shiftKey && | ||
| !event.altKey | ||
| ) { | ||
| onEnterSubmitRef.current(); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does ProseMirror handleKeyDown fire during IME composition isComposing keyCode 229
💡 Result:
ProseMirror is designed to ignore key events during IME composition to prevent them from interfering with the browser's native handling of composition sessions [1][2]. In its core implementation, ProseMirror's keydown handler utilizes an internal function—typically inOrNearComposition(view, event)—to check if a composition is currently active or recently ended [1][2]. When this check identifies that an IME composition is in progress, the handler returns early, effectively preventing the handleKeyDown prop from being invoked [1][2]. Regarding keyCode 229: 1. The value 229 is the "Composition" key code, a legacy identifier often used by browsers (notably older versions of Safari) to signal that a key event is being processed as part of an IME composition session [3][4][5]. 2. While modern browsers primarily use the isComposing flag on the event object to indicate composition, ProseMirror acknowledges the complexity of cross-browser IME behavior [2][5]. 3. ProseMirror's internal logic avoids triggering standard keydown processing when isComposing is true or when it detects that the event is associated with an active composition state [1][2]. If you are writing custom plugins or handlers, you should similarly guard your handleKeyDown logic by checking event.isComposing [5]. Additionally, some developers include a check for event.keyCode === 229 as a fallback to ensure compatibility with older or specific browser environments that may not reliably set the isComposing flag [4][5][6]. Top results: [1][2][5]
Citations:
- 1: https://github.com/ProseMirror/prosemirror-view/blob/master/src/input.ts
- 2: Enter at the end of the document doesn't work on mobile in some situations ProseMirror/prosemirror#904
- 3: https://www.stum.de/2016/06/24/handling-ime-events-in-javascript/
- 4: online-go/online-go.com@62b9901
- 5: https://dev.to/yukimi-inu/why-16-billion-east-asians-are-quietly-raging-at-your-enter-key-handler-1po0
- 6: haowei2000/Cheers@ffd365e
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate MarkdownEditor file and related package files"
fd -a 'MarkdownEditor\.tsx|package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json' . | sed 's#^\./##' | head -80
echo
echo "MarkdownEditor outline"
ast-grep outline apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx --view expanded || true
echo
echo "Relevant MarkdownEditor sections"
wc -l apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
sed -n '200,390p' apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
echo
echo "Search for tiptap/prosemirror imports and usages"
rg -n "tiptap|prosemirror|handleKeyDown|onEnterSubmit|isComposing|ProseMirror" apps/desktop/src/renderer/components/MarkdownEditor apps/desktop -g '*.{ts,tsx}' -g 'package.json' -g 'pnpm-lock.yaml' -g 'yarn.lock' -g 'package-lock.json' | head -200Repository: superset-sh/superset
Length of output: 40431
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Lockfile/package manager"
git ls-files | rg '(^|/)package\.json$|(^|/)pnpm-lock\.yaml$|(^|/)yarn\.lock$|(^|/)package-lock\.json$' | head -20
echo
echo "Desktop package dependencies"
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('apps/desktop/package.json','utf8'));
for (const section of ['dependencies','devDependencies']) {
console.log(`\n[${section}]`);
for (const [k,v] of Object.entries(p[section]||{}).filter(([k]) => k.includes('tiptap') || k.includes('prosemirror') || k.includes('electron'))) {
console.log(`${k}: ${v}`);
}
}
JS
echo
echo "Root lock snippets for tiptap/prosemirror/electron"
for f in pnpm-lock.yaml yarn.lock package-lock.json; do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n "`@tiptap`|prosemirror|electron(-main|preload)?|electron(\@|:)|desktop app|electron|`@electron`" "$f" | head -120 || true
done
echo
echo "KeyboardHandler implementation"
sed -n '100,135p' apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxRepository: superset-sh/superset
Length of output: 3265
🌐 Web query:
@tiptap/react 3 handleKeyDown isComposing composition events ProseMirror
💡 Result:
In Tiptap (built on ProseMirror), handling keyboard events during an IME composition session requires caution because browsers fire events in complex sequences that can cause input corruption if intercepted improperly [1][2]. To properly manage handleKeyDown in a Tiptap or ProseMirror context while respecting composition: 1. Use event.isComposing: The standard way to detect if a keyboard event is occurring during an IME composition session is to check the read-only isComposing property of the KeyboardEvent [3]. In your handleKeyDown plugin prop, you should generally ignore events where event.isComposing is true to avoid interfering with the native composition process [2]. 2. Understand ProseMirror's Role: ProseMirror's core view logic already includes internal guards—such as inOrNearComposition—to prevent the editor from attempting to handle key events that occur during or immediately around an IME composition session [1]. Interfering with this by trying to force-handle or block events can lead to duplicated characters, incorrect text placement, or broken composition flow [2]. 3. Implementation Example: When writing a custom extension that uses a ProseMirror plugin to intercept keys, you can implement the check as follows: import { Extension } from '@tiptap/core'; import { Plugin, PluginKey } from 'prosemirror-state'; export const MyKeyboardExtension = Extension.create({ name: 'my-keyboard-extension', addProseMirrorPlugins { return [ new Plugin({ key: new PluginKey('my-key-handler'), props: { handleKeyDown: (view, event) => { // Ignore events during composition to prevent input corruption if (event.isComposing) { return false; } // Your custom key handling logic if (event.key === 'Enter') { //... your logic... return true; } return false; }, }, }), ]; }, }); Key Considerations: - Do not attempt to start or end composition events programmatically; these are managed by the browser's IME [4][5]. - Be aware that on certain mobile browsers (particularly Android/Chrome), composition events can behave unpredictably [6][1]. The native ProseMirror input handlers are specifically designed to suppress certain keys (like Enter) during these states to maintain stability [1]. - If you find your logic is still being triggered during composition, ensure your check is placed at the very beginning of your handleKeyDown handler [2].
Citations:
- 1: https://github.com/ProseMirror/prosemirror-view/blob/master/src/input.ts
- 2: fix(react,pro): document-describing chrome renders nothing without a document eigenpal/docx-editor#179
- 3: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing
- 4: https://discuss.prosemirror.net/t/programmatically-start-compositionevent-with-prosemirror/4454
- 5: Bad performence when composition with ime ProseMirror/website#172
- 6: [Bug]: On Android browsers, if text style is set for the editor, the first letter entered on a blank line will fire compositionend event ueberdosis/tiptap#4606
🌐 Web query:
Tiptap 3 handleKeyDown composition events source code event.isComposing
💡 Result:
In Tiptap (which is built on ProseMirror), composition events and the isComposing state are handled primarily at the ProseMirror level rather than within Tiptap's core source code [1][2]. When you implement custom keyboard handling via Tiptap extensions—such as using the handleKeyDown prop—it is important to respect the browser's native composition session to avoid issues with IME (Input Method Editor) input, which is common for languages like Chinese, Japanese, and Korean [1][2]. The KeyboardEvent.isComposing property is a standard Web API that indicates whether a key event occurs during a composition session (i.e., after compositionstart and before compositionend) [3]. In ProseMirror's event handling architecture, this is used to prevent the editor from prematurely processing key inputs that are intended to be part of an ongoing composition [4][2]. If you are using handleKeyDown in a Tiptap extension, you can check the isComposing property on the event object to decide whether to bypass your custom logic [5][2]. A common pattern looks like this: handleKeyDown(view, event) { if (event.isComposing) { return false; // Let the browser/ProseMirror handle the event during composition } // Your custom logic here return true; } ProseMirror internally manages composition events (via compositionstart, compositionupdate, and compositionend) to ensure the document state remains consistent with the DOM [4][2]. Manually interfering with these events is generally discouraged, as the browser must handle them normally for composition to function correctly [2]. Issues involving composition in Tiptap are often inherited from ProseMirror's input handling, where special care is required to ensure that editor state updates do not interrupt or prematurely finalize a composition session [1][4].
Citations:
- 1: CJK compatibility issue: last character gets disappered when newline is inserted ueberdosis/tiptap#2545
- 2: Bad performence when composition with ime ProseMirror/website#172
- 3: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing
- 4: [Bug]: On Android browsers, if text style is set for the editor, the first letter entered on a blank line will fire compositionend event ueberdosis/tiptap#4606
- 5: Handle Enter extensions ueberdosis/tiptap#1655
Check event.isComposing before submitting on Enter.
Even though Ti/Tiptap uses ProseMirror under the hood, this custom editorProps.handleKeyDown can still observe key events during IME composition. Guarding with !event.isComposing prevents committing an IME candidate from firing onEnterSubmit and submitting prematurely.
🐛 Proposed guard
if (
onEnterSubmitRef.current &&
event.key === "Enter" &&
+ !event.isComposing &&
!event.shiftKey &&
!event.altKey
) {
onEnterSubmitRef.current();
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| onEnterSubmitRef.current && | |
| event.key === "Enter" && | |
| !event.shiftKey && | |
| !event.altKey | |
| ) { | |
| onEnterSubmitRef.current(); | |
| return true; | |
| } | |
| if ( | |
| onEnterSubmitRef.current && | |
| event.key === "Enter" && | |
| !event.isComposing && | |
| !event.shiftKey && | |
| !event.altKey | |
| ) { | |
| onEnterSubmitRef.current(); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`
around lines 364 - 372, Update the Enter submission condition in the custom
editor keydown handler to also require !event.isComposing before invoking
onEnterSubmitRef.current(), preserving submission for non-composition Enter
presses.
| if (isError && errorMessage) { | ||
| return ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild>{body}</TooltipTrigger> | ||
| <TooltipContent>{errorMessage}</TooltipContent> | ||
| </Tooltip> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline "apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsx" || true
echo "== relevant lines =="
cat -n "apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsx" | sed -n '1,180p'
echo "== search for errorMessage / main.log / log error patterns =="
rg -n "errorMessage|main\.log|console\.log|log\\(" apps/desktop/src -g '*.ts' -g '*.tsx' | sed -n '1,220p'Repository: superset-sh/superset
Length of output: 43696
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsx"
echo "== git diff stat and specific diff if available =="
git diff -- "$file" || true
git diff -- "$file" | sed -n '1,220p' || true
echo "== attachment card references/usages =="
rg -n "AttachmentCard|file\.url|uploadError|error" apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal -g '*.ts' -g '*.tsx' | sed -n '1,260p'Repository: superset-sh/superset
Length of output: 13628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
store="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useUploadAttachments/store.ts"
hook="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useUploadAttachments/useUploadAttachments.ts"
submit="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts"
echo "== store outline =="
ast-grep outline "$store" || true
echo "== store tail =="
cat -n "$store" | sed -n '1,140p'
echo "== hook lines =="
cat -n "$hook" | sed -n '1,120p'
echo "== submit error surface =="
cat -n "$submit" | sed -n '1,80p'
cat -n "$submit" | sed -n '110,180p'
echo "== exact main.log occurrences =="
rg -n "main\.log|mainLog|mainLog\(" apps/desktop/src apps/desktop -g '*.ts' -g '*.tsx' || trueRepository: superset-sh/superset
Length of output: 15332
Ensure attachment upload failures are written to main.log.
The upload path collects failures and shows them in a tooltip/toast, but main.log is not written anywhere for attachment upload failures. Add server-side or main-process logging when the host-side attachment upload fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/AttachmentCard/AttachmentCard.tsx`
around lines 120 - 127, Update the attachment upload failure path associated
with AttachmentCard so host-side upload errors are logged to main.log in
addition to displaying the existing errorMessage tooltip/toast. Add the logging
at the server or main-process boundary where the upload failure is detected,
preserving the current UI behavior.
Source: Learnings
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…uctions Short labels in the UI, real instructions inserted on click: the setup prompt walks the agent through .superset/config.json (setup/teardown/run scripts, per-workspace ports) with links to docs.superset.sh. Claude-Session: https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/constants.ts`:
- Line 13: Update the prompt string in the Sample Prompts constants to instruct
copying only explicitly allowlisted files, such as .env, from
$SUPERSET_ROOT_PATH into new workspaces; remove the broad “untracked files”
wording while preserving the setup, teardown, port, and summary requirements.
🪄 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 Plus
Run ID: 7021bd59-163a-4252-83c4-93b0951de145
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/SamplePrompts.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/SamplePrompts.tsx
| { | ||
| id: "set-up-project", | ||
| label: "Set up this project for Superset", | ||
| prompt: `Set up this repository to work well with Superset workspaces. Read https://docs.superset.sh/setup-teardown-scripts and create a .superset/config.json with: setup commands that install dependencies and copy untracked files (like .env) from "$SUPERSET_ROOT_PATH" into new workspaces, teardown commands that stop anything setup starts, and a run command that launches the dev server. If parallel workspaces would collide on dev-server ports, make the scripts pick a free port per workspace (see https://docs.superset.sh/ports). When you're done, summarize what you configured and how to use it.`, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not copy arbitrary untracked files.
This wording could lead the agent to bulk-copy secrets, credentials, build outputs, or other host-only files into every workspace. Restrict the instruction to an explicit allowlist such as .env; the official setup docs demonstrate copying only that file. (docs.superset.sh)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen/components/SamplePrompts/constants.ts`
at line 13, Update the prompt string in the Sample Prompts constants to instruct
copying only explicitly allowlisted files, such as .env, from
$SUPERSET_ROOT_PATH into new workspaces; remove the broad “untracked files”
wording while preserving the setup, teardown, port, and summary requirements.
Main (#5908) replaced counts with isReady in useAccessibleV2Workspaces — textually clean merge but semantically broken (the pinned empty-state check referenced the removed counts). Resolved by gating the pinned create screen on isReady + zero rows + default filters, which also fixes the transient-zero flash the old counts check was exposed to. Claude-Session: https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
- ref-guard preSelectedProjectId so the project picker isn't snapped back to the URL param on every change (mirrors the control modal) - gate the pinned empty state on the default device filter so filtered-to-empty device views keep the normal empty state - poison ambiguous same-name entries in the attachment path map so reveal-in-Finder never opens the wrong file Claude-Session: https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
…6557) The suggestion block was positioned `absolute inset-x-6 bottom-full`, so it took no layout space. The heading above it is the flex-1 spacer, which therefore centered itself into space the suggestions then drew on top of — the cards rendered straight through "What should we build next?". Latent since the screen shipped in #5907; it only needed a tall enough suggestion set to surface. The returning-tier explain-superset card wrapped its title to two lines, which was enough. Putting the block back in flow fixes it structurally: the flex-1 heading absorbs the height, and because the composer block is bottom-anchored and PromptInput sits below the suggestions, the composer itself does not move — only the heading recenters. The root is already overflow-y-auto, so windows too short for both scroll rather than collide. Also shortens the explain-superset label to "Get more out of Superset" so the card title fits one line.
Links
Summary
new-workspace-screenexperiment: a page-based create surface for new users — Superset mark, "What should we build next?", three clickable sample prompts, and a decluttered composer — replacing the ~20-control modal. Control arm renders the existing modal, byte-identical.Why / Context
Sample prompts + reduced clutter at the exact moment new users stall ("what do I type?"). Primary metric:
workspace_created(type != main) funnel from exposure; guardrails:agent_session_launchand 1–7d return retention, so a creation lift must be backed by real engagement.How It Works
useNewWorkspaceScreenVariantgates on account age (NEW_WORKSPACE_SCREEN_EXPERIMENT_START), waits for PostHog flags to load before evaluating (avoids the undefined-flag race that would cross-contaminate brand-new users), and falls back to control-without-exposure after 2s offline. Thenew-workspace-screen-overrideflag short-circuits to the test UI with zero experiment exposure (team/dev access)./new-workspace): store opens ("+", ⌘N, onboarding hand-off) redirect there; an empty dashboard renders the screen pinned as its zero state, replacing "No workspaces yet". TopBar hides under the same expanded-sidebar condition as the v2 workspace route (page brings its own drag strip).NewWorkspaceScreen), reusing only non-visual plumbing (useSubmitWorkspace, draft store, pickers). Enter submits / Shift+Enter newlines (opt-inonEnterSubmiton MarkdownEditor), empty submit blocked, sample click fills + focuses the editor, suggestions animate out while typing.external.saveToDownloads), file cards with file-tree extension icons + reveal-in-Finder. Linked Linear/GitHub issue and PR parity with the modal (pills + link buttons + "based off PR #n").Manual QA Checklist
Verified live over CDP against the dev stack (real mouse/keyboard input, screenshots captured):
/new-workspacewith sidebar visible; no TopBar strip with expanded sidebarPromptGroup/ModalContenthave zero diff)/new-workspacewith the new project pre-selected (needs a fresh-signup run)Testing
bun run typecheckbun run lintDockBadgeControllersetState-in-render from sidebar items — unrelated, tracked separately)Design Decisions
Deployment / Rollout
@superset.shaccounts see the screen (override flag, already live).NEW_WORKSPACE_SCREEN_EXPERIMENT_STARTto the release date, then launch in PostHog. New signups install the current build, so ramp is immediate.Rollback
control100% (or stopping the experiment); code paths degrade to the existing modal with no data impact.https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
Summary by cubic
Adds an experiment arm that replaces the dense “New workspace” modal with a full‑screen create page and sample prompts to help new users start faster. Control is unchanged.
New Features
isReady, there are zero workspaces, and default filters are active (prevents transient flashes and respects filtered views)..superset/config.jsonsetup). Enter submits; Shift+Enter adds a newline. Empty submit is blocked.external.saveToDownloads, and reveal-in-Finder.NEW_WORKSPACE_SCREENwith eligibility fromNEW_WORKSPACE_SCREEN_EXPERIMENT_START. Evaluated only when the surface opens, after flags load, with a 2s offline fallback to control;NEW_WORKSPACE_SCREEN_OVERRIDEforces the screen without exposure.MarkdownEditornow supportsonEnterSubmit.Bug Fixes
projectIdURL param while editing.Written for commit 1a34f55. Summary will update on new commits.
Summary by CodeRabbit
/new-workspace, including project preselection, prompt/configuration, and attachment cards.