Merge agmsg v1.1.12 and fix Windows Codex monitor PID checks - #36
Merged
Conversation
…ne (fujibee#446) * feat(app): render terminals via WebGL, attached only to the active pane * fix(app): dispose partially-initialized WebGL addon on loadAddon failure
* feat(app): drag files onto a pane to insert their path koit bug report: dragging a file (e.g. an image) onto the app handed off to the webview's default drop navigation, replacing the window's content and getting the app stuck. Fix + feature are the same change: tauri.conf.json's dragDropEnabled now routes external OS file drops through Tauri's native onDragDropEvent (real absolute paths, no webview navigation fallback left to trigger) instead of the browser's own drop handling. Dropping onto a pane types its absolute path into that pane's terminal input (pty_write, no auto-Enter) -- the pane under the cursor if there is one, else the active tab's most recently focused pane, else its first pane. Multiple files become one space-separated line; deliberately not shell-quoted -- quoting broke Claude Code's own file-path recognition in live testing (Codex tolerated it fine, but the common case needs to work for both). dragDropEnabled and the webview's native HTML5 drag-and-drop are mutually exclusive by Tauri's own design (confirmed via upstream issues and source) -- enabling one disables the other. The app's existing pane/tab reordering (swap, directional split, move to tab/new-tab, cross-window) was built entirely on HTML5 draggable/dragstart/dragover/drop, so it's rewritten here on pointer events (pointerdown/pointermove/pointerup) instead, matching the existing divider-drag implementation's own mousedown/mousemove/mouseup pattern. The click-to-arm/click-to-swap path is untouched: a pointerdown that never crosses the drag threshold does nothing, and the native click event fires normally afterward. TerminalPane gains an onFocusPane callback (xterm's internal textarea receiving DOM focus) so the app can track which pane was last actually used, for the drop-missed-every-pane-cell fallback. * fix(app): reject control chars in dropped paths, harden pointer-drag lifecycle Two P1s from co1 review (PR fujibee#481): 1. joinDroppedPaths wrote dropped paths straight into the PTY with no validation. A crafted filename containing a newline would submit whatever's on the target prompt line the instant it's dropped; ESC-prefixed bytes are terminal control sequences, not text. hasUnsafeDropPath rejects the WHOLE drop (writes nothing) if any path contains a C0 control character or DEL -- stripping the bad byte instead would silently turn it into a different, wrong path. 2. startPaneDrag only listened for pointermove/pointerup/Escape. Hardened: pointerId is captured and checked on every event (a second pointer must never drive or finish someone else's gesture); setPointerCapture on the source button (released on cleanup) so a pointer released outside the OS window still delivers pointerup instead of leaking a stuck "ghost" drag; pointercancel and window blur both force a cancel, same as Escape; a real drag that ends back over the source button still fires a native click there same as any other same-element press/release pair, which would otherwise also run onClick's swap-arm toggle right after finish() already handled it as a drag -- a one-shot capture-phase click listener consumes exactly that one click. Component-unmount cleanup added too, via a ref finish() registers itself on (the document/window listeners live outside React's own teardown). * fix(app): bound the post-drag click suppression instead of an unbounded listener co1 review, PR fujibee#481, 2nd round: the previous fix added a one-shot click listener to the pane-header button whenever a real drag finished, to swallow the native click that can follow a same-element press/release. But a drag ending via blur/pointercancel/unmount with the pointer released outside the app never gets a matching click at all -- the listener would then sit on the button forever and wrongly swallow the next, wholly unrelated real click days later. Replaced with a short bounded window: dragJustFinishedAtRef records when a real drag last ended (committed or cancelled), and the button's own onClick no-ops if a drag finished recently (shouldSuppressClickAfterDrag, 300ms) rather than depending on a click ever actually arriving to consume. Also cancels any still- active prior gesture at the start of a new one, so multi-pointer input can't overwrite activePaneDragCancelRef without tearing down the first gesture's listeners. shouldSuppressClickAfterDrag is a pure time-window check, unit- testable without simulating real pointer/click sequences -- covers both co1-requested regressions (suppressed shortly after a drag, not suppressed once the window has passed or when no drag ever finished). * fix(app): scope the post-drag click suppression to the dragged pane co1 review, PR fujibee#481, 3rd round: dragJustFinishedAtRef was a bare global timestamp, so it suppressed a click on ANY pane header within the window, not just the one that was actually dragged -- clicking pane B right after finishing a drag on pane A silently ate B's swap-arm click. It also never cleared after suppressing once, so a second, genuinely separate click on the SAME pane within the same window was also wrongly swallowed. Replaced with dragJustFinishedRef holding { paneId, finishedAt } | null. shouldSuppressClickAfterDrag now takes the pane id being clicked and only returns true for a match; the pane's own onClick clears the ref (consumes it) whenever it does, so a follow-up click on that same pane isn't also suppressed. Added the two co1-requested regressions: a different pane's click is never suppressed, and consuming clears suppression for a subsequent click on the same pane.
…et/rename-team/team/api (fujibee#482) * fix(scripts): close fujibee#87-class SQLi gaps in rename/leave/reset/rename-team/team/api join.sh (PR272/fujibee#87) validated and SQL-escaped agent/team names but never spliced them into a raw JSON path; its sibling scripts did neither: - rename.sh, leave.sh, reset.sh spliced OLD_NAME/NEW_NAME/AGENT_ID/ TARGET_AGENT directly into '$.agents.<name>' path literals with no validation and no escaping — a name with a single quote could break out of the surrounding SQL statement, and one with '.', '/', '[', ']', or '"' could misroute the path to the wrong key. - reset.sh also spliced AGENT_TYPE unescaped into a SQL literal. - rename-team.sh set the renamed team's $.name field from an unescaped NEW_TEAM value. - api.sh's 'get teams <team> members' never validated <team> before splicing it into a filesystem path (path traversal). Fixed by wiring in the existing agmsg_validate_agent_name/ agmsg_validate_team_name gate at every entry point, and switching every $.agents.<name> lookup to concatenate an escaped SQL string literal ('$.agents.' || '<escaped>') rather than splicing the raw name into the path text. That surfaced a deeper, pre-existing bug the fix would otherwise have silently inherited: rename.sh/leave.sh/reset.sh/rename-team.sh/team.sh all read a team's config via '.param set :json '<escaped>'' — but the sqlite3 shell's dot-command tokenizer does not honour SQL '' escaping (confirmed directly: '.param set :x '\'a''b\''' errors instead of binding a''b), so this call silently mis-parsed for any config that already contained a single quote (e.g. one existing agent already named with a quote), corrupting every query built on top of it. team.sh's case was worst: it printed '.param'\''s own usage text as fake member rows with exit 0. Fixed every such call site to splice the (already-escaped) JSON blob as a genuine SQL string literal instead of binding it via '.param set'. * test: cover the fujibee#87-class SQLi/path-hazard fix across rename/leave/reset/rename-team/team/api
* fix: shell-quote delivery hook paths and harden dispatch.sh identity resolution delivery.sh spliced $project/$type into SessionStart/SessionEnd/Stop hook commands with a naive '$var' wrap; a project path containing a single quote broke the argument boundary and let the rest run as shell syntax on the next hook event. Add _agmsg_shq (proper '...' escaping with embedded quotes doubled via '\'') and use it for every hook argument. windows/dispatch.sh's kv_get() word-split whoami.sh's output and returned the first "key=value" match. Since the only attacker-choosable field (a registered agent name) always sorts first in that line, a name containing a space and a fake "teams=" token could shadow the real trailing team field and silently misdirect a caller's inbox/send/history to an attacker-controlled team. kv_get now keeps the last match, matching the rightmost-wins behavior check-inbox.sh's own parser already relies on. * revert: drop insufficient kv_get last-match fix for F19 co1's PR fujibee#487 review caught that this doesn't close the finding: whoami.sh's single-match line is "agent=<n> teams=<t> type=<ty> project=<p>", and project (like agent name) is attacker-influenceable and sorts LAST. A project path crafted to contain a trailing "teams=<x>" token defeats last-match the same way the original bug defeated first-match — the line's ambiguity isn't resolvable by picking a match order, since both ends can carry attacker content. A real fix needs whoami.sh to emit a delimiter-safe format and every consumer (check-inbox.sh, identities.sh, rename-team.sh, reset.sh, dispatch.sh) to be updated to match — an interface change, not a contained parsing fix. Reverting to keep this PR scoped to the F14 fix, which stands on its own.
…test (fujibee#440) Issue fujibee#124: a single kill -0 check right after the pidfile flips to the successor's pid races the predecessor's TERM trap under a loaded runner — the pidfile write and the actual process death are not atomic. The equivalent check in test_watch.bats already polls for exit instead of checking once; apply the same bounded-poll pattern here.
…ecords (fujibee#473) * fix(spawn): explicit bash invocation for psmux on Windows (fujibee#335) psmux (Windows tmux-compatible multiplexer) hands the split-window / new-window command token to CreateProcess/ShellExecute. An extensionless boot script has no file association, so Windows shows an "Open with" dialog instead of executing it — the agent never starts. PR fujibee#329 gated the .command rename to Darwin (fujibee#282) but assumed tmux would honor the shebang ("runs it via its shebang (tmux)"). This holds for Unix tmux but not psmux: the root cause was left unaddressed and the symptom changed from Notepad to the "Open with" dialog. Prefix `bash -l` on Windows (MINGW/MSYS/CYGWIN) in launch_in_tmux(), matching launch_windows_terminal's existing `wt.exe new-tab bash -l` pattern. Applied to both new-window and split-window branches. macOS/Linux unchanged. Tested on psmux v3.3.4 + Windows 11: - codex spawn (default-shell=bash): OK - codex spawn (default-shell=pwsh): OK - claude-code spawn (default-shell=bash): OK Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revert "fix(spawn): explicit bash invocation for psmux on Windows (fujibee#335)" This reverts commit 333a359. * fix(codex): make record-session project arg optional, reject poison values On Windows, codex's shell_command runs through PowerShell, so a "$(pwd)"/"$PWD" argument in the template prompt is expanded — or mangled — by PowerShell BEFORE bash sees it. The model's quoting choice produces three non-deterministic variants: V1 bash -lc "... \"$PWD\"" → project=\ (poison: launcher MISMATCH → silent delivery stop) V2 "$PWD" / "$(pwd)" → C:\... (Windows form, fragile but works) V3 bash -lc '...' → /c/... (correct MSYS form) Fix: make <project> optional (defaults to the script's own $PWD, which is deterministically correct under bash regardless of the caller's shell), canonicalize via agmsg_canonical_path before recording, and reject non-existent paths and filesystem/drive roots as poison. Also canonicalize the project in actas-claim.sh so role-session records carry one path form across agent types, and add a Shell-requirement warning to the codex template about the \"$VAR\" quoting hazard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDU62pkpovxcukJbvbgDPG --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ee#492) Two independent defects were making `bats (macos-latest)` and `bats (windows-latest, install helpers)` — both REQUIRED checks — fail on pull requests that changed nothing relevant to them. 1. The macOS bats timeout was stale. `timeout-minutes: 15` was set when the suite ran in about 5 minutes. The suite is now 728 tests and macOS measures 11m57s-14m10s, so only 1-3 minutes of headroom remained. Nine of nineteen sampled runs were killed at ~15m18s, each at whatever test it happened to be executing (the stop point varied between tests 528 and 640, which is what distinguishes a timeout from a hang). Raised to 25, matching the app-test-windows job. 2. The Windows sqlite3 install was unverified. `choco install sqlite` occasionally fails when the chocolatey community feed returns 504, logging "Chocolatey installed 0/0 packages". The verification step ended in `|| echo "sqlite3 not found on PATH"`, so it exited zero and the job continued, failing about three minutes later inside the install tests with "sqlite3 is required but not found" — a message that reads like a defect in the pull request under test. The install now retries, and the verification step fails immediately when sqlite3 is genuinely absent. No test or product behaviour changes.
… focus (fujibee#470) launch_macos_terminal() used a plain `open -a Terminal "$BOOT"` / `open -a iTerm "$BOOT"`, which activates the launched app and steals keyboard focus from whatever the caller was doing. This path runs whenever $TMUX is unset at spawn time -- notably when the spawning process itself has no tmux context (e.g. a GUI app), where a foreground terminal popup interrupts the user mid-interaction. `-g`/`--background` keeps the window opening without bringing it to the front; spawn's behavior (boot script execution, readiness handling) is otherwise unaffected. Adds a regression test: the existing suite bypasses launch_macos_terminal() entirely via the AGMSG_TERMINAL={cmd} stub template, so there was no coverage of the real `open` invocation before this. The new test stubs `open` directly (unsetting AGMSG_TERMINAL and TERM_PROGRAM for determinism) and asserts `-g` appears in both the Terminal.app and iTerm code paths.
…subscriptions (fujibee#477) * fix(watch): fail loudly on shifted args instead of running with zero subscriptions A grok-build monitor watcher launched from the rule template could run with zero subscriptions, silently: the template baked watch.sh "$GROK_SESSION_ID" into the command line, and grok's monitor tool re-evaluates that line in a shell where the variable is unset — the quoted-but-empty expansion is dropped as a word, shifting every later argument one slot left. watch.sh then parsed the project path as the session id and an agent name as the type, identities.sh resolved no pairs, and the watcher polled forever delivering nothing. Two minimal layers: - Caller side: the grok-build templates now pass "${GROK_SESSION_ID:--}". When unset this yields the sentinel '-', which survives re-evaluation as a real argument; watch.sh folds '-' into the existing empty-arg resolution path (fujibee#236 behavior preserved). - watch.sh defense: validate agent_type against the registered types and fail loudly (one line on stdout, fujibee#197 pattern) when unknown, naming the shifted-args cause. A built-in type is confirmed by a single type.conf stat to keep the launch hot path free; non-builtin names fall back to the type registry so trusted plugin types still validate. Related to fujibee#475. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(watch): surface missing-arg and path-like type failures on stdout Review follow-ups: a shifted three-argument launch leaves only two args, and the old ${3:?} guard died on stderr — invisible to a monitor tool consuming stdout. Replace it with an explicit check that fails on stdout and names the shifted-args cause. Also reject '/' and '..' in the type slot outright instead of letting the registry fallback concatenate a path-like value into a manifest path. Related to fujibee#475. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…fujibee#447) Under Claude Code's command sandbox, `kill -0 <pid>` on a live process outside the current session returns EPERM ("Operation not permitted"), not ESRCH ("No such process"). The liveness checks treated any non-zero `kill -0` exit as dead, so a live parent/sibling pid was misread as dead, causing the monitor watcher to self-exit (silent outage) and the SessionStart/SessionEnd GC to kill live watchers / delete live markers. - _agmsg_pid_alive now inspects kill -0 stderr and treats only ESRCH as dead; every other failure (EPERM included) is alive. - Route the destructive / dedup liveness gates through it: session-start, session-end, check-inbox, watch.sh prev-watcher dedup, resolve-project marker GC (+ guard when the helper isn't loaded), and the emit_monitor_directive dedup in delivery.sh / grok-build. - Leave "wait until a pid dies" loops and non-destructive checks as raw kill -0 by design. Tests: _agmsg_pid_alive ESRCH/EPERM/unknown cases and marker-GC keep/drop/guard. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… it (fujibee#494) `agmsg_sqlite` called `_agmsg_escape_flag` inside a command substitution. `$(...)` runs in a subshell, so the `_AGMSG_ESCAPE_PROBED` / `_AGMSG_ESCAPE_FLAG` assignments were discarded when that subshell exited and the probe `sqlite3 -escape off :memory: "SELECT 1;"` ran again on the next call. Every database access therefore spawned two sqlite3 processes instead of one. The function's own comment says "probe once", so this was unintended. Call the probe in the current shell and expand the flag as a variable. The memo now survives, and because subshells inherit shell variables, a later `$(agmsg_sqlite ...)` reuses it rather than re-probing. Measured with a counting shim, five queries in one shell: before: 10 sqlite3 invocations after: 6 sqlite3 invocations (five queries plus one probe) and, once the memo is primed, five queries made inside command substitutions drop from 10 invocations to 5. The memo is per shell, not per machine: a process whose first database access happens inside a command substitution still probes in that subshell, so a one-shot script making a single query pays the same two invocations as before. Removing that would need the result cached across processes, which is a separate change. `-escape off` behaviour is unchanged — `SELECT 'x'||char(31)||'y'` is byte-identical before and after (checked with od -c), so the fujibee#102 / fujibee#143 fix is not regressed. Reported with the mechanism, a reproduction harness and the invocation counts in fujibee#462. Closes fujibee#462
…t replay it (fujibee#439) `inbox.sh` and `check-inbox.sh` both mark delivered rows' `read_at` before displaying them, but `watch.sh` — the live Monitor-mode delivery path — never touched `read_at` at all. A message delivered live through a watcher stayed `read_at IS NULL` forever, so a later `inbox.sh` / `check-inbox.sh` call, or the `history.sh` unread marker, would surface it again as if it were still unread. Adds a `mark_read()` helper called from both the normal delivery branch and the `ctrl:despawn` control-row branch, so control messages handled by the watcher also stop lingering as unread. The helper is scoped to the definitive receiver for a role. A broad watcher (no `actas` name) subscribes to every registered role in the project, so marking unconditionally would let a leader's default watcher write read state for a role that has its own exclusive watcher. It skips when an exclusive ready sentinel exists for that (team, agent). This is a best-effort mark on local write success, not a delivery acknowledgement; a stronger guarantee needs the claim/ack redesign tracked in issue fujibee#373. The same defect was independently diagnosed and fixed in PR fujibee#486 by @u-ichi, who reached the same root cause — that the watermark stops a watcher re-streaming a row but never touches `read_at`. That PR is closed in favour of this one, which additionally guards the broad-watcher case, keeps the mark idempotent, and degrades rather than exiting when the database is unavailable. Issue fujibee#373, which fujibee#486's author filed, is where the durable fix belongs. Closes fujibee#439
…d ceiling (fujibee#443) * fix(codex-bridge): make the turn watchdog an idle timeout, not a fixed ceiling startTurnWatchdog() armed a single fixed-duration timer from turn start (default 60s) and never touched it again until the turn ended. A turn that was genuinely still working — actively reasoning/tool-calling for longer than that, e.g. diagnosing a multi-step failure before replying — got cut off by this timer before it ever reached its own send.sh call. onTurnEnded() then ran exactly as it does on a real completion, so the bridge silently re-armed as if the turn had ended with nothing to report; whatever the turn was about to send never went out. item/agentMessage/delta notifications already tell the bridge the turn is producing output. onAgentMessageDelta now re-arms the same watchdog on every chunk, turning it into a true idle timeout: only turnTimeout seconds of silence trips it, regardless of how long the turn has been running in total. This preserves the watchdog's actual purpose (rescue a turn that will never send turn/completed — the app-server does not reliably send it, see fujibee#41) — a turn that is visibly still working is not that case. Observed in production: a Codex bridge turn spent >60s diagnosing a GitHub auth/DNS failure (correctly identifying an invalid gh CLI token and a network-restricted sandbox) but the watchdog fired before it reached send.sh, so the diagnosis was silently lost and the requesting peer saw only silence. * fix(codex-bridge): re-arm the idle watchdog on ANY thread activity, not just message deltas Review finding (self-multi-model, Codex, P1): the previous commit only re-armed the watchdog from item/agentMessage/delta, but a Codex turn spends most of its time in other notification types the bridge has no handler for — reasoning deltas, tool-call/command-output progress, etc. Those still went through handleLine() but were silently dropped before dispatch (no handler registered), so the watchdog kept counting from turn start and could still fire mid-turn during exactly the kind of multi-step work this fix is meant to protect. Added a generic AppServerClient/WebSocketAppServerClient.onThreadActivity hook that fires for every thread-scoped notification/request in handleLine(), regardless of whether a specific handler is registered for its method. CodexBridge wires it to re-arm the watchdog for any activity on its own active turn. onAgentMessageDelta's specific re-arm is now redundant (the generic hook already covers it) and has been removed. New test proves the fix covers non-message activity: a turn that only emits item/reasoning/textDelta and item/commandExecution/outputDelta (never agentMessage/delta) still survives past turn-timeout without being cut off.
* feat(spawn): add herdr placement support for spawn/despawn/watch Add herdr pane management as a placement target alongside tmux and OS terminals, so agents can be spawned into herdr panes when running inside a herdr environment (HERDR_ENV=1, HERDR_PANE_ID set, herdr binary on PATH). - spawn.sh: new launch_in_herdr() using `herdr pane split` (default) or `herdr tab create` (--window), with herdr: scheme-tagged placement records; priority order is $TMUX → herdr → OS terminal for backward compat with tmux-inside-herdr setups - despawn.sh: kill_recorded_placement() handles herdr:* ids via `herdr pane close` - watch.sh: ctrl:despawn handler closes own herdr pane when HERDR_PANE_ID is set - tests: unset HERDR_ENV/HERDR_PANE_ID in all test setups to prevent accidental real-herdr calls from the test runner; add fake-herdr stub tests for spawn split/window/fallback, despawn --force, and watch ctrl:despawn Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(spawn): silence herdr rename stdout leak and ensure run/ dir for placement record - herdr pane rename: redirect stdout+stderr to /dev/null so its JSON response does not pollute spawn's output - mkdir -p the run/ directory before writing the herdr placement record, preventing 'No such file or directory' on fresh installs where run/ does not yet exist Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(watch): use a registered sender in the herdr ctrl:despawn test Rebase-induced fix, not part of the original contribution. The new test sent ctrl:despawn from 'boss', the sender convention in test_despawn.bats, but test_watch.bats' setup registers only alice and bob. On current main send.sh rejects an unregistered sender, so the test failed before the watcher ever saw the message. Switch the sender to bob, matching every other send in this suite. The assertion is unchanged: the watcher must call herdr pane close with its own HERDR_PANE_ID. * fix(spawn): address herdr pane ids by JSON path, not by text position herdr replies with structured JSON, but the pane id was pulled out with sed. Two shapes break that: - The split reader matched the LAST "pane_id" in the whole response. A reply carrying a second pane object after the target one therefore parsed successfully and returned somebody else's pane. spawn would go on to rename it, run the boot script inside it, and persist that id as the placement record, so a later despawn would close a pane the user had never spawned. Silent, and wrong in both directions. - The tab-create reader delimited root_pane with [^}]*, so any nested field ahead of pane_id (scroll, agent_session — both real herdr fields that sort before it) ended the match early and extraction failed. Neither is a maintenance nit: JSON object key order is not a consumer contract, and 0.7.3 happening to emit a convenient order is luck. Read the value by explicit path with sqlite3's JSON1, already a core dependency here (whoami.sh, api.sh): $.result.pane.pane_id for a split and $.result.root_pane.pane_id for a tab. json_type gates on the value actually being a string, so invalid JSON, a missing path, a null, a number, and an empty string all come back empty and the caller dies instead of proceeding on a guess. The response is escaped before it reaches the SQL literal. Tests cover the two shapes that broke positional matching, a reordered pane object, and the fail-closed paths — asserting there that no placement record is written and no pane is driven. * fix(watch): only close a herdr pane agmsg placed for this role On ctrl:despawn the watcher closed a herdr pane whenever HERDR_PANE_ID was non-empty and the binary was on PATH. That is not evidence of ownership. Every descendant of a herdr pane inherits HERDR_*, so a watcher merely STARTED inside one carries the host pane's id — an agent that actas'd by hand, or a test suite run from a herdr terminal. Acting on the inherited value closes the host. It is not hypothetical: a live session was killed this way while this branch was under review. Require two things instead: - HERDR_ENV=1, matching spawn's own is_herdr_env. A stale HERDR_PANE_ID surviving in the environment of a process that is not herdr-hosted no longer qualifies. - The placement record for this (team, agent) names exactly this pane. spawn writes herdr:<pane_id> there, so ownership is checkable rather than assumed. This is the herdr counterpart of the tmux path's ACTIVE_NAME gating (fujibee#109). Anything else falls through to the existing "close this window manually" branch, which is the correct outcome for a pane agmsg did not create. The record is read BEFORE reset.sh: reset.sh releases the actas lock, and the leader's despawn deletes the record as soon as it sees that lock go free, so reading afterwards races the cleanup and would intermittently find nothing. Tests: the positive case now records a placement, and three negative cases cover no record, a record naming a different pane, and HERDR_ENV unset — each asserting no close call and the manual-close message. * test(despawn): strip the herdr environment in setup, not per launch This suite guarded HERDR_ENV/HERDR_PANE_ID on individual watch.sh launches. The read_at test added later (fujibee#439) did not carry the guard, so a watcher started there kept the real host pane id and, on ctrl:despawn, closed the developer's own pane — which is how a live session running this suite from inside herdr got killed. Per-launch guarding needs every future test to remember. Unset once in setup, the way test_spawn.bats and test_watch.bats already do, so a new test cannot reintroduce the gap by omission. --------- Co-authored-by: asayamakk <kodai.asayama@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…a framing (fujibee#497) * feat(codex): offer monitor as the default delivery mode, drop the beta framing The Codex monitor bridge has been the advanced third option behind a BETA label since it landed, with turn as the default. It has since run as the normal delivery path long enough that presenting it as experimental misdescribes it, so it now leads the choice the way it does for Claude Code. Two things change: - The first-run delivery prompt is reordered to monitor / turn / off with monitor recommended, and empty input now selects monitor instead of turn. This affects the prompt a project sees when it joins; projects that already chose a mode keep it, since nothing rewrites stored settings. - BETA/beta wording is removed from the prompt, both README files, the setup doc, and the bridge/shim/installer comments and messages. Known limitations stay exactly as they were: monitor still changes how `codex` starts, still needs a restart plus one message to take effect, and still carries the orphan-on-TUI-close (fujibee#149) and one-identity-per-project (fujibee#150) caveats. Dropping the beta label is not a claim that those are gone, so the warning block keeps them and only stops calling the feature experimental. Out of scope on purpose: no mechanism or behaviour changes beyond the prompt order and default, no README restructuring, and no rename of docs/codex-monitor-beta.md — the filename is linked from delivery.sh, spawn.sh and both READMEs, and may be linked externally, so it stays put and only its contents are reworded. grok-build's monitor keeps its BETA label; this graduates codex only. * docs(codex): drop the resolved one-identity-per-project limitation The monitor warning listed "one identity per project (fujibee#150)" alongside the orphan caveat. That limitation is gone: fujibee#150 closed as completed on 2026-07-19, fujibee#419 landed the multi-identity delivery fix, and the launcher now starts one bridge per recorded role with "Multiple identities are intentional (fujibee#150)" stated in its own comment. What codex-bridge.js still refuses is handing several identities to a single unscoped bridge, which is a per-bridge rule, not a per-project one. Preserving the known limitations while dropping the beta label was right; preserving one that no longer exists was not. On a GA notice it actively misinforms — it tells people they cannot register multiple Codex roles in a project when they can. Removed from both READMEs and from the setup doc's warning block. The orphan-on-TUI-close caveat (fujibee#149) stays: that issue is still open. Code comments referencing fujibee#150 are left alone; they describe the implementation that resolved it and are accurate.
… a core (fujibee#496) * fix(codex): stop a replacement dispatcher from doubling the role children A role child is `nohup`'d and bound to the shared app-server, while the dispatcher that spawned it runs in the TUI's process group. A pane teardown delivers SIGHUP, which the dispatcher does not trap (only INT and TERM are), so it dies without running its EXIT trap and leaves the dispatcher lock row owned by a dead pid. The next launcher reclaims that stale lock, starts with an empty known_pairs, and re-spawns the ENTIRE child set -- while the previous generation's children are still running. One extra full set of role children per dispatcher replacement, each polling forever. Give every role child its own runtime lock, keyed on (project, pair), using the existing resource-generic agmsg_runtime_lock_* primitive. A re-spawned duplicate now fails to acquire it and exits on arrival. The dispatcher lock's acquire helper is generalized to take the resource rather than duplicating the CAS-reclaim protocol; the trap releases whichever resource this process holds, so it survives the child's `exec "$0" ...` re-entry (exec keeps the pid, so the row is re-acquired by the plain insert path on the way back in). Retire a child whose own registration is gone. Losing the registration only sent it back through the re-exec, whose startup loop then spun for as long as the app-server lived -- measured still running 10 s after its role was removed. Two consecutive empty resolutions are required, because identities.sh also reports empty on a transient read failure. The startup loop is bounded for the same reason, as the backstop for a child started with no registration at all. Have the dispatcher forget a pair once it is no longer registered. That is required for the above to be safe: known_pairs would otherwise still list a pair whose child has exited, so re-registering that role would silently never get a bridge again for the rest of the app-server's life. * perf(codex): stop the bridge launcher polling loops burning a core One role-child iteration cost ~157 processes and ~410 ms against a 300 ms sleep -- the body took longer than the interval it was waiting for, so each child ran shell work roughly 58% of wall-clock time, forever. Seven roles put four cores' worth of bash in permanent flight, which is the reported load average. Three sources, all removed with builtins; no new dependency, bash 3.2 safe. identities.sh opens and parses EVERY teams/*/config.json per call -- two sqlite3 processes per team file, 57 processes and 145 ms on an eight-team install -- and the loops called it several times a second. The registrations only change when join/leave/actas rewrite a config, so the result is cached behind an mtime guard evaluated with glob expansion and `[ -nt ]` alone: a hit costs nothing. The marker is touched before the resolve and a config counts as changed unless the marker is strictly newer, because bash 3.2 compares whole seconds and a write landing inside the marker's own second must not be missed; the file count is compared too, since a removed team leaves nothing for `-nt` to see. _agmsg_role_session_path is memoized per process. It is a pure function of (SKILL_DIR, team, agent) but costs two awk processes, and the launcher re-derived it for the same few pairs on every tick. Parallel arrays, not an associative array -- macOS ships bash 3.2. _actas_lock_encode itself is left alone on purpose: it is shared with the actas lock filenames, so a reimplementation that diverged would orphan live state files. _agmsg_role_session_field was `sed -n | head -1`, two processes per field and two fields per role per tick; it is a shell read loop now, verified equivalent against the old implementation across empty values, values containing '=', records with no trailing newline, and a key that prefixes another. The `cat` reads of the request/pid/appserver/thread files and the `printf | grep -Fxq` membership test go the same way. The interval now backs off, 0.3s through 2s, and resets to fast on any observed change -- a config actually moving, a child being spawned, a bridge being launched. The startup race the tight interval exists for is unaffected because it is a change. No env override: an interval knob would be an interface addition, and nothing here needs to differ per user. Measured on the same eight-team, four-role harness, steady state over a 10 s window: 1059 spawns (105.9/s) before, 28 (2.8/s) after; sqlite3 288 -> 10. * test(codex): count launcher children by ppid instead of command line The fujibee#485 tests read the child count straight out of `ps`, which is wrong in two ways that only showed up on the Linux CI leg. Every command substitution the launcher runs forks a subshell, and a forked subshell inherits its parent's argv -- so those subshells are identical to a real role child by command line alone. Sampling during one reads 3 children where there is 1. Count only processes whose parent is not itself a match, which is the property the tests are about. Both assertions also sampled on the first sighting rather than on the settled state. Spawn and exit are asynchronous, and the duplicate-prevention case in particular is defined by a transition: the second child IS spawned, then loses the lock race and exits. Wait for the count to settle instead. Verified on Linux (ubuntu:24.04, procps-ng 4.0.4, bats 1.10) and macOS: all 14 pass with the fix, and both fujibee#485 tests still fail against an unpatched launcher on both platforms. * fix(role-session): make the record-path memo survive its caller The memoization added for fujibee#466 never held. Every caller resolved the path as `path="$(_agmsg_role_session_path ...)"`, so the array write landed in the command substitution's subshell and was discarded with it -- the next call missed again and re-ran two awk processes plus a third command substitution for the directory. The perf mechanism the change existed for was dead code. Same trap as fujibee#494, walked into from the other side. Add `_agmsg_role_session_path_into`, which assigns to a variable in the caller's own shell, and route the getters through it. A memo written in a long-lived shell is inherited by every subshell forked afterwards, so warming it once is what makes the poll path free; that contract is now stated where the cache is defined, because it is not obvious from the call site. Add `agmsg_role_session_load`, which fills AGMSG_ROLE_SESSION_UUID and AGMSG_ROLE_SESSION_PROJECT in one pass. The launcher wanted both fields for the same pair several times a second and was paying a subshell and a separate read of the same file for each. The launcher now resolves records without command substitution: the safety fingerprint assigns to SAFETY_STATE instead of printing (it was captured with `$(...)`, which discarded the memo again) and reads its roles from a herestring instead of a pipeline (same reason). Both were subshells wrapping the very state that needed to persist. Regression: resolving a pair twice in one shell must encode once, and a subshell forked after the memo is warm must not encode at all. Verified to fail against the subshell-writing shape. Measured steady state is unchanged at 28 spawns / 10 s (2.8/s) on the eight-team, four-role harness -- the memo now does the work the identity cache was already masking.
A recurring question with no answer anywhere in the repo. Both readings are in use and neither is being declared the official one. Placed as the last FAQ entry rather than under the tagline: the top of a README is for what the thing does, and leading with how to say the name reads as more self-conscious than the answer warrants. Someone who wonders about it is already reading, and the FAQ is where they look.
…ujibee#500) * test: close inherited bats fd 3 in all backgrounded process launches bats keeps test-file teardown waiting until fd 3 is closed by every process that inherited it. A backgrounded watcher (or helper process) launched without 3>&- holds that fd for as long as it lives, so a launch whose kill/cleanup races or fails pins the whole bats run at the end of the file until the job-level timeout cancels it. The fujibee#251 sweep (06-27) added 3>&- to these launches in test_watch.bats only. This closes the remaining 18 sites: the fujibee#245 burst test it missed (the launch behind the run that stalled after test 786), 14 watcher launches in test_delivery.bats, 2 in test_install.bats, and the python listener in test_codex_monitor.bats. Redirect-only change; all 218 tests across the four files pass. * test: extend the fd 3 close to every backgrounded launch in tests/ The first commit swept only line-final watch.sh launches; review found the same class surviving in three shapes it missed: continuation-style launches whose & sits on a redirect line, long-lived helpers (sleep 600 stand-in session pids, sleep 30/60 decoys, node fake servers, the compat stub), and mid-line launches (... & pid=$!). Any of these that outlives a failed assertion holds bats fd 3 and pins file teardown — the v1.1.11 release run's attempt 1 went silent 12m58s after its last test before the job timeout cancelled it. Rule is now uniform: every backgrounded launch under tests/ closes fd 3 at the launch site, including short-lived ones, so future copy-paste starts from a safe template. Redirect-only; 787 tests recognized, all 12 changed files pass locally.
The suite is ~790 tests and takes 11 min on ubuntu and 12-17 on macOS, which is the dominant wait on every PR. Split it across four runners per OS. The split is computed from the tree by .github/scripts/shard-tests.sh rather than listed in the workflow. A static list has one specific failure mode: a newly added test file lands in no shard, and CI goes on reporting green while silently no longer running it. Here every tests/*.bats file is assigned to exactly one shard by construction, and the summary job verifies from the shards' own manifests that all of them actually ran, on both OSes -- so a partition bug, a deleted matrix entry, or an unassigned new file surfaces as a named missing file rather than as a green run that tests less than yesterday's. The required status check becomes the `bats` summary job instead of the per-shard jobs. Shard names carry their index, so making them required would mean branch protection had to be re-pointed every time the shard count changed, and would enforce stale check names until someone did. The summary job's name is stable, and it fails unless the whole matrix succeeded -- including when the matrix was skipped, since a suite that did not run is not a suite that passed. The docs-only fast path is unchanged: shards always run and always report, and only the heavy steps are gated. The matrix stays a literal list rather than something the `changes` job emits. A dynamic matrix cannot be evaluated when the job feeding it failed, which would turn a broken `changes` into a dead suite instead of the current fail-open-to-running-everything. Balancing is by @test count, greedy longest-processing-time first. Measured on macOS, that gives 860s -> 366s rather than a nominal 4x: per-test cost ranges from ~0s to ~8s, so count is a loose proxy for runtime. A table of measured per-file seconds would buy ~150s more but goes stale the moment the suite's fixed sleeps become condition polling, which is the next CI change queued. The floor is the slowest single file (test_spawn.bats, 199s) either way.
…es (fujibee#503) * test: wait for conditions instead of fixed sleeps in the watcher suites A fixed `sleep 1` after launching a watcher is wrong in both directions at once: it costs a whole second when the watcher was ready in 40ms, and it still races on a loaded runner that needed 1.2s. Wait for the condition instead -- the watermark file for "the mark is taken", the delivered line for "the row was processed", process exit for "it is gone". Two negative assertions gain rigour rather than losing it. Where the old code slept and hoped enough poll iterations had gone by, it now waits for a later row to arrive, which proves the watcher already scanned the row the assertion denies. One of the two needed a sentinel row for that ordering to hold. Two fixed sleeps stay and are marked deliberate: one separates two mtimes, the other asserts nothing further is emitted. Neither has an event to poll for. * test: wait for both watchers in the global-stop test, not just one `delivery.sh stop` sends TERM to each watcher and returns; the order they actually die in is not guaranteed. Waiting on A and asserting B in the same breath races B's exit trap, so the condition-wait conversion was incomplete exactly where the contract under test is "stops ALL of them". The `sleep 1` this replaced happened to cover both. The two project-scoped tests above stay at one wait on purpose: their second watcher is asserted to still be ALIVE, which needs no grace period. * test: wait for the watermark to persist, not just for the delivered line The restart test kills the first watcher and asserts the second does not re-deliver what the first already streamed. Waiting for the message to appear on stdout is the wrong signal for that: watch.sh writes the line first and persists the watermark after, so killing on the line can drop the mark and make the restart re-deliver -- the exact thing the test denies. It failed on macOS in CI for that reason. Wait for the watermark file to hold the delivered row's id instead. The fixed `sleep 2` this replaced never guaranteed the write either; it just usually won. * test: give the session-end test a fixture it can actually kill session-end.sh only kills a pid whose command line still looks like watch.sh, which is deliberate (pid recycling). The fixture was `sleep 30`, so the kill could never fire -- and the assertion passed anyway: with main's code, `ps` shows the process alive immediately before and after `! kill -0` succeeds. The test never checked what its name claims. Converting the wait to a poll is what exposed it. One check after a fixed sleep missed it; checking repeatedly did not. Launch a real watcher as the fixture so the command-line guard is satisfied and the kill actually happens. * test: require kill(2) and ps to agree before calling a pid gone wait_for_pid_exit read a failed `kill -0` as "exited". That failure is ESRCH (dead) or EPERM (alive, but not signalable by us — sandboxes do this, and a live instance of it was found in delivery.sh status the same day). Reading every failure as death is how a wait-for-exit helper reports success for a running process, which is the defect the session-end test was just fixed for, reintroduced one level down in the helper meant to prevent it. Saying "gone" now needs kill(2) and the process table to agree, mirroring _agmsg_pid_alive. The loop also reaps first, since an unreaped zombie still answers kill -0 and would otherwise look alive for the whole timeout. Covered by tests/test_wait_helpers.bats. The EPERM branch cannot be produced portably in-suite, so the decision rule is pinned instead: anything other than "no such process" counts as alive.
…one (fujibee#505) * fix(liveness): make kill(2) and ps agree before calling a pid dead _agmsg_pid_alive already read the ESRCH/EPERM distinction. Add the ps cross-check fujibee#503 settled on, so "dead" needs a source that does not depend on signalling permission at all, and a fork-free fast path, so the common answer still costs no subshell — callers poll this in loops that exist to be fork-free. * fix(liveness): route every liveness check through the EPERM-aware helper `kill -0` answers "can I signal this", not "is this running". Under a sandbox a live watcher, bridge or app-server fails it, and every shipped caller read the exit status directly: status printed live processes as stale pidfiles, session-start and codex-monitor started a second bridge and app-server beside the running ones, and the launcher reclaimed a live owner's lock — the fujibee#485 duplicate-children shape. All seventeen sites now call _agmsg_pid_alive; instance-id.sh is sourced where it was only reachable transitively, or not at all. * test(liveness): pin EPERM as alive, and sweep for bare kill -0 pid 1 is a live process this user cannot signal, so it stages the distinction directly; the suite skips where no such pid exists. A dead-pid case guards the other direction, since "assume alive" must not make everything look alive. The sweep is fujibee#500's lesson as a test: a partially-hardened file reads like a fixed one, so no shipped script may call kill -0 outside the helper. * fix(liveness): reject 0 as a pid — it names this process group, not a process `kill -0 0` succeeds because 0 addresses the caller's own process group, so a digits-only check called it alive, and callers kill what liveness reports alive: `kill 0` TERMs the group, the caller included. A pidfile holding 0 was all it took. Leading zeros go too — nothing writes them and kill(1) may read them as octal. The check is split out as _agmsg_pid_valid and also gates the two launcher sites that kill a recorded pid without asking about liveness first. Reported by review of fujibee#505. * fix(liveness): bound a pid to pid_t, and stop testing liveness against a number Past INT32_MAX kill(1) rejects the ARGUMENT rather than reporting ESRCH, and everything that is not ESRCH reads as EPERM, i.e. alive. An oversized value in a pidfile was therefore alive forever: lock never reclaimed, bridge never restarted. Bounding the input is what keeps "not ESRCH" meaning "EPERM". The ESRCH tests stubbed kill and passed the literal 999, which is a RUNNING process on some CI hosts — so the new ps cross-check correctly called it alive and the tests failed there and only there. They now use a pid that is genuinely gone, and the cross-check gets a test of its own. Reported by review of fujibee#505. * fix(liveness): make the pid ceiling the platform's, not one number A Windows process id is a DWORD, and liveness there reads the native process table through tasklist rather than kill(1)'s signed pid_t. The INT32_MAX bound sat in front of that branch, so 2147483648..4294967295 — legitimate native pids — were called dead, and a live watcher or lock owner stale. Reported by review of fujibee#505. * docs(liveness): describe both pid ceilings, not just pid_t The comment under the ceiling selection still explained only the POSIX side, which reads as if the Windows branch were an exception to a rule rather than the other half of it. Reported by review of fujibee#505.
…ever it is given (fujibee#508) delivery.sh set now rejects a project_path that is empty/whitespace-only, contains CR/LF, is not a directory, or cannot be entered — instead of silently mkdir-ing whatever it was handed. Legal POSIX paths, including ones with leading/trailing spaces or tabs, are accepted literally. Fixes fujibee#493.
…aths (fujibee#511) delivery.sh status compared the pidfile metadata project against the current project as verbatim strings, so any representational difference (trailing slash, symlink, 8.3/case variance on Windows) labeled a live bridge "stale pidfile (metadata mismatch)". Both sides now normalize to canonical paths before comparing, using the same helper order as the liveness checks. Verified on the reporting host (Windows 11, Git Bash). Fixes fujibee#459.
…ujibee#512) The app-server port scraper matched the banner line only when it arrived uncolored; a build that emits SGR sequences made the scrape miss, so the monitor fell open to plain codex. The parser now strips ANSI sequences before matching, and the test fake terminates with its parent instead of blocking the shard for a minute.
…ujibee#541) * test(watch): wait for the watcher, do not sleep a guess at it run_watcher_for slept a fixed 1.5-2s and then asserted, which encodes a claim about how fast the machine is rather than about the watcher. On a loaded macOS runner that claim is false: watch: persists a watermark file for the session failed twice on main this way, and watch: restart delivers messages that arrived while the watcher was down failed the same way the day before. Same defect class as the watcher-suite sleeps removed in fujibee#503. Each call site now waits for the condition it is about to assert and stops the watcher as soon as it holds. A wait that never completes returns non-zero from the helper, so the failure names the condition instead of surfacing later as a missing grep. The broad-watcher test asserts an absence, which cannot be waited for, so it waits for the watermark instead: a positive signal from the same startup path, past the point where an actas watcher would have written its sentinel. The fixed sleep there was weak in the other direction, since too short a window makes the absence vacuous. The launch stays written out in each helper. A pid=$(...) factoring would put the watcher under a subshell that exits immediately, and watch.sh stops within one interval once its session is gone (fujibee#67). * test(watch): make the broad-watcher absence check able to fail The previous revision waited for the watermark before asserting that no ready sentinel exists. Review caught that the order is the other way round: watch.sh persists the watermark, then runs the DB-open healthcheck, and only then writes the sentinel — so stopping at the watermark leaves the ready block unreached and the absence holds for the wrong reason. Wait for streamed delivery instead. The marker is sent once the watermark exists, so it carries a higher id than the startup mark and is streamed rather than absorbed; arrival proves the main loop is running, which is past the ready block. Checking that exposed a second defect this test has always had: it asserted after stopping the watcher, and cleanup removes on exit every sentinel the watcher owns, so the assertion held whether or not one was ever created. Verified by injection — with the ACTIVE_NAME guard removed so a broad watcher writes sentinels, the kill-then-assert form still passes. The check now runs while the watcher is alive, and fails under that injection naming the sentinel it found.
…ibee#558) (fujibee#560) * fix(codex): bound watch-once by its lifetime, not by its polling The deadline was taken after startup, so real wall time was startup plus TIMEOUT. The bridge force-kills the child at timeout + interval + 10 from spawn, so once startup exceeds interval + 10 the child dies at 124 before reaching its own clean exit 2 — every re-arm, not intermittently. Three of those and the bridge self-destructs, the launcher restarts it, and orphan watch-once processes pile up (the retention half of fujibee#149). Startup is ~0.2s here and ~29s under MSYS fork emulation, which is why this only ever appeared on Windows. Reported with measurements and this fix by 東リ屋 (fujibee#558). A slow startup now eats into the polling window instead of overrunning the ceiling. It cannot skip the check: the loop queries before testing the deadline, so a deadline already past still yields one full inbox check. Tests delay awk, which startup uses to resolve pairs while the poll goes through sqlite3, so startup slows without touching the query. One test pins the lifetime ceiling and fails without the fix (8s against a 4s timeout); the other pins that an already-past deadline still checks, which guards the fix rather than the bug. * test(watch-once): require the startup delay the test depends on The awk shim is the premise, not scaffolding: without the delay an unfixed watch-once finishes in about TIMEOUT and satisfies the ceiling, so the test would go green against the bug it exists for. That happens silently the day startup stops routing through awk. Assert the marker the shim writes, so a vanished seam fails loudly and names what to do. Verified by disabling the shim: the guard fires with "the awk shim never fired ... re-pick the seam" instead of passing.
…Code (fujibee#551) * docs(skill): document the permission allowlist agmsg needs on Claude Code Every step runs through the Bash tool, so without an allowlist entry the user confirms essentially every call. Neither the shared SKILL.md nor the Claude Code template said so, leaving each user to work the entries out. Both files change, because they reach different readers: install.sh generates the installed SKILL.md from the per-type template, so guidance added only to the repository SKILL.md never reaches an installed skill. The Claude Code template is the one that gets it — permissions.allow is a Claude Code concept, so the other type templates stay unchanged. Four entries rather than one: a rule matches the command string as written, and these scripts are invoked both as ~/... and as an absolute path, with or without a bash prefix. Also states that a rule does not carry across shell operators, so the scripts must be called one per Bash call. That is the part an allowlist alone does not fix: batching two steps behind ; or && prompts anyway. Fixes fujibee#488, reported by Paccho-Kun. * docs(skill): state the compound-command rule as the spec has it The previous wording read the permission docs backwards. The rule is that each subcommand must match independently, so two allowlisted scripts chained together are both covered; the official safe-cmd && other-cmd example is refused because the second half matches nothing. Written as "batching two agmsg steps prompts anyway", it imposed a one-script-per-call constraint the spec does not support. The report that prompted this documents the real case, and its example says so: the prompt in delivery.sh ... ; printenv AGMSG_SPAWNED comes from printenv, which no agmsg entry covers, not from the separator. * docs(skill): say what splitting the call does and does not do Offering "keep it in its own call, or allowlist it" as alternatives read as if separating the command removed the prompt. It does not — it only stops that prompt from gating the agmsg call. Only allowlisting makes it prompt-free, so the two are a sequence rather than a choice.
Codex on native Windows often runs agent-typed commands from PowerShell, where a bare bash may be the WSL shim and POSIX quote-splicing breaks -lc payloads. Add an explicit Git Bash example next to whoami in the Codex skill template, and lock the installed SKILL.md text via an install smoke test (templates are what install.sh ships — root SKILL.md is not).
… run --interactive opencode 1.17.15's `opencode run --interactive` exits as soon as the boot prompt's turn completes, so the spawned worker never stays resident and cannot receive further agmsg messages. `opencode --prompt "<text>"` (TUI mode) auto-sends the initial prompt and keeps the TUI resident, confirmed by live testing. Switch the manifest to the existing prompt_arg mechanism already used by copilot/antigravity instead of a fixed multi-word cli prefix. (cherry picked from commit 8c76dec)
Companion to the previous spawn --prompt switch: updates README.md, README.ja.md, and docs/opencode.md so they no longer claim spawn is unsupported for opencode. monitor/both stay listed as unsupported (real-time push is a separate concern, addressed in a follow-up). (cherry picked from commit eb40e64)
Without cmd_prefix=$, agmsg_actas_prompt falls back to the default '/', producing '/agmsg actas <name>' in the boot script — which OpenCode's TUI does not recognize (it invokes skills via '$agmsg', like codex and gemini). The spawned worker would boot but never claim its role. Also tightens the opencode spawn test to assert the '$agmsg actas' pattern is present and '/<cmd> actas' is absent, mirroring the codex fujibee#283 test — the old assertion checked --prompt and actas independently, so a boot script with --prompt followed by a bare-positional '/agmsg actas' would still pass. Copilot review feedback on type.conf L7. (cherry picked from commit 762ad02)
Route opencode's real-time delivery through the sentinel_monitor tool (same shape as Claude Code's Monitor), with a turn-mode fallback when the tool is unavailable. (cherry picked from commit 0f4b8e6)
Companion to the monitor-delivery implementation: updates README.md, README.ja.md, and docs/opencode.md so the delivery-modes table and the OpenCode section list monitor as supported (via the external opencode-sentinel plugin), describe the turn fallback when the plugin is absent, and drop the stale 'no Monitor tool' / 'not supported' claims about monitor and spawn. (cherry picked from commit 4fc39bc)
- watch.sh command now passes "${SENTINEL_SESSION_ID:--}" (4 sites:
_delivery.sh + template.md x3) instead of "$SENTINEL_SESSION_ID", so
launcher shells that drop a quoted-but-empty first arg don't shift
later watch.sh parameters. watch.sh documents this hazard for
GROK_SESSION_ID and recommends the same "${VAR:--}" shape.
- template.md actas/drop steps in monitor mode now explicitly say to
skip the sentinel_* tool calls when the plugin is unavailable, instead
of assuming the tools exist whenever mode=monitor.
- README.md / README.ja.md delivery-modes table: monitor row notes
OpenCode requires the plugin; turn row scopes OpenCode to the
plugin-not-installed case (the template picker now defaults to
monitor when the plugin is present).
Copilot review feedback on fujibee#547.
(cherry picked from commit e03fe89)
…rced The page said monitor never silently drops messages. agmsg writes the rule and does not detect whether the sentinel tool exists, so the fallback is an instruction the agent follows, not a path agmsg enforces — an agent that ignores it delivers nothing and nothing reports that. Reworded to degrades-to-turn, with what is and is not guaranteed spelled out and turn named as the mode to pick when delivery must not depend on the agent honouring a rule. The implementation is unchanged; only the promise now matches it.
…ands The rule wrapped the path in literal single quotes, which holds only while the path contains none. delivery.sh accepts an apostrophe because it is a legal POSIX path character, so such a path ended the quoted argument early in both generated commands — the sentinel_monitor watcher and the fallback check-inbox — and anything after it became live shell syntax. Quote once with %q and use that in both. $type stays as it was: it comes from the type registry, not the caller. The regression runs the generated command lines through bash -n rather than matching text, so it fails on a broken quote instead of on a wording change. Verified against the unquoted form, where it reports the watch.sh line as unparseable. Also carries the fallback wording the rest of this branch already fixed into README.md, README.ja.md, and the opencode doc summary, which still described the fallback as unconditional. Found in review; the earlier sweep looked for the "never silently drops" phrasing and missed these.
…DME (fujibee#571) * docs(site): add agmsg-bubblelog and agmsg-tui to the showcase and README The README listed three derivative projects the site showcase never carried, so the two lists disagreed about what agmsg is built with. They now name the same five, with agmsg-bubblelog and agmsg-tui joining the three already on the site. Dropping the older three is a curation call, not a judgement on them; they are live and can come back if their authors want that. bubblelog reuses a frame of the demo GIF the repository publishes (MIT), cropped to the card ratio rather than letterboxed — the source is portrait, so a plain crop would have shown a band of it. The frame is one that states on screen that it is demo data. agmsg-tui has no screenshot yet, so it uses its GitHub social card, the same treatment agkanban already gets, until the author supplies one. Its blurb stays inside what the repository itself claims, since nobody here has run it. bubblelog is described by what the demo shows — replay and per-agent avatars — because the existing viewer entry already had the messenger-style ground. Star counts refreshed to the live numbers, and the contributor list picks up those whose work has actually landed on main. * docs(site): translate the two new showcase entries Native-checked copy for the eight non-English locales, written per language rather than translated from a draft, matching the existing entries in voice. Three constraints held across all of them: bubblelog keeps replay and per-agent avatars as what separates it from the viewer entry, which already holds the messenger-style ground; agmsg-tui gains no capability or praise the repository does not claim, since nobody here has run it; ratatui stays as written. * docs(site): record where the showcase images came from The two new images are copies of third-party material — a frame of an MIT-licensed demo GIF, and a GitHub-generated social card — and their provenance lived only in a pull request body. MIT asks for its notice to travel with the copy, and a pull request does not travel with a clone or with the built site. The file states, per image, the upstream repository, the licence and its holder where one applies, the source asset, and exactly what was done to it. The social card is marked as what it is: generated by GitHub rather than part of the repository, so the project licence is not what governs it, and a placeholder until its author offers a real capture. The three older images are listed as unrecorded rather than folded in, since assuming they arrived the same way would put a claim in the tree that nobody has checked. * fix(site): supply the MIT notice, and drop the card we have no right to Two rights problems in the provenance file, both found in review. MIT asks for the copyright notice AND the permission notice to travel with a copy. The file named the licence and then claimed that naming it satisfied the condition, which it does not. The upstream licence now sits beside the image verbatim, and the wording says which of the two is doing the work. The agmsg-tui card had no grant behind it at all. Ruling out the repository licence was right — GitHub generates the card rather than shipping it — but that left nothing in its place, and being served publicly is not permission to save a copy and redistribute it from our own site. It is removed. The entry stays, without an image: the showcase card now renders a plain repository name when none is supplied, so a listing no longer depends on having a picture we may not be entitled to use. An image goes back when its author offers one. * docs(site): restore the agmsg-tui card The card is generated by GitHub so other sites can show it when linking to the repository, and a showcase entry pointing at that repository is that use. Removing it read the absence of a source licence as an absence of any basis; the basis is what the card is produced for. Owner decision. The optional-image path stays. It was added to unblock this entry, but it holds on its own — a listing should not depend on having a picture — and a future entry without one now renders instead of breaking. The bubblelog licence file also stays. That one is unrelated: we ship a derivative of someone else`s MIT asset, and MIT asks for its notice to travel with the copy. * docs(site): drop the provenance page It shipped with the site, so it published claims about ourselves that were not true: that three existing images had no recorded origin, and that a GitHub card could not be used. The first reads as a defect notice for something nobody had questioned; the second contradicts the decision that the card is used as generated. The licence file stays. It is not documentation about us — it is the notice MIT requires to travel with the bubblelog derivative we ship.
orangewk
commented
Aug 2, 2026
orangewk
left a comment
orangewk
left a comment
Owner
Author
There was a problem hiding this comment.
全体像
93 files / +5053 −644 のうち、大半は upstream v1.1.12 のマージ (3aa02f3) で、fork 固有の作業は 4 commit・9 files。
- fork 固有ファイル (
scripts/remote.sh, ADR 0005/0006,tests/test_remote_sync.bats等) はマージ後も全て残存 — マージ事故なし - CI: ubuntu/macOS/Windows マトリクス全 green、コンフリクト残骸なし
- 根本原因の分析 (MSYS pid space vs native process table) は正確で、コミット分割も読みやすい
良い点 (検証済み)
_agmsg_pid_aliveにフラグを足さず別 helper を立てた判断が正しい。 呼び出し側で pid space が明示され、混同が grep 可能になるgc.shの変換が漏れなく正しい。 変換されなかったgc.sh:75 / 212 / 255は全てcompat_get_native_cmdlineとペアで、変換された126 / 162はcompat_get_cmdlineとペア。manifest.sh:281が明文化しているペアリング規約 (msys →kill -0/compat_get_cmdline、native →_agmsg_pid_alive/compat_get_native_cmdline) に一致idle-ttl.shの caller contract 更新が完結している。idle_ttl_run_loopの呼び出し元はcodex-monitor.sh:281のbash -c文字列のみで、そこにsource instance-id.shが追加済み。cmdline マッチ (*"idle_ttl_run_loop $PORT "*) も壊れていない- 未定義関数リスクなし。
session-start.shはactas-lock.sh(36行目) 経由でgc.sh(50行目) より先にinstance-id.shを得ており、delivery.shは直接 source - 非 Windows では実質的な強化になっている。
gc.shの manifest 系が barekill -0から EPERM-aware 経路へ、加えて_agmsg_pid_validが効くのでpid=0(プロセスグループ) を「生存」と誤読しなくなった - 新規の codex-monitor 再利用テストは、報告バグそのものの回帰テストになっている
指摘
1. 同じバグ級が watcher 系に未変換のまま残っている (中)
watch.sh:196 は echo $$ > "$PIDFILE" (= MSYS pid) で書き、以下が全てそれを _agmsg_pid_alive (Windows では native table 照会) で読んでいる:
| 箇所 | Windows での帰結 |
|---|---|
scripts/watch.sh:183 |
前の watcher を生存と見なせず kill せず → 二重 watcher |
scripts/delivery.sh:319 |
生存 watcher に気付かず Monitor 指示を再発行 → 二重 watcher |
scripts/check-inbox.sh:81 |
Stop hook が live watcher に譲らない → 二重配信 |
scripts/session-end.sh:59 |
終了時に live watcher を kill せず → orphan |
scripts/session-start.sh:171 |
orphan watcher を回収できない |
うち watch.sh / session-end.sh / session-start.sh は、直後に compat_get_cmdline (MSYS 側) を呼んでおり、この PR 自身が根拠にしているペアリング規約に同一ブロック内で違反している。 #35 のスコープ外なので本 PR で直すかは判断だが、少なくとも follow-up issue 化を推奨。
2. Windows CI がこの修正を守っていない (中)
.github/workflows/tests.yml:146 の bats マトリクスは [ubuntu-latest, macos-latest] のみ。Windows job は bats (windows-latest, install helpers) で install helpers 限定。つまり Windows 固有の修正なのに、回帰検出はローカル手動実行 (PR 本文の 14/14 PASS) にしか依存していない。 次の upstream merge で静かに退行する。
test_instance_id.bats / test_codex_bridge_launcher.bats / test_codex_monitor.bats を Windows job に追加するのが費用対効果として最大 (今回テストを ps 非依存に書き換えたので、まさに実行可能になったはず)。
3. #485 テストのカバレッジが実質的に弱くなった (小)
count_child_launchers (ps ベースの実プロセス数) → lock owner 一致判定への置換は移植性として妥当だが、「ロック取得に失敗した重複 child が exit せず生き残る」退行は新テストでは検出できない (旧テストは検出できた)。ps -Ao が使える OS でのみ旧カウント検証を残す (Windows は skip) 形なら両取りできる。
4. 新テストが Windows 固有性を検証していない (小)
instance-id: MSYS-space liveness accepts a bash child on Windows は Linux/macOS では自明に通る。死んだ pid → 1 を返す負ケースも、MSYSTEM を偽装して分岐が変わることの確認もない。新規共有 helper のテストとしては薄い。
5. _agmsg_pid_valid の上限が pid space と噛み合っていない (小)
_agmsg_msys_pid_alive は Windows で kill -0 (POSIX pid_t) を使うのに、validator は MSYSTEM を見て DWORD 上限 4294967295 を適用する。結果、10 桁 pid が valid を通って kill -0 に "not a pid or valid job spec" で弾かれ、非 ESRCH なのに dead 判定になる。実 MSYS pid は小さいので実害は理論上だが、_agmsg_pid_valid に pid space を引数で渡すのが筋。
6. MSYSTEM の case リストが 3 関数で不一致 (小)
_agmsg_pid_valid / _agmsg_pid_alive は MINGW*|MSYS*|CLANGARM*、_agmsg_msys_pid_alive だけ CYGWIN* を追加。意図的なら理由をコメントに、そうでなければ _agmsg_is_windows_shell() に一本化を。
7. bats のエラー処理 (nit)
[ -f "$CAPTURE" ] && lines="$(wc -l < "$CAPTURE" | tr -d ' ')" は CAPTURE 不在時にその行自体が非 0 を返し、意図した assertion 行ではなくここでテストが落ちる。if ... fi にすると失敗原因が読める。
8. ガードテストの緩和は最小限だが、守備範囲は広がっていない (情報)
grep -v ':[0-9]*: *//' の追加は scripts/lib/manifest.js:21 のコメント 1 行のためだけで、行頭コメントしか除外しないため緩みは最小。ただし現ガードは「helper を通すこと」しか強制せず、どちらの helper かは強制していない。指摘 1 を CI で捕まえたいなら、「同一ブロックに _agmsg_pid_alive と compat_get_cmdline が同居していないか」を検査する lint のほうが効く。
9. VERSION が 1.1.12 のまま (情報)
fork 固有パッチを載せた状態で上流と同一バージョンを名乗るので、バグ報告時に区別できない。過去の merge (1.1.8/1.1.9/1.1.10) も同運用なので既定路線なら OK だが、1.1.12+fork.1 等を検討する余地あり。
結論
修正内容そのものは正しく、マージも安全。approve 相当。 ブロッカーなし。
優先度としては 2 (Windows CI) → 1 (watcher 系の未変換) の順で follow-up を切るのを勧める。1 を放置すると、今回と同じ調査を watcher 側でもう一度やることになる。
🤖 Reviewed with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ps -Aolauncher tests with runtime-lock ownership checksRoot cause
The v1.1.12 liveness consolidation routed app-server
$!, launcher$$, dispatcher/role-child lock owners, and parent lifetime PIDs through_agmsg_pid_alive. On Windows that helper checks the native process table, while these values are MSYS-space PIDs. Live shell-owned processes were therefore treated as dead.The fix adds
_agmsg_msys_pid_alivefor explicitly shell-owned PIDs while retaining_agmsg_pid_alivefor the native bridge pidfile.Impact
Windows Codex CLI monitor sessions can keep their app-server, dispatcher, role child, and bridge running long enough to wake an idle TUI and deliver queued agmsg messages.
Validation
git diff --check: PASSCloses #35