Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions docs/issues/cua-settings-empty-message/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# CUA Settings Empty Message Plan

## Scope

The issue is isolated to the static Computer Use plugin settings UI in
`plugins/cua/settings/assets/index.js`.

## Implementation

- Keep `setText()` as the fallback helper for data rows that should display `Unknown`.
- Change `setMessage()` so the transient status/error area writes the provided text verbatim and
allows an empty string after success.
- Add a jsdom regression test that loads the static settings script, invokes the Check button, and
verifies the message area is empty after a successful permission check.

## Test Strategy

- Run the focused renderer test for the plugin settings script.
- Run repository-required formatting, i18n, and lint checks.

## Risks

- Low. The change only affects the bottom message element and keeps status rows unchanged.
22 changes: 22 additions & 0 deletions docs/issues/cua-settings-empty-message/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# CUA Settings Empty Message

## User Story

As a user checking Computer Use plugin permissions, I want the plugin settings page to show the
permission results only in the status rows so that a completed check does not leave an unrelated
`Unknown` message at the bottom of the page.

## Acceptance Criteria

- A successful permission check updates the Accessibility and Screen Recording rows.
- After a successful permission check with no diagnostic error, the bottom message area is empty.
- Runtime fields can still show `Unknown` when their underlying status values are unavailable.

## Non-goals

- Redesigning the plugin settings layout.
- Changing runtime or permission detection behavior.

## Open Questions

None.
6 changes: 6 additions & 0 deletions docs/issues/cua-settings-empty-message/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# CUA Settings Empty Message Tasks

- [x] Confirm the source of the bottom `Unknown` message.
- [x] Update message rendering to allow an empty success state.
- [x] Add focused regression coverage.
- [x] Run `pnpm run format`, `pnpm run i18n`, and `pnpm run lint`.
4 changes: 3 additions & 1 deletion plugins/cua/settings/assets/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ function setText(node, value) {
}

function setMessage(value) {
setText(messageNode, value)
if (messageNode) {
messageNode.textContent = value || ''
}
}

function setState(enabled) {
Expand Down
77 changes: 77 additions & 0 deletions test/renderer/plugins/cuaSettings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { readFile } from 'node:fs/promises'
import { resolve } from 'node:path'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const scriptPath = resolve(process.cwd(), 'plugins/cua/settings/assets/index.js')

const flushPromises = async (): Promise<void> => {
await new Promise((resolve) => setTimeout(resolve, 0))
}

const renderSettingsDom = (): void => {
document.body.innerHTML = `
<span id="plugin-state"></span>
<strong id="runtime-state"></strong>
<strong id="runtime-version"></strong>
<code id="runtime-command"></code>
<code id="runtime-helper-app"></code>
<strong id="mcp-state"></strong>
<strong id="permission-accessibility"></strong>
<strong id="permission-screen-recording"></strong>
<p id="message"></p>
<a id="project-link"></a>
<button id="check"></button>
<button id="guide"></button>
<button id="disable"></button>
`
}

type CuaSettingsWindow = Window & { deepchatPlugin?: unknown }

describe('CUA plugin settings', () => {
beforeEach(() => {
renderSettingsDom()
delete (window as CuaSettingsWindow).deepchatPlugin
})

it('clears the bottom message after a successful permission check', async () => {
const pluginWindow = window as CuaSettingsWindow

pluginWindow.deepchatPlugin = {
getStatus: vi.fn().mockResolvedValue({
enabled: true,
runtime: {
state: 'ready',
version: '0.1.5',
command: '/mock/cua-driver',
helperAppPath: '/mock/DeepChat Computer Use.app'
},
mcpServers: [
{
serverId: 'cua-driver',
enabled: true,
running: true
}
]
}),
invokeAction: vi.fn().mockResolvedValue({
ok: true,
data: {
accessibility: 'granted',
screenRecording: 'denied'
}
}),
disable: vi.fn()
}

window.eval(await readFile(scriptPath, 'utf8'))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify there are no eval usages left in renderer tests.
rg -nP --type=ts '\b(?:window\.)?eval\s*\(' test/renderer

# Verify the CUA settings test now loads via import().
rg -n -C2 'pathToFileURL|import\(' test/renderer/plugins/cuaSettings.test.ts

Repository: ThinkInAIXYZ/deepchat

Length of output: 162


Replace window.eval to avoid security lint failure.

Line 67 uses window.eval, which is flagged by lint/security/noGlobalEval and will fail required lint checks.

🔧 Proposed fix (load script via ESM import)
-import { readFile } from 'node:fs/promises'
 import { resolve } from 'node:path'
+import { pathToFileURL } from 'node:url'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
@@
-    window.eval(await readFile(scriptPath, 'utf8'))
+    await import(`${pathToFileURL(scriptPath).href}?t=${Date.now()}`)
     await flushPromises()
📝 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.

Suggested change
window.eval(await readFile(scriptPath, 'utf8'))
import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'
import { beforeEach, describe, expect, it, vi } from 'vitest'
// ... other code ...
await import(`${pathToFileURL(scriptPath).href}?t=${Date.now()}`)
await flushPromises()
🧰 Tools
🪛 Biome (2.4.14)

[error] 67-67: eval() exposes to security risks and performance issues.

(lint/security/noGlobalEval)

🤖 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 `@test/renderer/plugins/cuaSettings.test.ts` at line 67, The test currently
uses window.eval(await readFile(scriptPath, 'utf8')) which triggers the security
lint rule noGlobalEval; replace this with an ESM dynamic import of the script
file instead (e.g., convert scriptPath into a file:// URL with url.pathToFileURL
and await import(fileUrl)) so the module is loaded as an ES module rather than
evaluated via global eval; update the test to await import(fileUrl) and remove
window.eval usage while ensuring the test environment supports dynamic import.

await flushPromises()

document.getElementById('check')?.click()
await flushPromises()

expect(document.getElementById('permission-accessibility')?.textContent).toBe('Granted')
expect(document.getElementById('permission-screen-recording')?.textContent).toBe('Denied')
expect(document.getElementById('message')?.textContent).toBe('')
})
})