Repository navigation
feat(windows): Windows support across core services + Claude Code core - #1789
Conversation
4cdeacd to
4e14306
Compare
… core
Adds a cross-platform abstraction layer (src/platform.ts + src/sutando_platform.py)
and PowerShell twins of the bash startup/restart/notify/watch + skill installer
scripts so the headless Sutando loop (voice, screen capture, task bridge,
dashboard, agent API, Claude Code core) runs on Windows. macOS-only tools
(AppleScript-driven app automation, QuickTime/Chrome control) degrade with a
clear `macOSOnly` error instead of silently no-op'ing.
- src/platform.{ts,py}: notify / clipboard / screen capture / process probe /
port-in-use / open-with-default helpers; macOS path = legacy osascript +
pbcopy + screencapture, Windows path = PowerShell System.Drawing + Get/Set-
Clipboard + Win32_Process + netstat.
- src/{startup,restart,notify,watch-tasks-stream}.ps1: PowerShell twins of the
.sh originals. startup.ps1 also installs skills (skills/install.ps1) and
auto-launches the core Claude Code session via scripts/start-cli.ps1 in a
fresh Windows Terminal window.
- scripts/start-cli.ps1: Windows twin of scripts/start-cli.sh. Generates a
temp launcher .ps1 to avoid shell-quoting hell across wt.exe + pwsh.
- skills/install.ps1: junction-based skill installer (no admin needed) so
/schedule-crons and other slash commands resolve in the core agent.
- src/inline-tools.ts + browser-tools.ts + meeting-tools.ts + recording-tools.ts:
gate macOS-only tools with macOSOnlyError; rewrite clipboard tool to route
through platform.ts (cross-platform); replace hardcoded /tmp/ with tmpdir().
- src/voice-agent.ts: relax assertMacOS to a warning; gate Zoom auto-detect
to darwin; route fatal-close notification through platformNotify; accept the
AQ.A... Gemini key prefix in addition to legacy AIza...
- src/task-bridge.ts: replace pgrep watcher-liveness probe with a PID-file
sentinel on Windows; skip fast-path concat (bash-dependent) on win32.
- src/screen-capture-server.py: branch single-display capture through
sutando_platform.capture_screen so PowerShell System.Drawing is used on
Windows; per-display screencapture -D<n> stays macOS-only.
- README.md: new "Windows support" section documenting what works, what
returns macOSOnly errors, and the Windows quickstart.
Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Claude Code 2.1.168 on Windows does not include the Monitor tool (verified: not in the agent's runtime tool list AND the literal "Monitor" string is absent from claude.exe). Without Monitor there is no push-based file-watch primitive available to the long-running sutando-core agent, so user chat tasks would only be picked up on the */5 proactive-loop cron tick — too slow for chat. - src/task-dispatcher.ps1: standalone FileSystemWatcher over tasks/. Claims each new task via atomic rename to .processing, pipes the body into `claude --print --dangerously-skip-permissions`, writes results/<id>.txt, archives to tasks/archive/. Single-instance via PID sentinel. Smoke-tested: ~5s end-to-end per task. - src/startup.ps1: auto-launches the dispatcher in a hidden background window. Opt out with -SkipDispatcher. - src/restart.ps1: added task-dispatcher to the kill-pattern list. - scripts/start-cli.ps1: boot prompt changed from /schedule-crons to /proactive-loop so the first cron tick is not gated on a 5min wait; also generates a temp launcher .ps1 to bypass wt.exe/pwsh argv quoting issues; renamed $pid loop var to $corePid (PowerShell automatic). - src/watch-tasks-stream.ps1: rewrote to use FileSystemWatcher.WaitForChanged in the main pipeline so Write-Output reaches the script's stdout (the previous Register-ObjectEvent path emitted into a separate runspace's output stream, never reaching parent stdout). - CLAUDE.md + skills/proactive-loop/SKILL.md: kept the macOS Monitor instructions verbatim, added a Windows-specific paragraph explaining that Monitor is absent on Windows and pointing at task-dispatcher.ps1. - README.md: replaced the cron-workaround paragraph (wrong premise) with an accurate description of the Monitor gap and the dispatcher design, including the per-task no-shared-context trade-off. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Previously every dispatched task was a fresh `claude --print` subprocess with no shared context across chat turns. Now each task is mapped to a per-channel claude session that persists across dispatcher restarts. - First task on a channel gets a freshly-minted UUID passed to `claude --print --session-id <uuid> --output-format json`. Subsequent tasks on the same channel use `--resume <uuid>` so claude continues the same conversation. - Channel resolved from the task file's `channel_id` header (defaults to `local`). Voice / discord / telegram / etc. each keep their own thread, so contexts don't bleed. - Session map persisted at <workspace>/state/dispatcher-sessions.json, written via tmp-file + rename for crash safety. Loaded on startup, serialized under a Monitor lock so concurrent task processing doesn't race on the map. - --output-format json so we can extract the `.result` field and check `.is_error` structurally instead of pattern-matching stdout. If claude rotates the session_id mid-conversation (spec-allowed), the new id is captured and persisted. Verified end-to-end on Windows: turn 1 (channel=continuity-test): "remember pineapple" -> "Got it..." turn 2 (same channel): "what was the word?" -> "pineapple" session map persisted across both turns; turn 2 = 4s (resume vs 21s cold). Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
…ival - src/task-dispatcher.ps1: `Get-TaskHeader` now stops scanning at the first `task:` line, so a multi-line task body that legitimately contains text like `channel_id: forge` cannot spoof a header. Mirrors the TS-side stop-at-task convention from PR sonichi#982 (`task-bridge.ts:_isVoiceTask`). - skills/proactive-loop/SKILL.md step 1: spell out that the consumer must move processed task files from `tasks/<id>.txt` to `tasks/archive/<id>.txt`. On macOS the long-lived task-bridge.ts archives automatically; on Windows the core agent is sometimes the consumer (dispatcher restart, etc.) and must archive itself — otherwise the watcher's `$seen` set only suppresses re-emission within one watcher lifetime, and a restart re-emits every file still in `tasks/`. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
On Windows, str(repo) yields backslash paths (C:\code\sutando), so the forward-slash-only replace left the slug unchanged. Passing that absolute path into Path.home()/...slug discards the home prefix entirely, so the memory-dir check always probed an impossible path and perpetually reported "not yet created" regardless of actual memory state. Replace ':' and '\' too, matching Claude Code's project-slug convention (C--code-sutando). Co-Authored-By: Claude <noreply@anthropic.com>
…rm find_pids health-check shelled out to /usr/bin/pgrep, which doesn't exist on Windows — the call raised FileNotFoundError, was swallowed, and every bridge was reported "configured but not running" even with a live process. Add find_pids() to sutando_platform (pgrep on macOS/Linux, Get-CimInstance on Windows) and route the discord/telegram bridge check through it. find_pids honors a trailing `$` as an end-of-command-line anchor on both platforms (pgrep regex; PowerShell EndsWith), so the real `python src/<name>-bridge.py` process matches while transient shells that merely mention the script mid-command-line are excluded — otherwise the Windows substring match tripped the "multiple processes" zombie warning. Verified on Windows: discord-bridge (live PID) now reports ok/running. Co-Authored-By: Claude <noreply@anthropic.com>
…artup Add msvcrt byte-range locking fallback in single_instance.py for Windows (POSIX fcntl path unchanged), launch the Discord bridge from startup.ps1, and make the single-instance test suite Windows-safe by closing lock FDs before temp-dir cleanup and passing paths via env instead of source interpolation. Co-Authored-By: Claude <noreply@anthropic.com>
Adds tests/find-pids.test.py exercising the matcher used by health-check's bridge/service detection: nonsense→[], finds a spawned child by a unique marker, and the `$` end-anchor matches a trailing marker while rejecting a mid-command-line one (the property that keeps the bridge check from matching transient shells that merely mention the script). Writing the test surfaced a self-match in the Windows branch: the pattern is interpolated into the Get-CimInstance command, so the probe's own powershell process matched unanchored `.Contains` patterns. Tag the script with a fixed sentinel and skip it (and $PID). Anchored callers (the only shipping use, e.g. `discord-bridge\.py$`) were already unaffected; verified still ok. Run with `python` (not `python3`) on Windows. Co-Authored-By: Claude <noreply@anthropic.com>
The dispatcher piped raw task files to the chat subprocess with no framing, and two parse bugs let task metadata leak into the prompt: - Get-TaskPrompt swallowed trailing headers (source, channel_id, access_tier, ...) into the body, so the model treated them as the user's message and narrated its own internals back. - Get-TaskHeader bailed at the `task:` line, so channel_id/source were never read; every chat task collapsed onto one shared session and the framing never applied. Add a per-source conversational preamble (Discord/Telegram/Slack/voice) instructing the reply to answer directly and never mention dispatchers, task IDs, sessions, files, or repo state. Stop the prompt body at the first known header; honor a fixed header allowlist when scanning. Force UTF-8 end to end so degree/dash glyphs stop arriving as mojibake. Co-Authored-By: Claude <noreply@anthropic.com>
Path.rename raises FileExistsError on Windows when the target exists; Path.replace overwrites atomically on all platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bridge-supervisor.ps1 keeps a chat bridge online via a restart-on-exit loop with capped backoff; install-bridge-task.ps1 registers it as a scheduled task. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On Windows, `python3` on PATH is the Microsoft Store app-execution-alias stub: `command -v python3` succeeds but running it exits 49 without executing. This broke `scripts/sutando-config.sh` — the foundation of workspace resolution the proactive loop + startup chain depend on. Add `scripts/python-bin.sh::resolve_python_bin` — returns the first candidate that actually runs via a functional `-c pass` probe (python3 → py → python), rejecting the stub. No-op on macOS/Linux where real python3 stays first. Honors SUTANDO_PYTHON_BIN override. Route all interpreter calls in sutando-config.sh through the resolved $PYBIN. Co-Authored-By: Claude <noreply@anthropic.com>
All five Windows PowerShell scripts resolved the workspace with pre-M0 logic ($SUTANDO_WORKSPACE else ~/.sutando/workspace). The M0 contract (v0.8 / sonichi#1440) makes the default <repo>/workspace/ and no longer honors $SUTANDO_WORKSPACE, so on a stock box the bash/Python services and the .ps1 services resolved different workspaces — a split-brain where the dispatcher polled one tasks/ dir while the watcher/bridges used another. Add src/workspace_default.ps1::Resolve-SutandoWorkspace — shells to the canonical `bash scripts/sutando-config.sh workspace`, falls back to <repo>/workspace/ when bash is unavailable, CWD-independent via $PSScriptRoot. Twin of workspace_default.py / .ts. Repoint startup, watch-tasks-stream, task-dispatcher, bridge-supervisor, and notify to dot-source it. Co-Authored-By: Claude <noreply@anthropic.com>
The Windows task-dispatcher feeds the task: body to `claude --print --dangerously-skip-permissions` (full caps, no sandbox). For non-owner Discord tasks the bridge appends a ===SUTANDO SYSTEM INSTRUCTIONS=== fence telling the consumer to run under codex --sandbox read-only, but writes it AFTER the metadata headers. Get-TaskPrompt stops at the first header line, so the fence is discarded and the bare body would run unsandboxed — violating CLAUDE.md's non-owner-MUST-be-sandboxed contract. macOS is unaffected (core agent reads the whole file incl. the fence). Add a tier guard in Process-Task: read access_tier via Get-TaskHeader; if set and not owner, write the bridge's `Sandbox unavailable; refusing non-owner task.` sentinel, archive, and return without invoking claude. Owner tier and no-tier tasks (voice/local) are untouched. Verified on Windows 11: synthetic non-owner task (malicious body + fence) → tier reads `other`, guard fires, fence confirmed stripped from prompt. Owner/no-tier → guard does not fire. PSParser clean; find-pids/single-instance/proactive-routing suites still pass. Co-Authored-By: Claude <noreply@anthropic.com>
The macOS Zoom auto-detect block lost its catch clause during the rebase conflict resolution, leaving the top-level try unterminated. esbuild/tsx then failed to parse the module (TransformError: Unexpected brace at the next top-level import). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Thin shortcut for restart.ps1 -StopOnly, mirroring stop.sh's delegation to restart.sh --stop-only. Documented in the README Windows-scripts list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
discord-bridge.py hardcoded INBOX_DIR = Path("/tmp/discord-inbox") and
called .mkdir(exist_ok=True). On Windows /tmp has no parent on the drive
root, so module import died with WinError 3 — blocking even importing the
bridge (and the discord-bridge-mod-judge-dispatcher test suite) on Windows.
Use the OS temp dir off-POSIX; the macOS/Linux /tmp path is byte-for-byte
unchanged (os.name == "posix" branch). Added parents=True so the Windows
temp subdir is created robustly.
Co-Authored-By: Claude <noreply@anthropic.com>
The non-owner codex dispatch wrote its prompt to a hardcoded
`/tmp/sutando-<id>.txt`. On Windows `Path("/tmp/...").write_text()` raises
WinError 3 (no parent), the same class of bug already fixed for INBOX_DIR in
e8c29a5 but missed here. Resolve the dir via tempfile.gettempdir() off-POSIX;
the posix branch keeps the literal `/tmp` path so macOS/Linux is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
Get-TaskHeader scanned every line for key: value, so a non-owner could forge �ccess_tier: owner on a second line of the ask: body and bypass the tier sandbox guard in Process-Task. Mirror Get-TaskPrompt's body boundary: once inside the ask: body, ignore lines until a genuine trailing-header line or the ---/fence marker, matching keys only outside the body. Defense-in-depth beyond producer-side confine_user_content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4e14306 to
436446d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 872cc934bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… is not forge defense The Get-TaskHeader body-skip comment (436446d) claimed to close the forged `access_tier: owner` privilege-escalation hole as defense-in-depth. Empirically it does not: the body boundary is detected by HEADER_LINE_RE, so a forged `access_tier:` line ends the body and is then read as a trusted header (a `task:`+`access_tier: owner` two-liner returns 'owner', verified via pwsh). The real, sufficient defense is producer-side confine_user_content, which U+200B-prefixes header-shaped user lines so they no longer match here. Comment now states this accurately and scopes the unconfined hand-dropped file as needing a producer-side body fence, not a dispatcher heuristic. Behavior unchanged (comment-only). Co-Authored-By: Claude <noreply@anthropic.com>
Filter Get-CimInstance to python processes before the CommandLine scan instead of enumerating every Win32_Process. Co-Authored-By: Claude <noreply@anthropic.com>
john-the-dev
left a comment
There was a problem hiding this comment.
Reviewed the macOS-path changes and cross-platform abstractions. CLA signed ✓, all CI checks pass ✓ (tsc + tests, workspace-resolution lints).
Changes to existing macOS files — look correct:
discord-bridge.py:tmp.replace()instead oftmp.rename()— right fix; on Windowsrename()raises if the destination exists,replace()is the cross-platform atomic swap. Thetempfile.gettempdir()substitutions for/tmpare correct.health-check.py:find_pidsreplaces the hardcoded/usr/bin/pgrep -f; slug now handles:and\for Windows drive paths.inline-tools.ts/browser-tools.ts/recording-tools.ts/meeting-tools.ts:macOSOnlyErrorguards at the top of each macOS-specific execute() — clean degradation, no macOS behavior change.scripts/sutando-config.sh: sourcespython-bin.shto avoid the Windows Store stub. No-op on macOS (probe passes immediately). Adds onepython3 -c passcall per invocation — fast in practice since this is called once per pass, not in a tight loop.
One question for author/maintainer:
README documents the Windows workspace default as %USERPROFILE%\.sutando\workspace\, while macOS defaults to <repo>/workspace/ (in-repo, per M0 contract). That's a platform-divergent default — intentional? If the workspace is expected to hold git-ignored runtime state and the user is running from a git checkout, an in-repo default would be consistent across platforms. If the Windows default is intentional (user profile = more standard on Windows), that's fine, but it should be noted in docs/workspace-config.md so users migrating between machines know the paths differ.
Cannot verify on macOS:
The task-dispatcher.ps1 security properties (non-owner access-tier guard, atomic file claim) and the PowerShell-specific services (startup.ps1, bridge-supervisor.ps1) need a Windows reviewer. The PR description's claim that Get-TaskHeader is confined outside the task: body (to prevent header-forgery by a non-owner) is load-bearing from a security standpoint — worth a second set of Windows eyes before merge.
Overall the macOS path is clean. Deferring to a Windows reviewer + the maintainer for the workspace-default question.
stellaxiang
left a comment
There was a problem hiding this comment.
Reviewed the macOS changes, platform abstraction, and dispatcher security model. Architecture is sound; one documentation bug needs fixing before merge.
What looks correct:
src/platform.ts: Clean abstraction layer. SoftmacOSOnlyErroron unsupported platform — no crash. PowerShell clipboard/notify/kill/port-check implementations look correct for the stated behaviors.task-dispatcher.ps1security model:Get-TaskHeaderskips the task body region to avoid body-to-header confusion. The comment honestly disclaims thataccess_tier: ownerforged in a trailing header position would be read as real — and correctly notes the actual defense isconfine_user_content(PR #1743, already in main). Non-owner tasks are refused rather than silently downgraded: that's the right conservative behavior for a host that has nocodex --sandboxequivalent available.workspace_default.ps1: Correctly shells tobash scripts/sutando-config.sh workspacefirst (M0 canonical), falls back to<repo>/workspace/, and explicitly does NOT honor$SUTANDO_WORKSPACE(matches the v0.8/#1440 deprecation). Good.
Blocking: README workspace default doesn't match the code
README Windows section says:
defaults to
%USERPROFILE%\.sutando\workspace\unlessSUTANDO_WORKSPACEis set in.env
But workspace_default.ps1 does not implement that default anywhere — it falls back to <repo>/workspace/ (the M0 in-repo default). And $SUTANDO_WORKSPACE is explicitly NOT honored per the resolver code (NOTE: $SUTANDO_WORKSPACE is intentionally NOT honored.). A Windows user following the README would look for files in %USERPROFILE%\.sutando\workspace\ and set SUTANDO_WORKSPACE — both wrong. Fix the README to match the actual resolver: <repo>/workspace/ (same as macOS), configurable via sutando.config.local.json.
Cannot verify — Windows CI gap
The CI doesn't have a Windows runner, so the task-dispatcher.ps1 end-to-end flow, FileSystemWatcher atomic rename, and bridge-supervisor.ps1 lifecycle management can't be CI-verified. The PR notes this. John's comment about needing Windows eyes for the dispatcher is correct — this is a known gap, just flagging for the maintainer to track.
Minor: macOS behavior unchanged
Confirmed: every macOS-specific path is still taken when process.platform === 'darwin'; no new code runs there. John's earlier review of the macOS changes is accurate.
(Bot review — no merge; owner merges.)
Stand: Stella Act I
startup.ps1 launches the core through scripts/start-cli.ps1, which handed Start-Process argument arrays containing $psExe.Source (C:\Program Files\PowerShell\7\pwsh.exe) and a %TEMP% launcher path. Start-Process joins array elements with bare spaces, so both paths split at the space: Windows Terminal received `C:\Program` as the command, and a profile directory with a space broke the launcher path on the plain-window branch too. startup.ps1 already solved this for its own launches with ConvertTo-NativeArgumentString, but the helper was defined inside startup.ps1 only. The helper moves verbatim to scripts/native-arguments.ps1 (next to scripts/python-binary.ps1, the existing dot-sourced helper), and startup.ps1, start-cli.ps1 and the dispatcher's -Background self-launch all dot-source it, so there is one quoting policy. The dispatcher was already quoting $PSCommandPath by hand; it now uses the shared helper, which also handles embedded quotes and trailing backslashes. The generated launcher interpolated the repo path, the claude path and the arguments into single-quoted literals, and only the arguments had their quotes doubled. A path such as C:\Users\O'Brien produced a launcher that does not parse. Every interpolated value now goes through one ConvertTo-SingleQuotedLiteral helper. tests/windows-startup.test.ps1 now dot-sources the shared helper and runs start-cli.ps1 end to end with mocked process discovery, for both the Windows Terminal and the plain pwsh branch, with pwsh under "Program Files", a claude path containing an apostrophe and a TEMP directory containing spaces and an apostrophe. The mocked Start-Process joins arrays the way the real cmdlet does, and the resulting command line is round-tripped through a real child process to prove the argv boundaries survive; the generated launcher must parse and must resolve to the same claude path and arguments. Against the previous start-cli the round trip fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
restart.ps1 ran `taskkill /T /F` on every task-dispatcher match and on the sutando-core claude process. A restart requested from chat runs as task-dispatcher -> claude --print -> pwsh restart.ps1 (or sutando-core -> pwsh restart.ps1), so tree-killing the dispatcher or the core also killed restart.ps1 before it reached startup.ps1, leaving every service stopped. Nothing restarts them until the user notices. Two mechanisms, each covering what the other cannot: 1. A full restart now relaunches itself detached (Start-Process with redirected output to <workspace>/logs/restart.log, -Detached and -LauncherPid) and the launcher exits immediately. The detached instance waits for the launcher to exit, then stops and starts. This is what keeps the restart alive after its caller dies: without it, a restart that merely avoided killing itself would still be a child of `claude --print` and bound by its Bash tool timeout, which can end a long startup together with the services it launched. The launch uses the shared ConvertTo-NativeArgumentString, so a repo path with spaces survives. 2. The stop list is computed by Get-SutandoStopPlan from one Win32_Process snapshot: it walks the ParentProcessId chain from the current process (terminating on unknown parents and on cycles from recycled ids) and never tree-kills an ancestor. An ancestor match is stopped on its own with `taskkill /F`, which removes the stale dispatcher or core without reaching this script. This is the correctness guarantee for -StopOnly (stop.ps1 from chat, which does not detach because it has no startup to protect) and for a detached instance whose parent link still resolves. Relaunching detached was chosen over only excluding ancestors because exclusion alone leaves the restart inside the caller's lifetime and timeouts. Detaching alone was not enough either: taskkill /T follows parent ids, so correctness would rest on the launcher having exited first. The cost is that an interactive full restart prints the log path instead of streaming startup output, and its exit code no longer reflects startup's; the README says where the log is. tests/windows-restart.test.ps1 runs the real restart.ps1 against synthetic process tables with Get-CimInstance, taskkill, Start-Process and pwsh mocked: the dispatcher-ancestor chat chain (dispatcher stopped alone, other services and the core tree-killed, startup run once), the core-ancestor StopOnly chain (no relaunch, no startup), an orphaned detached instance, a recycled-id cycle, and the plain invocation, which must only relaunch itself detached with the exact quoted arguments. The core-ancestor case fails against the previous script. It is wired into the Windows runtime workflow next to windows-startup.test.ps1 and also passes on macOS pwsh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
I pushed fixes for the three code issues from my re-review (as a maintainer edit on top of N1: a failed archive no longer stops the dispatcher (
|
|
Native Windows review at head Environment. Windows 11 Home 10.0.26200 · pwsh 7.6.6 · Python 3.13.7 · Node 24.19.0 · New: the voice agent does not know it is on WindowsThis is not in any existing review, and it only shows up in a live session.
On Windows that is the only statement about the host the model ever receives — The single correction anywhere in the system is inside Observed (reproduced, live voice session at this head): the agent believes it is on a Mac, then switches to knowing it is on Windows partway through a session — deterministically, at the first macOS-only tool failure. It fails safely ( Already fixed upstream — @thinkall landed One correction worth keeping for anyone who revisits the tool surface: Also confirmed working by voice in the same session: clipboard round trip with @bassilkhilo-ag2's N1 — verified against the trigger it actually describesThe committed regression, Two different error paths, and the sharing-violation one had no coverage on the only platform where it happens. So I injected the real trigger: an open handle with Then the fixed dispatcher against it (reproduced, isolated workspace): It survives the failed sweep, keeps serving, survives a restart against the same claim, archives cleanly on release, and invokes Why this matters in practiceMy own production Windows install runs a build predating this fix, and it shows the bug this PR removes: A N2 — checked on a real process treeN2 was filed "Read, not run on Windows". I built a genuine three-deep chain whose ancestor's command line matches Ancestors land in the plan with Scope honesty:
|
| Leg | Result |
|---|---|
claude resolution |
claude.ps1 — the shim path, no .exe on this box |
| Owner turn | OWNER_LIVE_OK via the real API |
| Post-restart turn | POSTRESTART_OK, session resumed, not recreated |
| Guest → sandbox routing | correct |
Unsigned collaborator: true |
not trusted into the owner path — routed to the sandbox |
This closes the shim question from the original Codex review: a .ps1 shim resolves correctly, a case CI's claude.cmd never exercised.
Two legs remain open and I am not claiming them. No Discord bridge delivery — access_tier derives from the Discord sender_id, so the collaborator and non-owner legs need messages from other people's accounts. And the sandbox produced no reply: codex is installed but unauthenticated here (404 on model refresh), so the guest path returned Sandbox unavailable. Routing is proven; the sandbox reply is not. I would keep live-test-requested set until the Discord legs are done.
Other gates, run natively
windows-dispatcher-live.test.ps1 full green (interrupted_claim_archived, restart_descendants_stopped, post_restart_result: WINDOWS_OWNER_OK, reused_pid_ignored, duplicate_dispatcher_blocked, sandbox + collaborator + structured-error paths) · windows-restart.test.ps1 · PowerShell parse gate over every .ps1 · tsc --noEmit rc 0 · ruff clean · 10-test Python regression batch, 0 failures.
Also fixed in thinkall#6
N3 is broader than filed. resize_image fails on U+2018 and U+201B as well as U+2019 — reproduced before/after:
| path segment | before | after |
|---|---|---|
Smith, O'Brien (U+0027) |
ok | ok |
O’Brien (U+2019) |
fail | ok |
O‘Brien (U+2018) |
fail | ok |
O‛Brien (U+201B) |
fail | ok |
spaces, $, backtick, [](), ;&, CJK |
ok | ok |
Both paths now travel in the environment, matching openWithDefault.
windows-dispatcher-live.test.ps1 fails open on workspace isolation. It isolates via SUTANDO_TEST_MODE=1 + SUTANDO_WORKSPACE, but that override is honored only inside src/sutando_config.resolve_workspace, so Resolve-SutandoWorkspace reaches it only when the Python probe works. With python not resolvable on the PATH the test builds, the resolver silently returns <repo>/workspace — and the test drives the real dispatcher against a developer's live workspace, then fails on a timeout with that dispatcher still running, because $dispatcherPid was never learned so the cleanup tree-kill never fires.
Reproduced — I hit this before diagnosing it. With python hidden from PATH:
python visible: False
intended : C:\Users\...\Temp\iso-12517ccf...
resolved : C:\Repos\...\repo\workspace
The test now resolves through the production resolver and refuses to start unless it matches the temp path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 01ca5234-bb52-47e4-8e86-33e745ea385a
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 01ca5234-bb52-47e4-8e86-33e745ea385a
|
@elCaptnCode — coordinating on my last standing blocker for this review. CI at head That's exactly the gap you already have the setup to close — the live Rui-Mac / your-Windows Discord bridge from your 2026-09-09 comment on this thread. Could you run one more round at this head (or whatever head is final when you get to it): a genuine non-owner/collaborator message through that live Discord setup, and post the actual request + reply here, plus whatever confirms it went through Codex read-only routing (log line, timestamp, or similar) and which revision it ran against. No credentials in the post, obviously. This closes the last item on my |
|
Reviewer decision, explicit as requested: I'm not converting to approve yet, and I'm keeping the live non-owner round trip as a pre-merge item rather than a documented follow-up. Explaining why, since this is a real ask to reconsider and deserves a real answer, not just a restated position. What CI green actually proves here, precisely. Why that specific gap matters on this PR, not as a general principle. This isn't a case of holding a routine Windows port to some abstract live-test standard. This PR's own history is the evidence: HOME unset breaking the capture-token path, Why I'm not treating it as a documented follow-up. This is a security boundary, not a feature gap — it's the mechanism that keeps a non-owner from getting direct/unsandboxed access. A documented-follow-up gap on a security boundary means it ships live and gets found in production if it's wrong, which is a worse place to find it than here. Making it easier to close, since I don't think the ask should just get a flat no. It doesn't have to go through Discord specifically — that's just the fastest existing harness. Any genuine live invocation works: elCaptnCode (or anyone with a real Windows install) hand-crafts one non-owner task file, runs it through the actual dispatcher at this head, and posts the real output. That's a much smaller lift than a live cross-instance Discord session, and it's the same evidence I asked for either way. I'll re-review and move my CHANGES_REQUESTED the moment that lands — everything else on this PR is genuinely done. |
resize_image built its PowerShell script by interpolating the input and output paths into single-quoted literals, escaping only the ASCII apostrophe (U+0027). PowerShell also treats U+2018-U+201B as single-quote delimiters, so a profile or folder named O’Brien produced a script that does not parse: the resize failed and screen-capture-server returned HTTP 500 for every frame over the 400 KB cap. Both paths now travel in the environment (SUTANDO_RESIZE_INPUT / SUTANDO_RESIZE_OUTPUT) and the script reads $env:, so no path character reaches script source. This is the pattern openWithDefault already uses in src/platform.ts for the same class of bug. The two integers were already int()-cast and stay interpolated. Reproduced and verified on Windows 11 / pwsh 7.6.6: before the change U+2019 fails while ASCII ' succeeds; after it, all of plain, ASCII ', U+2018, U+2019, U+201B, spaces, $, backtick, brackets, semicolon, ampersand and CJK path segments resize correctly. tests/image-resize.test.py gains a native Windows regression that resizes a real frame under an O’Brien directory, and the existing mock case now asserts the path is absent from script source and present in the child environment. Both fail against the previous implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ails windows-dispatcher-live.test.ps1 isolates itself by setting SUTANDO_TEST_MODE=1 and SUTANDO_WORKSPACE to a temp directory. That override is honored only inside src/sutando_config.resolve_workspace, so Resolve-SutandoWorkspace reaches it only when the Python probe works. If python is not resolvable on the PATH the test builds, the resolver silently returns <repo>/workspace instead, and the test drives the real dispatcher against the developer's live workspace: it creates workspace scaffolding in the checkout, answers whatever real tasks are queued with the fake claude shim, and then fails on a timeout with that dispatcher still running, because $dispatcherPid was never learned so the cleanup tree-kill never fires. Confirmed on Windows 11: with python hidden from PATH the resolver returns C:\...\repo\workspace while SUTANDO_WORKSPACE points at TEMP, and a dispatcher started that way logs workspace=<repo>\workspace. The test now resolves the workspace through the production resolver and refuses to start the dispatcher unless it matches the intended temp path. Cleanup was already guarded against deleting outside TEMP; this guards the startup side, before any process is spawned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
REVIEW.md caps code comment blocks at 2 lines; the explanation of the quote-delimiter class belongs in the PR body, which carries it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…upted notice An empty prompt threw out of Process-Task, so the drain's recovery path published 'This task was interrupted. Its actions may have completed…' for a task that never ran. The dispatcher now publishes a specific outcome, archives the claim and keeps serving. The native live suite gains two cases: an empty body, and an archive blocked by a scanner-style open handle (FileShare.Read, no delete share). The shim now counts invocations so the suite proves neither the stuck claim nor the interrupted claim is replayed (7 calls for 7 turns). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
describe_screen, point_at and the recording describer shelled out to sips unconditionally, so Windows sent full-size frames to the model and point_at wrote its temp file under /tmp. resizeImage() in platform.ts keeps the sips argv contract on macOS and uses System.Drawing through PowerShell on Windows, with both paths passed in the environment so a typographic apostrophe in a profile name cannot break the script. Tests: mocked Windows and macOS contracts, the failure fallback, and a native Windows case that paints a 1200x300 PNG and checks the 400x100 JPEG output. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The tool table was static, so on Windows the model was shown press_key, type_text, volume, brightness, toggle_tasks, slide_control and fullscreen and planned around them until the first macOSOnlyError. forHostPlatform() drops those seven from inlineTools and ownerOnlyTools off macOS; switch_app stays because it has a Windows branch. macOS keeps the identical table and order. The seven join envDependentToolNames, so the behaviour-anchor fixture is host independent again; the regenerated fixture only removes their entries, the instruction hashes are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…prove sh sourcing workspace_default.ps1 now wraps both [Console]::OutputEncoding assignments in try/catch like task-dispatcher.ps1 does, so a host without a console cannot throw out of the resolver. docs/subagent-delegation.md returns to main's text (the only change was a removed blank line). The sh/dash no-candidates case in python-binary-sh.test.sh now echoes a sentinel after sourcing, so it fails against a file that cannot be sourced under POSIX sh instead of passing on empty output. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@cla-assistant check |
|
Native Windows pass at head Host. Windows 11 Home 10.0.26200 · pwsh 7.6.6 · Python 3.12.10 ( What landedCherry-picked from thinkall#6 with authorship preserved (the anchor commit there is superseded by New, one concern per commit:
N5 (probe budget): measured here, Before/afterSuites run natively at c6d49bf
Live round trip on this install (real
|
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Re-review at head c6d49bf7, run natively on Windows 11 (details and raw output in my comment above, #1789 (comment)).
Decision: APPROVE. This replaces my CHANGES_REQUESTED at 5aa03212.
Status of the "still needed" list from that review:
| # | Item | Status at c6d49bf |
|---|---|---|
| 1 | N1: dispatcher survives a failed archive | Fixed in d1ea3de9; reproduced natively with a FileShare.Read handle on the claim (new case in windows-dispatcher-live.test.ps1): result published once, claim left in place, next task served, restart survives, claim archived after release, 7 claude calls for 7 turns. |
| 2 | N2: restart must not kill its own caller | Fixed in 06526e91; windows-restart.test.ps1 passes natively ("4 stop plans and the detached relaunch"). A production restart.ps1 -StopOnly run from inside a Claude Code session on this host stopped the dispatcher and services and left the calling session alive. |
| 3 | start-cli.ps1 quoting |
Fixed in f23718d5; windows-startup.test.ps1 passes natively (real argv round trips, 2 start-cli launches, 5 install cases). |
| 4 | Live Windows round trip at the final head | Done at c6d49bf7 with the real claude.ps1: owner turn (OWNER_LIVE_OK), production stop + startup.ps1, post-restart turn on the same channel resumed session 760ed043 and recalled the earlier token (POSTRESTART_OK). Guest task routed to the codex read-only path and returned the fail-closed sentinel; Codex is not authenticated on this host and the owner has chosen not to set it up here, so the sandbox reply itself is not exercised on Windows. Bridge delivery and a verified-collaborator envelope were not run (no bot token on this host). |
| 5 | liususan091219's CHANGES_REQUESTED at d8c33719 |
Her asks are met in the tree: src/platform.ts, src/sutando_platform.py, src/startup.ps1, src/task-dispatcher.ps1 exist; the PR diff against main adds no ~/.sutando/ literal (the lint check passes). Still needs her re-review or a maintainer dismissal; I am not dismissing it in this review. |
| 6 | Independent second approval | This one. Note: like the 09-15 fixes, the 63f7cb1b..c6d49bf7 commits were pushed from this account as maintainer edits, so treat this approval with the same caveat john-the-dev gave his. |
Non-blocking notes from that review: N3, N4, N6 and the workspace_default.ps1 / doc-hunk / sh-sentinel items are fixed at this head; N5 is measured at ~1 s per probe here and left alone. elCaptnCode's tool-surface finding is fixed (91c3d8e2).
CI at c6d49bf7: see the check list on this PR; I approved the fork-held runs and re-checked them before recording this review.
Merge still requires liususan091219's change request to be cleared and the code-owner rule to be satisfied.
🤖 Generated with Claude Code
Dismissed at Rui’s explicit request. Susan withdrew this finding in review 5059865086; Bassil reconfirmed it resolved and approved current head c6d49bf in review 5226924662. The platform abstraction and PowerShell files are present.
…loads on Windows The Windows CI job (added by #1789, after this branch forked) failed the whole suite at import time: `import fcntl` at module scope has no Windows equivalent, so every test file that imports notify_reviewers.py crashed before running a single case. Guarded the import (try/except ModuleNotFoundError -> msvcrt) and gave _ledger_lock a lock_fd/unlock_fd pair that branches on which primitive is available -- same shape as src/file_lock.py's cross-platform helper, which landed on main via #1789 after this branch forked; duplicated here rather than rebasing 60+ commits to reach it, with a comment pointing at the fold. Verified: all 19 notify-reviewers/sci-notify-reviewers test files pass (rc=0 each), including notify-reviewers-routing-unit.test.py and notify-reviewers-park-claim.test.py which exercise _ledger_lock directly -- the POSIX path is unchanged behaviorally (still plain fcntl.flock under the same call sites). ruff clean, no unused imports. Cannot execute the msvcrt branch on this host; the Windows CI job on this push is the real witness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What changed, and why?
Adds native Windows support for Sutando's voice, screen capture, clipboard, notifications, task dispatch, dashboard/API, and messaging infrastructure, with PowerShell startup and shutdown scripts. AppleScript automation and the Swift menu-bar app remain macOS-only.
The September 15 follow-up fixes the review blockers on parent
722c77c3: POSIX Python resolver parsing, Windows file-opening command interpretation, notification owner routing, startup argument quoting and dependency installation, Unicode workspace/clipboard handling, abandoned dispatcher claims, and Windows image resizing. Process-probe failures now report unknown status instead of claiming the service is stopped. The unrelated subagent-policy additions were removed.Review first
src/platform.ts,src/sutando_platform.py: OS boundaries, literal file opening, image resizing and process probes.src/startup.ps1,src/workspace_default.ps1,scripts/python-binary.{sh,ps1}: clean install, interpreter selection and launch arguments.src/task-dispatcher.ps1,src/restart.ps1: authorization, instance ownership, interruption recovery and child cleanup.src/notify.ps1: delegation to the existingdm-result.pyowner-resolution and send policy..github/workflows/windows-runtime.yml.Authorization and task lifecycle
Owner tasks run through Claude. Discord collaborators require a verified envelope and a current access entry; their tasks also run through Claude. Other non-owner tasks use
codex exec --sandbox read-only, or are refused when Codex is unavailable. Owner and collaborator turns in the same channel share a resumed session. Producer-side task-body confinement and collaborator envelope verification remain essential boundaries.The dispatcher holds an exclusive lifetime lock, publishes results atomically, and archives interrupted claims with an explicit interruption result. It never automatically replays them: actions may already have completed or may still be running after a crash. Restart stops the process tree. Recovery preserves an existing result, but does not promise exactly-once delivery if a result was consumed before the archive step.
Before/after evidence
Parent:
722c77c3. Runtime fix:fb07a5f8; current head511a2068also fixes Windows newline conversion in the notification test fixture, reports all Windows regression failures, uses a Python 3.9-compatible watcher annotation, ports pending-question locks, and shares the existing bounded Windows atomic-replace retry policy between pending questions and process pins. Local host: macOS arm64, Node 24.4.1 and temporary PowerShell 7.6.6. Tests use isolated workspaces and deterministic CLI/delivery shims.The dispatcher test starts the actual PowerShell dispatcher and FileSystemWatcher, checks instance contention, abandoned-claim archival without replay and existing-result preservation, force-kills and restarts it, then receives
PORTABLE_OWNER_OKfor a fresh task. This is production-path integration with a deterministic inference shim, not a live Windows account-delivery claim.The workspace encoding test fails on the parent by falling back to the default workspace under cp1252/ASCII; the fix returns the configured Unicode path and restores the caller's encoding settings.
Validation
Local passing checks:
Final-head CI (
511a2068):Native CI exposed two additional pending-question portability defects: POSIX-only
fcntlimports and reader/writer sharing violations during replacement. Dismissal now uses the shared cross-platform sidecar lock; Windows readers join that lock, and atomic publication shares the existing bounded Windows replacement retry policy with process pins. The 54 dismissal tests pass natively, including concurrent reads, cross-process writers, bounded retries and failure cleanup. Local process-pin atomicity tests also pass 16/16.Documentation, deployment and remaining review gates
README and canonical
docs/built-in-tools.mddescribe the current Windows behavior. No data migration is introduced. Restart Windows services when deploying; interrupted tasks require checking their outcome before resubmission.