feat(ci): wire up JS/TS tests in CI, fix all failures, ban source-regex tests

[ethernet8023]

What does this PR do?

Wires up JS/TS tests (ui-tui + apps/desktop) into CI with npm run check at the workspace root, and fixes all pre-existing test failures across both packages so the CI gate is green from the start. Also bans the "regex-scan source code in tests" antipattern in AGENTS.md and converts all offending tests to real behavior-testing via small extracted pure/DI-testable modules.

Related Issue

Fixes #

Type of Change

• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)

Changes Made

CI & infrastructure

• feat(ci): run JS tests in CI, add npm run check in ws root — adds a js-tests.yml workflow that runs npm run check across all npm workspaces.
• feat(ci): load npm workspaces from package.json — the CI matrix dynamically discovers workspaces via npm query .workspace instead of a hardcoded list, so new packages are picked up automatically. Added ui-tui/packages/* to the root workspaces array so @hermes/ink gets its own typecheck job.
• change(desktop): add vitest config for the desktop app — adds vitest.config.ts + vitest.setup.ts with IS_REACT_ACT_ENVIRONMENT=true and auto-cleanup. Two vitest projects: ui (jsdom, src/**/*.test.{ts,tsx}) and electron (node, electron/**/*.test.ts).
• change(ci/desktop): move desktop app build into check job — folds the desktop build into the check script so npm run check covers typecheck + test + test:desktop:all + build end-to-end.
• cleanup(ci): make all tsbuildinfo gitignored — adds *.tsbuildinfo to .gitignore.

AGENTS.md: ban source-regex tests

• feat(agent): ban regex-scanning source code in tests — new AGENTS.md antipattern section: tests must not readFileSync a source file and assert.match/toMatch a regex against it. Prescribes the extraction pattern (pure functions, injected deps, no Electron import) with backend-probes.ts/backend-command.ts as canonical examples. Carves out a build-artifact exception (reading dist/entry.js is legit).

Desktop: extract source-regex tests into real unit tests

• test(desktop): extract hiddenWindowsChildOptions + stopBackendChild — the test file with a literal // TODO FIXME this is an antipattern comment. Extracted hiddenWindowsChildOptions() into windows-child-options.ts and stopBackendChild() into backend-child.ts, both pure/DI-testable. Consolidated the duplicate definition between main.ts and bootstrap-runner.ts.
• test(desktop): extract Windows hermes-resolution helpers — extracted buildPathExtCandidates(), chooseUpdaterArgs(), resolveVenvHermesCommand() into windows-hermes-path.ts with deps injected (fileExists, canImportHermesCli, etc).
• test(desktop): extract profile-delete routing decision — extracted profileNameFromDeleteRequest(), decideProfileDeleteAction(), resolveRouteProfile() into profile-delete-routing.ts.
• test(desktop): fix relative import in oauth-net-request.test.ts — the oauth behavior was already covered by real unit tests; just needed the .ts extension fix and removal of the dead source-regex test.
• test(desktop): replace windows-child-process.test.ts regex with real tests — removed the tracked source-regex test file entirely; its call-site contracts are now covered by the extracted behavior/DI-level tests above.
• test(tui): extract cursor-layout + fast-echo helpers — extracted resolveCursorLayout(), fastBackspaceEffect(), fastAppendEffect() from textInput.tsx so the cursor-drift regression is tested by calling the pure function with a deliberately stale cur vs fresh curRefCurrent.

Desktop: fix broken/stale tests

• test(desktop): fix a handful of broken tests — model-options.test.ts (explicit_only/explicitOnly defaults), model-settings.test.tsx (setApiRequestProfile mock + QueryClientProvider wrapper), panes.test.ts (widthOverride persisted), pane-shell.test.tsx (resizable prop), onboarding.test.ts (startsWith for query-param paths), use-prompt-actions/index.test.tsx (source: 'desktop' on recovery retry path), model-menu-panel.test.tsx (document.body for portal content).
• test(desktop): fix React act() warnings — wrapped render()/fireEvent() in await act(async () => { ... }) across 8 test files. Added actRender helper in use-prompt-actions. Reduced warnings from 44 → 0.
• test(desktop): move node tests to vitest as well — migrated the Electron node --test suite to vitest's electron project so both UI and Electron tests run under a single vitest run invocation.
• fix(desktop): stage-native-deps falls back to electron-rebuild — stageNodePty({ platform, arch }) now takes explicit target platform/arch, checks prebuilds/<platform>-<arch> for the correct target, and falls back to electron-rebuild -f -w node-pty when no prebuild or compiled binary exists. Fixes the "Missing node-pty native binary dir for linux-x64" CI failure.

Desktop: build cleanup

• Removed tsc -b from the build script — it was generating 691 orphaned .js files + 1528 .d.ts files into a gitignored build/electron-types/ directory that nothing ever consumed.
• Removed the HERMES_DESKTOP_ALLOW_UNSUPPORTED_PLATFORM_BUILD env var hack — linux is now natively allowed in ensurePlatformBuilds().
• Fixed linux binary name from 'hermes' → 'Hermes' (capital H, matching electron-builder's rename).

TUI: test fixes

• fix(js): fix long-time broken tests — statusRule.test.ts (stale cost field), virtualHeights.test.ts (horizontalReserve bump made both prompts collapse to the 20-col floor).
• tests(tui): longer timeout for wrap ansi test — the cursorDriftRegression test is a thorough brute-force O(n²) loop; gave it a longer timeout.

Lint & formatting

• change(lint): don't ignore config files in eslint conf — removes *.config.* from eslint ignores so config files get linted/formatted; fixes the no-restricted-globals rule style (single quotes, proper indentation).
• cleanup(desktop): lint&fmt all — prettier + eslint across all changed files.
• cleanup(desktop): remove 'use strict' in ts & mjs — ESM modules are strict by default.
• cleanup(desktop): note that ts imports don't need extension in one comment.
• fix(desktop): prettier-clean vitest.config.ts — single quotes + trailing newline to satisfy prettier.

How to Test

1. npm run --prefix ui-tui check — builds @hermes/ink, typechecks, runs vitest (107 test files, 1117 tests pass, 1 skipped)
2. npm run --prefix apps/desktop check — typecheck + vitest run (both ui and electron projects, 146 test files / 1180 tests) + desktop bundle validation + build, all on Linux
3. npm run --prefix ui-tui/packages/hermes-ink check — nested @hermes/ink typecheck (also covered by the CI matrix)

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — N/A (JS/TS only)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Linux (NixOS)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — AGENTS.md updated with new antipattern section
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — desktop build + test validated on Linux; macOS/Windows paths already existed
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[austinpickett]

Review

Overall this is a strong, well-motivated PR — wiring the JS/TS packages into CI, killing the "regex-scan the source in a test" antipattern, and doing the extractions as pure/DI modules is exactly the right call. I spot-checked the refactors and the behavior parity looks faithful (hiddenWindowsChildOptions, stopBackendChild, resolveVenvHermesCommand, buildPathExtCandidates, chooseUpdaterArgs, and the profile-delete-routing decision/route split all preserve the original logic). Nice docstrings, too.

A few findings, roughly in priority order:

1. The extracted electron unit tests don't actually run in CI ⚠️

This is the big one given the PR's title. CI runs npm run check, and the desktop check is:

"check": "npm run typecheck && npm run test:ui && npm run test:desktop:all && npm run build"

• test:ui is vitest with include: ["src/**/*.test.{ts,tsx}"] — so it only covers src/, not electron/.
• test:desktop:all is test-desktop.mjs all, which builds + validates the packaged bundle (ensurePackagedApp() + validateBundle()); it does not run node --test.
• test:desktop:platforms (the node --test electron/*.test.ts suite) is not referenced by check or any workflow.

Net effect: the very tests this PR extracts/adds — windows-child-options.test.ts, windows-hermes-path.test.ts, profile-delete-routing.test.ts, backend-child, etc. — never execute in CI. All the type-level stuff is checked by tsc, but the asserted behavior isn't. I'd add npm run test:desktop:platforms to the desktop check (or a dedicated matrix step) so the extraction work is actually gated.

2. Typo breaks npm run preview

apps/desktop/package.json:

"preview": "node scripts/assert-root-install.mjPs && vite preview --host 127.0.0.1 --port 4174",

assert-root-install.mjPs → .mjs. Not caught by CI because check doesn't invoke preview.

3. stage-native-deps.mjs electron-rebuild fallback is Linux-only in practice

spawnSync(process.execPath, ['../../node_modules/.bin/electron-rebuild', '-f', '-w', 'node-pty'], { cwd: projectRoot, ... })

node_modules/.bin/electron-rebuild is a symlink to cli.js on POSIX, so node <that> works — but on Windows the .bin entry is a shell/cmd shim (the extensionless file isn't node-runnable), so this branch would fail exactly on the platform where node-pty prebuild gaps also happen. Safer to resolve the JS entry directly, e.g. resolve(projectRoot, '../../node_modules/@electron/rebuild/lib/cli.js') (or require.resolve). Low priority since it only fires when no prebuild/binary exists and this was validated on Linux.

4. Minor: the source-regex ban isn't actually enforced, and the new eslint rule is off-style

• The AGENTS.md antipattern is documented but there's no lint rule that fails on readFileSync(...source...) + assert.match, so it relies entirely on reviewers. Worth a no-restricted-syntax / custom rule if you want it to stick — especially since desktop check doesn't run lint at all.
• The added rule is indented with 5 spaces and uses double quotes ("no-restricted-globals": ['warn', 'document']) vs. the single-quote style elsewhere; and eslint.config.mjs is under ignores: ['*.config.*'], so it won't get auto-formatted.

Nit

• resolveCursorLayout(display, cur, curRefCurrent, columns) taking cur only to void it (same for the unused newValue/newCursor from fastAppendEffect) is a slight smell — passing an intentionally-dead arg to "prove" it's ignored. Harmless, just noting.

None of these are hard blockers except arguably #1 (which defeats part of the PR's purpose) and the trivial #2. Happy to approve once those two are addressed.

[kshitijk4poor]

Thanks for putting this together — the overall direction is good, and the new JS/TS matrix is green across all five workspaces. I also verified the desktop UI suite (146 files / 1,180 tests), TUI suite (107 files / 1,115 passed), web suite (9 files / 59 tests), desktop build, and native host build locally.

There are a few blockers before this is ready:

1. The desktop preview command is broken. apps/desktop/package.json changes scripts/assert-root-install.mjs to the nonexistent scripts/assert-root-install.mjPs. npm run --prefix apps/desktop preview fails immediately with MODULE_NOT_FOUND.

2. The Electron node tests are not part of the new CI gate. .github/workflows/js-tests.yml runs each workspace's check, but apps/desktop's check never invokes test:desktop:platforms; test:desktop:all is packaged-app validation, not the Electron node-test suite. The script currently names 30 Electron tests, with four additional top-level Electron tests omitted from it as well.

3. The source-regex cleanup is incomplete. apps/desktop/electron/windows-child-process.test.ts remains tracked even though it was removed from test:desktop:platforms. It still reads and regex-scans source, directly contradicting the new AGENTS.md rule, and now fails 4/5 tests because the implementation moved into extracted helpers. Please remove it and preserve any still-useful call-site contracts with behavior/DI-level tests.

4. Cross-target native staging can package the wrong binary. stageNodePty() accepts an unqualified host build/Release/*.node as satisfying any requested target. If no binary exists, it invokes electron-rebuild without the requested target architecture, despite beforePack passing the real target arch. That can compile and stage a host binary for a cross-arch target lacking a prebuild. The existing scripts/rebuild-native.mjs already accepts arch; please reuse/extend that path, only accept build/Release for a host-matching target, and fail unsupported cross-OS targets without a matching prebuild.

5. The nested @hermes/ink typecheck is not run by CI. The matrix runs ui-tui, whose check builds Ink but only typechecks the parent ui-tui/src/**. The new ui-tui/packages/hermes-ink check script is therefore unused by this workflow. Please call the nested package's check from the parent or add it to the matrix.

6. The PR description contains an inaccurate performance claim. It says this PR adds a 512-entry wrapAnsi LRU cache and halves the test time, but no such cache implementation appears in this diff; current main already has a separate bounded wrap cache. Please remove or correct that claim.

7. Formatting is not clean. apps/desktop/vitest.config.ts:18 contains trailing whitespace; both git diff --check and Prettier flag the file.

The extracted desktop/TUI helpers themselves look sound, the current production wiring preserves behavior, and the CI workflow rename is wired consistently. Once the issues above are addressed, this should be in much better shape for another pass.

[OutThisLife]

Re-reviewed the latest head (08b8044) in a worktree off the PR branch. The blockers from the prior two rounds are all resolved — nice work:

• Electron production code is typechecked again (tsc -p . and tsc -p tsconfig.electron.json).
• Cross-OS native staging now fails closed (requesting linux-arm64 from a darwin host throws instead of staging a host binary).
• ui-tui dev calls build:ink (no more Missing script: build-ink).
• preview typo fixed; the Electron suite runs under the vitest electron project and is now part of check; the nested @hermes/ink typecheck is in the workspace matrix; empty-matrix discovery fails closed.

Locally: electron project = 37 files / 404 passed / 1 skipped, electron typecheck clean, and it merges cleanly onto current main.

One new blocker, though:

1. classifyNativeBinary rejects real Darwin .node files → macOS build/check fails ⚠️

The Mach-O magic bytes are byte-reversed. Real node-pty Darwin thin binaries are stored little-endian on disk:

$ xxd -l 4 node_modules/node-pty/prebuilds/darwin-arm64/pty.node
00000000: cffa edfe    # MH_CIGAM_64 = 0xcffaedfe  (arm64 + x64 both)

but the classifier checks the big-endian form:

// Mach-O 64-bit: feedfacf
if (buf[0] === 0xfe && buf[1] === 0xed && buf[2] === 0xfa && buf[3] === 0xcf) return 'darwin'

so both Darwin prebuilds classify as null:

darwin-arm64/pty.node => null
darwin-x64/pty.node   => null
win32-x64/pty.node    => win32

On a Darwin host, stageNodePtyInto(..., { platform: 'darwin' }) then hits validateStagedBinaries and throws:

$ node apps/desktop/scripts/stage-native-deps.mjs   # host = darwin
Error: [stage-native-deps] native binary platform mismatch (target=darwin):
  prebuilds/darwin-arm64/pty.node: expected darwin, got unknown
$ echo $?
1

Since build ends with stage-native-deps.mjs and check ends with build, npm run check is broken for every macOS contributor. CI doesn't catch it because the runners are Linux (the ELF path is correct), and the new tests only plant big-endian fake headers — the exact false-confidence class this PR is trying to eliminate. The same command succeeds on main.

Fix: recognize the on-disk (little-endian) forms MH_CIGAM/MH_CIGAM_64 (ce fa ed fe / cf fa ed fe), and add a regression test that classifies a real Darwin prebuild (or writes CIGAM bytes). Worth handling FAT_CIGAM (be ba fe ca) too.

2. (carryover, low priority) electron-rebuild invoked via node_modules/.bin/electron-rebuild

Fine on POSIX, but on Windows the .bin entry is a shim rather than a Node script, so this fallback would fail exactly where node-pty prebuild gaps happen. Prefer resolving @electron/rebuild/lib/cli.js (or require.resolve). Only fires on the no-prebuild rebuild path.

Nit

validateStagedBinaries checks platform but not arch, so a same-platform wrong-arch binary could still slip through. Secondary to #1.

Overall the direction and the extraction work are solid — happy to approve once #1 is fixed and re-verified against a real Darwin prebuild (or a CIGAM fixture) in stage-native-deps.test.mjs.

[OutThisLife]

Re-reviewed at ede5024 — the classifier fix is correct and complete.

classifyNativeBinary now recognizes the little-endian on-disk forms (MH_CIGAM ce fa ed fe, MH_CIGAM_64 cf fa ed fe, FAT_CIGAM be ba fe ca) alongside the big-endian constants. Re-ran the exact repro that failed before:

• node apps/desktop/scripts/stage-native-deps.mjs on a Darwin host → exit 0 (was exit 1); staged darwin-arm64/pty.node validates as a real Mach-O 64-bit bundle arm64.
• classifyNativeBinary on the real node-pty prebuilds → darwin-arm64 => darwin, darwin-x64 => darwin, win32-* => win32 (all previously null).
• The test fixture now writes the real LE CIGAM_64 bytes instead of the big-endian form, so makeFakeNode('darwin') exercises the true on-disk path, plus dedicated regression tests for both LE forms. stage-native-deps.test.mjs: 15 passed.

Full electron project: 37 files / 408 passed / 1 skipped. Merges cleanly onto current main.

The two remaining items are non-blocking and can be follow-ups if you want:

• electron-rebuild invoked via node_modules/.bin/electron-rebuild (Windows .bin shim isn't node-runnable) — only fires on the no-prebuild rebuild path.
• validateStagedBinaries checks platform but not arch.

LGTM — approving. Nice work landing the JS/TS CI gate and the source-regex ban.