fix(ci): address review findings

.github/actions/install-node-dependencies/action.yml

@@ -57,9 +57,6 @@ runs:
         pnpm install \
           --no-frozen-lockfile \
           --prefer-frozen-lockfile=false \
-          --os=current \
-          --os=win32 \
-          --cpu=current \
           --ignore-scripts
         git diff --exit-code package.json pnpm-lock.yaml
 

.github/workflows/pr.yml

@@ -12,6 +12,9 @@ concurrency:
   group: pr-checks-${{ github.event.pull_request.number }}
   cancel-in-progress: true
 
+permissions:
+  contents: read
+
 jobs:
   static_analysis:
     name: static analysis

build-plugins/plain-node-entry-guard.js

@@ -1,120 +0,0 @@
-import { spawnSync } from 'node:child_process'
-import { join } from 'node:path'
-// Why: v1.4.129-rc.1 shipped a dead terminal daemon because a shared main
-// chunk gained `require("electron")` (an import edge added in #7642), and the
-// daemon is forked as a plain-Node process where electron cannot be required.
-// Nothing in CI executes the built daemon-entry under plain Node, so the leak
-// stayed invisible until an adopted old daemon died. This guard fails the
-// build when any chunk reachable from a plain-Node fork entry requires
-// electron, and smoke-loads daemon-entry under plain Node to prove its module
-// graph still resolves.
-// Entries executed as plain Node (ELECTRON_RUN_AS_NODE / no electron runtime):
-// forked daemon, parcel-watcher and computer sidecars, and the CLI-run
-// agent-hooks entry. require("electron") throws MODULE_NOT_FOUND in all of them.
-const PLAIN_NODE_ENTRY_NAMES = [
-  'daemon-entry',
-  'parcel-watcher-process-entry',
-  'computer-sidecar',
-  'agent-hooks/managed-agent-hook-controls',
-  'codex/codex-app-server-grant-entry'
-]
-const ELECTRON_REQUIRE_RE = /require\(\s*["']electron["']\s*\)/
-function collectReachableChunks(entry, byFileName) {
-  const seen = new Set()
-  const reachable = []
-  const stack = [entry.fileName]
-  while (stack.length > 0) {
-    const fileName = stack.pop()
-    if (seen.has(fileName)) {
-      continue
-    }
-    seen.add(fileName)
-    const chunk = byFileName.get(fileName)
-    if (!chunk) {
-      continue
-    }
-    reachable.push(chunk)
-    for (const imported of [...chunk.imports, ...chunk.dynamicImports]) {
-      stack.push(imported)
-    }
-  }
-  return reachable
-}
-function assertNoElectronRequire(entryName, entry, byFileName) {
-  for (const chunk of collectReachableChunks(entry, byFileName)) {
-    if (ELECTRON_REQUIRE_RE.test(chunk.code)) {
-      throw new Error(
-        `[plain-node-entry-guard] "${entryName}" reaches chunk "${chunk.fileName}" that ` +
-          `requires electron. "${entryName}" runs as a plain-Node process, where ` +
-          `require("electron") throws MODULE_NOT_FOUND and kills it at startup (the ` +
-          `v1.4.129-rc.1 daemon outage). Keep electron imports out of its module graph.`
-      )
-    }
-  }
-}
-// Why: proves the whole daemon-entry graph resolves under plain Node (no
-// unresolved requires). require("electron") does not throw in a dev tree with
-// node_modules present, so the static scan above — not this smoke — is the
-// electron regression guard; this only catches gross load failures.
-function smokeLoadDaemonEntry(outputDir) {
-  const entryPath = join(outputDir, 'daemon-entry.js')
-  const result = spawnSync(process.execPath, [entryPath], {
-    encoding: 'utf8',
-    timeout: 15_000
-  })
-  if (result.error) {
-    throw new Error(
-      `[plain-node-entry-guard] could not smoke-load daemon-entry.js under plain Node: ` +
-        `${result.error.message}`
-    )
-  }
-  const stderr = result.stderr ?? ''
-  if (/Cannot find module|MODULE_NOT_FOUND/.test(stderr)) {
-    throw new Error(
-      `[plain-node-entry-guard] daemon-entry.js failed to load under plain Node:\n${stderr}`
-    )
-  }
-  if (!stderr.includes('Usage: daemon-entry')) {
-    throw new Error(
-      `[plain-node-entry-guard] daemon-entry.js did not reach argv parsing under plain Node ` +
-        `(expected the "Usage: daemon-entry" error). stderr:\n${stderr}`
-    )
-  }
-}
-export function createPlainNodeEntryGuardPlugin() {
-  let daemonOutputDir
-  return {
-    name: 'orca-plain-node-entry-guard',
-    writeBundle(options, bundle) {
-      // Why: skip in `electron-vite dev` watch mode — the smoke would respawn on
-      // every rebuild, and the guard only needs to gate produced builds.
-      if (this.meta.watchMode) {
-        return
-      }
-      const chunks = Object.values(bundle).filter((item) => item.type === 'chunk')
-      const byFileName = new Map(chunks.map((chunk) => [chunk.fileName, chunk]))
-      const entryByName = new Map()
-      for (const chunk of chunks) {
-        if (chunk.isEntry && chunk.name) {
-          entryByName.set(chunk.name, chunk)
-        }
-      }
-      for (const entryName of PLAIN_NODE_ENTRY_NAMES) {
-        const entry = entryByName.get(entryName)
-        if (entry) {
-          assertNoElectronRequire(entryName, entry, byFileName)
-        }
-      }
-      if (entryByName.has('daemon-entry') && options.dir) {
-        daemonOutputDir = options.dir
-      }
-    },
-    closeBundle() {
-      if (daemonOutputDir) {
-        const outputDir = daemonOutputDir
-        daemonOutputDir = undefined
-        smokeLoadDaemonEntry(outputDir)
-      }
-    }
-  }
-}

config/electron-vite-target.config.ts

@@ -8,7 +8,7 @@ const configByTarget = {
   renderer: { renderer: electronViteConfig.renderer }
 }
 
-if (!target || !(target in configByTarget)) {
+if (!target || !Object.prototype.hasOwnProperty.call(configByTarget, target)) {
   throw new Error(`Invalid ORCA_ELECTRON_VITE_TARGET: ${target ?? '<unset>'}`)
 }
 

config/scripts/electron-vite-output-contract.test.ts

@@ -1,6 +1,9 @@
+import { readFileSync } from 'node:fs'
 import { describe, expect, it } from 'vitest'
 import { electronViteConfig } from '../../electron.vite.config'
 
+const targetConfig = readFileSync('config/electron-vite-target.config.ts', 'utf8')
+
 describe('Electron Vite output contract', () => {
   it('keeps main-process and plain-Node entries at stable CommonJS paths', () => {
     const output = electronViteConfig.main?.build?.rollupOptions?.output
@@ -32,4 +35,8 @@ describe('Electron Vite output contract', () => {
       'strict'
     )
   })
+
+  it('rejects prototype properties as build targets', () => {
+    expect(targetConfig).toContain('Object.prototype.hasOwnProperty.call(configByTarget, target)')
+  })
 })

config/scripts/pr-workflow-parallelism.test.mjs

@@ -33,6 +33,10 @@ describe('PR workflow parallelism', () => {
     expect(workflow.concurrency['cancel-in-progress']).toBe(true)
   })
 
+  it('grants the PR workflow read-only repository access', () => {
+    expect(workflow.permissions).toEqual({ contents: 'read' })
+  })
+
   it('shards the general test suite across sixteen runners', () => {
     expect(workflow.jobs.test.strategy.matrix.shard).toEqual(
       Array.from({ length: 16 }, (_, index) => index + 1)
@@ -103,6 +107,8 @@ describe('PR workflow parallelism', () => {
       (step) => step.uses === './.github/actions/install-node-dependencies'
     )
 
+    expect(cacheIndex).toBeGreaterThanOrEqual(0)
+    expect(installIndex).toBeGreaterThanOrEqual(0)
     expect(cacheIndex).toBeLessThan(installIndex)
   })
 
@@ -127,6 +133,12 @@ describe('PR workflow parallelism', () => {
     )
     expect(dependencyInstall.run).toContain('--no-frozen-lockfile')
     expect(dependencyInstall.run).toContain('--ignore-scripts')
+    expect(dependencyInstall.run).not.toContain('--os=')
+    expect(dependencyInstall.run).not.toContain('--cpu=')
+    expect(packageJson.pnpm.supportedArchitectures.os).toEqual(
+      expect.arrayContaining(['current', 'win32'])
+    )
+    expect(packageJson.pnpm.supportedArchitectures.cpu).toContain('current')
     const prepareRuntime = dependencyAction.runs.steps.find(
       (step) => step.name === 'Prepare native runtime'
     )