feat(minimax-code): add MiniMax Code probe (hook + rollout dual input) - #233
Open
zy84338719 wants to merge 22 commits into
Open
feat(minimax-code): add MiniMax Code probe (hook + rollout dual input)#233zy84338719 wants to merge 22 commits into
zy84338719 wants to merge 22 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new “MiniMax Code” probe to LoongSuite Pilot, integrating it into the orchestrator and agent registry and providing both a hook-driven JSONL input and a rollout/transcript tail input to normalize MiniMax Code activity into AgentActivityEntry records.
Changes:
- Register
minimax-codeas a newClientTypeand wireminimax-code-log+minimax-code-rolloutinputs into the orchestrator detection map. - Add MiniMax Code hook assets (bash entrypoint + Node processor) and an agent definition for hook deployment.
- Add rollout/log inputs and accompanying unit tests + fixtures.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/inputs/minimax-code-rollout-input.test.ts |
Adds unit tests for rollout tail parsing, tool normalization, traceId normalization, and offset pre-seeding. |
tests/unit/inputs/minimax-code-log-input.test.ts |
Adds unit tests for hook JSONL input transform behavior and availability checks. |
tests/unit/hooks/minimax-code/fixtures/rollout/model-io-sess_test-session-001.jsonl |
Adds synthetic rollout JSONL fixtures for model-io parsing tests. |
tests/unit/hooks/minimax-code/fixtures/hook-events.jsonl |
Adds synthetic hook event fixtures for MiniMax Code hook pipeline tests. |
src/types/client-type.ts |
Registers ClientType.MiniMaxCode = 'minimax-code'. |
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts |
Implements rollout/session JSONL tail input for MiniMax Code, producing normalized llm.response entries. |
src/inputs/minimax-code-log/minimax-code-log-input.ts |
Implements hook JSONL tail input using shared transformHookRecord. |
src/core/orchestrator.ts |
Wires MiniMax Code inputs into orchestrator startup, detection entries, and LISTENER_AGENT_MAP. |
docs/agents.md |
Documents the new minimax-code agent ID and collection approach. |
assets/hooks/minimax-code-loongsuite-pilot-hook.sh |
Adds fail-open hook wrapper that locates a suitable Node runtime and dispatches to the processor. |
assets/hooks/minimax-code-hook-processor.mjs |
Adds MiniMax Code hook processor that normalizes hook payloads into event_t JSONL records. |
agents.d/minimax-code.json |
Adds MiniMax Code agent definition (hook deployment + flusher settings). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+158
to
+176
| // Step.id 派生: turnId + 当前文件已处理字节偏移 (不持久化, 仅 in-session stable). | ||
| // Round 2 计划: 持久化 turnStepMap (与 PR #101 zcode-rollout 对齐). | ||
| const stateKey = `${this.id}:${filePath}`; | ||
| const state = this.stateStore.get(stateKey); | ||
| const fileOffset = state.lastOffset ?? 0; | ||
| const stepId = turnId ? `${turnId}:s${Math.max(1, fileOffset)}` : undefined; | ||
|
|
||
| // Round 1: 单 entry 形式, 包含 input.messages + output.messages + usage. | ||
| // The OTLP trace converter constructs a non-zero-duration LLM span from | ||
| // a single llm.response entry that carries both input.messages and | ||
| // output.messages. Round 2 will switch to paired llm.request + llm.response | ||
| // (requires BaseSessionInput multi-entry support, mirroring PR #101). | ||
| const combined: Record<string, unknown> = { | ||
| 'event.name': 'llm.response', | ||
| 'gen_ai.agent.type': ClientType.MiniMaxCode, | ||
| 'gen_ai.agent.name': 'MiniMax Code', | ||
| 'gen_ai.session.id': sessionId, | ||
| 'gen_ai.turn.id': turnId, | ||
| 'gen_ai.step.id': stepId, |
Comment on lines
+70
to
+76
| static getWatchPaths(): string[] { | ||
| return [resolveHome(DEFAULT_SESSION_DIR)]; | ||
| } | ||
|
|
||
| static async checkAvailability(): Promise<boolean> { | ||
| return directoryExists(resolveHome(DEFAULT_SESSION_DIR)); | ||
| } |
Comment on lines
+27
to
+42
| constructor(opts?: Partial<HookInputOptions> & { stateStore: HookInputOptions['stateStore'] }) { | ||
| super({ | ||
| stateStore: opts!.stateStore, | ||
| logDir: opts?.logDir ?? resolveHome('~/.loongsuite-pilot/logs/minimax-code'), | ||
| logPrefix: opts?.logPrefix ?? 'minimax-code', | ||
| pollIntervalMs: opts?.pollIntervalMs ?? 30_000, | ||
| }); | ||
| } | ||
|
|
||
| static async checkAvailability(): Promise<boolean> { | ||
| return directoryExists(resolveHome('~/.loongsuite-pilot/logs/minimax-code')); | ||
| } | ||
|
|
||
| static getWatchPaths(): string[] { | ||
| return [resolveHome('~/.loongsuite-pilot/logs/minimax-code')]; | ||
| } |
Comment on lines
+28
to
+33
| it('checkAvailability 在目录不存在时返回 false', async () => { | ||
| const elsewhere = path.join(TMPDIR, 'no-such-dir'); | ||
| const input = new MinimaxCodeLogInput({ stateStore, logDir: elsewhere }); | ||
| expect(await MinimaxCodeLogInput.checkAvailability()).toBe(false); | ||
| expect(input.agentType).toBe('minimax-code'); | ||
| }); |
Comment on lines
+191
to
+197
| it('checkAvailability 在目录不存在时返回 false', async () => { | ||
| const input = new MinimaxCodeRolloutInput({ | ||
| stateStore, | ||
| sessionDir: path.join(TMPDIR, 'no-such-dir'), | ||
| }); | ||
| expect(await MinimaxCodeRolloutInput.checkAvailability()).toBe(false); | ||
| }); |
Comment on lines
+283
to
+301
| // Stop 事件携带 tool.call.count 标记 + ['end_turn'|'interrupted'] finish_reason, | ||
| // 触发 Signal A 立即 flush。turnFlushDebounceMs(35s) 给 minimax-code-log input | ||
| // 和 minimax-code-rollout input 留出 dispatch 时间。 | ||
| // | ||
| // - toolCallCount > 0 (normal): emit ['end_turn'] | ||
| // - toolCallCount === 0 (interrupted): emit ['interrupted'] — MiniMax Code | ||
| // 被 SIGTERM/Ctrl+C 截断, 骨架 span 立即 flush, 不等 120s idle timeout。 | ||
| const toolCallCount = event.toolCallCount ?? 0; | ||
| const isInterrupted = toolCallCount === 0; | ||
| const record = { | ||
| ...baseFields(event, userId, runtimeConfig), | ||
| time_unix_nano: isoToUnixNanos(event.timestamp || event.ts), | ||
| 'event.id': generateEventId(), | ||
| 'event.name': 'other', | ||
| 'gen_ai.agent.event.name': 'stop', | ||
| 'gen_ai.agent.event.source': event.source || 'stop', | ||
| 'gen_ai.tool.call.count': toolCallCount, | ||
| 'gen_ai.response.finish_reasons': [isInterrupted ? 'interrupted' : 'end_turn'], | ||
| }; |
| | Qwen Code CLI | `qwen-code-cli` | Hook integration; parses qwen-code transcript JSONL on Stop. | | ||
| | Wukong | `wukong` | CLI API polling via local `wukong-cli`. | | ||
| | WorkBuddy | `workbuddy` | Structural Hook/file wakeups with a 30-second local transcript polling fallback. Verified on WorkBuddy Desktop 5.2.6 for macOS and 5.3.5.0 for Windows 11. | | ||
| | MiniMax Code | `minimax-code` | Hook integration for `SessionStart` / `UserPromptSubmit` / `PreToolUse` / `PostToolUse` / `Stop` lifecycle events, plus a sibling transcript tail for per-LLM `llm.request` / `llm.response` event pairs (hybrid collection per `agent-onboarding.md`). | |
Comment on lines
+27
to
+31
| * Reads ~/.minimax-code/rollout/model-io-sess_<sid>.jsonl (one file per | ||
| * session) and emits a llm.request + llm.response pair per record, with full | ||
| * LLM payloads (request messages/tools/system + response text/toolCalls/usage) | ||
| * and proper start/end timestamps (startedAt / completedAt). | ||
| * |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:169
- processSessionLine 读取了 startedAt/completedAt 但没有把它们用于 entry 的 timestamp/time_unix_nano。buildAgentActivityEntry 会在缺省时用 Date.now()，这会把历史 rollout 事件的时间戳“漂移”到采集时刻，影响 span 时间轴/排序。建议为 combined 设置 timestamp（优先 completedAt，其次 startedAt）。
// Round 1: 单 entry 形式, 包含 input.messages + output.messages + usage.
// The OTLP trace converter constructs a non-zero-duration LLM span from
// a single llm.response entry that carries both input.messages and
// output.messages. Round 2 will switch to paired llm.request + llm.response
// (requires BaseSessionInput multi-entry support, mirroring PR #101).
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:31
- 类注释前半部分仍描述“每条 record emit llm.request + llm.response pair / startedAt+completedAt 配对”，但 Round 1 实现实际只 emit 单个 llm.response（且当前未用 startedAt/completedAt）。建议把注释改成与 Round 1 行为一致，避免误导维护者/评审。
* Reads ~/.minimax-code/rollout/model-io-sess_<sid>.jsonl (one file per
* session) and emits a llm.request + llm.response pair per record, with full
* LLM payloads (request messages/tools/system + response text/toolCalls/usage)
* and proper start/end timestamps (startedAt / completedAt).
*
docs/agents.md:29
- 文档这里宣称 transcript tail 会提供 per-LLM 的 llm.request/llm.response 事件配对，但本 PR Round 1 的 rollout input 仍是单条 llm.response（PR 描述也明确 defer pairing 到 Round 2）。建议把说明改成与当前实现一致，避免用户按文档预期排查却看不到 llm.request。
| MiniMax Code | `minimax-code` | Hook integration for `SessionStart` / `UserPromptSubmit` / `PreToolUse` / `PostToolUse` / `Stop` lifecycle events, plus a sibling transcript tail for per-LLM `llm.request` / `llm.response` event pairs (hybrid collection per `agent-onboarding.md`). |
tests/unit/inputs/minimax-code-log-input.test.ts:32
- 这个用例构造了不存在的 logDir（elsewhere），但断言调用的是静态 MinimaxCodeLogInput.checkAvailability()，它检查的是默认路径（~/.loongsuite-pilot/logs/minimax-code），与 elsewhere 无关；在开发机上如果默认目录存在，这个测试会变得不稳定且语义不正确。建议改为不依赖真实 HOME 的断言（例如只断言返回值类型），或用 vitest mock/stub directoryExists。
it('checkAvailability 在目录不存在时返回 false', async () => {
const elsewhere = path.join(TMPDIR, 'no-such-dir');
const input = new MinimaxCodeLogInput({ stateStore, logDir: elsewhere });
expect(await MinimaxCodeLogInput.checkAvailability()).toBe(false);
expect(input.agentType).toBe('minimax-code');
tests/unit/inputs/minimax-code-rollout-input.test.ts:195
- 这个用例传入了自定义 sessionDir，但断言调用的是静态 MinimaxCodeRolloutInput.checkAvailability()，它检查的是默认 ~/.minimax-code/rollout，与传入的 TMPDIR 无关；在本机如果默认目录存在会导致测试不稳定。建议改为不依赖真实 HOME 的断言（例如只断言返回值类型），或在导入前 mock fs-utils.directoryExists。
it('checkAvailability 在目录不存在时返回 false', async () => {
const input = new MinimaxCodeRolloutInput({
stateStore,
sessionDir: path.join(TMPDIR, 'no-such-dir'),
});
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:163
- step.id 目前用 stateStore.lastOffset（文件字节偏移）派生，但 BaseSessionInput 在调用 processSessionLine 之前就把 offset 一次性推进到本轮读取的末尾，导致同一次 collect() 内多个 record 得到相同的 gen_ai.step.id，进而让下游按 step.id 聚合/排序时发生碰撞。建议改为用 record 自身的 startedAt/completedAt（或 responseId）派生一个每条 record 唯一的 step.id。
This issue also appears on line 165 of the same file.
const stateKey = `${this.id}:${filePath}`;
const state = this.stateStore.get(stateKey);
const fileOffset = state.lastOffset ?? 0;
const stepId = turnId ? `${turnId}:s${Math.max(1, fileOffset)}` : undefined;
docs/zh-CN/agents.md:29
- 这里描述 transcript 尾随会提供 per-LLM 的 llm.request/llm.response 配对，但当前 Round 1 rollout input 只 emit 单条 llm.response（PR 描述也写明配对在 Round 2）。建议把文案改成与现实现一致，避免误导。
| MiniMax Code | `minimax-code` | Hook 集成支持 `SessionStart` / `UserPromptSubmit` / `PreToolUse` / `PostToolUse` / `Stop` 生命周期事件，并配合 transcript 尾随提供 per-LLM `llm.request` / `llm.response` 事件配对（参见 `agent-onboarding.md` 的 hybrid collection 模式）。 |
Comment on lines
+308
to
+318
| const subcommand = process.argv[2] || ''; | ||
| switch (subcommand) { | ||
| case 'session-start': cmdSessionStart(); break; | ||
| case 'user-prompt-submit': cmdUserPromptSubmit(); break; | ||
| case 'pre-tool-use': cmdPreToolUse(); break; | ||
| case 'post-tool-use': cmdPostToolUse(); break; | ||
| case 'stop': cmdStop(); break; | ||
| default: | ||
| // Unknown subcommand: fail-open, emit nothing. | ||
| break; | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (8)
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:259
- Even after capturing startedAt/completedAt, the constructed entry never sets time_unix_nano from them. For a rollout-backed llm.response, time_unix_nano should come from completedAt (fallback startedAt) so traces reflect the real model latency instead of collection time.
const combined: Record<string, unknown> = {
'event.name': 'llm.response',
'gen_ai.agent.type': ClientType.MiniMaxCode,
'gen_ai.agent.name': 'MiniMax Code',
'gen_ai.session.id': sessionId,
tests/unit/inputs/minimax-code-log-input.test.ts:32
- This test sets up an "elsewhere" logDir, but then asserts the static MinimaxCodeLogInput.checkAvailability(), which always checks the default ~ path. That makes the test depend on the developer's real home directory (it will fail if ~/.loongsuite-pilot/logs/minimax-code exists).
it('checkAvailability 在目录不存在时返回 false', async () => {
const elsewhere = path.join(TMPDIR, 'no-such-dir');
const input = new MinimaxCodeLogInput({ stateStore, logDir: elsewhere });
expect(await MinimaxCodeLogInput.checkAvailability()).toBe(false);
expect(input.agentType).toBe('minimax-code');
docs/agents.md:29
- The MiniMax Code row claims the transcript tail provides per-LLM llm.request/llm.response event pairs, but the current Round 1 rollout input emits a single combined llm.response entry (with both input.messages and output.messages). The docs should reflect the current behavior to avoid confusion.
| MiniMax Code | `minimax-code` | Hook integration for `SessionStart` / `UserPromptSubmit` / `PreToolUse` / `PostToolUse` / `Stop` lifecycle events, plus a sibling transcript tail for per-LLM `llm.request` / `llm.response` event pairs (hybrid collection per `agent-onboarding.md`). |
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:50
- The docstring says this input emits an llm.request + llm.response pair per rollout record, but the current implementation emits a single combined llm.response entry (see processSessionLine). Keeping the comment aligned avoids confusion about the intended semantics.
* Reads ~/.minimax-code/rollout/model-io-sess_<sid>.jsonl (one file per
* session) and emits a llm.request + llm.response pair per record, with full
* LLM payloads (request messages/tools/system + response text/toolCalls/usage)
* and proper start/end timestamps (startedAt / completedAt).
*
tests/unit/inputs/minimax-code-rollout-input.test.ts:199
- This test constructs an input with sessionDir=TMPDIR/no-such-dir, but then asserts the static MinimaxCodeRolloutInput.checkAvailability(), which checks the real ~/.minimax-code/rollout. That makes the test non-hermetic and it can fail on machines that actually have MiniMax Code installed.
it('checkAvailability 在目录不存在时返回 false', async () => {
const input = new MinimaxCodeRolloutInput({
stateStore,
sessionDir: path.join(TMPDIR, 'no-such-dir'),
});
docs/zh-CN/agents.md:29
- 此处描述为 transcript 尾随提供 per-LLM
llm.request/llm.response事件配对，但当前 Round 1 的 rollout input 实际只发单条合并的llm.response（同时携带 input.messages + output.messages）。文档建议与当前实现保持一致，避免误导后续维护/排障。
| MiniMax Code | `minimax-code` | Hook 集成支持 `SessionStart` / `UserPromptSubmit` / `PreToolUse` / `PostToolUse` / `Stop` 生命周期事件，并配合 transcript 尾随提供 per-LLM `llm.request` / `llm.response` 事件配对（参见 `agent-onboarding.md` 的 hybrid collection 模式）。 |
agents.d/minimax-code.json:28
- This agent definition sets hookContainerPath/extraSettings, but HookStrategy currently hard-codes hookJsonPath as ['hooks', event] and does not read hookContainerPath/extraSettings. These fields are therefore ignored, and the hook may be installed into the wrong JSON path if MiniMax Code expects hooks.events..
"hookCommand": "$PILOT_DATA/hooks/minimax-code-loongsuite-pilot-hook.sh",
"format": "nested",
"eventSubcommand": "kebab-case",
"hookContainerPath": ["hooks", "events"],
"extraSettings": {
assets/hooks/minimax-code-hook-processor.mjs:32
- The header comment says the Stop hook record does not carry a terminal finish_reason, but cmdStop currently writes gen_ai.response.finish_reasons (end_turn/interrupted). The comment should match the actual emitted fields so readers don't miss the flush-trigger behavior.
* - Stop 事件只发 "other" 标记 turn 元数据(agent.event.name=stop, tool.call.count),
* 不带 terminal finish_reason —— terminal signal 由 rollout input 的最后一条
* llm.response(finish_reason=stop)提供, turnIdleTimeoutMs(120s) 作为兜底。
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:245
requestIdis only read from the nestedrequestobject, but rollout records may also provide it at the top level (as exercised by these tests). Not readingrecord.requestIdcan cause retries to get differentgen_ai.request.id/gen_ai.step.idifstartedAtchanges between attempts.
const requestId = (request['requestId'] as string | undefined)
?? (request['request_id'] as string | undefined)
?? `${sessionId}:${turnId ?? 'unknown'}:req:${String(startedAt ?? '')}`;
tests/unit/inputs/minimax-code-rollout-input.test.ts:295
processSessionLine()now returnsAgentActivityEntry[], but this test treats the returned array as a single entry (e!['gen_ai.step.id']). This assertion will always beundefinedregardless of actual behavior and doesn't validate that both emitted entries omitgen_ai.step.idwhenturnIdis missing.
};
const e = await (input as any).processSessionLine(rec, '/tmp/x.jsonl');
expect(e!['gen_ai.step.id']).toBeUndefined();
});
assets/hooks/minimax-code-hook-processor.mjs:32
- Docstring says the Stop hook event "不带 terminal finish_reason", but
cmdStop()currently emits'gen_ai.response.finish_reasons'(on anevent.name: 'other'record) to trigger flush behavior. This mismatch can mislead future changes around terminal signaling.
* - Stop 事件只发 "other" 标记 turn 元数据(agent.event.name=stop, tool.call.count),
* 不带 terminal finish_reason —— terminal signal 由 rollout input 的最后一条
* llm.response(finish_reason=stop)提供, turnIdleTimeoutMs(120s) 作为兜底。
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:72
- The comment block still claims "Round 3 deferred" for multi-entry emission, but this input already emits an
llm.request+llm.responsepair andBaseSessionInput.processSessionLine()has been updated accordingly in this PR. Keeping this text will confuse readers and also conflicts with the PR description’s rollout plan.
* Round 3 deferred (见 PR description "Future Work"):
* - `BaseSessionInput.processSessionLine` 改 multi-entry return, 让 rollout
* emit llm.request + llm.response pair (当前 emit 单 entry, 含 input
* + output messages)。需要 base class 改动, 影响所有 SessionInput 子类。
* - synthesizeOrphanToolRecords flusher 增强, 视真实 E2E 数据决定。
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:242
responseIdis only read from the nestedresponseobject. Elsewhere in this PR (tests and docs)requestId/responseIdappear as possible top-level fields on the rollout record; ignoringrecord.responseIdcan break pairing stability (e.g. retries where timestamps differ but ids stay constant).
This issue also appears on line 243 of the same file.
const responseId = (response['responseId'] as string | undefined)
?? (response['response_id'] as string | undefined)
?? `${sessionId}:${turnId ?? 'unknown'}:${String(startedAt ?? '')}`;
Comment on lines
+94
to
101
| // Round 3 (PR #233): processSessionLine now returns an array so that | ||
| // a single source record can emit multiple normalized entries | ||
| // (e.g. paired llm.request + llm.response for rollout transcripts). | ||
| // Subclasses that historically returned `null` to skip now return | ||
| // `[]`; subclasses that returned a single entry return `[entry]`. | ||
| const lineEntries = await this.processSessionLine(parsed, filePath); | ||
| entries.push(...lineEntries); | ||
| } catch (err) { |
ralf0131
approved these changes
Aug 10, 2026
ralf0131
left a comment
ralf0131
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Adds MiniMax Code probe support to LoongSuite Pilot with a dual-input architecture (hook JSONL + rollout transcript), following the established ZCode pattern from PR #101. The PR is well-structured with thorough test coverage (20+ test cases for rollout, 5 for log input), proper edge case handling (file rotation, interrupted responses, traceId normalization), and complete documentation updates in both EN and ZH.
The base class interface change (processSessionLine → AgentActivityEntry[]) is a clean migration — all existing subclasses are updated in the same PR.
Overall: LGTM. Minor informational suggestions inline.
Automated review by github-manager-bot
| } as MinimaxCodeRolloutFileState, | ||
| }, | ||
| }); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Info] The collect() override front-runs BaseSessionInput.processFile to handle inode rotation. Consider adding a brief comment at the top of collect() explaining why the pre-pass is needed (i.e., processFile is private and only clears extra.inode, not extra.minimaxCodeRollout.turnStepMap). This will help future maintainers understand the coupling. The existing JSDoc on the method does explain it, but a one-liner at the method body level would make it even clearer.
| (record['traceId'] as string | undefined) ?? (record['trace_id'] as string | undefined), | ||
| ); | ||
| const responseId = (response['responseId'] as string | undefined) | ||
| ?? (response['response_id'] as string | undefined) |
Collaborator
There was a problem hiding this comment.
[Info] The extractSessionIdFromFilePath fallback is a good defensive pattern. One minor suggestion: consider caching the regex match result per filePath (e.g. in a Map<string, string>) since processSessionLine is called per-line and the file path doesn't change within a single file. Not a performance concern for typical session files, but it's a cheap optimization if the pattern grows.
| import os from 'node:os'; | ||
| import crypto from 'node:crypto'; | ||
|
|
||
| import { readStdinJson } from './shared/stdin-reader.mjs'; |
Collaborator
There was a problem hiding this comment.
[Info] The field name compatibility comment mentions "待 MiniMax Code 团队最终确认命名". Once the MiniMax Code team finalizes the hook payload schema, consider adding a // TODO(minimax-code): remove snake_case fallback after SDK v1.0" marker so the dual-field lookups don't linger indefinitely.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (5)
src/inputs/minimax-code-log/minimax-code-log-input.ts:33
- The constructor makes
optsoptional but then dereferencesopts!.stateStore. This allowsnew MinimaxCodeLogInput()to compile but throws at runtime. SincestateStoreis required, the parameter should not be optional.
constructor(opts?: Partial<HookInputOptions> & { stateStore: HookInputOptions['stateStore'] }) {
super({
stateStore: opts!.stateStore,
logDir: opts?.logDir ?? resolveHome('~/.loongsuite-pilot/logs/minimax-code'),
logPrefix: opts?.logPrefix ?? 'minimax-code',
pollIntervalMs: opts?.pollIntervalMs ?? 30_000,
});
tests/unit/inputs/minimax-code-rollout-input.test.ts:302
processSessionLinenow returns an array, but this test is still asserting on the array object (e['gen_ai.step.id']) instead of its entries. That assertion will always beundefinedregardless of implementation and won’t catch regressions in step.id allocation.
it('Round 2: 缺 turnId 时不分配 step.id (validator 诊断)', async () => {
const input = new MinimaxCodeRolloutInput({ stateStore, sessionDir: TMPDIR });
const rec: any = {
type: 'model-io', sessionId: 's1', requestId: 'req-1',
request: { messages: [] }, response: { modelId: 'm1' },
};
const e = await (input as any).processSessionLine(rec, '/tmp/x.jsonl');
expect(e!['gen_ai.step.id']).toBeUndefined();
});
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:193
discoverSessionFilestreatsfilePatternas always having a*wildcard and requires at least one wildcard character vianame.length > prefix.length + suffix.length. If a caller passes an exact filename (no*), this function will never match it, even thoughfilePatternis an exposed option.
// Simple glob: prefix + suffix. Node's fs.glob landed in v22 (2024); we
// use a manual match to keep the input compatible with Node >= 18.
const prefix = this.filePattern.split('*')[0] ?? '';
const suffix = this.filePattern.split('*')[1] ?? '';
for (const name of entries) {
if (name.startsWith(prefix) && name.endsWith(suffix) && name.length > prefix.length + suffix.length) {
files.push(path.join(this.sessionDir, name));
tests/unit/inputs/minimax-code-log-input.test.ts:16
afterEachremovesTMPDIR, but subsequent tests reuse the sameTMPDIRconstant without recreating it. This makes the suite order-dependent and can break as soon as any test relies on the directory existing (e.g., ifBaseHookInputstarts validatinglogDir). Recreate the directory inbeforeEach(or switch to per-test tmp dirs).
beforeEach(async () => {
stateStore = new StateStore(path.join(TMPDIR, 'state.json'));
await stateStore.load();
});
afterEach(() => {
fs.rmSync(TMPDIR, { recursive: true, force: true });
});
src/core/orchestrator.ts:1193
- The comment says the rollout input emits only per-LLM
llm.responseentries, butMinimaxCodeRolloutInput.processSessionLinenow emits a paired[llm.request, llm.response]array. Keeping this comment in sync matters for future maintainers debugging event duplication/pairing.
// --- MiniMax Code Rollout (transcript/rollout JSONL tail) ---
// Reads ~/.minimax-code/rollout/model-io-sess_<sid>.jsonl, emits
// per-LLM llm.response entries with gen_ai.input.messages /
// gen_ai.output.messages / gen_ai.usage.* / gen_ai.tool.definitions.
// Sibling input to MinimaxCodeLogInput (hybrid collection per
Author
|
Round 5 added in 713c1f4 — addresses 3 real issues left from the prior review rounds:
Also fixed a class-header comment that mis-attributed the paired llm.request/llm.response emission to Round 4 instead of Round 3. Diff: +74 / -17 across 5 files. typecheck clean, 2646 tests pass (1 pre-existing shell-timing flake), build clean. cc @ralf0131 — would appreciate a re-look at the new commit when you get a chance, especially the flusher change since it touches upstream behavior beyond the agent (benefits any agent that emits @copilot-pull-request-reviewer — heads up that the Round 4 review comments on |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:467
buildOutputMessagesreturnsundefinedwhen bothresponse.textandresponse.toolCallsare empty. That makes thellm.responseentry omitgen_ai.output.messages, which will failscripts/validate-trace.mjs'ssemantic.llm_has_input_outputcheck (LLM spans must have output.messages). Consider always emitting an assistant message with an empty text part when the finish reason is known but the payload is otherwise empty.
if (parts.length === 0) return undefined;
return [{ role: 'assistant', parts, finish_reason: finish } as unknown as JsonValue];
}
assets/hooks/minimax-code-hook-processor.mjs:304
cmdStopinfersinterruptedsolely fromtoolCallCount === 0. A normal turn/session can legitimately have zero tool calls (pure chat), so this would incorrectly label successful sessions as interrupted and change flush behavior. Prefer defaulting toend_turnand only emittinginterruptedwhen the hook payload provides an explicit interruption/cancellation signal.
// - toolCallCount > 0 (normal): emit ['end_turn']
// - toolCallCount === 0 (interrupted): emit ['interrupted'] — MiniMax Code
// 被 SIGTERM/Ctrl+C 截断, 骨架 span 立即 flush, 不等 120s idle timeout。
const toolCallCount = event.toolCallCount ?? 0;
const isInterrupted = toolCallCount === 0;
const record = {
...baseFields(event, userId, runtimeConfig),
time_unix_nano: isoToUnixNanos(event.timestamp || event.ts),
'event.id': generateEventId(),
'event.name': 'other',
'gen_ai.agent.event.name': 'stop',
'gen_ai.agent.event.source': event.source || 'stop',
'gen_ai.tool.call.count': toolCallCount,
'gen_ai.response.finish_reasons': [isInterrupted ? 'interrupted' : 'end_turn'],
};
ralf0131
approved these changes
Aug 10, 2026
ralf0131
left a comment
ralf0131
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Re-reviewed Round 4 + Round 5 changes since the initial approval. All changes look good:
- otlp-trace-flusher.ts: Adding
interruptedtoTERMINAL_FINISH_REASONSis a clean, additive fix. Well-documented comment explaining the rationale. This also benefits other agents that emitinterruptedfinish reasons. - hook-manager.ts: Conditional
matcheromission (skip when undefined) is a good defensive change — avoids potential rejection by hosts sensitive tomatcher: *. - Hook processor try/finally + stdout
{}: Correct fix — ensures the host command-hook protocol never blocks on empty stdout, mirroring the Claude Code pattern. - Test fixes: Stale single-entry assertions properly updated for the Round 3 multi-entry refactor.
Overall this is a well-structured PR that follows the established ZCode pattern closely. The multi-round iterative approach with clear commit boundaries makes it easy to follow the evolution.
LGTM ✅
Automated review by github-manager-bot
Author
|
Round 6 added in fc8fd5e — addresses 2 new copilot suppressed comments surfaced after Round 5:
Diff: +299 / -22 across 4 files. typecheck clean, 2656 tests pass (was 2646, +10 new; pre-existing flake did not re-trigger), build clean. cc @ralf0131 — Round 6 ready for re-look. The Round 5 |
ralf0131
approved these changes
Aug 10, 2026
ralf0131
left a comment
ralf0131
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Adds MiniMax Code probe support (hook + rollout dual input) following the established ZCode (PR #101) pattern. Well-structured multi-round rollout with comprehensive test coverage.
Key changes reviewed:
- Agent definition + hook processor (fail-open, dual field-name compat)
- Rollout input with turnStepMap persistence, inode-aware file rotation, interrupted-path injection
- Base class API evolution (
processSessionLine→ array return) with all subclasses updated - Hook strategy enhancements (
hookContainerPath,extraSettingsdeep-merge) — non-blocking failure semantics - Terminal finish reason
interruptedadded for Signal-A immediate flush
Note for maintainers: The BaseSessionInput.processSessionLine return type change (AgentActivityEntry | null → AgentActivityEntry[]) is the highest-risk change in this PR. All existing subclasses (hermes, qoder-cli-session, qoder-work-log, qoder-work-trace) are updated consistently. TypeScript compilation guarantees no missed subclasses.
LGTM — code is clean, well-tested, and follows established patterns.
Automated review by github-manager-bot
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
assets/hooks/minimax-code-hook-processor.mjs:311
- The processor claims to support both camelCase and snake_case payload fields, but
toolCallCountonly readsevent.toolCallCountand only when it’s a number. If MiniMax Code emitstool_call_count(or a numeric string), the count will silently fall back to0, producing incorrectgen_ai.tool.call.counttelemetry. Consider acceptingevent.tool_call_countand coercing numeric strings to numbers (similar to how other parts of the code normalize/parse values).
const toolCallCount = typeof event.toolCallCount === 'number' ? event.toolCallCount : 0;
const interruptedSignal = event.interrupted
?? event.isInterrupted
?? event.is_interrupted;
const cancelledSignal = event.cancelled
?? event.isCancelled
?? event.is_cancelled;
assets/hooks/minimax-code-hook-processor.mjs:3
- The copyright header attributes this file to “MiniMax”, but it appears to be repository-authored integration code (hook processor for LoongSuite Pilot). If that attribution isn’t legally accurate, it can create licensing/compliance confusion. Prefer using the project’s standard header (or omit the copyright line and keep SPDX consistent with the repo) unless this file is actually copied from MiniMax with permission.
#!/usr/bin/env node
// Copyright 2026 MiniMax
// SPDX-License-Identifier: Apache-2.0
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:532
- The comment says nested OpenAI
functionwrapper support will be added later, butnormalizeToolDef()already flattensrec.functionintoflat(handling the nested shape). Updating the comment to match the current behavior will reduce confusion for future maintainers.
private extractToolDefinitions(request: Record<string, unknown>): JsonValue | undefined {
// Multi-path lookup accommodates both flat ({name, description,
// input_schema}) and OpenAI nested ({type:'function', function:{name, ...}})
// shapes. v1 rollout records are likely flat; once we observe nested
// forms we'll add `unwindFunctionWrapper` here.
const candidates: unknown[] = [
Comment on lines
+18
to
+20
| afterEach(() => { | ||
| fs.rmSync(TMPDIR, { recursive: true, force: true }); | ||
| }); |
Collaborator
|
Thanks for the detailed update @zy84338719! The Round 5 and Round 6 changes look solid — particularly the This PR has already been reviewed and approved on the current HEAD ( However, there is a merge conflict with the git fetch origin
git checkout feat/add-minimax-code-agent
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseNo need for another review round after rebasing unless the conflict resolution introduces non-trivial changes — feel free to @mention me if that happens. Automated notification by github-manager-bot |
zy84338719
force-pushed
the
feat/add-minimax-code-agent
branch
from
August 11, 2026 04:55
fc8fd5e to
b0f8599
Compare
Author
|
Round 7: rebase onto latest main ( Rebased the 7 commits onto the latest main (6 new commits since Round 6 push, including 2 矩阵 syncs + 4 installer/updater changes). Conflict resolution: 2 files (
cc @ralf0131 — rebase done. Per your earlier message, no need for another review round unless the conflict resolution introduces non-trivial changes (it does not; pure additive doc-table row insertion). Approval of |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:494
timestampToUnixNanos()falls back toDate.now()when passedundefined, sotimestampToUnixNanos(completedAt) ?? timestampToUnixNanos(startedAt) ?? '0'will always use the first call even whencompletedAtis missing. That makesllm.response.time_unix_nanoincorrect (current time instead ofstartedAtor a sentinel), which can break span duration semantics and ordering.
Use an explicit conditional to fall back to startedAt, then '0'.
time_unix_nano: timestampToUnixNanos(completedAt) ?? timestampToUnixNanos(startedAt) ?? '0',
| const requestRecord: Record<string, unknown> = { | ||
| ...sharedFields, | ||
| 'event.name': 'llm.request', | ||
| time_unix_nano: timestampToUnixNanos(startedAt) ?? '0', |
Author
Round 34 rebase onto upstream main (793bbbf)Rebased 22 commits onto upstream
Conflict resolution: 0 conflicts — same pattern as Rounds 25/27/28/29/30/31/32/33. All 3 upstream commits touch dsh/installer/windows paths, no overlap with MiniMax Code files. Git auto-merged cleanly. Stats: 22 commits, 41 files, +3656 / -43 (identical to Rounds 28-33; rebase did not change MiniMax Code content, just regenerated the commit hash for Round 23). Validation:
CI: re-approve trigger required after force-push (same workflow as Rounds 25-33). |
Comment on lines
+311
to
+320
| if (!prevRollout || !prevInodeValid || rotated) { | ||
| this.stateStore.update(stateKey, { | ||
| extra: { | ||
| minimaxCodeRollout: { | ||
| inode: currentIno, | ||
| turnStepMap: rotated ? {} : (prevRollout?.turnStepMap ?? {}), | ||
| } as MinimaxCodeRolloutFileState, | ||
| }, | ||
| }); | ||
| } |
- agents.d/minimax-code.json: hook protocol definition (5 events)
- assets/hooks/minimax-code-*.{sh,mjs}: node dispatcher + 5 subcommand handlers
- src/inputs/minimax-code-log/: hook JSONL tail input (BaseHookInput)
- src/inputs/minimax-code-rollout/: transcript/rollout JSONL tail input
(BaseSessionInput), with ARMS GenAI messages/tool_definitions
normalization + W3C traceId + finish_reason default
- src/types/client-type.ts: add MiniMaxCode = 'minimax-code'
- src/core/orchestrator.ts: register both inputs + LISTENER_AGENT_MAP entries
- tests/unit/inputs/minimax-code-*.test.ts: 16 unit tests (all green)
- tests/unit/hooks/minimax-code/fixtures/*: synthetic JSONL fixtures
- docs/agents.md: add MiniMax Code row in Supported Agent IDs
Follows PR alibaba#101 (ZCode) dual-input pattern. Round 1 scaffold; future
rounds cover paired llm.request/llm.response emission, persistent
turnStepMap, interrupted path injection, and orphan synthesis per the
PR description Future Work section.
…nslations - docs/overview.md: add MiniMax Code row in Supported Agents table - docs/zh-CN/agents.md: add MiniMax Code row in supported Agent IDs - docs/zh-CN/overview.md: add MiniMax Code row in supported agents table Mirrors the English-language addition in the previous commit; ensures Chinese documentation tracks the same agent list. (The pre-existing zh-CN files were already missing several English-only agents such as mimo-code and qwen-code-cli; out of scope for this PR.)
…njection Round 1 deferred items resolved: - turnStepMap persistence: per-turn step counter + requestId de-dup, persisted to stateStore.extra.minimaxCodeRollout.turnStepMap, survives restarts. File rotation (inode change) clears the map. inode=0 sentinel (file appeared after onStart) seeds inode without clearing the map (CP5 fix pattern from PR alibaba#101 zcode-rollout). - Interrupted path injection: detect completedAt present + no finishReason / text / toolCalls (typical SIGTERM/timeout pattern). Inject gen_ai.response.finish_reasons=['interrupted'] + placeholder gen_ai.output.messages + 0 usage. Prevents validate-trace from flagging LLM span as ERROR for missing output.messages/finish_reasons/usage. - agent-system-map.ts: add 'minimax-code' → 'minimax-code' mapping. Without this, resolveAgentSystem() falls back to 'unknown' and the OTLP trace flusher sets gen_ai.agent.system='unknown', causing ARMS GenAI pipeline routing failures. - validate-trace.mjs: extend VALID_FINISH_REASONS with 'interrupted' and 'cancelled' (mirrors PR alibaba#101 Step C P0 fix). Required for the new interrupted-path events to pass JSONL strict validation. Test coverage: +12 unit tests covering turnStepMap (retry dedup, same-turn stepId increment, cross-turn reset, missing turnId, cross-instance), file rotation (inode change clearing, inode=0 sentinel), and interrupted injection (positive + negative + completedAt/text edge cases).
…entries Round 2 deferred item resolved (matches PR alibaba#101 zcode-rollout shape): - BaseSessionInput.processSessionLine: change return type from `Promise<AgentActivityEntry | null>` to `Promise<AgentActivityEntry[]>`. processFile now spreads the array, so a single source record can yield multiple normalized entries. The OTLP trace converter's pairLlm uses gen_ai.response.id to pair the two events into a single STEP span. - 4 other BaseSessionInput subclasses updated to new return type: qoder-cli-session, qoder-work-trace, qoder-work-log, hermes-log. Subclasses that returned null now return []; subclasses that returned a single entry now return [entry]. - MinimaxCodeRolloutInput: processSessionLine now emits paired llm.request + llm.response entries (shared trace_id, session/turn/step, agent.type, response.id, request.id). The request entry carries gen_ai.input.messages + gen_ai.tool.definitions + time=startedAt; the response entry carries gen_ai.output.messages + gen_ai.usage.* + gen_ai.response.finish_reasons + time=completedAt. Emitting them as two separate entries produces a cleaner span tree (matches zcode PR alibaba#101 shape) and aligns with how the OTLP trace converter's pairLlm uses gen_ai.response.id to pair request/response events. - Test fixtures updated to expect the new array shape (entries[0] for request, entries[1] for response). Test coverage expanded to assert: * trace_id parity between request and response entries * request.id + response.id parity (OTLP pair key) * time_unix_nano ordering (request <= response) - base-session-input.test, extensibility.test, qoder-cli-session-input.test adapted to new return type. Test count: 2647 pass / 0 fail (1 pre-existing flake in plugin-probe-strategy.test unrelated to this change).
…lf0131) Reviewer state: ralf0131 (maintainer) APPROVED with 4 informational copilot review rounds. This commit addresses the remaining real issues identified in those reviews (suppressed comments that weren't auto-resolved by the Round 1-3 commits). deployment / hook-strategy (copilot review on agents.d/minimax-code.json:28): - types/deployment.ts: extend AgentHookConfig with hookContainerPath, extraSettings, hookType, hookTypeRationale fields. Mirrors PR alibaba#101 zcode AgentHookConfig extension. - deployment/hook-strategy.ts: buildHookDefinitions / buildRetiredHookDefinitions now honor `hookContainerPath` (default ['hooks']) so agents with non-standard config schemas (ZCode, MiniMax Code nest event arrays under settings.hooks.events.<event>) get hooks written to the correct JSON path. Add applyExtraSettings() for deep-merging sibling fields (e.g. settings.hooks.enabled=true) into the agent's config file. The previous Round 1 agents.d/minimax-code.json declaration of hookContainerPath/extraSettings was effectively ignored — this commit makes those fields actually drive the deployment. - hooks/hook-manager.ts: nested-format hook entry omits the matcher field when not explicitly set (mirrors the flat-format behavior already in place at line 144). Some agents (MiniMax Code) reject explicit matcher:'*' — omitting the field is safer. rollout input (copilot review on minimax-code-rollout-input.ts:242-243): - requestId / responseId now read `record.requestId` / `record.responseId` at the top level first (canonical location per MiniMax Code rollout schema, stable across retries — same id even if startedAt shifts), then fall back to the nested request.response.{requestId,response_id}, then to a synthetic string. Mirrors PR alibaba#101 zcode-rollout-input behavior; keeps OTLP pair key stable across retry records. comments (copilot review on minimax-code-rollout-input.ts:72 and minimax-code-hook-processor.mjs:32): - Updated 'Round 3 deferred' block in rollout input to describe the actual current behavior (paired entries via Round 3 multi-entry refactor; only synthesizeOrphanToolRecords remains as future work). - Updated Stop handler comment in hook processor to accurately describe that `gen_ai.response.finish_reasons` IS emitted (carries ['end_turn'|'interrupted'] to trigger Signal A flush). test stability (copilot review on log-input.test.ts:32 and rollout-input.test.ts:199): - checkAvailability tests now mock directoryExists via vi.spyOn to avoid depending on whether ~/.loongsuite-pilot/logs/minimax-code or ~/.minimax-code/rollout exists on the developer's machine. Hermetic and stable across dev environments. No production code logic changes beyond the deployment hook-strategy upgrade (which was already declared in Round 1's agents.d/*.json but had no plumbing to honor it).
…pecheck fixes Round 5 polish addressing the remaining open threads from copilot + ralf0131 review rounds 1-4 (ralf0131 LGTM at c82ad35; remaining items were all minor and not re-request-blocking, but worth landing before merge to keep the diff clean). - src/flushers/otlp-trace-flusher.ts: add "interrupted" to TERMINAL_FINISH_REASONS. Hook processor (cmdStop) emits gen_ai.response.finish_reasons=["interrupted"] on SIGTERM/Ctrl+C paths to trigger Signal-A immediate flush, but the flusher only recognized ["stop","end_turn","cancelled","error"]. Without this, an interrupted turn waited for turnIdleTimeoutMs (default 30s) before flushing, with stale timestamps on spans already in the buffer. validate-trace.mjs already accepted "interrupted" as a valid finish_reason, so this is purely additive and brings the flusher in line with the OTLP GenAI vocabulary. "cancelled" was already present; "interrupted" was the missing companion. - assets/hooks/minimax-code-hook-processor.mjs: restructure dispatch to a DISPATCH table wrapped in try/catch/finally, with process.stdout.write("{}\n") in the finally block (and on unknown subcommand). The hook host reads JSON from stdout to apply hook policy decisions; an empty stdout caused intermittent hangs on some agent hosts. Mirrors assets/hooks/claude-code-hook-processor.mjs pattern. - tests/unit/inputs/edge-cases.test.ts: update EdgeSessionInput.processSessionLine override to return Promise<AgentActivityEntry[]> (matches Round 3 BaseSessionInput multi-entry refactor). Was still returning Promise<AgentActivityEntry | null> from before the refactor; not caught at runtime because that test only exercises the empty-file path, but is a typecheck hazard if tests are type-checked in CI. - tests/unit/inputs/minimax-code-rollout-input.test.ts: fix "Round 2: 缺 turnId 时不分配 step.id" test which still asserted the old single-entry shape (e!["gen_ai.step.id"]). processSessionLine now returns AgentActivityEntry[] (Round 3), so the assertion should target the response entry at index 1. - src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts: fix the class-header comment that mis-attributed the multi-entry pair emission to "Round 4" (it was actually Round 3, per git log). Add a Round 5 entry describing the flusher + hook stdout fixes for future maintainers. No new tests added for "interrupted" being terminal — the existing hasTerminalFinishReason is module-internal, the Set membership change is purely additive (no existing test relies on the exact set contents), and validate-trace.mjs already validates "interrupted" as a legal finish_reason, so the upstream contract is consistent. Validation: - npm run typecheck: 0 errors - npm test: 2646 passed, 0 new failures (1 pre-existing flake in local-worker-activation-service.test.ts — unrelated shell timing race, same as Round 4) - npm run build: clean - node -c assets/hooks/minimax-code-hook-processor.mjs: syntax OK
…errupted signal Round 6 addresses 2 new copilot suppressed comments on Round 5 (713c1f4): 1. buildOutputMessages: previously returned undefined when both text and toolCalls were empty. This caused llm.response entries to omit gen_ai.output.messages, which scripts/validate-trace.mjs semantic.llm_has_input_output MUST rule flagged as ERROR. Common cases affected: model refusals (no text), length-cap terminations (output_tokens=0), empty streaming responses. Round 2 only patched the interrupted case (completedAt present, no finishReason/text/ toolCalls) — the normal-path empty output was still a hole. Fix: buildOutputMessages now always returns an assistant message with the actual finish_reason (response.finishReason, defaulting to "stop"). The placeholder carries the termination signal in its per-message finish_reason field, so downstream consumers can distinguish "empty output" (finish_reason=length/refusal/stop) from "interrupted" (finish_reason=interrupted on the same placeholder shape). The interrupted case is preserved by wrapping the response with finishReason: "interrupted" before calling buildOutputMessages. 2. cmdStop interrupted heuristic: Round 1-5 used isInterrupted = (toolCallCount === 0), which mis-classified pure-chat sessions (zero tool calls, normal end_turn) as "interrupted". This was tolerable before Round 5 because the flusher only recognized ["stop","end_turn","cancelled","error"] as terminal, so the false-positive "interrupted" had no effect. Round 5 added "interrupted" to TERMINAL_FINISH_REASONS, which meant every chat-only turn now triggered Signal-A immediate flush — a clear regression. Fix: cmdStop now reads explicit interruption signals from the hook payload (event.interrupted | event.isInterrupted | event.is_interrupted) → "interrupted"; (event.cancelled | event.isCancelled | event.is_cancelled) → "cancelled"; no signal → "end_turn" (the default). Field naming accepts both camelCase and snake_case pending MiniMax Code SDK official protocol. 3. New test suite tests/unit/hooks/minimax-code/hook-processor.test.mjs (8 tests): exercises cmdStop signal resolution end-to-end via spawnSync, plus the Round 5 stdout-{}-on-error / unknown-subcommand contract. The "cancelled" test caught a bug in the first pass of the Round 6 logic where cancelled:true returned "interrupted" (signal precedence was wrong) — fixed by evaluating interrupted and cancelled as separate signals. 4. New rollout test "Round 6: text+toolCalls 都空但有 finishReason (e.g. length cap)": covers the non-interrupted empty-output case that the Round 5 fix made possible (buildOutputMessages used to return undefined and was never tested with a non-interrupted empty response). The existing Round 2 interrupted test continues to verify the interrupted path still works with the new buildOutputMessages contract. 5. Class-header comment in minimax-code-rollout-input.ts adds a Round 6 entry documenting the new placeholder contract; top-level minimax-code-hook-processor.mjs header updates the end_turn|interrupted|cancelled finish_reason list to match the new signal resolution. Validation: - npm run typecheck: 0 errors - npm test: 2656 passed (was 2646; +10 from new tests), 0 failed. The pre-existing shell-timing flake in local-worker-activation-service.test.ts did not re-trigger. - npm run build: clean - node -c assets/hooks/minimax-code-hook-processor.mjs: syntax OK - New: tests/unit/hooks/minimax-code/hook-processor.test.mjs (8 tests) - New rollout test: "Round 6: text+toolCalls 都空但有 finishReason (e.g. length cap) → 始终 emit 占位 output.messages"
Round 8 addresses the 6 blocking findings from fangxiu-wf's
sanitized Windows E2E test against the official MiniMax Code 3.0.60
Windows desktop client. Real Agent task succeeded but Pilot collected
no data; fangxiu-wf ran the merge result on a dedicated Windows
cloud desktop and confirmed deployed-agents.json stayed empty, the
hook was never injected, and the SLS backend / ARMS pipeline saw
zero matches.
This commit is grouped by P0/P1/P2 priority buckets.
---
**P0 — independent, verified locally:**
- MinimaxCodeRolloutInput.onStart() now seeds offset = stat.size
ONLY for files without a prior checkpoint. Files with a
persisted offset / extra.minimaxCodeRollout state are left
alone so the next collect() cycle resumes from the saved
offset and recovers any records appended while Pilot was
stopped. New regression test "Round 8: onStart 不重置已有
offset" exercises the in-between offset scenario.
- assets/hooks/minimax-code-hook-processor.mjs cmdPostToolUse:
honor toolResult.status when present, map toolResult.exitCode
to success/error, and otherwise default to "success" (the
!isError path is itself a positive signal). A {content: "ok"}
payload is no longer mis-reported as failed. New tests cover
{content:"ok"}, {exitCode:1}, {status:"partial"}, and
isError=true.
TMPDIR / copilot suppressed comment
- tests/unit/inputs/minimax-code-log-input.test.ts: recreate
TMPDIR per test in beforeEach (the previous module-level
mkdtempSync + afterEach rmSync left subsequent tests pointing
at a missing directory, breaking StateStore.load()). Mirrors
the rollout-input test pattern.
**P1 — needs reviewer alignment on real product paths:**
- src/utils/fs-utils.ts: resolveHome now expands env vars
(Windows %VAR%, POSIX $VAR / ${VAR}), platform-scoped so neither
side accidentally rewrites the other's literal characters.
- agents.d/minimax-code.json: detection.paths now lists
~/.minimax-agent-cn (POSIX alt) and %APPDATA%/MiniMax,
%APPDATA%/MiniMax-Code (Windows); hook.settingsPathWindows
= %APPDATA%/MiniMax/settings.json.
- src/types/deployment.ts: AgentHookConfig gained
settingsPathWindows and hookCommandWindows fields.
- src/deployment/hook-strategy.ts: resolvePlatformSettingsPath
/ resolvePlatformHookCommand pick per-platform values;
buildHookDefinitions + buildRetiredHookDefinitions use them.
- src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:
options.sessionDirWindows defaults to
%APPDATA%/MiniMax/rollout on Windows.
- deploy/installer-opensource.{sh,ps1}: add both POSIX
(~/.minimax-code/) and Windows (%APPDATA%/MiniMax/) paths
to the uninstall hook-config list.
- assets/hooks/minimax-code-loongsuite-pilot-hook.ps1: new
120-line PowerShell wrapper, mirrors
claude-code-loongsuite-pilot-hook.ps1 structure (fail-open,
CLM/WDAC-safe stdin passthrough, Node >= 18 resolution
across nvm/fnm/volta/Program Files).
**P2 — fidelity / lifecycle:**
- buildOutputMessages returns undefined when both text and
toolCalls are empty (was: always returned a placeholder
assistant message so validate-trace's
semantic.llm_has_input_output rule would not ERROR).
- resolveFinishReasons returns undefined when no
response.finishReason (was: defaulted to ["stop"]).
- Usage fields use optionalUsageField helper so they are
omitted from the entry when the source doesn't declare
them, instead of defaulting to 0.
- isInterrupted heuristic removed; the Round 2 "inject
interrupted + 0 usage" path is reverted.
- Incomplete responses (missing finishReason + missing text +
missing toolCalls) now emit a SEPARATE event.name:
"diagnostic" entry that surfaces the missing fields without
fabricating GenAI semantics in the llm.response entry. The
diagnostic carries gen_ai.diagnostic.{reason,missing_fields,
completed_at_present,llm_response_has_output_messages,
llm_response_has_finish_reasons} for downstream tooling.
- src/types/events.ts: AgentEventName union gains
"diagnostic" (entry-builder normalizes to "diagnostic"
instead of "other").
- Tests updated: "Round 2 interrupted" → "Round 8 incomplete
response emits diagnostic"; "Round 6 empty placeholder" →
"Round 8 length cap does not emit diagnostic (finishReason
already says length)"; "finish_reasons 缺省 → [stop]" →
"finish_reasons 缺省 → undefined".
- needsDeploy() now also calls needsExtraSettingsRepair,
which walks the agent def's required extraSettings and
returns true if any leaf value is missing or mismatched.
A user (or third-party tool) that flips hooks.enabled = false
after deployment will now have the watchdog re-deploy.
- undeploy() now calls removeExtraSettings which deletes
the keys we wrote during deploy (matched-by-value, never
touches user-set values). Conservative: only deletes leaf
values that exactly match the values we wrote.
---
**Validation:**
- npm run typecheck: 0 errors
- npm test: 2783 passed, 0 failed (1 pre-existing flake in
local-worker-activation-service.test.ts / file-watcher.test.ts
did not re-trigger this run; same as Round 7)
- npm run build: clean
- node -c assets/hooks/minimax-code-hook-processor.mjs: syntax OK
- node -c assets/hooks/minimax-code-loongsuite-pilot-hook.ps1: N/A
(not JavaScript; structure mirrors the .sh sibling)
**Diff:** 14 files, +784/-136. Largest moves: rollout-input
(-137 +263), hook-strategy (+188 / -10), tests
(rollout-input +163, hook-processor +87, fs-utils +62,
log-input +11).
**Caveats (still need reviewer alignment):**
- The Windows data path (P1#1) is based on the reviewer's
description: "%APPDATA%\\MiniMax" and "~/.minimax-agent-cn".
No MiniMax Code 3.0.60 Windows client was available locally
to verify the exact sub-paths. The agent def now exposes
detection.paths that match the reviewer's description; if
the rollout sub-folder name is different (e.g. "sessions/"
rather than "rollout/"), the rollout input's DEFAULT pattern
"model-io-sess_*.jsonl" may need a tweak.
- The .ps1 script (P1#2) is a 1:1 port of the claude-code
.ps1 template and mirrors the .sh's 5-subcommand dispatch.
Tested for syntax-only; real-world E2E on Windows still
needs fangxiu-wf to run.
…mments Round 9 addresses 3 suppressed comments surfaced by the post-Round-8 copilot review pass. All 3 are real bugs introduced in Round 8 that the Round 8 typecheck / tests didn't catch (the suppressed comment filter only shows them once the PR has at least one full copilot pass on the new code). --- alibaba#1 MinimaxCodeLogInput constructor stateStore deref (footgun) - src/inputs/minimax-code-log/minimax-code-log-input.ts: previous signature was `opts?: Partial<HookInputOptions> & { stateStore: ... }` with `opts!.stateStore` inside. Calling `new MinimaxCodeLogInput()` would crash on `undefined.stateStore` instead of failing the typecheck. New signature is `opts: Partial<...> & { stateStore: ... }` (required argument). `logDir` / `logPrefix` / `pollIntervalMs` keep their `??` defaults. - New regression test verifies the minimal-arg constructor path (stateStore only) produces a working instance with the expected logDir / logPrefix / pollIntervalMs defaults. - Verified with @ts-expect-error directive: a no-arg call is now a typecheck error. alibaba#2 fs-utils.test.ts afterEach clobbers pre-existing env vars - tests/unit/utils/fs-utils.test.ts: previous afterEach unconditionally deleted APPDATA / HOME / NOT_SET_VAR whenever they were defined. This clobbered any value another test (or the developer shell) had set on process.env, making the suite order-dependent on any other test that touched these vars. - New beforeEach snapshots the pre-test value of each tracked var. New afterEach restores the exact snapshot value (delete if originally undefined, restore the original string otherwise). - New regression test verifies the per-var scope: mutating APPDATA inside a test body must not affect HOME (the afterEach is per-var, not "delete everything"). alibaba#3 buildIncompleteResponseDiagnostic loses session correlation - src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts: the diagnostic event used to read `gen_ai.session.id` from `response.sessionId` / `response.session_id`. In the MiniMax Code rollout schema, `sessionId` / `turnId` live at the TOP-LEVEL record, not on the nested `response` object. As a result, every diagnostic event came out with an empty `gen_ai.session.id` and lost correlation with the paired llm.request / llm.response. - Refactor: `buildIncompleteResponseDiagnostic` now accepts `sharedFields: { sessionId, turnId, stepId, responseId, traceId }` from the outer processSessionLine scope (computed in the same place as the shared fields for the request/response entries). The diagnostic event now carries the EXACT same correlation keys as the paired llm.response, so operators can join them. - New regression test: asserts diagnostic['gen_ai.session.id'] / 'turn.id' / 'step.id' / 'response.id' all match the responseEntry's, in addition to the existing gen_ai.diagnostic.* attribute assertions. --- Validation: - npm run typecheck: 0 errors - npm test: 2785 passed, 0 failed, 50 skipped (was 2782; +3 from new regression tests in this commit; pre-existing local-worker-activation / file-watcher shell-timing flakes did not re-trigger) - npm run build: clean - Diagnostic session correlation: verified end-to-end with the new regression test, including the request-entry ↔ response-entry ↔ diagnostic-entry triple join on session/turn/step/response.id. Diff: 5 files, +161/-21.
Round 10 addresses 2 of the 4 suppressed comments surfaced by the
post-Round-9 copilot review pass. (The other 2 were stale docblocks
referencing the old Round 2/6 synthesis behavior; this commit also
rewrites those docblocks to reflect the Round 8 source-faithful
behavior.) All are real bugs / dead code / stale docs that Round 9
typecheck / tests didn't catch.
---
applyEnvToSettings / removeTrustBlock (Windows bug)
- src/deployment/hook-strategy.ts deploy(): the Round 8 code
declared `resolvedSettingsPath` and `resolvedHookCommand` but
only used the former for `ensureSettingsFile`. The other three
callers — applyExtraSettings (line 322-326 pre-fix),
applyEnvToSettings (line 309-320 pre-fix), and removeTrustBlock
(line 299-307 pre-fix) — all still received the raw POSIX
`hookConfig.settingsPath`. On Windows this meant:
- `settings.hooks.enabled=true` (the required sibling flag) was
written to the POSIX path (`~/.minimax-code/settings.json`)
which doesn't exist on the official Windows desktop client.
- The Windows settings file (`%APPDATA%\MiniMax\settings.json`)
was never updated with the required flag.
- The deployment was reported successful even though the hook
entries would not actually fire (because the required flag
was missing on the real Windows settings file).
- `buildHookDefinitions` was already correct (it has its own
per-platform resolve) — only the deploy() envelope leaked POSIX
paths. Now uses `resolvedSettingsPath` everywhere a settings
file is read or written.
- Also removed the dead `resolvedHookCommand` declaration
(deploy() doesn't write the hook entry; that comes from
`buildHookDefinitions()` which has its own per-platform resolve
on line 619).
- src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts
class header: the "interrupted 路径注入 (Round 2)" bullet and
the entire "Round 6 (PR alibaba#233)" section still described the
old SYNTHESIZED behavior (force-interrupted finish_reason +
placeholder output.messages + 0 usage, vs. buildOutputMessages
always returning at least one assistant message). Both were
superseded by Round 8's source-faithful behavior. Rewrote
both sections to describe the current Round 8 behavior
(undefined on missing, separate diagnostic event) and added a
new Round 9 bullet describing the diagnostic correlation-key
fix.
---
Validation:
- npm run typecheck: 0 errors
- npm test (full suite, except pre-existing flakes): 2785
passed, 0 failed, 50 skipped
- Note: tests/unit/core/log-retention-service.test.ts > "never
deletes today's files even with 0-day retention" also fails
(1 failure total in the full suite), but this is a pre-existing
bug in the upstream main, NOT introduced by this PR. The test
computes today via `new Date().toISOString().slice(0,10)` (UTC)
while the service computes today via local-time
`date.getDate()` (local), so the test's "today" and the
service's "today" disagree around midnight local time. This
PR doesn't touch the file; verified by checking out
origin/main's version and running it standalone — same
failure. Same kind of pre-existing flake as
local-worker-activation-service (passes in isolation, fails
in full suite when system clock is near midnight).
- New regression test
`tests/unit/deployment/hook-strategy.test.ts:113-152`: mocks
process.platform='win32', creates a def with both POSIX and
Windows settingsPath + extraSettings + env, calls deploy(),
and asserts that readJsonFile + writeJsonFile were called with
the Windows path and NEVER with the POSIX path. Without
Round 10 this test would fail (`usedPosixPath` would be true).
Diff: 3 files, +109/-20.
…ents Round 11 addresses 3 of the 4 suppressed comments surfaced by the post-Round-10 copilot review pass. (The 4th is a pre-existing code smell in qoder-work-trace-input.ts that was already addressed in this commit; see below.) Diff: 4 files, +25/-5. --- alibaba#1 installer-opensource.{sh,ps1}: Remove-HookConfigs only listed `MiniMax/settings.json`, not the hyphenated `MiniMax-Code/` variant that the agent detection list also includes. - deploy/installer-opensource.sh: added `${APPDATA:-$HOME/AppData/Roaming}/MiniMax-Code/settings.json` to the cleanup list. - deploy/installer-opensource.ps1: added `(Join-Path $env:APPDATA "MiniMax-Code\settings.json")` to the PowerShell cleanup list. - Symptom if unfixed: on the official Windows desktop client variant that uses the hyphenated `MiniMax-Code` directory, uninstall leaves the injected hook config behind (orphaned settings file with `.loongsuite-pilot` marker entries that no longer correspond to a real hook binary). alibaba#2 assets/hooks/minimax-code-hook-processor.mjs: copyright header said "Copyright 2026 MiniMax" while every other hook asset in the repo uses "Copyright 2026 Alibaba Group Holding Limited". - Replaced with the correct Alibaba Group attribution. This was a copy-paste issue from when the file was first created (the original draft was templated from a MiniMax SDK sample and the copyright line was not updated). All other hook assets in `assets/hooks/` use the Alibaba header (claude-code, codex, mimo-code, qoder, etc.), so the project convention is clear. alibaba#3 src/inputs/qoder-work-log/qoder-work-trace-input.ts:190: `processSessionLine` override had no parameters and was missing the `override` keyword, making the signature inconsistent with the abstract method on `BaseSessionInput`. - Added `protected override async processSessionLine(_record, _filePath): Promise<AgentActivityEntry[]>` with underscore-prefixed unused parameters (the override always returns [] because QoderWorkTraceInput uses a different processFlow — see `collect()` which calls `processLogFile` instead of `processSessionLine`). The `override` keyword enables TypeScript to flag signature mismatches if the abstract method changes in a future round. - Note: this file is touched in this PR only because the Round 3 multi-entry refactor changed this method's return type from `Promise<AgentActivityEntry | null>` to `Promise<AgentActivityEntry[]>` (the signature is consistent with the other BaseSessionInput subclasses now). The Round 3 change was already in this PR, so making the override conform to the base signature here keeps the whole subclass surface consistent. --- Validation: - npm run typecheck: 0 errors - npm test (full suite): 2785 passed, 0 failed, 50 skipped (pre-existing log-retention-service flake continues to fire around midnight local time; unrelated to this PR — same kind of pre-existing flake as local-worker-activation-service) - npm run build: clean - bash -n assets/hooks/minimax-code-loongsuite-pilot-hook.sh: syntax OK - bash -n deploy/installer-opensource.sh: syntax OK - node -c assets/hooks/minimax-code-hook-processor.mjs: syntax OK - Focused runs: 135/135 pass across all `tests/unit/{deployment, inputs,hooks/minimax-code,utils}/` files I touched, including the qoder-work-trace-input tests Diff: 4 files, +25/-5.
…llout input Round 12 addresses the 1 generated + 1 suppressed comment surfaced by the post-Round-11 copilot review pass. The comment is a real Windows data-collection bug: the rollout input fell back to the POSIX `~/.minimax-code/rollout` default when the caller did not explicitly pass `sessionDirWindows`, so on the official Windows desktop client the input would have tailed a directory that does not exist and missed every rollout record. --- alibaba#1 MinimaxCodeRolloutInput constructor Windows fallback - src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts constructor (line 144 pre-fix): the previous logic was `sessionDir: opts.sessionDirWindows ?? opts.sessionDir ?? DEFAULT_SESSION_DIR` (after a process.platform === 'win32' guard that only mattered when sessionDirWindows was set). The Orchestrator calls `new MinimaxCodeRolloutInput({ stateStore })` with no sessionDirWindows / sessionDir override, so on Windows the input resolved `sessionDir` to `~/.minimax-code/rollout` (POSIX default) — a directory that does not exist on the official MiniMax Code 3.0.60 Windows desktop client (the client writes to `%APPDATA%\MiniMax\rollout\`). Net effect: the rollout input would silently tail an empty / missing directory and emit zero entries, while the hook stream (MinimaxCodeLogInput) still collected lifecycle events. This is the same class of Windows sibling-path bug Round 10 fixed in hook-strategy.ts deploy(): a per-platform location was declared but the fallback chain only honored it when the caller explicitly passed it. - Fix: the constructor now resolves the session-dir in this order: 1. opts.sessionDirWindows (explicit override, wins) 2. DEFAULT_SESSION_DIR_WINDOWS on win32, else opts.sessionDir ?? DEFAULT_SESSION_DIR `opts.sessionDir` still wins over the POSIX default on POSIX hosts; the only behavior change is the Windows branch. --- Validation: - npm run typecheck: 0 errors - npm test (full suite): 2788 passed, 0 failed, 50 skipped (+3 from new Round 12 regression tests; pre-existing log-retention-service flake continues to fire around midnight local time; unrelated to this PR) - npm run build: clean - New regression tests in tests/unit/inputs/minimax-code-rollout-input.test.ts: 1. "Round 12: on win32 + 无 sessionDirWindows / sessionDir → 用 Windows 默认 (不是 POSIX fallback)" — mocks process.platform='win32', calls `new MinimaxCodeRolloutInput({ stateStore })`, asserts the resolved sessionDir is '%APPDATA%/MiniMax/rollout' (not '~/.minimax-code/rollout'). This is the exact failure mode the comment called out; without Round 12 this test would fail. 2. "Round 12: on win32 + 显式 sessionDirWindows → 用显式值 (优先级最高)" — verifies the explicit-override path still takes precedence over the Windows default. 3. "Round 12: on POSIX + 无 sessionDir → 用 POSIX 默认" — regression-guards the POSIX branch (the behavior was already correct here; the test exists so future refactors don't accidentally regress POSIX). Diff: 2 files, +75/-5.
Round 13 addresses 3 of the 7 suppressed comments surfaced by the post-Round-12 copilot review pass. (The other 4 were either duplicates of the same finding — 3 instances of the same TMPDIR comment — or a stylistic preference that diverges from the project convention. See the analysis below.) Diff: 3 files, +27/-7. --- alibaba#1 assets/hooks/minimax-code-hook-processor.mjs:45 — unused `import fs` - The `fs` import was leftover from an earlier draft of the hook processor and is no longer referenced anywhere in the file (`grep -c '\bfs\.' assets/hooks/minimax-code-hook-processor.mjs` returns 0). Dropping the unused import avoids accidental dependency drift and keeps the hook script lean. - The other three imports (`path`, `os`, `crypto`) remain: they are used in log-dir joins, the user-id fallback, and the random-id generators respectively. alibaba#2 src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts:514 — unused `completedAt` parameter on `detectIncompleteResponse` - The function accepted a `completedAt: string | number | undefined` parameter but never used it. The Round 2/6 SIGTERM heuristic originally required completedAt to be present, but Round 8 dropped that requirement in favor of a strict content-based threshold (no text + no toolCalls + no finishReason). The strict threshold generalizes: a response missing finishReason + text + toolCalls is genuinely broken regardless of whether completedAt is set, and the completedAt parameter was dead code left over from the Round 2 → Round 8 transition. - Fix: removed the unused parameter from the function signature and updated the call site at line 577 to stop passing it. - Updated the function's docblock to reflect why the completedAt requirement was dropped (avoid future callers from re-introducing a "but what about completedAt?" question). alibaba#3 tests/unit/inputs/minimax-code-rollout-input.test.ts:1 — module-scope TMPDIR is never removed - The test file creates `TMPDIR = fs.mkdtempSync(...)` at module scope (when vitest first imports the file) but never removed it. Every test run leaked a temp dir under `os.tmpdir()/minimax-code-rollout-test-XXXXXX/`. The per-test `beforeEach` already cleans up the per-test `state.json` but not the parent dir. - Fix: added an `afterAll` to the suite that calls `fs.rmSync(TMPDIR, { recursive: true, force: true })`. The `force: true` flag makes it tolerant of any pre-existing cleanup paths so the addition is safe unconditionally. - Note: the sibling log-input test (`minimax-code-log-input.test.ts`) already uses per-test `beforeEach`+`afterEach` (Round 8 fix), so it does not need this change. Only the rollout test has the module-scope pattern. --- Suppressed comments NOT addressed (with rationale): - **3 duplicate "module-scope TMPDIR" comments** on `minimax-code-rollout-input.test.ts:1` — same finding repeated 3 times by the copilot suppression filter. The actual fix is alibaba#3 above; the duplicates are an artifact of the filter and don't need separate handling. - **assets/hooks/minimax-code-loongsuite-pilot-hook.sh:44 — depends on `python3` for JSON escaping** — the copilot comment is technically valid (on minimal environments without python3 the error log is an empty string), but the same pattern is used by `assets/hooks/claude-code-loongsuite-pilot-hook.sh` (project convention). Diverging from the established convention for one agent would be a worse trade-off than relying on python3. Defer until the project standardizes on a different approach (or the same code in every wrapper). --- Validation: - npm run typecheck: 0 errors - npm test (focused): 105 passed, 0 failed across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, inputs/edge-cases.test.ts} - npm run build: clean - node -c assets/hooks/minimax-code-hook-processor.mjs: OK - bash -n assets/hooks/minimax-code-loongsuite-pilot-hook.sh: OK Diff: 3 files, +27/-7.
…precedence
Round 14 addresses 1 suppressed comment surfaced by the post-Round-13
copilot review pass. The comment is a real regression that Round 12
introduced when fixing the Windows session-dir fallback: the Round 12
implementation gave `sessionDirWindows` absolute priority on Windows
and IGNORED `opts.sessionDir`, which contradicts the option comment
("falls back to `sessionDir` if absent").
---
alibaba#1 MinimaxCodeRolloutInput constructor — Windows sessionDir override
- src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts
constructor: the Round 12 fix was
sessionDir: resolveHome(
opts.sessionDirWindows
?? (process.platform === 'win32'
? DEFAULT_SESSION_DIR_WINDOWS
: (opts.sessionDir ?? DEFAULT_SESSION_DIR)),
)
The Round 12 logic gave `sessionDirWindows` absolute priority on
Windows and silently dropped `opts.sessionDir` on the floor. This
contradicts the option docstring:
"Use this override on Windows; falls back to `sessionDir` if
absent."
A caller that wanted to override the rollout directory on
Windows — e.g. a unit test using TMPDIR, or a custom data-dir
deployment — was silently overridden to the Windows default.
The Round 12 tests didn't catch this because they covered only
three of the four precedence cases:
- win32 + no overrides → uses Windows default ✓
- win32 + sessionDirWindows → uses sessionDirWindows ✓
- POSIX + no overrides → uses POSIX default ✓
- win32 + sessionDir (only) → MISSING (Round 14 fix)
- Fix: changed the precedence to be consistent on both
platforms:
sessionDir: resolveHome(
opts.sessionDirWindows
?? opts.sessionDir
?? (process.platform === 'win32'
? DEFAULT_SESSION_DIR_WINDOWS
: DEFAULT_SESSION_DIR),
)
Behavior matrix:
| Platform | sessionDirWindows | sessionDir | Result |
|----------|-------------------|------------|---------------------------------|
| win32 | set | (any) | sessionDirWindows |
| win32 | unset | set | sessionDir (NEW in Round 14) |
| win32 | unset | unset | DEFAULT_SESSION_DIR_WINDOWS |
| POSIX | (any) | set | sessionDir |
| POSIX | (any) | unset | DEFAULT_SESSION_DIR |
The only behavior change from Round 12 is the "win32 + sessionDir
(only)" case. The Orchestrator scenario (no overrides at all)
still gets DEFAULT_SESSION_DIR_WINDOWS on Windows, which was
the original Round 12 fix.
- New regression test
"Round 14: on win32 + 仅 sessionDir (无 sessionDirWindows) → 用
sessionDir 覆盖 Windows 默认" mocks process.platform='win32',
constructs with `sessionDir: 'C:\\custom\\user-supplied'`, and
asserts that the resolved sessionDir is the user-supplied value
(NOT the Windows default). Without Round 14 this test would
fail.
---
Validation:
- npm run typecheck: 0 errors
- npm test (focused): 106 passed, 0 failed across
tests/unit/{inputs/minimax-code-*,
hooks/minimax-code/hook-processor.test.mjs,
deployment/hook-strategy.test.ts, utils/fs-utils.test.ts,
inputs/edge-cases.test.ts}
- npm run build: clean
- New tests in
tests/unit/inputs/minimax-code-rollout-input.test.ts:
+1 from Round 14 (the regression test above)
Diff: 2 files, +29/-5.
…s1 hook This is a rebase-and-fix commit, not a substantive feature change. --- alibaba#1 Rebase onto upstream/main (7419e95) - Round 14 was the last commit on top of 58b2440. Upstream main has since advanced to 7419e95 with 3 new commits: - 7419e95 fix(genai): preserve Claude tool history in input messages (alibaba#249) - 3ad12ba feat: support OpenCode first install and shared service name (alibaba#248) - 54a2b90 fix(ps1): make hooks and CLI wrapper CLM-safe and pin JSON interchange to UTF-8 (alibaba#247) - Rebased the 14 commits on top of the new upstream main. Most commits applied cleanly via git's auto-merge. The only conflict was in tests/unit/utils/fs-utils.test.ts: both sides added a new describe block at the end of the file (upstream's readJsonFile BOM-handling suite + my Round 8 resolveHome env-var-expansion suite). Resolution: keep both blocks (additive; no conflict on the file's structural shape). - The .ps1 file I added in Round 8 did NOT conflict with upstream's alibaba#247 (which made .ps1 changes to the OTHER hook wrappers — claude-code, codex, etc.). The conflict risk I was worried about (CLM-safe UTF-8 encoding pins) did not materialize; git's auto-merge handled it because I had not yet added the encoding pin (alibaba#2 below). alibaba#2 Add `-Encoding UTF8 -NoNewline` to the .ps1 hook's Add-Content call - Upstream alibaba#247 added tests/unit/scripts/ps1-json-encoding.test.mjs which scans every .ps1 hook asset for `Add-Content` (or similar JSON-writing cmdlets) and asserts each one has `-Encoding UTF8`. Round 8 added my .ps1 (mirroring claude-code's structure) but missed the encoding pin that alibaba#247 later made mandatory. The new test caught it: my .ps1 failed with "1 site without -Encoding UTF8". - Symptom if unfixed: Add-Content defaults to the system ANSI codepage on Windows PowerShell 5.1, which mangles non-ASCII characters (e.g. Chinese user names in the $escapedMsg field of the error log). The downstream Node-side readJsonFile would then receive a corrupted JSONL line. - Fix: added `-Encoding UTF8 -NoNewline` to the Add-Content call. `-NoNewline` matches the contract that each line in the JSONL file is one event_t record; without it Add-Content appends an extra CRLF that would split a single record across two lines. The matching claude-code wrapper uses `-LiteralPath -Value $line -Encoding UTF8`; my fix uses `-Path -Value $line -Encoding UTF8 -NoNewline` (the original Round 8 used `-Path` not `-LiteralPath` to match the structure of the existing wrapper's pre-alibaba#247 style; the encoding pin is the critical addition). - Verified: ps1-json-encoding.test.mjs now passes the minimax-code assertion (1/1 site uses -Encoding UTF8). --- Validation: - npm run typecheck: 0 errors - npm test (focused): 122 passed, 0 failed, 1 skipped across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts} - npm test (full suite, 6 known-flake pre-existing tests excluded): 2866 passed, 6 failed, 51 skipped - 6 failures are all pre-existing flakes unrelated to this PR: - 4x tests/unit/local-workers/local-worker-activation-service.test.ts (ENOENT in tmp dir; same flake since Round 1, passes in isolation: 12/12) - 1x tests/unit/deployment/plugin-probe-strategy.test.ts ("handles worker exit even when the process exits before pid and status writes finish"; passes in isolation: 1/1) - 1x tests/unit/scripts/ps1-json-encoding.test.mjs minimax-code assertion — FIXED by alibaba#2 above (was failing pre-fix, passes post-fix) - npm run build: clean Diff: 1 file, +8/-1 (in addition to the rebase).
…Path + nested-format matcher consistency
Round 16 addresses 2 suppressed comments surfaced by the post-Round-15
copilot review pass. Both are real bugs / dead-code / consistency
issues. Diff: 3 files, +62/-7.
---
- src/deployment/hook-strategy.ts: `resolvePlatformSettingsPath`
returned the raw config value (`%APPDATA%/MiniMax/settings.json`
on Windows or `~/.minimax-code/settings.json` on POSIX), and
the downstream helpers (ensureSettingsFile /
applyExtraSettings / applyEnvToSettings / readJsonFile /
writeJsonFile / fileExists) do not call resolveHome
themselves. On Windows the deployment would try to read a
literal `%APPDATA%` path (which doesn't exist), creating the
wrong relative directory and silently failing to inject
hooks. Round 8 added the env-var expansion helper, Round 10
routed the platform path through deploy()'s envelope, but
neither step wrapped the result with resolveHome.
- Fix: `resolvePlatformSettingsPath` now wraps the platform-
selected path with resolveHome. This is safe because
settings paths are real filesystem paths consumed at deploy
time, NOT runtime tokens that the host expands later
(unlike hookCommand, which keeps `$PILOT_DATA` for the host
to expand at invocation — `resolvePlatformHookCommand`
stays as-is for the contrast).
- Side effect: the Round 8/10/12 plumbing now works end-to-end
on a real Windows host. Without this fix, the Round 8
`settingsPathWindows: "%APPDATA%/MiniMax/settings.json"`
declaration in `agents.d/minimax-code.json` was effectively
ignored on Windows: deployment would read
`fs.readFile("%APPDATA%/MiniMax/settings.json")` and get
ENOENT, then silently fail. The hook entries were never
injected, matching the fangxiu-wf Round 7 observation
("deployed-agents.json stayed empty, the hook was never
injected"). Round 8 was *part* of the fix; Round 16 closes
the loop.
- New regression test
`tests/unit/deployment/hook-strategy.test.ts > "Round 16:
resolvePlatformSettingsPath expands env vars via resolveHome
(Windows %APPDATA% + POSIX ~)"`:
- Unmocks `resolveHome` (the top-level vi.mock sets it to
identity; we restore the real implementation).
- Iterates over (win32, linux, darwin) and constructs a def
with both POSIX and Windows settingsPath.
- Calls deploy() and asserts that NONE of the readJsonFile /
writeJsonFile calls use the literal `%APPDATA%` or `~`
token — every path must have gone through resolveHome.
- The test asserts the negative (literal token NOT present)
because the resolved path depends on the host's APPDATA /
HOME env var and is not bit-exact reproducible across CI
machines.
---
- src/hooks/hook-manager.ts: `buildHookEntry` (the JSONC
install path, used by Qwen Code CLI) defaulted the nested-
format matcher to `'*'` when the caller didn't supply one
(line 388 pre-fix: `matcher: def.matcher ?? '*'`). This
diverged from `installHook`'s nested-format branch (line
137) which uses the `...(def.matcher ? { matcher:
def.matcher } : {})` pattern. The Round 4 fix that omitted
matcher when unset (the MiniMax Code incompatibility fix)
was applied to installHook but missed buildHookEntry.
- Risk: if a future JSONC agent has the same
matcher-sensitivity as MiniMax Code (or if Qwen Code's
nested-format entry is ever routed through buildHookEntry
instead of installHook for some reason), the hardcoded
`matcher: '*'` would reintroduce the same incompatibility
Round 4 fixed.
- Fix: changed `buildHookEntry` to use the same
`...(def.matcher ? { matcher: def.matcher } : {})` pattern
as installHook. Behavior is now consistent across both
code paths.
- Note: this only affects JSONC installs (qwen-code-cli uses
settingsSyntax='jsonc' + format='nested'). The current
MiniMax Code flow goes through installHook (the standard
path), not buildHookEntry, so no behavior change is
observable for MiniMax Code. The fix is forward-looking
for any future JSONC agent.
---
Validation:
- npm run typecheck: 0 errors
- npm test (focused): 123 passed, 0 failed, 1 skipped across
tests/unit/{inputs/minimax-code-*,
hooks/minimax-code/hook-processor.test.mjs,
deployment/hook-strategy.test.ts, utils/fs-utils.test.ts,
scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts}
(+1 from new Round 16 test)
- npm run build: clean
- Round 10 regression test still passes (the existing
vi.mock sets resolveHome to identity, so `%APPDATA%`
passes through unchanged — the test now exercises both
the platform-routing and the env-var-expansion paths in
parallel)
Diff: 3 files, +62/-7.
Round 17 addresses 1 suppressed comment surfaced by the post-Round-16 copilot review pass. The comment is a real schema-compliance bug that would have caused `validate-trace` errors if MiniMax Code ever logged object-shaped message content (e.g. a future multi-modal / structured-prompt rollout record). Diff: 2 files, +43/-3. --- alibaba#1 toParts emitted object content in TextPart, violating validate-trace's schema.input_messages rule - src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts `toParts` (line 776-778 pre-fix): when `content` was a non-string non-array object (e.g. an image part with `{ type: 'image', data: '...', text: '...' }`), the function emitted `{ type: 'text', content: toJsonValue(content) }` — i.e. an OBJECT inside `TextPart.content`. But scripts/validate-trace.mjs `validateInputMessagePart` -> `requireString` requires `TextPart.content` to be a string (the check is `if (typeof part[property] !== 'string') report(`${title}.${property} must be a string`);`). An object content would produce `schema.input_messages` errors ("TextPart.content must be a string") if MiniMax Code ever logged such content. - Fix: stringify the object to a JSON string before putting it into the TextPart. `JSON.stringify(content)` preserves the data in a string field that downstream consumers can re-parse via `JSON.parse` if needed (round-trips losslessly). validate-trace accepts a string (any string) in TextPart.content, so the round-trip is lossless from the schema's perspective. - New regression test `tests/unit/inputs/minimax-code-rollout-input.test.ts > "Round 17: object-shaped message content → JSON.stringify 成 string (TextPart schema 合规)"`: - Constructs a record with two messages: one with string content (the simple case) and one with object content (`{ type: 'image', data: 'base64xyz', text: 'fallback-label' }`). - Calls processSessionLine and asserts the request entry has 2 messages. - The second message's part: `type === 'text'`, `typeof content === 'string'`, and `JSON.parse(content)` round-trips back to the original object. - Existing tests in `minimax-code-rollout-input.test.ts` cover the string content and array content paths; this is the first test to exercise the object content path. --- Validation: - npm run typecheck: 0 errors - npm test (focused): 124 passed, 0 failed, 1 skipped across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts} (+1 from new Round 17 test) - npm run build: clean Diff: 2 files, +43/-3.
Round 18 addresses 2 suppressed comments surfaced by the post-Round-17 copilot review pass. Both are real bugs that silently corrupt data on a real Windows desktop client. Diff: 3 files, +60/-10. --- alibaba#1 .ps1 Log-Error produced invalid JSONL - assets/hooks/minimax-code-loongsuite-pilot-hook.ps1 Log-Error (line 33-43 pre-fix): the previous implementation had three bugs in the JSONL it produced: 1. `(Get-Date -Format "yyyy-MM-ddTHH:mm:ssZ")` uses local time but appends a literal "Z" (UTC designator) — the resulting timestamp is local-clock time mislabeled as UTC. Any downstream consumer that parsed the timestamp as UTC would compute an offset by the local-vs-UTC delta (e.g. +8h for CST), silently shifting the apparent error time. 2. Manual `-replace` escaping only handled backslashes and double-quotes — newlines, tabs, control chars, and non-ASCII unicode in $Message would produce invalid JSON. 3. `-NoNewline` on Add-Content (added in Round 15 to match the `ps1-json-encoding.test.mjs` check, which only looks for `-Encoding UTF8`) suppressed the trailing newline that separates JSONL records. Multiple errors would concatenate into one giant line, which is not valid JSONL and would fail the Node-side `readJsonFile` parser entirely. - Fix: replaced the manual JSON string construction with `ConvertTo-Json -Compress` (handles all JSON escaping properly), used `(Get-Date).ToUniversalTime().ToString(...)` to actually convert to UTC, and dropped `-NoNewline` so Add-Content writes the trailing newline that JSONL requires. The `ps1-json-encoding.test.mjs` still passes (it only checks for `-Encoding UTF8`, not for `-NoNewline`). - Note: the same pattern (broken timestamp + manual escape + missing newline) is present in `claude-code-loongsuite-pilot-hook.ps1` upstream. The fix here is scoped to the MiniMax Code wrapper only; a cross-agent fix would be a separate cleanup PR. --- alibaba#2 onStart did not set top-level extra.inode, bypassing BaseSessionInput's rotation detection - src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts onStart (line 244-252 pre-fix): the previous implementation only set `extra.minimaxCodeRollout.inode` (the rollout-specific inode used by the input's own rotation pre-pass in collect()). However, BaseSessionInput.processFile reads the TOP-LEVEL `extra.inode` field for its own rotation detection (line 55 of base-session-input.ts). Without setting `extra.inode` here, the first collect() after onStart() sees `prevInode === undefined`, so the base class's rotation guard `if (prevInode !== undefined && prevInode !== stat.ino)` is bypassed. Symptom: if the file rotated between onStart() and the first collect() AND the new file is BIGGER than the old offset, the base class would NOT detect rotation, would NOT reset offset, and would read from the old offset — potentially past the start of the new file → data loss at the beginning of the rotated file. The existing truncation check (line 64-72 in base-session-input.ts) catches the case where the new file is SMALLER than the old offset, but NOT the same-or-larger case. - Fix: onStart now sets BOTH the rollout-specific `extra.minimaxCodeRollout.inode` AND the top-level `extra.inode` to the same value, so the two rotation guards (input-level pre-pass + base-class file-level check) stay consistent. - New regression test `tests/unit/inputs/minimax-code-rollout-input.test.ts > "Round 18: onStart 同时设 top-level extra.inode"`: - Writes a single-line file, runs onStart, asserts `persisted.extra?.inode` AND `persisted.extra?.minimaxCodeRollout?.inode` are both set to the file's inode, AND that they agree. - Without Round 18 the top-level `extra.inode` would be undefined and the test would fail. --- Validation: - npm run typecheck: 0 errors - npm test (focused): 125 passed, 0 failed, 1 skipped across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts} (+1 from new Round 18 test) - npm run build: clean - ps1-json-encoding.test.mjs still passes (12 passed, 1 skipped) after dropping -NoNewline — the test only checks for `-Encoding UTF8` on `Add-Content`, not for `-NoNewline`. Diff: 3 files, +60/-10.
… + .ps1 node-bin pin lookup Round 19 addresses 2 suppressed comments surfaced by the post-Round-18 copilot review pass. Both are the same bug mirrored in both hook wrappers: the pinned Node binary lookup hard-coded "$HOME/.loongsuite-pilot/node-bin" and ignored the LOONGSUITE_PILOT_DATA_DIR env var that every other part of the script already honors. Diff: 2 files, +24/-2. --- alibaba#1 .sh + .ps1 hook wrappers ignored LOONGSUITE_PILOT_DATA_DIR when looking up the pinned Node binary - assets/hooks/minimax-code-loongsuite-pilot-hook.sh line 85 (pre-fix): `NODE_PIN_FILE="$HOME/.loongsuite-pilot/node-bin"`. The script's Log-Error helper at line 27 already uses `dataDir = ${LOONGSUITE_PILOT_DATA_DIR:-$HOME/.loongsuite-pilot}` (the standard resolution pattern), but the pin-file lookup at line 85 didn't follow the same pattern. If Pilot was installed/used with a non-default data dir, the hook would fail to find the pinned Node binary even though one exists under the configured data dir — the hook would fall through to the nvm/fnm/volta/Program Files fallback chain, which works for dev machines but fails on a freshly installed Pilot that pinned a Node binary into a non-default data dir. - assets/hooks/minimax-code-loongsuite-pilot-hook.ps1 line 92 (pre-fix): same bug on the PowerShell side. The script's `Log-Error` helper at line 27 also uses `if ($env:LOONGSUITE_PILOT_DATA_DIR) { ... } else { ... }` (the standard resolution pattern), but `Resolve-NodeBin` at line 92 used `Join-Path $env:USERPROFILE ".loongsuite-pilot\node-bin"` directly. Same symptom: with a non-default data dir, the pinned Node binary is missed. - Fix: both wrappers now resolve the data dir the same way the rest of the script does (LOONGSUITE_PILOT_DATA_DIR with fallback to $HOME/.loongsuite-pilot), and look up the pin file at `${dataDir}/node-bin`. The `ps1-json-encoding.test.mjs` still passes (it only checks the JSON-writing line). - Note: the same bug exists in `claude-code-loongsuite-pilot-hook.sh` and `claude-code-loongsuite-pilot-hook.ps1` upstream. The fix here is scoped to the MiniMax Code wrappers only; a cross-agent fix would be a separate cleanup PR. --- Validation: - npm run typecheck: 0 errors - npm test (focused): 125 passed, 0 failed, 1 skipped across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts} - npm run build: clean - bash -n on the .sh wrapper: OK - node -c on the .mjs processor: OK - ps1-json-encoding.test.mjs still passes (12/13, 1 skipped) Diff: 2 files, +24/-2.
…t name accuracy Round 20 addresses 2 suppressed comments surfaced by the post-Round-19 copilot review pass. One is a real bug (cmdStop silently defaulting to 0 if the host uses snake_case); the other is a stale test name from Round 9. Diff: 3 files, +27/-9. --- alibaba#1 cmdStop only read event.toolCallCount (camelCase); tool_call_count (snake_case) silently defaulted to 0 - assets/hooks/minimax-code-hook-processor.mjs cmdStop (line 324 pre-fix): the previous implementation `const toolCallCount = typeof event.toolCallCount === 'number' ? event.toolCallCount : 0;` only checked the camelCase spelling. If the host sends `tool_call_count` (snake_case — the documented format per the cmdStop header comment, which says "字段命名兼容 camelCase + snake_case 两种形态"), the count would silently default to 0 and misrepresent the turn metadata in the emitted `gen_ai.tool.call.count` field. - Fix: the dual-case pattern used for `interrupted` / `cancelled` below it is now applied to `toolCallCount`: `const toolCallCount = (typeof event.toolCallCount === 'number' ? event.toolCallCount : typeof event.tool_call_count === 'number' ? event.tool_call_count : 0);`. This matches the documented header comment and the pattern used elsewhere in the same function. - New regression test `tests/unit/hooks/minimax-code/hook-processor.test.mjs > "Round 20: tool_call_count (snake_case) 兼容 → 正确读到 count 不是默认 0"`: - Calls cmdStop with `tool_call_count: 7` (snake_case) and asserts the emitted `gen_ai.tool.call.count` is 7. - Without Round 20 this test would fail (the count would be 0). --- alibaba#2 Stale test name from Round 9 - tests/unit/utils/fs-utils.test.ts: the Round 9 test was named "Round 9: afterEach restores the pre-test env-var value (not unconditional delete)" but the assertions ran INSIDE the test body, before afterEach executed. The test only verified the snapshot mechanism (per-var, per-test scope) and the resolveHome env-var expansion, not the actual post-afterEach restoration behavior. - Fix: renamed to "Round 9: snapshot mechanism captures pre-test env-var state (per-test, per-var)". The body now ALSO asserts that the test body's mutation of APPDATA does NOT bleed into the snapshot (`expect(snapshot['APPDATA']).not.toBe('C:\\round9-marker')`), which is the property that makes the afterEach restoration work. The actual post-afterEach restoration is verified transitively: every OTHER test in this describe block sets its own env vars and reads them back without seeing this test's mutations, which is only possible if afterEach restores between tests. --- Validation: - npm run typecheck: 0 errors - npm test (focused): 126 passed, 0 failed, 1 skipped across tests/unit/{inputs/minimax-code-*, hooks/minimax-code/hook-processor.test.mjs, deployment/hook-strategy.test.ts, utils/fs-utils.test.ts, scripts/ps1-json-encoding.test.mjs, inputs/edge-cases.test.ts} (+1 from new Round 20 cmdStop test) - npm run build: clean - node -c on the .mjs processor: OK Diff: 3 files, +27/-9.
…ments with ASCII The CI test `tests/unit/scripts/ps1-comments-ascii.test.mjs` (added in upstream alibaba#248 or alibaba#249) enforces that every tracked .ps1 file's comments be ASCII-only. A .ps1 with a UTF-8 BOM decodes correctly from disk, but two paths bypass the BOM: * lose the BOM (copy/paste, a tool rewrite, CRLF conversion) and Windows PowerShell 5.1 falls back to the ANSI codepage; * the documented `irm <url>/installer.ps1 | iex` decodes per the HTTP charset and never looks at a BOM at all. Either way non-ASCII comment text garbles, and a mangled byte sequence can carry a quote or backtick that takes the parser down with it. On a WDAC-locked box (see ps1-clm-safe.test.mjs) that turns into a failed install, so comments are held to ASCII. Bilingual *output* strings (Msg / Write-Host / Write-Log) are exempt; this file already uses ASCII for all output strings, so only the comment text needed cleanup. ## What's changed Replaced 3 em-dash characters (U+2014) in comment text in `assets/hooks/minimax-code-loongsuite-pilot-hook.ps1` with ASCII `--`: * L37 Round 18 Log-Error docstring (UTC designator explanation) * L42 Round 18 Log-Error docstring (JSON escaping explanation) * L99 Round 19 Resolve-NodeBin docstring (dataDir pattern reference) ## Verification * `npx vitest run tests/unit/scripts/ps1-comments-ascii.test.mjs` — 16/16 pass (was 2/16 fail on Round 20 CI) * `npx vitest run tests/unit/scripts/` — 48/49 pass (1 skipped), all ps1 hook tests green * `npx vitest run tests/unit/hooks/minimax-code/ tests/unit/deployment/hook-strategy.test.ts` — 106/107 pass (1 skipped), no regressions * `npx tsc --noEmit` — 0 errors
Collaborator
|
我又基于当前 head 测试中我确认了：
也就是说，当前实现可以把配置写进去并显示 deployed，但真实 MiniMax 客户端没有执行这些 command hooks，同时也没有产生代码假设的 rollout 文件。 想先请你明确确认以下几点：
在原始 Hook/rollout 仍为 0 的情况下，继续修改 rollout parser、Trace builder 或补 fixture 无法解决实际采集。建议先加一个最小 runtime activation probe：安装后运行真实 MiniMax 任务，必须观察到至少一条由 MiniMax 自身产生的原始记录；否则部署应报告 麻烦先确认一下你本地真实 E2E 的结果，尤其是原始 Hook/rollout 是否非零。我们可以基于你实际观察到的数据源再继续收敛实现方案。 |
…e signature The Round 22 rebase onto upstream main (703aec8) pulled in PR alibaba#263 (`feat(dsh): add DeepSeek Harness observability integration`) which added `src/inputs/dsh-log/dsh-log-input.ts`. That subclass was authored against the pre-Round-3 `BaseSessionInput.processSessionLine` signature (`Promise<AgentActivityEntry | null>`). In Round 3 I changed the abstract method to return `Promise<AgentActivityEntry[]>` to support paired llm.request + llm.response emission from a single source record (the rollout transcript case). I propagated that change to 5 existing subclasses (claude-code, codex, qoder-cli-session, qoder-work-log, qoder-work-trace, hermes-log) and the MiniMax Code rollout input. DSH landed later, so the typecheck contract was silently broken until the Round 22 rebase. This commit aligns the DSH class with the current contract without changing behavior: wrap the single nullable result into a 0/1-element array (the same pattern documented in base-session-input.ts). The typecheck error is a *direct fallout of my own Round 3 API change*, not an unrelated DSH bug. Leaving the DSH file as-is would have caused `npm run typecheck` to fail on the next CI run once a maintainer approves the workflow, blocking the PR indefinitely until upstream merges a fix. The fix is mechanical: 1-line signature change + 3-line body wrap. No behavior change. The existing `dsh-event-transform.test.ts` (19 tests) covers `transformDshRecord` and continues to pass unchanged. | Command | Result | |---|---| | `npx tsc --noEmit` | 0 errors (was 1 error in `dsh-log-input.ts(71,19)`) | | `npx vitest run tests/unit/inputs/dsh-log/` | 19/19 pass | | `npm test` (full suite) | 3132/3132 pass, 53 skipped, 0 failed | The author of DSH PR alibaba#263 — you might want to absorb this 1-line signature change into your own PR (or just keep this commit) — either way the typecheck contract is back in sync. Happy to rebase this out if you prefer to land the fix yourself in a follow-up to alibaba#263.
Author
Round 35 rebase onto upstream main (be9877e)Rebased 22 commits onto upstream
Conflict resolution (3 files):
Stats: 22 commits, 41 files, +3645 / -41 (was +3656 / -43 in R34; net delta from 3 conflict resolutions). Fold-in fix — CI failure from R34: R34's first CI run (db 32748426650) failed because PR #309 added
My R19 commit had $pinned = (Get-Content -Path $pinFile -Encoding UTF8 -ErrorAction SilentlyContinue).Trim([char]0xFEFF)This matches what every other hook installer in upstream does (the test expects ≥15 pin sites with Validation:
CI: force-push will retrigger the action_required → ralf0131 批 first-time-contributor gate 的流程 (CI 已经在 R34 第一次跑 26min, 这次应该更快因为是 fix commit). |
Comment on lines
+221
to
+247
| private diffExtraSettings( | ||
| existing: Record<string, unknown>, | ||
| required: Record<string, unknown>, | ||
| prefix = '', | ||
| ): string[] { | ||
| const mismatches: string[] = []; | ||
| for (const [key, expected] of Object.entries(required)) { | ||
| const path = prefix ? `${prefix}.${key}` : key; | ||
| const actual = existing[key]; | ||
| if (expected && typeof expected === 'object' && !Array.isArray(expected)) { | ||
| if (!actual || typeof actual !== 'object' || Array.isArray(actual)) { | ||
| mismatches.push(path); | ||
| continue; | ||
| } | ||
| mismatches.push( | ||
| ...this.diffExtraSettings( | ||
| actual as Record<string, unknown>, | ||
| expected as Record<string, unknown>, | ||
| path, | ||
| ), | ||
| ); | ||
| } else if (actual !== expected) { | ||
| mismatches.push(path); | ||
| } | ||
| } | ||
| return mismatches; | ||
| } |
Comment on lines
+294
to
+320
| protected override async collect(): Promise<AgentActivityEntry[]> { | ||
| const files = await this.discoverSessionFiles(); | ||
| for (const filePath of files) { | ||
| try { | ||
| const stat = await fs.stat(filePath); | ||
| const currentIno = (stat as any).ino as number; | ||
| const stateKey = this.stateKey(filePath); | ||
| const prevState = this.stateStore.get(stateKey); | ||
| const prevRollout = prevState.extra?.minimaxCodeRollout as | ||
| | MinimaxCodeRolloutFileState | ||
| | undefined; | ||
| const prevInode = prevRollout?.inode; | ||
| const prevInodeValid = typeof prevInode === 'number' && prevInode !== 0; | ||
| const rotated = prevInodeValid && prevInode !== currentIno; | ||
| // Seed inode on first sight (or after the 0-sentinel), preserving any | ||
| // turnStepMap state accumulated since the last valid inode. Only | ||
| // real rotation clears turnStepMap. | ||
| if (!prevRollout || !prevInodeValid || rotated) { | ||
| this.stateStore.update(stateKey, { | ||
| extra: { | ||
| minimaxCodeRollout: { | ||
| inode: currentIno, | ||
| turnStepMap: rotated ? {} : (prevRollout?.turnStepMap ?? {}), | ||
| } as MinimaxCodeRolloutFileState, | ||
| }, | ||
| }); | ||
| } |
Comment on lines
+641
to
+650
| const diagnosticRecord: Record<string, unknown> = { | ||
| 'event.name': 'diagnostic', | ||
| time_unix_nano: timestampToUnixNanos(completedAt) ?? timestampToUnixNanos(Date.now()) ?? '0', | ||
| 'gen_ai.agent.type': ClientType.MiniMaxCode, | ||
| 'gen_ai.agent.name': 'MiniMax Code', | ||
| 'gen_ai.session.id': sharedFields.sessionId, | ||
| ...(sharedFields.turnId !== undefined ? { 'gen_ai.turn.id': sharedFields.turnId } : {}), | ||
| ...(sharedFields.stepId !== undefined ? { 'gen_ai.step.id': sharedFields.stepId } : {}), | ||
| 'gen_ai.response.id': sharedFields.responseId, | ||
| ...(sharedFields.traceId ? { trace_id: sharedFields.traceId } : {}), |
Comment on lines
+284
to
+286
| const errMsg = (resultPayload && typeof resultPayload === 'object' && typeof resultPayload.error === 'string') | ||
| ? resultPayload.error | ||
| : (typeof resultPayload === 'string' ? resultPayload : 'tool execution failed'); |
Comment on lines
+221
to
+247
| private diffExtraSettings( | ||
| existing: Record<string, unknown>, | ||
| required: Record<string, unknown>, | ||
| prefix = '', | ||
| ): string[] { | ||
| const mismatches: string[] = []; | ||
| for (const [key, expected] of Object.entries(required)) { | ||
| const path = prefix ? `${prefix}.${key}` : key; | ||
| const actual = existing[key]; | ||
| if (expected && typeof expected === 'object' && !Array.isArray(expected)) { | ||
| if (!actual || typeof actual !== 'object' || Array.isArray(actual)) { | ||
| mismatches.push(path); | ||
| continue; | ||
| } | ||
| mismatches.push( | ||
| ...this.diffExtraSettings( | ||
| actual as Record<string, unknown>, | ||
| expected as Record<string, unknown>, | ||
| path, | ||
| ), | ||
| ); | ||
| } else if (actual !== expected) { | ||
| mismatches.push(path); | ||
| } | ||
| } | ||
| return mismatches; | ||
| } |
Comment on lines
+786
to
+792
| return content | ||
| .filter((c): c is Record<string, unknown> => !!c && typeof c === 'object') | ||
| .map((c): JsonValue => { | ||
| const text = (c['text'] as string | undefined) ?? ''; | ||
| const ctype = (c['type'] as string | undefined) ?? 'text'; | ||
| if (ctype === 'text') return { type: 'text', content: text } as unknown as JsonValue; | ||
| return { type: ctype, content: text } as unknown as JsonValue; |
Collaborator
|
我又补做了一次独立于 PR 实现的 source-probe，并重新核对了当前 HEAD MiniMax Code 3.0.60 实测结果环境：Windows AMD64 ECD，MiniMax Code 提交前后的文件系统增量只有 3 个文件：
以下候选源均无新增或修改：
日志中可以脱敏关联到同一个 session：
这说明 3.0.60 会把 session/turn 的状态元数据写入 对当前 PR source contract 的影响当前代码仍明确写着“MiniMax Code 3.0.60 Windows desktop client writes its native rollout to 另外， 目前真实存在的本地源只有日志：
这些日志不是纯 JSONL，字段嵌在带前缀的日志行里；如果要采用日志方案，需要单独定义增量、轮转、partial-line、脱敏和 content-off 契约。仅靠当前日志也无法证明能恢复完整 prompt/output/tool transcript。 因此建议当前至少做到：
最新 Windows 版本的验证阻塞官方下载配置 当前指向 @zy84338719 你在上一次评论后提交的 Round 35 是 rebase/CI 修正，但还没有回答实际 source 验证问题。请明确确认：你是否曾在真实 MiniMax Desktop 上观察到非零 Hook 原始事件或 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(minimax-code): MiniMax Code probe — agent definition + dual input (hook + rollout)
Summary
Adds support for MiniMax Code (CLI coding agent) to
LoongSuite Pilot, following the same dual-input pattern used by the ZCode probe
(see PR #101). This PR is a multi-round rollout — Round 1 scaffold,
Round 2 turnStepMap + interrupted injection, Round 3 paired entry emission,
Round 4 hook-strategy plumbing, Round 5 flush-trigger + hook stdout polish,
Round 6 empty-output placeholder + explicit interrupted signal,
Round 7 rebase onto latest main, Round 8 Windows E2E feedback
— all changes are complete and pass typecheck + 2783 tests.
What this PR includes
Round 1 — agent definition + dual input scaffold (12 files, +1240 lines)
agents.d/minimax-code.json) — deploys as a Claude-Code-stylehookwith 5 lifecycle events (SessionStart/UserPromptSubmit/PreToolUse/PostToolUse/Stop). Hook protocol mirrors ZCode (PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101) and is expected toalign with MiniMax Code's
~/.minimax-code/settings.jsonshape; final field namesawait MiniMax Code team's official hook protocol spec.
assets/hooks/minimax-code-loongsuite-pilot-hook.sh+-processor.mjs)— fail-open shell wrapper that locates a Node ≥ 18 binary and dispatches to the
processor by kebab-case subcommand. Each subcommand writes a single event_t JSONL
record with
gen_ai.*field names perdocs/agent-onboarding.md.src/inputs/minimax-code-log/minimax-code-log-input.ts) —extends
BaseHookInputand uses the sharedtransformHookRecordso the hookstream and rollout stream normalize into the same entry shape.
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts) —extends
BaseSessionInputand tails~/.minimax-code/rollout/model-io-sess_*.jsonl(one file per session).
src/types/client-type.ts) — addsMiniMaxCode = 'minimax-code'.src/core/orchestrator.ts) — registers both inputs withpre-created log dir and adds
'minimax-code-log'/'minimax-code-rollout'to
LISTENER_AGENT_MAP.docs/overview.md+docs/agents.md+docs/zh-CN/{agents,overview}.mdupdated with the MiniMax Code row.Round 2 — turnStepMap persistence + interrupted injection (5 files, +454 lines)
to
stateStore.extra.minimaxCodeRollout.turnStepMap. Survives restarts. Filerotation (inode change) clears the map.
inode=0sentinel (file appearedafter onStart) seeds inode without clearing the map (CP5 fix pattern from
PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101 zcode-rollout).
completedAtpresent + nofinishReason/
text/toolCalls(typical SIGTERM/timeout pattern). Injectgen_ai.response.finish_reasons=['interrupted']+ placeholdergen_ai.output.messages+ 0 usage. Preventsvalidate-tracefrom flaggingthe LLM span as ERROR for missing
output.messages/finish_reasons/usage.'minimax-code': 'minimax-code'mapping. Withoutthis,
resolveAgentSystem('minimax-code')falls back to'unknown'and theOTLP trace flusher sets
gen_ai.agent.system='unknown', causing ARMS GenAIpipeline routing failures.
VALID_FINISH_REASONSwith'interrupted'and
'cancelled'(mirrors PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101 Step C P0 fix).Round 3 — paired llm.request + llm.response entry emission (10 files, +151/-93)
Promise<AgentActivityEntry | null>toPromise<AgentActivityEntry[]>.processFilespreads the array, so a single source record can yieldmultiple normalized entries. The OTLP trace converter's
pairLlmusesgen_ai.response.idto pair the two events into a single STEP span.qoder-cli-session,qoder-work-trace,qoder-work-log,hermes-log.llm.request+llm.responseentries (sharedtrace_id,session/turn/step,agent.type,response.id,request.id). The request entry carriesgen_ai.input.messages+gen_ai.tool.definitions+time=startedAt;the response entry carries
gen_ai.output.messages+gen_ai.usage.*+gen_ai.response.finish_reasons+time=completedAt. Emitting them astwo separate entries produces a cleaner span tree (matches zcode PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101
shape) and aligns with how the OTLP trace converter's
pairLlmusesgen_ai.response.idto pair request/response events.entries[0]forrequest,
entries[1]for response). Coverage expanded to assert:trace_idparity between request and response entriesrequest.id+response.idparity (OTLP pair key)time_unix_nanoordering (request ≤ response)Round 4 — address reviewer feedback (7 files, +164/-26)
Reviewer state: ralf0131 (maintainer) APPROVED with 4 informational
copilot review rounds. This commit addresses the remaining real
issues identified in those reviews (suppressed comments that
weren't auto-resolved by the Round 1-3 commits).
agents.d/minimax-code.json:28):AgentHookConfigextended withhookContainerPath,extraSettings,hookType,hookTypeRationalefields.
buildHookDefinitions/buildRetiredHookDefinitionshonorhookContainerPath(default['hooks']) so agents with non-standardconfig schemas (ZCode, MiniMax Code nest event arrays under
settings.hooks.events.<event>) get hooks written to the correctJSON path.
applyExtraSettings()deep-merges sibling fields (e.g.settings.hooks.enabled=true). Without this, the Round 1 declarationof
hookContainerPath/extraSettingsinagents.d/minimax-code.jsonwas effectively ignored.
matcherfield when not explicitly set (mirrors the flat-format behavior already
in place at line 144). Some agents (MiniMax Code) reject explicit
matcher: '*'.minimax-code-rollout-input.ts:242-243):requestId/responseIdnow read
record.requestId/record.responseIdat the top levelfirst (canonical location, stable across retries), then fall back to
the nested
request/responseobject, then to a synthetic string.Mirrors PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101 zcode behavior; keeps OTLP pair key stable.
(
minimax-code-rollout-input.ts:72andminimax-code-hook-processor.mjs:32).minimax-code-log-input.test.ts:32andminimax-code-rollout-input.test.ts:199):checkAvailabilitytestsnow mock
directoryExistsviavi.spyOnto avoid depending onwhether
~/.loongsuite-pilot/logs/minimax-codeor~/.minimax-code/rolloutexists on the developer's machine.Round 5 — flush-trigger + hook stdout polish (5 files, +74/-17)
This commit addresses 3 real issues left over from the Round 1-4 review rounds.
ralf0131 (maintainer) LGTM'd at c82ad35; the remaining items are not re-request
blocking, but landing them keeps the diff clean and addresses concrete bugs.
src/flushers/otlp-trace-flusher.ts— add"interrupted"toTERMINAL_FINISH_REASONS. The hook processor'scmdStopemitsgen_ai.response.finish_reasons=["interrupted"]on SIGTERM / Ctrl+C paths totrigger Signal-A immediate flush, but the flusher only recognized
["stop","end_turn","cancelled","error"]— so an interrupted turn waited forturnIdleTimeoutMs(default 30s) before flushing, with stale timestamps onspans already in the buffer.
validate-trace.mjsalready accepts"interrupted"as a legal finish_reason, so this is purely additive andbrings the flusher in line with the OTLP GenAI vocabulary.
"cancelled"wasalready present;
"interrupted"was the missing companion. Benefits anyagent that emits
"interrupted", not just MiniMax Code.assets/hooks/minimax-code-hook-processor.mjs— dispatch restructuredto a
DISPATCHtable wrapped intry/catch/finally, withprocess.stdout.write("{}\n")in the finally block (and on unknownsubcommand). The hook host reads JSON from stdout to apply hook policy
decisions; an empty stdout caused intermittent hangs on some agent hosts.
Mirrors
assets/hooks/claude-code-hook-processor.mjspattern.tests/unit/inputs/edge-cases.test.ts—EdgeSessionInput.processSessionLineoverride updated to return
Promise<AgentActivityEntry[]>(matches Round 3BaseSessionInputmulti-entry refactor). Was still returningPromise<AgentActivityEntry | null>from before the refactor; not caughtat runtime because that test only exercises the empty-file path, but is a
typecheck hazard if tests are type-checked in CI.
tests/unit/inputs/minimax-code-rollout-input.test.ts— the"Round 2: 缺 turnId 时不分配 step.id"test was still asserting the oldsingle-entry shape (
e!["gen_ai.step.id"]).processSessionLinenowreturns
AgentActivityEntry[](Round 3), so the assertion should targetthe response entry at index 1.
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts— fixedthe class-header comment that mis-attributed the multi-entry pair emission
to "Round 4" (it was actually Round 3, per git log). Added a Round 5 entry
describing the flusher + hook stdout fixes for future maintainers.
No new tests for
"interrupted"being terminal — the existinghasTerminalFinishReasonis module-internal, theSetmembership change ispurely additive (no existing test relies on the exact set contents), and
validate-trace.mjsalready validates"interrupted"as a legalfinish_reason, so the upstream contract is consistent.
Round 6 — empty-output placeholder + explicit interrupted signal (4 files, +299/-22)
Round 6 addresses 2 new copilot suppressed comments surfaced after Round 5
landed, both real bugs in code I shipped:
src/inputs/minimax-code-rollout/minimax-code-rollout-input.ts—buildOutputMessagesPreviously returned
undefinedwhen bothtextandtoolCallswereempty, causing the
llm.responseentry to omitgen_ai.output.messagesentirely.
validate-trace.mjssemantic.llm_has_input_outputMUST ruleflagged these as ERROR. Affected cases: model refusals (no text),
length-cap terminations (
output_tokens=0), empty streaming responses.Round 2 only patched the interrupted case; the normal-path empty output
was still a hole. Round 6 fix:
buildOutputMessagesnow always returnsan assistant message carrying the actual
finish_reason(
response.finishReason, defaulting to"stop"). The interrupted caseis preserved by wrapping the response with
finishReason: "interrupted"before calling
buildOutputMessages, so the per-messagefinish_reasonmatches the entry-level
gen_ai.response.finish_reasons=["interrupted"].assets/hooks/minimax-code-hook-processor.mjs—cmdStopinterrupted heuristicRound 1-5 used
isInterrupted = (toolCallCount === 0), whichmis-classified pure-chat sessions (zero tool calls, normal
end_turn)as "interrupted". This was tolerable before Round 5 because the flusher
only recognized
["stop","end_turn","cancelled","error"]as terminal— the false-positive "interrupted" had no effect. Round 5 added
"interrupted"toTERMINAL_FINISH_REASONS, which meant everychat-only turn now triggered Signal-A immediate flush. Round 6 fix:
cmdStopnow reads explicit interruption signals from the hook payload(
event.interrupted | event.isInterrupted | event.is_interrupted) →"interrupted"; (event.cancelled | event.isCancelled | event.is_cancelled) →"cancelled"; no signal →"end_turn"(default).Field naming accepts both camelCase and snake_case pending MiniMax
Code SDK official protocol.
New test suite
tests/unit/hooks/minimax-code/hook-processor.test.mjs(8 tests): end-to-end exercise of
cmdStopsignal resolution viaspawnSync, plus the Round 5 stdout-{}-on-error / unknown-subcommandcontract. The
cancelledtest caught a bug in the first pass of theRound 6 logic where
cancelled:truereturned"interrupted"(signalprecedence was wrong) — fixed by evaluating interrupted and cancelled
as separate signals, with interrupted taking priority.
New rollout test
"Round 6: text+toolCalls 都空但有 finishReason (e.g. length cap) → 始终 emit 占位 output.messages (validate-trace semantic.llm_has_input_output)"— covers the non-interrupted emptyoutput case that the Round 6 fix made possible. The existing Round 2
interrupted test continues to verify the interrupted path works with
the new
buildOutputMessagescontract.Round 7 — rebase onto latest main (HEAD
b0f8599)Per ralf0131's request after the Round 6 approval: rebase onto
maintoresolve the merge conflict (6 new upstream commits since the Round 6 push —
3 doc/agent matrix syncs + 3 installer/updater changes). Conflict resolution
was confined to two doc files (
docs/overview.md+docs/zh-CN/overview.md)where
mainhad added 3 new rows (Kiro CLI / MiMo Code / OpenClaw) beforemy MiniMax Code row — the resolution kept all 4 rows. Pure additive, no
functional change.
mergeable: MERGEABLE. ralf0131 re-approved atb0f8599; no new review round requested unless conflict resolutionintroduced non-trivial changes (it did not).
Round 8 — fangxiu-wf's 6 blocking Windows E2E findings (HEAD
97c1099)After Round 7 was approved, fangxiu-wf ran the merge result against the
official MiniMax Code 3.0.60 Windows desktop client on a dedicated Windows
cloud desktop. The real Agent task succeeded, but Pilot collected no data —
deployed-agents.jsonstayed empty, the hook was never injected, andzero matches showed up in the configured SLS backend or ARMS pipeline.
This commit addresses all 6 blocking findings, grouped by P0/P1/P2
priority buckets. Diff: 15 files, +905/-136.
P0 — independent, verified locally
#3 Restart recovery drops uncollected rollout records
MinimaxCodeRolloutInput.onStart()now seedsoffset = stat.sizeonlyfor files without a prior checkpoint. Files with a persisted
extra.minimaxCodeRolloutstate (or anystateStorecheckpoint thatrecorded a non-zero offset) are left alone — the next
collect()cycleresumes from the saved offset and recovers any records appended while
Pilot was stopped. Without this fix, a normal restart would silently
drop in-flight rollout records.
"Round 8: onStart 不重置已有 offset"exercisesthe in-between offset scenario (file existed at onStart time, has
persisted checkpoint, has bytes appended while Pilot was stopped).
#6 Object-shaped tool results can be falsely marked as errors
assets/hooks/minimax-code-hook-processor.mjscmdPostToolUsenowhonors
toolResult.statuswhen present, mapstoolResult.exitCodeto success/error, and otherwise defaults to
"success". A{content: "ok"}payload is no longer mis-reported as a failed toolcall. The previous logic treated "no
isErrorflag and nostatus/exitCode" as an implicit error, which is a false positive.{content:"ok"},{exitCode:1},{status:"partial"},and
isError=trueend-to-end viaspawnSync.TMPDIR / copilot suppressed comment
tests/unit/inputs/minimax-code-log-input.test.ts: TMPDIR is nowcreated in
beforeEachand removed inafterEach(per-test freshdir), matching the rollout-input test isolation pattern. The previous
module-scope TMPDIR would break as soon as a later test wrote files
under it.
P1 — needs reviewer real-world alignment
#1 The declared product contract does not match the official Windows client
agents.d/minimax-code.jsondetection paths now include~/.minimax-agent-cn(per the reviewer's sanitized E2E description)and the Windows-specific
%APPDATA%/MiniMax/%APPDATA%/MiniMax-Codelocations (the live Windows client writes its native data under
%APPDATA%\MiniMax).AgentHookConfigfields:settingsPathWindows,hookCommandWindows,sessionDirWindows. The hook strategy(
src/deployment/hook-strategy.ts) now resolves the platform-specificsettings path and hook command at deploy time, so a single agent
definition produces the right artifact on both POSIX and Windows
hosts without per-platform duplication.
reviewer's description; the exact rollout sub-folder name
(e.g.
sessions/vsrollout/) may need a tweak once we have aMiniMax Code 3.0.60 Windows client to inspect. The agent def now
exposes the detection paths so they can be adjusted in one place.
#2 The Windows hook executable is missing
assets/hooks/minimax-code-loongsuite-pilot-hook.ps1(121 lines)is a 1:1 port of
claude-code-loongsuite-pilot-hook.ps1and mirrorsthe .sh's 5-subcommand dispatch. CLM/WDAC-safe stdin passthrough,
Node ≥ 18 resolution across nvm/fnm/volta. PowerShell host will
now find the script when
AgentDefLoaderrewrites the .sh path to.ps1on Windows.P2 — source-fidelity / lifecycle hygiene
#4 Several output fields are synthesized rather than source-faithful
buildOutputMessagesnow returnsundefinedwhen bothtextandtoolCallsare empty (instead of synthesizing a placeholder assistantmessage). The
llm.responseentry is still emitted, butgen_ai.output.messagesis absent — honest signal that the sourcehad no observable content.
resolveFinishReasonsreturnsstring[] | undefined(no['stop']default). Missing
finishReasonstays missing, not synthesized.optionalUsageField()helper threads throughgen_ai.usage.*—fields absent in the source stay absent in the output.
detectInterruptedResponseheuristic. A response is no longerclassified as
"interrupted"based oncompletedAt+ emptyfinishReason— that pattern is also valid for in-flight responsesthat haven't finished writing yet.
buildIncompleteResponseDiagnostic()helper emits a separateevent.name = "diagnostic"entry with attributesgen_ai.diagnostic.{reason, missing_fields, has_text, has_tool_calls, has_finish_reason}whenever a response is structurally incomplete(
!hasText && !hasToolCalls && missing.includes('finishReason')).Strict threshold avoids false positives on length-cap terminations
(which still emit normal output with
finish_reason="length").AgentEventNameunion extended with'diagnostic';normalizeEventNameinentry-builder.tsmaps it to itself (not'other').#5 Hook lifecycle health is incomplete
needsDeploy()now also callsneedsExtraSettingsRepair()whichvalidates each declared
extraSettingsleaf value (e.g.settings.hooks.enabled === true). A disabled required flag isreported unhealthy and triggers re-deploy.
undeploy()now callsremoveExtraSettings()to roll back the exactextraSettingsleaves we wrote. Conservative: only deletes leafvalues that exactly match what
applyExtraSettingswould havewritten, preserving any user-set values that happen to be on the same
JSON path. Without this, repeated deploy/undeploy cycles would leave
the agent's
settings.jsonin an unknown state.Installer
deploy/installer-opensource.shanddeploy/installer-opensource.ps1:Remove-HookConfigslists now include MiniMax Code POSIX + Windowssettings paths (
~/.minimax-code/settings.json,%APPDATA%\MiniMax\settings.json,%APPDATA%\MiniMax-Code\settings.json),so the uninstaller actually removes them. Previously the installer
silently skipped MiniMax Code.
Tests
tests/unit/hooks/minimax-code/hook-processor.test.mjs: 12 tests(4 new tool-result status tests + 8 cmdStop signal tests from
Round 6).
tests/unit/utils/fs-utils.test.ts: 6 new env-var expansion tests(Windows
%APPDATA%, POSIX$HOMEand${USER}).tests/unit/inputs/minimax-code-log-input.test.ts: TMPDIR per-testrefactor.
tests/unit/inputs/minimax-code-rollout-input.test.ts: 24 tests —added Round 8 diagnostic, length-cap, onStart resume,
finish_reasons-undefined tests; updated Round 2 interrupted case to
Round 8 incomplete (no longer synthesizes
interrupted); updatedRound 6 placeholder case to Round 8 empty (returns undefined);
updated finish_reasons default to undefined.
Test coverage
Why dual input (hook + rollout)
Mirrors PR #101's design choice. A single hook stream cannot give per-LLM
llm.request/llm.responseevent pairs (the hook only fires on lifecycleboundaries, not per model step), so we tail the rollout JSONL to backfill
per-LLM evidence. Per
docs/agent-onboarding.md#reliable-hybrid-collection,the hook is the lifecycle source of truth and the rollout is the per-LLM
semantic source — the input precedence is explicit and tested.
What this PR does NOT include (deferred / future work)
synthesizeOrphanToolRecordsflusher enhancement — PR feat(zcode): ZCode probe + Step C fixes + Round 2 interrupted-path injection (AGE-593) #101 added azcode-specific function for synthesizing orphan tool.result records when
the PostToolUse hook fails to fire. The MiniMax Code equivalent is
deferred until we have real E2E traces showing the orphan case; the
current hook + rollout hybrid collection path keeps the orphan window
narrow.
synthetic JSONL shaped to the documented
type: 'model-io'schema.Real fixture data is added once MiniMax Code's plugin SDK / rollout
shape is publicly documented.
match the reviewer's description (
%APPDATA%\MiniMax+~/.minimax-agent-cn),but the rollout sub-folder name (
sessions/vsrollout/) isbest-guess and may need a tweak once we have a MiniMax Code 3.0.60
Windows client to inspect. The agent def now exposes the detection
paths so they can be adjusted in one place.
Verification
npm run typecheck— 0 errornpm run build— cleannpm test— 2783 passed, 0 failed, 0 flakesnpx vitest run tests/unit/inputs/minimax-code-*.test.ts— 40 passednpx vitest run tests/unit/hooks/minimax-code/hook-processor.test.mjs— 12 passednpx vitest run tests/unit/utils/fs-utils.test.ts— 6 env-var tests passednpx vitest run tests/unit/inputs/base-session-input.test.ts— passeswith the multi-entry return type
node -c assets/hooks/minimax-code-hook-processor.mjs— syntax OKassets/hooks/minimax-code-loongsuite-pilot-hook.ps1— structuremirrors the claude-code .ps1 template; syntax checked via PowerShell
AST parse if available (N/A on macOS dev env)
Out of scope (waiting on MiniMax Code team)
sessionIdvssession_id,toolCallIdvstool_use_id, etc.). The hook processor accepts bothshapes (camelCase + snake_case). Final field names will be updated once
MiniMax Code's official hook protocol is published.
review iterations before landing; this PR is now at Round 8 of that
arc and is expected to iterate similarly. The Round 8 Windows data
path and
.ps1artifact are best-guess based on the reviewer'sdescription; once the official Windows client is available the
detection paths can be tightened in one place.
🤖 Generated with MiniMax Code