Skip to content

Dev#2

Merged
im4codes merged 58 commits intomasterfrom
dev
Apr 7, 2026
Merged

Dev#2
im4codes merged 58 commits intomasterfrom
dev

Conversation

@im4codes
Copy link
Copy Markdown
Owner

@im4codes im4codes commented Apr 7, 2026

merge

imcodes-win and others added 30 commits April 6, 2026 04:19
…shortcut

The daemon runs from a restricted environment (Startup shortcut / schtask)
where bare 'cmd.exe' / 'cmd' fails CreateProcess PATH resolution.

- command-handler.ts: upgrade batch spawn now uses COMSPEC absolute path
- windows-daemon.ts: tryStartStartupShortcut uses COMSPEC absolute path
- This was the root cause of empty upgrade logs — the batch never executed

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…hdog

Root cause: npm install -g deletes old package before writing new one.
The watchdog loop tried to restart the daemon during this window, hitting
MODULE_NOT_FOUND. After that the watchdog died and never recovered.

Fixes:
1. Upgrade batch now kills the entire watchdog tree BEFORE npm install,
   preventing the race entirely
2. On install failure, abort paths restart the old daemon via VBS so
   it's never left dead
3. After successful install, starts fresh watchdog via VBS directly
   instead of calling `imcodes restart` (avoids version-coupling)
4. Watchdog.cmd now uses the npm global shim (`imcodes.cmd`) instead
   of hard-coded node+script paths — always launches whatever version
   is currently installed, surviving npm upgrades without repair-watchdog
5. command-handler upgrade spawn uses COMSPEC absolute path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tall

Instead of relying on killing the watchdog tree (which doesn't work
when upgrading from an old version without tree-kill logic), use a
simple lock file (~/.imcodes/upgrade.lock):

1. Upgrade batch creates lock file BEFORE npm install
2. Kills old watchdog + daemon (old watchdog doesn't know about lock)
3. npm install runs safely — nothing tries to restart
4. repair-watchdog generates new watchdog.cmd with lock-checking loop
5. Starts new watchdog via VBS — it sees lock, waits
6. Deletes lock file — watchdog immediately starts the new daemon
7. On any abort path: delete lock + restart VBS so daemon is never left dead

Also:
- watchdog.cmd now uses npm global shim (imcodes.cmd) instead of
  hard-coded node+script paths — survives npm upgrades without
  repair-watchdog
- restartWindowsDaemon simplified: just kill daemon + launch VBS,
  let watchdog handle the restart loop

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
32 tests covering every daemon recovery scenario:

windows-daemon.test.ts (10 tests):
- Launcher priority: VBS > schtask > startup shortcut
- Daemon kill: by PID, handles already-dead, skips own PID
- PID wait: new PID detection, fresh start without previous PID
- windowsHide on all spawn calls

windows-upgrade-script.test.ts (16 tests):
- Lock file lifecycle: created before install, deleted on every exit path
- Every abort path: deletes lock AND restarts VBS (daemon never left dead)
- Success path: VBS started before lock removal (watchdog waits on lock)
- Old watchdog tree-kill before npm install
- Version verification and repair-watchdog ordering
- No imcodes restart on success path (watchdog handles it)
- Health check after restart
- No visible windows (minimized cleanup)
- "latest" target skips version comparison

windows-launch-artifacts.test.ts (6 tests):
- Watchdog.cmd: lock file check before daemon launch
- Watchdog.cmd: uses npm global shim, falls back to node+script
- Watchdog.cmd: infinite loop with 5s retry
- VBS launcher: hidden window (style 0)
- UPGRADE_LOCK_FILE path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
IM.codes and others added 28 commits April 6, 2026 23:09
…~ as home

Previous fix made `~` return Windows drive roots instead of home directory.
This broke file browsing on every platform:
- Linux/Mac: `~` test was checking for the WINDOWS_DRIVES_ROOT marker
- Windows: home directory was completely inaccessible — only drives shown

Fix:
- Daemon: `~` always means home dir (every platform). Drives are accessed
  via the explicit `:drives:` sentinel path on Windows only.
- Web: Add a 💾 / 🏠 toolbar button (Windows-only, detected by current
  path looking like X:\) to switch between home and drives view.
- i18n: New keys `file_browser.this_pc` and `file_browser.home` added to
  all 7 locale files.
- Tests: 11 daemon tests cover ~ + :drives: behavior on win32 and linux.
  27 web tests cover the toolbar button visibility and toggle behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…opups

The upgrade batch was launched with `windowsHide: true` but child processes
spawned inside it (wmic, find, tasklist via `for /f`) flashed brief console
windows on some Windows versions. This caused visible "find" / "cmd" popups
during daemon upgrade.

Fix:
1. Generate an upgrade.vbs wrapper that runs upgrade.cmd with WshShell.Run
   window style 0 (fully hidden, no taskbar flash, propagates to children).
2. Spawn `wscript upgrade.vbs` instead of `cmd /c upgrade.cmd`.
3. Replaced all `start "" /min cmd /c cleanup.cmd` with `wscript cleanup.vbs`
   (also wraps cleanup in a hidden VBS) — `start /min` still flashed in
   the taskbar.

Verified end-to-end on Windows: triggered the upgrade batch via wscript and
confirmed no visible windows. All 95 daemon-side tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three independent Windows-only bugs fixed in one shot.

1. SDK providers (claude-code-sdk, codex-sdk, qwen) failed silently on
   Windows machines that only have npm `.cmd` shims (no real `.exe`).
   Node's child_process.spawn() on Windows raises EINVAL when given a
   `.cmd`/`.bat` path without `shell: true`, so the SDK process never
   started — hence "starts but doesn't reply" reports.

   Fix: New helper `resolveExecutableForSpawn()` that:
   - Walks PATH + PATHEXT to find the real binary (preferring `.cmd`
     over the extensionless Unix shim that npm also drops)
   - Parses the npm `.cmd` shim to extract the underlying Node script
   - Returns `(node.exe, [script.js])` so spawn() works without shell:true

   For Claude SDK we use the SDK's `spawnClaudeCodeProcess` hook to
   override its internal spawn. Codex SDK and Qwen call spawn() directly
   so we wrap binary resolution at that point.

2. VBS files (daemon-launcher.vbs, cleanup.vbs, upgrade.vbs) were written
   as UTF-8 without BOM. Windows wscript reads BOM-less files as ANSI
   (system codepage, e.g. GBK on Chinese Windows), so a username with
   non-ASCII characters (e.g. "云科I") got mangled and wscript couldn't
   find the .cmd target — popping up "system cannot find the file"
   error dialogs.

   Fix: New `encodeVbsAsUtf16()` writes VBS as UTF-16 LE + BOM (Windows
   wscript native Unicode). New `encodeCmdAsUtf8Bom()` writes .cmd
   batch files as UTF-8 + BOM (cmd.exe respects this BOM).

3. Even when wscript fails to find the target, it must NEVER pop up an
   error dialog (visible to the user). Added `On Error Resume Next` to
   all generated VBS launchers — failures are now silent.

Tests added:
- transport-paths.test.ts: 12 tests for parseNpmCmdShim,
  resolveBinaryOnWindows, resolveExecutableForSpawn (npm shim parsing,
  PATHEXT preference, fallback behavior)
- windows-launch-artifacts.test.ts: VBS UTF-16 BOM, watchdog UTF-8 BOM,
  Chinese path roundtrip, On Error Resume Next presence
- windows-upgrade-script.test.ts: VBS error suppression for cleanup
  and upgrade wrappers, Chinese path handling

Verified end-to-end on Windows:
- All 131 daemon-side tests pass
- claude → resolves to real .exe (passthrough)
- codex/qwen → resolved to (node.exe, [cli.js]), spawn confirmed working
- Daemon restart picks up the new providers cleanly

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… resolver

CI failures from previous commit:

1. transport-paths.ts used path.win32.join/dirname/isAbsolute, which on
   non-Windows hosts (CI macOS / Linux) produces nonsensical paths when
   the test fakes `process.platform = 'win32'` but the underlying tmp dir
   is a posix path like /private/var/folders/...
   Fix: use the native `path.*` (path.join, path.dirname, path.isAbsolute)
   so the resolver works whether or not the host is real Windows.
   This is purely a test-correctness fix; on real Windows, native path
   IS path.win32, so behavior is identical.

2. Several test files mocked execFile with a 3-arg signature
   `(file, args, cb)` but the providers now call execFile with 4 args
   `(file, args, opts, cb)` to pass `windowsHide: true`. The 3-arg mocks
   threw "cb is not a function".
   Fix: rewrite mocks to accept both signatures via positional argument
   inspection.
   Files: claude-code-sdk-provider.test.ts, codex-sdk-provider.test.ts,
   qwen-cancel.test.ts, sdk-transport-restore.test.ts

Linux/Mac code paths unchanged — `resolveExecutableForSpawn` and
`resolveBinaryOnWindows` both early-return on `process.platform !== 'win32'`
so non-Windows daemons are byte-identical to before.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous fix used `delimiter` from `node:path`, which is the HOST OS
delimiter (':' on Linux, ';' on Windows). When CI tests fake
`process.platform = 'win32'` on a Linux/macOS runner, the imported
`delimiter` is still ':', so PATHEXT (which always uses ';') failed to
split into individual extensions.

Fix: hard-code ';' as the Windows delimiter inside the win32 branch —
this is correct on real Windows and works under platform-faking tests on
non-Windows hosts.

Verified locally: 147 tests pass across all affected files.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…names

CI failures on Linux runners (case-sensitive filesystems):
- Tests created `tool.cmd`, `tool.exe`, `mytool.cmd` (lowercase)
- PATHEXT was `.COM;.EXE;.BAT;.CMD` (uppercase)
- existsSync('tool.CMD') returns false on Linux even when 'tool.cmd' exists

Fix: lowercase the test PATHEXT so the fake-Windows lookup actually finds
the fixture files.  On real Windows the FS is case-insensitive so this
also works there (and on macOS HFS+ which is case-insensitive too).

Verified locally: 12/12 transport-paths tests pass on Windows.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three independent bugs that combined to crash the daemon on Windows:

1. **Duplicate uncaughtException handlers** — lifecycle.ts registered a
   handler that called shutdown(1), index.ts registered a 'stays alive'
   handler. Node calls ALL listeners, so the shutdown handler always
   ran and the keep-alive log was just decoration. Daemon died on every
   uncaught exception.

   Fix: removed the lifecycle.ts handler. Single global handler in
   index.ts.

2. **Stays-alive handler registered AFTER startup()** — node-pty can
   throw synchronously from socket event callbacks during startup
   (e.g. Cannot resize a pty that has already exited). If that fires
   before line 125 of index.ts, no handler is installed yet and the
   daemon dies on Node's default uncaughtException behavior.

   Fix: moved the global handlers to the very top of index.ts (lines
   3-30), BEFORE any imports. Now the handler is in place before any
   module-load-time side effect can fire.

3. **conptyResize crashed on already-exited PTY** — node-pty's resize()
   throws synchronously if the PTY has exited but the close event
   hasn't propagated yet. This was hitting ConPTY sessions during
   normal teardown.

   Fix: skip resize when session.exited is true; wrap the call in
   try/catch and mark exited if node-pty throws anyway.

Verified end-to-end on Windows: daemon survived multiple terminal
resize events that previously killed it. PID 1404180 still running.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…spawn

Three layers of defense to ensure the daemon NEVER dies:

1. **Top-level handlers use console.error, not pino logger**
   The previous handler tried to require('./util/logger.js') from inside
   the uncaughtException callback. ESM doesn't have require, so it threw
   silently and the error went unhandled.  Switched to plain console.error
   which is always available.  Also added a 'warning' listener.

2. **child.on('error') on every spawn**
   Node ChildProcess emits 'error' on spawn failure (ENOENT, EACCES, etc).
   If no listener is attached, it escalates to uncaughtException.  Added
   persistent error listeners to:
   - codex-sdk app-server child
   - qwen child (post-spawn — there was only a startup-race once())
   - claude-code-sdk Windows spawn override (the SDK may not listen)

3. **Verified sweep**: every spawn() in src/ now has either a try/catch
   wrapper, an error listener, or runs through node-pty (which uses an
   internal event handler that we wrap in our conpty.ts try/catch).

Combined with the previous lifecycle.ts handler removal and the index.ts
top-of-file global handlers, the daemon should now survive:
- node-pty resize-on-dead-pty
- spawn ENOENT / EACCES
- SDK transport process crashes
- any unhandled promise rejection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rror toast

Users were left in the dark when the daemon caught an error in the global
uncaughtException handler — the daemon kept running but they had no idea
why their session stopped responding.

Now:
1. uncaughtException / unhandledRejection broadcasts a `daemon.error`
   message via the global ServerLink (exposed as
   __imcodesGlobalServerLink in lifecycle.ts).
2. server bridge default-broadcasts unrecognized message types, so no
   server-side change needed.
3. web app subscribes to `daemon.error` and shows a 10-second toast with
   the error message + logs the stack to console.

Per-session errors (codex/qwen child process spawn errors etc.) still
flow through the existing transport-relay → timeline.event path so users
see ⚠️ Error inline in the affected session.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@im4codes im4codes merged commit ecfe6d7 into master Apr 7, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant