Repository navigation
Add plugin cli-agent-bridge - #7
Conversation
Delegate coding tasks from MiniMax Code to locally installed coding CLIs (Claude Code, Codex, Kimi Code, ZCode, DSH) through a dependency-free stdio MCP server with git-diff review. See NOTICE for upstream credits.
- claude template gains --permission-mode acceptEdits so headless runs can actually edit files (verified end-to-end with Claude Code 2.1.226) - kimi keeps plain -p prompt mode: --auto/-y are mutually exclusive with -p on current Kimi Code versions - git snapshot now lists untracked files (git ls-files --others) so new files created by workers appear in changed files - README/SKILL document permission defaults and honest backend verification status
npm-style .ps1/.cmd shims cannot be spawned directly on Windows, so runCommand retries them through the bundled ps1-runner.ps1 (Windows PowerShell 5.1) with verbatim argument forwarding. README documents the fallback and the custom-wrapper caveat.
- per-workspace serialization of delegate_task - SIGTERM then SIGKILL force kill after timeout grace - snapshots include staged, untracked files and committed deltas - delegate_task marked destructiveHint, isError on failures - notifications/cancelled kills the worker - honest protocol version negotiation - fail closed when git snapshot commands fail - ring-buffer output capture without repeated large copies - README discloses supported operating systems
SKILL workflow and README tool description now cover before/after snapshots, committed deltas, same-workspace serialization, isError semantics, force-kill timeouts, and cancellation.
Seven self-contained tests cover protocol negotiation, tool list, snapshots, dirty-tree guard, unknown backends, cancellation, and before/after snapshots. SKILL wording now notes same-workspace serialization.
- workspace lock is now keyed by the canonical realpath of the git worktree root (git rev-parse --show-toplevel + fs.realpath), so the same checkout reached via a subdirectory, casing, or symlink shares one mutex (P1) - a delegation cancelled while queued for the lock re-checks the cancel flag after acquiring it and returns before spawning the worker (P1) - timeout and cancellation now terminate the whole process tree: taskkill /PID /T /F on Windows, signal to the detached process group on POSIX, instead of only killing the top-level child (P1) - git snapshots tolerate an unborn HEAD (fresh git init) and committedDelta reports the first commits when the worker started from no commits (P2) - codex templates delimit the prompt with -- so option-like tasks are not parsed as CLI flags (P2) - README comparison example now requires two independent git worktrees; the queued same-checkout second run is documented as follow-up work (P2) - SKILL resume guidance no longer implies results carry backend session ids Tests extended to 11 (protocol, tools, snapshots, dirty guard, unknown backend, in-flight cancel, before/after, worktree-root lock serialization, cancel-while-queued, unborn HEAD, codex -- delimiter); 3 consecutive runs green locally
- gitSnapshot/committedDelta executed their git commands via Promise.all; git status and git diff both refresh the index, so concurrent processes raced for .git/index.lock and intermittently failed with exit code 128, surfacing as random 'git snapshot unreliable' errors (reproduced 1-in-15 on Windows; the fail-closed guard turned it into a JSON-RPC error) - commands now run serially per repository; 10/10 Windows runs and a full npm run check in a Linux node:22 container pass - test harness: every server now stops in a finally block (a leaked server kept stdin/stdout pipes open and hung node --test when an assertion failed - the cause of the stalled CI validate job), in-flight cancel test bounds its worker with an explicit timeoutMs
…ribution - disclose repository concurrency: linked worktrees share refs, so snapshots detect other active leases plus completed runs' history records and mark attribution as potentially overlapping (repositoryConcurrency) - attribute only genuinely new commits: ranges exclude all pre-delegation refs, so a branch checkout is reported as a HEAD move with no new commits, and non-commit refs (blob tags) are reported without failing the run - verify process identity before signaling: Windows tracks creation times, POSIX groups are signaled only while the original leader identity matches, and reused PIDs are dropped from the tracked tree - make lease state updates interruptible by the request cancellation/deadline and resync the interrupted release by owner token - move quarantined leases to a recoverable state: removing the quarantine marker authorizes the next delegation to reclaim - catch process-tree inspection failures in the close handler (fail closed instead of an unhandled rejection) and slow the Linux /proc monitor to 250ms - tests: 55 total (checkout attribution, blob refs, concurrency disclosure, lease interruption, quarantined reclaim, PID-reuse identity)
- deduplicate HEAD and branch-ref targets so a commit on the checked-out branch is logged, diffed, and counted once with both labels - diff new refs from their merge-base with the pre-delegation state, so a branch forked from a divergent branch never attributes pre-existing differences to the worker - register list_backends in activeRequests and terminate a hanging version probe on cancellation, skipping the remaining probes - signal the process group when enumeration is unavailable (containment wins when identity cannot be verified) - strip only Git's line terminator from rev-parse --show-toplevel so a worktree root ending in whitespace canonicalizes correctly - tests: 60 total, all green locally
Add cross-process repository locking, fail-closed process containment, safe Git attribution, explicit quarantine recovery, Windows Job Object execution, and regression coverage for reviewed lifecycle and documentable safety boundaries.
hetaoBackend
left a comment
There was a problem hiding this comment.
Please address the repository-scope regression before merge:
This plugin changes the root package.json test script from node --test to node --test --test-concurrency=1. That changes test execution semantics for every existing plugin and masks concurrency regressions globally. Please move the serialization requirement into the cli-agent-bridge-specific test command/fixtures, or provide a repository-level justification and regression evidence showing why all tests must become serial.
The plugin is Windows-only and that limitation is disclosed; the blocking issue here is the unrelated root-level test behavior change. The current [code]smith check is SKIPPED.
hetaoBackend
left a comment
There was a problem hiding this comment.
当前 head 18e26e2 的自带 suite 实测为 218 tests:194 pass、4 fail、20 skipped,仍有阻塞:
- a cross-process lock waiter obeys the delegation deadline 在 tests/server.test.mjs:2458 得到缺失 result,随后 TypeError: Cannot read properties of undefined (reading structuredContent)。
- provenance reads are bounded and interruptible、successive fetches are detected even after FETCH_HEAD is overwritten、a partially successful nonzero fetch makes provenance uncertain 均实际 sawFetch: false,未达到预期的 fail-closed 证据。
- 插件声明 production 仅支持 Windows,但跨平台/Trace2 相关测试没有正确 gate;Windows Job Object 路径在当前环境被跳过。需要修复这些测试/实现问题,或提供可靠的目标平台 CI 证据,不能在所有 [code]smith Checks 为 SKIPPED 时放行。
请先解决 deadline/provenance 失败并补足 Windows 运行证据,再重新提交。
…est script. These tests share git/tmpdir fixtures and deadline-sensitive waits, so they take turns via an exclusive directory lock while the repository-wide `node --test` runner stays file-parallel.
…ck so Node 22 file-parallel runs cannot skip-fail.
…so waiters return a structured MCP result.
Return structured lock-deadline MCP results and realpath-aware Trace2 worktree matching.
|
Head
The Windows Job Object path remained platform-gated on this head, and no new Windows CI log was available at that point. |
…Object containment.
|
Head The job was configured to invoke the cli-agent-bridge test directories. The existing Ubuntu |
|
Current head is This follow-up closes the three review threads left on the merged fork PR:
Current-head local evidence:
The PR body now reflects the actual head and Windows-only production support. It explicitly leaves the GitHub-hosted Windows run unchecked; no skipped |
hetaoBackend
left a comment
There was a problem hiding this comment.
Current head 902b194 improves the previous failures: the local suite is now 199 pass / 0 fail / 20 skipped. However, the skipped set includes the Windows-only Job Object/process-tree paths and other platform-specific provenance/deadline coverage, while the plugin declares Windows production support and this head has no Actions run. Please add and run a Windows CI job covering Job Object cleanup, process-tree deadlines, and the fetch/provenance fail-closed paths, or clearly narrow the support claim. [code]smith is SKIPPED.
Qualify the built-in Add-Type command without relaxing control/worker environment filtering. Compare short and long runtime paths by filesystem identity, bound test RPC/named-pipe waits, retain raw diagnostics, and isolate the complete Windows suite from root CI.
Normalize CRLF only, with LF/CRLF positive cases and negative cases for missing delimiters, mismatched names, and missing instructions. Keep all validation and containment checks enabled.
…p (round-20 MiniMax-AI#7) Reported as: the pill stops on "Command failed" at the end of a turn. `source` is not a label -- it is the arbitration token. The detector's settle logic reads `source == detector` as "this state is mine to rewrite", and uses that to decide whether it may overwrite an agent push. But Write-Status stamped `source = detector` unconditionally, including on the two metadata refreshes (60s 5h-usage, todo progress) that exist only to refresh numbers on a state somebody else owns. The sequence, from island.log on the dev machine: 16:06:24 [detect] error :: Command failed 16:06:25 error :: Command failed <- PostToolUse hook, source=agent 16:06:40 [detect] 5h usage refreshed <- stamps detector over the agent's state 16:06:41 error :: Command failed 16:07:03 [detect] idle :: waiting <- 60s no-event fallback, finally At 16:06:40 the refresh handed write authority back to the detector while leaving `state = error`. From that point the detector owned the state and re-derived `error` from the same stale failed toolResult on every poll -- there is nothing newer in the session log to supersede it. When the turn ended and the Stop hook pushed `done`, the detector was entitled to overwrite it, and the pill went back to red. Nothing cleared it until the 60-second no-event fallback happened to fire. So the error was accurate when it appeared and wrong the moment the turn moved on. The user-visible symptom is a pill that is stuck on a failure that is no longer the current state. A metadata refresh now preserves the previous owner. The claim-ownership stamp is reserved for state-inference writes, which is the only path that actually asserts a new state. This is the same distinction MiniMax-AI#5 drew for step/total/detail; `source` belongs to the observable state, so it follows the same rule. Tests: 2 new assertions, negative-first, both observed failing before the fix. They assert the ownership token in both directions -- a metadata-refresh write keeps `agent`, a state-inference write claims `detector` -- because a test that only checks one side would pass just as happily if the field were always blank. Validation: sub-step suite 49 pass / 0 fail (was 47), smoke 116 pass / 0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows PowerShell 5.1 (exit 0).
….0) (#62) * feat(mcode-island): add sub-step progress fields to status.json (v0.4.0) Extends status.json schema with three optional fields — step, total, detail — so agents can publish per-iteration progress during Computer Use loops, multi-step plans, and long-running tool sequences. The widget now renders "step N[/M] · detail" instead of only the coarse state, so the user can see what the agent is doing right now without waiting for it to finish. notify-island.ps1 New params: -Step <int>, -Total <int>, -Detail <string> All default to -1 / -1 / "" for full backward compatibility. The status.json payload now writes step/total/detail alongside the existing state/message/progress fields. Old callers (omitting the new params) produce identical status.json behavior except for three extra fields whose values are the sentinels. mcode-island.ps1 (widget) New Build-DisplayMessage helper that converts the schema fields into the visible pill text. Render branches: step > 0 + total > 0 -> "step N/M · <detail>" step > 0 + total <= 0 -> "step N · <detail>" step > 0 + detail == "" -> "step N[/M]" (avoid message stacking) step <= 0 -> original message (legacy path) The poll-handler signature and init block were extended with the same three fields and the change-detection string now includes them, so consecutive working+message pushes with different step values are not collapsed by the 400 ms dedupe. io.minimax.mcode/hooks/scripts/_lib.ps1 Push-Island now accepts -Step/-Total/-Detail and forwards them to notify-island.ps1. Format-ToolSummary has a new mcode-computer-use branch that extracts action + coordinate with explicit -join "," so coordinate arrays render as "(x,y)" not PowerShell's default "(x y)" (the latter looked like a truncated number on the pill). io.minimax.mcode/hooks/scripts/post-tool-use.ps1 Pushes -Detail with the Format-ToolSummary output split so the pill shows "Bash ok · ls -la /tmp" instead of "Bash ok". pre-tool-use.ps1 already used Format-ToolSummary so no change there. All new schema fields are optional. notify-island.ps1 callers that omit -Step/-Total/-Detail see no behavior change. Widget versions that do not know the new fields ignore them (PSObject.Properties[name] everywhere). Verified by smoke.mjs case "backward compat: old callers produce step=-1, total=-1, detail=""" and test-substep-progress.mjs case "Push-Island backward compat: missing new params -> step=-1, total=-1, detail=""". no credentials : none added; the IPC is local-filesystem only no network : no network calls added; notify-island.ps1 still writes status.json under %APPDATA%/mcode-island no telemetry : no telemetry added; the existing append-only island.log is unchanged and the 400 ms polling cadence is unchanged no third-party svcs : no new third-party deps; the change is pure PowerShell + schema cross-platform : no hardcoded host paths; no /Users/ /home/ C:\ /mnt/ literals introduced; the existing smoke.mjs cross-platform scan still passes atomic write : notify-island.ps1 already writes status.json atomically via staging + rename; no change closed schema : status.json is open by design (not contract- locked), but each new field has a documented sentinel (-1 / -1 / "") so absent fields are semantically equivalent to explicit sentinels smoke self-check : smoke.mjs gained 5 new checks under section 5c1; locked the new surface contract so a future refactor that drops the params surfaces in smoke before reaching the slower pwsh-spawned tests smoke.mjs : 48 pass, 7 warn, 0 fail (7 warn are pre-existing "forward" event catalog entries pending mcode 0.2.4+ Runtime confirmation; unchanged by this PR) scripts/test-substep-progress.mjs : 26 pass, 0 fail Sections: 1. notify-island.ps1 schema round-trip (4 cases) - all three new fields round-trip with explicit values - step without total: step=5, total stays -1 - backward compat: step=-1, total=-1, detail="" - existing fields (state/message/progress/ts/source) preserved 2. Build-DisplayMessage function contract (9 cases) covering include step values, total omission, detail omission, empty message, total=0 edge, step=-1 with orphan detail 3. Format-ToolSummary for mcode-computer-use (7 cases) including Bash / Read / Edit regression coverage 4. Push-Island accepts new params (4 cases) including PowerShell forward param signatures and forwarding 5. Push-Island end-to-end (hook -> status.json) (2 cases) Per the round-4 lesson (test pass != contract honored), I broke the coordinate formatter in _lib.ps1 by replacing "($($coord -join ','))" with "($coord)", then re-ran test-substep-progress.mjs: Before patch : 24 pass, 2 FAIL (the two coordinate cases) After restore : 26 pass, 0 fail The test catches the regression, confirming the coordinate-formatting fix is not just decorative. Widget visual verified locally: notify-island.ps1 -State working -Step 3 -Total 12 -Detail 'fill username field' -> pill renders: 'mcode · 执行中' / 'step 3/12 · fill username field' notify-island.ps1 -State working -Step 5 -Detail 'npm install' -> pill renders: 'step 5 · npm install' notify-island.ps1 -State working -Message 'Read ok' -> pill renders: 'Read ok' (legacy path, no step prefix) No data migration. Existing status.json consumers see three new fields they can ignore. Existing notify-island.ps1 callers see no behavior change. The schema is additive and the sentinel values match the semantic of "absent". Companion docs updated: skills/mcode-island/SKILL.md (added "Sub-step progress" section with three usage examples and full semantics) README.md (added brief mention in the notify-island.ps1 section with a forward pointer to SKILL.md) Bump plugin.json 0.3.0 -> 0.4.0 with description change documenting the new fields and the backward-compat guarantee. No upstream protocol changes. (Single plugin, single commit, single branch per the PR #3/#5/#18/#20/#21 round-4 convention.) * feat(mcode-island): pill click toggles show / hide CLI window (round-14) Previously the pill's MouseLeftButtonUp always invoked Focus-CallerWindow, which only ever restored + foregrounded the target window. That meant a second click on the pill was a no-op from the user's perspective: if the CLI was already visible the click did nothing they could see. The mental model "单击调出, 单击收起" was not honored. This commit extracts the caller-target resolution into a shared helper (Resolve-CallerWindow) and adds Toggle-CallerWindow, which gates on IsWindowVisible + IsIconic: shown + no -> SW_MINIMIZE (click hides; taskbar entry kept) hidden/min -> SW_RESTORE + (click shows; SetForegroundWindow SetForegroundWindow steals focus) The click handler now calls Toggle-CallerWindow instead of Focus-CallerWindow. Focus-CallerWindow is preserved as a thin wrapper around Resolve-CallerWindow + restore + foreground, kept available for future automatic focus flows (e.g. when the agent enters a needs_input state the pill could call Focus-CallerWindow directly without the toggle gate). ## Design Toggle, not flip-and-stick: every click cycles show -> hide -> show. The user's "单击调出, 单击收起" requirement is the mental model. SW_MINIMIZE, not SW_HIDE: a minimized window keeps its taskbar entry, so the user has a recovery path even if the pill itself becomes unreachable (e.g. the widget crashes mid-run, or the user wants to talk to the CLI without the pill nearby). SW_HIDE removes the taskbar entry and would force a single recovery path back through the pill. Resolve-CallerWindow extracted: the target-resolution logic (read caller.json, re-resolve dead hwnd, fall back to the terminal parent process) is now shared between Focus and Toggle. Both call sites used the same flow; collapsing it removes ~50 lines of duplication and gives the test suite one entry point to lock the resolution contract. Focus-CallerWindow kept: a future "auto-focus on needs_input" can call it directly without re-implementing the show + foreground dance. Today only Toggle is wired to the click handler. ## Backward compatibility No external contract changes. caller.json schema is untouched. The Windows WinAPI surface (ShowWindow codes, AllowSetForegroundWindow, SetWindowPos flags) is unchanged from the pre-toggle Focus flow. Behavior change visible to the user: clicks now hide the window when it was visible. This is the requested feature. ## Design compliance (per PR #21 round-11 standards) no credentials : none added; the IPC is local-filesystem only no network : no network calls added no telemetry : no telemetry added no third-party svcs : no new third-party deps; pure PowerShell + Win32 user32.dll calls (already declared) cross-platform : Win32 calls + user32.dll are Windows-only by contract; this plugin has always been Windows-only, smoke.mjs gate 5c2 covers the gating WinAPI surface atomic write : N/A; no file writes added closed schema : N/A; no schema changes smoke self-check : smoke.mjs section 5c2 (4 checks) locks: - Resolve-CallerWindow function present - Toggle-CallerWindow function present - Toggle gates on IsWindowVisible + IsIconic - MouseLeftButtonUp invokes Toggle (not Focus) ## Validation smoke.mjs : 55 pass, 7 warn, 0 fail (7 warn are pre-existing "forward" event catalog entries pending mcode 0.2.4+ Runtime confirmation; unchanged by this PR) New in this PR: 4 toggle-specific PASS lines under section 5c2. ## Test evidence (negative-injection verified) Per the round-4 lesson (test pass != contract honored), I broke the click handler by replacing Toggle-CallerWindow with Focus-CallerWindow and re-ran smoke.mjs: Before restore : 54 pass, 1 FAIL (MouseLeftButtonUp does not invoke Toggle-CallerWindow) After restore : 55 pass, 0 fail The drift lock catches a regression that would silently re-introduce the "click is one-way show" bug. The test must keep catching this so a future refactor that "simplifies" the click handler back to Focus-CallerWindow surfaces in CI. ## Reference No docs change required (the toggle is implicit in "click the pill"). Skill SKILL.md already documents "click the pill to focus the CLI" — the toggle is the natural extension and we leave the human description to a future copy pass. Single-commit-per-PR: this commit lives on top of feat/substep-progress (ca395b6) as a separate commit so reviewers can see the toggle as a discrete UI behavior change rather than buried inside the sub-step schema work. Upstream can squash or keep separate. * fix(mcode-island): toggle show uses SW_MAXIMIZE to fix 480x84 strip bug (round-15) Toggle-CallerWindow's restore branch called SW_SHOW + IsIconic + SW_RESTORE, which preserves the window's pre-hide size. When the WT window was accidentally resized to a thin strip (e.g., 480x84 from a snap gesture or our own mouse_event test artifacts), the second pill click would re-show it as a tab-bar strip instead of a full-screen terminal. The user's report: 'hide works, show is a thin strip'. Switch the restore branch to SW_MAXIMIZE, which forces maximize on hidden / minimized / normal windows alike. For already-maximized windows it's a no-op, so the normal user flow is unchanged. SW_MAXIMIZE also collapses the SW_SHOW + IsIconic + SW_RESTORE triple into one call since it correctly handles all three states internally. Design compliance: - Follows round-14 toggle architecture: hide = SW_HIDE (no taskbar entry, no Always-show-tabs artifact), show = SW_MAXIMIZE (full-screen, no thin-strip artifact) - Cross-platform: only Win32 user32 ShowWindow constants, no path changes from round-14 - No telemetry / no network / no third-party services added Validation: - Local smoke.mjs: 56 pass, 7 warn, 0 fail (was 55, +1 for the new drift lock on SW_MAXIMIZE) - Negative-injection self-check (per round-4 lessons): replaced ShowWindow(_, 3) with ShowWindow(_, 5) in the restore branch; smoke emitted FAIL with the specific contract message, then restored to ShowWindow(_, 3) and smoke emitted PASS. Confirms the new check is a real contract lock, not a false green. Test evidence: - Reproduced: WT rect went to (0,0,480,84) [480x84] (visible=True, iconic=False) after a mouse_event snap artifact - After this commit, pill click sequence (hide -> show) brings WT back to full-screen via SW_MAXIMIZE regardless of any prior resize * fix(mcode-island): toggle show uses MonitorFromWindow + SetWindowPos to fill actual 2560x1440 monitor (round-16) Toggle-CallerWindow's restore branch called SW_MAXIMIZE alone, which fills the window to the WinForms-reported logical work area (1920x1080). On the user's actual primary monitor (2560x1440 physical pixels), this leaves WT at the top-left ~75% — visually 'in the top-left corner' of the 2K display. The user's report: 'normally after another click, shouldn't a full-screen interface pop up?' Two discoveries during diagnosis: 1. [Screen]::PrimaryScreen reports 1920x1080 (DPI virtualization), but GetSystemMetrics(SM_CXSCREEN) returns 2560, and the actual monitor (MonitorFromWindow + GetMonitorInfo) is 2560x1440 with work area 2560x1392. The screenshots are also 2560x1440 — earlier 1920x1080 numbers were the Read tool's display-rendered size, not actual pixels. 2. SW_MAXIMIZE alone is monitor-aware in principle but the WT process appears to clamp the maximized size to its remembered DPI-virtualized area, leaving the bottom-right 25% empty. Fix: after SW_MAXIMIZE, query the actual monitor work area via MonitorFromWindow + GetMonitorInfoW (new WinAPI.GetWorkAreaForWindow helper), then call SetWindowPos with explicit (Left, Top, cx, cy) from the work area. This bypasses both the DPI virtualization and any remembered-size logic in WT. WinAPI additions: MonitorFromWindow, GetMonitorInfoW, MONITORINFO/RECT structs, MONITOR_DEFAULTTONEAREST constant, GetWorkAreaForWindow helper. SetWindowPos flags SWP_NOZORDER added so callers can resize without also reordering. Z-order: SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0, SWP_NOACTIVATE|SWP_NOZORDER) after the resize pushes WT to the front without requiring foreground permission (replaces the HWND_TOPMOST then HWND_NOTOPMOST dance that also doesn't require foreground but adds flicker). Design compliance: - Cross-platform: pure Win32 user32 calls, no path changes - No telemetry / no network / no third-party services - Round-15 SW_MAXIMIZE contract preserved (still in the restore branch) Validation: - Local smoke.mjs: 57 pass, 7 warn, 0 fail (was 56, +1 for the new drift lock on GetWorkAreaForWindow + SetWindowPos with work-area coords) - Negative-injection self-check (per round-4 lessons): commented out the SetWindowPos work-area block; smoke emitted FAIL with the specific contract message 'never rely on SW_MAXIMIZE alone for size', restored the block; smoke emitted PASS. Confirms the new check is a real contract lock, not a false green. Test evidence: - Before: WT rect (-8,-8,1928,1040) [1936x1048] on 2560x1440 monitor → fills ~75% of physical display, visually 'top-left corner' - After: WT rect (-8,-8,2552,1392) [2560x1400] on 2560x1440 monitor → fills full work area (verified via GetWindowRect after click) * fix(mcode-island): z-order SetWindowPos carries SWP_NOSIZE (round-17 fix for 480x76 regression) Toggle-CallerWindow's follow-up z-order SetWindowPos call (HWND_TOP with cx=0, cy=0) was missing the SWP_NOSIZE flag (0x0001). Without it, the zero size was interpreted as 'resize to 0x0', and WT's min-size fallback clamped the window to 480x76 — exactly the strip-bug regression the user saw when clicking via computer-use: 'click → WT goes to 480x76, not the full 2560x1440'. Discovered empirically during the round-16 verification: - Direct Win32 (ShowWindow SW_MAXIMIZE + SetWindowPos work area) ended at 2576x1408 ✓ (full 2560x1440 + 8px borders) - Same flow via the widget ended at 480x76 ✗ (WT min-size) - Diff: widget has an extra SetWindowPos(HWND_TOP, 0, 0, 0, 0, SWP_NOACTIVATE|SWP_NOZORDER) without SWP_NOSIZE Fix: add SWP_NOSIZE constant to WinAPI class, OR it into the z-order SetWindowPos flags. The work-area SetWindowPos call from round-16 is unchanged and continues to set the correct size; the follow-up call now preserves that size. Validation: - Local smoke.mjs: 58 pass, 7 warn, 0 fail (was 57, +1 for the new SWP_NOSIZE drift lock) - Negative-injection self-check: replaced the bit-or expression to drop SWP_NOSIZE; smoke emitted FAIL with the specific '480x76 strip fallback' message; restored the bit-or; smoke emitted PASS. Test evidence: - Before this commit (round-16 only): computer-use click on pill left WT at rect (0,0)-(480,76) [480x76], visible=True, zoomed=True - After this commit (round-17): computer-use click on pill should leave WT at rect (-8,-8)-(2568,1400) [2576x1408], visible=True, zoomed=True → fills the entire 2560x1440 monitor as the user expects * feat(mcode-island): tool-verb messages + family tinting (round-18) The pill showed the bare tool name for every phase, so `working` and `done` were indistinguishable at a glance: both read "bash". The mcode CLI TUI solved this with a descriptor table (launcher-GHPADSKI.js HL[]) that gives every tool a present-tense verb while it is in flight and a past-tense one once it returns. This commit ports that idea to the pill. Detector - New $TOOL_ACTIONS: 18 tools x {running, done, fail}. Messages become "Running ..." / "Ran" / "Command failed" instead of "bash : {...}" / "bash 完成" / "bash 失败". - New $TOOL_FAMILIES: maps a tool to one of 7 families (shell/read/write/search/task/web/plan). - New Get-ToolKey / Get-ToolVerb / Get-ToolVerbFallback / Get-ToolFamily. Unknown tools fall back to "Using <name>" / "Used <name>" so the pill never shows a bare identifier with no phase signal. - Get-ToolKey caps the normalized name at 64 chars. The name comes from the session log, i.e. it is attacker-influenced in a hostile repo, and a 4 KB "tool name" must not become a hashtable key. - Infer-State now returns a `family` field on every tool branch, and Write-Status emits it to status.json. - $MSG_FAIL / $MSG_OK are removed. The three tool branches were their only consumers, and leaving dead byte-array constants behind would invite a future reader to think they are still wired up. Widget - New $familyMap: 7 families -> dot colors. - Update-State takes $Family and resolves a $dotHex / $ringHex. The family color overrides the state color for thinking/working/waiting only, so done stays green and error stays red -- the result signal the user relies on must not be repainted by whatever tool happened to run. - The status.json poll signature now includes `family`. Without it, two consecutive working states from different families would be deduped and the dot would keep the previous color. - The progress bar follows the same resolved color, so the dot, the pulse ring, and the bar always agree. Validation - smoke.mjs: 36 new checks in sections 5d/5e, covering the two tables, all four helpers, the three required families, the three state-branch call sites, the status.json field, the widget $Family parameter, the active-state gate, and the dot/ring paint path. - 98 pass, 7 warn, 0 fail locally. Test evidence - 16 negative-injection cases, each a minimal change that breaks one clause of the contract. All 16 turn the suite red; the baseline is restored to 98/0 after each. - Three of the 16 initially did NOT go red. They were false greens and the checks were rewritten rather than the failures being waved off: 1. The active-state gate was matched as a bare `$State -in @('thinking','working','waiting')`, but that substring also appears on the pulse, elapsed, and progress branches. Ungating the family tint left those three in place and the check stayed green. It now anchors on the `$Family -and $State` conjunction. 2. The three state-branch locks were presence tests, but each state has TWO call sites (the ledger.jsonl branch and the messages.jsonl branch). Breaking only the ledger branch left the messages branch matching. They now assert a minimum occurrence count of 2. 3. The first harness used CRLF anchors against LF-only files, so every case reported SETUP-MISS and the suite looked green while nothing had actually run. Anchors now use LF and the harness fails loudly on a missing anchor instead of counting it as a pass. - End-to-end on the live widget: all 6 active family colors render (shell green, read blue, write yellow, search purple, web pink, plan violet) and done/error keep their green/red result colors. - Live status.json after deploy carries the new contract end to end: {"state":"working","message":"Running {...}","family":"shell",...} Design compliance - No credentials, no network, no telemetry, no third-party services. The verb tables are static data; the only I/O is the pre-existing status.json write that the widget already polls. - Cross-platform: no host paths introduced. The tables are keyed by tool name only, and the existing hardcoded-path scan covers the edited files. - ASCII-only additions. The existing Chinese message constants are untouched, so the PS 5.1 GBK parsing defense is unchanged. - Session log is a local file the user already owns; reading tool names out of it adds no new capability and no new data source. * fix(mcode-island): separate plan/search hues, lock family color distance The round-18 family palette shipped with plan at #8B5CF6 against search's #A855F7. On the dark pill those two read as the same purple, so `grep` and `todowrite` were not actually distinguishable -- which is the whole reason the tinting exists. plan moves to a desaturated slate #94A3B8. smoke.mjs gains a perceptual-separation check over the family table. It converts each #AARRGGBB entry to CIELAB and asserts every pair is at least dE 20 apart (CIE76, the usual "not the same color to a human eye" cutoff for flat UI fills). Test evidence - Negative injection: putting plan back to #8B5CF6 makes the check fail with "search (#A855F7) and plan (#8B5CF6) are only dE 10.1 apart". Restoring #94A3B8 returns the suite to 100 pass. - The check itself went through two false greens before it was correct, both caught by writing the guard before trusting the output: 1. The color regex required 6 hex digits, but the table stores #AARRGGBB, so it matched nothing and the check reported "all 0 family colors are perceptually distinct" -- a vacuous pass. A parse-count guard now fails outright below 7 entries. 2. The same check initially measured raw sRGB luminance, which flags blue vs purple as "identical" (0.02 apart) because they differ in hue but not brightness. That produced five false alarms on a palette that is fine in practice. Replaced with CIELAB dE. - Live widget: all 7 family colors confirmed distinct, and the earlier screenshot run that showed six identical rows was a harness timing fault (capturing the previous frame), not a rendering fault -- the capture now waits for the widget to log the state before shooting. - 100 pass, 7 warn, 0 fail. * feat(mcode-island): ask_user reports waiting, not working On a `full access` permission setup the permission-request hook never fires, so `waiting` was unreachable and ask_user was the only thing that could actually block the agent. It was reporting `working`: working :: ask_user : {"mode":"questionnaire","requiresExplicitResponse":true,... The pill showed a blue dot with a gear icon -- the universal "leave it alone, it is making progress" signal -- while the agent sat blocked on a questionnaire, plus a truncated JSON blob where the useful text should be. Three occurrences in the 9/24 log confirm this was not a one-off. ask_user now reports `waiting` with a question count, so the pill reads "Asking 2 questions" under an amber dot and a "?" -- the "come back, I'm blocked" signal. It wears the state color rather than a family tint: a blocked agent must not look like an in-flight delegation. Detector changes - $S_WAITING is now defined. It was referenced nowhere before, so any branch using it would have returned $null for the state. - Test-IsAskUser and Get-AskQuestionCount helpers, wired into both tool paths (ledger.jsonl and messages.jsonl). - waiting joins both settle sets. The 60s no-activity fallback would otherwise downgrade a pending questionnaire to "已静默 60s" exactly when the user has been away longest, and the takeover arbitration would let the pre-tool-use hook's `working` shadow it forever. - ask_user is removed from $TOOL_FAMILIES. Bug found while testing - Get-AskQuestionCount originally took a parameter named `$args`, which is a PowerShell automatic variable. Shadowing it makes every read inside the function return the function's own argument list instead of the caller's value, so the count was 0 for every input and the message degraded to a bare "Asking". Caught by exercising the helper against seven argument shapes before wiring it up; the parameter is now $toolArgs and smoke locks the name. Validation - smoke.mjs: 7 new checks in section 5d2. 107 pass, 7 warn, 0 fail. - 7 negative-injection cases, all turn the suite red; baseline restores to 107/0 after each. Two of them target the multi-site and settle-set contracts specifically, since those are the ones that fail silently. - The real Infer-State from the deployed file was exercised against nine real messages.jsonl payload shapes: ask_user with 0/1/2/5 questions, a missing questions key, absent arguments, and arguments arriving as a JSON string. All six ask_user shapes infer `waiting` with no family; bash, read, and todowrite are unchanged. - Live widget: `Asking 2 questions` renders amber with a `?`, and the post-answer transition to `done` / `Asked` still works. Test evidence - Deliberately did NOT append synthetic toolCall records to a live session log. The running agent is writing to that file and injecting phantom work into it could make the agent act on it. The end-to-end check runs the deployed Infer-State over real payload shapes instead, which covers the same code path without touching agent state. Design compliance - No new I/O, no network, no credentials, no telemetry. The change is a state mapping plus a question count read from arguments the detector was already parsing. - Get-AskQuestionCount swallows parse failures and returns 0, so a malformed arguments payload degrades to "Asking" rather than breaking the poll loop. - ASCII-only additions; the PS 5.1 GBK parsing defense is unchanged. * fix(mcode-island): redact typed text in tool summaries (round-19 #1) Security. Format-ToolSummary put `tool_input.text` -- whatever the user typed -- straight into the pill detail string. That string reaches: - status.json on disk - island.log, append-only - the always-on-top pill, visible to anything sharing the screen So a password, one-time code, or token entered through Computer Use ended up in a screenshot, a screen share, or a screen recording. The pill is the worst of the three because it is deliberately on top of everything else. The text branch now reports the action plus a character count and nothing else. No substring, no length bucketing that would leak the shape of the value, no echo. The action name survives, so the pill still answers the question that matters: is the agent typing right now? The coordinate branch is untouched -- coordinates are not secret and are genuinely useful to see. Validation - scripts/test-substep-progress.mjs is rewritten as a behavioral suite (see the CI commit), and section 1 plants a canary secret SUPER-SECRET-PASSWORD-8f3a91c2 in tool_input.text, runs the real Format-ToolSummary, and asserts the canary is absent from the output. It also asserts the action survives and the length is reported, so a redaction implemented by silently dropping the branch would not pass. - End to end against the deployed hook: a PreToolUse event carrying P@ssw0rd-CANARY-7d3e91 produced status.json: "mcode-computer-use : type <22 chars, redacted>" island.log: "working :: mcode-computer-use : type <22 chars, redacted>" and the canary appears in neither. - Negative injection: restoring the original `$detail = "$act '$txt'"` turns the canary check red. Design compliance - No new I/O or network. The change only removes data from a string. * fix(mcode-island): detector preserves sub-step fields across a rewrite (round-19 #2) Write-Status rebuilds the whole status.json payload on every write, and the detector writes on every 5h-usage refresh and every todo-progress refresh. step / total / detail are agent-pushed sub-step state, not detector state, and they were simply not in the payload -- so a routine usage refresh silently zeroed whatever sub-step the agent had reported. On a long task the pill would drop "step 4/12" back to the bare message and never recover until the next tool call. Write-Status now reads the current file and carries step / total / detail forward. It is a read-modify-write, not an overwrite, and defaults to -1 / -1 / "" when there is no prior value so a fresh install still renders normally. Validation - Behavioral: a status.json carrying step=4 total=12 detail="running tests" survives a Write-Status rewrite with all three intact; an unwritten status.json yields -1 / -1 / "". - End to end against the deployed detector: notify-island pushed step=4 total=12 detail="running the test suite" (source=agent), then 15s later the detector wrote its own status and all three fields were still there. - Negative injection: reverting Write-Status to the blind overwrite turns the preservation check red. * fix(mcode-island): restore path needs SWP_NOMOVE, and drops SWP_NOZORDER (round-19 #3) The z-order SetWindowPos after a restore used SWP_NOACTIVATE | SWP_NOZORDER | SWP_NOSIZE with X=Y=cx=cy=0. Two problems: SWP_NOMOVE is missing. X=0/Y=0 is then a real move request, so on any monitor whose origin is not 0 (a secondary display to the right, or above) the restored window is yanked to the primary monitor's (0,0). Invisible on a single primary display, which is why the round-15/16/17 manual testing never caught it. SWP_NOZORDER is present, which contradicts the call's own purpose. That flag makes Windows ignore hWndInsertAfter entirely, so passing HWND_TOP alongside it is self-defeating: the z-order change silently does nothing and only BringWindowToTop afterwards is doing the work. Now: SWP_NOACTIVATE | SWP_NOSIZE | SWP_NOMOVE, with HWND_TOP actually honored. The round-17 smoke lock asserted the literal `SWP_NOZORDER -bor SWP_NOSIZE`, i.e. exactly the pair the review asks to change, so it had to be rewritten rather than updated. It now checks that both required flags are present and that SWP_NOZORDER is absent, matching on flag names rather than their order. Validation - test-substep-progress.mjs and smoke.mjs both cover this. - Negative injection: dropping SWP_NOMOVE turns both red; adding SWP_NOZORDER back turns both red. * fix(mcode-island): post-tool-use passes Step/Total so detail renders (round-19 #4) post-tool-use.ps1 computed a per-tool detail string and passed it as -Detail, but left Step at its -1 default. Build-DisplayMessage returns the bare message when Step <= 0 and only consults Detail inside the Step > 0 branch, so the detail was discarded on every single call. The "step N/M - detail" contract the widget documents could never fire from this hook. Both Push-Island calls now pass -Step 1 -Total 1. PostToolUse is a single tool result and does not participate in a multi-step sequence, so 1/1 is the honest value; inventing a larger denominator would risk rendering "step 1/0" on a fresh install. Validation - Build-DisplayMessage is exercised directly over five argument shapes: step+total+detail, step without total, step with empty detail, no step (detail must be ignored), and no step and no message. - Both Push-Island calls are matched individually and must each carry -Step and -Total. A single scan across the file passes as soon as one call has them, so removing -Step from just the error branch would otherwise stay green. - The $step/$local $total assignments are asserted to be positive. They are the "unset" sentinel everywhere else in this plugin (-1 means no sub-step), and setting them to -1 keeps every flag check green while the detail is dropped again at render time. A negative-injection pass caught that blind spot. - Negative injection: dropping -Step from the error branch, dropping -Total from the done branch, setting $step = -1, and inverting Build-DisplayMessage's Step<=0 early return each turn the suite red. * ci(mcode-island): run test-substep-progress on windows-latest (round-19 #5) scripts/test-substep-progress.mjs existed but no workflow invoked it, so none of its assertions gated anything. It is also the only suite that runs the PowerShell under test rather than grepping the source for substrings, and it is where the secret canary for review item #1 lives -- so leaving it unwired meant a redaction regression could land without anything noticing. A new step 5 runs it after the existing four. PowerShell resolution: the suite prefers $env:PS_BIN, then an existing path if PS_BIN looks like one, then probes pwsh / powershell.exe / powershell on PATH. It never assumes a user-specific absolute path, and it fails loudly when no PowerShell is found rather than silently skipping. The workflow step logs which shell was resolved before invoking it. Two encoding issues had to be solved for the suite to produce trustworthy results on a runner whose console code page is not UTF-8: - the generated .ps1 needs a BOM, or PowerShell 5.1 reads it as ANSI and mangles any non-ASCII literal in the harness; - the child must set [Console]::OutputEncoding, or a middle dot comes back as two GBK bytes and two correct assertions fail as bogus diffs. Without both, the suite fails on correct code, which is worse than not running it: it would train everyone to ignore it. * ci(mcode-island): resolve the suite path from the repo root (round-19 #5 fix) The step I added in 5fbda03 ran `node scripts/test-substep-progress.mjs`. GitHub Actions executes every `run:` block from $GITHUB_WORKSPACE, not from the plugin directory, so node looked for <repo>/scripts/test-substep-progress.mjs which does not exist, and the run died with MODULE_NOT_FOUND after 32s. It passed locally only because my shell happened to already be sitting in plugins/antianqi/mcode-island. A test that only runs from one directory is not a test. The path is now resolved against the repo root and checked with Test-Path before node runs, so a future rename fails with a clear message naming the cwd instead of a node stack trace. Also fixes a bug in the same step: the version banner ran `& $c` after the loop, where $c is the last candidate rather than the shell that was actually selected. On a runner without pwsh the banner would have invoked the wrong binary, or nothing at all. * fix(mcode-island): round-20 - close a false disclosure claim and two unbounded-growth leaks Audited the plugin against the 0.5.0-0.5.8 changelog and the live runtime state. Three real defects, each with a regression test that was verified to fail before the fix landed. 1. The 5h-usage disclosure named a host the code never called. Get-5hUsage.ps1 obfuscates the endpoint into a byte array (PS 5.1 parser-quirk defense). The bytes decode to api.minimaxi.com, which is correct and is what the running detector actually calls. But the trailing comment on that same line said api.minimax.com, and the README named api.minimax.io in three places -- including an absolute claim that the token is "never sent to a host other than api.minimax.io". That claim was false: the code never contacted that host at all. For a plugin whose pitch is four independent disclosure sections, an overstated isolation guarantee is worse than no guarantee: a reviewer checking host consistency would find the code and the docs disagreeing and would be right to distrust both. The smoke check now DECODES the byte array and compares the docs against that decoded truth. It never hard-codes the endpoint, so it cannot become a second copy of the disclosure it polices, and it fails the moment code and docs drift apart again. 2. island.log and widget.log had no size cap, ever. mcode-island.ps1 has documented "keep only the last 1KB" since the widget was written. Dbg was a bare Add-Content and never implemented it. Measured on this machine: widget.log reached 37.2 MB and island.log 18.0 MB over 38 days, with widget.log growing ~3.6 MB/day during active use. Extrapolated, that is over a GB a year in %APPDATA%, and nothing ever trimmed it. Both writers now cap at 1 MB and keep the last 300 lines. The size check is counter-gated (fires once per 256 KB written) so the 400 ms poll path does not stat the file on every call. 3. Tool input carrying credentials was persisted verbatim. Round-19 #1 redacted Computer Use keystrokes because they can be passwords. The Bash branch was never redacted, so `curl -H "Authorization: Bearer ..."` or `export API_KEY=sk-...` reached all three sinks -- status.json, the append-only log, and the always-on-top pill -- in the clear, and the log kept it indefinitely. There are two independent producers of that text: the hook path (_lib.ps1) and the detector, which re-derives messages from mcode's session log rather than from a hook event. A helper wired into only one is a silent half-fix -- the pill looks clean while the detector keeps writing raw text to disk. Both now route through a shared scripts/lib/Protect-Text.ps1, and smoke check 9 asserts BOTH consumers invoke it rather than merely defining it. Scope is deliberately credential-shaped runs, not whole commands: the command text on the pill is a documented feature, and blanking every command would trade a real capability for a marginal privacy gain. Also adds a regression guard for hook invocations under an install path containing spaces. mcode 0.5.4 fixed its own managed updater for exactly that case; ${PLUGIN_ROOT} is substituted at runtime, so the only way to break a spaced path is to hand the shell a joined string instead of an argv array. hooks.json already uses the array form, so this is a guard, not a fix. Validation ---------- - smoke: 115 pass / 0 fail (7 pre-existing "forward" event warnings). Baseline before this commit was 108 pass / 0 fail; the 9 new failures were the new checks, observed failing first. - Negative-first: each check was confirmed to FAIL before its fix landed, and check 9 was re-verified by deleting the detector's call site and watching it fail, then restoring. - Protect-SecretText: 16/16 cases, covering 10 credential shapes (bearer header, sk-, ghp-, bare JWT, JSON quoted key incl. "x-auth-token", single-quoted JSON, --password flag, key=value, the plugin's own set-token.ps1 invocation) and 6 near-miss inputs that must NOT be redacted (--mode production, "Select-String token", Windows paths, --secret-config, --author=, {"author":"Bob"}). - Log cap: wrote 3.4 MB of input through the new Log-Line body; final file 0.64 MB / 7064 lines. Cap held across repeated truncation. - PowerShell parser check clean on all five modified/new .ps1 files. - The 5h call was verified live against the real endpoint, not assumed: Get-5hUsage returns a real remainingPct with the configured token. Not included ------------ The context-meter and token-speed readouts proposed in the same audit are deliberately NOT in this commit. Structurally enumerating the session store (excluding the session this audit was run in) shows the assistant message carries model / usage.{input,output,cacheRead,cacheWrite, totalTokens,cost} and nothing else: there is no contextWindow and no tps field. usage.totalTokens is a usable context numerator, but the window size lives in mcode's provider config, and a token-rate derived from message timestamps would fold in thinking and tool time -- which 0.5.6 explicitly excludes from the TUI's own figure. Shipping either without that denominator would put a number on the pill that disagrees with the TUI. Left for a follow-up that can source the window size properly. * fix(mcode-island): every non-ASCII .ps1 needs a UTF-8 BOM for PS 5.1 (round-20 #4) Found while landing round-20: syncing the committed tree into the install directory with the documented `Copy-Item -Recurse -Force` flow produced a detector that could not start at all. Root cause, verified against Windows PowerShell 5.1 rather than inferred: Windows PowerShell 5.1 decodes a BOM-less script using the system ANSI codepage. On a zh-CN host that is GBK, so a script containing non-ASCII comments is mis-decoded. The C# here-string in mcode-status-detect.ps1 then stops being a here-string, `using System;` is parsed as PowerShell, and the script dies before its first statement with 92 lines of "missing using directive". The reported line number (38) is offset from the real one (46) because the mis-decode also desynchronizes the tokenizer's line count. mcode-island.ps1 never showed this because it happens to carry a BOM. Nothing else in the plugin did. The trap is that .gitattributes pins `*.ps1 text eol=lf`, and its comment claimed "PowerShell 5.1 reads CRLF fine, so LF avoids both failure modes". That is backwards, and the comment is what let the bug survive. The policy is part of the cause, not a mitigation. Control experiment, to be clear that this is pre-existing and not a round-20 regression: the detector as of ea96d75 (before any of this round's work) fails under 5.1 with byte-identical output. It simply had never been synced from a clean checkout on this machine before. Why a BOM rather than flipping .gitattributes to CRLF: Both were tested against 5.1 and both fix the parse -- bare LF does not. CRLF would re-break the Linux-side validate.mjs CRLF handling that the same comment was written to avoid, so the BOM is the narrower fix: three bytes at the head of the file, line-ending policy untouched. Validation ---------- - Control: ea96d75's detector, run under 5.1, fails identically. Not a regression introduced here. - Isolated: the same file, byte-identical except for a prepended BOM, parses cleanly under 5.1 and runs to completion (the only remaining error was a TEMP-path artifact of the harness, since $PSScriptRoot had no scripts/lib beside it). - Also confirmed: CRLF without a BOM also parses, so bare LF is the trigger, not the encoding per se. - smoke check 11: written first, observed FAIL on exactly the 8 affected files, then PASS after the BOMs landed. 116 pass / 0 fail. - All 20 non-ASCII .ps1 tokenized with the 5.1 PSParser: 0 parse errors (was 8 files failing before). Files given a BOM (8): mcode-status-detect.ps1, set-token.ps1, _lib.ps1, post-tool-use.ps1, pre-compact.ps1, session-end.ps1, stop.ps1, test-windows-workflow-local.ps1. .g gitattributes now documents the real constraint, states that bare LF is part of the problem, and warns that the BOMs must not be "cleaned up" -- an editor that drops them breaks every sync. Note for reviewers: this changes the first byte range of eight files, so the diff reads as one changed line each. That is the BOM and nothing else. * fix(mcode-island): a detector state write clears a stale sub-step (round-20 #5) Live symptom: the pill sat on "step 1/1" showing a curl command from a tool call that finished several turns earlier, and nothing ever cleared it. Cause: round-19 #2 made Write-Status merge the previous step/total/detail back in on every write, which is correct for the two metadata refreshes (60s 5h-usage, todo progress) because those restate the current state rather than announcing a new step. But the detector also writes on state inference -- it read a new tool call out of the session log and is asserting "the agent is doing THIS now". That message has no relationship to whatever sub-step a hook pushed last, so it inherited a counter that had no meaning there. Every subsequent write inherited it again, which is why it never cleared. Write-Status now takes -KeepSubStep: - state inference (default) resets step/total/detail to -1/-1/"" - the 5h-usage and todo refreshes pass -KeepSubStep and preserve The two paths are distinguished explicitly by the caller, not inferred from the message text. Distinguishing is necessary because the state-inference call also has to be able to win the settle race against an agent push (round-19), so "who wrote last" is not a usable signal. Tests ----- New section 5 in test-substep-progress.mjs, negative-first: observed failing on 3 assertions before the fix, all passing after. - Write-Status exposes -KeepSubStep - state-inference write clears a stale sub-step (-1|-1|) - metadata-refresh write still preserves it - the inference call site does NOT pass -KeepSubStep - exactly 2 call sites DO (5h-usage, todo) The call-site assertions are the ones that matter: a switch nobody passes is the dead-code shape, and a test that only exercises the function body would not notice. Two harness repairs, both regressions from round-20 that this work surfaced: - Format-ToolSummary now calls Protect-SecretText, which lives in a separate lib. The harness extracted the function body and ran it standalone, so it died on an unresolvable command rather than on an assertion. The harness now dot-sources the lib, as _lib.ps1 does. This is a genuine break of an existing test that round-20 shipped; smoke did not catch it because smoke is static and this suite is not. - Section 3 anchored its extraction regex on `$prev` appearing before `$detailField`. Wrapping the block in `if ($KeepSubStep)` inverted that order and the regex silently stopped matching, which would have been reported as "Write-Status preserves sub-step fields: could not find the block" -- a confusing failure for a function that works. It now anchors on the initialisers and the payload construction. Validation: sub-step suite 32 pass / 0 fail, smoke 116 pass / 0 fail. Detector re-verified under Windows PowerShell 5.1 (exit 0) after the restart, since that is the interpreter this runs under in production. * fix(mcode-island): the pill never renders raw tool JSON (round-20 #6) The running-state message was built as "$verb " + (ConvertTo-Json $args -Compress) truncated to 60 chars so the always-on-top pill showed things like Running {"command":"$ErrorActionPreference=\u0027Continue\u0027\n... Reading task {"task_id":"bg_0f858500-...","wai... Escaped quotes, a JSON key, and a truncation that lands mid-token. It was the noisiest thing on screen and the least informative. The `done` and `fail` branches were always fine -- they render the verb alone. Only the `running` branch serialised arguments. Format-ToolArgs now returns the single most informative field per tool: command for shell, file_path for read/write/edit, pattern for grep/glob, query for web_search, url for web_fetch, name for skill, task_id for the task-output family. Unknown tools get a generic sweep of the common field names before giving up. `skill` is not in $TOOL_ACTIONS or $TOOL_FAMILIES, so it always took the JSON path -- one of the most frequent tools on screen was the least readable. That is the case that made this worth fixing rather than tuning. There is no fallback to JSON. If Format-ToolArgs returns nothing the pill wears the bare verb: "Using" reads fine, "Using {\"name\":...}" does not. An unknown tool should degrade to less information, never to a dump. Also fixes the display of multi-line commands. A PowerShell command is frequently several lines; the old code collapsed and truncated the whole thing, so the visible text was usually the middle of a script fragment. It now takes the first non-empty line. Redaction is unchanged and still applied on this path -- the summary runs through Protect-SecretText exactly as the JSON did, so a credential in a command still cannot reach the pill or island.log by this route. Tests: 16 new assertions in test-substep-progress.mjs, negative-first. The blanket rule is asserted per case: no result may contain a JSON key, an escaped quote, or a \uXXXX sequence. Cases cover bash, multi-line bash, read/write/edit, grep, glob, web_search, web_fetch, skill, task_output, an unknown tool with an unrecognised argument shape, and a bearer token inside a command. Validation: sub-step suite 47 pass / 0 fail, smoke 116 pass / 0 fail, all 30 .ps1 parse, and the detector still initialises cleanly under Windows PowerShell 5.1 (exit 0) -- which is the interpreter it runs under in production, and the one that breaks on a malformed file. One trap worth recording, because it produced a very misleading failure: the second parameter is deliberately NOT named `$args`. That is a reserved automatic variable in PowerShell, and binding a parameter to it yields the unbound-argument Object[] rather than the object passed in. The symptom was an empty summary for every single tool, which reads as "no data" rather than "bad binding". Verified directly: function T($n, $args) { $args.command } T bash ([PSCustomObject]@{ command = 'npm test' }) # -> '' * fix(mcode-island): a metadata refresh no longer steals state ownership (round-20 #7) Reported as: the pill stops on "Command failed" at the end of a turn. `source` is not a label -- it is the arbitration token. The detector's settle logic reads `source == detector` as "this state is mine to rewrite", and uses that to decide whether it may overwrite an agent push. But Write-Status stamped `source = detector` unconditionally, including on the two metadata refreshes (60s 5h-usage, todo progress) that exist only to refresh numbers on a state somebody else owns. The sequence, from island.log on the dev machine: 16:06:24 [detect] error :: Command failed 16:06:25 error :: Command failed <- PostToolUse hook, source=agent 16:06:40 [detect] 5h usage refreshed <- stamps detector over the agent's state 16:06:41 error :: Command failed 16:07:03 [detect] idle :: waiting <- 60s no-event fallback, finally At 16:06:40 the refresh handed write authority back to the detector while leaving `state = error`. From that point the detector owned the state and re-derived `error` from the same stale failed toolResult on every poll -- there is nothing newer in the session log to supersede it. When the turn ended and the Stop hook pushed `done`, the detector was entitled to overwrite it, and the pill went back to red. Nothing cleared it until the 60-second no-event fallback happened to fire. So the error was accurate when it appeared and wrong the moment the turn moved on. The user-visible symptom is a pill that is stuck on a failure that is no longer the current state. A metadata refresh now preserves the previous owner. The claim-ownership stamp is reserved for state-inference writes, which is the only path that actually asserts a new state. This is the same distinction #5 drew for step/total/detail; `source` belongs to the observable state, so it follows the same rule. Tests: 2 new assertions, negative-first, both observed failing before the fix. They assert the ownership token in both directions -- a metadata-refresh write keeps `agent`, a state-inference write claims `detector` -- because a test that only checks one side would pass just as happily if the field were always blank. Validation: sub-step suite 49 pass / 0 fail (was 47), smoke 116 pass / 0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows PowerShell 5.1 (exit 0). * fix(mcode-island): skip the assignment preamble in shell summaries (round-20 #8) Round-20 #6 stopped the pill from rendering raw JSON, but the result was still frequently uninformative: Running $ErrorActionPreference='Continue' That is the opening line of most agent commands, and it says nothing about what is being worked on. On a detached status pill it is close to indistinguishable from the pill being stuck -- which is not a good look for a widget whose entire job is to answer "what is it doing right now". Format-ToolArgs now prefers the first line that actually EXECUTES something, skipping variable-assignment and comment lines: $ErrorActionPreference='Continue' <- skipped $env:TEMP="..." <- skipped npm test <- shown The fallback matters: if every line is boilerplate, the first line is still shown rather than leaving the pill blank. A summary that is uninformative beats no summary at all, because the bare verb alone cannot be distinguished from the detector having lost track. Comment lines are skipped on the same rationale. Tests: 3 cases, negative-first, all observed failing before the fix -- the preamble was shown, and the comment line was shown. A fourth case (an all-assignment command) was written at the same time and passed before the fix as well, which is the point: it pins the fallback so a future "skip more lines" change cannot silently blank the pill. Validation: sub-step suite 51 pass / 0 fail (was 49), smoke 116 pass / 0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows PowerShell 5.1 (exit 0). * fix(mcode-island): the detector survives a terminating error in the main loop The main loop had no catch. Any terminating error escaped the loop, fired the finally block, logged 'detector stopped' and exited the process. The widget kept running, so the pill just froze on the last status.json with no visible cause -- status.json silently stopped changing while widget.log kept logging TOPMOST re-raises. This is the same failure family the round-21 review flagged for the status.json write path (a shared staging file let one process rename another's half-written file into place, and the resulting Move-Item failure killed the daemon). The lock in the next commit removes that specific trigger; this commit makes the daemon survive the ones we cannot enumerate. The catch logs the exception type as well as the message, so 'loop error' alone does not have to be disambiguated between a JSON parse, a WMI failure and an IO lock. The message is truncated to 300 characters so a runaway error cannot flood island.log every tick. Validation - the guard is a try/catch around the existing loop body; the loop's own logic is untouched - the detector still initialises cleanly under Windows PowerShell 5.1 (exit 0), the interpreter it runs under in production - all .ps1 files in the plugin still parse without syntax errors Design compliance no credentials / no network / no telemetry / no third-party services the change only adds an error handler around existing local work * fix(mcode-island): status.json writes are atomic across processes Round-21 review (hetaoBackend, 2026-10-09) blocking issue #1: the detector's metadata refresh was not an atomic read-modify-write. The loop read old state/message (mcode-status-detect.ps1:923), then Write-Status -KeepSubStep performed a SECOND independent read for step/detail/source (:714-721) before overwriting the file (:723-739). An agent write between the two reads produced a mixed-generation payload, and a write after the second read was lost. There was a second writer behind that, and it made the outcome worse than a stale payload. notify-island.ps1 staged status.json through "$statusFile.tmp" and the detector staged through the same path. Two processes, one temp file: one could rename the other's half-written file into place, and the loser then failed its Move-Item. The detector runs under $ErrorActionPreference = 'Stop' with no catch around the write, so that failure killed the daemon outright -- the pill froze on the last status.json with no visible cause. (The preceding commit adds the per-tick guard that stops the daemon dying on ANY terminating error; this commit removes this particular trigger.) ## What changed scripts/lib/Status-Store.ps1 (new) The one place the status.json protocol lives, shared by both writers: Enter-StatusLock / Exit-StatusLock An exclusive cross-process lock on a sibling lock file, opened with FileShare.None. That is a real OS file lock, not a flag file whose presence we poll -- a flag file has the same check-then-act hole the payload has. If a holder dies, the OS releases the handle. Read-StatusSnapshot One read of the whole status object. Callers read once and reuse that snapshot. Write-StatusSnapshot Atomic replace (staging file + rename) that stamps the monotonic `version` counter and `ts`. mcode-status-detect.ps1 Write-Status now takes the lock, reads the snapshot ONCE, and writes. The body moved to Write-StatusUnderLock so the arbitration path can read the snapshot and act on it without taking the lock twice (the lock is not re-entrant: a nested take would deadlock against itself). The arbitration read and the write it authorises are now one critical section. Previously the read sat outside the lock, so a hook push could be judged against a stale snapshot and then overwritten by the write that stale snapshot had authorised. -KeepSubStep became -MetadataRefresh. The old switch only said "preserve the sub-step"; the call site still had to read the file itself to restate the state, which is where the second read came from. One switch now names the whole semantics: a metadata refresh restates the current state, preserves the sub-step fields and preserves the state owner. A state-inference write resets them and claims ownership. The two metadata call sites no longer touch the file at all. Read-StatusObj is gone. It read status.json without the lock, and it was the read that made a lost update possible. notify-island.ps1 The agent-side writer now uses the same lock, the same snapshot and the same staging file. caller.json moved inside the same critical section, so its staging path is no longer shared between two hook processes that can run at once. ## Two defects found while doing this 1. The replace could lose a sharing violation. MoveFileEx with REPLACE_EXISTING fails when the target is open by a reader that did not share delete, and the widget polls status.json every 400 ms with Get-Content, which opens with FileShare.Read. A write landing in that window threw and the update was lost -- the staging file survived, the target kept the old payload. Measured on this host, not inferred. The replace now retries briefly; the reader holds the file for microseconds, so one short retry clears it. 2. The write path returned the version, and the detector calls it from inside its main loop -- so the daemon's stdout got a bare number on every write. The store's write function returns nothing now, and a check pins that. ## Test evidence (negative-injection verified) The behavioral suite now runs the REAL Write-Status against a REAL status.json in an isolated %APPDATA%. The old harness stubbed the reader and pasted a fragment of the function body into a synthetic script -- which could not tell a working implementation from one that read the file twice, because the second read was exactly what the stub hid. That is the bug the review named, and the old test could not see it. test-substep-progress.mjs: 84 pass, 0 fail (was 51) smoke.mjs: 125 pass, 7 warn, 0 fail (was 116) New: two real PowerShell processes interleaving on one status.json. Writer A is the detector's read-modify-write, writer B is the agent's full push. A signals that it has read, then waits for B to signal that it is about to write; B's signal is published BEFORE B takes the lock, so the ordering is deterministic either way. with the lock A holds it, B blocks, A writes, B writes last -> the file carries B's values without the lock B writes immediately, A writes 400 ms later from its stale read -> the file carries A's stale values and B's write is gone The version counter is a second, independent witness: both writers derive it from the snapshot they read, so two writers that read the same generation mint the same version. 9 negative-injection cases, each a minimal change that breaks one clause of the contract. All 9 turn the suite red; the baseline restores to 80/0 after each: N1 lock removed from the write path -> 2 fail N2 the write path reads the file twice -> 2 fail N3 metadata refresh stops preserving sub-step -> 4 fail N4 state-inference stops clearing the sub-step -> 4 fail N5 the version counter stops advancing -> 3 fail N6 notify-island stages through a shared path -> 1 fail N7 the inference site passes -MetadataRefresh -> 1 fail N8 notify-island stops taking the lock -> 1 fail N9 metadata refresh stops preserving the owner -> 1 fail N6 and N8 did not bite on the first pass, and the reason was worth fixing rather than papering over: N6's check looked for the literal "$statusFile.tmp". A rewrite that spells the same shared path differently walks straight past it, so the suite stayed green while the bug was back. The check now asserts the POSITIVE contract -- status.json may only be written through the shared store -- instead of one spelling of the bug. N8's patch did not apply, because the indentation in the patch text did not match the file. A setup miss is reported as a setup miss, not as a pass: counting it as green would have been a false negative in the harness itself. The two layers are bound together explicitly. Section 7 proves the PROTOCOL -- two real processes, one file, no lost write -- using hand-written writers, because the deterministic choreography needs the read to be separable from the write, and the real code deliberately does not separate them. The static checks are what say that the real detector code goes through that protocol: the arbitration block takes the lock and writes through the locked body, and no other call site writes at all. Also verified: the detector still initialises and writes under Windows PowerShell 5.1 (exit 0, the interpreter it runs under in production), and the local Windows runner passes all 4 steps, including the real PreToolUse hook path through the new locked write. ## Design compliance no credentials : none added; the lock is a local file handle no network : no network calls added no telemetry : none added; island.log unchanged no third-party svcs : no new dependencies; pure PowerShell cross-platform : no host paths introduced; the existing hardcoded-path scan still passes atomic write : staging + rename, now under an exclusive lock schema : additive. `version` is a new optional field; the widget reads every field through …
What changes
Adds the hosted Plugin
plugins/Hylouis233/cli-agent-bridge: a dependency-free stdio MCP serverand same-name Skill that let MiniMax Code delegate bounded coding tasks to locally installed
Claude Code, Codex CLI, Kimi Code, ZCode, or DSH processes inside a Git repository, then review
the resulting workspace and commit changes.
The server exposes three tools:
list_backendsreports configured CLIs, availability, versions, and experimental notes.workspace_statusreturns a bounded Git snapshot: status, staged/unstaged diff stats, changedfiles (including untracked paths), HEAD and symbolic HEAD, and refs.
delegate_taskruns one backend and returns bounded output, before/after Git snapshots, movedrefs, and an attributed commits block. Dirty worktrees are rejected unless
allowDirty=true;unsupported resume requests fail instead of silently starting a fresh session.
Safety and concurrency behavior
linked worktrees, independent MCP clients, and bridge processes. Independent clones can run in
parallel.
enters the target repository. Lease acquisition and recovery use compare-and-swap owner records;
uncertain worker termination fails closed behind a shared quarantine marker.
post-run attribution. The supported Windows path uses a kill-on-close Job Object and confirms
tree termination before releasing the repository lease.
fetched history, more than 256 baseline tips, linked worktrees, submodules, unusual path bytes,
and staged/untracked changes without presenting truncated evidence as complete.
repeated identities, and fails commit attribution closed when a POSIX realpath cannot be proven.
CLI uses the user's own authentication and may send task/workspace data to its provider.
Requirements and platform disclosure
PATH; the target workspace must be a Git repository.fail closed before backend configuration, workspace access, Git resolution, or executable
probing because this dependency-free server cannot prove containment after an arbitrary local
worker asks a host service to launch outside its original process tree.
Codex, and Kimi templates are non-experimental; ZCode and DSH remain explicitly experimental.
Latest review follow-ups
Current head also closes the review gaps left after the earlier Copilot follow-up:
rejected
realpathpromise unhandled;closed;
context.skip();canonical 40-character
actions/checkout@v7.0.1commit SHA.Verification
Verified locally on current head
902b194:npm run validate: all 9 hosted Plugins and both examples validated.The skips are Windows-only or other OS-specific fixtures on the macOS verification host.
unresolved-identity fail-closed behavior, successive fetch detection, and partial-fetch
uncertainty.
node --checkpasses for the server and integration test file.actionlintreports zero diagnostics for.github/workflows/ci.yml.git diff --checkis clean.The upstream PR currently has no GitHub-hosted Windows run for this head. The workflow is present
and statically validated, but no hosted runner result is currently available. No skipped
[code]smithcheck is being presented as test evidence.Submission checklist
behavior, and experimental backends are disclosed.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.