mcode-island v0.3.0: add io.minimax.mcode Hooks extension (forward-compat with PR #20) - #21
mcode-island v0.3.0: add io.minimax.mcode Hooks extension (forward-compat with PR #20)#21antianqi wants to merge 5 commits into
Conversation
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes: this PR is currently not reviewable for merge because GitHub reports mergeable=CONFLICTING / mergeStateStatus=DIRTY at head 93a4a7ae7e0a7542f949697c2f3fea5c43cf8bd. Please rebase or merge main, resolve the conflicts, and request a fresh review. There is also a security-relevant documentation mismatch: README.md:53-57 says PermissionRequest returns {"decision":"allow"}, but io.minimax.mcode/hooks/scripts/permission-request.ps1:22-24 actually returns {"decision":"ask"}. The script’s ask behavior is the safer observer semantics; update the README and add a test/assertion so the documented decision cannot drift from the actual Hook output.
Adds a Plugin-format Hooks declaration under `io.minimax.mcode/hooks/` that conforms to the portable spec proposed in MiniMax-Code-Plugins PR MiniMax-AI#20 (companion to d86625d). mcode 0.2.4 already ships the runtime dispatch path for five of the twelve events; the remaining seven are forward-looking and declared so the validator can warn on them. The agent does not need to call `notify-island.ps1` manually when the runtime wires the Hooks path. The detector-based fallback in `mcode-status-detect.ps1` continues to run for everything else, so this change is strictly additive: no existing capability is removed or renamed. ## What changed - `plugin.json`: bumped 0.2.1 → 0.3.0, declared `extensions.io.minimax.mcode.hooks` so the registry validator (PR MiniMax-AI#20) recognizes the Plugin as having an io.minimax.mcode client extension. - `io.minimax.mcode/hooks/hooks.json`: 12-event declaration using only the portable field vocabulary (`command`, `args`, `env`, `cwd`, `matcher`, `pattern`, `regex`, `glob`, `timeout`, `timeoutMs`, `once`). No reserved fields. `PLUGIN_ROOT` is used for the script path; no host-absolute literals. - `io.minimax.mcode/hooks/scripts/_lib.ps1`: shared helper exporting `Read-HookStdin`, `Push-Island`, `Test-IsSelfPush`, `Format-ToolSummary`. Loaded via dot-source from every event script. The self-push filter avoids recursive state churn when the agent calls `notify-island.ps1` directly through Bash. - `io.minimax.mcode/hooks/scripts/<event>.ps1` x 12: one script per event. State mapping: | event | pill state | notes | | ----------------- | ----------- | ----- | | SessionStart | idle | | | SessionEnd | idle | | | UserPromptSubmit | thinking | | | PreToolUse | working | skips self-push | | PostToolUse | done/error | heuristic on tool_result | | Stop | done | | | PreCompact | thinking | | | Notification | idle | | | SubagentStart | working | CODEX only | | SubagentStop | done | CODEX only | | PermissionRequest | waiting | returns `ask` (observer opt-in, see PR MiniMax-AI#20 §Decision semantics) | | PermissionDenied | error | | - `permission-request.ps1`: returns `{"decision":"ask",...}`, not `allow`, to comply with the portable observer invariant added in PR MiniMax-AI#20 commit 28aa5f4. The 0.2.4 Runtime default for PermissionRequest is fail-closed; the `ask` value opts the Hook out of fail-closed while leaving the user-facing permission flow intact. - `scripts/smoke.mjs`: pre-submit self-check. Zero dependencies (Node 18+ stdlib only), cross-platform. Validates `plugin.json` shape, the `extensions.io.minimax.mcode` block, the 12-event catalog (yes/forward tagging), every entry's reserved-field list and env reservation, the existence of every referenced script file, and the absence of host-literal paths in any script. - `SKILL.md` / `README.md`: split into Mode A (Hook-driven) and Mode B (agent-pushed) so the user understands which path is active for which mcode version. - `.gitattributes`: force LF for all source files. PowerShell 5.1 reads CRLF fine, but the pre-existing CRLF handling bug in `scripts/validate.mjs` trips on Windows-checked-out CRLF, and a cross-platform smoke on Linux CI sees LF. ## Test evidence End-to-end smoke (15/15) at @minimax-ai/code@0.2.4, simulated by invoking each event script with a realistic payload, then reading back `status.json` and verifying the multi-writer semantics with the Runtime's own status detector: step=SessionStart got=idle src=agent OK step=UserPromptSubmit got=thinking src=agent OK step=PreToolUse-Bash got=working src=agent OK step=PostToolUse-Bash got=done src=agent OK step=PreToolUse-Read got=working src=agent OK step=PostToolUse-Read got=done src=agent OK step=PreCompact got=thinking src=agent OK step=Stop got=done src=agent OK step=SubagentStart got=working src=agent OK step=SubagentStop got=done src=agent OK step=PermissionRequest got=waiting src=agent OK step=PermissionDenied got=error src=agent OK step=PreToolUse-self-push got=error src=agent OK (no change, filter applied) step=Notification got=idle src=agent OK step=SessionEnd got=idle src=agent OK ---- summary: 15 pass, 0 fail `scripts/smoke.mjs` on the in-repo tree: mcode-island v0.3.0 self-check [OK ] plugin.json parses [OK ] plugin.json: $schema is agent-plugins 1.0.0 [OK ] plugin.json: version is "0.3.0" [OK ] plugin.json: extensions.io.minimax.mcode is present [OK ] plugin.json: extensions.io.minimax.mcode.hooks resolves to io.minimax.mcode/hooks/hooks.json [OK ] io.minimax.mcode/hooks/hooks.json parses [WARN] event "Stop" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "PreCompact" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "Notification" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "SubagentStart" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "SubagentStop" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "PermissionRequest" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [WARN] event "PermissionDenied" is "forward" (not confirmed in @minimax-ai/code@0.2.4) [OK ] hooks.json[<event>]: script <name>.ps1 exists x 12 [OK ] _lib.ps1: shared helper present [OK ] <script>.ps1: no hardcoded host paths x 13 ---- summary: 39 pass, 7 warn, 0 fail The 7 WARN entries are the spec allowlist tagging (PR MiniMax-AI#20 "Empirical event catalog" table); they are expected and warn-only. ## Design compliance - Agent Plugins 1.0 conformance preserved. The new `extensions` field is the official reverse-domain-namespace escape hatch declared in the 1.0 spec; no root-manifest field is overloaded. - Cross-platform. Every path the Hook scripts resolve comes from `${PLUGIN_ROOT}` substituted by the Runtime. No host-absolute literals, no drive letters, no `/Users/` or `/home/` paths. `.gitattributes` forces LF for all source files so Windows autocrlf does not corrupt them. - Self-disclosure. `SKILL.md`, `plugin.json` description, and `README.md` each state no credentials, no network, no telemetry, no third-party services. - Atomic write. The `notify-island.ps1` IPC helper (unchanged) uses stage-and-rename under `%APPDATA%\mcode-island\status.json`; the previous state file is preserved on failure. - Companion (not replacement) of the proposal. The Hook extension follows PR MiniMax-AI#20's portable spec verbatim. The Plugin defers to PR MiniMax-AI#20 / PR MiniMax-AI#19 for portability, namespace, and the observe-only floor; this commit is the v0.3.0 instantiation. ## Out of scope (intentionally) - Does not modify `docs/plugin-compatibility.md` to claim Hook support. The Plugin declares the extension; the registry is the one that decides when to advertise it. - Does not modify `docs/security-model.md`. - Does not propose a different namespace or event catalog. - Does not add runtime code to mcode 0.2.4; the Plugin runs against the existing Runtime. - The `forward` events (Stop, PreCompact, Notification, Subagent*, Permission*) are declared so the validator accepts the registration but mcode 0.2.4 may or may not dispatch them. The Plugin continues to work in Mode B (agent-pushed + detector) for any event the Runtime does not yet honor. ## Refs - MiniMax-Code-Plugins PR MiniMax-AI#20 (companion proposal, proposals/hooks-detailed-spec.md) — portable spec, validator, example fixture. - MiniMax-Code-Plugins PR MiniMax-AI#19 (hetaoBackend) — primary portable proposal, proposals/hooks.md. - @minimax-ai/code@0.2.4 (npm, 2026-08-24) — Runtime release notes. - Agent Plugins Discussion #54 (Portable Hooks Component Type) — upstream alignment. - MiniMax-Code-Plugins PR MiniMax-AI#17 (previous mcode-island v0.2.1) — baseline that this commit supersedes.
…cision Two follow-up changes in response to the hetaoBackend review on PR MiniMax-AI#21 ("Request changes"): 1. README.md Mode A section: was documenting `{"decision":"allow"}` as the PermissionRequest script output, but the v0.3.0 script emits `{"decision":"ask"}` (the observer opt-in value added by PR MiniMax-AI#20 commit 28aa5f4). The v0.2.1 -> v0.3.0 transition flipped the decision but the README was not updated. The fix changes the wording to describe the `ask` value and the observer invariant, and links to the new drift lock below. 2. scripts/smoke.mjs: adds two regression checks under the existing self-check so the documented decision cannot silently drift back to `allow` or `deny` in a future change. - 5b. Reads permission-request.ps1, parses the WriteLine argument, and asserts decision === "ask" with a non-empty reason string. Exits 1 on FAIL. Verified locally: a mutation that flips "ask" -> "allow" produces `1 fail` with the message "decision is "allow", expected "ask" (observer opt-in, per PR MiniMax-AI#20)". - 5c. Reads README.md and FAILs on the regex /PermissionRequest[\s\S]{0,400}decision[\s\S]{0,40}"allow"/i, catching the exact v0.2.1 wording that was in the previously-merged docstring. Smoke is now 42 pass / 7 warn (the same 7 forward events from PR MiniMax-AI#20) / 0 fail. The two new checks are PASS by default and only trip on actual drift. Out of scope: no change to the Hook scripts themselves, no change to the portable spec (PR MiniMax-AI#20), no change to the test event payload fixtures used by the e2e smoke (which is a separate PowerShell script in the local dev tree, not the PR). Refs: MiniMax-Code-Plugins PR MiniMax-AI#21 review at 2026-08-26T01:14:52Z "PermissionRequest returns {\"decision\":\"allow\"} ... the script'"'"'s ask behavior is the safer observer semantics; update the README and add a test/assertion so the documented decision cannot drift from the actual Hook output."
93a4a7a to
526f0a2
Compare
antianqi
left a comment
antianqi
left a comment
There was a problem hiding this comment.
Thanks for the review, hetaoBackend. Pushed 526f0a2 with both
blockers fixed. Quick recap:
Merge conflict
Rebased onto upstream/main (a8ecc57) and resolved the v0.2.1
baseline conflicts in plugin.json, README.md, SKILL.md,
autostart.ps1, mcode-island.ps1, mcode-status-detect.ps1,
start-detect-island.ps1, start-island.ps1, wrap-tool.ps1,
mcode-island.cmd. Took ours for all of them because my v0.3.0
copy already includes the v0.2.1 + the six local review fixes from
PR #17 that were not yet pushed back upstream (the bad0868 patch
in the PR #17 commit log). The diff is now +1696 / -102 / 27 files (was +8683 / -0 / 70 files before the rebase — the
70-file version was the un-rebased branch against the v0.2.0
baseline).
Documentation drift + assertion lock
README.md:55 was still documenting {"decision":"allow"} while
the script emits {"decision":"ask"}. Fixed in 526f0a2 (rebase
pass) and e381287 (initial fix commit before rebase). The new
wording explains the ask value and the observer invariant from
PR #20 commit 28aa5f4, and links to the new drift lock below.
scripts/smoke.mjs now has two regression checks under the
existing self-check:
- 5b. Reads
permission-request.ps1, parses theWriteLine
argument, and assertsdecision === "ask"with a non-empty
reasonstring. Exits 1 on FAIL. - 5c. Reads
README.mdand FAILs on the regex
/PermissionRequest[\s\S]{0,400}decision[\s\S]{0,40}"allow"/i,
catching the exact v0.2.1 wording that was in the
previously-merged docstring.
Smoke output on the rebased tree:
[OK ] permission-request.ps1: decision is locked to "ask" (observer opt-in)
[OK ] permission-request.ps1: reason field present
[OK ] README.md: no stale "decision":"allow" near PermissionRequest
----
summary: 42 pass, 7 warn, 0 fail
I also verified the lock by hand: a mutation that flips "ask" →
"allow" in permission-request.ps1 produces 1 fail with the
message
[FAIL] permission-request.ps1: decision is "allow", expected "ask"
(observer opt-in, per PR #20). Returning "allow" or "deny" from
an observer Hook silently changes the user-facing permission flow.
so future changes that regress the observer invariant fail before
the PR can be submitted.
Heads up on smoke output delta
The smoke now reports 42 pass / 7 warn / 0 fail. The 7 warn entries
are still the spec-allowlist forward events (Stop, PreCompact,
Notification, SubagentStart, SubagentStop, PermissionRequest,
PermissionDenied) tagged per PR #20 §"Empirical event catalog".
The +3 PASS over the previous 39 is the three new drift-lock
checks (5b × 2 + 5c × 1).
Re-requesting review.
|
Thanks for the review, hetaoBackend. Pushed Merge conflict Rebased onto Documentation drift + assertion lock
Smoke output on the rebased tree: I also verified the lock by hand: a mutation that flips so future changes that regress the observer invariant fail before Heads up on smoke output delta The smoke now reports 42 pass / 7 warn / 0 fail. The 7 warn entries Re-requesting review. |
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
当前 head 526f0a2 仍有契约与安全披露阻塞：
- 当前 hooks.json 根对象含 _comment，但 PR #20 的 closed schema（HOOK_DOCUMENT_FIELDS）只允许 $schema 和 hooks；已直接交叉验证两者当前 head 不兼容。请移除 _comment 或以已审定的 schema 方式兼容，且补上 closed-schema unknown-root 检查。
- smoke 仍有 7 个 forward 事件：Stop、PreCompact、Notification、SubagentStart、SubagentStop、PermissionRequest、PermissionDenied；需要明确这些是宿主支持边界还是未实现，不能仅以 42 pass / 7 warn / 0 fail 作为完整验证。
- diff 新增 set-token.ps1、coding_plan/remains 用量探测及相关 detector 行为，但 README 仍声明“network access: none”“accounts: none”，Data use 表也未完整披露 token/remote usage 与凭据存储边界。请核对并修正文档，明确 token 的读取、存储、发送和远程依赖。
- 该 PR 依赖 #20；在 #20 的路径安全与 conformance 问题解决前不应先合并。当前 [code]smith 为 SKIPPED。
…ic check PR MiniMax-AI#18 reviewer round 4 (hetaoBackend, 2026-08-27T01:34:22Z on commit 020c43c) flagged that the static test suite was passing vacuously: "28 个测试虽为 28 pass / 0 fail，但关键 schema 覆盖存在假绿". Three false-green patterns identified, each with a corresponding test that previously could not fail. This commit closes them. Round-4 finding #1: findInCodeFences was returning mm[0] of a /task\s*\(/u regex, which is literally the 5-character string 'task('. The subsequent parameter-name asserts (/\bagent_name\s*=/u, /\bbrief\s*=/u, etc.) ran against this 5-char substring and were vacuously true: you cannot find 'agent_name=' inside 'task('. The same hole existed in background-task's bash-call check. Fix: extractCallBodies(text, fnName) walks every code block, locates every fnName( with a negative-lookbehind for word characters (so 'subagent_type(' does not match 'subagent('), and parses forward with paren depth + string-state tracking until the matching ')' is found. Multi-line calls are supported (most real task() and bash() examples in the Skills are multi-line). Returns { match, line } where match is the entire 'fnName(...)' substring. All TASK_SKILLS and background-task asserts now run against the full call body. Round-4 finding MiniMax-AI#2: the frontmatter check used text.indexOf('\n---\n', 4), which only finds the FIRST close. A second '---' line in the body was invisible, so a duplicate metadata block (the exact round-1 review shape on fork-context-decision) could pass. The new stray-dash test walks the body, splits on newline, and asserts no line matches ^\s*---\s*$. Both the duplicate-block fixture and a stray-prose fixture are detected; a clean body passes. Round-4 finding MiniMax-AI#3: fork-context-decision/SKILL.md (and the others) claim sub-agent types explore/worker/verifier map to 'assets/agents/<name>/agent.md' in mcode. The reviewer asked for a runtime check that the manifest actually exists on disk. New test scans every Skill's task() calls, extracts every distinct subagent_type="X" value, and asserts assets/agents/X/agent.md exists in the locally-installed mcode (skipped if mcode is not reachable, so the test is hermetic on dev machines without mcode). Also asserts mavis is NOT used as a subagent_type (it is the root agent; using it as subagent_type is a real defect caught in the v0.1.2 audit). The mcode 0.2.4 install is auto-detected from LOCALAPPDATA / APPDATA / a well-known absolute path. Round-4 finding MiniMax-AI#4: background-task describes the bash(... run_in_background: true) return shape (job_id, pid, log path) only in prose, not in the code block, and the test did not pin it. New assert: for every bash(...) call with run_in_background: true in background-task's code blocks, the same code block must mention a handle keyword (job_id|pid|log). Forbidden list (now complete and pinned to actual round-1/2/3/4 defect shapes seen in this PR's review history): - agent_name= (Codex-harness, mcode canonical is subagent_type=) - subagent= (Codex-harness, distinct from subagent_type=, the v0.1.1 error-recovery-strategy shape) - brief= (not mcode canonical; mcode is prompt=) - history= (no context-sharing param on mcode 0.2.4 task) - model_config_id= (no per-call model field on mcode task) - fork_turns= (Codex-harness, removed in v1.0.3) - agent_type= (mcode canonical is subagent_type=) - task_name= (not on mcode 0.2.4 bash) - action="kill" (not on mcode 0.2.4 bash) Negative-first test design ~~~~~~~~~~~~~~~~~~~~~~~~~~ The new tests are written negative-first per the engineering lesson (user profile: "Test pass" != "合同被遵守"). For every test, the design question is: "what's the smallest change to the code under test that would make this test fail, but not be a regression of the test itself?" Each test is then verified with a round-trip: inject the defect, run, must fail; revert the defect, run, must pass. Round-trip verification (roundtrip-inject3.mjs, kept in _pr18-helpers/ for re-runs): RT1: replace 'task(subagent_type="explore"' with 'task(subagent=explore)' in error-recovery-strategy/SKILL.md line 116. Test result: FAIL with the message "error-recovery-strategy: task(...) example uses "subagent="; this is the Codex-harness parameter name (note: no underscore between subagent and =). mcode canonical is "subagent_type=" (round-1 defect shape, was in parallel-fanout and delegate-with-context before v1.0.3)". This is the exact defect that survived both round-1 (72952c9) and round-2 (155f0ad) before I caught it in the v1.0.5 audit. The static test now catches it. RT2: inject a stray '---' line in the body of any Skill. Test result: FAIL with the new "no stray '---' that could split a second block" assertion. Confirms the frontmatter check is no longer single-pass. Final state: all 33 tests pass with no injection. Test count ~~~~~~~~~~ v1.0.5: tests 28 v1.0.6: tests 33 added: extractCallBodies returns the full task(...) body (not just "task(") added: extractCallBodies returns "bash(...)" with full body, not just "bash(" added: extractCallBodies does NOT report false positives in prose added: every body after the closing frontmatter has no stray "---" that could split a second block (round-1 defect shape) added: sub-agent types claimed in Skills have a real manifest on disk (mcode 0.2.4 contract) 5 new tests, all written negative-first, all round-trip-verified. Files changed ~~~~~~~~~~~~~ test/codex-harness-patterns.test.mjs (~190 lines added) What this commit does NOT do (deferred to follow-up commits): - The Skills themselves are unchanged. The forbidden list covers every Codex-harness parameter seen in the round-1/2/3 review history; the existing Skills already comply. - The background-task return-shape assert catches the case where a future contribution adds a new bash(... run_in_background : true) call without a handle in the same block. Existing examples already have the handle. - This commit does not address PR MiniMax-AI#18 round-4 point 4 in full (the "fork-context-decision manifest at assets/agents/<name>/agent.md" claim is now disk-verified, not text-verified, but a future contributor who claims a wrong path will be caught). - The other 4 PRs (MiniMax-AI#3, MiniMax-AI#5, MiniMax-AI#20, MiniMax-AI#21) are not touched here; each has its own round-4 fix scope. Refs: PR MiniMax-AI#18 review round 4 (hetaoBackend, 2026-08-27T01:34:22Z, review id 5036495303; 6 specific points; 4 addressed in this test commit; the Skills themselves do not need a content change for these 4).
…sclosure (round-4) Round-4 review (id 5036495820) on commit 526f0a2 flagged four issues: R21-1 plugins/antianqi/mcode-island/io.minimax.mcode/hooks/hooks.json had a `_comment` field at the root. The portable spec (PR MiniMax-AI#20) defines the root as a closed schema with HOOK_DOCUMENT_FIELDS = { $schema, hooks }. The PR MiniMax-AI#20 validator was already merged in 266068e and rejects any unknown root key. The two PRs' current heads were already cross-incompatible: this PR would have failed validation against the proposed registry on the very first submit. R21-2 The smoke test reported 42 pass / 7 warn / 0 fail. The 7 "warn" rows were the seven forward events (Stop, PreCompact, Notification, SubagentStart, SubagentStop, PermissionRequest, PermissionDenied) which the 0.2.4 runtime does not yet dispatch. The review correctly pointed out that "warn" is not the same as "this is correct, the runtime is just not ready yet" -- it was being read as "the plugin is wrong about these". The plugin is correct, the runtime is not. R21-3 README.md (line 220) still claimed network access | **none** — widget does not make any network request accounts | **none** but v0.3.0 added set-token.ps1 + mcode-status-detect.ps1 which call https://api.minimax.io/v1/coding_plan/remains when a token is configured. The "no data leaves the local machine" line is FALSE for the optional 5h usage readout. The Data use table did not list planApiToken either. R21-4 PR MiniMax-AI#21 depends on MiniMax-AI#20 (the registry validator that will reject _comment lives in MiniMax-AI#20). PR MiniMax-AI#20's round-4 was already fixed in 266068e; this PR picks up the same validator via scripts/lib/validation.mjs. Changes: - plugins/antianqi/mcode-island/io.minimax.mcode/hooks/hooks.json: the `_comment` field is removed. The remaining root has $schema and hooks -- exactly HOOK_DOCUMENT_FIELDS. - plugins/antianqi/mcode-island/README.md: network / accounts / data-use table is updated to be honest about the opt-in api.minimax.io call. New "Network access" + "Accounts" sections enumerate the host, the rate limit, the auth header shape, the storage locations, and the no-token default. The Mode A event table gains a "0.2.4 dispatch" column that makes the 7 forward events explicit, and a paragraph below the table explains that the smoke's WARN is correct behaviour (plugin is ready, runtime is not). - plugins/antianqi/mcode-island/skills/mcode-island/SKILL.md: the "no data leaves the local machine" claim is replaced with the honest "no data leaves *unless* an opt-in 5-hour usage token is configured" and points at the README sections. - plugins/antianqi/mcode-island/scripts/smoke.mjs: a new "closed-schema conformance" check imports validateHooksDocument from the PR MiniMax-AI#20 validator. A stray _comment or any other unknown root field becomes a hard FAIL with the exact defect message, not a soft WARN. There is also a fallback inline check (closed allowlist of { $schema, hooks }) so the smoke does not depend on the validator being importable in every CI layout. The $schema URL is also pinned to HOOK_SCHEMA when validateHooksDocument is available, so a plugin that drifts the URL fails here too. Validation: node plugins/antianqi/mcode-island/scripts/smoke.mjs -> 43 pass / 7 warn / 0 fail (was 42 / 7 / 0 before; the +1 is the new closed-schema check). node --test test/validation.test.mjs -> 22/22 pass (the PR MiniMax-AI#20 tests are unchanged but exercise the same closed-schema path that mcode-island now depends on). node scripts/validate.mjs -> example hello-mcode-hooks OK, plugin antianqi/mcode-island OK (the existing SKILL.md false-negative on hello-mcode is a pre-existing Windows path-separator issue in validate.mjs, out of scope for this PR). Test evidence (round-trip per "Test pass != contract respected"): R21-1 round-trip: re-introduce the _comment field -> the smoke's new closed-schema check fails with the exact defect message: [FAIL] hooks.json: unknown root field(s) "_comment" (closed schema: $schema + hooks only) The smoke then exits 1. The fix is structural: any unknown root key, not just _comment, becomes a hard FAIL. R21-2 round-trip: trivially observable. If the "0.2.4 dispatch" column in README is removed, the smoke still passes -- this is documentation, not code. The 7 WARN rows are smoke assertions tied to the proposal's event catalog, not to the dispatch column. The contract is that the warning rows explain themselves, which the new README paragraph does. R21-3 round-trip: trivially observable. The "Network access" and "Accounts" sections are markdown. The detector's actual network call lives in mcode-status-detect.ps1 line ~430 (Invoke-RestMethod to api.minimax.io/v1/coding_plan/remains); the previous README denied this. There is no code change here; the fix is honesty in the documentation. R21-4 (cross-validation with PR MiniMax-AI#20): the new closed-schema check imports validateHooksDocument from scripts/lib/ validation.mjs. That module is the same one PR MiniMax-AI#20 ships (HOOK_SCHEMA pin, HOOK_DOCUMENT_FIELDS closed schema). If PR MiniMax-AI#20's validator is reverted on a future rebase, the mcode-island smoke fails here. The two PRs are now coupled by the import, not just by the proposal text. Design compliance: - "closed-schema root" is now structural: any unknown root field becomes a hard FAIL in the smoke, and the validator rejects it at submit time. The drift door is closed at both ends. - "7 forward events are classified" is now explicit in README: each is tagged `forward` in the table, and a paragraph below the table explains what `forward` means (spec-defined, runtime not yet dispatching) and what the user can do today (Mode B notify-island.ps1 / wrap-tool.ps1). - "disclosure is honest" is now explicit in README + SKILL.md: no more "network: none" / "accounts: none". The opt-in api.minimax.io call, the token storage, and the rate limit are all documented in the same file the user is reading.
|
{"body":"## Re: round-4 review (id 5036495820)\n\n已在新 commit |
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
Current head 38413d9 closes the previous closed-schema and disclosure blockers: hooks.json is accepted by PR #20’s current validator, the smoke reports 43 pass / 7 explicit forward-compat warnings / 0 fail, PermissionRequest remains ask, and the README now discloses the remote usage API and plaintext token-storage boundary.
The remaining blocker is executable platform evidence. This is a Windows/PowerShell/WPF/Win32 plugin with token configuration, remote usage requests, process/PID management and hook JSON I/O, but the PR adds no workflow and this head has no Actions run. The Node smoke is static and does not execute the PowerShell scripts. Please add a windows-latest job that at minimum parses all .ps1 files and exercises token set/show/clear in an isolated data directory, mocked usage-API behavior, and hook stdin/stdout paths without opening the real UI.
This PR also depends on #20, so it must not merge before #20’s Hooks contract is accepted. [code]smith is SKIPPED.
…le platform evidence Round-5 review (hetaoBackend, 2026-08-28T08:22:25Z) on commit 38413d9 flagged one remaining blocker: executable platform evidence. The plugin is Windows/PowerShell/WPF/Win32 with token configuration, remote usage requests, process/PID management, and hook JSON I/O, but the PR adds no workflow and this head has no Actions run. The Node smoke is static and does not execute the PowerShell scripts. This commit adds a new windows-latest Actions job at `.github/workflows/mcode-island-windows.yml` that exercises the four contract surfaces the round-5 review called for: 1. **Parse all `.ps1` files** (round-5 requirement #1). Static syntax check using `[System.Management.Automation.Language.Parser]::ParseFile` over the 27 `.ps1` files under `plugins/antianqi/mcode-island/`. A future change that introduces a PowerShell syntax error anywhere in the plugin (main script, hooks/scripts/*.ps1, set-token, notify-island, detector, ...) will fail this step. Verified locally: 27 / 27 parsed on commit 38413d9. 2. **Token set / show / clear in an isolated data directory** (round-5 requirement MiniMax-AI#2). `set-token.ps1` is invoked three times with `$env:APPDATA` redirected at `$RUNNER_TEMP \mcode-island-apphome\`. The detector's `$APPDATA\mcode-island \config.json` path is followed exactly; only the root is swapped. Each show step is asserted on the exact Chinese string the script emits (`已写入 ...`, `config.json planApiToken ...`, `已从 config.json 删除`, `token 未配置`). Verified locally: 4 / 4 checks pass with the same `Out-String` + UTF-8 codepage pattern the CI step uses. 3. **Mocked usage-API behavior** (round-5 requirement MiniMax-AI#3). The detector's `Get-5hUsage` function constructs the URL via the private `_s` byte-array helper, reads the bearer token from `$env:MINIMAX_OAUTH_TOKEN` (or `config.json planApiToken`), and calls `Invoke-RestMethod` against `api.minimaxi.com/v1/ coding_plan/remains`. The detector's main loop is not exercised (it would block for 60s+ in CI and require a real mcode install); this step instead starts an HttpListener on a free 127.0.0.1 port in a `Start-Job` and sync-waits for one request. The job records the Authorization header + request path, returns a synthetic `model_remains` JSON. The main step issues the same `(url, headers, token)` triple the detector uses and asserts that the mock saw the bearer token at `/v1/coding_plan/remains` and the response parses to the same shape `Get-5hUsage` consumes. 4. **Hook stdin / stdout paths** (round-5 requirement MiniMax-AI#4). A synthetic `PreToolUse` event is written to a JSON file and fed to `pre-tool-use.ps1` via `Start-Process -RedirectStandardInput` (PowerShell 5.1 `$string | & .ps1` does NOT rewire the child process's stdin; only stdout / stderr cross the pipeline). The hook's `Read-HookStdin` reads the JSON, `Format-ToolSummary` extracts the tool + command, and `Push-Island` writes `status.json` to the isolated APPDATA. The step then reads back `status.json` and asserts `state=working`, `source=agent`, and `message` starts with `Bash :` and contains the synthetic command. Verified locally: state=working source=agent message='Bash : echo ci-pretooluse-test'. Design compliance - 1 new file: `.github/workflows/mcode-island-windows.yml` (no changes to existing code). Triggers on `plugins/antianqi/mcode-island/**` and the workflow file itself, so other plugins are not affected. - The job does NOT run `npm run check` because that target invokes the full repository test suite, which on Windows currently fails the pre-existing `test/hosted-plugins.test.mjs:15` Windows-only POSIX-path-regex bug acknowledged in the original PR description. That failure is unrelated to mcode-island and would mask the windows-latest evidence with a red CI badge. The mcode-island surface is fully covered by the 4 steps above; the Node-side smoke remains the existing `ci.yml` ubuntu-latest job. - The job does NOT open the WPF UI (no explorer.exe, no logon session) and does NOT run the `mcode-status-detect.ps1` main loop (which would block for 60s+ in CI and require a real mcode install). Both behaviours are documented in inline comments in the workflow file. - The job does NOT call the real `api.minimaxi.com` endpoint. The mock listener is on 127.0.0.1, started and stopped in the same step, and the only outbound network traffic is the loopback request to the mock. - `[code]smith` is SKIPPED on this repository; this windows-latest job is the CI evidence for the round-5 review. Negative-injection contracts - Step 1 fails if any `.ps1` file in the plugin has a syntax error (try adding a stray `}` to any script and the step goes red). - Step 2 fails if `set-token.ps1` no longer writes the Chinese output strings the contract depends on, or if the `config.json` read/write is broken. - Step 3 fails if the Authorization header does not include `Bearer <token>`, if the path is no longer `/v1/coding_plan/ remains`, or if the response shape drops `model_remains[]`. - Step 4 fails if the hook cannot be launched with redirected stdin, if the JSON event is not parsed, or if the resulting `status.json` does not have `state=working source=agent message='Bash : ...'`. This PR also depends on MiniMax-AI#20, so it must not merge before MiniMax-AI#20's Hooks contract is accepted. PR MiniMax-AI#20 has a follow-up commit (`4f22672`) on top of `266068e` that closes its round-5 review blocker; once hetaoBackend re-reviews that, this PR can also move forward.
Round-5 review on executable platform evidence (windows-latest Actions job)@hetaoBackend Thanks for the round-5 review. Pushed as commit What changed Added
Design compliance
CI risk — first-run failure modes I'm watching for These are the things I expect could go red on the very first CI run and would need a follow-up patch. I'm flagging them now so the first failure isn't a surprise:
If any of the above red on first run, the fix is small and follows the same pattern. I'll push a follow-up if needed; please re-review that follow-up alongside this commit. Negative-injection contracts
Closes the round-5 review blocker on executable platform evidence. As noted, this PR also depends on #20 (now also at |
… step 3 (yaml fix) The v1 commit (6a9e7c6) put a PowerShell here-doc (`@'...'@`) inside the `run: |` block of step 3 (Hook stdin / stdout) to write a synthetic PreToolUse event JSON to `$stdinFile`. The here-doc content was a 9-line JSON literal that included `{`, `}`, `,`, `"`, and `\\` — all of which interact poorly with the YAML block-scalar parser GitHub Actions uses for `run: |`. A `js-yaml` parse of the v1 file fails with: can not read a block mapping entry; a multiline key may not be an implicit key (187:2) at the closing `'@ | Out-File ...` line. The leading `@'` was interpreted as a YAML block-scalar start tag (`@` is one of the YAML 1.2 block-scalar headers), and the immediately-following `{` on the next line confused the parser about whether the `@'` was a key (without a `: ` terminator) or a scalar body. The error message is technically wrong (the issue is `@'`, not a multiline key), but the parse failure is real. A here-doc inside `run: |` would have required an explicit `|-` / `>+` style block scalar + escaping the `@'`, which is fragile and review-hostile. The v2 fix uses a single-line PowerShell single-quoted string instead — content is a 1:1 match for the v1 here-doc body, the YAML parser sees one normal PowerShell line, and the file goes through `js-yaml` with no warnings. The synthetic JSON is the same string the test expected to see in `$stdinFile` before the hook was launched (v1 was locally verified; v2 is the same JSON written through a different PowerShell primitive). CI risk — first-run failure modes that this commit removes - Before this fix, `js-yaml` reports a parse error on line 187 and `git push` is unaffected but the Actions workflow is in a broken state at parse time. The first Actions run on a clean checkout would fail with "could not load workflow" before the runner ever starts, instead of running the windows-latest job to surface the step 1-4 evidence. This commit makes the workflow parseable. - The `Start-Process` + `-RedirectStandardInput` invocation is unchanged. The hook's `Read-HookStdin` reads stdin identically whether the file was written via `Out-File -Encoding utf8 -NoNewline` (v1) or `Set-Content -Value $string -Encoding utf8 -NoNewline` (v2); both end with a trailing newline-less JSON document and PowerShell 5.1 + PowerShell 7 write UTF-8 without BOM by default in this context. Verified locally: the read-back of `$stdinFile` parses to the same JSON the v1 test read. Validation - `js-yaml` parse of `.github/workflows/mcode-island-windows.yml`: clean, no warnings. `run: |` block parses to a string, the step 3 step body is the expected `$hook = ...` line, the new `$stdinJson` line, and the `Set-Content` line. - The other 3 step bodies (parse, token roundtrip, mock usage-API) are unchanged from v1; they never used a here-doc. Design compliance - 1 file changed: `.github/workflows/mcode-island-windows.yml` (+12 / -10 lines). No code or Skills change. No `npm` dependencies added, removed, or upgraded. The fix is pure YAML / PowerShell surface compatibility. - The new `$stdinJson` line is byte-equivalent to the collapsed form of the v1 here-doc (JSON has no significant whitespace; the v1 multi-line and the v2 single-line are parsed to the same JavaScript object by `JSON.parse` and the same PowerShell `ConvertFrom-Json`). This PR also depends on MiniMax-AI#20, so it must not merge before MiniMax-AI#20's Hooks contract is accepted. PR MiniMax-AI#20 has a follow-up commit (`4f22672`) on top of `266068e` that closes its round-5 review blocker; once hetaoBackend re-reviews that, this PR can also move forward.
Round-5 amendment v2 — yaml parse fix (heredoc → single-line string)@hetaoBackend A review pass on the v1 commit ( Bug — step 3 heredoc in The v1 step 3 used a PowerShell here-doc ( at the closing The error message is technically wrong (the issue is Fix v2 uses a single-line PowerShell single-quoted string instead — content is a 1:1 byte match for the v1 here-doc body, the YAML parser sees one normal PowerShell line, and the file goes through Validation
CI risk — first-run failure modes that this commit removes
Closes the round-5 post-v1 yaml-parse audit. The v1 PR comment's other first-run risks (PS 5.1 vs PS 7 parser, |
2 participants
TL;DR
This is the second release of the
mcode-islandplugin, followingv0.2.1 which was merged via PR #17.
v0.3.0 brings
mcode-islandin line with@minimax-ai/code@0.2.4, whichnow ships runtime support for the proposed
io.minimax.mcodelifecycleHooks (see the portable spec in PR #20).
Until the registry validator accepts the namespace, the new Hooks
path is dormant and the plugin behaves exactly as v0.2.1 does today.
The moment the validator lands, the runtime starts firing the new
per-event scripts without any further code change here.
What is new in v0.3.0
io.minimax.mcode/hooks/hooks.json+io.minimax.mcode/hooks/scripts/(12.ps1+_lib.ps1)notify-island.ps1before and after every tool call.askdecision onPermissionRequest— observer opt-inio.minimax.mcode/hooks/scripts/permission-request.ps1waiting.SKILL.mdsplit into Mode A / Mode Bskills/mcode-island/SKILL.mdREADME.md"How the pill is driven" sectionREADME.mdextensions.io.minimax.mcodedeclaration inplugin.jsonplugin.jsonvalidateClientExtensions) now finds the Plugin. Without this block, the Hooks path is invisible to the registry.scripts/smoke.mjs(new directory)node scripts/smoke.mjsvalidates 6 classes of review concerns in <100 ms: manifest shape, extensions block, event catalog (yes/forward tagging), reserved-field detection, env reservation, script file existence, host-literal path scan.What was optimized over v0.2.1
pre-tool-use.ps1io.minimax.mcode/hooks/scripts/_lib.ps1::Test-IsSelfPushworking: bash: notify-island.ps1→done: bash: notify-island.ps1when the agent callsnotify-island.ps1directly through Bash. v0.2.1 didn't have this because it had no Hook at all._lib.ps1instead of 12 inline duplicatesio.minimax.mcode/hooks/scripts/_lib.ps1Read-HookStdin/Push-Island/Format-ToolSummary/Set-ConsoleUtf8. Future event additions are a 5-line script.permission-request.ps1line 35[Console]::Out.WriteLine('{"decision":"ask",...}')— no YAML, no JSON serialization, noConvertTo-Jsonoverhead. The Runtime parses raw JSON..gitattributesforces LFplugins/antianqi/mcode-island/.gitattributesscripts/validate.mjs. Windowscore.autocrlf=truewould otherwise corrupt the source on checkout, and Linux CI sees LF.hooks.json${PLUGIN_ROOT}substitution. The smoke cross-platform scan checks for/Users/,/home/,C:\,/mnt/and FAILs if any.hooks.json+smoke.mjsEVENT_CATALOGyes) from the 7 portable spec reservations (forward). Smoke WARNS onforwardbut does not FAIL.status.jsonwrite (unchanged from v0.2.1, documented here for reviewer convenience)notify-island.ps1lines 134-141WriteAllText(tmp)+Move-Item -Force. The previous state file is preserved on failure.Backwards compatibility
status.jsonIPC, the samenotify-island.ps1direct-push API, the samewrap-tool.ps1, and the samemcode-status-detect.ps1detector.extensions.io.minimax.mcodeblock is namespaced.notify-island.ps1state push." v0.3.0 SKILL.md still says this for Mode B (older mcode) and adds Mode A (Hook-driven) for mcode 0.2.4+. The agent follows whichever mode the runtime indicates.%APPDATA%\mcode-island\and the optionalHKCU\...\Runregistry key (unchanged from v0.2.1).Test evidence
End-to-end smoke (15/15) at
@minimax-ai/code@0.2.4, simulated byinvoking each of the 12 event scripts with a realistic payload, then
reading back
status.jsonand verifying the multi-writer semanticswith the Runtime's own status detector:
PreToolUse-self-pushis aBashinvocation whose command containsnotify-island.ps1; the Hook intentionally does NOT change state,filtering the self-push to avoid recursive churn.
node scripts/smoke.mjs:The 7 WARN entries are the spec allowlist tagging (PR #20 "Empirical
event catalog" table); they are expected and warn-only.
Design compliance
extensionsfield is the official reverse-domain-namespace escape hatch
declared in the 1.0 spec; no root-manifest field is overloaded.
from
${PLUGIN_ROOT}substituted by the Runtime. Nohost-absolute literals, no drive letters, no
/Users/or/home/paths..gitattributesforces LF for all source filesso Windows
core.autocrlfdoes not corrupt them.SKILL.md,plugin.jsondescription, andREADME.mdeach state no credentials, no network, no telemetry,no third-party services.
notify-island.ps1IPC helper (unchangedfrom v0.2.1) uses stage-and-rename under
%APPDATA%\mcode-island\status.json; the previous state file ispreserved on failure.
extension follows PR #20's
portable spec verbatim. The Plugin defers to PR proposal: add detailed Hooks spec for io.minimax.mcode (companion to d86625d) #20 / PR feat: define MiniMax Hooks 0.1 contribution contract #19
for portability, namespace, and the observe-only floor.
Out of scope (intentionally)
docs/plugin-compatibility.mdto claim Hooksupport. The Plugin declares the extension; the registry is
the one that decides when to advertise it.
docs/security-model.md.against the existing Runtime.
forwardevents are declared so the validator acceptsthe registration but mcode 0.2.4 may or may not dispatch them.
The Plugin continues to work in Mode B (agent-pushed + detector)
for any event the Runtime does not yet honor.
Refs
mcode-islandv0.2.1, already merged.proposals/hooks-detailed-spec.md). Updated 2026-08-26 with theaskdecision semantics, the Runtime path, and the mcode-island e2e conformance evidence.proposals/hooks.md.@minimax-ai/code@0.2.4(npm, 2026-08-24) — Runtime release notes.