Repository navigation
Conversation
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/MiniMax-AI#5/MiniMax-AI#18/MiniMax-AI#20/MiniMax-AI#21 round-4 convention.)
834544c to
ca395b6
Compare
|
Added second commit on top of
Diff stat for just this commit: +120 / -33 across 2 files (mcode-island.ps1, smoke.mjs). Upstream can squash or keep the two commits separate — they are different features (sub-step schema vs click toggle) with different blast radius, so I left them as two commits for review clarity. |
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 MiniMax-AI#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.
88bc7f8 to
e2f0dd3
Compare
|
Updated commit Behavior change:
Drift lock updated accordingly: smoke.mjs now requires PR comment Local validation:
|
…ug (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
round-15 fix: toggle show uses
|
…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 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
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head b73752ba9bd23d2b5f5387e3660941e037520c0e.
Blocking issues:
plugins/antianqi/mcode-island/io.minimax.mcode/hooks/scripts/_lib.ps1:110-118includes Computer Usetool_input.textin the status summary. PreToolUse persists/displays it throughpre-tool-use.ps1:10-12,notify-island.ps1:115-124, andmcode-island.ps1:463,541-544. Passwords, tokens, verification codes, or other typed secrets can therefore appear in the always-on-top pill,status.json, and append-only log. Never include typed text in persistent/display summaries; add secret canaries.- Detector rewrites rebuild the entire status object without
step,total, ordetail(mcode-status-detect.ps1:398-419,588-612), so normal usage/todo refreshes erase the new sub-step state. Merge/preserve these fields and add an end-to-end writer-interleaving regression. - The restore
SetWindowPoscall (mcode-island.ps1:675-676) usesSWP_NOSIZE|SWP_NOZORDERbut omitsSWP_NOMOVE; with X/Y=0 it can move a restored secondary-display window to(0,0), whileSWP_NOZORDERalso ignoresHWND_TOP. Correct the flags and test nonzero monitor origins. - PostToolUse passes
-Detailwith defaultStep=-1, but rendering returns only the base message whenStep <= 0(mcode-island.ps1:417-426), so the advertised visible detail is never shown. Align implementation, tests, and docs. scripts/test-substep-progress.mjsis not invoked by the exact-head Windows workflow and its PowerShell lookup unconditionally returns a user-specific path instead of falling back to PATH. Wire the test into CI and fix executable discovery.
The existing parse/token/hook/mock-API Windows job, Ubuntu validation, and CodeQL are green, but they do not cover these new behaviors. [code]smith is skipped and is not evidence.
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.
Round 18 — tool-verb messages + family tinting
The problemThe pill showed the bare tool name for every phase:
This port applies the same idea to the pill. What the pill shows now
Plus family tinting, so the dot, pulse ring, and progress bar all shift color by what kind of work is happening:
Family color applies to Implementation notes
Validation
16 negative-injection cases, each a minimal change that breaks one clause of the contract. All 16 turn the suite red; baseline restores to 98/0 after each. Three of the 16 initially did not go red. Those were false greens, and the checks were rewritten rather than the failures being waved off:
Design compliance
|
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.
Follow-up:
|
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.
ask_user now reports
|
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.
…e (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.
…DER (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.
…(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.
MiniMax-AI#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.
All five blocking items addressedFive commits, one per review item, so each can be verified independently.
#1 — #2 — #3 — two separate bugs on the restore path. #4 — the detail can now render. #5 — the suite is wired. It is now the only suite that executes the PowerShell rather than grepping for substrings, which matters because it is where the canary lives. Two things I got wrong while fixing this, both caught by negative injectionA false green in my own new test. The A blind spot the flag check cannot see. Passing Validation
One note on the CI step: on a runner whose console code page is not UTF-8 the suite fails on correct code unless the generated |
…iniMax-AI#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.
…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.
…(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.
…und-20 MiniMax-AI#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.
Round 20 - five defects found auditing this plugin against the mcode 0.5.0-0.5.8 changelogThree commits on top of the round-19 head ( #1 - the 5h-usage disclosure named a host the code never calledThis one is a correctness problem for the disclosure itself, so I want to lead with it.
But the trailing comment on that same line said
That claim is false. The code never contacted that host at all. For a plugin whose proposal rests on four independent disclosure sections, an overstated isolation guarantee is worse than no guarantee - a reviewer checking host consistency finds code and docs disagreeing and is right to distrust both. The new check decodes the byte array and compares the docs against that decoded truth. It deliberately does not hard-code the endpoint, so it cannot rot into a second copy of the disclosure it is policing, and it fails the moment the two drift apart again. #2 - both append-only logs had no size cap, ever
Measured on my machine over 38 days: Both writers now cap at 1 MB and keep the last 300 lines. The check is counter-gated (fires once per 256 KB written) so the 400 ms poll path does not stat the file on every call. Verified by pushing 3.4 MB through the new #3 - tool input carrying credentials was persisted verbatimRound-19 #1 redacted Computer Use keystrokes, correctly, because they can be passwords. The Bash branch was never touched, so a command like The subtlety worth flagging for review: there are two independent producers of this text. The hook path builds it from the hook event; the detector re-derives it from mcode's session log. A helper wired into only one is a silent half-fix - the pill looks clean while the detector keeps writing raw text to disk. So the helper lives in a shared Scope is deliberately credential-shaped runs rather than whole commands. The command text on the pill is a documented feature ( 16/16 cases, including 6 near-miss inputs that must not be redacted ( #4 - every non-ASCII
|
| Step | Result |
|---|---|
1 parse all .ps1 |
30/30 |
2 token set/show/clear (isolated APPDATA) |
8/8 |
| 3 hook stdin/stdout under PS 5.1 | 5/5 |
4 Get-5hUsage via lib + mock listener |
5/5 |
| 5 behavioral suite | 32/0 |
Steps 2 and 3 were the ones at real risk from the BOM and the new dot-source; both run green.
Not in this PR, on purpose
The context-meter and token-speed readouts proposed during the same audit are excluded. Structurally enumerating the session store shows the assistant message carries model and 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 rate derived from message timestamps would fold in thinking and tool time, which mcode 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. I would rather leave the pill without the readout than show a figure that is quietly wrong.
…iMax-AI#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' }) # -> ''
Round 20 #6 - the pill no longer renders raw tool JSONReported as: the status line always trails some code. The so the pill showed: A JSON key, escaped quotes, and a truncation that lands mid-token. It was the noisiest thing on screen and the least informative. The Before / after, same command:
There is no fallback to JSON. If This also fixes multi-line commands. A PowerShell command is usually several lines, so the old collapse-then-truncate showed 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 Tests16 new assertions, negative-first. The blanket rule is asserted per case: no result may contain a JSON key, an escaped quote, or a One trap, because it produced a genuinely misleading failureThe second parameter is deliberately not named function T($n, $args) { $args.command }
T bash ([PSCustomObject]@{ command = 'npm test' }) # -> ''The symptom is an empty summary for every tool, which reads as "no data" rather than "bad parameter binding". It cost me one debug cycle and would have read as a data problem to anyone else. Validation
One thing I checked and did not change
|
…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).
Round 20 #7 - a metadata refresh no longer steals state ownershipReported as: the pill stops on
The sequence, from At 16:06:40 the refresh handed write authority back to the detector while leaving So the error was accurate when it appeared and wrong the moment the turn moved on. The pill shows a failure that is no longer the current state, and there is no signal that would clear it. 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 Tests2 new assertions, negative-first, both observed failing before the fix. They check the ownership token in both directions -- a metadata-refresh write keeps Validation: sub-step suite 49 pass / 0 fail (was 47), smoke 116 pass / 0 fail, all 30 |
…ound-20 MiniMax-AI#8) Round-20 MiniMax-AI#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).
Round 20 #8 - the pill skips the assignment preambleFollow-up to #6, from using the result. #6 stopped the pill rendering raw JSON, but the result was still often uninformative: That is the opening line of most agent commands and 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 a poor look for a widget whose entire job is to answer "what is it doing right now".
Verified live against the running detector with exactly that shape of command: The fallback is the part worth arguing aboutIf every line is boilerplate, the first line is still shown rather than leaving the pill blank. An uninformative summary beats no summary, because a bare verb with no detail is indistinguishable from the detector having lost track of the session. That reasoning is pinned by a test that was written alongside the failing ones and passed before the fix as well - an all-assignment command. It exists so a future "skip more line shapes" change cannot silently blank the pill. Tests that only ever fail before the fix do not constrain the fallback. Validation: sub-step suite 51 pass / 0 fail (was 49), smoke 116 pass / 0 fail, all 30 |
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head a6a17f3eba855e1fe563db0a2948ab3cbfdf65d6.
The typed Computer Use secret path, Win32 flags, visible PostToolUse detail, workflow wiring and PowerShell discovery are improved, but blockers remain:
- Detector metadata refresh is not an atomic read-modify-write. The loop reads old state/message (
mcode-status-detect.ps1:923-943), thenWrite-Status -KeepSubStepperforms a second independent read for step/detail/source (:714-721) before overwriting the file (:723-739). An agent write between the reads produces a mixed-generation payload, or a write after the second read is lost. Use one consistent snapshot plus version/CAS/lock and add a real two-writer interleaving regression. - PostToolUse error classification stringifies the result and only recognizes textual error prefixes (
io.minimax.mcode/hooks/scripts/post-tool-use.ps1:14-24). A structured{isError:true,...}result can be reported asdone. Prefer the structured boolean and test object/string/null success and failure payloads through the real hook subprocess. - The local Windows fallback runner still executes only four steps and omits
test-substep-progress.mjs, so local reproduction differs from the GitHub workflow. Add the behavioral suite and fail on nonzero exit.
Exact-head Ubuntu, Windows and CodeQL checks are green, but existing tests do not cover the interleaving or structured-result paths. [code]smith is skipped and is not evidence.
…ain 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
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 PSObject.Properties, so an older widget ignores it and a newer one tolerates its absence. The sentinel convention is unchanged: absent means -1 / -1 / "". ASCII-only : the new lib is ASCII-only, so it needs no BOM and cannot trip the PS 5.1 codepage trap
Round-21 review (hetaoBackend, 2026-10-09) blocking issue #2: "PostToolUse error classification stringifies the result and only recognizes textual error prefixes (io.minimax.mcode/hooks/scripts/post-tool-use.ps1:14-24). A structured {isError:true,...} result can be reported as `done`. Prefer the structured boolean and test object/string/null success and failure payloads through the real hook subprocess." The old code did: $s = [string]$result if ([string]::IsNullOrEmpty($s)) { $isError = $true } elseif ($s -match '^\s*(Error|ERROR|...|\[ERROR\])') { $isError = $true } A structured result stringifies to "@{isError=True; content=...}", which matches no prefix -- so a FAILED tool was reported as a SUCCESS and the pill showed green. The detector already read the structured boolean ($msg.isError -eq $true) from the session log, so the two producers of the same state disagreed about what a failure was. ## What changed io.minimax.mcode/hooks/scripts/_lib.ps1 New Test-ToolResultError. The order matters: the structured boolean wins, because it is what the producer actually set. The textual heuristic is only a fallback for a result that arrives as a bare string. A result that is a JSON string is parsed and its flag read, because a producer that serialises the result before handing it to the hook is still telling us the truth in a structured field. A null result and an empty string remain failures: the tool produced no result at all. io.minimax.mcode/hooks/scripts/post-tool-use.ps1 The inline classification is replaced by the shared helper. The header comment said "if the tool_result is empty or matches an error pattern", which is now no longer what the code does. ## Test evidence (negative-injection verified) 10 cases, every one of them through the REAL hook as a REAL child process, with a real event on stdin, reading back what the hook wrote to status.json. A function-body test would not prove the hook wires the classification to the state it pushes. structured failure {isError:true} -> error structured success {isError:false} -> done structured failure, nested {isError:true, content:{...}} -> error structured success, nested {isError:false, content:{...}} -> done string with an error prefix "Error: command not found" -> error plain string "all good" -> done JSON string with the flag '{"isError":true,...}' -> error JSON string with the flag '{"isError":false,...}' -> done null result null -> error empty string result "" -> error Each case also asserts the pushed message ("Bash failed" / "Bash ok"), because a correct state with a wrong message is still a broken pill. Each case runs in its own isolated %APPDATA%, so the assertion cannot read a previous case's status.json. test-substep-progress.mjs: 116 pass, 0 fail (was 84) 3 new negative-injection cases, each reverting one clause: N10 post-tool-use goes back to string-only classification -> 7 fail N11 the structured isError flag is ignored -> 4 fail N12 the structured failure is classified as a success -> 8 fail The harness around this had two of its own faults, both fixed rather than worked around: - The suite waited for writer A's marker with an UNBOUNDED spin. If A died first, the suite hung instead of failing, and a hanging test looks like a stuck CI job rather than a broken contract. The wait is bounded now, and the writers are bounded too. - The negative-injection harness ran the suite in the same process as itself, so the harness's own error handling could change how the suite's output was treated. It runs in its own job now, with a timeout, so a hung suite becomes a TIMEOUT verdict instead of a stuck harness. ## Design compliance no credentials : none added; the helper only inspects a flag no network : no network calls added no telemetry : none added no third-party svcs : no new dependencies; pure PowerShell cross-platform : no host paths introduced ASCII-only : the new helper adds one non-ASCII character that was already in the file (the error glyph), so the existing BOM stays required and unchanged
Round-21 review (hetaoBackend, 2026-10-09) blocking issue #3: "The local Windows fallback runner still executes only four steps and omits test-substep-progress.mjs, so local reproduction differs from the GitHub workflow. Add the behavioral suite and fail on nonzero exit." The GitHub workflow has had step 5 since round-19 MiniMax-AI#5. The local runner did not, so the two paths were not the same test. That is worse than having no local runner at all: a runner that claims to mirror CI while running fewer steps than CI gives the person running it false confidence about what CI will say. The behavioral suite is the only suite that executes the PowerShell under test. It carries the secret canary (a typed Computer Use password must never reach status.json or the append-only log) and the two-writer interleaving regression (a status.json write must not lose a concurrent write). Neither of those was reachable from the local path. ## What changed scripts/test-windows-workflow-local.ps1 Step 5 added, 1:1 with .github/workflows/mcode-island-windows.yml step 5: resolve the suite against the repo root, run it with node, and fail on a non-zero exit code. The PowerShell report line is kept because it answers the question a failing run raises first: which interpreter did the suite use. The suite does its own resolution and falls back to PATH; it never assumes a user-specific absolute path. The repo root is now resolved from this script's own location instead of from the current directory. The workflow resolves it the same way (from $GITHUB_WORKSPACE). The old version only worked when invoked from the repo root -- from anywhere else it died at step 1 with a path error, which is a runner that silently reproduces nothing rather than a runner that reproduces CI. The header comment listed four steps; it now lists five and says why step 5 exists. ## Validation The local runner now executes all 5 steps and exits 0, with the behavioral suite reporting 116 pass, 0 fail inside it. The suite takes about 85 s on this machine, so the runner takes roughly two minutes. The GitHub job allows ten. ## Note on a cost this exposed, not fixed here The suite's ten PostToolUse cases run the real hook, and the hook pays for the caller tracking: Get-CallerFocusInfo asks WMI for the process chain, one query per level. Where the hook is spawned without a console -- a CI runner, a detached service -- GetConsoleWindow returns zero and the whole chain is walked, which measured 6.5 s per hook invocation on this host (Add-Type is 97 ms and a single CIM query is ~500 ms, so it is the chain, not the compile). Where a console IS inherited, the first branch returns immediately. That is a real latency cost for any deployment without a console, but it is not what the review asked about and it is a separate change with its own blast radius, so it is left alone here rather than folded into a commit about something else. The suite is inside the CI budget as measured, so the wiring stands either way.
Round 21 — the three remaining blockersHead
#1 — the metadata refresh was not an atomic read-modify-write
Agreed, and there was a second writer behind it that made the outcome Both writers staged through the same What the fix is. A new
In the detector,
Schema: additive. The two-writer interleaving regressionTwo real PowerShell processes on one The Two more defects found while doing this
About the test harness itselfThe old behavioral suite extracted a fragment of The two layers are bound explicitly, because section 7 proves the #2 — PostToolUse classified a structured result as a success
Agreed. A structured result stringifies to
Ten cases, every one of them through the real hook as a real child
Each case also asserts the pushed message ( #3 — the local runner ran fewer steps than the workflow
Agreed. The workflow has had step 5 since round-19 #5. A runner that Step 5 is added, 1:1 with the workflow's step 5, and fails on a ValidationThe 7 smoke warnings are pre-existing 12 negative-injection cases. Each is a minimal change that breaks
N6 and N8 did not bite on the first pass, and both were fixed rather
Two faults in the harness itself were found and fixed along the way,
What is not covered, stated rather than implied
One cost this exposed, not fixed here
That is a real latency cost for any deployment without a console, but Design compliance
|
The ubuntu validate job went red on the previous push. `node --test` discovers this file by its name (`test-*.mjs`), so the repository's ubuntu job runs the behavioral suite as well as the Windows one. The new PostToolUse section failed there, and it failed for a real reason rather than a flaky one: the hook calls `chcp 65001` and `Add-Type` with Win32 P/Invoke signatures, so it cannot run anywhere else. There is no console to change the code page of and no user32.dll to bind. The section is now gated to Windows and skipped LOUDLY -- the skip is counted and printed in the summary, because a skip that is invisible reads as a pass. The Windows job runs the whole suite with nothing skipped. The rest of the suite runs on both platforms, and that was already true before this change: the status.json protocol is plain PowerShell and the suite supplies an isolated %APPDATA%, so the store resolves to a scratch directory on either host. The ubuntu log confirms it -- sections 3, 5, 7 and 8 all passed there, and the only failures were the ten hook cases. Validation windows : 116 pass, 0 fail, 0 skipped the ubuntu job is the evidence for the other side
Dragging the pill showed a white rectangle the size of the window
(320x70) in place of the pill.
## Why
The drag used WPF's Window.DragMove(), which sends
WM_SYSCOMMAND / SC_MOVE and enters the Win32 modal move loop. Inside
that loop the WPF render thread is not composited: DWM stops
recompositing the layered window -- the per-pixel alpha surface that
AllowsTransparency="True" gives the pill its rounded, shadowed shape.
What is left on screen is that surface before it was ever painted: a
white rectangle with the window's exact bounds.
The DropShadowEffect makes it more obvious, because the effect is
software-rendered and that frame is not drawn at all.
## The fix
The drag is implemented directly: the normal message pump updates
Window.Left / Window.Top, so WPF composites every frame as usual and
the artifact cannot appear. The cost is the system-level move feel of
DragMove() (edge snapping), which this pill does not use.
Two details that are easy to get wrong:
- GetPosition($window) is relative to the window, and the window is
following the mouse, so the relative delta is always ~0. The
screen coordinates (PointToScreen) are differenced instead, and
converted back to the device-independent units Left/Top use.
- The 5px threshold gates the START of the drag, not the movement.
A guard that returns once didDrag is set makes the pill follow the
cursor for exactly one event and then stop.
The mouse is captured on button-down so the drag does not stick when
the pointer leaves the 320x70 window, and released on button-up.
## The BOM is not cosmetic
This file contains non-ASCII, and it had no UTF-8 BOM. Windows
PowerShell 5.1 -- the interpreter the widget actually runs under --
decodes a BOM-less script with the system ANSI codepage, so the C#
here-string stops being a here-string and the file does not parse at
all. Measured on this host, both ways, on the exact committed bytes:
no BOM -> PS 5.1: FAILS: Unexpected token '}' in expression or statement.
with BOM -> PS 5.1: parses OK
This is the round-20 #4 contract ("every non-ASCII .ps1 carries a UTF-8
BOM"), and this file was the one that had slipped through it.
## Test evidence
- Windows PowerShell 5.1 parses the committed file: OK
- the fix was verified by hand on the target OS: dragging the pill
shows no white box and the pill follows the cursor
A note on the automated attempt, because it is a trap worth recording:
injecting a synthetic drag with SetCursorPos + mouse_event does NOT
reproduce a drag. SetCursorPos moves the pointer without generating
mouse-move messages, so the widget never sees the movement and the
harness measures nothing while looking like it measured something. The
evidence above is the manual interaction plus the parse check; the
synthetic harness was dropped rather than kept as false evidence.
## Design compliance
no credentials : none added
no network : no network calls added
no telemetry : none added; widget.log unchanged
no third-party svcs : no new dependencies; pure PowerShell + WPF
cross-platform : no host-absolute paths introduced
encoding : the file keeps a UTF-8 BOM, as the round-20 #4
contract requires for every non-ASCII .ps1
behaviour change : the pill is no longer moved by the OS move
loop, so edge snapping is gone. Nothing else
about the drag changes: the same 5px threshold
separates a click from a drag, and a click still
toggles the CLI window.
The white box when dragging the pillSeparate from the three blockers, one more commit: Dragging the pill showed a white rectangle the size of the window WhyThe drag used WPF's The The fixThe drag is implemented directly: the normal message pump updates Two details that are easy to get wrong, and both bit during this work:
The mouse is captured on button-down so the drag does not stick when The BOM in this commit is not cosmeticThis file contains non-ASCII and had no UTF-8 BOM. Windows This is the round-20 #4 contract ("every non-ASCII Evidence
One note on the automated attempt, because it is a trap worth |
…ting it also starts the detector Two problems in start-island.ps1, both visible in daily use. 1. Readiness was decided by `Process.MainWindowHandle != 0`. That Win32 heuristic does not work for this window: it is borderless, always-on-top and transparent, so the handle stays 0 forever. Every start therefore reported "starting" and waited out the full 8 seconds, even though the widget was up in about one. The report itself was harmless, but it looks exactly like "it is stuck", which sends whoever is debugging it down the wrong path. The widget now writes "starting Application.Run" before it enters the message loop, and start-island.ps1 waits for exactly that line. Only lines appended after this launch count -- widget.log is append-only, so a stale marker from a previous run must not be mistaken for this one's readiness (otherwise a stop followed by a start reads the old marker). 2. The detector was not started by start-island.ps1. On 2026-10-06 14:37:42 the detector died on an unhandled terminating error and left a single "detector stopped" line behind. The widget stayed alive, so status.json froze and the pill looked dead for about 23 hours. The only recovery a user can think of is "restart the island", and if that does not bring the detector back, restarting achieves nothing. start-island.ps1 now starts the detector too. start-detect-island.ps1 is idempotent (it checks detect.pid and verifies that the PID still points at mcode-status-detect.ps1), so an already running detector is not started twice. A failure to start the detector is a warning only: the widget is up either way.
The MiniMax Marketplace review of the mcode-island submission package
rejected the build for five reasons. Three of them are real defects in
the hosted plugin rather than packaging issues, and they are fixed here.
1. autostart.ps1 ran `New-Item -Path $runKey -Force` on the
HKCU\Software\Microsoft\Windows\CurrentVersion\Run key. With -Force
that recreates the key, so every other autostart entry the user
registered (Everything, OneDrive, proxy clients, ...) was wiped. The
key is now created only when it is missing, the two mcode-island
values are written with New-ItemProperty, and the script verifies
afterwards that every unrelated entry in that key is still there.
2. The state detector could not run on the desktop app at all. It
resolved the MiniMax Code *CLI* install root and exited with "Cannot
find mcode install root" when that directory was absent, which is the
normal case for a desktop-only install. It also used the CLI-only
.mcode-active directory to decide whether the app was still running,
so a desktop user got a permanent "mcode has exited" error and the
session log was never read. The root is now resolved from the desktop
data directory ($env:MINIMAX_DATA_DIR, or <userprofile>/.minimax), the
process check is gone, and the "60s without a new event -> idle"
fallback closes the loop.
3. MSG_IDLE_FMT encoded 0xE9,0x9C,0xA8 (U+9728) instead of
0xE9,0xBB,0x98 (U+9ED8), so the pill rendered the wrong character in
the idle message.
On top of that the 5h-usage feature is now gated on the presence of
scripts/lib/Get-5hUsage.ps1: the token resolution and the refresh logic
moved into that lib, and the detector only enables the feature when the
lib is there. The hosted plugin keeps the 5h badge; a build that does not
ship the lib now reads no credential and makes no network request at all.
Validation
* the autostart registry test against the hosted plugin: Enable kept
all 7 unrelated Run entries, wrote exactly our two values, Disable
removed only ours and left the unrelated entries alone
* Windows PowerShell 5.1: mcode-status-detect.ps1 -Once reads the
desktop session store (~/.minimax/v2/sessions) and infers the state
(state=working, usage5h=58, source=detector)
Test evidence
* the registry test plants a sentinel value next to the plugin's own
entries, runs Enable and Disable, and asserts the sentinel survives
both times; the user's own values are snapshotted and restored
* the detector was run under Windows PowerShell 5.1 against the desktop
data directory, before and after the refactor
Design compliance
* the 5h usage feature stays optional: the hosted plugin still shows
the badge, and a build without the lib can read no token
* the detector no longer depends on the CLI, on the .mcode-active
directory or on a running node process
…vers it Topmost="True" in the XAML is evaluated once, when the window is created. Every other SetWindowPos in this file targets the *caller* window, not the pill. So once anything else lands above the pill -- a maximized terminal being activated, a fullscreen app, another always-on-top tool -- the pill stays buried forever. WS_EX_TOPMOST is a persistent style, but it only orders the pill against the topmost band: whichever topmost window was raised last wins, and a plain maximized window that gets re-shown can paint over the pill too. Observed on 2026-09-28: the pill was fully invisible behind Windows Terminal while IsWindowVisible=1, IsIconic=0 and DWMWA_CLOAKED=0, and the widget was still polling. The fix hit-tests the pill's centre pixel on a 1.5s throttle and re-asserts HWND_TOPMOST when the pixel belongs to another process. SWP_NOACTIVATE means it never steals focus from the user's terminal, and the check is skipped entirely while the pill is hidden so it cannot resurrect a pill the user closed. Transient UI that is supposed to be above the pill is exempt: menus and tooltips (#32768), tray balloons (NotifyIconTooltipWindow) and Windows.UI.Core.CoreWindow. Without that exemption the pill would fight every context menu the user opens over it. "mcode-island show" passes -Force, because $window.Show() is a no-op on an already visible WPF window and therefore never restored the z-order on its own.
…ing a surface, and grabbing credentials
The Marketplace review rejected the mcode-island submission for five
reasons. Two of them were packaging rules, but three were about how this
plugin handles the 5h usage readout, and those applied to the hosted
plugin too. This commit fixes them at the source, so the hosted plugin
says the same things the package does.
1. The request carried "MM-API-Source: Minimax-MCP", i.e. it claimed to
be a MiniMax-owned surface. A probe without any header, with
"MM-API-Source: Minimax-MCP" and with "MM-API-Source: mcode-island"
all return the same 1004 "Please carry the API secret key in the
'Authorization' field", so the endpoint never read the header. It
only claimed a MiniMax identity. The header is gone; the request
carries Authorization and nothing else.
2. The endpoint and 32 other strings were assembled from byte arrays
through a private `_s` helper. The stated reason was a PowerShell 5.1
parser quirk, but every one of those constants is assigned to a
variable and only ever compared as a variable, so the literal never
sits after an -eq where the quirk applies. Replacing them with plain
literals parses without an error under Windows PowerShell 5.1 and
behaves the same, and the `_s` helper is deleted with them. The
endpoint is now a plain string in the lib, and the README names the
same host, so the disclosure check can compare the two directly
instead of decoding one to check the other.
3. The token was stored in the clear in config.json, and an ambient
MINIMAX_OAUTH_TOKEN / MINIMAX_API_KEY was picked up and sent without
the user ever opting in.
The token is now encrypted with DPAPI (CurrentUser scope) and stored
as config.json:planApiTokenEnc. Only the same Windows user on the
same machine can decrypt it. A config.json written by an older
version, which kept the token in the clear under planApiToken, is
migrated in place on the next detector start: the token is encrypted
and the plaintext field is removed. No action is required from the
user.
The environment variables are only read when the user has explicitly
opted in with `set-token.ps1 -AllowEnvToken`, which persists
allowEnvToken=true in config.json. An environment variable that
happens to be set is not consent to send it to an API. The existing
configuration keeps working unchanged: it uses the config.json token,
not the environment.
The request goes to the user's own MiniMax account endpoint and
nothing else is sent. The README now says exactly that.
Validation
* node plugins/antianqi/mcode-island/scripts/smoke.mjs -> 131 pass,
7 warn, 0 fail
* pwsh -File plugins/antianqi/mcode-island/scripts/test-windows-workflow-local.ps1
-> all 5 steps OK (step 5: 116 pass, 0 fail)
* Windows PowerShell 5.1: mcode-status-detect.ps1 parses without an
error and -Once still infers the state from the session log
* token migration on a config copy: planApiToken (plaintext) became
planApiTokenEnc (DPAPI), the plaintext field is gone and the 5h
readout still works (usage5h=70)
Test evidence
* the smoke test gained the checks that would have caught this: no
.ps1 may send an MM-API-Source header, no .ps1 may build a string
from a byte array, set-token.ps1 must store planApiTokenEnc and
never write a plaintext planApiToken, and the environment token must
sit behind the allowEnvToken gate
* the CI local runner exercises the real resolution path
(Resolve-PlanToken) for all three token sources: the env token
after an explicit opt-in, the DPAPI-encrypted config token, and no
token at all
* the smoke test's disclosure check now reads the plain endpoint and
compares it against every host the README and the Skill name
Design compliance
* the credential is encrypted at rest, is never logged, is never sent
anywhere but api.minimaxi.com, and is only taken from the
environment with the user's explicit consent
* the request no longer claims a MiniMax-owned identity
* nothing in the plugin is hidden from a reader anymore
…sking for a credential
The 5h usage readout does not have to be a credentialed feature. The
MiniMax Code runtime already resolves the token plan with the user's own
login -- the mcode launcher's "MCode status" panel renders it, with the
footer "Local view - credentials hidden" -- and it keeps the numbers in
memory. The plugin can read the same numbers from a local file the runtime
writes, and then it never holds a credential and never makes a request of
its own.
scripts/lib/Plan-Usage.ps1 reads that file. The contract is the runtime's
own field names, so the runtime can dump what it already has:
{
"fetchedAtMs": 1792800000000,
"fiveHour": { "remainingPercent": 58, "resetAtMs": 1792800000000, "unlimited": false },
"weekly": { "remainingPercent": 40, "resetAtMs": 1792900000000, "unlimited": false },
"video": { "remainingCount": 12, "totalCount": 40 }
}
The canonical path is %APPDATA%\mcode-island\usage.json; the runtime's own
data directories are read as a fallback, and MCODE_PLAN_USAGE_FILE
overrides everything.
The detector now prefers that file and only falls back to the credentialed
query when the file is absent. The fallback is the existing
scripts/lib/Get-5hUsage.ps1 path, which stays as it is: the token is
DPAPI-encrypted, the environment token needs an explicit opt-in, and the
request no longer claims to be a MiniMax-owned surface. A build that does
not ship Get-5hUsage.ps1 -- the Marketplace package -- therefore has no
credential path at all, and the readout keeps working from the runtime's
file alone.
Validation
* with the token removed from config.json, the detector still reports
usage5h=58 and a reset countdown in ~60 minutes, read from
usage.json
* without usage.json the detector falls back to the credentialed query
* the Marketplace package drops the fallback line and ships
scripts/lib/Plan-Usage.ps1 instead, so the readout there is
credential-free
Design compliance
* the plugin reads local data the runtime produced; it does not request,
store or transmit any credential
* the credentialed path remains available for the hosted plugin, where
the user opted in explicitly
* the two paths share the same output fields, so the widget does not care
which one produced the numbers
… window
In the desktop app the agent is hosted by the app and runs in a terminal the
app opened. Hiding that terminal does not stop the agent: the user is left
looking at a static pill while the work continues behind it. The action that
matches the intent is "take me back to the app window", so the click target
is now resolved on every click, in this order:
1. a manual pin (pin.json) -> focus that window
2. the desktop app main window -> focus it, never hide it
3. the caller's terminal (caller.json) -> the original show/hide toggle
The desktop app is found by process name ("MiniMax Inside Code",
"MiniMax Code", "Mavis") inside the widget's own session: the app is not an
ancestor of the agent process, so the process tree cannot be used to locate
it.
The pin moves from caller.json into its own pin.json. caller.json is rewritten
on every status transition, so a pin stored there did not survive the next
notify -- with the pin in the way, "the pin beats the desktop app" would have
been unreachable in practice. `mcode-island unpin` now clears both files.
Validation
-----------
* Windows PowerShell 5.1 parser: 0 errors on mcode-island.ps1 and
pin-island.ps1.
* The click-target resolver was exercised against the live desktop app: with
MiniMax Inside Code running the click resolves to the app window
(mode=focus); with a pin.json present it resolves to the pinned window
(mode=focus); with neither it falls back to the caller's terminal
(mode=toggle).
* The live focus path was executed against the running desktop app: the click
resolved to "MiniMax Inside Code" (mode=focus, origin=desktop-app), the app
window came to the foreground, and it stayed visible.
* Negative injection: 8 deliberate mutations of this contract (priority
inverted, focus path hiding the target, pin reading caller.json, click
handler bypassing the dispatcher, dispatcher ignoring the Mode, cross-session
app lookup, pin writer writing caller.json, unpin not clearing pin.json)
each trip at least one drift lock; the unmutated source passes 146/146.
Test evidence
-------------
* node scripts/smoke.mjs -> 146 pass, 7 warn, 0 fail (was 131).
* node scripts/test-substep-progress.mjs -> 116 pass, 0 fail.
* pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK.
* Windows PowerShell 5.1 parser: 0 errors on both changed scripts.
Design compliance
-----------------
* The behaviour lives in the plugin source, not in the Marketplace package
builder, so the CLI build and the Marketplace build share one
implementation, and the CLI keeps its original behaviour when no desktop
app is running.
* No new credential, no new network call, no new file outside
%APPDATA%\mcode-island.
* The focus path never calls ShowWindow(SW_HIDE); only the terminal toggle
does.
… last line is fat
Reported symptom: "the first test after the start is fine, but in a new
conversation the pill stops updating and keeps showing the previous task's
finished state".
Root cause, reproduced against the real detector:
One session-log line can be much larger than the tail window the reader
looked at. An agent that reads a screenshot writes the base64 image into a
single line -- 200 KB to 560 KB in the wild, and this machine's own sessions
carry lines of 561 KB and 1.4 MB. The reader took the last 64 KB of the file
and parsed the fragments backwards, so the window held nothing but a fragment
of that one line. ConvertFrom-Json threw on the fragment, and the catch-all
answered with the previously cached inference -- a message that belonged to
an EARLIER session, often an earlier conversation.
The cache made it stick: it was keyed on the mtime alone, so the mtime of the
new file was recorded before the failed read, and every later poll returned
that same foreign message until the file grew again.
Three defects, one fix:
* The tail reader now grows its window until a line actually parses, and it
reads the last COMPLETE line (the first fragment of the window is a cut
line and is never trusted unless the whole file is inside the window). A
fat line costs a few hundred KB of read, once per change.
* The message cache is keyed on path AND mtime, so a session switch can
never answer with the previous file's message.
* A read failure now answers "no information" instead of another session's
message. The status stays as it was, which is the honest answer.
Validation
-----------
* Reproduced first: with the pre-fix detector, a new session whose only line
is 200 KB reported the PREVIOUS session's tool ("Read" instead of "Ran").
After the fix it reports the new session's own tool.
* The regression test is behavioral: the real functions are extracted from
the real detector and run against a synthetic session tree, so a
text-shaped lock cannot pass while the behaviour is broken.
* Negative injection: the test fails on the pre-fix detector (the reader is
not extractable) AND on a mutant that keeps the function name but restores
the fixed 64 KB window (expected "bash", got ""). The unmutated source
passes.
Test evidence
-------------
* node scripts/smoke.mjs -> 150 pass, 7 warn, 0 fail (was 146).
* node scripts/test-substep-progress.mjs -> 119 pass, 0 fail (was 116).
* pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK.
* Windows PowerShell 5.1 parser: 0 errors on mcode-status-detect.ps1.
Design compliance
-----------------
* Detector-only change: no credential, no network request, no new file.
* The reader is still bounded: one line may not pull more than 8 MB into
memory, and the mtime cache keeps the work off the hot path.
* The Marketplace package ships this detector, so the fix has to be
republished there as well (v0.8.1).
The detector follows the session that wrote its log most recently. That is a decision, not an accident: the conversation the user is looking at is the one writing to the log, and the runtime exposes no signal that would let a plugin pin a session or ask which window is focused. The boundary is written down too: with several conversations running at once the pill can briefly show a neighbour's state, and the detector deliberately does not try to disambiguate that. Two guards keep it small -- the state is re-derived on every change, and an unparseable log line never falls back to another session's message.
…different click targets
The two are different products, and the click behaviour is where that shows:
* the CLI runtime loads the io.minimax.mcode hooks, so the agent pushes the
state itself, and a click keeps the CLI habit -- one click hides the
terminal, the next brings it back;
* the Marketplace package ships no Hook material (the submission rules
forbid it), so the detector is the only source of state; and the agent is
hosted by the desktop app, so the terminal belongs to the app session and
hiding it leaves a static pill behind. There the click has to focus the
app window instead.
Until now the shared source hardcoded the desktop behaviour, so the CLI
install would have lost its own habit the moment the desktop adaptation
landed -- and the CLI user is the one with the app open all day, which is
exactly the case where the two answers differ.
So the source carries ONE switch:
$script:clickPrefers = 'terminal'
and the Marketplace builder flips that exact line to 'app', failing the
build when it cannot find it. A renamed switch cannot silently ship the
CLI behaviour to the desktop users.
The pin still wins in both builds: an explicit `mcode-island pin` is the
user's own choice and outranks the preference.
Validation
-----------
* The resolution is checked behaviorally on both sides: the resolver is
extracted from the real file, both lookups are stubbed so a desktop app
AND a caller terminal exist, and the CLI install has to resolve to the
terminal with the show/hide toggle while the package has to resolve to
the app window with focus.
* On this branch the package half is skipped loudly (the package is not
here); the CLI half is checked.
* Negative injection: the drift lock fails when the switch is renamed, and
the behavioural half fails when the resolver stops branching on it.
Test evidence
-------------
* node scripts/smoke.mjs -> 152 pass, 7 warn, 0 fail (was 150).
* node scripts/test-substep-progress.mjs -> 122 pass, 0 fail, 1 skipped
(the package half; this branch has no package).
* Windows PowerShell 5.1 parser: 0 errors on mcode-island.ps1.
Design compliance
-----------------
* One implementation, two profiles. The behaviour difference is a single
named switch, not a second copy of the resolver.
* No credential, no network request, no new file.
* The Marketplace package is rebuilt from this source; the builder owns the
flip and fails loudly when the anchor is gone.
The source, the SKILL and the README all described the desktop order (app window first) because that is what the shared code used to do. It does not any more: the CLI install prefers the caller's terminal, and only falls back to the desktop app window when there is no caller window to go back to. The Marketplace build keeps the desktop order, and the source now says which build it is describing and why the two differ.
…the plaintext one
The token handling changed: the token is stored DPAPI-encrypted in
config.json as planApiTokenEnc, and the plaintext planApiToken field is
migrated away on read. The local runner
(scripts/test-windows-workflow-local.ps1) was updated with that change.
The Windows workflow was not. Its step 2 still asserted
`config.json planApiToken == FAKE_TOKEN`, so the job went red on the
first push that carried the new token handling:
set: config.json planApiToken mismatch (got: )
This step is the CI mirror of the local runner's step 2, so it now
asserts the same thing:
- config.json has no plaintext planApiToken
- config.json has planApiTokenEnc
- planApiTokenEnc decrypts back to the token
- -Show reports "config.json planApiTokenEnc" with the masked prefix
- -Clear removes both fields
The point of the check is unchanged -- the token must land in the
isolated APPDATA and come back out of it -- but it now goes through
the same door the code uses.
The pill's 5h badge was blank almost all the time, while the detector's
own log said "5h usage refreshed: remaining=94%". The number was
arriving, and being dropped again on the very next write.
Mechanism, confirmed against the live status.json with a 25ms watcher:
- The detector's step 4 calls Get-PlanUsage on every tick, to prefer
the plan usage the MiniMax Code runtime already wrote to disk.
- Get-PlanUsage cleared $script:plan5hRemainingPct and
$script:plan5hResetMs on the way in, before it went looking for a
file.
- The only thing that refills them is Refresh-5hUsage, whose 60s TTL
gate returns early WITHOUT touching them.
So on every tick except the one right after the API call, the cached
value was $null, and the write path emitted usage5h: null. The number
survived exactly two writes -- the refresh itself and the state
inference right after it -- and was gone for the remaining ~58 seconds
of every minute.
Measured on the widget's own poll log: 4 of 28 polls carried a value
in the 15:40-15:42 window, 24 of 24 carry one after the fix.
The fix is in the lib, not in the detector. Get-PlanUsage is a SOURCE.
A source with nothing to offer has to leave the current value alone --
that is what the credentialed path already does when its TTL gate
blocks a refill, so the two paths now behave the same way. The
detector's call order does not change, and no write path changes.
The latent bug shipped with fbdfd53, but the detector that calls
Get-PlanUsage only reached this machine at 13:50 today, so the badge
was healthy until the 13:58 restart and has been broken since.
Validation
- scripts/test-substep-progress.mjs section 11. 11a is the unit
half: the lib must not wipe a value it has no replacement for.
11b is the end-to-end half: two detector ticks against a real
status.json, the second write still carrying the number.
- The suite on this branch: 130 pass, 0 fail, 1 skipped (platform)
on Windows PowerShell 5.1, which is what CI runs.
- Negative injection: putting the two clearing lines back makes
exactly 4 checks fail -- 11a's two and 11b's two -- while
"both ticks wrote" and "the second write carried the state" stay
green. The writes really happen; only the number goes missing,
which is the production symptom. 126 pass / 4 fail with the
injection, 130 pass / 0 fail after the revert.
- Live: the lib was redeployed, the detector restarted, and a 25ms
watcher over status.json recorded 23 consecutive writes, every one
of them carrying usage5h=90. The reset countdown now advances at
the 60s refresh and holds its value in between, instead of being
blanked for a minute at a time.
- The pill itself renders "4h04m 90%" now.
Test evidence
- node scripts/test-substep-progress.mjs -> 130 pass, 0 fail, 1 skipped
- negative injection -> 126 pass, 4 fail (reverted afterwards)
Design compliance
- No credentials, no network, no telemetry, no third-party service:
the fix touches no token handling and makes no request. It only
decides whether a number that has already been fetched survives
the next tick.
- The runtime-usage path stays preferred: when the runtime file is
there, its numbers still win; the credentialed fallback still runs
when it is not.
- The detector's write semantics are untouched: -MetadataRefresh
still restates the state, the state inference still claims it.
- ASCII only, no BOM, LF line endings -- the lib is dot-sourced by
a Windows PowerShell 5.1 detector on a zh-CN host, where a BOM-less
script with non-ASCII would not even parse.
f504326 to
d3115f3
Compare
Round 22: the 5h usage badge stays on the pill
What actually happensThe detector's step 4 calls So on every tick except the one right after the API call, the cached value was Measured on the widget's own poll log: 4 of 28 polls carried a value in the 15:40-15:42 window, 24 of 24 carry one after the fix. The fixIn the lib, not in the detector. The detector's call order does not change, and no write path changes. Validation
Design compliance
The sibling branch ( |



What this changes
1. The status.json write protocol is now atomic across processes
status.jsonhas two independent writers: the detector daemon(
mcode-status-detect.ps1) and the agent hooks (throughnotify-island.ps1, a child process per tool call). They had no mutualexclusion at all.
One place holds the protocol, shared by both writers:
Enter-StatusLock/Exit-StatusLockFileShare.None— a real OS file lock, not a flag file whose presence we pollRead-StatusSnapshotWrite-StatusSnapshotversionandtsThree defects came out of this:
The detector read the file twice per write. The loop read
state/message/familyat the call site, thenWrite-Statusreadstep/detail/sourceagain. A hook write landing between the tworeads was either mixed into a stale payload or lost outright.
Everything now derives from one snapshot, taken under the lock.
Both writers staged through the same
status.json.tmppath. Onecould 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, sothat failure killed the daemon — the pill froze on the last
status.jsonwith no visible cause.The replace could lose a sharing violation.
MoveFileExwithREPLACE_EXISTINGfails when the target is open by a reader that didnot share delete, and the widget polls
status.jsonevery 400 mswith
Get-Content, which opens withFileShare.Read. Measured onthe dev host, not inferred. The replace now retries briefly.
The arbitration read and the write it authorises are now one critical
section too: previously a hook push could be judged against a stale
snapshot and then overwritten by the write that stale snapshot had
authorised.
Schema change (additive):
status.jsongains aversionfield. Itis optional — the widget reads every field through
PSObject.Properties, so an older widget ignores it and a newer onetolerates its absence. The existing sentinel convention is unchanged:
absent means
-1/-1/"".2. A failed tool no longer shows as a success
PostToolUseclassified the tool result by stringifying it and matchinga textual error prefix. A structured
{isError: true}stringifies to@{isError=True; ...}, which matches no prefix — so a failed toolwas reported as
doneand the pill showed green. The detector alreadyread the structured boolean from the session log, so the two producers
of the same state disagreed about what a failure was.
Test-ToolResultErrornow reads the structured boolean first and fallsback to the textual heuristic only for a bare string.
3. The detector survives a terminating error in the main loop
The loop had no
catch. Any terminating error escaped it, fired thefinally, logged "detector stopped" and exited the process — while thewidget kept running, so the pill just froze with no visible cause. The
loop now logs the exception type and retries on the next tick.
4. The pill: click-to-toggle, tool verbs, family tinting
SW_MAXIMIZE:SW_SHOWandSW_RESTOREboth preserve the window'spre-hide size, so a window that had been accidentally resized came
back as a 480×84 strip.
bash : {"command":...}became
Running git status/Ran/Command failed. The mcode CLITUI already had this descriptor table; the pill now does too.
task / web / plan) each get their own colour on the dot, the pulse
ring and the progress bar. The palette is asserted to be at least
CIE76 dE 20 apart in CIELAB —
planandsearchwere bothpurple at dE 10.1 and are not distinguishable on the pill.
ask_userreportswaiting, notworking. On afull accesssetup the permission hook never fires, so
waitingwas unreachableand
ask_userwas the only thing that could actually block theagent — while wearing the blue "leave it alone, it is making progress"
signal.
5. Redaction, log bounds, and the UTF-8 BOM
reaching
status.json, the always-on-top pill and the append-onlylog — so anything typed through Computer Use landed in a screenshot or
a screen recording. It now reports the action plus a character count.
never touched, so
curl -H "Authorization: Bearer ..."reached allthree sinks in the clear. Two independent producers of that text
exist (the hook path and the detector re-deriving it from the session
log), so the helper lives in a shared lib and the check asserts that
both consumers invoke it.
widget.logmeasured 37.2 MB and
island.log18.0 MB after 38 days, withwidget.loggrowing ~3.6 MB/day..ps1files carry a UTF-8 BOM. Windows PowerShell 5.1decodes a BOM-less script with the system ANSI codepage; on a zh-CN
host that is GBK, so the detector's C# here-string stopped being a
here-string and the script died before its first statement. This is
pre-existing, not a regression from this branch — the round-19 head
fails under 5.1 with byte-identical output. It had simply never been
synced from a clean checkout on a non-UTF-8 host.
README claimed the token is "never sent to a host other than
api.minimax.io" — the code never contacted that host at all. Thecheck now decodes the endpoint's byte array and compares the docs
against that decoded truth, so it cannot rot into a second copy of the
disclosure it is policing.
6. The pill no longer renders raw tool JSON, and skips boilerplate
The
runningmessage used to be"$verb " + (ConvertTo-Json $args), sothe pill showed
Running {"command":"$ErrorActionPreference=\u0027Con….Format-ToolArgsnow returns the single most informative field pertool, and there is no fallback to JSON: an unknown tool degrades to
less information, never to a dump. It also prefers the first line that
actually executes something, skipping the
$var = ...preamble thatalmost every agent command opens with.
Design compliance
island.log/widget.logare local append-only files, now size-cappedversionis optional; the sentinel convention is unchangedValidation
The 7 smoke warnings are pre-existing
forwardevent-catalog entriespending mcode 0.2.4+ Runtime confirmation; unchanged by this PR.
Negative-injection evidence
Per the round-4 lesson — test pass ≠ contract honored — every new
check was observed failing before its fix. 12 cases, each a minimal
change that breaks one clause of the contract; all 12 turn the suite
red, and the baseline restores to green after each:
notify-islandstages through a shared path again-MetadataRefreshnotify-islandstops taking the lockPostToolUsegoes back to string-only classificationisErrorflag is ignoredThree of these checks were false greens before they were right, and the
checks were rewritten rather than the failures being waved off:
Write-Statusandstubbed the reader. That stub is exactly what hid the second read —
the bug the review named. The suite now runs the real
Write-Statusagainst a realstatus.jsonin an isolated%APPDATA%.surviving payload merely looked plausible). The
versioncounter isa second, independent witness: two writers that read the same
generation mint the same version.
which reports blue and purple as identical (0.02 apart) because they
differ in hue but not brightness, and its regex asked for 6 hex
digits while the table stores
#AARRGGBB— so it matched nothingand reported "all 0 family colors are perceptually distinct", a
vacuous pass.
The two-writer regression, in detail
Two real PowerShell processes interleave on one
status.json: writer Ais 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.
CI
The
mcode-island (windows-latest)job is the evidence for thisplugin — it is Windows/PowerShell/WPF/Win32, and
[code]smithisskipped on this repository. Five steps: parse every
.ps1, tokenset/show/clear in an isolated
APPDATA, the real PreToolUse hook pathover stdin, a mocked usage-API roundtrip on a localhost listener, and
the behavioral suite. The local runner
(
scripts/test-windows-workflow-local.ps1) mirrors it 1:1, all fivesteps, so local reproduction and CI are the same test.
Not in this PR, on purpose
carries
modelandusage.{input,output,cacheRead,cacheWrite,totalTokens,cost}and nothing else — there is no
contextWindowand notpsfield.usage.totalTokensis a usable numerator, but the window size livesin mcode's provider config, and a rate derived from message
timestamps would fold in thinking and tool time, which mcode 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. Better no readout than a quietly wrong one.
Get-CallerFocusInfoasks WMI for theprocess chain, one query per level. Where the hook is spawned without
a console, that measured 6.5 s per hook invocation on the dev host
(where a console is inherited, the first branch returns
immediately). Real, but a separate change with its own blast radius.