From 8be2f09aa4b658748655386cb2221363ed3b9aff Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:10:10 -0400 Subject: [PATCH] fix(planning): kill the server by native pid in the watch clean-stop test on Windows Case (o) of watch.test.sh stopped the server with Git Bash `kill`, which does not know the native Windows pid the server records, so the server kept answering and the case failed on Windows Git Bash while watch.sh itself was correct. The case now uses `taskkill //F //PID` on MINGW*, MSYS* and CYGWIN* and `kill` elsewhere. watch.test.sh passes 27 of 27 on Linux and on Windows Git Bash, and the surface README states the verified Windows behavior. Closes #5723 Co-Authored-By: Claude Opus 5.5 --- plugins/planning/.claude-plugin/plugin.json | 2 +- plugins/planning/CHANGELOG.md | 6 ++++++ plugins/planning/surface/README.md | 2 +- plugins/planning/surface/watch.test.sh | 6 +++++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/plugins/planning/.claude-plugin/plugin.json b/plugins/planning/.claude-plugin/plugin.json index c41a4d36ae..513eb4e974 100644 --- a/plugins/planning/.claude-plugin/plugin.json +++ b/plugins/planning/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "planning", - "version": "0.58.3", + "version": "0.58.4", "userConfig": { "surface": { "type": "string", diff --git a/plugins/planning/CHANGELOG.md b/plugins/planning/CHANGELOG.md index ed3833750c..9c427af0e1 100644 --- a/plugins/planning/CHANGELOG.md +++ b/plugins/planning/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to the `planning` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.58.4] - 2026-10-01 + +### Fixed + +- **The `watch.test.sh` clean-stop case kills the server by native pid on Windows Git Bash.** `kill` does not know a native Windows pid, so the server kept answering and the case failed while `watch.sh` was correct; the case now uses `taskkill` on `MINGW*`, `MSYS*` and `CYGWIN*` and `kill` elsewhere. The suite passes 27 of 27 on Windows Git Bash, and after `round.sh stop` the watcher exits 3 with the stop message and no `watch.sh` or curl long-poll is left, which the surface README now states ([#5723](https://github.com/melodic-software/claude-code-plugins/issues/5723)). + ## [0.58.3] - 2026-10-01 ### Fixed diff --git a/plugins/planning/surface/README.md b/plugins/planning/surface/README.md index 21fde86dd5..4ee0972c38 100644 --- a/plugins/planning/surface/README.md +++ b/plugins/planning/surface/README.md @@ -26,7 +26,7 @@ bash round.sh --dir '' ensure-running [--port P] [--open] [--user-sett bash round.sh --dir '' stop ``` -`ensure-running` checks for curl, reuses the server already running for the data dir (same PID), and otherwise starts one in the background (on Windows through the base interpreter, with no console window) on the first free port of `--port`, the recorded port, and the resolved `port` setting (an explicit `--port 0` skips the setting), else a free port. It waits for the server's own session files, prints the URL, and with `--open` opens the page unless the resolved `openBrowser` is `false`, through the `browserCommand` of the `--user-settings` file this same call passes, else the default browser. Without `--user-settings`, the user file the running server recorded still supplies `openBrowser` and `waitTimeout`, but never the opener. The URL is always built from the recorded port as `http://127.0.0.1:/`. `--emoji-markers` takes any value: only `true`, `1`, `yes` and `on` (any case) mean true, and anything else, an empty string or an unexpanded `user_config` token included, means false. The value is written to `meta.emojiMarkers`; an absent flag keeps the recorded value, and a new file records false, so pass the session's value each time, including on a restart. `stop` ends the recorded PID only after `/api/ping` on the recorded port answers with that PID; otherwise it only clears the session files. Before it ends the server, `stop` posts a `finish` (`by: "stop"`, no Brief path) when the skill posted none, and waits one second so open tabs receive it; the tab then reads the stop as a finished interview, not a lost connection. Each `watch.sh` poll sends its process id (`&pid=`), which the lease records and `/api/state` shows; after the finish, `stop` sends that PID SIGTERM when its command line is a `watch.sh` for this data dir, so no watcher for it is left running (a watcher on an older poll gets a refused connection once the env file is gone and exits 3 within one 5 s retry). It signals nothing on Windows, where Git Bash's `$$` is not a native PID; a watcher there ends through the refused-poll exit. That path has not been exercised on Windows Git Bash. `stop` removes the pid, token and nonce but leaves the port in `.interview-session.json`, so the next `ensure-running` on the data dir reuses that port when it is free, and open tabs and their per-origin browser settings carry over. `ensure-running` removes a `finished` left by an earlier stop. A restart issues a new token: an armed watcher exits 2 at once with "token changed: re-run ensure-running", so re-arm it. +`ensure-running` checks for curl, reuses the server already running for the data dir (same PID), and otherwise starts one in the background (on Windows through the base interpreter, with no console window) on the first free port of `--port`, the recorded port, and the resolved `port` setting (an explicit `--port 0` skips the setting), else a free port. It waits for the server's own session files, prints the URL, and with `--open` opens the page unless the resolved `openBrowser` is `false`, through the `browserCommand` of the `--user-settings` file this same call passes, else the default browser. Without `--user-settings`, the user file the running server recorded still supplies `openBrowser` and `waitTimeout`, but never the opener. The URL is always built from the recorded port as `http://127.0.0.1:/`. `--emoji-markers` takes any value: only `true`, `1`, `yes` and `on` (any case) mean true, and anything else, an empty string or an unexpanded `user_config` token included, means false. The value is written to `meta.emojiMarkers`; an absent flag keeps the recorded value, and a new file records false, so pass the session's value each time, including on a restart. `stop` ends the recorded PID only after `/api/ping` on the recorded port answers with that PID; otherwise it only clears the session files. Before it ends the server, `stop` posts a `finish` (`by: "stop"`, no Brief path) when the skill posted none, and waits one second so open tabs receive it; the tab then reads the stop as a finished interview, not a lost connection. Each `watch.sh` poll sends its process id (`&pid=`), which the lease records and `/api/state` shows; after the finish, `stop` sends that PID SIGTERM when its command line is a `watch.sh` for this data dir, so no watcher for it is left running (a watcher on an older poll gets a refused connection once the env file is gone and exits 3 within one 5 s retry). It signals nothing on Windows, where Git Bash's `$$` is not a native PID; a watcher there ends through the refused-poll exit. On Windows Git Bash that exit ends the watcher: it exits 3 with the stop message, and no `watch.sh` or curl long-poll is left within 10 s of `stop`. `stop` removes the pid, token and nonce but leaves the port in `.interview-session.json`, so the next `ensure-running` on the data dir reuses that port when it is free, and open tabs and their per-origin browser settings carry over. `ensure-running` removes a `finished` left by an earlier stop. A restart issues a new token: an armed watcher exits 2 at once with "token changed: re-run ensure-running", so re-arm it. ## Watcher protocol diff --git a/plugins/planning/surface/watch.test.sh b/plugins/planning/surface/watch.test.sh index 16833b1308..853bffb9a0 100755 --- a/plugins/planning/surface/watch.test.sh +++ b/plugins/planning/surface/watch.test.sh @@ -446,7 +446,11 @@ if bash "$here/round.sh" --dir "$fast" ensure-running --port 0 >/dev/null 2>&1; until_waiting "$(sed -n 's/^PORT=//p' "$fast/.interview-session.env" | tr -d '\r')" spid=$(sed -n 's/.*"pid": *\([0-9]*\).*/\1/p' "$fast/.interview-session.json") rm -f "$fast/.interview-session.env" - kill "$spid" 2>/dev/null + # On Windows $spid is a native pid, which Git Bash's kill does not know. + case "$(uname -s)" in + MINGW* | MSYS* | CYGWIN*) taskkill //F //PID "$spid" >/dev/null 2>&1 ;; + *) kill "$spid" 2>/dev/null ;; + esac end=$((SECONDS + 10)) while kill -0 "$wpid" 2>/dev/null && [[ "$SECONDS" -lt "$end" ]]; do sleep 0.1; done if kill -0 "$wpid" 2>/dev/null; then