Fix v3 config persistence; add tabbed workspace UI#236
Fix v3 config persistence; add tabbed workspace UI#236Aleel101 wants to merge 7 commits intoOpenCoworkAI:mainfrom
Conversation
- Downgraded `@tailwindcss/postcss` and `tailwindcss` to version 4.0.0. - Downgraded `electron` from 39.8.9 to 39.8.8. - Added IPC handlers for window management (minimize, maximize, close). - Updated window creation to remove the native title bar and manage menu visibility based on platform. - Enhanced state management in onboarding and design generation processes. - Introduced support for fast model configuration in onboarding IPC. - Improved design generation state handling to support multiple active generations. - Refactored various components to utilize the new state management logic.
- Introduced `preservedV3OptionalsForWrite` function to maintain optional fields during configuration updates. - Updated various IPC handlers to utilize the new function, ensuring optional fields like `designSystem` and `imageGeneration` are preserved when writing configurations. - Added tests for the new functionality to verify correct behavior in different scenarios. - Adjusted dependencies in `pnpm-lock.yaml` to reflect version changes for several packages.
- Reorganized imports and adjusted formatting in several files for better readability. - Enhanced the structure of function parameters and return values for clarity. - Made minor adjustments to the console and files panels for consistent styling. - Updated the handling of console logs and design generation states for improved maintainability. - Refactored the ModelSwitcher and PreviewToolbar components to streamline their logic and enhance user experience.
… tests - Updated onboarding state tests to include a new `modelFast` field initialized to null. - Enhanced the `PreviewPane` test to include an `onConsoleLog` handler for better logging during tests. - Ensured consistency across various test files by adding the `modelFast` field to relevant configurations.
- Added `activeGenerations` state to `useCodesignStore` for tracking ongoing design generations. - Updated `ConsolePanel` to improve log entry keys for better uniqueness and stability. - Refactored dependencies in `ModelSwitcher` and `StickyTodoHeader` to ensure proper state handling and performance.
- Updated `CanvasTab` type to include 'history' and 'code' tabs, improving tab management. - Refactored `CanvasTabBar` to display new tab types with appropriate icons and labels. - Adjusted `PreviewPane` to handle new tab types, ensuring correct rendering of content based on active tab. - Enhanced localization by adding translations for new tab labels in English, Portuguese, and Chinese. - Improved overall UI consistency and user experience across components related to canvas tabs.
There was a problem hiding this comment.
Findings
-
[Major] Fast-model loading can get stuck forever or fail silently when provider model discovery fails —
apps/desktop/src/renderer/src/components/Settings.tsx:2041,apps/desktop/src/renderer/src/components/Settings.tsx:2260.listForProvider()is only handled in.then(...): a rejected IPC/network call never clearslistLoading, and an{ ok: false }response quietly downgrades the UI to an empty/manual state with no error surfaced.
Suggested fix:void window.codesign.models .listForProvider(config.provider) .then((res) => { if (cancelled) return; setListLoading(false); if (res.ok) { setAvailableModels(res.models); return; } setAvailableModels([]); pushToast({ variant: 'error', title: t('settings.modelRouting.saveFailed'), description: res.message, }); }) .catch((err) => { if (cancelled) return; setListLoading(false); setAvailableModels([]); pushToast({ variant: 'error', title: t('settings.modelRouting.saveFailed'), description: cleanIpcError(err) || t('settings.common.unknownError'), }); });
-
[Major] Restoring a snapshot updates the preview HTML but leaves comment filtering on the old snapshot —
apps/desktop/src/renderer/src/components/HistoryPanel.tsx:53,apps/desktop/src/renderer/src/components/HistoryPanel.tsx:61, with the stale filter applied inapps/desktop/src/renderer/src/components/PreviewPane.tsx:405andapps/desktop/src/renderer/src/components/PreviewPane.tsx:507. The new snapshot returned bysnapshots.create()is ignored, socurrentSnapshotIdis never refreshed and pins/comments can belong to the previous version after restore.
Suggested fix:const loadCommentsForCurrentDesign = useCodesignStore((s) => s.loadCommentsForCurrentDesign); const restored = await window.codesign.snapshots.create({ designId: currentDesignId, parentId: selected.id, type: 'fork', prompt: selected.prompt, artifactType: selected.artifactType, artifactSource: selected.artifactSource, }); setPreviewHtml(restored.artifactSource); await loadCommentsForCurrentDesign();
-
[Minor] The new console panel bypasses the shared token system with raw colors and a hardcoded mono stack —
apps/desktop/src/renderer/src/components/ConsolePanel.tsx:9,apps/desktop/src/renderer/src/components/ConsolePanel.tsx:18,apps/desktop/src/renderer/src/components/ConsolePanel.tsx:28,apps/desktop/src/renderer/src/components/ConsolePanel.tsx:81. This conflicts with the app-chrome rule to route UI values throughpackages/uitokens.
Suggested fix:const LEVEL_BG: Record<ConsoleLevel, string> = { log: '', info: '', warn: 'bg-[color-mix(in_srgb,var(--color-warning)_6%,transparent)]', error: 'bg-[color-mix(in_srgb,var(--color-error)_6%,transparent)]', debug: '', }; <div className="... font-[var(--font-mono)] ..." /> <span className="... bg-[var(--color-warning)] text-[var(--color-on-accent)] ..." />
Summary
- Review mode: initial. Three issues found: the fast-model loader has an unhandled failure path, snapshot restore does not resync snapshot-scoped comments, and the new console UI reintroduces raw app-chrome values instead of
packages/uitokens. - Residual risk: I did not find coverage for the fast-model discovery failure path or for history-restore snapshot/comment synchronization.
Testing
- Not run (automation)
open-codesign Bot
| setQuery(''); | ||
| setListLoading(true); | ||
| setAvailableModels([]); | ||
| void window.codesign.models.listForProvider(config.provider).then((res) => { |
There was a problem hiding this comment.
This listForProvider() call needs a failure path. Right now a rejected IPC/network call never clears listLoading, and an { ok: false } response silently degrades into an empty/manual model picker instead of surfacing the error.
Suggested fix:
void window.codesign.models
.listForProvider(config.provider)
.then((res) => {
if (cancelled) return;
setListLoading(false);
if (res.ok) {
setAvailableModels(res.models);
return;
}
setAvailableModels([]);
pushToast({
variant: 'error',
title: t('settings.modelRouting.saveFailed'),
description: res.message,
});
})
.catch((err) => {
if (cancelled) return;
setListLoading(false);
setAvailableModels([]);
pushToast({
variant: 'error',
title: t('settings.modelRouting.saveFailed'),
description: cleanIpcError(err) || t('settings.common.unknownError'),
});
});| artifactType: selected.artifactType, | ||
| artifactSource: selected.artifactSource, | ||
| }); | ||
| setPreviewHtml(selected.artifactSource); |
There was a problem hiding this comment.
The restore path creates a new snapshot but never refreshes currentSnapshotId, so PreviewPane and CommentsPanel keep filtering pins/comments against the previous snapshot while the HTML switches to the restored version.
Suggested fix:
const loadCommentsForCurrentDesign = useCodesignStore((s) => s.loadCommentsForCurrentDesign);
const restored = await window.codesign.snapshots.create({
designId: currentDesignId,
parentId: selected.id,
type: 'fork',
prompt: selected.prompt,
artifactType: selected.artifactType,
artifactSource: selected.artifactSource,
});
setPreviewHtml(restored.artifactSource);
await loadCommentsForCurrentDesign();|
|
||
| const LEVEL_STYLE: Record<ConsoleLevel, string> = { | ||
| log: 'text-[var(--color-text-primary)]', | ||
| info: 'text-[#3b82f6]', |
There was a problem hiding this comment.
This new panel reintroduces raw app-chrome values (#f59e0b, #3b82f6, ui-monospace,Menlo,monospace) instead of going through packages/ui tokens, which conflicts with the repo's token-only UI rule.
Suggested fix:
const LEVEL_BG: Record<ConsoleLevel, string> = {
log: '',
info: '',
warn: 'bg-[color-mix(in_srgb,var(--color-warning)_6%,transparent)]',
error: 'bg-[color-mix(in_srgb,var(--color-error)_6%,transparent)]',
debug: '',
};
<div className="... font-[var(--font-mono)] ..." />
<span className="... bg-[var(--color-warning)] text-[var(--color-on-accent)] ..." />…ings panel - Replace lang-mode selector (auto/html/js) with unified html language mode - Add Shiki v4 TextMate grammar tokenization via @shikijs/monaco for VS Code-quality syntax highlighting across HTML, CSS, JS, and embedded regions - Add babel injection grammar so <script type="text/babel"> blocks highlight as JS - Configure monaco.languages.html.htmlDefaults and javascriptDefaults for richer IntelliSense inside embedded script/style regions - Add editor settings panel: theme (system/light/dark), font size, word wrap, minimap, line numbers, tab size - Fix async theme registration race: apply Shiki theme imperatively after grammars load - Update ConsolePanel to use CSS variable colors and --font-mono - Fix HistoryPanel restore to use artifactSource from restored snapshot and call loadCommentsForCurrentDesign after restore - Add error handling for listForProvider in Settings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Findings
-
[Major] History-load failures are still swallowed and rendered as an empty history state —
apps/desktop/src/renderer/src/components/HistoryPanel.tsx:33,apps/desktop/src/renderer/src/components/HistoryPanel.tsx:42,apps/desktop/src/renderer/src/components/HistoryPanel.tsx:105. The new History view still catcheswindow.codesign.snapshots.list()with an empty handler, clearsloadinginfinally, and then falls through to theNo history yet.branch. If the snapshots IPC or DB read fails, users lose access to history and get misleading UI instead of an actionable error.
Suggested fix:const [loadError, setLoadError] = useState<string | null>(null); window.codesign.snapshots .list(currentDesignId) .then((rows) => { setLoadError(null); setSnapshots(rows); setSelected(rows[0] ?? null); }) .catch((err) => { const message = err instanceof Error ? err.message : String(err); setLoadError(message); pushToast({ variant: 'error', title: 'Failed to load history', description: message, }); }) .finally(() => setLoading(false));
-
[Minor] Monaco/TextMate bootstrap failures are still unhandled —
apps/desktop/src/renderer/src/components/PreviewPane.tsx:922,apps/desktop/src/renderer/src/lib/setupMonacoTextmate.ts:35. If one of the lazy imports orcreateHighlighter()fails, the rejection only lands in the console; the Code tab never surfaces why editor setup failed, which violates the repo's no-silent-fallback rule.
Suggested fix:const pushToast = useCodesignStore((s) => s.pushToast); setupTextmateGrammars(monaco) .then(() => { monaco.editor.setTheme(resolvedThemeRef.current); }) .catch((err) => { pushToast({ variant: 'error', title: 'Editor setup failed', description: err instanceof Error ? err.message : String(err), }); });
Summary
- Review mode: follow-up after new commits. Two issues remain on the current head: the History tab still hides snapshot-load failures behind an empty-state fallback, and the new Monaco/Shiki bootstrap path still has an unreported failure mode.
- Residual risk: I did not find coverage for the history-load error path or for Monaco/TextMate bootstrap failures.
Testing
- Not run (automation)
open-codesign Bot
| setSnapshots(rows); | ||
| setSelected(rows[0] ?? null); | ||
| }) | ||
| .catch(() => {}) |
There was a problem hiding this comment.
This still swallows snapshots.list() failures and then falls through to the No history yet. empty state once finally clears loading. That is both a misleading UX regression and a direct violation of the repo's no-silent-fallback rule.
Suggested fix:
const [loadError, setLoadError] = useState<string | null>(null);
window.codesign.snapshots
.list(currentDesignId)
.then((rows) => {
setLoadError(null);
setSnapshots(rows);
setSelected(rows[0] ?? null);
})
.catch((err) => {
const message = err instanceof Error ? err.message : String(err);
setLoadError(message);
pushToast({
variant: 'error',
title: 'Failed to load history',
description: message,
});
})
.finally(() => setLoading(false));| }); | ||
|
|
||
| monacoRef.current = monaco; | ||
| setupTextmateGrammars(monaco).then(() => { |
There was a problem hiding this comment.
setupTextmateGrammars() is now on the critical path for the Code tab, but this call still has no failure handling. If the lazy Shiki/Monaco import or createHighlighter() rejects, the user only gets a console rejection and no surfaced error, which conflicts with the repo's no-silent-fallback rule.
Suggested fix:
const pushToast = useCodesignStore((s) => s.pushToast);
setupTextmateGrammars(monaco)
.then(() => {
monaco.editor.setTheme(resolvedThemeRef.current);
})
.catch((err) => {
pushToast({
variant: 'error',
title: 'Editor setup failed',
description: err instanceof Error ? err.message : String(err),
});
});
Summary
This PR introduces a targeted config persistence fix alongside a cohesive desktop UX refinement built on the existing workspace model.
Aleel101/main(e.g.88046c2) vsOpenCoworkAI/upstream/maingit fetch upstream && git log upstream/main..HEADorgit diff upstream/main...HEADKey Changes
Config Persistence Fix (v3 optionals)
Fixes an issue where optional v3 config fields (
modelFast,imageGeneration,designSystem) could be unintentionally dropped during partial config writes.preservedV3OptionalsForWritein shared config.changeset/persist-fast-model-v3.mdDesktop UX and Workspace Model
Refines how existing features are organized and presented rather than introducing entirely new surfaces.
Files · History · Codeare now first-class tabsNote: Console and History views already existed. This PR focuses on integrating them into the tabbed layout and improving UX consistency.
Type of Change
Checklist
Dependencies
No new product dependencies. Any
package.jsonor lockfile changes are version alignment only.Screenshots
Files · History · Code)(See attached images)
p.s. Do what ya like, just fixed some things that were annoying me and thought some people may like these changes as well.