Skip to content

Commit 92e53ef

Browse files
mrubensvalekseev
authored andcommitted
MDM check improvements (RooCodeInc#4770)
* Better check for production clerk * Send to account tab if required * Fix tests
1 parent ca18f13 commit 92e53ef

File tree

6 files changed

+51
-30
lines changed

6 files changed

+51
-30
lines changed

packages/cloud/src/Config.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,7 @@
1-
export const getClerkBaseUrl = () => process.env.CLERK_BASE_URL || "https://clerk.roocode.com"
2-
export const getRooCodeApiUrl = () => process.env.ROO_CODE_API_URL || "https://app.roocode.com"
1+
// Production constants
2+
export const PRODUCTION_CLERK_BASE_URL = "https://clerk.roocode.com"
3+
export const PRODUCTION_ROO_CODE_API_URL = "https://app.roocode.com"
4+
5+
// Functions with environment variable fallbacks
6+
export const getClerkBaseUrl = () => process.env.CLERK_BASE_URL || PRODUCTION_CLERK_BASE_URL
7+
export const getRooCodeApiUrl = () => process.env.ROO_CODE_API_URL || PRODUCTION_ROO_CODE_API_URL

src/core/webview/ClineProvider.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -556,11 +556,6 @@ export class ClineProvider
556556
>
557557
> = {},
558558
) {
559-
// Check MDM compliance before proceeding
560-
if (!this.checkMdmCompliance()) {
561-
return // Block task creation if not compliant
562-
}
563-
564559
const {
565560
apiConfiguration,
566561
organizationAllowList,
@@ -1280,6 +1275,11 @@ export class ClineProvider
12801275

12811276
// Update VSCode context for experiments
12821277
await this.updateVSCodeContext()
1278+
1279+
// Check MDM compliance and send user to account tab if not compliant
1280+
if (!this.checkMdmCompliance()) {
1281+
await this.postMessageToWebview({ type: "action", action: "accountButtonClicked" })
1282+
}
12831283
}
12841284

12851285
/**
@@ -1494,6 +1494,7 @@ export class ClineProvider
14941494
codebaseIndexEmbedderBaseUrl: "",
14951495
codebaseIndexEmbedderModelId: "",
14961496
},
1497+
mdmCompliant: this.checkMdmCompliance(),
14971498
}
14981499
}
14991500

@@ -1749,11 +1750,6 @@ export class ClineProvider
17491750
const compliance = this.mdmService.isCompliant()
17501751

17511752
if (!compliance.compliant) {
1752-
vscode.window.showErrorMessage(compliance.reason, "Sign In").then((selection) => {
1753-
if (selection === "Sign In") {
1754-
this.postMessageToWebview({ type: "action", action: "accountButtonClicked" })
1755-
}
1756-
})
17571753
return false
17581754
}
17591755

src/services/mdm/MdmService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as os from "os"
44
import * as vscode from "vscode"
55
import { z } from "zod"
66

7-
import { CloudService } from "@roo-code/cloud"
7+
import { CloudService, getClerkBaseUrl, PRODUCTION_CLERK_BASE_URL } from "@roo-code/cloud"
88
import { Package } from "../../shared/package"
99

1010
// MDM Configuration Schema
@@ -145,7 +145,7 @@ export class MdmService {
145145
*/
146146
private getMdmConfigPath(): string {
147147
const platform = os.platform()
148-
const isProduction = process.env.NODE_ENV === "production"
148+
const isProduction = getClerkBaseUrl() === PRODUCTION_CLERK_BASE_URL
149149
const configFileName = isProduction ? "mdm.json" : "mdm.dev.json"
150150

151151
switch (platform) {

src/services/mdm/__tests__/MdmService.spec.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ vi.mock("@roo-code/cloud", () => ({
1919
getOrganizationId: vi.fn(),
2020
},
2121
},
22+
getClerkBaseUrl: vi.fn(),
23+
PRODUCTION_CLERK_BASE_URL: "https://clerk.roocode.com",
2224
}))
2325

2426
vi.mock("vscode", () => ({
@@ -44,12 +46,13 @@ import * as fs from "fs"
4446
import * as os from "os"
4547
import * as vscode from "vscode"
4648
import { MdmService } from "../MdmService"
47-
import { CloudService } from "@roo-code/cloud"
49+
import { CloudService, getClerkBaseUrl, PRODUCTION_CLERK_BASE_URL } from "@roo-code/cloud"
4850

4951
const mockFs = fs as any
5052
const mockOs = os as any
5153
const mockCloudService = CloudService as any
5254
const mockVscode = vscode as any
55+
const mockGetClerkBaseUrl = getClerkBaseUrl as any
5356

5457
describe("MdmService", () => {
5558
let originalPlatform: string
@@ -64,6 +67,9 @@ describe("MdmService", () => {
6467
// Set default platform for tests
6568
mockOs.platform.mockReturnValue("darwin")
6669

70+
// Setup default mock for getClerkBaseUrl to return development URL
71+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
72+
6773
// Setup VSCode mocks
6874
const mockConfig = {
6975
get: vi.fn().mockReturnValue(false),
@@ -73,6 +79,8 @@ describe("MdmService", () => {
7379

7480
// Reset mocks
7581
vi.clearAllMocks()
82+
// Re-setup the default after clearing
83+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
7684
})
7785

7886
afterEach(() => {
@@ -142,7 +150,7 @@ describe("MdmService", () => {
142150
it("should use correct path for Windows in production", async () => {
143151
mockOs.platform.mockReturnValue("win32")
144152
process.env.PROGRAMDATA = "C:\\ProgramData"
145-
process.env.NODE_ENV = "production"
153+
mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
146154

147155
mockFs.existsSync.mockReturnValue(false)
148156

@@ -154,7 +162,7 @@ describe("MdmService", () => {
154162
it("should use correct path for Windows in development", async () => {
155163
mockOs.platform.mockReturnValue("win32")
156164
process.env.PROGRAMDATA = "C:\\ProgramData"
157-
process.env.NODE_ENV = "development"
165+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
158166

159167
mockFs.existsSync.mockReturnValue(false)
160168

@@ -165,7 +173,7 @@ describe("MdmService", () => {
165173

166174
it("should use correct path for macOS in production", async () => {
167175
mockOs.platform.mockReturnValue("darwin")
168-
process.env.NODE_ENV = "production"
176+
mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
169177

170178
mockFs.existsSync.mockReturnValue(false)
171179

@@ -176,7 +184,7 @@ describe("MdmService", () => {
176184

177185
it("should use correct path for macOS in development", async () => {
178186
mockOs.platform.mockReturnValue("darwin")
179-
process.env.NODE_ENV = "development"
187+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
180188

181189
mockFs.existsSync.mockReturnValue(false)
182190

@@ -187,7 +195,7 @@ describe("MdmService", () => {
187195

188196
it("should use correct path for Linux in production", async () => {
189197
mockOs.platform.mockReturnValue("linux")
190-
process.env.NODE_ENV = "production"
198+
mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
191199

192200
mockFs.existsSync.mockReturnValue(false)
193201

@@ -198,7 +206,7 @@ describe("MdmService", () => {
198206

199207
it("should use correct path for Linux in development", async () => {
200208
mockOs.platform.mockReturnValue("linux")
201-
process.env.NODE_ENV = "development"
209+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
202210

203211
mockFs.existsSync.mockReturnValue(false)
204212

@@ -209,7 +217,7 @@ describe("MdmService", () => {
209217

210218
it("should default to dev config when NODE_ENV is not set", async () => {
211219
mockOs.platform.mockReturnValue("darwin")
212-
delete process.env.NODE_ENV
220+
mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
213221

214222
mockFs.existsSync.mockReturnValue(false)
215223

@@ -248,6 +256,7 @@ describe("MdmService", () => {
248256
mockFs.existsSync.mockReturnValue(true)
249257
mockFs.readFileSync.mockReturnValue(JSON.stringify(mockConfig))
250258

259+
// Mock CloudService to indicate no instance or no active session
251260
mockCloudService.hasInstance.mockReturnValue(false)
252261

253262
const service = await MdmService.createInstance()
@@ -267,6 +276,7 @@ describe("MdmService", () => {
267276
mockFs.existsSync.mockReturnValue(true)
268277
mockFs.readFileSync.mockReturnValue(JSON.stringify(mockConfig))
269278

279+
// Mock CloudService to have instance and active session but wrong org
270280
mockCloudService.hasInstance.mockReturnValue(true)
271281
mockCloudService.instance.hasActiveSession.mockReturnValue(true)
272282
mockCloudService.instance.getOrganizationId.mockReturnValue("different-org-456")

webview-ui/src/App.tsx

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const App = () => {
4242
experiments,
4343
cloudUserInfo,
4444
cloudIsAuthenticated,
45+
mdmCompliant,
4546
} = useExtensionState()
4647

4748
// Create a persistent state manager
@@ -63,15 +64,23 @@ const App = () => {
6364
const settingsRef = useRef<SettingsViewRef>(null)
6465
const chatViewRef = useRef<ChatViewRef>(null)
6566

66-
const switchTab = useCallback((newTab: Tab) => {
67-
setCurrentSection(undefined)
67+
const switchTab = useCallback(
68+
(newTab: Tab) => {
69+
// Check MDM compliance before allowing tab switching
70+
if (mdmCompliant === false && newTab !== "account") {
71+
return
72+
}
6873

69-
if (settingsRef.current?.checkUnsaveChanges) {
70-
settingsRef.current.checkUnsaveChanges(() => setTab(newTab))
71-
} else {
72-
setTab(newTab)
73-
}
74-
}, [])
74+
setCurrentSection(undefined)
75+
76+
if (settingsRef.current?.checkUnsaveChanges) {
77+
settingsRef.current.checkUnsaveChanges(() => setTab(newTab))
78+
} else {
79+
setTab(newTab)
80+
}
81+
},
82+
[mdmCompliant],
83+
)
7584

7685
const [currentSection, setCurrentSection] = useState<string | undefined>(undefined)
7786

webview-ui/src/context/ExtensionStateContext.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export interface ExtensionStateContextType extends ExtensionState {
3737
cloudIsAuthenticated: boolean
3838
sharingEnabled: boolean
3939
maxConcurrentFileReads?: number
40+
mdmCompliant?: boolean
4041
condensingApiConfigId?: string
4142
setCondensingApiConfigId: (value: string) => void
4243
customCondensingPrompt?: string

0 commit comments

Comments
 (0)