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
|
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.
The 5h-usage fix landed on the base branch. Merging it in keeps this PR's diff limited to the multi-session feature: both branches appended a section to scripts/test-substep-progress.mjs, so the conflict was resolved by keeping BOTH sections -- the multi-session tracker (10) and the 5h-usage regression (11).
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 ( |
The expanded list showed one line per session: a status dot, the first
question, and the state. That is enough to recognise a session but not
to tell what it is doing RIGHT NOW -- "the detector follows whichever
session wrote last" is the whole reason this list exists.
The layout now follows the expanded-state reference the user pointed at:
two lines of text per session and a timestamp on the right.
[dot] first question............. just now
last thing said........... running
---------------------------------------------
[dot] another question........... 41 min ago
running <tool>............ running
What each piece costs:
first line the session title (already there)
second line the last thing said, or the tool being run
right column the last activity time, and the state
The second line is the one that carries information. Looking at the
first question alone, a session that is thinking and a session that is
stuck on one long tool look identical.
Data plumbing
The tracker already had the last message in memory -- Get-LastJsonLine
reads it and Get-SessionPhase parses its content -- so the summary is
free: no extra IO, no extra file reads.
preview the text of the last message (agent's or user's)
lastTool the tool name of the last message
lastAtMs the last activity time (the log's mtime)
Three rules for what a summary may be, because "the last thing said" is
not the same as "the last thing in the file":
- a toolResult's text is tool output (code, logs), not conversation.
It never becomes a summary.
- a toolCall has no text; the tool name stands in for it.
- a toolResult has no toolCall in its content, but it DOES carry
message.toolName. Without that fallback the second line was empty
for every session whose last line was a tool result -- which is
most of the time while an agent is working.
The tracker stores the raw values and the widget does the formatting.
Changing the layout must not require touching the detector.
One existing bug fixed on the way
The tracker only marked itself dirty when the STATE changed. The
summary and the timestamp change without the state changing (an agent
runs three tools in a row: still running, but the summary and the
clock move). The in-memory entry was right and the file on disk was
not, so the list rendered an empty second line and a frozen timestamp.
Now any change to the content marks the file dirty.
Validation
- scripts/test-session-tracker.ps1 grew from 20 to 37 checks: the
summary, the tool name, the timestamp, the file round-trip, the
restart, and the same-state-but-changed-content case.
- The behavioural suite: 133 pass, 0 fail, 1 skipped (platform) on
Windows PowerShell 5.1, which is what CI runs.
- Negative injection, 7 mutations, each one expected to turn a
specific check red:
N1 drop the toolResult exclusion -> "toolResult's text is not
a summary" goes red
N2 stop recording lastAtMs -> the two timestamp checks
go red
N3 stop restoring preview -> "the preview survives a
restart" goes red
N4 stop recording lastTool -> the two tool-name checks
go red
N5 make the summary always empty -> the four summary checks
go red
N6 revert the dirty check -> "a changed tool name with
an unchanged state still
reaches the file" goes red
N7 drop the toolName fallback -> "a toolResult's tool name
is recorded" goes red
Every mutation was applied on the bytes, not on a decoded string:
a text-level round trip through the wrong code page turned the file
from 16132 into 12651 characters and made every anchor miss.
- Live, on this machine: the deployed plugin was restarted, the
mouse rested on the pill, and the screenshot shows the layout
above -- three sessions, two lines each, "just now / 41 min ago /
15:39" on the right, "running" under it, a rule between the rows.
The first row's second line is the agent's own last sentence; the
second row's is "running grep".
Test evidence
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
- scripts/test-session-tracker.ps1 -> 37 pass, 0 fail
- 7 negative injections -> each turned its own check red; the suite
was green again after every revert
Design compliance
- The pill itself is unchanged: same window, same width, same drag
handling, same 400ms poll. The list still grows out of row 1, so
the pill does not move while the list opens.
- The click handler moved to the row's outermost element, so a click
anywhere in the row dismisses that session and nothing fires
twice.
- No credentials, no network, no telemetry, no third-party service.
The summary comes from the session log the detector already reads.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
The list's first line was the first question of the session, cut at 40
characters. That is not a name: half the sessions start with a URL, and
the rest start with a sentence that means nothing once it is cut ("go
on and finish the capability list panel. first look at where the
static scanner stands.").
The runtime already keeps a title for every session:
~/.minimax/v2/sqlite/runtime-state.sqlite
local_runtime_sessions.title
413 of the 417 sessions have one, and they are distilled subject lines
("fix the MCode 0.6.13 update failure", "what the PR 62 review says",
"how mcode's miniapp works"). That is what the session is called.
Reading it
PowerShell has no SQLite reader and the plugin ships no binaries (the
Skill-only, zero-npm-dependency rule). So the read borrows a node that
is already on the machine:
- node:sqlite is a BUILT-IN module of Node 22.5+, not a dependency;
- the CLI install ships one: ~/.minimax-code/runtime/node-*/node.exe;
- the desktop app is Electron, and ELECTRON_RUN_AS_NODE=1 turns it
into a node (verified: node 24.18.0, node:sqlite available).
The runtime is looked up in that order: node on PATH, then the CLI's
own, then the desktop app as node. If none of them works, the whole
feature is off and the list falls back to the first question -- exactly
what it did before. The same shape as scripts/lib/Get-5hUsage.ps1: an
optional lib, and when the lib is not there the chain is simply closed.
The read is read-only and does not touch the app's data. It runs once a
minute, not once a tick: the titles barely change, and opening a 1.9 GB
database is not something to do every second. Measured on this
machine: 391 ms for the first read, then cached.
Two things that had to be right
- The temporary script is written as .cjs. A package.json with
"type": "module" (there is one in the user profile) makes node load
a .js file as an ES module, and require() then fails with "require
is not defined in ES module scope".
- The database path is the second argv, not the first: argv[1] is the
script's own path.
Fallback
The runtime title is preferred; the first question stays the fallback.
The fallback is not a formality: the runtime keeps no title for four
sessions, and a machine without a usable node keeps none at all.
Validation
- scripts/test-session-tracker.ps1 grew from 37 to 44 checks. The new
ones build a REAL sqlite database with node:sqlite (a temporary one,
pointed at through MCODE_RUNTIME_STATE_DB) and assert: the title
comes back, a session that is not in the database has no title, an
empty title stays empty, the list prefers the runtime title over the
first question, and both fallbacks land on the first question.
- The behavioural suite: 133 pass, 0 fail, 1 skipped (platform).
- Negative injection, 10 mutations, each expected to turn a specific
check red. Three of them are new:
N8 the display name stops asking for the runtime title
-> "the list prefers the runtime title" goes red
N9 the fallback to the first question is removed
-> "no runtime title falls back to the first question" and
"a missing database falls back to the first question" go red
N10 the title map is never read
-> "the title comes back from the runtime database" goes red
The seven earlier ones (preview, timestamp, tool name, the dirty
check) still hold.
- Live, on this machine: the list now reads
"look at the new reply on MiniMax-Code-Plugins PR 62"
"how mcode's miniapp works"
where it used to read a cut-off first question.
Test evidence
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
- scripts/test-session-tracker.ps1 -> 44 pass, 0 fail
- 10 negative injections -> each turned its own check red; the suite
was green again after every revert
Design compliance
- The plugin still ships no binaries and no npm dependencies.
node:sqlite is built into the node it borrows; nothing is bundled.
- No credentials, no network, no telemetry, no third-party service.
The title is read from a local database the host already keeps, and
the read is read-only.
- The feature degrades to the previous behaviour instead of failing:
no node, no database, no title in the database, a locked database,
a timeout -- every one of those leaves the list exactly as it was.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
… paths
The ubuntu CI job failed on the previous commit, and it failed twice for
two different reasons. Both were the same mistake: building a path out
of an environment variable that does not exist everywhere.
First: $env:USERPROFILE. Windows has it, the ubuntu runner does not,
and Join-Path does not accept an empty path. Two places built a path
out of it -- the CLI runtime lookup (~/.minimax-code/runtime) and the
runtime database (~/.minimax/v2/sqlite). Both were outside the try
blocks, so the exception escaped and took the tracker with it.
Then: $env:TEMP. Linux PowerShell has TMPDIR, not TEMP. The temporary
script for the node read went through Join-Path $env:TEMP, threw, and
the catch swallowed it -- so the read returned nothing and the list
silently fell back to the first question. That one is nastier: nothing
crashed, the feature just quietly did not work, and the failure
reported by the suite was "the title came back empty", a long way from
"your temp directory variable is missing".
The fix is the same shape in both places:
- a home-directory lookup that accepts USERPROFILE or HOME and
returns an empty string instead of throwing;
- the temporary directory comes from .NET's GetTempPath(), which
knows the right path on both platforms;
- every function in the title path -- Get-NodeCommand,
Read-RuntimeTitles, Get-RuntimeSessionTitle -- swallows its
exceptions, because a missing title is not an error, and the
detector's main loop calls this every tick.
A missing home directory, a missing temp directory, no node, no
database, a locked database, a timeout -- all of them now mean the same
thing: the list shows the first question, exactly as it did before the
feature existed.
Validation
- Two new checks in scripts/test-session-tracker.ps1:
* with USERPROFILE, HOME and MCODE_RUNTIME_STATE_DB cleared, the
three title functions run without throwing;
* with $env:TEMP cleared but a real database pointed at, the
title STILL comes back. The second one is the one that locks
this bug down: the first attempt at this fix only handled the
home directory and the ubuntu job still failed.
- The suite on this machine: 46 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Reproduced locally: a child PowerShell with USERPROFILE, HOME and
TEMP all cleared runs the title path and comes back with no
exception and no title.
Test evidence
- scripts/test-session-tracker.ps1 -> 46 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The title is still an optional extra. Every failure mode leaves the
list exactly as it was before the feature existed.
- No new dependency: the same node that was already borrowed, the
same read-only database, no binaries, no network, no credentials.
The list said "running" for a session that had been quiet for almost an
hour.
The cause is a freeze, not a wrong reading. The tracker decides whether
a session is running from the type of its last message: a toolCall or a
toolResult means the agent is busy. That is right while the agent works
and wrong the moment it stops -- after the last tool runs, the last
message stays a toolCall forever.
On top of that, the update loop skips a session whose log has not
changed:
if ($entry -and $entry.MTime -eq $g.MTime) { continue }
So once the agent stopped, the phase was never recomputed again. It
froze on "running" and stayed there until something else happened to
touch the file. The detector's own log shows it:
18:13:44 session restarted: mvs_cd09012264b5 (cycle 7)
19:31:47 session restarted: mvs_cd09012264b5 (cycle 8)
Seventy-eight minutes apart, and in between the session was shown as
running while it was doing nothing.
The fix
"Running" now needs a liveness witness as well as a message type. If a
running session has been quiet for more than five minutes it is not
running, it is stopped, and it says so:
running the agent is busy (blue)
waiting the agent is waiting for you (amber)
stopped the agent is gone; the last message was a tool call that
never came back (grey)
Five minutes is long enough that a slow tool or a long model turn is
not mistaken for a dead session, and short enough that nobody waits an
hour to see the state change.
The downgrade happens in the branch that used to `continue` -- the one
that runs when the log has not changed. That is the point: the state
has to be able to change without the log changing.
"Waiting" is untouched: the ball is in the user's court, and a session
the user has not answered for half an hour is still waiting.
"Stopped" ends a round just like "waiting" does, so a session that
comes back to life counts as a new round and the dismissal constraint
is lifted -- the same rule that already applied to waiting.
Validation
- scripts/test-session-tracker.ps1 grew from 46 to 52 checks. The new
ones stop a session (its log's timestamp is moved ten minutes into
the past), let the tracker run twice -- the second run is the one
that takes the "the log has not changed" path -- and assert that it
is no longer reported as running, that it is reported as stopped,
that a waiting session stays waiting, and that a stopped session
that comes back counts as a new round.
- The behavioural suite: 133 pass, 0 fail, 1 skipped (platform).
- Negative injection, 3 mutations:
N11 remove the liveness downgrade -> "a session that stopped
for ten minutes is no longer running" goes red
N12 stretch the window to ten hours -> the same checks go red
N13 stop counting "stopped" as the end of a round
-> "a stopped session that comes back counts as a new
round" goes red
- Live, on this machine: the deployed copy was temporarily given a
five-second window, and the pill then showed the stopped state
with a grey dot and the word "stopped" next to a session whose
log was quiet. The window was restored to five minutes afterwards.
Test evidence
- scripts/test-session-tracker.ps1 -> 52 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
- 3 negative injections -> each turned its own check red; the suite
was green again after every revert
Design compliance
- The list keeps its three states: nothing is removed from it, so a
session that stopped stays visible and says why it is idle.
- Sorting follows the same order: running first, then waiting, then
stopped.
- No new dependency, no network, no credentials. The change is one
threshold and one extra state.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
The pill could say "waiting for approval" and "done"; the list could
say "running", "waiting" and "stopped". Two different vocabularies for
the same thing, and they disagreed: when the user answered a question,
the pill said "waiting for the user" (the last message was role=user,
which the detector reads as idle) while the list said "running" (the
tracker read role=user as "the agent is about to move").
The user could see one session showing two different things at once, in
two places on the same screen.
The fix
The list now produces the pill's state vocabulary and nothing else:
working the agent is busy -> running
thinking the agent is thinking -> thinking
waiting the agent is waiting for approval -> waiting for approval
idle the agent is done, or the user just spoke -> waiting for you
done a tool finished -> done
error a tool failed -> error
Same words, same colours, same meaning in both places. The tracker
reads the message the same way the detector does -- the same scan for
toolCall / thinking / text, the same ask_user check -- and the list
just renders the result.
The "stopped" state stays: it is not part of the pill's vocabulary, and
it is not a message type. It is what a session looks like when its last
message says "running" but nothing has happened for five minutes.
The round-end bookkeeping follows the new vocabulary: idle, waiting,
done, error and stopped all end a round, and working or thinking starts
the next one. That is what releases a dismissed session.
Validation
- scripts/test-session-tracker.ps1 grew from 52 to 57 checks. The new
ones pin the whole vocabulary: toolCall -> working, assistant text
-> idle, ask_user -> waiting, user just spoke -> idle, tool result
-> done, failed tool -> error, thinking only -> thinking. Plus the
liveness fallback for a thinking session, which was not covered
before: a session stuck in thinking used to stay "thinking" for
ever, the same class of bug as the one that was "running" for ever.
- The suite on this machine: 57 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Negative injection, 5 mutations:
N14 a tool result is no longer "done"
-> "a tool result means done" goes red
N15 the user having just spoken is "running" again (the old list
semantics) -> "the user just spoke means waiting for you"
goes red
N16 assistant text becomes "waiting" again
-> "assistant text means waiting for you" goes red
N17 the thinking state is gone
-> "an assistant that only thinks is thinking" goes red
N18 the liveness fallback no longer knows about thinking
-> "a thinking session that stopped is downgraded" goes red
- Live: the list showed "running" in blue and "done" in green, with
the agent's own last sentence as the second line.
Test evidence
- scripts/test-session-tracker.ps1 -> 57 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The pill's own behaviour is untouched. This change only makes the
list agree with it.
- The states file keeps its "phase" field name, so a sessions.json
written by an older detector still loads; the old "running" value is
still understood.
- No new dependency, no network, no credentials.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
The list said "done" for any session whose last message was a tool
result. Read in the list -- where one row is one session -- that says
"this session is finished". It is not: the tool finished, the round did
not. The agent still has to write its next message, or call the next
tool, or hand control back.
So the pill and the list were answering two different questions and
showing it in the same words. The pill's "done" is a flash of feedback
about the last action ("done" plus "Read", "Edited"); the list's "done"
sat there for as long as the session did, saying something that was not
true.
A tool result -- successful or failed -- now means "working". The round
is still going. The list says "running", the agent's own last sentence
or the tool it is calling fills the second line, and the only thing
that ends a round is the agent finishing its turn, which is "waiting for
you".
That also makes the two places agree without making them identical. The
pill still flashes "done" when a tool finishes -- it is answering "what
just happened". The list answers "what state is this session in", and
a session that just ran a tool is running.
The "stopped" state is unaffected: it is not a message type either, it
is what a session looks like when its last message says "running" but
nothing has happened for five minutes.
Validation
- scripts/test-session-tracker.ps1 grew from 57 to 59 checks. The new
ones pin the tool result in both message formats: a tool result in
messages.jsonl and the same event in ledger.jsonl both mean
"working", not "done" and not "idle".
- The suite on this machine: 59 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Negative injection, 6 mutations:
N14 a tool result counts as the end of the round
-> "a tool result means running" goes red
N19 the same rule broken in the ledger branch
-> "a ledger tool result means running" goes red
N15 the user having just spoken is "running" again
-> "the user just spoke means waiting for you" goes red
N16 assistant text becomes "waiting" again
-> "assistant text means waiting for you" goes red
N17 the thinking state is gone
-> "an assistant that only thinks is thinking" goes red
N18 the liveness fallback no longer knows about thinking
-> "a thinking session that stopped is downgraded" goes red
- Live: the list shows running, running and waiting for you. The pill
above it still says "done" for the instant after a tool finishes.
Test evidence
- scripts/test-session-tracker.ps1 -> 59 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The pill's own behaviour is untouched. It still flashes "done" after
a tool, which is what a user wants to see at that moment.
- The list keeps the session-level vocabulary only: running, thinking,
waiting for approval, waiting for you, stopped.
- No new dependency, no network, no credentials.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
Clicking a session in the list opened a new window running
"mcode --session <id>". The session was already running somewhere, so
there were now two terminals for one session: the old one, still going,
and the new one, just started. The ask was to bring back the window that
was already there.
How the session is found
Windows Terminal names its tabs "<state> | <session name> | MCode" --
the name is set by mcode itself through an ANSI OSC 0 sequence. The
pill's own window cannot do the switching: Win32's SetForegroundWindow
only activates the window's CURRENT tab, which is not the tab you want.
UI Automation can enumerate the tabs and select the right one. Verified
by reading the selection back after switching: the target tab was the
selected one.
The tab's name is whatever the session was called at some moment, while
the title the tracker holds is the newest one (the runtime keeps it in
its SQLite). They drift apart. So the tracker now records every title a
session has had -- the newest one and the first user message, which is
usually the name the tab is showing -- and the widget matches against
all of them.
What happens when it does not match
Nothing opens a second terminal. The Terminal window is brought to the
front instead and the user finds the tab themselves. A name that no
longer matches should cost the user one glance, not a duplicate
process. A new terminal is opened only when there is no Terminal window
at all to bring forward -- the desktop app's own embedded terminal is
like that, and there opening one really is the only way to reach the
session.
The "ack" that dismisses the row from the list is unchanged: clicking
still means "I am dealing with this one now".
Validation
- scripts/test-session-tracker.ps1 grew from 59 to 61 checks: the
title history has to reach sessions.json, and it has to contain the
first user message, not just the current title.
- The suite on this machine: 61 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Negative injection, 7 mutations, each one expected to turn a
specific check red -- including "the title history is not recorded"
-> "sessions.json keeps the title history" goes red.
- Live, on this machine, clicking a row and counting the tabs in
Windows Terminal before and after:
* a session whose name matches a live tab: "session terminal
switched", the target tab became the selected one, and the tab
count did not change;
* a session whose name does not match: "session terminal
activated (tab not matched)", the Terminal came forward, and
the tab count did not change either.
Before this change the same two clicks left 7 and 8 tabs behind
where there had been 6.
Test evidence
- scripts/test-session-tracker.ps1 -> 61 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The name matching is a heuristic and is treated as one: it decides
which tab, never whether a second terminal appears.
- UI Automation is Windows-only and the plugin is Windows-only. If the
assemblies or the window are not there, the code falls through
instead of failing.
- No credentials, no network, no telemetry, no third-party service.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
The list showed "I will close the 5h usage chain next. First look at the
current state file and the two write paths." where it used to show
"review the new replies on MiniMax-Code-Plugins PR 62". The title was
not gone -- it had fallen back to the first user message, because the
real title could not be read any more.
The cause is a PowerShell variable-name trap. Variable names are
case-insensitive, so "$home" is the read-only automatic variable
$HOME, the user's home directory. Assigning to it throws
"VariableNotWritable". The assignment sat inside the try/catch that
guards the title read, so the exception was swallowed and the read
returned an empty map -- 11 ms, no node process started at all. The
tracker then fell back to the first user message, which is a sentence
rather than a name.
Fixed by not using that name. The read now returns 413 titles in
196 ms, and the session's title is the name again.
It was my own bug, introduced while making the read work on a host
without USERPROFILE: the fallback path introduced a local $home. It
never showed up in the tests, because the tests point the read at a
temporary database through MCODE_RUNTIME_STATE_DB and never reach the
fallback branch, and because a swallowed exception is invisible from
the outside -- the feature simply did not work.
Two checks now guard it:
- the source of Runtime-Titles.ps1 may not contain "$home" any more;
- Get-UserHome has to return something.
Neither of them would have caught this on their own. What caught it
was the list on screen showing the wrong thing, which is why the title
is also read back from the live database after the fix.
Validation
- scripts/test-session-tracker.ps1 grew from 61 to 63 checks.
- The suite on this machine: 63 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Live: the list now reads "review the new replies on
MiniMax-Code-Plugins PR 62", and the title history holds both that
name and the earlier sentence.
- A scan of every .ps1 in the plugin for assignments to read-only
automatic variables ($HOME, $PID, $PROFILE, $HOST, $MATCHES, ...)
found no other one.
Test evidence
- scripts/test-session-tracker.ps1 -> 63 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The fallback behaviour is unchanged: when the runtime title cannot
be read, the first user message is used. That fallback is what made
the bug visible instead of silent.
- No new dependency, no network, no credentials.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
…said
The second line of a row showed "<background-task-finished> Local
backend...". The system pushes background-task notifications, task
reminders and agent context into the session log as user messages.
They are not something anyone said, and they were being used as the
summary, and as the title when the runtime title was unavailable.
The title and the summary now skip anything that starts with one of
those injected tags and keep looking. A session whose last message is
only an injected notification simply has no summary, which is honest.
Validation
- scripts/test-session-tracker.ps1 grew from 63 to 65 checks: an
injected message must not become the preview, and the title must
not come from one either.
- The suite on this machine: 65 pass, 0 fail for the tracker, and
133 pass, 0 fail, 1 skipped (platform) for the behavioural suite.
- Negative injection, 2 mutations:
N21 the preview stops skipping injected messages
-> "an injected message is not the preview" goes red
N22 the title stops skipping injected messages
-> "the title does not come from an injected message" goes red
- Live: the list shows the session's name on the first line, and the
second line is empty rather than a wall of XML.
Test evidence
- scripts/test-session-tracker.ps1 -> 65 pass, 0 fail
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- Only the injected tags are filtered, and only at the start of the
text. A person writing "<hello>" still gets a summary.
- No new dependency, no network, no credentials.
- UTF-8 with a BOM, LF line endings: both scripts are dot-sourced or
run by Windows PowerShell 5.1 on a zh-CN host.
The bottom row of the list was clipped -- half a row of it, with empty
space below it inside the window, which made it look even more like a
drawing mistake than a layout one.
The height was computed rather than measured. The list was given
"rows x 48px", and the window was given that plus a constant. Two things
were left out:
- SessionPanel carries a 14px top margin, 12px of padding and a 1px
border of its own -- 28px that never showed up in the arithmetic;
- a row is 46px, not the 48 that was assumed.
With two rows the content needed 106px, the row was given 94px, and the
last row lost 12px. The window still had 14px of slack at the bottom,
because that 14px was the margin, which lives outside the content.
The height is now measured instead of guessed: row 1 is set to Auto, the
layout is forced, and the window is sized from the panel's DesiredSize
plus the margin. That is the height the content actually asks for, and
it stays right when a row is taller than usual.
Two comments in this file had been carrying a replacement character --
text that had been mangled when it was written. One of them ("the
verified ... of it") was left over from an earlier change and was never
looked at; the other came with this one. Both are fixed here, and the
whole plugin was scanned: no replacement characters are left.
Validation
- The behavioural suite: 133 pass, 0 fail, 1 skipped (platform).
- Live, on this machine: with two sessions in the list the window is
now 205px tall instead of 192px, the last row's bottom sits at
191px and the window's bottom at 219px -- the row is whole, with
28px of slack instead of a clipped half row.
- The layout is measured on screen, so there is no unit test for it
here; the numbers above are the measurement.
Test evidence
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The pill itself does not move: row 0 stays locked to the pill
height, and the list grows below it, exactly as before.
- The 300ms collapse grace period is untouched.
- UTF-8 with a BOM, LF line endings: the file is dot-sourced or run by
Windows PowerShell 5.1 on a zh-CN host.
Clicking a session switched the tab but nothing appeared: the window
stayed behind whatever the user was looking at.
The cause is the foreground lock. SetForegroundWindow is refused for a
process that is not already in the foreground, and the widget is one.
The tab had switched -- the title bar said so -- but the window never
came forward, so from the user's side the click did nothing.
The widget already had this solved for the pill's show/hide toggle: set
the window TOPMOST, then NOTOPMOST again, which lets BringWindowToTop
push it all the way up, and only then SetForegroundWindow. The new code
calls that same routine instead of calling SetForegroundWindow on its
own.
The log now says which process is in the foreground afterwards, so a
switch that did not take is visible instead of silent.
Validation
- Live, on this machine: the foreground window before the click was
the desktop app (pid 2496); after the click it was
WindowsTerminal.exe (pid 59084). The window came forward.
- The behavioural suite: 133 pass, 0 fail, 1 skipped (platform).
Test evidence
- node scripts/test-substep-progress.mjs -> 133 pass, 0 fail, 1 skipped
Design compliance
- The pill's own focus path is untouched; this reuses it.
- No new dependency, no network, no credentials.
- UTF-8 with a BOM, LF line endings: the file is dot-sourced or run by
Windows PowerShell 5.1 on a zh-CN host.
Clicking a row in the session list switched the terminal tab but never
brought the terminal window to the front. From the user's side nothing
happened.
Root cause: `$pid = [uint32]0` in Focus-TerminalWindow and in
Get-ForegroundPid. PowerShell variable names are case-insensitive, so
`$pid` IS the read-only automatic `$PID`; the assignment throws
"Cannot overwrite variable PID because it is read-only or constant".
Both call sites sit inside a `try { } catch { }`, so the exception was
swallowed: Focus-TerminalWindow returned $false before it ever called
Focus-Window, and Get-ForegroundPid always returned 0. The caller
ignored the return value and logged "session terminal switched:
... foregroundPid=0" anyway, so the log looked like a success.
The same class of bug already bit `$home` in Runtime-Titles and `$args`
in the detector. It is worth a guard, not just a rename.
Fix:
* rename the variable to $procId in both functions
* read the foreground window back after every focus attempt
(Test-ForegroundPid) and log FOCUS CONFIRMED / FOCUS NOT
CONFIRMED. SetForegroundWindow fails silently, so a log line can
never be the evidence that the window came forward
* Focus-TerminalWindow falls back to the z-order recipe the pill's
own toggle already uses (SetWindowPos HWND_TOP without
SWP_NOACTIVATE, which activates the window)
* a failed focus must never fall back to opening a second terminal:
Select-SessionTerminal and Activate-TerminalWindow now report the
terminal window as found even when the foreground could not be
confirmed, so one session never ends up with two terminals again
* Focus-Window's log line says DONE instead of OK -- it only means
the calls did not throw
Test evidence:
* node scripts/smoke.mjs -> 153 pass / 0 fail
* node scripts/test-substep-progress.mjs -> 133 pass / 0 fail / 1 skipped
* pwsh -File scripts/test-session-tracker.ps1 -> 65 pass / 0 fail
* pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK
* new guard in smoke.mjs fails the build on any assignment to a
read-only PowerShell automatic variable ($pid, $home, $host, ...),
including inside a param() block. Negative injections N23-N27 make
it fail, N28-N31 (comment / string / here-string / the `$null =`
discard) must not fire, and the tree restores byte-identical.
* real-machine end-to-end with a real mouse_event LEFTDOWN/LEFTUP on
the session row: with this build the foreground window becomes
WindowsTerminal.exe and the selected tab is the clicked session.
With the previous build (the parent commit) the foreground stays on
the app (pid 2496) and the same check fails. The log now shows
"FOCUS CONFIRMED: pid=59084" followed by
"session terminal switched: ... foregroundPid=59084", where it used
to say foregroundPid=0.
Design compliance:
* one commit, one thing
* reuses the existing Focus-Window / Toggle-Window recipe instead of
introducing a third way to steal the foreground
* a session click never opens a new terminal window
Clicking a session row brought the terminal forward, but as a ~480x84
strip in the top-left corner of the screen instead of a full-screen
window. The user reported it with a screenshot: "点完它是在左上角小小
的一条,而不是全屏".
Root cause: Focus-Window's two SetWindowPos calls pass cx=0/cy=0 and
X=0/Y=0 as a shorthand for "keep the current size / position", but
that shorthand only holds when the matching flag is present. The flags
were SWP_NOACTIVATE alone:
SetWindowPos(hwnd, HWND_TOPMOST, 0, 0, 0, 0, SWP_NOACTIVATE)
SetWindowPos(hwnd, HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOACTIVATE)
Windows therefore read them as "resize to 0x0" and "move to (0,0)".
Windows Terminal clamps a 0x0 request to its minimum size, which is
the 480x84 strip, and the window really moved to the top-left corner.
The window's restored ("normal") placement then stayed 480x84@(0,0),
so every later restore brought the strip back.
The bug was latent until the previous commit made Focus-TerminalWindow
actually reach Focus-Window -- before that, the $PID collision threw
first and the whole focus step never ran. This file already documents
the trap in Toggle-Window ("SWP_NOSIZE: cx=0/cy=0 缺它会被 Windows 当成
resize 到 0x0, 触发 WT 的 min-size 兜底, 变成 480x76 strip"), it just
was never applied to Focus-Window.
Fix:
* Focus-Window's SetWindowPos calls now carry SWP_NOSIZE | SWP_NOMOVE
(plus SWP_NOACTIVATE), so they only touch the z-order
* Focus-Window no longer calls SW_SHOW on a window that is already
visible -- pointless state churn on an already-maximized window
* "show it full-screen" now lives in one helper, Show-WindowFullScreen,
used by both the pill's toggle and the session-list click, so the
two paths cannot drift apart again
* the helper runs SW_RESTORE -> SetWindowPos(work area) -> SW_MAXIMIZE,
in that order. The SetWindowPos has to happen while the window is
in its normal state: on an already-maximized window Windows does not
update the restored size, so a stale size (the 480x84 strip) would
survive the next restore. SetWindowPlacement was tried and does not
work on Windows Terminal -- the rcNormalPosition it is given reads
back as 0x0.
Test evidence:
* node scripts/smoke.mjs -> 154 pass / 0 fail
* node scripts/test-substep-progress.mjs -> 133 pass / 0 fail / 1 skipped
* pwsh -File scripts/test-session-tracker.ps1 -> 65 pass / 0 fail
* pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK
* new guard in smoke.mjs fails the build on any SetWindowPos that
passes cx=0/cy=0 without SWP_NOSIZE, or X=0/Y=0 without SWP_NOMOVE.
Negative injections N32-N34 fire it; N35/N38 (an explicit size with
SWP_NOMOVE, work-area coordinates as variables) must not; the tree
restores byte-identical.
* the round-15/16 drift locks moved with the implementation: they now
check Show-WindowFullScreen and both of its callers instead of the
text inside Toggle-Window. Negative injection N36 (SW_MAXIMIZE
regressed to SW_SHOW) and N37 (the session click stops
full-screening) fire them.
* real-machine end-to-end, real mouse_event LEFTDOWN/LEFTUP on the
session row: with this build the foreground window is Windows
Terminal, its rect is 2576x1408 (covers the 2560x1392 work area)
and its restored placement is 2560x1392 -- a real full-screen size.
With the previous build the foreground stays on the app and the
window is a strip; the same check fails.
Design compliance:
* one commit, one thing
* reuses the existing Toggle-Window full-screen recipe instead of
introducing a fourth way to raise a window
* a session click never opens a new terminal window
The session list itself landed in 8cc8df5..71436c8 (tracking every live session, the real session name, the list layout, the shared state vocabulary) and the click behaviour in a644ab4, with the focus and full-screen fixes in f9c98da and 0c22cc7. This commit packages the feature for release: the plugin version, the description in plugin.json, and the README section that documents what the list shows and what a click does. Why this is one commit rather than part of each feature commit: the version number, the marketplace description and the README are release artifacts, and they only have a single correct value once the feature is done. Splitting them across the feature commits would have meant claiming "0.5.0" in the first of them. What changed: * plugin.json: version 0.4.0 -> 0.5.0, and the description now says what the session list does -- every live session on hover, the real session name from the runtime's SQLite store, the last message, the relative time, and a click that switches back to that session's terminal tab without ever opening a second terminal. * README.md: a new "The session list" section covering the row layout, where the title comes from (and the fallback), why the state words are the pill's words, what the extra 已停止 state means, and the click behaviour including the "never open a second terminal" rule. * smoke.mjs: the version check moves with the version. No behaviour change in this commit -- the widget and the detector are byte-identical to 0c22cc7. Test evidence: * node scripts/smoke.mjs -> 154 pass / 0 fail * node scripts/test-substep-progress.mjs -> 133 pass / 0 fail / 1 skipped * pwsh -File scripts/test-session-tracker.ps1 -> 65 pass / 0 fail * pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK
Round 23: the session listThis PR now carries the session list on top of the sub-step progress The featureResting the mouse on the pill expands a list of every session that is Each row carries four things:
The title is the session's name, not a summary -- that is the whole The list reuses the pill's vocabulary instead of inventing its own: Clicking a row goes back to that session. It never opens a second The detector writes The commits
A bug this PR already had
[WinAPI]::SetWindowPos($Target.Hwnd, [WinAPI]::HWND_TOPMOST, 0, 0, 0, 0, [WinAPI]::SWP_NOACTIVATE) | Out-NullWindows read that as "resize to 0x0" and "move to (0,0)". Windows Round 19 item #3 already asked for Fixed in One more of the same familyThe new focus code assigned to Test evidence
Both new guards were negative-injected: changing one line makes the The window behaviour was verified on the real machine, not just in the
The same check also asserts the window's restored placement is a real |
Review response: all four blockers, and where they landedThis is a response to the two Current head is
09-25 #1 -- typed text
09-25 #2 -- the detector erasing the sub-step fieldsThe detector now carries the fields through its rewrite instead of 10-09 #1 -- the metadata refreshThree changes, together:
The regression is 10-09 #2 -- structured
|
…ktop app The session list is only worth having if a click takes you somewhere. On the desktop app the session lives in the app's sidebar, and the row click had exactly one way to go: fall through to "open a new terminal", which is the one thing the list promised never to do -- it spawns `mcode --session <id>` and leaves a second terminal running the same session, next to the app that already had it. So the click now has somewhere better to go. The app's sidebar renders every conversation as a Button whose accessibility name is the conversation title, so UI Automation can invoke it -- the same technique the list already uses for Windows Terminal tabs (there with SelectionItemPattern.Select, here with InvokePattern.Invoke). The app registers a `minimax-cn://` deep link, but it only routes one action, `navigate` (a whole-page URL), and its own 275 routes are all `/minimax-desktop/api/v1/...` APIs -- there is no URL that identifies a conversation, so there is no supported entry point and the sidebar has to be clicked. The order is now: CLI profile terminal tab -> terminal window -> open a new terminal desktop profile terminal tab -> app conversation -> app window The pin and the terminal tab keep their priority: a session that really is open in a terminal tab is switched there, exactly as before. The desktop profile simply gets the app conversation before it would ever open a second terminal. The lookup filters by name (47ms measured) instead of walking the app's accessibility tree (540ms for 2180 elements), because the click handler runs on the UI thread and a half-second scan would freeze the pill. Title matching has the same "two conversations with the same title" caveat the terminal path always had; when no row matches, the click falls through to the profile's fallback, and the log says which path it took. Validation ----------- * Live, end to end: with the real widget running, hovering the pill expanded the list, clicking the row for "了解 mcode 的 miniapp" (a session that has no terminal tab) logged `APP: desktop app MiniMax Inside Code`, `app conversation switched: 了解 mcode 的 miniapp`, and the app's window came to the foreground. * The same run also showed the terminal path still winning when the session really is in a terminal tab (`session terminal switched`, foreground WindowsTerminal). * Behavioral test: the UIA lookup and the app lookup are stubbed, so the contract is checked without the app running -- a matching row is invoked, a session that is not in the sidebar falls through, and no app running falls through. * Drift locks: the session click must try the app conversation, the desktop branch must return before the new-terminal fallback, and the lookup must filter by name. All three were verified by negative injection (each mutation trips its lock). Test evidence ------------- * node scripts/smoke.mjs -> 158 pass, 7 warn, 0 fail (was 154). * node scripts/test-substep-progress.mjs -> 138 pass, 0 fail, 1 skipped. * pwsh -File scripts/test-windows-workflow-local.ps1 -> all 5 steps OK. * Windows PowerShell 5.1 parser: 0 errors on mcode-island.ps1. Design compliance ----------------- * One implementation, two profiles: the difference is the existing $script:clickPrefers switch, so the Marketplace builder needs no new rule for this. * No credential, no network request, no new file. UI Automation talks to the local app window, exactly like the terminal path already does.
The session list (v0.5.0) brought a 500-line suite of its own, and it was in no workflow and no local run: the feature's core tests only ran when somebody remembered to run them by hand. A suite that is not wired into CI is a suite that rots. The workflow and the local runner now run it as step 6. Also ignores *.bak-r16, the stale widget backups that debugging sessions leave behind.
The Marketplace review rejected 0.9.0 with five points about the session list. All five are addressed here, in the plugin source, so both builds get them. 1. The detector read every historical session log every second and threw the result away. The expiry check now comes BEFORE the read: a session that has not moved is never opened. On this machine that is 400+ logs a second, every second, read for nothing. 2. The runtime's SQLite is no longer read, and the MiniMax Code programme is no longer borrowed to run a script. The session title is the first thing the user said in that session -- the only source left, and the one the app itself shows. `scripts/lib/Runtime-Titles.ps1` is gone, and with it the ELECTRON_RUN_AS_NODE trick and the node lookup that needed it. 3. The conversation click is scoped to the sidebar's conversation list. It used to look for a button with that name anywhere in the window, which is not a conversation click: the same name can be a heading or a button in another panel. The list is now found first -- the one element that holds SEVERAL known session titles -- and the target is looked up only inside it. The search still filters by name (47ms measured) instead of walking the whole accessibility tree (540ms), because the click handler runs on the UI thread. 4. Titles and previews are things a user said, so they can contain a key. Both are truncated (40 and 120 characters) and passed through the same secret redactor the plugin uses for log output BEFORE they are written to sessions.json and before they are drawn on screen. The README now states exactly which data is read: the first user message, the last message, and the log's mtime. Nothing else -- not the runtime's database, not the MiniMax Code programme. 5. The list is cleaned up. It is "the sessions that are alive now", not a history: a session that has not moved for 30 minutes is dropped from the list and from sessions.json, and comes back when it moves again. Without it, a morning of twenty conversations left twenty rows on the pill and twenty conversations' worth of text in the state file. Validation ----------- * The tracker's own suite: 70 pass / 0 fail (was 65). The new checks cover the expiry-before-read order (the read is counted, and an expired session's log must not appear in it), the redaction of the title and of the preview, the redaction of what lands in sessions.json, the truncation, and the prune (the session leaves the list AND the file). * The behavioural suite: 139 pass / 0 fail. The conversation switch is still stubbed at the UIA boundary, and it now also asserts that the lookup is handed the known titles -- that is what scopes it to the conversation list. * node scripts/smoke.mjs -> 160 pass, 7 warn, 0 fail. The new locks: the conversation list is resolved first, the button lookup goes through it, and the lookup does not start from the window root. * Negative injection: four mutations (the expiry check moved back after the read, the preview not redacted, the prune removed, the button lookup back to the window root) each trip at least one lock. * Live, against the running desktop app: the scoped lookup found the conversation list at depth 1 (the sidebar's list, not the window), located the row inside it, and invoked it -- "app conversation switched". The terminal path still wins when the session really has a terminal tab. Design compliance ----------------- * The session list keeps working without the runtime's database: the title falls back to the first user message, which is what the app shows in its own sidebar. * No credential, no outbound request, no file outside %APPDATA%\mcode-island. * One implementation, two profiles: the CLI / desktop difference is still the single $script:clickPrefers switch.
The Marketplace build and the audit reject the package if it contains the name of the environment variable that turns the MiniMax Code programme into node -- and the tracker test mentioned it in a comment, so the package build failed on the test's own wording. The check now describes the thing without spelling the variable, and the assertion still matches the usage.
…mment The Marketplace build and the audit reject the package if it contains the name of the environment variable that turns the MiniMax Code programme into node. The tracker's comment still spelled it, so the package build failed on the source's own wording. The comment now describes the thing without spelling the variable.
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head ff25a4b.
Blocking findings:
-
The PR deletes the complete existing plugins/weekbin/mcode-trajectory-studio and its workflow relative to base 39ddd2c: 161 files and about 45168 lines. This is an unrelated product deletion and must be reverted or explicitly justified and approved.
-
Command injection in plugins/antianqi/mcode-island/mcode-island.ps1:933-943: Start-SessionTerminal interpolates the session id into a PowerShell -Command string. The id is decoded from session directory names in scripts/lib/Session-Tracker.ps1:81-90 without a strict allowlist, and restored sessions.json ids are trusted. A crafted id such as x; Set-Content $env:TEMP\pr62-pwned 1; can execute arbitrary PowerShell when a row is clicked. Validate the id and pass it as a non-evaluated argument; add a regression test.
-
Get-SessionTitle and Get-SessionPreview in scripts/lib/Session-Tracker.ps1:140-193 and 303-323 write user or assistant text to sessions.json and render it in the always-on-top pill without Protect-SecretText. Tokens, passwords, or API keys in prompts can therefore be persisted and displayed. Redact before persistence and add JSON, Bearer, JWT, and key-value canaries.
-
The tracker has no maximum session count, preview size, sessions.json size, or UI height. Session-Tracker.ps1:400-579 retains stopped sessions and serializes all full previews, while mcode-island.ps1:617-633 creates a row for every entry. Add bounded data and eviction tests.
-
If title matching cannot find an existing terminal, mcode-island.ps1:929-943 starts another mcode --session terminal. This contradicts the no-second-terminal contract and can duplicate a live session. Fail closed or prove the session is not already running before launching a new terminal.
The current repository-owned CI, CodeQL, Windows job, and validate checks are green, but they do not cover these blockers.
Review round 3 on ff25a4b raised five blockers. This commit closes four of them; the fifth (the deleted weekbin plugin) was a merge artefact and is restored by the upstream merge in f246247. 1. Command injection in the session-click path. Start-SessionTerminal built `powershell -Command "mcode --session <id>"` from a value that comes from a session directory's base64 and from sessions.json -- neither is trusted. An id shaped like `x; Set-Content ...` was executable PowerShell. The function is deleted rather than quoted more carefully: the point is to remove the path that turns data into code, not to sanitise it. On top of that, both entry points now check the id's shape (^[A-Za-z0-9][A-Za-z0-9_-]{0,127}$): Get-SessionIdFromDir drops a directory whose decoded name is not an id, Restore-SessionTracker drops such an entry from sessions.json, and Invoke-SessionClick refuses it. 2. "No match" no longer opens a terminal (fail closed). The click handler logs why and stops. A session that runs in a host we cannot see (embedded terminal, another terminal app) would otherwise get a second terminal -- the exact situation the list exists to prevent. 3. Bounds on the list. Data side: MAX_SESSIONS = 20, evicting by last activity. Only idle sessions (log untouched for the 60s activity window) are evicted: an active session that gets evicted is re-added by the next scan, so evicting it produces a per-tick evict/re-add flicker. Active sessions may therefore briefly exceed the cap; the UI side is what bounds the screen. UI side: MAX_SESSION_ROWS = 12, a "还有 N 条" row instead of silent truncation, and the list height is clamped to the work area. A behavioural test caught a real bug in the first draft of the cap: @($count) made an array, so the subtraction threw. 4. Title/preview redaction. Both already went through Protect-SessionText on ff25a4b; what was missing was evidence of coverage. The tracker suite now dot-sources the real Protect-Text.ps1 instead of a stub that only knew `sk-`, and plants four canaries (JWT, Bearer, JSON token, kv password) in the title and the preview, asserting the raw value is absent, the redaction marker is present (so "absent" is not just truncation), and sessions.json is clean. Validation - scripts/smoke.mjs: 163 pass, 7 warn, 0 fail (new locks: no Start-SessionTerminal, no --session interpolation, click handler validates the id and spawns nothing, row cap, overflow notice, work-area clamp). - scripts/test-session-tracker.ps1: 110 pass, 0 fail (id whitelist, decoded directory names, restore path, cap, idle-only eviction, four canaries). - scripts/test-substep-progress.mjs: 139 pass, 0 fail, 1 skipped. Every new lock was negative-injected: 12 mutations, each turned its own assertion red, then the file was restored byte-identically and the suites went green again. A5 (the overflow notice) initially stayed green because the lock only looked for the call, not its guard -- the lock now matches the `if ($hidden -gt 0) { ... }` pair.
|
Round-3 blockers, all addressed in ad4d4ac (current head of this PR).
Verification: |
Two CI-only fixes for the merged tree, both caught by the first run of the
new head.
The token roundtrip step still asserted a plaintext `planApiToken` in
config.json. set-token.ps1 stores `planApiTokenEnc` (DPAPI, CurrentUser)
and removes the plaintext key, so the step died on its first assertion
("mismatch (got: )"). It now requires the encrypted field, forbids the
plaintext field, and after -Clear requires both to be gone. Verified
locally against the real set-token.ps1 in an isolated APPDATA: 4/4.
test-substep-progress.mjs section 11 Add-Type's the UIAutomation
assemblies, which do not exist on Linux. Every assertion in that section
failed in the ubuntu job. It now skips with the same
"[skip] ... (platform)" pattern the PostToolUse section already uses; the
contract stays covered on windows-latest, the only platform this code runs
on.



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.