test(desktop): lint the POSIX-path-literal defect instead of hunting it

.github/workflows/desktop-install-windows.yml

@@ -38,6 +38,12 @@ name: Desktop install (Windows)
 # `process.platform !== 'win32'`, which had been silently turning them into
 # no-mux connections here.
 #
+# That file list is also read by the desktop ESLint config, which applies
+# `hermes/no-posix-path-literals` to exactly these suites — the defect that
+# broke three of them (a hard-coded POSIX path meeting a `path.join`ed one)
+# is now a lint error rather than something this job has to discover. Adding
+# a name below therefore turns the rule on for that file too.
+#
 # ── What this does NOT catch, deliberately stated ───────────────────────────
 #
 # It cannot reproduce the `ERR_DLOPEN_FAILED loading index.win32-x64-msvc.node`

apps/desktop/electron/windows-hermes-path.test.ts

@@ -113,6 +113,11 @@ test('resolveVenvHermesCommand: probes the venv python before trusting it (retur
   const deps = makeDeps({
     canImportHermesCli: (python: string) => {
       probed = true
+      // Host-independent, so the POSIX literal is safe on the Windows lane:
+      // every path helper this resolver touches is injected by makeDeps and
+      // joins with '/', so the value under test is POSIX on Windows too. Not
+      // the #177 defect, where the expectation met a real path.join.
+      // eslint-disable-next-line hermes/no-posix-path-literals -- injected POSIX helpers, see above
       assert.equal(python, '/root/venv/Scripts/python.exe')
 
       return false
@@ -130,6 +135,9 @@ test('resolveVenvHermesCommand: returns the resolved python backend descriptor w
   const result = resolveVenvHermesCommand('/root/venv/Scripts/hermes.exe', ['serve', '--port', '0'], deps)
 
   assert.ok(result, 'a passing probe must return a backend descriptor, not null')
+  // `command` comes from the injected getVenvPython, which interpolates '/'
+  // regardless of platform — host-independent, as in the test above.
+  // eslint-disable-next-line hermes/no-posix-path-literals -- injected POSIX helpers, see above
   assert.equal(result.command, '/root/venv/Scripts/python.exe')
   assert.deepEqual(result.args, ['-m', 'hermes_cli.main', 'serve', '--port', '0'])
   assert.equal(result.bootstrap, false)

apps/desktop/eslint-rules/no-posix-path-literals.mjs

@@ -0,0 +1,213 @@
+/**
+ * Ban the POSIX-path-literal defect from tests that run on the Windows lane.
+ *
+ * The same bug was found three times in three files: a hard-coded absolute
+ * POSIX path on one side of a comparison whose other side is built with
+ * `path.join`. On Windows `path.join` emits backslashes and `path.resolve`
+ * prepends a drive letter, so the two sides can never be equal and the
+ * assertion is dead on that platform — silently, because the file had only
+ * ever run on Linux.
+ *
+ *   #177  update-relaunch.test.ts     const ROOT = '/home/u/.hermes/hermes-agent'
+ *   #177  windows-hermes-path.test.ts p => p === '/venv/lib/python3.12/site-packages'
+ *   #180  ssh-connection.test.ts      assert.match(a, /\/[0-9a-f]{16}\.sock$/)
+ *
+ * Each was found by hand after the lane went red (or, for the third, by
+ * reading the file before adding it). This rule is the cheaper version of
+ * that search.
+ *
+ * ── Why these three shapes and not "no leading-slash literals" ──────────────
+ *
+ * A blanket ban is unusable: the seven lane files contain 71 absolute POSIX
+ * string literals, and almost all are *inputs* — a fake `execPath`, a
+ * ControlPath handed to ssh, a path the stub filesystem is asked about. Those
+ * are fine; the code under test is what has to cope with them. The defect
+ * only appears where a literal meets a computed path, so the rule keys on
+ * that meeting rather than on the literal alone:
+ *
+ *   joined-root   `path.join(X, …)` where X is, or is a const bound to, an
+ *                 absolute POSIX literal. Joining onto a driveless root gives
+ *                 a value that can never equal a `path.resolve`d one.
+ *                 Fix: `path.resolve('/…')` — a no-op on POSIX.
+ *
+ *   comparison    `x === '/a/b'`. Covers both a bare `===` in a stub predicate
+ *                 and the expected-value slot of an assertion, since that is
+ *                 the same comparison written by a helper.
+ *
+ *   regex         `assert.match(x, /\/…/)` where the pattern opens with a path
+ *                 separator, which only a POSIX rendering can match.
+ *                 Fix: assert on `path.basename(x)`, or build with `path.sep`.
+ *
+ * Single-segment literals ('/tmp', '/') are ignored — a lone root is usually
+ * an opaque token rather than a path that gets joined and compared.
+ *
+ * ── Scope ───────────────────────────────────────────────────────────────────
+ *
+ * Applied only to the suites named in `test:desktop:win-install`, and that
+ * list is read from package.json rather than restated here, so adding a suite
+ * to the Windows lane turns the rule on for it in the same edit. Files that
+ * run only on Linux keep their POSIX literals, which is the honest outcome:
+ * this is a cross-platform-correctness rule, not a style rule.
+ *
+ * A legitimately-POSIX value in a lane file (a remote shell command — remotes
+ * are always POSIX) takes an eslint-disable-next-line with the reason.
+ */
+
+// Absolute, with at least one interior separator: '/a/b' yes, '/tmp' no.
+const ABSOLUTE_POSIX = /^\/[^/\s]+\//
+
+// A pattern that opens with a separator, optionally behind a start anchor.
+const LEADING_SEPARATOR = /^\^?\\?\//
+
+const COMPARISONS = new Set(['===', '!==', '==', '!='])
+
+const ASSERT_EQUAL = new Set([
+  'deepEqual',
+  'deepStrictEqual',
+  'equal',
+  'notDeepStrictEqual',
+  'notEqual',
+  'notStrictEqual',
+  'strictEqual',
+  'toBe',
+  'toEqual',
+  'toStrictEqual'
+])
+
+const ASSERT_MATCH = new Set(['doesNotMatch', 'match', 'toMatch'])
+
+/**
+ * Which argument holds the *expected* value.
+ *
+ * `assert.equal(actual, expected)` puts it second; `expect(actual).toBe(expected)`
+ * puts it first, because the actual value went to `expect()`. Getting this
+ * wrong is silent — the rule simply stops firing on one of the two styles.
+ */
+function expectedArgumentIndex(callee) {
+  const isExpectChain =
+    callee.object?.type === 'CallExpression' && callee.object.callee?.name === 'expect'
+
+  return isExpectChain ? 0 : 1
+}
+
+function isPosixPathLiteral(node) {
+  return (
+    node?.type === 'Literal' &&
+    typeof node.value === 'string' &&
+    ABSOLUTE_POSIX.test(node.value)
+  )
+}
+
+// `path.join(...)` — but not `path.win32.join` / `path.posix.join`, where the
+// flavour is stated and a matching literal is deliberate.
+function isPathJoin(node) {
+  const callee = node.callee
+
+  if (callee?.type === 'Identifier') {
+    return callee.name === 'join'
+  }
+
+  if (callee?.type !== 'MemberExpression' || callee.computed) {
+    return false
+  }
+
+  return callee.property.name === 'join' && callee.object.type === 'Identifier' && callee.object.name === 'path'
+}
+
+// One hop through a `const` binding, which is how a shared fixture root is
+// always written. Deliberately not a general dataflow analysis.
+function resolveConstInit(node, scope) {
+  if (node?.type !== 'Identifier') {
+    return node
+  }
+
+  for (let s = scope; s; s = s.upper) {
+    const variable = s.variables.find(v => v.name === node.name)
+
+    if (!variable) {
+      continue
+    }
+
+    if (variable.defs.length !== 1) {
+      return node
+    }
+
+    const def = variable.defs[0]
+
+    if (def.type === 'Variable' && def.parent?.kind === 'const' && def.node.init) {
+      return def.node.init
+    }
+
+    return node
+  }
+
+  return node
+}
+
+export default {
+  meta: {
+    docs: {
+      description:
+        'Disallow absolute POSIX path literals where they meet a computed path, in tests that run on Windows'
+    },
+    messages: {
+      comparison:
+        'Comparing against the absolute POSIX literal "{{value}}". If the other side is built with path.join it can never match on Windows — build the expectation with path.join too, or assert on path.basename.',
+      joinedRoot:
+        'path.join() onto the absolute POSIX root "{{value}}". path.resolve prepends a drive on Windows, so a driveless root can never equal a resolved path — wrap the root in path.resolve (a no-op on POSIX).',
+      regex:
+        'This pattern opens with a path separator, so it can only match a POSIX rendering. Assert on path.basename(), or build the separator with path.sep.'
+    },
+    schema: [],
+    type: 'problem'
+  },
+
+  create(context) {
+    const { sourceCode } = context
+
+    function report(node, messageId, value) {
+      context.report({ data: value === undefined ? {} : { value }, messageId, node })
+    }
+
+    return {
+      BinaryExpression(node) {
+        if (!COMPARISONS.has(node.operator)) {
+          return
+        }
+
+        for (const side of [node.left, node.right]) {
+          if (isPosixPathLiteral(side)) {
+            report(side, 'comparison', side.value)
+          }
+        }
+      },
+
+      CallExpression(node) {
+        if (isPathJoin(node) && node.arguments.length > 0) {
+          const first = resolveConstInit(node.arguments[0], sourceCode.getScope(node))
+
+          if (isPosixPathLiteral(first)) {
+            report(node.arguments[0], 'joinedRoot', first.value)
+          }
+        }
+
+        // assert.equal(actual, '/a/b') and friends: the same comparison, via a
+        // helper. Only the expected slot — the actual slot is an input.
+        const callee = node.callee
+
+        if (callee?.type === 'MemberExpression' && !callee.computed) {
+          const name = callee.property.name
+          const expected = node.arguments[expectedArgumentIndex(callee)]
+
+          if (ASSERT_EQUAL.has(name) && isPosixPathLiteral(expected)) {
+            report(expected, 'comparison', expected.value)
+          }
+
+          if (ASSERT_MATCH.has(name) && expected?.regex && LEADING_SEPARATOR.test(expected.regex.pattern)) {
+            report(expected, 'regex')
+          }
+        }
+      }
+    }
+  }
+}

apps/desktop/eslint-rules/no-posix-path-literals.test.mjs

@@ -0,0 +1,108 @@
+import { RuleTester } from 'eslint'
+import tseslint from 'typescript-eslint'
+import { describe, it } from 'vitest'
+
+import rule from './no-posix-path-literals.mjs'
+
+// `tseslint.parser`, the same handle the shared config uses, rather than
+// reaching for @typescript-eslint/parser — which is only a transitive here.
+
+// RuleTester drives `describe`/`it` off globals; vitest's are imported here
+// because this project does not run with `globals: true`.
+RuleTester.describe = describe
+RuleTester.it = it
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parser: tseslint.parser,
+    parserOptions: { ecmaVersion: 'latest', sourceType: 'module' }
+  }
+})
+
+ruleTester.run('no-posix-path-literals', rule, {
+  invalid: [
+    // ── The three real defects, in the form they were actually committed ────
+    //
+    // Reconstructed from the pre-fix files rather than invented, so a change
+    // that stops catching them fails here. Full-file positive control: running
+    // this rule over `1770376^` and `50e2b2b^` reports exactly these and
+    // nothing else.
+    {
+      // #177, update-relaunch.test.ts — a driveless root, then joined onto.
+      code: `
+        const ROOT = '/home/u/.hermes/hermes-agent'
+        const UNPACKED = path.join(ROOT, 'apps', 'desktop', 'release', 'linux-unpacked')
+      `,
+      errors: [{ messageId: 'joinedRoot' }]
+    },
+    {
+      // #177, windows-hermes-path.test.ts — a stub predicate comparing against
+      // a path the implementation builds with the host separator.
+      code: `const deps = { directoryExists: p => p === '/venv/lib/python3.12/site-packages' }`,
+      errors: [{ messageId: 'comparison' }]
+    },
+    {
+      // #180, ssh-connection.test.ts — a pattern that opens with a separator.
+      code: String.raw`assert.match(a, /\/[0-9a-f]{16}\.sock$/)`,
+      errors: [{ messageId: 'regex' }]
+    },
+
+    // ── The same shapes, generalised ───────────────────────────────────────
+    { code: `const x = path.join('/srv/app', 'bin')`, errors: [{ messageId: 'joinedRoot' }] },
+    { code: `const x = join('/srv/app', 'bin')`, errors: [{ messageId: 'joinedRoot' }] },
+    { code: `if (p !== '/srv/app/bin') { fail() }`, errors: [{ messageId: 'comparison' }] },
+    { code: `assert.equal(actual, '/srv/app/bin')`, errors: [{ messageId: 'comparison' }] },
+    { code: `assert.deepEqual(actual, '/srv/app/bin')`, errors: [{ messageId: 'comparison' }] },
+    { code: `expect(actual).toBe('/srv/app/bin')`, errors: [{ messageId: 'comparison' }] },
+    { code: String.raw`assert.match(p, /^\/srv\/app/)`, errors: [{ messageId: 'regex' }] },
+
+    // Both sides of one comparison are reported, because both are wrong.
+    {
+      code: `const same = '/a/b/c' === '/a/b/c'`,
+      errors: [{ messageId: 'comparison' }, { messageId: 'comparison' }]
+    }
+  ],
+
+  valid: [
+    // ── The fixes that were actually applied ───────────────────────────────
+    `
+      const ROOT = path.resolve('/home/u/.hermes/hermes-agent')
+      const UNPACKED = path.join(ROOT, 'apps', 'desktop', 'release', 'linux-unpacked')
+    `,
+    `const expected = path.join('/venv', 'lib', 'python3.12', 'site-packages')
+     const deps = { directoryExists: p => p === expected }`,
+    String.raw`assert.match(path.basename(a), /^[0-9a-f]{16}\.sock$/)`,
+
+    // ── Inputs, which are the overwhelming majority and must stay quiet ────
+    //
+    // The seven lane files hold 71 absolute POSIX literals; all but the three
+    // above are values handed *to* the code under test, which is exactly what
+    // a path-handling implementation is supposed to cope with.
+    `resolveVenvHermesCommand('/root/venv/Scripts/hermes.exe', [], deps)`,
+    `const deps = makeDeps({ hermesHome: '/fake/hermes-home' })`,
+    `startSsh({ controlPath: '/tmp/x.sock' })`,
+
+    // A single-segment root is a token, not a path that gets joined.
+    `const x = path.join('/tmp', 'a')`,
+    `assert.equal(dirname('/a'), '/')`,
+
+    // A stated flavour means the literal is deliberate.
+    `const x = path.posix.join('/srv/app', 'bin')`,
+    `const x = path.win32.join('/srv/app', 'bin')`,
+
+    // Not paths at all.
+    `assert.equal(url, 'https://example.com/a/b')`,
+    `const x = path.join(base, '/srv/app/bin')`,
+
+    // A shebang check is content, not a path — the pattern must open with the
+    // separator, not merely contain one.
+    String.raw`assert.match(script, /^#!\/bin\/bash/)`,
+    String.raw`assert.match(cmd, /^cd '\/home\/me\/project' 2>\/dev\/null$/)`,
+
+    // A reassigned binding is not followed: one hop through a const is the
+    // whole of the analysis, and guessing past that would misreport.
+    `let root = '/srv/app/bin'
+     root = process.cwd()
+     const x = path.join(root, 'bin')`
+  ]
+})