mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(desktop): guard against compiled .js shadowing .ts renderer sources
A stray tsc run can emit foo.js next to foo.ts under apps/shared/src or apps/desktop/src. .gitignore hides the artifact from git status, but Vite resolves extensionless imports .js-before-.ts, so the renderer silently runs the stale compiled copy. This bit for real: a Jul 16 artifact of websocket-url.js predated the #68250 getGatewayWsUrl contract change ({ ok, wsUrl } IPC result), so its old 'if (fresh) return fresh' handed the whole result object to new WebSocket(), dialing ws://127.0.0.1:5174/[object%20Object] on every boot. The desktop app could never connect, and the failure survived reboots and cache wipes because the poison lived in src/. Two layers: - scripts/assert-no-stale-js.mjs runs before vite in dev:renderer and at the head of build; it fails loud with the exact shadowing files and the rm command when a .js sits next to a sibling .ts/.tsx in either src tree. - JsonRpcGatewayClient.connect() now rejects non-ws:// URLs with a readable error instead of letting new WebSocket() coerce an object into [object%20Object], so any future contract skew fails diagnosably.
This commit is contained in:
parent
cbc1054e23
commit
c064487dbe
5 changed files with 255 additions and 2 deletions
|
|
@ -14,13 +14,13 @@
|
|||
"dev": "concurrently -k \"npm:dev:renderer\" \"npm:dev:electron\"",
|
||||
"dev:fake-boot": "cross-env HERMES_DESKTOP_BOOT_FAKE=1 HERMES_DESKTOP_BOOT_FAKE_STEP_MS=650 npm run dev",
|
||||
"dev:mock": "node scripts/dev-mock.mjs",
|
||||
"dev:renderer": "node scripts/assert-root-install.mjs && vite --host 127.0.0.1 --port 5174",
|
||||
"dev:renderer": "node scripts/assert-root-install.mjs && node scripts/assert-no-stale-js.mjs && vite --host 127.0.0.1 --port 5174",
|
||||
"dev:electron": "wait-on http://127.0.0.1:5174 && node scripts/bundle-electron-main.mjs --dev && cross-env XCURSOR_SIZE=24 HERMES_DESKTOP_DEV_SERVER=http://127.0.0.1:5174 electron .",
|
||||
"profile:main": "wait-on http://127.0.0.1:5174 && node scripts/bundle-electron-main.mjs --dev && cross-env XCURSOR_SIZE=24 HERMES_DESKTOP_DEV_SERVER=http://127.0.0.1:5174 electron --inspect=9229 .",
|
||||
"profile:main:cpu": "wait-on http://127.0.0.1:5174 && node scripts/bundle-electron-main.mjs --dev && cross-env XCURSOR_SIZE=24 NODE_OPTIONS=--cpu-prof HERMES_DESKTOP_DEV_SERVER=http://127.0.0.1:5174 electron .",
|
||||
"start": "npm run build && electron .",
|
||||
"prebuild": "tsc -b . --clean",
|
||||
"build": "node scripts/assert-root-install.mjs && node scripts/write-build-stamp.mjs && vite build && node scripts/bundle-electron-main.mjs && node scripts/stage-native-deps.mjs",
|
||||
"build": "node scripts/assert-root-install.mjs && node scripts/assert-no-stale-js.mjs && node scripts/write-build-stamp.mjs && vite build && node scripts/bundle-electron-main.mjs && node scripts/stage-native-deps.mjs",
|
||||
"postbuild": "node scripts/assert-dist-built.mjs",
|
||||
"prebuilder": "node scripts/patch-electron-builder-mac-binary.mjs",
|
||||
"builder": "cross-env NODE_OPTIONS=--max-old-space-size=16384 node scripts/run-electron-builder.mjs",
|
||||
|
|
|
|||
91
apps/desktop/scripts/assert-no-stale-js.mjs
Normal file
91
apps/desktop/scripts/assert-no-stale-js.mjs
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
// Dev/build-time guard: refuse to run against compiled .js files shadowing
|
||||
// .ts/.tsx sources in the renderer source trees.
|
||||
//
|
||||
// A stray `tsc` invocation (no project file, editor compile-on-save, etc.)
|
||||
// can emit `foo.js` next to `foo.ts` under apps/shared/src or
|
||||
// apps/desktop/src. `.gitignore` already hides those artifacts from git,
|
||||
// which makes them invisible in `git status` — but Vite resolves
|
||||
// extensionless imports with `.js` BEFORE `.ts`, so the renderer silently
|
||||
// runs the stale compiled copy instead of the real source.
|
||||
//
|
||||
// This bit hard in July 2026: a Jul 16 artifact of websocket-url.js predated
|
||||
// the #68250 getGatewayWsUrl contract change, so every boot dialed
|
||||
// "ws://<origin>/[object%20Object]" and the desktop app could never connect,
|
||||
// surviving reboots and cache wipes because the poison lived in src/.
|
||||
//
|
||||
// Runs before `vite` in dev and at the head of `build`. Fails loud with the
|
||||
// exact files and the fix instead of letting the app boot on stale code.
|
||||
|
||||
import { existsSync, readdirSync, statSync } from "fs"
|
||||
import { join, resolve } from "path"
|
||||
import { isMain } from "./utils.mjs"
|
||||
|
||||
// Pure scan — returns { ok: true } or { ok: false, stale: ["/abs/foo.js", ...] }.
|
||||
// A .js file is stale when a sibling .ts or .tsx source exists. Standalone
|
||||
// .js files (fixtures, configs) are left alone.
|
||||
export function checkNoStaleJs(srcDirs) {
|
||||
const stale = []
|
||||
|
||||
const walk = dir => {
|
||||
if (!existsSync(dir) || !statSync(dir).isDirectory()) {
|
||||
return
|
||||
}
|
||||
|
||||
for (const name of readdirSync(dir)) {
|
||||
const full = join(dir, name)
|
||||
|
||||
if (statSync(full).isDirectory()) {
|
||||
if (name !== "node_modules") {
|
||||
walk(full)
|
||||
}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
if (!name.endsWith(".js")) {
|
||||
continue
|
||||
}
|
||||
|
||||
const base = full.slice(0, -".js".length)
|
||||
|
||||
if (existsSync(`${base}.ts`) || existsSync(`${base}.tsx`)) {
|
||||
stale.push(full)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const dir of srcDirs) {
|
||||
walk(dir)
|
||||
}
|
||||
|
||||
return stale.length ? { ok: false, stale } : { ok: true }
|
||||
}
|
||||
|
||||
function main() {
|
||||
const desktopRoot = resolve(import.meta.dirname, "..")
|
||||
const result = checkNoStaleJs([
|
||||
join(desktopRoot, "src"),
|
||||
resolve(desktopRoot, "..", "shared", "src")
|
||||
])
|
||||
|
||||
if (!result.ok) {
|
||||
console.error("\n✗ assert-no-stale-js: compiled .js artifacts are shadowing .ts sources:")
|
||||
|
||||
for (const file of result.stale) {
|
||||
console.error(` ${file}`)
|
||||
}
|
||||
|
||||
console.error(" Vite resolves .js before .ts, so the app would run these stale")
|
||||
console.error(" compiled copies instead of the real sources. Delete them:")
|
||||
console.error(` rm ${result.stale.join(" ")}\n`)
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
console.log("✓ assert-no-stale-js: no compiled .js shadowing .ts sources")
|
||||
}
|
||||
|
||||
if (isMain(import.meta.url)) {
|
||||
main()
|
||||
}
|
||||
|
||||
export default { checkNoStaleJs }
|
||||
85
apps/desktop/scripts/assert-no-stale-js.test.mjs
Normal file
85
apps/desktop/scripts/assert-no-stale-js.test.mjs
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
import assert from 'node:assert/strict'
|
||||
import fs from 'node:fs'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { checkNoStaleJs } from '../scripts/assert-no-stale-js.mjs'
|
||||
|
||||
function makeSrcTree(build) {
|
||||
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-stale-js-'))
|
||||
const srcDir = path.join(tempRoot, 'src')
|
||||
fs.mkdirSync(srcDir, { recursive: true })
|
||||
if (build) build(srcDir)
|
||||
return { tempRoot, srcDir }
|
||||
}
|
||||
|
||||
test('passes on a clean tree of .ts sources', () => {
|
||||
const { tempRoot, srcDir } = makeSrcTree(d => {
|
||||
fs.writeFileSync(path.join(d, 'websocket-url.ts'), 'export {}', 'utf8')
|
||||
fs.writeFileSync(path.join(d, 'index.ts'), 'export {}', 'utf8')
|
||||
})
|
||||
try {
|
||||
assert.deepEqual(checkNoStaleJs([srcDir]), { ok: true })
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('flags a compiled .js shadowing a sibling .ts (the #68250 incident shape)', () => {
|
||||
const { tempRoot, srcDir } = makeSrcTree(d => {
|
||||
fs.writeFileSync(path.join(d, 'websocket-url.ts'), 'export {}', 'utf8')
|
||||
fs.writeFileSync(path.join(d, 'websocket-url.js'), 'export {}', 'utf8')
|
||||
})
|
||||
try {
|
||||
const result = checkNoStaleJs([srcDir])
|
||||
assert.equal(result.ok, false)
|
||||
assert.equal(result.stale.length, 1)
|
||||
assert.match(result.stale[0], /websocket-url\.js$/)
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('flags a .js shadowing a .tsx source and recurses into subdirectories', () => {
|
||||
const { tempRoot, srcDir } = makeSrcTree(d => {
|
||||
const nested = path.join(d, 'app', 'chat')
|
||||
fs.mkdirSync(nested, { recursive: true })
|
||||
fs.writeFileSync(path.join(nested, 'view.tsx'), 'export {}', 'utf8')
|
||||
fs.writeFileSync(path.join(nested, 'view.js'), 'export {}', 'utf8')
|
||||
})
|
||||
try {
|
||||
const result = checkNoStaleJs([srcDir])
|
||||
assert.equal(result.ok, false)
|
||||
assert.match(result.stale[0], /app[/\\]chat[/\\]view\.js$/)
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('leaves standalone .js files (no sibling source) alone', () => {
|
||||
const { tempRoot, srcDir } = makeSrcTree(d => {
|
||||
fs.writeFileSync(path.join(d, 'fixture-data.js'), 'export {}', 'utf8')
|
||||
fs.writeFileSync(path.join(d, 'index.ts'), 'export {}', 'utf8')
|
||||
})
|
||||
try {
|
||||
assert.deepEqual(checkNoStaleJs([srcDir]), { ok: true })
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('skips node_modules and tolerates missing directories', () => {
|
||||
const { tempRoot, srcDir } = makeSrcTree(d => {
|
||||
const nm = path.join(d, 'node_modules', 'pkg')
|
||||
fs.mkdirSync(nm, { recursive: true })
|
||||
fs.writeFileSync(path.join(nm, 'mod.ts'), 'export {}', 'utf8')
|
||||
fs.writeFileSync(path.join(nm, 'mod.js'), 'export {}', 'utf8')
|
||||
})
|
||||
try {
|
||||
assert.deepEqual(checkNoStaleJs([srcDir, path.join(tempRoot, 'does-not-exist')]), { ok: true })
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
65
apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts
Normal file
65
apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
// connect() must reject malformed WS URLs loudly instead of letting
|
||||
// `new WebSocket()` coerce them. The failure this guards against is real:
|
||||
// a stale compiled websocket-url.js (predating the #68250 { ok, wsUrl }
|
||||
// IPC contract) returned the whole result object, and the browser dialed
|
||||
// "ws://<page-origin>/[object%20Object]" — an opaque, unfixable-looking
|
||||
// "Could not connect to Hermes gateway" boot loop.
|
||||
|
||||
import { JsonRpcGatewayClient } from '@hermes/shared'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
class FakeSocket {
|
||||
static OPEN = 1
|
||||
readyState = 0
|
||||
addEventListener = vi.fn((type: string, handler: () => void) => {
|
||||
if (type === 'open') {
|
||||
// Land the handshake asynchronously like a real socket.
|
||||
setTimeout(() => {
|
||||
this.readyState = FakeSocket.OPEN
|
||||
handler()
|
||||
}, 0)
|
||||
}
|
||||
})
|
||||
removeEventListener = vi.fn()
|
||||
close = vi.fn()
|
||||
send = vi.fn()
|
||||
}
|
||||
|
||||
describe('JsonRpcGatewayClient connect() URL guard', () => {
|
||||
beforeEach(() => {
|
||||
// jsdom has no WebSocket; the class reads WebSocket.OPEN when a socket exists.
|
||||
vi.stubGlobal('WebSocket', FakeSocket)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('rejects a non-string (an IPC result object passed through whole)', async () => {
|
||||
const client = new JsonRpcGatewayClient()
|
||||
await expect(
|
||||
client.connect({ ok: true, wsUrl: 'ws://127.0.0.1:1/api/ws' } as unknown as string)
|
||||
).rejects.toThrow(/requires a ws:\/\/ or wss:\/\/ URL string, got type "object"/)
|
||||
})
|
||||
|
||||
it('rejects a string that is not a ws:// or wss:// URL', async () => {
|
||||
const client = new JsonRpcGatewayClient()
|
||||
await expect(client.connect('http://127.0.0.1:1234/api/ws')).rejects.toThrow(
|
||||
/requires a ws:\/\/ or wss:\/\/ URL string/
|
||||
)
|
||||
})
|
||||
|
||||
it('does not flip connection state when rejecting a malformed URL', async () => {
|
||||
const client = new JsonRpcGatewayClient()
|
||||
await client.connect(undefined as unknown as string).catch(() => undefined)
|
||||
expect(client.connectionState).toBe('idle')
|
||||
})
|
||||
|
||||
it('accepts ws:// and wss:// URL strings', async () => {
|
||||
for (const url of ['ws://127.0.0.1:1234/api/ws?token=t', 'wss://gw.example.com/api/ws?ticket=t']) {
|
||||
const client = new JsonRpcGatewayClient({ socketFactory: () => new FakeSocket() as unknown as WebSocket })
|
||||
await client.connect(url)
|
||||
expect(client.connectionState).toBe('open')
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -95,6 +95,18 @@ export class JsonRpcGatewayClient {
|
|||
}
|
||||
|
||||
async connect(wsUrl: string): Promise<void> {
|
||||
// Fail loudly on a malformed URL instead of letting `new WebSocket()`
|
||||
// coerce it. A non-string here means an upstream contract skew (e.g. an
|
||||
// IPC result object passed through whole), which otherwise surfaces as an
|
||||
// opaque dial to "ws://<page-origin>/[object%20Object]" — a real incident:
|
||||
// a stale compiled copy of websocket-url.js shadowed the .ts source and
|
||||
// returned the raw `{ ok, wsUrl }` IPC result after #68250 changed the
|
||||
// getGatewayWsUrl contract.
|
||||
if (typeof wsUrl !== 'string' || !/^wss?:\/\//i.test(wsUrl)) {
|
||||
const got = typeof wsUrl === 'string' ? JSON.stringify(wsUrl) : `type "${typeof wsUrl}"`
|
||||
throw new Error(`gateway connect() requires a ws:// or wss:// URL string, got ${got}`)
|
||||
}
|
||||
|
||||
if (this.socket?.readyState === WebSocket.OPEN || this.state === 'connecting') {
|
||||
return
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue