main
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrew Demczuk
|
6e611a687d |
fix(harnesses): resolve native CLI launch via the readiness ladder, not bare shutil.which (#3341) (#3535)
The seven native resolvers (pi, hermes, kimi, cursor, goose, kiro, qwen) looked up their CLI with a bare shutil.which, while readiness and the SDK executors resolve through resolve_cli_binary's fallback ladder (the nvm/npm/homebrew bin dirs the daemon's frozen PATH omits). A CLI installed only in a ladder dir passes the readiness badge but fails at launch. Route the resolvers through resolve_cli_binary so the badge and the launch agree. resolve_cli_binary gains a `which` hook so the resolvers keep their existing test seam; the fallback ladder always uses the real filesystem. Signed-off-by: Andrew Demczuk <andrew.demczuk@gmail.com> |
||
|
Pat Sukprasert
|
18c782f1f1 |
fix(codex,claude): resolve CLI binary beyond the daemon's frozen PATH (#2788)
The host daemon snapshots PATH at spawn and never refreshes it, so a codex or claude CLI installed into an nvm/npm-managed global bin dir (only added to PATH by interactive shell init) is invisible to shutil.which. Native Codex readiness then reports 'binary-missing' and the claude-sdk executor can't find its system CLI — even though a foreground launch works, because that runs in the interactive shell's PATH. Add a shared resolve_cli_binary(name, env_var) in _platform.py: override env var -> PATH -> a ladder of common global install dirs (~/.local/bin, /usr/local/bin, /opt/homebrew/bin, ~/.npm-global/bin). Route _find_codex_cli (OMNIGENT_CODEX_PATH) and _find_system_claude (OMNIGENT_CLAUDE_PATH) through it, and the codex readiness gate too, so the readiness verdict and the actual launch can't disagree. Update the codex binary-missing UI message and the ImportErrors to point at the real fix (restart the host, or set the override) instead of 'omnigent setup', which doesn't address a stale PATH snapshot. Co-authored-by: Isaac Signed-off-by: Pat Sukprasert <pattara.sk127@gmail.com> |
||
|
Zeyi (Rice) Fan
|
c3af15235b |
feat(terminals): native "+ New shell" honors $SHELL and offers installed shells (#2166)
## Related issue
N/A
## Summary
- Native-harness sessions (`omnigent claude`/`codex`/`pi`/etc.) previously
always opened bash for "+ New shell"; they now open the user's login shell.
- `omnigent/_platform.py`: add `default_interactive_shell()` (basename of
`$SHELL` when it names a known shell on PATH, else bash) and
`installed_interactive_shells()` (that default first, then any of
bash/zsh/fish on PATH; always non-empty).
- `omnigent/native_coding_agents.py`: `native_shell_terminal_spec()` now
declares one unsandboxed caller-process terminal per installed shell, keyed
and commanded by the shell basename, `$SHELL` first. The 11 native wrappers
call this shared helper instead of a hardcoded `{"shell": {"command": "bash"}}`
block.
- `web/src/shell/NewTerminalButton.tsx`: branch on
`useTerminalFirst().isNativeWrapper` — native sessions with multiple shells
get a split button (primary click launches the `$SHELL` default; a caret opens
a picker of installed shells, default labeled). SDK agents with multiple
distinct-purpose terminals keep the existing plain dropdown unchanged.
- `examples/polly/config.yaml`: add a `zsh` terminal alongside the existing
bash `shell` for the builtin polly agent.
## Test Plan
- `uv run pytest tests/inner/test_proc_and_platform.py tests/test_native_coding_agents.py`
— new unit tests for shell detection and the multi-shell spec.
- `uv run pytest -k "native and (materialize or terminal or agent_spec)"` — 296
passed, including the runner create-session-terminal flow; updated 4 native
wrapper tests that asserted the old single-`shell` shape.
- `npx vitest run src/shell/NewTerminalButton.test.tsx` (+ related shell suites)
— split-button default launch, caret pick of a non-default shell, and SDK
dropdown-unchanged cases.
- ruff check/format, prettier, oxlint, and tsc clean on all touched files.
- Verified polly's YAML parses through `_parse_terminals` with both `shell`
(bash) and `zsh` terminals.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor / chore
- [ ] Docs
- [ ] Test / CI
- [ ] Breaking change
## Test coverage
- [x] Unit tests added / updated
- [ ] Integration tests added / updated
- [ ] E2E tests added / updated
- [x] Manual verification completed
- [ ] Existing tests cover this change
- [ ] Not applicable
## Coverage notes
Shell detection, the native multi-shell spec, and the frontend split-button
behavior are covered by new/updated unit tests (pytest + vitest). Manually
verified that `default_interactive_shell()`/`installed_interactive_shells()`
resolve the host's shells, all 11 native wrappers import cycle-free, and
polly's edited YAML parses through Omnigent's real terminal parser. The live
end-to-end (clicking "+ New shell" in a running native session and confirming
the shell that opens) was not exercised here as it needs an interactive session.
|
||
|
Zeyi (Rice) Fan
|
0548405741 | Native Windows support (core / degraded mode) — re-land (#1236) | ||
|
Serena Ruan
|
cfb05db785 |
Revert "Native Windows support (core / degraded mode) (#1109)" (#1129)
This reverts commit
|
||
|
Zeyi (Rice) Fan
|
c11c6a38d1 |
Native Windows support (core / degraded mode) (#1109)
* feat(platform): add cross-platform process + platform primitives Introduce two dependency-light foundation modules for native Windows support: - omnigent/_platform.py: IS_WINDOWS/IS_POSIX/IS_LINUX/IS_DARWIN flags, default_shell_argv() (cmd.exe on Windows, bash/sh on POSIX), and stable_user_id() (uid on POSIX, hashed login name on Windows). - omnigent/inner/_proc.py: spawn_kwargs() (start_new_session on POSIX, CREATE_NEW_PROCESS_GROUP on Windows), terminate_tree()/kill_tree() (process-group fast path on POSIX, psutil descendant walk everywhere), and process_alive() replacing os.kill(pid, 0). psutil is already a core dependency, so no new packages. POSIX-only symbols (os.killpg/getpgid, signal.SIGKILL) are resolved via getattr so the module imports and type-checks on Windows. No call sites switched yet; later phases migrate to these helpers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): stop POSIX import-time crashes so the package loads On Windows several modules crashed at import before anything could run, blocking `import omnigent`, `omnigent --help`, and `omnigent server`. - server/performance_metrics.py: make `import resource` optional and fall back to psutil (a core dep) for RSS on Windows; load average already degrades to None. - terminals/ws_bridge.py, claude_native.py: guard the POSIX-only fcntl/pty/termios/tty imports behind `sys.platform != win32` (mypy special-cases this and still type-checks them on the Linux CI). These drive the tmux/PTY terminals, which are disabled on Windows. - Replace module-level / core-path `os.getuid()` namespacing with _platform.stable_user_id() and `/tmp`/`TMPDIR` with tempfile.gettempdir() in claude_sdk_executor (core SDK path) and the cursor/goose/claude native bridges; guard the POSIX ownership check in claude_native_bridge. Verified: a full walk of every omnigent submodule reports zero POSIX import failures; `import omnigent`, `omnigent --help`, and importing server.app / runner.app / the harness manager all succeed on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(windows): route process spawn/kill/liveness through _proc Replace POSIX-only process management with the cross-platform _proc helpers so child agent/server/runner processes spawn, tear down, and are probed correctly on Windows. - Spawning: swap `start_new_session=True` (and the os.name-conditional variant) for `**_proc.spawn_kwargs()`, which yields start_new_session on POSIX and CREATE_NEW_PROCESS_GROUP on Windows. Sites: cli.py (×2), chat.py, host/local_server.py, codex_executor, codex_native_app_server, runner transports tcp/uds, update_check. - Teardown: replace os.killpg-based `_terminate/_kill_process_tree` and the transport `_kill()` paths with _proc.terminate_tree/kill_tree (process-group fast path on POSIX, psutil descendant walk everywhere). - Liveness: replace `os.kill(pid, 0)` probes with _proc.process_alive. This was an outright bug on Windows, where os.kill(pid, 0) maps to TerminateProcess and would KILL the probed process — including the parent-death watchdogs in runner/_entry and runtime/harnesses/_runner, and process_manager's orphan sweep. - Guard the remaining force-kill signal refs with getattr(signal, SIGKILL, signal.SIGTERM) for the bare-pid kill paths in cli.py and host/local_server.py. Remaining live SIGKILL/os.kill(pid,0) sites are POSIX-gated only (the tmux PTY ws_bridge and the Linux-only prctl). Verified: process_alive probes a live process without killing it; all touched modules import on Windows; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(windows): TCP-loopback server<->harness IPC; disable egress proxy The harness process manager talked to each conversation subprocess over a Unix-domain socket, which asyncio's Proactor loop cannot provide on Windows. Introduce a transport abstraction so the same manager works on both platforms. - process_manager.py: add `_HarnessEndpoint` encapsulating UDS (POSIX) vs TCP-loopback (Windows) — spawn flags, readiness probe, httpx wiring, and cleanup. `_HarnessEndpoint.create` picks UDS on POSIX and a free 127.0.0.1 port on Windows. `_wait_for_socket_bind` -> `_wait_for_bind` probes the endpoint generically; `_SubprocessEntry` now carries the endpoint. - _runner.py (child): accept `--bind host:port` alongside `--socket`, and configure uvicorn with host/port or uds accordingly. - egress/controller.py: fail loud when an agent requests L7 egress rules on Windows (the proxy is a Unix-socket MITM listener with no Windows analog). POSIX is unchanged (still UDS; the public socket_path() returns the same path the endpoint binds). Verified end-to-end on Windows: a real _runner child binds TCP loopback, _wait_for_bind detects readiness, and an httpx request over the TCP transport returns 200. process_manager unit tests pass (3/3). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(windows): windows_jobobject sandbox backend (process containment) Add a Windows platform-default sandbox backend that contains the helper process tree via a kernel Job Object, since Windows has no bwrap/seatbelt equivalent. - New SandboxBackend.post_spawn(policy, pid) hook (default no-op): acts on an already-running pid, the model Job Objects require (a process is assigned to a job only after it exists). Returns a ContainmentHandle the parent holds and closes on teardown. - New windows_jobobject_sandbox.py: CreateJobObject + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + AssignProcessToJobObject via ctypes/kernel32 (no new dependency). resolve() returns an active policy and warns once that this backend does NOT isolate filesystem/network (read/write/allow_network are advisory on Windows); activate() is a no-op. Degrades gracefully (logs, returns None) if the Win32 calls fail (e.g. a non-nestable parent job in CI). - sandbox.py: register windows_jobobject and make it the Windows platform default; an explicit linux_bwrap/darwin_seatbelt still errors loudly on Windows. The backend module is imported only on Windows (it touches ctypes.windll) to keep the POSIX import graph untouched. - os_env.py: after Popen, call post_spawn for active policies and store the handle; close it in _stop_locked so kill-on-close reaps any descendants that outlive proc.terminate(). Verified on Windows: default resolves to windows_jobobject; an explicit linux_bwrap errors; and assigning a live process to the job then closing the handle terminates it (kill-on-close). POSIX is unchanged (the launcher backends keep the no-op post_spawn default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(windows): disable native terminals, cross-platform shell, packaging Phase 5-7 of native Windows support. - Native terminals: gate create_terminal_instance (the tmux/PTY chokepoint) and the `omnigent claude`/`codex`/`cursor` CLI commands behind a clear, actionable Windows error pointing to the SDK harnesses / web UI, instead of letting them crash on tmux/PTY. - Shell: make os_env._shell_argv and the shell_path fallback Windows-aware (cmd.exe uses /c, PowerShell uses -NoProfile -Command; POSIX bash/sh unchanged), and route model_catalog's provider auth_command (a core auth path) through _platform.default_shell_argv instead of a hardcoded /bin/sh. - Packaging: mark pexpect/pyte (POSIX PTY libs, never imported on the core path) as `platform_system != 'Windows'`, and document the native Windows install path (uv) plus its degraded-mode caveats in the README. Verified on Windows: _shell_argv emits correct argv per shell; the native terminal entrypoint and create_terminal_instance both reject with the actionable message; all touched modules import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(windows): platform skip markers, primitives tests, Windows CI - Add posix_only / windows_only pytest markers and auto-skip wrong-OS tests in tests/conftest.py (keys off os.name). Keeps the Linux suite unchanged and lets a Windows run skip POSIX-only tests cleanly. - New tests/inner/test_proc_and_platform.py covering _platform flags + shell argv, _proc spawn/terminate/liveness (incl. the non-destructive probe regression), the UDS/TCP harness endpoint, and the windows_jobobject backend (default selection + kill-on-close + fail-loud bwrap), gated by platform markers. - New non-blocking .github/workflows/windows.yml: installs via uv, asserts import omnigent and omnigent --help, runs the Windows-support unit tests as a hard gate, and a broader not-posix_only sweep as continue-on-error. Not wired into merge-ready, so it does not block. - Regenerate uv.lock for the pexpect/pyte platform markers (normalizer check passes); needed so the existing locked uv sync CI stays green. Verified on Windows: the hard CI test set passes (16 passed, 1 skipped). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: ruff-format windows_jobobject_sandbox.py Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): force web-ui asset MIME types so the SPA loads Starlette StaticFiles derives Content-Type from mimetypes.guess_type, which on Windows reads the registry, where .js is commonly mapped to text/plain. Browsers then refuse to execute the bundled SPA ES modules (disallowed MIME type), so omnigent server served a blank web UI on Windows. Register the web asset types .js/.mjs/.css/.json/.map/.wasm/.svg explicitly at server import via mimetypes.add_type. Harmless and deterministic cross-platform; removes the dependency on the host MIME registry. Verified on Windows: a real built assets/*.js now serves as text/javascript through the actual _SPAStaticFiles path (was text/plain). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): dereference Git-symlink example bundles on no-symlink checkout The bundled polly/debby example agents are Git symlinks under omnigent/resources/examples pointing at the top-level examples dir. On a Windows checkout with core.symlinks false (Developer Mode off / Git not elevated), Git materializes each symlink as a regular text file whose content is the link target. The spec loader then read the stub instead of the agent directory and failed to parse it as a YAML mapping. Re-checking out with symlink support needs Developer Mode or admin, so fix it at runtime: add _platform.resolve_repo_symlink, which on Windows detects a small single-line regular file whose content resolves to an existing path (the Git-symlink stub shape) and returns the real target; a no-op for real dirs/files, multi-line or unresolvable content, and off Windows. Apply it in cli._bundled_example_path and the server polly/debby bundle sources. Verified on Windows: the polly example now resolves to the real examples/polly directory with config.yaml. Added windows_only unit tests for the stub dereference and the leave-real-specs-untouched guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): pass Windows system env vars to sandboxed helpers A sandboxed os_env helper is spawned with a deny-by-default env allowlist (build_helper_env). The allowlist was POSIX-only (PATH/HOME/USER/...), so on Windows the child got no SYSTEMROOT. Winsock loads its providers from %SystemRoot%\system32\mswsock.dll, so the helper died at import asyncio with WinError 10106 (WSAEPROVIDERFAILEDINIT). Because windows_jobobject makes the sandbox active by default, this hit every agent that runs an os_env helper on Windows. Add the non-sensitive Windows system constants to the passthrough allowlist: SYSTEMROOT (mandatory for Winsock), plus SYSTEMDRIVE, WINDIR, COMSPEC, PATHEXT, NUMBER_OF_PROCESSORS, and PROCESSOR_*. Python uppercases env keys on Windows, so the names match os.environ as stored; they are absent on POSIX, so listing them is a no-op there (only present vars pass through). The security posture is unchanged - these are system constants, not credential-bearing. Verified on Windows: build_helper_env for an active sandbox now contains SYSTEMROOT, and a child spawned with that env imports asyncio cleanly (was WinError 10106). Added a windows_only regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): pass USERPROFILE/home + appdata to spawned subprocesses The host->runner spawn (and the os_env helper spawn) filter the environment through a POSIX-centric allowlist. After SYSTEMROOT was added, the runner got past import asyncio but then crashed at Path.home with Could-not-determine-home-directory, because on Windows that needs USERPROFILE (or HOMEDRIVE+HOMEPATH), the analog of POSIX HOME which is already allowed. Consolidate the Windows passthrough set into _platform.WINDOWS_ENV_PASSTHROUGH (system constants plus USERPROFILE/HOMEDRIVE/HOMEPATH plus APPDATA/LOCALAPPDATA) and reference it from both os_env._DEFAULT_ENV_PASSTHROUGH and host.connect._RUNNER_ENV_ALLOWLIST, so the two allowlists can no longer diverge. All are non-sensitive path/identity constants, consistent with HOME/PATH already being allowed; absent on POSIX so a no-op there. Verified on Windows: the host runner env now carries SYSTEMROOT and USERPROFILE, and a child spawned with it imports asyncio, resolves Path.home, and imports ClaudeSDKExecutor. Extended the windows_only regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): use the real temp dir for the harness instance dir The harness process manager pinned its instance/socket parent to the literal /tmp/omnigent, which on Windows resolves to \tmp\omnigent on the current drive (the symptom: instance_dir=\tmp\omnigent\ap-... in the logs). Keep /tmp/omnigent on POSIX (Unix socket paths have a tight length limit and gettempdir can be a long /var/folders path on macOS), but on Windows use tempfile.gettempdir()/omnigent. Windows uses TCP loopback for the harness IPC, so there is no socket-path length concern there. Verified: _default_tmp_parent() now resolves under %LOCALAPPDATA%\Temp\omnigent on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(windows): stop parent-death watchdog from killing the runner instantly The runner spawned by the host daemon exited cleanly (code 0) the moment it finished startup. Cause: the parent-death watchdogs treat a getppid() mismatch as the parent having died. On POSIX that is a reliable, PID-reuse-proof signal (orphans reparent to init). On Windows there is no reparenting AND os.getppid() is unreliable: the venv interpreter launcher breaks the parent link, so a spawned child reports a getppid that does not match its spawner (measured: child 15880 vs spawner 19852). So the getppid check fired immediately, the killer requested graceful shutdown, and the runner tore itself down right after HarnessProcessManager started. On Windows, skip the getppid heuristic and rely solely on an explicit liveness probe of the passed-in parent_pid (_proc.process_alive, psutil). Fixes both watchdogs: runner._entry._parent_is_orphaned and runtime.harnesses._runner parent watchdog. Verified on Windows: _parent_is_orphaned(<live pid>) is False (runner stays up) and True for a dead pid. Added a windows_only regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(windows): actionable client error when a native terminal is used A claude/codex/cursor-native (tmux/PTY) agent run on Windows hits the create_terminal_instance guard and surfaces a generic see-runner-logs banner in the web UI. Make the client-facing message Windows-aware: tell the user native terminals are not supported on Windows and to use an SDK harness (claude-sdk/cursor/copilot/codex) or run on Linux/macOS. The full cause is still logged for operators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(security): use SHA-256 (not SHA-1) for the user-id namespacing digest CodeQL flagged stable_user_id() for hashing the login name with SHA-1. The digest is only used to namespace per-user scratch directories (a filesystem-safe token), not for security, but switch to SHA-256 with usedforsecurity=False to document intent and clear the weak-algorithm finding. Output is still a 12-char hex token; behavior is otherwise unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: address code-quality review comments - _proc._ProcessLike and sandbox.ContainmentHandle: give the Protocol methods pass bodies instead of bare ellipsis (clears the statement-has-no-effect finding). - windows_jobobject_sandbox: import ctypes.wintypes as a submodule import rather than mixing a plain ctypes import with a from-ctypes-import (clears the dual-import-style finding). - windows_jobobject_sandbox: replace the module-level warned flag plus global statement with a functools.cache one-time warner (clears the unused-global-variable finding; behavior unchanged, the caveat is still logged exactly once per process). ruff and mypy clean; tests/inner/test_proc_and_platform.py 18 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(runtime): restore _pid_alive POSIX semantics (zombie counts as present) Phase 2 of this PR switched process_manager._pid_alive from os.kill(pid, 0) to the psutil _proc.process_alive probe. Those differ for a killed-but-not- yet-reaped process: os.kill(pid, 0) reports the zombie as present, psutil reports it as dead. That broke test_get_client_respawns_after_crash (and risked ~17 other call sites): the test SIGKILLs a harness and waits on not _pid_alive(pid) as a proxy for fully-reaped, which is the moment the asyncio child watcher sets the subprocess returncode and get_client respawns. With zombie-as-dead the wait returned at the zombie stage, before the reap, so get_client saw returncode None, did not respawn, and the first request to the dead client raised httpx.ReadError every time. _pid_alive answers is-this-PID-present-in-the-table (the os.kill idiom); _proc.process_alive answers is-this-a-live-non-zombie-process (liveness, used by the parent-death watchdogs). They are different predicates. Restore os.kill(pid, 0) on POSIX for _pid_alive (exact pre-PR behavior; its only production caller, the orphan sweep, checks non-child PIDs where zombies never occur) and keep psutil only on Windows, where os.kill(pid, 0) would map to TerminateProcess. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |