Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8449d8b
feat(webview): per-view view-state identity and host-minted claim tok…
easonLiangWorldedtech Nov 5, 2026
9bd8aca
chore: strip BOM and fix lint imports
easonLiangWorldedtech Oct 29, 2026
e1afe96
fix(webview): address review findings on view-state durability
easonLiangWorldedtech Oct 29, 2026
1986816
chore: tighten the view-identity comments
easonLiangWorldedtech Oct 30, 2026
5cc1245
fix(webview): load persisted selections when a view id is registered
easonLiangWorldedtech Oct 30, 2026
adf9d2f
chore: tighten the shared view-state docs
easonLiangWorldedtech Oct 30, 2026
6f01d18
fix(webview): track buffer writes so a same-value save survives the load
easonLiangWorldedtech Oct 30, 2026
14a136a
chore: tighten the N1 test comments
easonLiangWorldedtech Oct 30, 2026
51982e7
fix(lint): drop the now-unused load snapshot and log spy
easonLiangWorldedtech Oct 30, 2026
7620684
fix(webview): harden viewStates reads and retry an unrestored view state
easonLiangWorldedtech Nov 3, 2026
d7145a8
fix(config): drop viewStates from settings imports
easonLiangWorldedtech Nov 4, 2026
86461a5
fix(webview): await view-state claim delivery and report an undeliver…
easonLiangWorldedtech Nov 4, 2026
3483702
test(webview): cover malformed view id rejections in setViewStateId
easonLiangWorldedtech Nov 4, 2026
66b5499
fix(webview): cancel queued view-state work with a disposal generation
easonLiangWorldedtech Nov 4, 2026
f402de7
fix(webview): report claim delivery and retry an undelivered claim on…
easonLiangWorldedtech Nov 4, 2026
583ed51
fix(webview): refuse non-portable settings keys posted by the webview
easonLiangWorldedtech Nov 4, 2026
2f2485e
fix(webview): hold view-state claims until the renderer launches and …
easonLiangWorldedtech Nov 4, 2026
6b76027
fix(webview): reset isViewLaunched when the renderer is replaced
easonLiangWorldedtech Nov 5, 2026
8b863f8
fix(webview): close the claim-boundary and persistence gaps from review
easonLiangWorldedtech Nov 5, 2026
733657f
fix(webview): detach rejected writes and restore view-local fields
easonLiangWorldedtech Nov 5, 2026
efbad6b
Merge upstream/main into n1-view-identity
easonLiangWorldedtech Nov 5, 2026
71277fa
style(webview): fold guard blocks that add no behaviour
easonLiangWorldedtech Nov 5, 2026
e9d0a2a
fix(extension): keep host-owned view states out of API.getConfigurati…
easonLiangWorldedtech Nov 5, 2026
e7a09f0
test(core): cover the export omission, the webview post result, and i…
easonLiangWorldedtech Nov 5, 2026
f3fa813
refactor(core): fold three copies of the view-state queue call into o…
easonLiangWorldedtech Nov 5, 2026
abe5683
test(extension): cover secret filtering and export retention, type th…
easonLiangWorldedtech Nov 5, 2026
3dd0854
fix(webview): undo a re-key whose view died before the entry reached …
easonLiangWorldedtech Nov 5, 2026
bc724b8
fix(webview): keep memory and storage agreeing when a re-key recovery…
easonLiangWorldedtech Nov 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions packages/types/src/__tests__/global-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,49 @@ describe("alwaysDenyUnapprovedCommands global setting", () => {
})
})

describe("viewStates global setting", () => {
it("parses a valid per-view record", () => {
const parsed = globalSettingsSchema.parse({
viewStates: {
"stable-view-1": { mode: "code", currentApiConfigName: "profile-a", updatedAt: 1730000000000 },
"stable-view-2": {},
},
})
expect(parsed.viewStates?.["stable-view-1"]).toEqual({
mode: "code",
currentApiConfigName: "profile-a",
updatedAt: 1730000000000,
})
expect(parsed.viewStates?.["stable-view-2"]).toEqual({})
expect(GLOBAL_SETTINGS_KEYS).toContain("viewStates")
})

it("parses an entry that carries the host-minted claim token", () => {
// The claim binds the entry to the webview that wrote it; a legacy entry without one
// still parses, because refusing it is setViewStateId's job, not the schema's.
const parsed = globalSettingsSchema.parse({
viewStates: {
"stable-view-1": { mode: "code", updatedAt: 1730000000000, claimToken: "7f9c-host-minted" },
"stable-view-2": { mode: "code" },
},
})
expect(parsed.viewStates?.["stable-view-1"]?.claimToken).toBe("7f9c-host-minted")
expect(parsed.viewStates?.["stable-view-2"]).toEqual({ mode: "code" })
})

it("rejects invalid field values inside a view entry", () => {
expect(() => globalSettingsSchema.parse({ viewStates: { v: { mode: 42 } } })).toThrow()
expect(() => globalSettingsSchema.parse({ viewStates: { v: { currentApiConfigName: 42 } } })).toThrow()
expect(() => globalSettingsSchema.parse({ viewStates: { v: { updatedAt: "soon" } } })).toThrow()
expect(() => globalSettingsSchema.parse({ viewStates: { v: { claimToken: 42 } } })).toThrow()
})

it("rejects non-object viewStates values", () => {
expect(() => globalSettingsSchema.parse({ viewStates: "not-a-record" })).toThrow()
expect(() => globalSettingsSchema.parse({ viewStates: { v: "not-an-entry" } })).toThrow()
})
})

describe("collapseCommandOutputByDefault global setting", () => {
it.each([true, false, undefined])("round-trips %s through the export/import schema", (value) => {
const settings = globalSettingsSchema.parse({ collapseCommandOutputByDefault: value })
Expand Down
5 changes: 5 additions & 0 deletions packages/types/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import { GLOBAL_STATE_KEYS } from "../index.js"

describe("GLOBAL_STATE_KEYS", () => {
it("should contain registered durable per-view state", () => {
expect(GLOBAL_STATE_KEYS).toContain("viewStates")
})

it("should contain provider settings keys", () => {
expect(GLOBAL_STATE_KEYS).toContain("autoApprovalEnabled")
})
Expand All @@ -13,6 +17,7 @@ describe("GLOBAL_STATE_KEYS", () => {

it("should not contain secret state keys", () => {
expect(GLOBAL_STATE_KEYS).not.toContain("openRouterApiKey")
expect(GLOBAL_STATE_KEYS).not.toContain("apiKey")
})

it("should contain OpenAI Compatible base URL setting", () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ export const MAX_CHECKPOINT_TIMEOUT_SECONDS = 60
*/
export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15

/**
* Persisted non-secret selections for a stable webview instance.
*/
export const viewStateSchema = z.object({
mode: z.string().optional(),
currentApiConfigName: z.string().optional(),
updatedAt: z.number().optional(),
/** Host-minted token binding this entry to the webview that wrote it. */
claimToken: z.string().optional(),
})

/**
* GlobalSettings
*/
Expand All @@ -122,6 +133,7 @@ export const globalSettingsSchema = z.object({
currentApiConfigName: z.string().optional(),
listApiConfigMeta: z.array(providerSettingsEntrySchema).optional(),
pinnedApiConfigs: z.record(z.string(), z.boolean()).optional(),
viewStates: z.record(z.string(), viewStateSchema).optional(),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

lastShownAnnouncementId: z.string().optional(),
customInstructions: z.string().optional(),
Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/vscode-extension-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export interface ExtensionMessage {
| "rooHistoryImportProgress"
| "themeFixtureProbeRequest"
| "diagnosticsRequest"
| "viewStateClaim"
text?: string
/** For fileContent: { path, content, error? } */
fileContent?: { path: string; content: string | null; error?: string }
Expand Down Expand Up @@ -171,6 +172,13 @@ export interface ExtensionMessage {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
values?: Record<string, any>
requestId?: string
/**
* For `viewStateClaim`: the durable `viewStates` key the delivered claim governs. The
* message is only ever sent to the webview that key belongs to.
*/
viewStateId?: string
/** For `viewStateClaim`: the host-minted token the webview stores and echoes on launch. */
viewStateClaim?: string
themeFixture?: WebviewThemeFixture
diagnostics?: WebviewDiagnosticsSnapshot
promptText?: string
Expand Down Expand Up @@ -712,6 +720,9 @@ export interface WebviewMessage {
| "themeFixtureProbeResponse"
| "diagnosticsResponse"
text?: string
viewStateId?: string
/** For `webviewDidLaunch`: the claim delivered for `viewStateId`, enforced by setViewStateId. */
viewStateClaim?: string
taskId?: string
editedMessageContent?: string
tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud"
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/extension.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ describe("extension.ts", () => {
const getAll = vi.mocked(ClineProvider.getAllInstances)
const post = vi.mocked(provider.postMessageToWebview)
getAll.mockReturnValue([provider, provider])
post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(undefined)
post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(true)
try {
const { activate } = await import("../extension")
await activate(mockContext)
Expand Down
24 changes: 23 additions & 1 deletion src/core/config/ContextProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,21 @@ const globalSettingsExportSchema = globalSettingsSchema.omit({
taskHistory: true,
listApiConfigMeta: true,
currentApiConfigName: true,
// Host-minted claim tokens and per-view selections are not user-settings: exporting
// them would let a settings file be replayed as another view's identity.
viewStates: true,
})

/**
* Global settings a settings file may not carry in either direction. Per-view selections and the
* claim token that binds them are minted and held by the extension host, so a file that could
* set them would be replayable as another view's identity. The export side omits the key above;
* the import side consults this list in `sanitizeGlobalSettings`.
*/
const NON_PORTABLE_SETTINGS_KEYS: readonly string[] = ["viewStates"]

export const isNonPortableSettingsKey = (key: string): boolean => NON_PORTABLE_SETTINGS_KEYS.includes(key)

export class ContextProxy {
private readonly originalContext: vscode.ExtensionContext

Expand Down Expand Up @@ -666,7 +679,16 @@ export class ContextProxy {
* rollback, must provide it themselves.
*/
public async setValues(values: RooCodeSettings) {
const entries = Object.entries(values) as [RooCodeSettingsKey, unknown][]
// viewStates is host-owned: per-view selections and the claim token that binds them are
// minted by the extension host, so no configuration path may carry them. Every caller of
// this setter takes input that did not originate inside the host - a posted updateSettings,
// a newTask taskConfiguration, the external API, an imported settings file - so the filter
// lives here rather than at whichever caller a reviewer happened to point at. The host's
// own view-state methods write through setValue("viewStates", ...), which stays open.
const entries = Object.entries(values).filter(([key]) => !isNonPortableSettingsKey(key)) as [
RooCodeSettingsKey,
unknown,
][]
const results = await Promise.allSettled(entries.map(([key, value]) => this.setValue(key, value)))

const committed: string[] = []
Expand Down
15 changes: 14 additions & 1 deletion src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ export const providerProfilesSchema = z.object({

export type ProviderProfiles = z.infer<typeof providerProfilesSchema>

/** Already-deleted profile: callers branch on this type instead of matching message text. */
export class ProviderSettingsNotFoundError extends Error {
constructor(message: string) {
super(message)
this.name = "ProviderSettingsNotFoundError"
}
}

export class ProviderSettingsManager {
private static readonly SCOPE_PREFIX = "roo_cline_config_"
private readonly defaultConfigId = this.generateId()
Expand Down Expand Up @@ -511,7 +519,7 @@ export class ProviderSettingsManager {
const providerProfiles = await this.load()

if (!providerProfiles.apiConfigs[name]) {
throw new Error(`Config '${name}' not found`)
throw new ProviderSettingsNotFoundError(`Config '${name}' not found`)
}

if (Object.keys(providerProfiles.apiConfigs).length === 1) {
Expand All @@ -522,6 +530,11 @@ export class ProviderSettingsManager {
await this.store(providerProfiles)
})
} catch (error) {
// A missing config is a caller-meaningful signal, not a failure: rethrow it
// unwrapped so callers can branch on the type instead of message text.
if (error instanceof ProviderSettingsNotFoundError) {
throw error
}
throw new Error(`Failed to delete config: ${error}`)
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/core/config/__tests__/ContextProxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,25 @@ describe("ContextProxy", () => {
})
})

describe("export()", () => {
it("omits host-owned viewStates from a settings export", async () => {
await proxy.setValue("viewStates", {
"export-view": { claimToken: "export-claim-token", mode: "architect" },
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// A portable global setting the export schema keeps: without it the test would pass on an
// empty export, because every remaining check only asserts absence.
await proxy.setValue("customInstructions", "retain these instructions")
// The cache carries the key; only the export must not.
expect(Object.keys(proxy.getValues())).toContain("viewStates")
const exported = await proxy.export()
expect(exported?.customInstructions).toBe("retain these instructions")
expect(exported && Object.keys(exported)).not.toContain("viewStates")
expect(exported && "viewStates" in exported).toBe(false)
// The value must not escape under another name either: the claim token is host state.
expect(JSON.stringify(exported)).not.toContain("export-claim-token")
})
})

describe("resetAllState", () => {
it("should clear all in-memory caches", async () => {
// Setup initial state in caches
Expand Down
7 changes: 6 additions & 1 deletion src/core/config/__tests__/ProviderSettingsManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { clearAllMocks } from "../../../test-utils/reset"
import { makeExtensionContext } from "../../../test-utils/vscode"

import { ProviderSettingsManager, ProviderProfiles } from "../ProviderSettingsManager"
import { ProviderSettingsManager, ProviderSettingsNotFoundError, ProviderProfiles } from "../ProviderSettingsManager"

// `export()` builds an API handler per profile to read model capabilities. Mock
// buildApiHandler with the real @roo-code/types model definitions so the token-field
Expand Down Expand Up @@ -973,6 +973,11 @@ describe("ProviderSettingsManager", () => {
}),
)

// The typed not-found signal is the contract callers branch on: a profile
// name containing "not found" must not be matchable via message text.
await expect(providerSettingsManager.deleteConfig("nonexistent")).rejects.toBeInstanceOf(
ProviderSettingsNotFoundError,
)
await expect(providerSettingsManager.deleteConfig("nonexistent")).rejects.toThrow(
"Config 'nonexistent' not found",
)
Expand Down
49 changes: 49 additions & 0 deletions src/core/config/__tests__/importExport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,55 @@ describe("importExport", () => {
})
})

describe("host-owned view state on import", () => {
const settingsWithImportedViewStates = JSON.stringify({
providerProfiles: {
currentApiConfigName: "test",
apiConfigs: {
test: { apiProvider: providerIdentifiers.openai, apiKey: "test-key", id: "test-id" },
},
},
globalSettings: {
customInstructions: "keep me",
viewStates: { "view-1": { mode: "architect", claimToken: "planted-claim" } },
},
})

beforeEach(() => {
;(fs.readFile as Mock).mockResolvedValue(settingsWithImportedViewStates)
mockProviderSettingsManager.export.mockResolvedValue({
currentApiConfigName: "default",
apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } },
})
mockProviderSettingsManager.import.mockResolvedValue(undefined)
mockProviderSettingsManager.listConfig.mockResolvedValue([])
mockContextProxy.setValues.mockResolvedValue(undefined)
mockContextProxy.setValue.mockResolvedValue(undefined)
mockContextProxy.setProviderSettings.mockResolvedValue(undefined)
})

it("drops viewStates so an imported file cannot plant another view's claim", async () => {
const result = await importSettingsFromPath("/mock/path/settings.json", {
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
})

expect(result).toMatchObject({ success: true })

// A claim token is host-minted: a file that could carry an entry could replay it as that
// view's identity, so the key is dropped before anything is written.
expect(mockContextProxy.setValues).toHaveBeenCalledTimes(1)
const [appliedSettings] = (mockContextProxy.setValues as Mock).mock.calls[0]
expect(appliedSettings).not.toHaveProperty("viewStates")
expect(appliedSettings).toMatchObject({ customInstructions: "keep me" })
const warnings = (result as { warnings?: string[] }).warnings ?? []
expect(warnings).toHaveLength(1)
expect(warnings[0]).toContain('globalSettings.viewStates" was skipped')
expect(warnings[0]).toContain("not portable")
})
})

describe("importSettings", () => {
it("should return success: false when user cancels file selection", async () => {
;(vscode.window.showOpenDialog as Mock).mockResolvedValue(undefined)
Expand Down
7 changes: 6 additions & 1 deletion src/core/config/importExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { TelemetryService } from "@roo-code/telemetry"

import { ProviderSettingsManager, providerProfilesSchema } from "./ProviderSettingsManager"
import { ContextProxy } from "./ContextProxy"
import { ContextProxy, isNonPortableSettingsKey } from "./ContextProxy"
import { CustomModesManager } from "./CustomModesManager"
import { downgradeLegacyRooConfig, ROUTER_REMOVAL_IMPORT_WARNING } from "./routerRemoval"
import { resolveDefaultSaveUri, saveLastExportPath } from "../../utils/export"
Expand Down Expand Up @@ -105,6 +105,11 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): {
continue
}

if (isNonPortableSettingsKey(key)) {
warnings.push(`Setting "${path}" was skipped: host-owned view state is not portable.`)
continue
}

let valueToValidate = rawValue

if (key === "imageGenerationProvider" && rawValue === retiredProviderIdentifiers.roo) {
Expand Down
Loading
Loading