Skip to content

Commit 1cf5a3c

Browse files
[Fix] Settings and marketplace stay inaccessible after importing Roo Router settings (#109)
* fix: restore settings access after Roo Router import downgrade * Address PR feedback on Roo import welcome gating * Handle Roo import redirect from gated tabs * Add tab-state matrix for Roo import redirect * Consume imported settings timestamp after sync --------- Co-authored-by: Roomote <roomote@roocode.com>
1 parent d09add8 commit 1cf5a3c

4 files changed

Lines changed: 270 additions & 5 deletions

File tree

src/core/config/__tests__/importExport.spec.ts

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -731,9 +731,12 @@ describe("importExport", () => {
731731
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
732732
])
733733

734+
const seenImportedAt: Array<number | undefined> = []
734735
const mockProvider = {
735-
settingsImportedAt: 0,
736-
postStateToWebview: vi.fn().mockResolvedValue(undefined),
736+
settingsImportedAt: undefined as number | undefined,
737+
postStateToWebview: vi.fn().mockImplementation(async () => {
738+
seenImportedAt.push(mockProvider.settingsImportedAt)
739+
}),
737740
}
738741

739742
const showWarningMessageSpy = vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined)
@@ -766,15 +769,67 @@ describe("importExport", () => {
766769
)
767770
expect(showInfoMessageSpy).not.toHaveBeenCalled()
768771

769-
// Provider state should still be updated
770-
expect(mockProvider.settingsImportedAt).toBeGreaterThan(0)
772+
// Provider state should be delivered once, then cleared.
773+
expect(seenImportedAt).toHaveLength(1)
774+
expect(seenImportedAt[0]).toBeGreaterThan(0)
775+
expect(mockProvider.settingsImportedAt).toBeUndefined()
771776
expect(mockProvider.postStateToWebview).toHaveBeenCalled()
772777

773778
showWarningMessageSpy.mockRestore()
774779
showInfoMessageSpy.mockRestore()
775780
consoleWarnSpy.mockRestore()
776781
})
777782

783+
it("clears settingsImportedAt after posting the imported state so later launches do not replay it", async () => {
784+
const filePath = "/mock/path/settings.json"
785+
const mockFileContent = JSON.stringify({
786+
providerProfiles: {
787+
currentApiConfigName: "valid-profile",
788+
apiConfigs: {
789+
"valid-profile": {
790+
apiProvider: "openai" as ProviderName,
791+
apiKey: "test-key",
792+
id: "valid-id",
793+
},
794+
},
795+
},
796+
globalSettings: { mode: "code" },
797+
})
798+
799+
;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
800+
;(fs.access as Mock).mockResolvedValue(undefined)
801+
802+
mockProviderSettingsManager.export.mockResolvedValue({
803+
currentApiConfigName: "default",
804+
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
805+
})
806+
mockProviderSettingsManager.listConfig.mockResolvedValue([
807+
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
808+
])
809+
810+
const seenImportedAt: Array<number | undefined> = []
811+
const mockProvider = {
812+
settingsImportedAt: undefined as number | undefined,
813+
postStateToWebview: vi.fn().mockImplementation(async () => {
814+
seenImportedAt.push(mockProvider.settingsImportedAt)
815+
}),
816+
}
817+
818+
await importSettingsWithFeedback(
819+
{
820+
providerSettingsManager: mockProviderSettingsManager,
821+
contextProxy: mockContextProxy,
822+
customModesManager: mockCustomModesManager,
823+
provider: mockProvider,
824+
},
825+
filePath,
826+
)
827+
828+
expect(seenImportedAt).toHaveLength(1)
829+
expect(seenImportedAt[0]).toBeGreaterThan(0)
830+
expect(mockProvider.settingsImportedAt).toBeUndefined()
831+
})
832+
778833
it("should handle multiple profiles with mixed valid and invalid providers", async () => {
779834
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])
780835

src/core/config/importExport.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ export const importSettingsWithFeedback = async (
327327
if (result.success) {
328328
provider.settingsImportedAt = Date.now()
329329
await provider.postStateToWebview()
330+
provider.settingsImportedAt = undefined
330331
const warnings = "warnings" in result ? result.warnings : undefined
331332

332333
// Show warnings if any profiles had issues but were still imported (with modifications)

webview-ui/src/App.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const App = () => {
5454
const {
5555
didHydrateState,
5656
showWelcome,
57+
settingsImportedAt,
5758
shouldShowAnnouncement,
5859
telemetrySetting,
5960
telemetryKey,
@@ -67,6 +68,7 @@ const App = () => {
6768

6869
const [showAnnouncement, setShowAnnouncement] = useState(false)
6970
const [tab, setTab] = useState<Tab>("chat")
71+
const handledImportRef = useRef<number | undefined>(undefined)
7072

7173
const [deleteMessageDialogState, setDeleteMessageDialogState] = useState<DeleteMessageDialogState>({
7274
isOpen: false,
@@ -169,6 +171,19 @@ const App = () => {
169171
}
170172
}, [shouldShowAnnouncement, tab])
171173

174+
useEffect(() => {
175+
const isRecoverableTab = tab === "settings" || tab === "marketplace"
176+
177+
if (showWelcome && settingsImportedAt && settingsImportedAt !== handledImportRef.current) {
178+
handledImportRef.current = settingsImportedAt
179+
if (!isRecoverableTab) {
180+
setCurrentSection("providers")
181+
setCurrentMarketplaceTab(undefined)
182+
setTab("settings")
183+
}
184+
}
185+
}, [showWelcome, settingsImportedAt, tab])
186+
172187
useEffect(() => {
173188
if (didHydrateState) {
174189
telemetryClient.updateTelemetryState(telemetrySetting, telemetryKey, machineId)
@@ -214,7 +229,9 @@ const App = () => {
214229

215230
// Do not conditionally load ChatView, it's expensive and there's state we
216231
// don't want to lose (user input, disableInput, askResponse promise, etc.)
217-
return showWelcome ? (
232+
const isSetupGatedTab = showWelcome && tab !== "settings" && tab !== "marketplace"
233+
234+
return isSetupGatedTab ? (
218235
<WelcomeView />
219236
) : (
220237
<>

webview-ui/src/__tests__/App.spec.tsx

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ vi.mock("@src/components/settings/SettingsView", () => ({
4747
},
4848
}))
4949

50+
vi.mock("@src/components/welcome/WelcomeViewProvider", () => ({
51+
__esModule: true,
52+
default: function WelcomeView() {
53+
return <div data-testid="welcome-view">Welcome View</div>
54+
},
55+
}))
56+
5057
vi.mock("@src/components/history/HistoryView", () => ({
5158
__esModule: true,
5259
default: function HistoryView({ onDone }: { onDone: () => void }) {
@@ -181,6 +188,15 @@ describe("App", () => {
181188
window.dispatchEvent(messageEvent)
182189
}
183190

191+
const createSetupIncompleteState = () => ({
192+
didHydrateState: true,
193+
showWelcome: true,
194+
shouldShowAnnouncement: false,
195+
experiments: {},
196+
language: "en",
197+
telemetrySetting: "enabled",
198+
})
199+
184200
it("shows chat view by default", () => {
185201
render(<AppWithProviders />)
186202

@@ -189,6 +205,22 @@ describe("App", () => {
189205
expect(chatView.getAttribute("data-hidden")).toBe("false")
190206
}, 10000)
191207

208+
it("shows welcome view when setup is incomplete", () => {
209+
mockUseExtensionState.mockReturnValue({
210+
didHydrateState: true,
211+
showWelcome: true,
212+
shouldShowAnnouncement: false,
213+
experiments: {},
214+
language: "en",
215+
telemetrySetting: "enabled",
216+
})
217+
218+
render(<AppWithProviders />)
219+
220+
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
221+
expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument()
222+
})
223+
192224
it("switches to settings view when receiving settingsButtonClicked action", async () => {
193225
render(<AppWithProviders />)
194226

@@ -203,6 +235,166 @@ describe("App", () => {
203235
expect(chatView.getAttribute("data-hidden")).toBe("true")
204236
})
205237

238+
it.each([
239+
["settings", "settings-view"],
240+
["marketplace", "marketplace-view"],
241+
])("still switches to %s while welcome gating is active", async (action, testId) => {
242+
mockUseExtensionState.mockReturnValue({
243+
didHydrateState: true,
244+
showWelcome: true,
245+
shouldShowAnnouncement: false,
246+
experiments: {},
247+
language: "en",
248+
telemetrySetting: "enabled",
249+
})
250+
251+
render(<AppWithProviders />)
252+
253+
act(() => {
254+
triggerMessage(`${action}ButtonClicked`)
255+
})
256+
257+
expect(await screen.findByTestId(testId)).toBeInTheDocument()
258+
expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument()
259+
})
260+
261+
it("keeps history behind the welcome gate while setup is incomplete", () => {
262+
mockUseExtensionState.mockReturnValue({
263+
didHydrateState: true,
264+
showWelcome: true,
265+
shouldShowAnnouncement: false,
266+
experiments: {},
267+
language: "en",
268+
telemetrySetting: "enabled",
269+
})
270+
271+
render(<AppWithProviders />)
272+
273+
act(() => {
274+
triggerMessage("historyButtonClicked")
275+
})
276+
277+
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
278+
expect(screen.queryByTestId("history-view")).not.toBeInTheDocument()
279+
})
280+
281+
it.each([
282+
{ label: "chat", action: undefined },
283+
{ label: "history", action: "historyButtonClicked" },
284+
])("redirects to providers settings when an import fires from the $label tab", async ({ action }) => {
285+
const state = {
286+
...createSetupIncompleteState(),
287+
settingsImportedAt: undefined as number | undefined,
288+
}
289+
290+
mockUseExtensionState.mockImplementation(() => state)
291+
292+
const { rerender } = render(<AppWithProviders />)
293+
294+
if (action) {
295+
act(() => {
296+
triggerMessage(action)
297+
})
298+
}
299+
300+
if (action === "historyButtonClicked") {
301+
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
302+
}
303+
304+
state.settingsImportedAt = Date.now()
305+
rerender(<AppWithProviders />)
306+
307+
expect(await screen.findByTestId("settings-view")).toBeInTheDocument()
308+
expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument()
309+
})
310+
311+
it.each([
312+
{
313+
label: "settings before returning to chat",
314+
action: "settingsButtonClicked",
315+
viewId: "settings-view",
316+
nextAction: undefined,
317+
},
318+
{
319+
label: "settings before switching to history",
320+
action: "settingsButtonClicked",
321+
viewId: "settings-view",
322+
nextAction: "historyButtonClicked",
323+
},
324+
{
325+
label: "marketplace before returning to chat",
326+
action: "marketplaceButtonClicked",
327+
viewId: "marketplace-view",
328+
nextAction: undefined,
329+
},
330+
{
331+
label: "marketplace before switching to history",
332+
action: "marketplaceButtonClicked",
333+
viewId: "marketplace-view",
334+
nextAction: "historyButtonClicked",
335+
},
336+
])(
337+
"consumes imported settings without a later redirect when already on $label",
338+
async ({ action, viewId, nextAction }) => {
339+
const state = {
340+
...createSetupIncompleteState(),
341+
settingsImportedAt: undefined as number | undefined,
342+
}
343+
344+
mockUseExtensionState.mockImplementation(() => state)
345+
346+
const { rerender } = render(<AppWithProviders />)
347+
348+
act(() => {
349+
triggerMessage(action)
350+
})
351+
352+
expect(await screen.findByTestId(viewId)).toBeInTheDocument()
353+
expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument()
354+
355+
state.settingsImportedAt = Date.now()
356+
rerender(<AppWithProviders />)
357+
358+
const currentView = await screen.findByTestId(viewId)
359+
expect(currentView).toBeInTheDocument()
360+
361+
if (nextAction) {
362+
act(() => {
363+
triggerMessage(nextAction)
364+
})
365+
} else {
366+
act(() => {
367+
currentView.click()
368+
})
369+
}
370+
371+
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
372+
expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument()
373+
expect(screen.queryByTestId("marketplace-view")).not.toBeInTheDocument()
374+
},
375+
)
376+
377+
it("does not bounce back to settings after the import redirect has already fired", async () => {
378+
const importedAt = Date.now()
379+
380+
mockUseExtensionState.mockReturnValue({
381+
...createSetupIncompleteState(),
382+
settingsImportedAt: importedAt,
383+
})
384+
385+
render(<AppWithProviders />)
386+
387+
const settingsView = await screen.findByTestId("settings-view")
388+
expect(settingsView).toBeInTheDocument()
389+
390+
act(() => {
391+
settingsView.click()
392+
})
393+
394+
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
395+
expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument()
396+
})
397+
206398
it("switches to history view when receiving historyButtonClicked action", async () => {
207399
render(<AppWithProviders />)
208400

0 commit comments

Comments
 (0)