fix(computer-use): address modifier safety review

config/scripts/computer-use-modifier-safety.test.mjs

@@ -8,36 +8,47 @@ function source(path) {
   return readFileSync(join(projectDir, path), 'utf8')
 }
 
+function sourceBetween(contents, startMarker, endMarker) {
+  const start = contents.indexOf(startMarker)
+  const end = contents.indexOf(endMarker, start + startMarker.length)
+  if (start < 0 || end < 0) {
+    throw new Error(`Missing source boundary: ${startMarker} → ${endMarker}`)
+  }
+  return contents.slice(start, end)
+}
+
 describe('computer-use modifier safety', () => {
   it('uses mouse-event flags instead of held modifier keys on macOS', () => {
     const macOS = source('native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift')
-    const clickInput = macOS.slice(
-      macOS.indexOf('static func click('),
-      macOS.indexOf('static func scroll(')
-    )
-    const mouseInput = macOS.slice(
-      macOS.indexOf('private static func mouse('),
-      macOS.indexOf('private static func keyEvent(')
+    const clickInput = sourceBetween(macOS, 'static func click(', 'static func scroll(')
+    const mouseInput = sourceBetween(
+      macOS,
+      'private static func mouse(',
+      'private static func keyEvent('
     )
 
     expect(mouseInput).toContain('event.flags = flags')
+    expect(clickInput.match(/flags: flags/gu)).toHaveLength(3)
     expect(clickInput).not.toContain('down: true')
   })
 
   it('submits each modified Windows click in a closed, timed SendInput batch', () => {
     const windows = source('native/computer-use-windows/runtime.ps1')
-    const modifiedClick = windows.slice(
-      windows.indexOf('public static void SendModifiedClick'),
-      windows.indexOf('private static INPUT KeyboardInput')
+    const modifiedClick = sourceBetween(
+      windows,
+      'public static void SendModifiedClick',
+      'private static INPUT KeyboardInput'
     )
-    const mouseClick = windows.slice(
-      windows.indexOf('function Send-OrcaMouseClick'),
-      windows.indexOf('function Send-OrcaDrag')
+    const mouseClick = sourceBetween(
+      windows,
+      'function Send-OrcaMouseClick',
+      'function Send-OrcaDrag'
     )
 
     expect(modifiedClick).toContain('SendInput((uint)values.Length, values')
     expect(modifiedClick).toContain('SendInput((uint)releaseValues.Length, releaseValues')
     expect(modifiedClick).toContain('if (sent != (uint)values.Length)')
+    expect(modifiedClick).toContain('releases.Add(MouseInput(mouseInput, mouseUp))')
     expect(modifiedClick).not.toContain('int count')
     expect(mouseClick).toMatch(
       /for \(\$i = 0; \$i -lt \$clickCount; \$i\+\+\) \{\s+\[OrcaDesktopWin32\]::SendModifiedClick\(/
@@ -48,10 +59,7 @@ describe('computer-use modifier safety', () => {
 
   it('keeps Linux modifier release in the xdotool sequence and a fallback', () => {
     const linux = source('native/computer-use-linux/runtime.py')
-    const modifiedClick = linux.slice(
-      linux.indexOf('def modified_click_at('),
-      linux.indexOf('def scroll_at(')
-    )
+    const modifiedClick = sourceBetween(linux, 'def modified_click_at(', 'def scroll_at(')
 
     expect(modifiedClick).toContain('command.extend(["keyup", modifier])')
     expect(modifiedClick).toContain('is_wayland')

config/scripts/computer-use-skill-guidance.test.mjs

@@ -1,6 +1,7 @@
 import { readFileSync } from 'node:fs'
 import { join, resolve } from 'node:path'
 import { describe, expect, it } from 'vitest'
+import { BUNDLED_SKILL_GUIDES } from '../../src/cli/bundled-skill-guides'
 
 const projectDir = resolve(import.meta.dirname, '../..')
 // Why: computer-use now ships a hybrid discovery stub, so its version-sensitive command
@@ -8,6 +9,7 @@ const projectDir = resolve(import.meta.dirname, '../..')
 // installable stub projection is checked separately below.
 const guidePath = join(projectDir, 'skill-guides', 'computer-use.md')
 const stubPath = join(projectDir, 'skills', 'computer-use', 'SKILL.md')
+const bundledGuide = BUNDLED_SKILL_GUIDES.find((guide) => guide.name === 'computer-use')?.markdown
 
 describe('computer-use skill guidance', () => {
   it('keeps web-app targeting on the computer-use surface', () => {
@@ -45,11 +47,13 @@ describe('computer-use skill guidance', () => {
     expect(skill).not.toContain('`result.elements`')
   })
 
-  it('requires atomic modifier-click actions', () => {
-    const skill = readFileSync(guidePath, 'utf8')
+  it('requires atomic modifier-click actions in the source and bundled guide', () => {
+    expect(bundledGuide).toBeDefined()
 
-    expect(skill).toContain('click --modifiers <chord>')
-    expect(skill).toContain('Never synthesize separate modifier-down and modifier-up commands')
+    for (const skill of [readFileSync(guidePath, 'utf8'), bundledGuide]) {
+      expect(skill).toContain('click --modifiers <chord>')
+      expect(skill).toContain('Never synthesize separate modifier-down and modifier-up commands')
+    }
   })
 })
 

native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift

@@ -2358,8 +2358,8 @@ private enum Input {
         var pressedModifiers: [KeyModifier] = []
         defer {
             for modifier in pressedModifiers.reversed() {
-                try? keyEvent(modifier.keyCode, down: false, flags: flags, pid: pid)
                 flags.remove(modifier.flag)
+                try? keyEvent(modifier.keyCode, down: false, flags: flags, pid: pid)
             }
         }
         for modifier in parsed.modifiers {

native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AgentEntrypointSourceSafetyTests.swift

@@ -23,7 +23,14 @@ final class AgentEntrypointSourceSafetyTests: XCTestCase {
         let source = try agentEntrypointSource()
 
         XCTAssertTrue(source.contains("var pressedModifiers: [KeyModifier] = []"))
-        XCTAssertTrue(source.contains("defer {\n            for modifier in pressedModifiers.reversed()"))
+        XCTAssertTrue(source.contains(
+            """
+            defer {
+                        for modifier in pressedModifiers.reversed() {
+                            flags.remove(modifier.flag)
+                            try? keyEvent(modifier.keyCode, down: false, flags: flags, pid: pid)
+            """
+        ))
         XCTAssertTrue(source.contains("event.flags = flags\n        event.postToPid(pid)"))
     }
 

native/computer-use-windows/runtime.ps1

@@ -111,6 +111,7 @@ public static class OrcaDesktopWin32 {
         var sent = SendInput((uint)values.Length, values, Marshal.SizeOf(typeof(INPUT)));
         if (sent != (uint)values.Length) {
             var releases = new List<INPUT>();
+            releases.Add(MouseInput(mouseInput, mouseUp));
             for (var index = modifiers.Length - 1; index >= 0; index--) {
                 releases.Add(KeyboardInput(keyboardInput, modifiers[index], keyUp));
             }

src/cli/handlers/computer-action-flags.ts

@@ -63,7 +63,8 @@ export function getComputerClickActionFlags(flags: Map<string, string | boolean>
   mouseButton?: string
   modifiers?: string
 } {
-  const modifiers = getOptionalStringFlag(flags, 'modifiers')
+  const rawModifiers = flags.get('modifiers')
+  const modifiers = typeof rawModifiers === 'string' ? rawModifiers : undefined
   const result = {
     elementIndex: getOptionalNonNegativeIntegerFlag(flags, 'element-index'),
     x: getOptionalNumberFlag(flags, 'x'),

src/cli/handlers/computer-action-validation.test.ts

@@ -312,6 +312,20 @@ describe('orca computer action CLI validation', () => {
     vi.mocked(console.error).mockClear()
     process.exitCode = undefined
 
+    await main(
+      ['computer', 'click', '--app', 'Finder', '--element-index', '1', '--modifiers', ''],
+      '/tmp/repo/src'
+    )
+
+    expect(callMock).not.toHaveBeenCalled()
+    expect(vi.mocked(console.error).mock.calls[0][0]).toContain(
+      'Click modifiers accept modifier keys only'
+    )
+    expect(process.exitCode).toBe(1)
+
+    vi.mocked(console.error).mockClear()
+    process.exitCode = undefined
+
     await main(
       ['computer', 'scroll', '--app', 'Finder', '--element-index', '1', '--direction', 'diagonal'],
       '/tmp/repo/src'