Skip to content

Add plugin document-skills - #6

Merged
hetaoBackend merged 39 commits into
MiniMax-AI:mainfrom
Hylouis233:plugin/document-skills
Sep 11, 2026
Merged

hetaoBackend merged 39 commits into
MiniMax-AI:mainfrom
Hylouis233:plugin/document-skills

Conversation

@Hylouis233

@Hylouis233 Hylouis233 commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Migrated from hetaoBackend/MiniMax-Code-Plugins#12; earlier review history remains there. Current head: 2b9e253.


What changes

Adds plugins/Hylouis233/document-skills, a Skill-only portable Agent Plugin for producing and
verifying DOCX, PDF, PPTX, and XLSX artifacts with standard open-source Python libraries and public
file-format knowledge.

The four hosted Skill names are deliberately namespaced to avoid colliding with built-in Skills:

  • document-skills-docx
  • document-skills-pdf
  • document-skills-pptx
  • document-skills-xlsx

Each Skill routes work to the appropriate document library, defines safe edit boundaries, and
requires reopening/verification before an artifact is handed back. The package contains no content
from Anthropic or another proprietary skill pack.

User value

The guidance prevents common silent artifact failures: corrupt hand-edited OOXML, spreadsheet
values where formulas belong, PDF pages without extractable text, and presentation text that
overflows its shapes.

Example:

Use the document-skills plugin: open sales-2024.xlsx, add a Summary sheet with per-region formulas,
a native bar chart of the top five products, currency formatting, and then reopen the workbook and
report the verified ranges.

Integration and scope

  • Plugin files live under plugins/Hylouis233/document-skills.
  • The isolated .github/workflows/document-skills.yml installs only fixture dependencies plus
    LibreOffice Writer, validates the repository, and runs fixtures.integration.mjs.
  • The workflow does not change the root test command or global test concurrency.
  • test/hosted-plugins.test.mjs accepts both POSIX and Windows path separators, fixing the portable
    scaffold assertion that previously failed on Windows.
  • The workflow checkout pin now uses the canonical 40-character actions/checkout@v7.0.1 SHA.

Requirements and disclosure

  • Python 3.9+ for the documented Skills; fixture validation uses a pinned Python dependency set.
  • LibreOffice is required only for the DOCX render smoke tests in CI. Pandoc and LibreOffice remain
    optional for end-user workflows when the relevant validation path is not requested.
  • Windows, macOS, and Linux are supported.
  • The plugin itself performs no network access and contains no credentials, telemetry, native
    binaries, installers, or private endpoints. It operates only on files selected by the user and
    writes temporary validation artifacts under a temporary directory.

Current-head verification

Verified locally on macOS against head 2b9e253:

  • npm run check: 27 tests passed, 0 failed, 0 skipped.
  • npm run validate: all 9 hosted Plugins and both examples validated.
  • Document fixture integration: 6 passed, 0 failed, 0 skipped.
    • XLSX fixture passed.
    • PPTX fixture passed.
    • PDF fixture passed.
    • DOCX fixture passed, including real soffice conversion.
    • The PDF Skill accessibility-claim guard passed.
  • Fixture environment: Python 3.14.7 with the exact pinned requirements; LibreOffice available on
    PATH.
  • Workflow YAML parsing and git diff --check: clean.

The upstream PR currently has no GitHub-hosted run for this newly added workflow. The workflow and
fixtures are locally validated; no hosted Ubuntu result is currently available. A skipped
[code]smith result is not being presented as CI evidence.

Submission checklist

  • Namespaced Skill directories avoid built-in-name collisions.
  • Plugin manifest, README, Apache-2.0 license, requirements, platforms, and data behavior are disclosed.
  • Repository validation, root tests, and all four document fixtures pass locally on the current head.
  • Fixture CI is isolated to a dedicated workflow.
  • GitHub-hosted Document Skills workflow passes on the current head.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Hylouis233 and others added 30 commits August 15, 2026 03:43
Four original workbench Skills (docx, pdf, pptx, xlsx) for creating, reading,
editing, and verifying Office and PDF documents with standard open-source
Python tooling.

- lives at plugins/Hylouis233/document-skills
- plugin.json + README + Apache-2.0 LICENSE + four Skills with references
- original guidance written for this plugin; no proprietary content reused
- no network, no credentials, no MCP server; scripts referenced are the
  user's own installed Python libraries (python-docx, python-pptx, openpyxl,
  pypdf, reportlab, pymupdf), not bundled
- npm run validate passes; documented patterns smoke-tested end to end
Review fixes (all fixture-verified in tests/):
- PDF: AcroForm route now clones pages + /AcroForm into the writer via
  append() and fills fields on writer pages; mediabox postcheck reduced to
  width/height pairs; CMYK/ICC pixmaps converted to RGB before PNG save
- PPTX: table cells edited at run level (cell.text rebuild proven lossy);
  inventory walker recurses into group shapes; font triage resolves
  inherited fonts with theme major/minor fallback
- XLSX: sniffed CSV dialect passed to DictReader; round_trip_losses()
  detects parts an openpyxl round trip drops before saving
- DOCX: numbering restart via cloned <w:num> + startOverride, rendered
  proof in fixture (restarts at 1 vs style-reuse continuing at 4)

Depth references (original content, no proprietary material):
- docx cjk.md: east-asian font slot, 字号 table, char-based indent,
  fixed line spacing, GB/T 9704 page geometry, font availability risks
- docx scenes.md: academic paper / resume / official document / contract
  skeletons with scene-specific verification
- xlsx formatting.md: conditional formatting rules, structured tables,
  honest pivot aggregation routes (openpyxl cannot create pivots)

tests/: one runnable fixture per format, 30 assertions, all passing
locally (pdf 9, pptx 10, xlsx 6, docx 5 incl. soffice-rendered proof)
- xlsx edit: round_trip_changes() now also detects extensions stripped from
  retained parts (x14/extLst markers), not just dropped archive members
- docx scenes: signature blocks get row-level cantSplit + keep_with_next on
  all rows (rendered fixture proves the table stays on one page)
- docx cjk: tofu postcheck switched to glyph-coverage (fontTools cmap) since
  text extraction cannot detect missing-glyph boxes; style snippet now
  defines font names before use (NameError fix)
- pptx edit: locator candidate collection recurses into groups with a
  stable nested path for the uniqueness assertion
- xlsx formatting: aggregation formulas build sheet refs from ws.title
  (quoting when needed) instead of hard-coded Data!
- xlsx read: profiles every sheet by default, not just sheetnames[0]
- pdf create: escape() rule for plain text into Paragraph; fixture shows
  unescaped markup silently swallows <...> runs

Fixtures extended to 44 assertions, all passing locally
- pdf extract: real table detection via page.find_tables() with span fallback
- pdf postcheck: interactive-only pages (AcroForm widgets) exempt from text gate
- docx read: unified block walker yields tables inside w:sdtContent
- README: drop the untagged-ReportLab accessibility claim, state the limitation
- xlsx: fullCalcOnLoad contract so manual-calc workbooks recalculate on open
- fixtures: table detection, widget exemption, sdt table walker, calc flags (all green)
- pptx analyze: sparse XY/bubble cache points keep their idx so x/y/bubble
  values pair correctly across blank points
- docx cjk: Hangul (jamo, compatibility jamo, extended-A, syllables) routes
  through the eastAsia font slot
- xlsx edit: extension detection matches namespace URIs and the local name
  extLst instead of arbitrary XML prefixes
- xlsx formatting: header-only sheets skip conditional formatting instead of
  building inverted ranges openpyxl rejects
- docx read: table extraction walks real w:tc elements and annotates
  gridSpan/vMerge instead of the merge-expanded row.cells view
- pdf transform: stamps are scaled and centered per destination page via
  merge_transformed_page; rotated pages flagged for visual verification
- fixtures extended for all six (with negative controls)
- pdf postcheck: real overflow check - text blocks extracted through an
  enlarged clip rectangle and compared to the page box (plain extraction
  silently drops fully off-page text)
- docx read: table cells rebuilt per paragraph with visible tab/break
  markers instead of concatenated w:t text
- docx/pptx health checks: explicit require() raises so every archive
  limit survives python -O
- pptx analyze: run fonts resolve a:latin/a:ea/a:cs by the run's scripts
  before falling back to the Latin-only font.name; triage walks table
  cells (graphic frames have no text frame)
- docx SKILL: pandoc-only installs can serve read/extract requests
- pdf extract: inline content-stream images enumerated via dict image
  blocks alongside the XObject loop; coordinate spans position-sorted
  before emission
- xlsx read: implausible sheet <dimension> reset before streaming
- fixtures extended for the overflow check (with negative control), cell
  paragraph boundaries, dimension corruption, script faces, table triage
Address the latest review round: add bounded XLSX/OPC package preflight with compression, member, materialization and range-token budgets; route all existing-package loads through validated snapshots; harden DOCX CJK/locator, PDF Type3 and PPTX merged-slot guidance; cover CSV formula neutralization, missing dimension scanning and internal hyperlink rewrites with real OPC fixtures.
Copilot AI lite review requested due to automatic review settings August 19, 2026 09:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

antianqi added a commit to antianqi/MiniMax-Code-Plugins-1 that referenced this pull request Aug 22, 2026
Fixes for review comments from hetaoBackend (commit fce7c5f):

  #1 detector hard-coded path: resolve the [userprofile]/.minimax-code
     directory at runtime via the mcode node process cmdline (regex on
     @minimax-ai/code/cli.js), with fallbacks to $env:USERPROFILE/.minimax-code,
     $env:APPDATA/minimax-code, and the current working directory.
     Override with -Root [path].

  #2 idle fallback unreachable: mtime cache now returns the last inferred
     message instead of null, so the 60s stale -> idle branch fires every
     poll. Verified locally: idle :: already idle 195s after 65s of inactivity.

  #2b session log: prefer ledger.jsonl (mcode v2 event stream) and fall
     back to messages.jsonl when ledger is missing. Both formats are handled
     in Infer-State (kind/phase for ledger, message.role for messages).

  #3 PID reuse safety: start/stop-{island,detect-island}.ps1 now verify
     the target PID command line contains the expected script path before
     acting. Stale PIDs and PID-reused processes are refused with a
     REFUSED log line instead of being killed.

  #4 wrap-tool.ps1 shell-injection: removed Invoke-Expression entirely.
     The wrapper is now status-only; the agent runs the command via mcode's
     own bash tool and passes -ExitCode to publish the outcome.
     Documented in README + SKILL.md.

  MiniMax-AI#5 README: -Enable -> -Action Enable to match autostart.ps1 parameter set.

  MiniMax-AI#6 start-island.ps1 readiness: dropped the 'about to ShowDialog' log wait
     (which was never emitted). Now polls MainWindowHandle != 0 every 500ms
     for up to 8s.

Tests: validator reports OK plugin antianqi/mcode-island. wrap-tool
6-state matrix verified locally (working / done / waiting / error).

@hetaoBackend hetaoBackend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve the integration/scope issues before merge:

  1. The plugin exports generic Skill names docx, pdf, pptx, and xlsx, which collide with the built-in MiniMax Code skills using those names. Please either define the intended precedence/namespace and prove that installation cannot create ambiguous selection, or rename the hosted skills and update the documentation.
  2. This plugin also changes the repository-wide .github/workflows/ci.yml to install Python packages and LibreOffice for its fixtures. Please justify that global CI change (or isolate it to plugin-specific validation) and ensure the hosted-plugin check runs with the documented dependency setup.

The current [code]smith check is SKIPPED, so the collision and CI integration need explicit validation.

hetaoBackend pushed a commit that referenced this pull request Aug 25, 2026
…ax Code agents

* Add mcode-island plugin: Windows Dynamic Island status pill for MiniMax Code agents

Adds a Skill-first plugin that surfaces the agent working state in a 320x60 WPF pill anchored to the top center of the primary display, so the user can leave the terminal in the background and still watch progress.

States: idle / thinking / working / waiting / done / error.

Includes wrap-tool.ps1, a thin bash wrapper that pushes working / done / error / waiting based on $LASTEXITCODE, so the user does not have to remember to call notify-island.ps1 for every shell command.

* Add mcode-status-detect v0.2.0: state inference from mcode session log

Adds a 1-second-polling daemon that reads the active mcode session messages.jsonl and infers the agent state (idle/thinking/working/done/error) without requiring the agent to call notify-island.ps1.

State mapping:

  role=user                  -> idle

  role=assistant + toolCall  -> working "<tool>: <args>"

  role=assistant + thinking  -> thinking

  role=assistant + text      -> idle (just replied)

  role=toolResult + !isError -> done "<tool> 完成"

  role=toolResult + isError  -> error "<tool> 失败"

  mcode 进程不在              -> error "mcode 进程已退出"

  60s 无新事件                -> idle 兑底

Priority logic: agent-pushed states (with Message) are preserved; detector takes over only for settle states (idle / error).

Tested on Windows 11 24H2 + PowerShell 5.1 against a live mcode session. All 6 state transitions verified, including mcode exit and recovery.

* fix: address review feedback on PR #17 (v0.2.1)

Fixes for review comments from hetaoBackend (commit fce7c5f):

  #1 detector hard-coded path: resolve the [userprofile]/.minimax-code
     directory at runtime via the mcode node process cmdline (regex on
     @minimax-ai/code/cli.js), with fallbacks to $env:USERPROFILE/.minimax-code,
     $env:APPDATA/minimax-code, and the current working directory.
     Override with -Root [path].

  #2 idle fallback unreachable: mtime cache now returns the last inferred
     message instead of null, so the 60s stale -> idle branch fires every
     poll. Verified locally: idle :: already idle 195s after 65s of inactivity.

  #2b session log: prefer ledger.jsonl (mcode v2 event stream) and fall
     back to messages.jsonl when ledger is missing. Both formats are handled
     in Infer-State (kind/phase for ledger, message.role for messages).

  #3 PID reuse safety: start/stop-{island,detect-island}.ps1 now verify
     the target PID command line contains the expected script path before
     acting. Stale PIDs and PID-reused processes are refused with a
     REFUSED log line instead of being killed.

  #4 wrap-tool.ps1 shell-injection: removed Invoke-Expression entirely.
     The wrapper is now status-only; the agent runs the command via mcode's
     own bash tool and passes -ExitCode to publish the outcome.
     Documented in README + SKILL.md.

  #5 README: -Enable -> -Action Enable to match autostart.ps1 parameter set.

  #6 start-island.ps1 readiness: dropped the 'about to ShowDialog' log wait
     (which was never emitted). Now polls MainWindowHandle != 0 every 500ms
     for up to 8s.

Tests: validator reports OK plugin antianqi/mcode-island. wrap-tool
6-state matrix verified locally (working / done / waiting / error).

* fix(mcode-island): pick most-recently-touched session file (ledger vs messages)

Get-LatestSessionFile always preferred ledger.jsonl when present, regardless
of which file was more recently written. On systems where mcode v0.2.x left
behind a stale ledger.jsonl from a previous session, the detector would
read the old ledger every poll, the 60s idle-fallback would fire against
an ancient mtime, and the widget would stay stuck on "已静默 NNNNNs"
forever (verified: 49549s = 13.76h against a ledger that was actually
{"action":"test ledger 1"} test residue).

Fix: compare mtimes and pick whichever is newer. Fall back to ledger if
messages is absent (original fallback contract), but never let a stale
ledger shadow a live messages.jsonl.

Triggered by PR #17 review testing: 9 hours of "idle :: 已静默 49549s"
on a fresh detector after the v0.2.1 fixes were deployed.

* fix(mcode-island): tag notify-island status writes with source='agent'

notify-island.ps1 was writing status.json with only {state, message,
progress, ts} and no source field. The detector's takeover logic keys
off `cur.source -eq 'detector'` to decide whether the live entry is its
own or an externally-pushed one. With no source field on agent-pushed
states, the detector treated every agent push as "no current status" and
immediately overwrote it with whatever it had just inferred — most often
idle (60s fallback), even when the agent had just pushed `working` or
`thinking`.

Concretely: pushing `notify-island.ps1 -State working` would survive for
roughly 1 second before the detector's next poll clobbered it back to
idle. This made the manual notify tool useless for any state the detector
cares about, and made the `wrap-tool.ps1 -State working` wrap pattern
invisible on the pill.

Fix: add `source = 'agent'` to the payload. With it set, the detector's
existing precedence rules work as documented:

- agent push of working/thinking/done → preserved (not overwritten by
  the same-state detector inference, since detector-inferred
  working/thinking/done is not "settled" and does not trigger the
  takeover branch when the current entry is not the detector's own);
- agent push of idle/error → can be taken over by detector's
  idle/error inference, matching the original "detector settles agent"
  contract.

Verified live: `notify-island.ps1 -State thinking` now persists across
multiple detector polls (ts unchanged after 3.5s, message intact,
source field present).

Pushed on top of 6e99c0b on add-mcode-island.

* fix(mcode-island): kill pipeline-thread leak in detector hot loop

The detector polled once per second, and every poll walked ~15 pipeline
cmdlets: Get-ChildItem -Recurse | Where-Object | Sort-Object |
Select-Object (×2), Get-Content -Raw | ConvertFrom-Json (×3-4),
$collection | Where-Object (×3), Get-Process (×1-2), etc. PS 5.1 hidden
window has a known issue where completed pipeline tasks aren't
immediately released back to the Runspace thread pool — the pool backs
up over multi-hour runs. After ~9 hours of polling, the process was
holding ~30k threads and Get-ChildItem was effectively starved:
status.json stopped updating, island.log stopped appending, the
process looked alive but the loop was no longer advancing. Only a
restart recovered it.

Fix in three layers:

1. Replace the most expensive pipeline calls with direct .NET method
   calls so no Runspace hop is incurred:
   - Get-LatestSessionFile: Get-ChildItem -Recurse | Where-Object |
     Sort-Object | Select-Object  →  a single
     [System.IO.Directory]::EnumerateFiles + manual mtime scan
   - Get-McodePid: Get-ChildItem | foreach { Get-Content |
     ConvertFrom-Json | Get-Process }  →  EnumerateFiles + File.ReadAllText
     + Process.GetProcessById
   - Read-LastMessage: Get-Item  →  [System.IO.FileInfo]::new(...)
   - Read-StatusObj: Get-Content -Raw  →  File.ReadAllText
   - Infer-State (assistant branch): $m.content | Where-Object ×3  →
     one foreach loop with early exit (toolCall wins, no need to scan
     the rest)

2. Add a 5s TTL cache for both `mcodePid` and `latestSessionFilePath`
   in the main loop. mcode doesn't churn sub-second, and a fresh
   session log only shows up when mcode itself starts a new session,
   which is also a sub-5s event in practice. 5s is a comfortable
   upper bound that cuts the heavy directory enumeration to once per
   5s without losing visible state fidelity (the existing mtime gate
   in Read-LastMessage already gates re-parse on real content
   changes, so cache staleness is invisible to the user).

3. Verified live: after the fix, restarting the detector and running
   for 30s reports 18-28 threads (was previously climbing into the
   thousands within minutes). State transitions (working → done →
   working) still fire correctly. The 60s-idle fallback still fires
   correctly.

Side benefit: the refactor also fixes a tiny correctness wart in
Get-McodePid — when multiple .json files happen to coexist in
.mcode-active (e.g. during a restart overlap), the previous code
returned the first hit; the new code picks the most-recently-touched
one, which matches what Get-LatestSessionFile does on the messages
side.

Pushed on top of db73c11 on add-mcode-island.

---------

Co-authored-by: antianqi <antianqi@users.noreply.github.com>

@hetaoBackend hetaoBackend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当前 head 56eab83 仍有可复现的 fixture 阻塞:

  • plugins/Hylouis233/document-skills/tests/xlsx_fixture.py:29-33 仍读取 plugins/Hylouis233/document-skills/skills/xlsx/references/package.md,但当前实际目录是 skills/document-skills-xlsx/...;fixtures.integration.mjs 实测因此以 FileNotFoundError 失败。
  • 当前 fixture 结果是 PDF/PPTX 主要路径通过,XLSX 因上述仓库内错误路径失败;DOCX 的剩余失败是本机缺少 soffice,属于环境前置条件,不应掩盖 XLSX 的代码/fixture 错误。

请修正 xlsx fixture 的实际路径,并在依赖安装后重新运行 document-skills fixtures;确认 DOCX 的环境失败与代码失败分开记录。当前 [code]smith 为 SKIPPED。

@Hylouis233

Hylouis233 commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor Author

a132aa8 相对上一次 CHANGES_REQUESTED 所对应的 56eab83 有以下变更:

  • 四个 Skill 目录现为 document-skills-docx / document-skills-pdf / document-skills-pptx / document-skills-xlsx,不再使用会与内置 Skill 撞名的 docx / pdf / pptx / xlsx。
  • tests/xlsx_fixture.py 已改为读取 skills/document-skills-xlsx/references/package.md,不再使用旧的 skills/xlsx/... 路径。

DOCX 若缺少 soffice,属于环境前置条件,与 XLSX fixture 路径问题相互独立。

@Hylouis233

Copy link
Copy Markdown
Contributor Author

Current head is 2b9e253.

Current-head evidence:

  • npm run check: 27 passed, 0 failed/skipped.
  • npm run validate: 9 hosted Plugins + both examples validated.
  • fixtures.integration.mjs: 6 passed, 0 failed/skipped; XLSX, PPTX, PDF, and DOCX all executed, with the DOCX path completing a real soffice conversion.
  • Workflow YAML and git diff --check: clean.

The four Skill directories remain namespaced, the XLSX fixture uses the corrected namespaced path, and fixture CI remains isolated from the root test command. This head also corrects the Document Skills workflow's checkout pin to the canonical 40-character v7.0.1 SHA.

The PR body now reflects the actual files, platform scope, current head, and local evidence. It explicitly leaves the GitHub-hosted workflow result unchecked rather than treating the skipped [code]smith check as evidence.

@hetaoBackend hetaoBackend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current head 2b9e253 was re-reviewed. Pinned Python dependencies make XLSX/PPTX/PDF fixtures pass; the remaining DOCX fixture failure is only the local absence of soffice, while the workflow installs libreoffice-writer before running the fixture suite. The previous XLSX path issue is fixed, the plugin manifest validates, and no in-scope security blocker remains. [code]smith is SKIPPED and was not used as evidence.

…affold assertion

Resolve the scaffold-test conflict using upstream's native path.join assertion. Keep document fixtures and their isolated workflow unchanged. The resulting tree adds only document-skills and its workflow to upstream f19cd52.
@hetaoBackend
hetaoBackend merged commit 1acf3b3 into MiniMax-AI:main Sep 11, 2026
5 checks passed
antianqi added a commit to antianqi/MiniMax-Code-Plugins-1 that referenced this pull request Sep 29, 2026
…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' })   # -> ''
antianqi added a commit to antianqi/MiniMax-Code-Plugins-1 that referenced this pull request Sep 29, 2026
…ound-20 MiniMax-AI#8)

Round-20 MiniMax-AI#6 stopped the pill from rendering raw JSON, but the result was
still frequently uninformative:

    Running $ErrorActionPreference='Continue'

That is the opening line of most agent commands, and it says nothing about
what is being worked on. On a detached status pill it is close to
indistinguishable from the pill being stuck -- which is not a good look
for a widget whose entire job is to answer "what is it doing right now".

Format-ToolArgs now prefers the first line that actually EXECUTES
something, skipping variable-assignment and comment lines:

    $ErrorActionPreference='Continue'   <- skipped
    $env:TEMP="..."                     <- skipped
    npm test                            <- shown

The fallback matters: if every line is boilerplate, the first line is
still shown rather than leaving the pill blank. A summary that is
uninformative beats no summary at all, because the bare verb alone cannot
be distinguished from the detector having lost track.

Comment lines are skipped on the same rationale.

Tests: 3 cases, negative-first, all observed failing before the fix --
the preamble was shown, and the comment line was shown. A fourth case
(an all-assignment command) was written at the same time and passed
before the fix as well, which is the point: it pins the fallback so a
future "skip more lines" change cannot silently blank the pill.

Validation: sub-step suite 51 pass / 0 fail (was 49), smoke 116 pass /
0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows
PowerShell 5.1 (exit 0).
hetaoBackend pushed a commit that referenced this pull request Nov 10, 2026
….0) (#62)

* feat(mcode-island): add sub-step progress fields to status.json (v0.4.0)

Extends status.json schema with three optional fields — step, total,
detail — so agents can publish per-iteration progress during Computer
Use loops, multi-step plans, and long-running tool sequences. The widget
now renders "step N[/M] · detail" instead of only the coarse state, so
the user can see what the agent is doing right now without waiting for
it to finish.

  notify-island.ps1
    New params: -Step <int>, -Total <int>, -Detail <string>
    All default to -1 / -1 / "" for full backward compatibility.
    The status.json payload now writes step/total/detail alongside the
    existing state/message/progress fields. Old callers (omitting the
    new params) produce identical status.json behavior except for three
    extra fields whose values are the sentinels.

  mcode-island.ps1 (widget)
    New Build-DisplayMessage helper that converts the schema fields into
    the visible pill text. Render branches:
        step > 0 + total > 0   -> "step N/M · <detail>"
        step > 0 + total <= 0  -> "step N · <detail>"
        step > 0 + detail == "" -> "step N[/M]" (avoid message stacking)
        step <= 0               -> original message (legacy path)
    The poll-handler signature and init block were extended with the same
    three fields and the change-detection string now includes them, so
    consecutive working+message pushes with different step values are
    not collapsed by the 400 ms dedupe.

  io.minimax.mcode/hooks/scripts/_lib.ps1
    Push-Island now accepts -Step/-Total/-Detail and forwards them to
    notify-island.ps1. Format-ToolSummary has a new mcode-computer-use
    branch that extracts action + coordinate with explicit -join ","
    so coordinate arrays render as "(x,y)" not PowerShell's default
    "(x y)" (the latter looked like a truncated number on the pill).

  io.minimax.mcode/hooks/scripts/post-tool-use.ps1
    Pushes -Detail with the Format-ToolSummary output split so the pill
    shows "Bash ok · ls -la /tmp" instead of "Bash ok". pre-tool-use.ps1
    already used Format-ToolSummary so no change there.

All new schema fields are optional. notify-island.ps1 callers that
omit -Step/-Total/-Detail see no behavior change. Widget versions that
do not know the new fields ignore them (PSObject.Properties[name]
everywhere). Verified by smoke.mjs case "backward compat: old callers
produce step=-1, total=-1, detail=""" and test-substep-progress.mjs
case "Push-Island backward compat: missing new params -> step=-1,
total=-1, detail=""".

  no credentials      : none added; the IPC is local-filesystem only
  no network          : no network calls added; notify-island.ps1 still
                        writes status.json under %APPDATA%/mcode-island
  no telemetry        : no telemetry added; the existing append-only
                        island.log is unchanged and the 400 ms polling
                        cadence is unchanged
  no third-party svcs : no new third-party deps; the change is pure
                        PowerShell + schema
  cross-platform      : no hardcoded host paths; no /Users/ /home/
                        C:\ /mnt/ literals introduced; the existing
                        smoke.mjs cross-platform scan still passes
  atomic write        : notify-island.ps1 already writes status.json
                        atomically via staging + rename; no change
  closed schema       : status.json is open by design (not contract-
                        locked), but each new field has a documented
                        sentinel (-1 / -1 / "") so absent fields are
                        semantically equivalent to explicit sentinels
  smoke self-check    : smoke.mjs gained 5 new checks under section
                        5c1; locked the new surface contract so a
                        future refactor that drops the params surfaces
                        in smoke before reaching the slower pwsh-spawned
                        tests

  smoke.mjs                          : 48 pass, 7 warn, 0 fail
    (7 warn are pre-existing "forward" event catalog entries pending
     mcode 0.2.4+ Runtime confirmation; unchanged by this PR)

  scripts/test-substep-progress.mjs  : 26 pass, 0 fail
    Sections:
      1. notify-island.ps1 schema round-trip        (4 cases)
         - all three new fields round-trip with explicit values
         - step without total: step=5, total stays -1
         - backward compat: step=-1, total=-1, detail=""
         - existing fields (state/message/progress/ts/source) preserved
      2. Build-DisplayMessage function contract   (9 cases)
         covering include step values, total omission, detail omission,
         empty message, total=0 edge, step=-1 with orphan detail
      3. Format-ToolSummary for mcode-computer-use (7 cases)
         including Bash / Read / Edit regression coverage
      4. Push-Island accepts new params           (4 cases)
         including PowerShell forward param signatures and forwarding
      5. Push-Island end-to-end (hook -> status.json) (2 cases)

  Per the round-4 lesson (test pass != contract honored), I broke the
  coordinate formatter in _lib.ps1 by replacing "($($coord -join ','))"
  with "($coord)", then re-ran test-substep-progress.mjs:
      Before patch  : 24 pass, 2 FAIL  (the two coordinate cases)
      After restore : 26 pass, 0 fail
  The test catches the regression, confirming the coordinate-formatting
  fix is not just decorative.

  Widget visual verified locally:
      notify-island.ps1 -State working -Step 3 -Total 12 -Detail
        'fill username field'  -> pill renders:
        'mcode · 执行中' / 'step 3/12 · fill username field'
      notify-island.ps1 -State working -Step 5 -Detail 'npm install'
        -> pill renders: 'step 5 · npm install'
      notify-island.ps1 -State working -Message 'Read ok'
        -> pill renders: 'Read ok'  (legacy path, no step prefix)

  No data migration. Existing status.json consumers see three new fields
  they can ignore. Existing notify-island.ps1 callers see no behavior
  change. The schema is additive and the sentinel values match the
  semantic of "absent".

  Companion docs updated:
    skills/mcode-island/SKILL.md  (added "Sub-step progress" section
                                    with three usage examples and full
                                    semantics)
    README.md                     (added brief mention in the
                                    notify-island.ps1 section with
                                    a forward pointer to SKILL.md)
  Bump plugin.json 0.3.0 -> 0.4.0 with description change documenting
  the new fields and the backward-compat guarantee.
  No upstream protocol changes. (Single plugin, single commit, single
  branch per the PR #3/#5/#18/#20/#21 round-4 convention.)

* feat(mcode-island): pill click toggles show / hide CLI window (round-14)

Previously the pill's MouseLeftButtonUp always invoked Focus-CallerWindow,
which only ever restored + foregrounded the target window. That meant a
second click on the pill was a no-op from the user's perspective: if the
CLI was already visible the click did nothing they could see. The mental
model "单击调出, 单击收起" was not honored.

This commit extracts the caller-target resolution into a shared helper
(Resolve-CallerWindow) and adds Toggle-CallerWindow, which gates on
IsWindowVisible + IsIconic:
    shown + no     -> SW_MINIMIZE   (click hides; taskbar entry kept)
    hidden/min     -> SW_RESTORE +  (click shows; SetForegroundWindow
                          SetForegroundWindow   steals focus)

The click handler now calls Toggle-CallerWindow instead of
Focus-CallerWindow. Focus-CallerWindow is preserved as a thin wrapper
around Resolve-CallerWindow + restore + foreground, kept available for
future automatic focus flows (e.g. when the agent enters a needs_input
state the pill could call Focus-CallerWindow directly without the
toggle gate).

## Design

  Toggle, not flip-and-stick: every click cycles show -> hide -> show.
  The user's "单击调出, 单击收起" requirement is the mental model.

  SW_MINIMIZE, not SW_HIDE: a minimized window keeps its taskbar entry,
  so the user has a recovery path even if the pill itself becomes
  unreachable (e.g. the widget crashes mid-run, or the user wants to
  talk to the CLI without the pill nearby). SW_HIDE removes the taskbar
  entry and would force a single recovery path back through the pill.

  Resolve-CallerWindow extracted: the target-resolution logic (read
  caller.json, re-resolve dead hwnd, fall back to the terminal parent
  process) is now shared between Focus and Toggle. Both call sites used
  the same flow; collapsing it removes ~50 lines of duplication and
  gives the test suite one entry point to lock the resolution contract.

  Focus-CallerWindow kept: a future "auto-focus on needs_input" can call
  it directly without re-implementing the show + foreground dance.
  Today only Toggle is wired to the click handler.

## Backward compatibility

  No external contract changes. caller.json schema is untouched. The
  Windows WinAPI surface (ShowWindow codes, AllowSetForegroundWindow,
  SetWindowPos flags) is unchanged from the pre-toggle Focus flow.

  Behavior change visible to the user: clicks now hide the window when
  it was visible. This is the requested feature.

## Design compliance (per PR #21 round-11 standards)

  no credentials      : none added; the IPC is local-filesystem only
  no network          : no network calls added
  no telemetry        : no telemetry added
  no third-party svcs : no new third-party deps; pure PowerShell +
                        Win32 user32.dll calls (already declared)
  cross-platform      : Win32 calls + user32.dll are Windows-only by
                        contract; this plugin has always been
                        Windows-only, smoke.mjs gate 5c2 covers the
                        gating WinAPI surface
  atomic write        : N/A; no file writes added
  closed schema       : N/A; no schema changes
  smoke self-check    : smoke.mjs section 5c2 (4 checks) locks:
                        - Resolve-CallerWindow function present
                        - Toggle-CallerWindow function present
                        - Toggle gates on IsWindowVisible + IsIconic
                        - MouseLeftButtonUp invokes Toggle (not Focus)

## Validation

  smoke.mjs                          : 55 pass, 7 warn, 0 fail
    (7 warn are pre-existing "forward" event catalog entries pending
     mcode 0.2.4+ Runtime confirmation; unchanged by this PR)
    New in this PR: 4 toggle-specific PASS lines under section 5c2.

## Test evidence (negative-injection verified)

  Per the round-4 lesson (test pass != contract honored), I broke the
  click handler by replacing Toggle-CallerWindow with Focus-CallerWindow
  and re-ran smoke.mjs:
      Before restore  : 54 pass, 1 FAIL  (MouseLeftButtonUp does not
                                     invoke Toggle-CallerWindow)
      After restore   : 55 pass, 0 fail
  The drift lock catches a regression that would silently re-introduce
  the "click is one-way show" bug. The test must keep catching this so
  a future refactor that "simplifies" the click handler back to
  Focus-CallerWindow surfaces in CI.

## Reference

  No docs change required (the toggle is implicit in "click the pill").
  Skill SKILL.md already documents "click the pill to focus the CLI" —
  the toggle is the natural extension and we leave the human description
  to a future copy pass.

  Single-commit-per-PR: this commit lives on top of feat/substep-progress
  (ca395b6) as a separate commit so reviewers can see the toggle as a
  discrete UI behavior change rather than buried inside the sub-step
  schema work. Upstream can squash or keep separate.

* fix(mcode-island): toggle show uses SW_MAXIMIZE to fix 480x84 strip bug (round-15)

Toggle-CallerWindow's restore branch called SW_SHOW + IsIconic + SW_RESTORE,
which preserves the window's pre-hide size. When the WT window was
accidentally resized to a thin strip (e.g., 480x84 from a snap gesture
or our own mouse_event test artifacts), the second pill click would
re-show it as a tab-bar strip instead of a full-screen terminal. The
user's report: 'hide works, show is a thin strip'.

Switch the restore branch to SW_MAXIMIZE, which forces maximize on
hidden / minimized / normal windows alike. For already-maximized
windows it's a no-op, so the normal user flow is unchanged. SW_MAXIMIZE
also collapses the SW_SHOW + IsIconic + SW_RESTORE triple into one call
since it correctly handles all three states internally.

Design compliance:
- Follows round-14 toggle architecture: hide = SW_HIDE (no taskbar
  entry, no Always-show-tabs artifact), show = SW_MAXIMIZE (full-screen,
  no thin-strip artifact)
- Cross-platform: only Win32 user32 ShowWindow constants, no path
  changes from round-14
- No telemetry / no network / no third-party services added

Validation:
- Local smoke.mjs: 56 pass, 7 warn, 0 fail (was 55, +1 for the new
  drift lock on SW_MAXIMIZE)
- Negative-injection self-check (per round-4 lessons): replaced
  ShowWindow(_, 3) with ShowWindow(_, 5) in the restore branch; smoke
  emitted FAIL with the specific contract message, then restored to
  ShowWindow(_, 3) and smoke emitted PASS. Confirms the new check is a
  real contract lock, not a false green.

Test evidence:
- Reproduced: WT rect went to (0,0,480,84) [480x84] (visible=True,
  iconic=False) after a mouse_event snap artifact
- After this commit, pill click sequence (hide -> show) brings WT
  back to full-screen via SW_MAXIMIZE regardless of any prior resize

* fix(mcode-island): toggle show uses MonitorFromWindow + SetWindowPos to fill actual 2560x1440 monitor (round-16)

Toggle-CallerWindow's restore branch called SW_MAXIMIZE alone, which fills
the window to the WinForms-reported logical work area (1920x1080). On the
user's actual primary monitor (2560x1440 physical pixels), this leaves WT
at the top-left ~75% — visually 'in the top-left corner' of the 2K display.
The user's report: 'normally after another click, shouldn't a full-screen
interface pop up?'

Two discoveries during diagnosis:
1. [Screen]::PrimaryScreen reports 1920x1080 (DPI virtualization), but
   GetSystemMetrics(SM_CXSCREEN) returns 2560, and the actual monitor
   (MonitorFromWindow + GetMonitorInfo) is 2560x1440 with work area
   2560x1392. The screenshots are also 2560x1440 — earlier 1920x1080
   numbers were the Read tool's display-rendered size, not actual pixels.
2. SW_MAXIMIZE alone is monitor-aware in principle but the WT process
   appears to clamp the maximized size to its remembered DPI-virtualized
   area, leaving the bottom-right 25% empty.

Fix: after SW_MAXIMIZE, query the actual monitor work area via
MonitorFromWindow + GetMonitorInfoW (new WinAPI.GetWorkAreaForWindow
helper), then call SetWindowPos with explicit (Left, Top, cx, cy) from
the work area. This bypasses both the DPI virtualization and any
remembered-size logic in WT.

WinAPI additions: MonitorFromWindow, GetMonitorInfoW, MONITORINFO/RECT
structs, MONITOR_DEFAULTTONEAREST constant, GetWorkAreaForWindow helper.
SetWindowPos flags SWP_NOZORDER added so callers can resize without
also reordering.

Z-order: SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0, SWP_NOACTIVATE|SWP_NOZORDER)
after the resize pushes WT to the front without requiring foreground
permission (replaces the HWND_TOPMOST then HWND_NOTOPMOST dance that
also doesn't require foreground but adds flicker).

Design compliance:
- Cross-platform: pure Win32 user32 calls, no path changes
- No telemetry / no network / no third-party services
- Round-15 SW_MAXIMIZE contract preserved (still in the restore branch)

Validation:
- Local smoke.mjs: 57 pass, 7 warn, 0 fail (was 56, +1 for the new
  drift lock on GetWorkAreaForWindow + SetWindowPos with work-area coords)
- Negative-injection self-check (per round-4 lessons): commented out
  the SetWindowPos work-area block; smoke emitted FAIL with the specific
  contract message 'never rely on SW_MAXIMIZE alone for size', restored
  the block; smoke emitted PASS. Confirms the new check is a real
  contract lock, not a false green.

Test evidence:
- Before: WT rect (-8,-8,1928,1040) [1936x1048] on 2560x1440 monitor
  → fills ~75% of physical display, visually 'top-left corner'
- After: WT rect (-8,-8,2552,1392) [2560x1400] on 2560x1440 monitor
  → fills full work area (verified via GetWindowRect after click)

* fix(mcode-island): z-order SetWindowPos carries SWP_NOSIZE (round-17 fix for 480x76 regression)

Toggle-CallerWindow's follow-up z-order SetWindowPos call (HWND_TOP with
cx=0, cy=0) was missing the SWP_NOSIZE flag (0x0001). Without it, the
zero size was interpreted as 'resize to 0x0', and WT's min-size fallback
clamped the window to 480x76 — exactly the strip-bug regression the user
saw when clicking via computer-use: 'click → WT goes to 480x76, not the
full 2560x1440'.

Discovered empirically during the round-16 verification:
- Direct Win32 (ShowWindow SW_MAXIMIZE + SetWindowPos work area) ended at
  2576x1408 ✓ (full 2560x1440 + 8px borders)
- Same flow via the widget ended at 480x76 ✗ (WT min-size)
- Diff: widget has an extra SetWindowPos(HWND_TOP, 0, 0, 0, 0,
  SWP_NOACTIVATE|SWP_NOZORDER) without SWP_NOSIZE

Fix: add SWP_NOSIZE constant to WinAPI class, OR it into the z-order
SetWindowPos flags. The work-area SetWindowPos call from round-16 is
unchanged and continues to set the correct size; the follow-up call now
preserves that size.

Validation:
- Local smoke.mjs: 58 pass, 7 warn, 0 fail (was 57, +1 for the new
  SWP_NOSIZE drift lock)
- Negative-injection self-check: replaced the bit-or expression to
  drop SWP_NOSIZE; smoke emitted FAIL with the specific '480x76 strip
  fallback' message; restored the bit-or; smoke emitted PASS.

Test evidence:
- Before this commit (round-16 only): computer-use click on pill left
  WT at rect (0,0)-(480,76) [480x76], visible=True, zoomed=True
- After this commit (round-17): computer-use click on pill should leave
  WT at rect (-8,-8)-(2568,1400) [2576x1408], visible=True, zoomed=True
  → fills the entire 2560x1440 monitor as the user expects

* feat(mcode-island): tool-verb messages + family tinting (round-18)

The pill showed the bare tool name for every phase, so `working` and
`done` were indistinguishable at a glance: both read "bash". The mcode CLI
TUI solved this with a descriptor table (launcher-GHPADSKI.js HL[]) that
gives every tool a present-tense verb while it is in flight and a
past-tense one once it returns. This commit ports that idea to the pill.

Detector
  - New $TOOL_ACTIONS: 18 tools x {running, done, fail}. Messages become
    "Running ..." / "Ran" / "Command failed" instead of "bash : {...}" /
    "bash 完成" / "bash 失败".
  - New $TOOL_FAMILIES: maps a tool to one of 7 families
    (shell/read/write/search/task/web/plan).
  - New Get-ToolKey / Get-ToolVerb / Get-ToolVerbFallback / Get-ToolFamily.
    Unknown tools fall back to "Using <name>" / "Used <name>" so the pill
    never shows a bare identifier with no phase signal.
  - Get-ToolKey caps the normalized name at 64 chars. The name comes from
    the session log, i.e. it is attacker-influenced in a hostile repo, and
    a 4 KB "tool name" must not become a hashtable key.
  - Infer-State now returns a `family` field on every tool branch, and
    Write-Status emits it to status.json.
  - $MSG_FAIL / $MSG_OK are removed. The three tool branches were their only
    consumers, and leaving dead byte-array constants behind would invite a
    future reader to think they are still wired up.

Widget
  - New $familyMap: 7 families -> dot colors.
  - Update-State takes $Family and resolves a $dotHex / $ringHex. The family
    color overrides the state color for thinking/working/waiting only, so
    done stays green and error stays red -- the result signal the user
    relies on must not be repainted by whatever tool happened to run.
  - The status.json poll signature now includes `family`. Without it, two
    consecutive working states from different families would be deduped
    and the dot would keep the previous color.
  - The progress bar follows the same resolved color, so the dot, the
    pulse ring, and the bar always agree.

Validation
  - smoke.mjs: 36 new checks in sections 5d/5e, covering the two tables,
    all four helpers, the three required families, the three state-branch
    call sites, the status.json field, the widget $Family parameter, the
    active-state gate, and the dot/ring paint path.
  - 98 pass, 7 warn, 0 fail locally.

Test evidence
  - 16 negative-injection cases, each a minimal change that breaks one
    clause of the contract. All 16 turn the suite red; the baseline is
    restored to 98/0 after each.
  - Three of the 16 initially did NOT go red. They were false greens and
    the checks were rewritten rather than the failures being waved off:
      1. The active-state gate was matched as a bare
         `$State -in @('thinking','working','waiting')`, but that substring
         also appears on the pulse, elapsed, and progress branches. Ungating
         the family tint left those three in place and the check stayed
         green. It now anchors on the `$Family -and $State` conjunction.
      2. The three state-branch locks were presence tests, but each state
         has TWO call sites (the ledger.jsonl branch and the messages.jsonl
         branch). Breaking only the ledger branch left the messages branch
         matching. They now assert a minimum occurrence count of 2.
      3. The first harness used CRLF anchors against LF-only files, so every
         case reported SETUP-MISS and the suite looked green while nothing
         had actually run. Anchors now use LF and the harness fails loudly
         on a missing anchor instead of counting it as a pass.
  - End-to-end on the live widget: all 6 active family colors render
    (shell green, read blue, write yellow, search purple, web pink, plan
    violet) and done/error keep their green/red result colors.
  - Live status.json after deploy carries the new contract end to end:
    {"state":"working","message":"Running {...}","family":"shell",...}

Design compliance
  - No credentials, no network, no telemetry, no third-party services. The
    verb tables are static data; the only I/O is the pre-existing
    status.json write that the widget already polls.
  - Cross-platform: no host paths introduced. The tables are keyed by tool
    name only, and the existing hardcoded-path scan covers the edited
    files.
  - ASCII-only additions. The existing Chinese message constants are
    untouched, so the PS 5.1 GBK parsing defense is unchanged.
  - Session log is a local file the user already owns; reading tool names
    out of it adds no new capability and no new data source.

* fix(mcode-island): separate plan/search hues, lock family color distance

The round-18 family palette shipped with plan at #8B5CF6 against search's
#A855F7. On the dark pill those two read as the same purple, so `grep` and
`todowrite` were not actually distinguishable -- which is the whole reason
the tinting exists. plan moves to a desaturated slate #94A3B8.

smoke.mjs gains a perceptual-separation check over the family table. It
converts each #AARRGGBB entry to CIELAB and asserts every pair is at least
dE 20 apart (CIE76, the usual "not the same color to a human eye" cutoff
for flat UI fills).

Test evidence
  - Negative injection: putting plan back to #8B5CF6 makes the check fail
    with "search (#A855F7) and plan (#8B5CF6) are only dE 10.1 apart".
    Restoring #94A3B8 returns the suite to 100 pass.
  - The check itself went through two false greens before it was correct,
    both caught by writing the guard before trusting the output:
      1. The color regex required 6 hex digits, but the table stores
         #AARRGGBB, so it matched nothing and the check reported
         "all 0 family colors are perceptually distinct" -- a vacuous pass.
         A parse-count guard now fails outright below 7 entries.
      2. The same check initially measured raw sRGB luminance, which flags
         blue vs purple as "identical" (0.02 apart) because they differ in
         hue but not brightness. That produced five false alarms on a
         palette that is fine in practice. Replaced with CIELAB dE.
  - Live widget: all 7 family colors confirmed distinct, and the earlier
    screenshot run that showed six identical rows was a harness timing
    fault (capturing the previous frame), not a rendering fault -- the
    capture now waits for the widget to log the state before shooting.
  - 100 pass, 7 warn, 0 fail.

* feat(mcode-island): ask_user reports waiting, not working

On a `full access` permission setup the permission-request hook never fires,
so `waiting` was unreachable and ask_user was the only thing that could
actually block the agent. It was reporting `working`:

    working :: ask_user : {"mode":"questionnaire","requiresExplicitResponse":true,...

The pill showed a blue dot with a gear icon -- the universal "leave it
alone, it is making progress" signal -- while the agent sat blocked on a
questionnaire, plus a truncated JSON blob where the useful text should be.
Three occurrences in the 9/24 log confirm this was not a one-off.

ask_user now reports `waiting` with a question count, so the pill reads
"Asking 2 questions" under an amber dot and a "?" -- the "come back, I'm
blocked" signal. It wears the state color rather than a family tint: a
blocked agent must not look like an in-flight delegation.

Detector changes
  - $S_WAITING is now defined. It was referenced nowhere before, so any
    branch using it would have returned $null for the state.
  - Test-IsAskUser and Get-AskQuestionCount helpers, wired into both tool
    paths (ledger.jsonl and messages.jsonl).
  - waiting joins both settle sets. The 60s no-activity fallback would
    otherwise downgrade a pending questionnaire to "已静默 60s" exactly
    when the user has been away longest, and the takeover arbitration
    would let the pre-tool-use hook's `working` shadow it forever.
  - ask_user is removed from $TOOL_FAMILIES.

Bug found while testing
  - Get-AskQuestionCount originally took a parameter named `$args`, which
    is a PowerShell automatic variable. Shadowing it makes every read
    inside the function return the function's own argument list instead of
    the caller's value, so the count was 0 for every input and the message
    degraded to a bare "Asking". Caught by exercising the helper against
    seven argument shapes before wiring it up; the parameter is now
    $toolArgs and smoke locks the name.

Validation
  - smoke.mjs: 7 new checks in section 5d2. 107 pass, 7 warn, 0 fail.
  - 7 negative-injection cases, all turn the suite red; baseline restores
    to 107/0 after each. Two of them target the multi-site and settle-set
    contracts specifically, since those are the ones that fail silently.
  - The real Infer-State from the deployed file was exercised against nine
    real messages.jsonl payload shapes: ask_user with 0/1/2/5 questions,
    a missing questions key, absent arguments, and arguments arriving as
    a JSON string. All six ask_user shapes infer `waiting` with no family;
    bash, read, and todowrite are unchanged.
  - Live widget: `Asking 2 questions` renders amber with a `?`, and the
    post-answer transition to `done` / `Asked` still works.

Test evidence
  - Deliberately did NOT append synthetic toolCall records to a live
    session log. The running agent is writing to that file and injecting
    phantom work into it could make the agent act on it. The end-to-end
    check runs the deployed Infer-State over real payload shapes instead,
    which covers the same code path without touching agent state.

Design compliance
  - No new I/O, no network, no credentials, no telemetry. The change is a
    state mapping plus a question count read from arguments the detector
    was already parsing.
  - Get-AskQuestionCount swallows parse failures and returns 0, so a
    malformed arguments payload degrades to "Asking" rather than breaking
    the poll loop.
  - ASCII-only additions; the PS 5.1 GBK parsing defense is unchanged.

* fix(mcode-island): redact typed text in tool summaries (round-19 #1)

Security. Format-ToolSummary put `tool_input.text` -- whatever the user
typed -- straight into the pill detail string. That string reaches:

  - status.json on disk
  - island.log, append-only
  - the always-on-top pill, visible to anything sharing the screen

So a password, one-time code, or token entered through Computer Use ended
up in a screenshot, a screen share, or a screen recording. The pill is the
worst of the three because it is deliberately on top of everything else.

The text branch now reports the action plus a character count and nothing
else. No substring, no length bucketing that would leak the shape of the
value, no echo. The action name survives, so the pill still answers the
question that matters: is the agent typing right now?

The coordinate branch is untouched -- coordinates are not secret and are
genuinely useful to see.

Validation
  - scripts/test-substep-progress.mjs is rewritten as a behavioral suite
    (see the CI commit), and section 1 plants a canary secret
    SUPER-SECRET-PASSWORD-8f3a91c2 in tool_input.text, runs the real
    Format-ToolSummary, and asserts the canary is absent from the output.
    It also asserts the action survives and the length is reported, so a
    redaction implemented by silently dropping the branch would not pass.
  - End to end against the deployed hook: a PreToolUse event carrying
    P@ssw0rd-CANARY-7d3e91 produced
      status.json: "mcode-computer-use : type <22 chars, redacted>"
      island.log:  "working :: mcode-computer-use : type <22 chars, redacted>"
    and the canary appears in neither.
  - Negative injection: restoring the original `$detail = "$act '$txt'"`
    turns the canary check red.

Design compliance
  - No new I/O or network. The change only removes data from a string.

* fix(mcode-island): detector preserves sub-step fields across a rewrite (round-19 #2)

Write-Status rebuilds the whole status.json payload on every write, and the
detector writes on every 5h-usage refresh and every todo-progress refresh.
step / total / detail are agent-pushed sub-step state, not detector state,
and they were simply not in the payload -- so a routine usage refresh
silently zeroed whatever sub-step the agent had reported. On a long task
the pill would drop "step 4/12" back to the bare message and never recover
until the next tool call.

Write-Status now reads the current file and carries step / total / detail
forward. It is a read-modify-write, not an overwrite, and defaults to
-1 / -1 / "" when there is no prior value so a fresh install still renders
normally.

Validation
  - Behavioral: a status.json carrying step=4 total=12 detail="running
    tests" survives a Write-Status rewrite with all three intact; an
    unwritten status.json yields -1 / -1 / "".
  - End to end against the deployed detector: notify-island pushed
    step=4 total=12 detail="running the test suite" (source=agent), then
    15s later the detector wrote its own status and all three fields were
    still there.
  - Negative injection: reverting Write-Status to the blind overwrite
    turns the preservation check red.

* fix(mcode-island): restore path needs SWP_NOMOVE, and drops SWP_NOZORDER (round-19 #3)

The z-order SetWindowPos after a restore used
SWP_NOACTIVATE | SWP_NOZORDER | SWP_NOSIZE with X=Y=cx=cy=0.

Two problems:

  SWP_NOMOVE is missing. X=0/Y=0 is then a real move request, so on any
  monitor whose origin is not 0 (a secondary display to the right, or above)
  the restored window is yanked to the primary monitor's (0,0). Invisible on
  a single primary display, which is why the round-15/16/17 manual testing
  never caught it.

  SWP_NOZORDER is present, which contradicts the call's own purpose. That
  flag makes Windows ignore hWndInsertAfter entirely, so passing HWND_TOP
  alongside it is self-defeating: the z-order change silently does nothing
  and only BringWindowToTop afterwards is doing the work.

Now: SWP_NOACTIVATE | SWP_NOSIZE | SWP_NOMOVE, with HWND_TOP actually
honored.

The round-17 smoke lock asserted the literal `SWP_NOZORDER -bor SWP_NOSIZE`,
i.e. exactly the pair the review asks to change, so it had to be rewritten
rather than updated. It now checks that both required flags are present and
that SWP_NOZORDER is absent, matching on flag names rather than their order.

Validation
  - test-substep-progress.mjs and smoke.mjs both cover this.
  - Negative injection: dropping SWP_NOMOVE turns both red; adding
    SWP_NOZORDER back turns both red.

* fix(mcode-island): post-tool-use passes Step/Total so detail renders (round-19 #4)

post-tool-use.ps1 computed a per-tool detail string and passed it as
-Detail, but left Step at its -1 default. Build-DisplayMessage returns the
bare message when Step <= 0 and only consults Detail inside the Step > 0
branch, so the detail was discarded on every single call. The
"step N/M - detail" contract the widget documents could never fire from
this hook.

Both Push-Island calls now pass -Step 1 -Total 1. PostToolUse is a single
tool result and does not participate in a multi-step sequence, so 1/1 is
the honest value; inventing a larger denominator would risk rendering
"step 1/0" on a fresh install.

Validation
  - Build-DisplayMessage is exercised directly over five argument shapes:
    step+total+detail, step without total, step with empty detail, no step
    (detail must be ignored), and no step and no message.
  - Both Push-Island calls are matched individually and must each carry
    -Step and -Total. A single scan across the file passes as soon as one
    call has them, so removing -Step from just the error branch would
    otherwise stay green.
  - The $step/$local $total assignments are asserted to be positive. They
    are the "unset" sentinel everywhere else in this plugin (-1 means no
    sub-step), and setting them to -1 keeps every flag check green while
    the detail is dropped again at render time. A negative-injection pass
    caught that blind spot.
  - Negative injection: dropping -Step from the error branch, dropping
    -Total from the done branch, setting $step = -1, and inverting
    Build-DisplayMessage's Step<=0 early return each turn the suite red.

* ci(mcode-island): run test-substep-progress on windows-latest (round-19 #5)

scripts/test-substep-progress.mjs existed but no workflow invoked it, so
none of its assertions gated anything. It is also the only suite that runs
the PowerShell under test rather than grepping the source for substrings,
and it is where the secret canary for review item #1 lives -- so leaving
it unwired meant a redaction regression could land without anything
noticing.

A new step 5 runs it after the existing four.

PowerShell resolution: the suite prefers $env:PS_BIN, then an existing path
if PS_BIN looks like one, then probes pwsh / powershell.exe / powershell on
PATH. It never assumes a user-specific absolute path, and it fails loudly
when no PowerShell is found rather than silently skipping. The workflow
step logs which shell was resolved before invoking it.

Two encoding issues had to be solved for the suite to produce trustworthy
results on a runner whose console code page is not UTF-8:

  - the generated .ps1 needs a BOM, or PowerShell 5.1 reads it as ANSI and
    mangles any non-ASCII literal in the harness;
  - the child must set [Console]::OutputEncoding, or a middle dot comes
    back as two GBK bytes and two correct assertions fail as bogus diffs.

Without both, the suite fails on correct code, which is worse than not
running it: it would train everyone to ignore it.

* ci(mcode-island): resolve the suite path from the repo root (round-19 #5 fix)

The step I added in 5fbda03 ran `node scripts/test-substep-progress.mjs`.
GitHub Actions executes every `run:` block from $GITHUB_WORKSPACE, not from
the plugin directory, so node looked for
  <repo>/scripts/test-substep-progress.mjs
which does not exist, and the run died with MODULE_NOT_FOUND after 32s.

It passed locally only because my shell happened to already be sitting in
plugins/antianqi/mcode-island. A test that only runs from one directory is
not a test.

The path is now resolved against the repo root and checked with Test-Path
before node runs, so a future rename fails with a clear message naming the
cwd instead of a node stack trace.

Also fixes a bug in the same step: the version banner ran `& $c` after the
loop, where $c is the last candidate rather than the shell that was
actually selected. On a runner without pwsh the banner would have invoked
the wrong binary, or nothing at all.

* fix(mcode-island): round-20 - close a false disclosure claim and two unbounded-growth leaks

Audited the plugin against the 0.5.0-0.5.8 changelog and the live runtime
state. Three real defects, each with a regression test that was verified
to fail before the fix landed.

1. The 5h-usage disclosure named a host the code never called.

   Get-5hUsage.ps1 obfuscates the endpoint into a byte array (PS 5.1
   parser-quirk defense). The bytes decode to api.minimaxi.com, which is
   correct and is what the running detector actually calls. But the
   trailing comment on that same line said api.minimax.com, and the
   README named api.minimax.io in three places -- including an absolute
   claim that the token is "never sent to a host other than
   api.minimax.io". That claim was false: the code never contacted that
   host at all.

   For a plugin whose pitch is four independent disclosure sections, an
   overstated isolation guarantee is worse than no guarantee: a reviewer
   checking host consistency would find the code and the docs disagreeing
   and would be right to distrust both.

   The smoke check now DECODES the byte array and compares the docs
   against that decoded truth. It never hard-codes the endpoint, so it
   cannot become a second copy of the disclosure it polices, and it
   fails the moment code and docs drift apart again.

2. island.log and widget.log had no size cap, ever.

   mcode-island.ps1 has documented "keep only the last 1KB" since the
   widget was written. Dbg was a bare Add-Content and never implemented
   it. Measured on this machine: widget.log reached 37.2 MB and
   island.log 18.0 MB over 38 days, with widget.log growing ~3.6 MB/day
   during active use. Extrapolated, that is over a GB a year in
   %APPDATA%, and nothing ever trimmed it.

   Both writers now cap at 1 MB and keep the last 300 lines. The size
   check is counter-gated (fires once per 256 KB written) so the 400 ms
   poll path does not stat the file on every call.

3. Tool input carrying credentials was persisted verbatim.

   Round-19 #1 redacted Computer Use keystrokes because they can be
   passwords. The Bash branch was never redacted, so `curl -H
   "Authorization: Bearer ..."` or `export API_KEY=sk-...` reached all
   three sinks -- status.json, the append-only log, and the always-on-top
   pill -- in the clear, and the log kept it indefinitely.

   There are two independent producers of that text: the hook path
   (_lib.ps1) and the detector, which re-derives messages from mcode's
   session log rather than from a hook event. A helper wired into only
   one is a silent half-fix -- the pill looks clean while the detector
   keeps writing raw text to disk. Both now route through a shared
   scripts/lib/Protect-Text.ps1, and smoke check 9 asserts BOTH
   consumers invoke it rather than merely defining it.

   Scope is deliberately credential-shaped runs, not whole commands: the
   command text on the pill is a documented feature, and blanking every
   command would trade a real capability for a marginal privacy gain.

Also adds a regression guard for hook invocations under an install path
containing spaces. mcode 0.5.4 fixed its own managed updater for exactly
that case; ${PLUGIN_ROOT} is substituted at runtime, so the only way to
break a spaced path is to hand the shell a joined string instead of an
argv array. hooks.json already uses the array form, so this is a guard,
not a fix.

Validation
----------
- smoke: 115 pass / 0 fail (7 pre-existing "forward" event warnings).
  Baseline before this commit was 108 pass / 0 fail; the 9 new failures
  were the new checks, observed failing first.
- Negative-first: each check was confirmed to FAIL before its fix
  landed, and check 9 was re-verified by deleting the detector's call
  site and watching it fail, then restoring.
- Protect-SecretText: 16/16 cases, covering 10 credential shapes
  (bearer header, sk-, ghp-, bare JWT, JSON quoted key incl.
  "x-auth-token", single-quoted JSON, --password flag, key=value, the
  plugin's own set-token.ps1 invocation) and 6 near-miss inputs that must
  NOT be redacted (--mode production, "Select-String token", Windows
  paths, --secret-config, --author=, {"author":"Bob"}).
- Log cap: wrote 3.4 MB of input through the new Log-Line body; final
  file 0.64 MB / 7064 lines. Cap held across repeated truncation.
- PowerShell parser check clean on all five modified/new .ps1 files.
- The 5h call was verified live against the real endpoint, not assumed:
  Get-5hUsage returns a real remainingPct with the configured token.

Not included
------------
The context-meter and token-speed readouts proposed in the same audit are
deliberately NOT in this commit. Structurally enumerating the session
store (excluding the session this audit was run in) shows the assistant
message carries model / usage.{input,output,cacheRead,cacheWrite,
totalTokens,cost} and nothing else: there is no contextWindow and no tps
field. usage.totalTokens is a usable context numerator, but the window
size lives in mcode's provider config, and a token-rate derived from
message timestamps would fold in thinking and tool time -- which 0.5.6
explicitly excludes from the TUI's own figure. Shipping either without
that denominator would put a number on the pill that disagrees with the
TUI. Left for a follow-up that can source the window size properly.

* fix(mcode-island): every non-ASCII .ps1 needs a UTF-8 BOM for PS 5.1 (round-20 #4)

Found while landing round-20: syncing the committed tree into the install
directory with the documented `Copy-Item -Recurse -Force` flow produced a
detector that could not start at all.

Root cause, verified against Windows PowerShell 5.1 rather than inferred:

  Windows PowerShell 5.1 decodes a BOM-less script using the system ANSI
  codepage. On a zh-CN host that is GBK, so a script containing non-ASCII
  comments is mis-decoded. The C# here-string in mcode-status-detect.ps1
  then stops being a here-string, `using System;` is parsed as PowerShell,
  and the script dies before its first statement with 92 lines of
  "missing using directive". The reported line number (38) is offset from
  the real one (46) because the mis-decode also desynchronizes the
  tokenizer's line count.

  mcode-island.ps1 never showed this because it happens to carry a BOM.
  Nothing else in the plugin did.

The trap is that .gitattributes pins `*.ps1 text eol=lf`, and its comment
claimed "PowerShell 5.1 reads CRLF fine, so LF avoids both failure modes".
That is backwards, and the comment is what let the bug survive. The policy
is part of the cause, not a mitigation.

Control experiment, to be clear that this is pre-existing and not a
round-20 regression: the detector as of ea96d75 (before any of this
round's work) fails under 5.1 with byte-identical output. It simply had
never been synced from a clean checkout on this machine before.

Why a BOM rather than flipping .gitattributes to CRLF:
  Both were tested against 5.1 and both fix the parse -- bare LF does not.
  CRLF would re-break the Linux-side validate.mjs CRLF handling that the
  same comment was written to avoid, so the BOM is the narrower fix: three
  bytes at the head of the file, line-ending policy untouched.

Validation
----------
- Control: ea96d75's detector, run under 5.1, fails identically. Not a
  regression introduced here.
- Isolated: the same file, byte-identical except for a prepended BOM,
  parses cleanly under 5.1 and runs to completion (the only remaining
  error was a TEMP-path artifact of the harness, since $PSScriptRoot had
  no scripts/lib beside it).
- Also confirmed: CRLF without a BOM also parses, so bare LF is the
  trigger, not the encoding per se.
- smoke check 11: written first, observed FAIL on exactly the 8 affected
  files, then PASS after the BOMs landed. 116 pass / 0 fail.
- All 20 non-ASCII .ps1 tokenized with the 5.1 PSParser: 0 parse errors
  (was 8 files failing before).

Files given a BOM (8): mcode-status-detect.ps1, set-token.ps1,
_lib.ps1, post-tool-use.ps1, pre-compact.ps1, session-end.ps1, stop.ps1,
test-windows-workflow-local.ps1.

.g
gitattributes now documents the real constraint, states that bare LF is
part of the problem, and warns that the BOMs must not be "cleaned up" --
an editor that drops them breaks every sync.

Note for reviewers: this changes the first byte range of eight files, so
the diff reads as one changed line each. That is the BOM and nothing else.

* fix(mcode-island): a detector state write clears a stale sub-step (round-20 #5)

Live symptom: the pill sat on "step 1/1" showing a curl command from a
tool call that finished several turns earlier, and nothing ever cleared
it.

Cause: round-19 #2 made Write-Status merge the previous step/total/detail
back in on every write, which is correct for the two metadata refreshes
(60s 5h-usage, todo progress) because those restate the current state
rather than announcing a new step. But the detector also writes on state
inference -- it read a new tool call out of the session log and is
asserting "the agent is doing THIS now". That message has no relationship
to whatever sub-step a hook pushed last, so it inherited a counter that
had no meaning there. Every subsequent write inherited it again, which is
why it never cleared.

Write-Status now takes -KeepSubStep:

  - state inference (default) resets step/total/detail to -1/-1/""
  - the 5h-usage and todo refreshes pass -KeepSubStep and preserve

The two paths are distinguished explicitly by the caller, not inferred
from the message text. Distinguishing is necessary because the
state-inference call also has to be able to win the settle race against
an agent push (round-19), so "who wrote last" is not a usable signal.

Tests
-----
New section 5 in test-substep-progress.mjs, negative-first: observed
failing on 3 assertions before the fix, all passing after.

  - Write-Status exposes -KeepSubStep
  - state-inference write clears a stale sub-step (-1|-1|)
  - metadata-refresh write still preserves it
  - the inference call site does NOT pass -KeepSubStep
  - exactly 2 call sites DO (5h-usage, todo)

The call-site assertions are the ones that matter: a switch nobody passes
is the dead-code shape, and a test that only exercises the function body
would not notice.

Two harness repairs, both regressions from round-20 that this work
surfaced:

  - Format-ToolSummary now calls Protect-SecretText, which lives in a
    separate lib. The harness extracted the function body and ran it
    standalone, so it died on an unresolvable command rather than on an
    assertion. The harness now dot-sources the lib, as _lib.ps1 does.
    This is a genuine break of an existing test that round-20 shipped;
    smoke did not catch it because smoke is static and this suite is not.
  - Section 3 anchored its extraction regex on `$prev` appearing before
    `$detailField`. Wrapping the block in `if ($KeepSubStep)` inverted
    that order and the regex silently stopped matching, which would have
    been reported as "Write-Status preserves sub-step fields: could not
    find the block" -- a confusing failure for a function that works.
    It now anchors on the initialisers and the payload construction.

Validation: sub-step suite 32 pass / 0 fail, smoke 116 pass / 0 fail.
Detector re-verified under Windows PowerShell 5.1 (exit 0) after the
restart, since that is the interpreter this runs under in production.

* fix(mcode-island): the pill never renders raw tool JSON (round-20 #6)

The running-state message was built as

    "$verb " + (ConvertTo-Json $args -Compress)  truncated to 60 chars

so the always-on-top pill showed things like

    Running {"command":"$ErrorActionPreference=\u0027Continue\u0027\n...
    Reading task {"task_id":"bg_0f858500-...","wai...

Escaped quotes, a JSON key, and a truncation that lands mid-token. It was
the noisiest thing on screen and the least informative.

The `done` and `fail` branches were always fine -- they render the verb
alone. Only the `running` branch serialised arguments.

Format-ToolArgs now returns the single most informative field per tool:
command for shell, file_path for read/write/edit, pattern for
grep/glob, query for web_search, url for web_fetch, name for skill,
task_id for the task-output family. Unknown tools get a generic sweep of
the common field names before giving up.

`skill` is not in $TOOL_ACTIONS or $TOOL_FAMILIES, so it always took the
JSON path -- one of the most frequent tools on screen was the least
readable. That is the case that made this worth fixing rather than
tuning.

There is no fallback to JSON. If Format-ToolArgs returns nothing the pill
wears the bare verb: "Using" reads fine, "Using {\"name\":...}" does not.
An unknown tool should degrade to less information, never to a dump.

Also fixes the display of multi-line commands. A PowerShell command is
frequently several lines; the old code collapsed and truncated the whole
thing, so the visible text was usually the middle of a script fragment.
It now takes the first non-empty line.

Redaction is unchanged and still applied on this path -- the summary runs
through Protect-SecretText exactly as the JSON did, so a credential in a
command still cannot reach the pill or island.log by this route.

Tests: 16 new assertions in test-substep-progress.mjs, negative-first.
The blanket rule is asserted per case: no result may contain a JSON key,
an escaped quote, or a \uXXXX sequence. Cases cover bash, multi-line
bash, read/write/edit, grep, glob, web_search, web_fetch, skill,
task_output, an unknown tool with an unrecognised argument shape, and a
bearer token inside a command.

Validation: sub-step suite 47 pass / 0 fail, smoke 116 pass / 0 fail, all
30 .ps1 parse, and the detector still initialises cleanly under Windows
PowerShell 5.1 (exit 0) -- which is the interpreter it runs under in
production, and the one that breaks on a malformed file.

One trap worth recording, because it produced a very misleading failure:
the second parameter is deliberately NOT named `$args`. That is a
reserved automatic variable in PowerShell, and binding a parameter to it
yields the unbound-argument Object[] rather than the object passed in.
The symptom was an empty summary for every single tool, which reads as
"no data" rather than "bad binding". Verified directly:

    function T($n, $args) { $args.command }
    T bash ([PSCustomObject]@{ command = 'npm test' })   # -> ''

* fix(mcode-island): a metadata refresh no longer steals state ownership (round-20 #7)

Reported as: the pill stops on "Command failed" at the end of a turn.

`source` is not a label -- it is the arbitration token. The detector's
settle logic reads `source == detector` as "this state is mine to
rewrite", and uses that to decide whether it may overwrite an agent
push. But Write-Status stamped `source = detector` unconditionally,
including on the two metadata refreshes (60s 5h-usage, todo progress)
that exist only to refresh numbers on a state somebody else owns.

The sequence, from island.log on the dev machine:

  16:06:24  [detect] error :: Command failed
  16:06:25  error      :: Command failed        <- PostToolUse hook, source=agent
  16:06:40  [detect] 5h usage refreshed        <- stamps detector over the agent's state
  16:06:41  error      :: Command failed
  16:07:03  [detect] idle :: waiting             <- 60s no-event fallback, finally

At 16:06:40 the refresh handed write authority back to the detector
while leaving `state = error`. From that point the detector owned the
state and re-derived `error` from the same stale failed toolResult on
every poll -- there is nothing newer in the session log to supersede it.
When the turn ended and the Stop hook pushed `done`, the detector was
entitled to overwrite it, and the pill went back to red. Nothing cleared
it until the 60-second no-event fallback happened to fire.

So the error was accurate when it appeared and wrong the moment the turn
moved on. The user-visible symptom is a pill that is stuck on a failure
that is no longer the current state.

A metadata refresh now preserves the previous owner. The claim-ownership
stamp is reserved for state-inference writes, which is the only path
that actually asserts a new state. This is the same distinction #5 drew
for step/total/detail; `source` belongs to the observable state, so it
follows the same rule.

Tests: 2 new assertions, negative-first, both observed failing before the
fix. They assert the ownership token in both directions -- a
metadata-refresh write keeps `agent`, a state-inference write claims
`detector` -- because a test that only checks one side would pass just as
happily if the field were always blank.

Validation: sub-step suite 49 pass / 0 fail (was 47), smoke 116 pass /
0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows
PowerShell 5.1 (exit 0).

* fix(mcode-island): skip the assignment preamble in shell summaries (round-20 #8)

Round-20 #6 stopped the pill from rendering raw JSON, but the result was
still frequently uninformative:

    Running $ErrorActionPreference='Continue'

That is the opening line of most agent commands, and it says nothing about
what is being worked on. On a detached status pill it is close to
indistinguishable from the pill being stuck -- which is not a good look
for a widget whose entire job is to answer "what is it doing right now".

Format-ToolArgs now prefers the first line that actually EXECUTES
something, skipping variable-assignment and comment lines:

    $ErrorActionPreference='Continue'   <- skipped
    $env:TEMP="..."                     <- skipped
    npm test                            <- shown

The fallback matters: if every line is boilerplate, the first line is
still shown rather than leaving the pill blank. A summary that is
uninformative beats no summary at all, because the bare verb alone cannot
be distinguished from the detector having lost track.

Comment lines are skipped on the same rationale.

Tests: 3 cases, negative-first, all observed failing before the fix --
the preamble was shown, and the comment line was shown. A fourth case
(an all-assignment command) was written at the same time and passed
before the fix as well, which is the point: it pins the fallback so a
future "skip more lines" change cannot silently blank the pill.

Validation: sub-step suite 51 pass / 0 fail (was 49), smoke 116 pass /
0 fail, all 30 .ps1 parse, detector initialises cleanly under Windows
PowerShell 5.1 (exit 0).

* fix(mcode-island): the detector survives a terminating error in the main loop

The main loop had no catch. Any terminating error escaped the loop,
fired the finally block, logged 'detector stopped' and exited the
process. The widget kept running, so the pill just froze on the last
status.json with no visible cause -- status.json silently stopped
changing while widget.log kept logging TOPMOST re-raises.

This is the same failure family the round-21 review flagged for the
status.json write path (a shared staging file let one process rename
another's half-written file into place, and the resulting Move-Item
failure killed the daemon). The lock in the next commit removes that
specific trigger; this commit makes the daemon survive the ones we
cannot enumerate.

The catch logs the exception type as well as the message, so 'loop
error' alone does not have to be disambiguated between a JSON parse, a
WMI failure and an IO lock. The message is truncated to 300 characters
so a runaway error cannot flood island.log every tick.

Validation
  - the guard is a try/catch around the existing loop body; the loop's
    own logic is untouched
  - the detector still initialises cleanly under Windows PowerShell 5.1
    (exit 0), the interpreter it runs under in production
  - all .ps1 files in the plugin still parse without syntax errors

Design compliance
  no credentials / no network / no telemetry / no third-party services
  the change only adds an error handler around existing local work

* fix(mcode-island): status.json writes are atomic across processes

Round-21 review (hetaoBackend, 2026-10-09) blocking issue #1: the
detector's metadata refresh was not an atomic read-modify-write.

  The loop read old state/message (mcode-status-detect.ps1:923), then
  Write-Status -KeepSubStep performed a SECOND independent read for
  step/detail/source (:714-721) before overwriting the file (:723-739).
  An agent write between the two reads produced a mixed-generation
  payload, and a write after the second read was lost.

There was a second writer behind that, and it made the outcome worse
than a stale payload. notify-island.ps1 staged status.json through
"$statusFile.tmp" and the detector staged through the same path. Two
processes, one temp file: one could rename the other's half-written
file into place, and the loser then failed its Move-Item. The detector
runs under $ErrorActionPreference = 'Stop' with no catch around the
write, so that failure killed the daemon outright -- the pill froze on
the last status.json with no visible cause. (The preceding commit adds
the per-tick guard that stops the daemon dying on ANY terminating
error; this commit removes this particular trigger.)

## What changed

  scripts/lib/Status-Store.ps1 (new)
    The one place the status.json protocol lives, shared by both
    writers:
      Enter-StatusLock / Exit-StatusLock
        An exclusive cross-process lock on a sibling lock file, opened
        with FileShare.None. That is a real OS file lock, not a flag
        file whose presence we poll -- a flag file has the same
        check-then-act hole the payload has. If a holder dies, the OS
        releases the handle.
      Read-StatusSnapshot
        One read of the whole status object. Callers read once and
        reuse that snapshot.
      Write-StatusSnapshot
        Atomic replace (staging file + rename) that stamps the
        monotonic `version` counter and `ts`.

  mcode-status-detect.ps1
    Write-Status now takes the lock, reads the snapshot ONCE, and
    writes. The body moved to Write-StatusUnderLock so the arbitration
    path can read the snapshot and act on it without taking the lock
    twice (the lock is not re-entrant: a nested take would deadlock
    against itself).

    The arbitration read and the write it authorises are now one
    critical section. Previously the read sat outside the lock, so a
    hook push could be judged against a stale snapshot and then
    overwritten by the write that stale snapshot had authorised.

    -KeepSubStep became -MetadataRefresh. The old switch only said
    "preserve the sub-step"; the call site still had to read the file
    itself to restate the state, which is where the second read came
    from. One switch now names the whole semantics: a metadata refresh
    restates the current state, preserves the sub-step fields and
    preserves the state owner. A state-inference write resets them and
    claims ownership. The two metadata call sites no longer touch the
    file at all.

    Read-StatusObj is gone. It read status.json without the lock, and
    it was the read that made a lost update possible.

  notify-island.ps1
    The agent-side writer now uses the same lock, the same snapshot
    and the same staging file. caller.json moved inside the same
    critical section, so its staging path is no longer shared between
    two hook processes that can run at once.

## Two defects found while doing this

1. The replace could lose a sharing violation. MoveFileEx with
   REPLACE_EXISTING fails when the target is open by a reader that did
   not share delete, and the widget polls status.json every 400 ms
   with Get-Content, which opens with FileShare.Read. A write landing
   in that window threw and the update was lost -- the staging file
   survived, the target kept the old payload. Measured on this host,
   not inferred. The replace now retries briefly; the reader holds the
   file for microseconds, so one short retry clears it.

2. The write path returned the version, and the detector calls it from
   inside its main loop -- so the daemon's stdout got a bare number on
   every write. The store's write function returns nothing now, and a
   check pins that.

## Test evidence (negative-injection verified)

The behavioral suite now runs the REAL Write-Status against a REAL
status.json in an isolated %APPDATA%. The old harness stubbed the
reader and pasted a fragment of the function body into a synthetic
script -- which could not tell a working implementation from one that
read the file twice, because the second read was exactly what the stub
hid. That is the bug the review named, and the old test could not see
it.

  test-substep-progress.mjs: 84 pass, 0 fail (was 51)
  smoke.mjs:                  125 pass, 7 warn, 0 fail (was 116)

  New: two real PowerShell processes interleaving on one status.json.
  Writer A is the detector's read-modify-write, writer B is the agent's
  full push. A signals that it has read, then waits for B to signal
  that it is about to write; B's signal is published BEFORE B takes the
  lock, so the ordering is deterministic either way.

    with the lock      A holds it, B blocks, A writes, B writes last
                       -> the file carries B's values
    without the lock   B writes immediately, A writes 400 ms later
                       from its stale read
                       -> the file carries A's stale values and B's
                          write is gone

  The version counter is a second, independent witness: both writers
  derive it from the snapshot they read, so two writers that read the
  same generation mint the same version.

  9 negative-injection cases, each a minimal change that breaks one
  clause of the contract. All 9 turn the suite red; the baseline
  restores to 80/0 after each:

    N1 lock removed from the write path            -> 2 fail
    N2 the write path reads the file twice         -> 2 fail
    N3 metadata refresh stops preserving sub-step  -> 4 fail
    N4 state-inference stops clearing the sub-step -> 4 fail
    N5 the version counter stops advancing         -> 3 fail
    N6 notify-island stages through a shared path  -> 1 fail
    N7 the inference site passes -MetadataRefresh  -> 1 fail
    N8 notify-island stops taking the lock         -> 1 fail
    N9 metadata refresh stops preserving the owner -> 1 fail

  N6 and N8 did not bite on the first pass, and the reason was worth
  fixing rather than papering over:

    N6's check looked for the literal "$statusFile.tmp". A rewrite that
    spells the same shared path differently walks straight past it, so
    the suite stayed green while the bug was back. The check now
    asserts the POSITIVE contract -- status.json may only be written
    through the shared store -- instead of one spelling of the bug.

    N8's patch did not apply, because the indentation in the patch text
    did not match the file. A setup miss is reported as a setup miss,
    not as a pass: counting it as green would have been a false
    negative in the harness itself.

  The two layers are bound together explicitly. Section 7 proves the
  PROTOCOL -- two real processes, one file, no lost write -- using
  hand-written writers, because the deterministic choreography needs the
  read to be separable from the write, and the real code deliberately
  does not separate them. The static checks are what say that the real
  detector code goes through that protocol: the arbitration block takes
  the lock and writes through the locked body, and no other call site
  writes at all.

  Also verified: the detector still initialises and writes under
  Windows PowerShell 5.1 (exit 0, the interpreter it runs under in
  production), and the local Windows runner passes all 4 steps,
  including the real PreToolUse hook path through the new locked write.

## Design compliance

  no credentials      : none added; the lock is a local file handle
  no network          : no network calls added
  no telemetry        : none added; island.log unchanged
  no third-party svcs : no new dependencies; pure PowerShell
  cross-platform      : no host paths introduced; the existing
                        hardcoded-path scan still passes
  atomic write        : staging + rename, now under an exclusive lock
  schema              : additive. `version` is a new optional field;
                        the widget reads every field through
            …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants