Skip to content

Commit 4876ef1

Browse files
committed
fix(mcode-island): a detector state write clears a stale sub-step (round-20 #5)
Live symptom: the pill sat on "step 1/1" showing a curl command from a tool call that finished several turns earlier, and nothing ever cleared it. Cause: round-19 #2 made Write-Status merge the previous step/total/detail back in on every write, which is correct for the two metadata refreshes (60s 5h-usage, todo progress) because those restate the current state rather than announcing a new step. But the detector also writes on state inference -- it read a new tool call out of the session log and is asserting "the agent is doing THIS now". That message has no relationship to whatever sub-step a hook pushed last, so it inherited a counter that had no meaning there. Every subsequent write inherited it again, which is why it never cleared. Write-Status now takes -KeepSubStep: - state inference (default) resets step/total/detail to -1/-1/"" - the 5h-usage and todo refreshes pass -KeepSubStep and preserve The two paths are distinguished explicitly by the caller, not inferred from the message text. Distinguishing is necessary because the state-inference call also has to be able to win the settle race against an agent push (round-19), so "who wrote last" is not a usable signal. Tests ----- New section 5 in test-substep-progress.mjs, negative-first: observed failing on 3 assertions before the fix, all passing after. - Write-Status exposes -KeepSubStep - state-inference write clears a stale sub-step (-1|-1|) - metadata-refresh write still preserves it - the inference call site does NOT pass -KeepSubStep - exactly 2 call sites DO (5h-usage, todo) The call-site assertions are the ones that matter: a switch nobody passes is the dead-code shape, and a test that only exercises the function body would not notice. Two harness repairs, both regressions from round-20 that this work surfaced: - Format-ToolSummary now calls Protect-SecretText, which lives in a separate lib. The harness extracted the function body and ran it standalone, so it died on an unresolvable command rather than on an assertion. The harness now dot-sources the lib, as _lib.ps1 does. This is a genuine break of an existing test that round-20 shipped; smoke did not catch it because smoke is static and this suite is not. - Section 3 anchored its extraction regex on `$prev` appearing before `$detailField`. Wrapping the block in `if ($KeepSubStep)` inverted that order and the regex silently stopped matching, which would have been reported as "Write-Status preserves sub-step fields: could not find the block" -- a confusing failure for a function that works. It now anchors on the initialisers and the payload construction. Validation: sub-step suite 32 pass / 0 fail, smoke 116 pass / 0 fail. Detector re-verified under Windows PowerShell 5.1 (exit 0) after the restart, since that is the interpreter this runs under in production.
1 parent df3c14d commit 4876ef1

2 files changed

Lines changed: 139 additions & 21 deletions

File tree

‎plugins/antianqi/mcode-island/mcode-status-detect.ps1‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ function Infer-State($msg) {
580580
return $null
581581
}
582582

583-
function Write-Status($state, $message, $family) {
583+
function Write-Status($state, $message, $family, [switch]$KeepSubStep) {
584584
$tmp = "$statusFile.tmp"
585585
# usage5h:0..100 表示"剩余"百分比(不是已用!);null = 未知/未拉到
586586
# usage5hResetMs:距下次刷新的毫秒数;null = 未知
@@ -592,17 +592,31 @@ function Write-Status($state, $message, $family) {
592592
$todoCnt = if ($null -ne $script:plan5hTodoData) { ("{0}/{1}" -f $script:plan5hTodoData.completed, $script:plan5hTodoData.total) } else { $null }
593593
$famField = if ($family) { [string]$family } else { $null }
594594
# step / total / detail 是 agent(hook)推的 sub-step 状态,不是 detector
595-
# 推的。detector 每次刷 5h 用量或 todo 进度都会重写整个 payload,如果这里
596-
# 不把现有值读回来合并,正常使用中的 sub-step 进度会被静默清零
597-
# (round-19 review hetaoBackend #2)。读-改-写,不是覆盖。
598-
$prev = Read-StatusObj
595+
# 推的。detector 重写整个 payload 时,这里怎么处理取决于"为什么写"。
596+
#
597+
# 两种写入语义完全不同(round-20 #5):
598+
#
599+
# - 元数据刷新(-KeepSubStep):每 60s 刷 5h 用量、每次刷 todo 进度。
600+
# 这两次的 state/message 是从当前 status.json 读回来原样再写回去的,
601+
# 不代表"agent 进入了新的一步",所以必须把已有的 sub-step 合并回来,
602+
# 否则正常使用中的进度会被静默清零(round-19 #2)。
603+
#
604+
# - 状态推断(默认):detector 从 session log 推出一个新状态并写入。
605+
# 这条消息的语义是"agent 现在在干这个",它和上一条 hook 推的
606+
# sub-step 没有任何承接关系。无条件合并会让 pill 一直挂着上一步的
607+
# 计数:实测里 agent 早已换工具、甚至这一轮都结束了,pill 还卡在
608+
# "step 1/1" 显示一条早就跑完的 curl 命令,而且没有任何后续写入会
609+
# 把它清掉。所以状态推断一律重置。
599610
$stepField = -1
600611
$totalField = -1
601612
$detailField = ''
602-
if ($prev) {
603-
if ($prev.PSObject.Properties['step']) { $stepField = [int]$prev.step }
604-
if ($prev.PSObject.Properties['total']) { $totalField = [int]$prev.total }
605-
if ($prev.PSObject.Properties['detail'] -and $null -ne $prev.detail) { $detailField = [string]$prev.detail }
613+
if ($KeepSubStep) {
614+
$prev = Read-StatusObj
615+
if ($prev) {
616+
if ($prev.PSObject.Properties['step']) { $stepField = [int]$prev.step }
617+
if ($prev.PSObject.Properties['total']) { $totalField = [int]$prev.total }
618+
if ($prev.PSObject.Properties['detail'] -and $null -ne $prev.detail) { $detailField = [string]$prev.detail }
619+
}
606620
}
607621
$payload = [PSCustomObject]@{
608622
state = $state
@@ -808,7 +822,8 @@ try {
808822
$sForU = if ($curForUsage) { [string]$curForUsage.state } else { $S_IDLE }
809823
$mForU = if ($curForUsage) { [string]$curForUsage.message } else { '' }
810824
$fForU = if ($curForUsage -and $curForUsage.PSObject.Properties['family']) { $curForUsage.family } else { $null }
811-
Write-Status $sForU $mForU $fForU
825+
# 元数据刷新:原样写回当前状态,必须保留 sub-step(-KeepSubStep)
826+
Write-Status $sForU $mForU $fForU -KeepSubStep
812827
Log-Line ("5h usage refreshed: remaining=" + $curPct + "% resetMs=" + $curMs)
813828
}
814829

@@ -822,7 +837,8 @@ try {
822837
$sForT = if ($curForTodo) { [string]$curForTodo.state } else { $S_IDLE }
823838
$mForT = if ($curForTodo) { [string]$curForTodo.message } else { '' }
824839
$fForT = if ($curForTodo -and $curForTodo.PSObject.Properties['family']) { $curForTodo.family } else { $null }
825-
Write-Status $sForT $mForT $fForT
840+
# 元数据刷新:原样写回当前状态,必须保留 sub-step(-KeepSubStep)
841+
Write-Status $sForT $mForT $fForT -KeepSubStep
826842
$script:plan5hLastWrittenTodoPct = $curTodoPct
827843
if ($null -ne $curTodoData) {
828844
Log-Line ("todo refreshed: " + $curTodoData.completed + "/" + $curTodoData.total + " = " + $curTodoPct + "%")

‎plugins/antianqi/mcode-island/scripts/test-substep-progress.mjs‎

Lines changed: 112 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,13 @@ if (!PS_BIN) {
139139
const libPath = join(PLUGIN_ROOT, 'io.minimax.mcode', 'hooks', 'scripts', '_lib.ps1');
140140
const lib = readFileSync(libPath, 'utf8');
141141

142+
// Format-ToolSummary calls Protect-SecretText, which lives in a
143+
// separate lib (round-20). Extracting the function body alone would
144+
// leave the call unresolvable and the harness would die on an
145+
// undefined command rather than on a real assertion -- so the harness
146+
// dot-sources the lib, exactly as _lib.ps1 does in production.
147+
const protectLibPath = join(PLUGIN_ROOT, 'scripts', 'lib', 'Protect-Text.ps1');
148+
142149
let formatToolSummary;
143150
try {
144151
formatToolSummary = extractFn(lib, 'Format-ToolSummary');
@@ -151,6 +158,7 @@ if (!PS_BIN) {
151158

152159
const harness = `
153160
$ErrorActionPreference = 'Stop'
161+
. '${protectLibPath.replace(/'/g, "''")}'
154162
${formatToolSummary}
155163
$evt = [PSCustomObject]@{
156164
tool_name = 'mcode-computer-use'
@@ -179,6 +187,7 @@ Format-ToolSummary $evt
179187
// the redaction could have been implemented by disabling the branch.
180188
const coordHarness = `
181189
$ErrorActionPreference = 'Stop'
190+
. '${protectLibPath.replace(/'/g, "''")}'
182191
${formatToolSummary}
183192
$evt = [PSCustomObject]@{
184193
tool_name = 'mcode-computer-use'
@@ -300,29 +309,33 @@ if (PS_BIN) {
300309

301310
const writeStatusFn = extractFn(detect, 'Write-Status');
302311

303-
// Extract just the sub-step preservation lines as a standalone repro of
304-
// the logic, so the test does not need the detector's whole environment.
305-
const preserveMatch = writeStatusFn.match(
306-
/\$prev = Read-StatusObj[\s\S]*?\$detailField = ''[\s\S]*?if \(\$prev\)\s*\{[\s\S]*?\n\s*\}/);
312+
// Extract the sub-step decision block as a standalone repro of the
313+
// logic, so the test does not need the detector's whole environment.
314+
//
315+
// The block is bounded by two stable statements rather than by the
316+
// internal ordering of the read-modify-write: round-20 #5 wrapped it
317+
// in `if ($KeepSubStep) { ... }`, so a regex anchored on `$prev`
318+
// appearing before `$detailField` silently stopped matching. Anchor on
319+
// the initialisers and the payload construction instead, and drive the
320+
// switch explicitly -- this section pins the PRESERVE half; section 5
321+
// pins the reset half and the call-site wiring.
322+
const preserveMatch = writeStatusFn.match(/\$stepField\s*=\s*-1[\s\S]*?\$payload\s*=/);
307323

308324
if (!preserveMatch) {
309325
bad('Write-Status preserves sub-step fields',
310-
'could not find the step/total/detail read-modify-write block in Write-Status');
326+
'could not find the step/total/detail decision block in Write-Status');
311327
} else {
312328
ok('Write-Status preserves sub-step fields');
313329

314330
const harness = `
315331
$ErrorActionPreference = 'Stop'
316332
$statusFile = $env:ISLAND_TEST_STATUS
333+
$KeepSubStep = $true
317334
function Read-StatusObj {
318335
if (!(Test-Path $statusFile)) { return $null }
319336
try { return ([System.IO.File]::ReadAllText($statusFile) | ConvertFrom-Json) } catch { return $null }
320337
}
321-
$prev = Read-StatusObj
322-
$stepField = -1
323-
$totalField = -1
324-
$detailField = ''
325-
${preserveMatch[0].split('\n').slice(1).join('\n')}
338+
${preserveMatch[0].replace(/\$payload\s*=$/, '')}
326339
"$stepField|$totalField|$detailField"
327340
`;
328341
const statusFile = join(TMP, 'status.json');
@@ -376,6 +389,95 @@ ${preserveMatch[0].split('\n').slice(1).join('\n')}
376389
}
377390
}
378391

392+
// ---------------------------------------------------------------------------
393+
// 5. A detector state-inference write must CLEAR a stale sub-step
394+
// (round-20 #5)
395+
// ---------------------------------------------------------------------------
396+
//
397+
// Section 3 pins the other half of the same contract: a metadata refresh
398+
// must PRESERVE step/total/detail, because the 60s 5h-usage rewrite and
399+
// the todo rewrite restate the current state rather than announcing a new
400+
// step. Preserving unconditionally is only half right.
401+
//
402+
// A detector state-inference write is different in kind. It read a new
403+
// tool call out of the session log and is asserting "the agent is doing
404+
// THIS now". That message has no relationship to whatever sub-step a hook
405+
// pushed last, so inheriting step/total/detail leaves the previous turn's
406+
// counter on the pill indefinitely. Observed live: the pill sat on
407+
// "step 1/1" showing a curl command from a finished tool call, through
408+
// several subsequent turns, because nothing ever cleared it.
409+
//
410+
// The two write paths must therefore be distinguished explicitly, not by
411+
// guessing from the message text.
412+
{
413+
const detectPath = join(PLUGIN_ROOT, 'mcode-status-detect.ps1');
414+
const detect = readFileSync(detectPath, 'utf8');
415+
const writeStatusFn = extractFn(detect, 'Write-Status');
416+
417+
const hasSwitch = /function\s+Write-Status[\s\S]*?\[switch\]\$KeepSubStep/.test(writeStatusFn);
418+
check('Write-Status exposes a -KeepSubStep switch',
419+
hasSwitch,
420+
'state-inference and metadata-refresh writes are not distinguished, so a stale sub-step can never be cleared');
421+
422+
// Anchor on the two stable statements either side of the decision so
423+
// the extraction survives reformatting inside the block.
424+
const block = writeStatusFn.match(/\$stepField\s*=\s*-1[\s\S]*?\$payload\s*=/);
425+
426+
if (!hasSwitch || !block) {
427+
bad('detector state write clears a stale sub-step',
428+
'could not extract the sub-step decision block from Write-Status');
429+
} else {
430+
ok('detector state write clears a stale sub-step');
431+
432+
const harness = `
433+
$ErrorActionPreference = 'Stop'
434+
$statusFile = $env:ISLAND_TEST_STATUS
435+
$KeepSubStep = [bool]::Parse($env:ISLAND_TEST_KEEP)
436+
function Read-StatusObj {
437+
if (!(Test-Path $statusFile)) { return $null }
438+
try { return ([System.IO.File]::ReadAllText($statusFile) | ConvertFrom-Json) } catch { return $null }
439+
}
440+
${block[0].replace(/\$payload\s*=$/, '')}
441+
"$stepField|$totalField|$detailField"
442+
`;
443+
444+
const statusFile = join(TMP, 'status.json');
445+
const run = (keep) => runPs(
446+
`$env:ISLAND_TEST_STATUS = '${statusFile.replace(/'/g, "''")}'\n` +
447+
`$env:ISLAND_TEST_KEEP = '${keep ? 'True' : 'False'}'\n` + harness).trim();
448+
449+
// Previous status carries a finished sub-step from an agent push.
450+
const stale = {
451+
state: 'done', message: 'Bash ok', family: 'shell',
452+
step: 1, total: 1, detail: 'curl -H "Authorization: Bearer <redacted>"',
453+
source: 'agent',
454+
};
455+
456+
writeFileSync(statusFile, JSON.stringify(stale), 'utf8');
457+
eq('state-inference write clears the stale sub-step', run(false), '-1|-1|');
458+
459+
writeFileSync(statusFile, JSON.stringify(stale), 'utf8');
460+
eq('metadata-refresh write (-KeepSubStep) still preserves it',
461+
run(true), '1|1|curl -H "Authorization: Bearer <redacted>"');
462+
}
463+
464+
// The call sites must actually differ. A switch nobody passes is the
465+
// dead-code shape this suite exists to catch.
466+
const callSites = detect.split('\n')
467+
.map((l, i) => ({ l, n: i + 1 }))
468+
.filter(({ l }) => /^\s*Write-Status\s+\$/.test(l));
469+
const keeps = callSites.filter(({ l }) => /-KeepSubStep/.test(l));
470+
const infers = callSites.filter(({ l }) => /\$inferred\.state/.test(l));
471+
472+
eq('Write-Status has 3 call sites', callSites.length, 3);
473+
check('the state-inference call site does NOT pass -KeepSubStep',
474+
infers.length === 1 && !/-KeepSubStep/.test(infers[0].l),
475+
`inference call site must clear stale sub-step (line ${infers[0]?.n})`);
476+
check('the 5h-usage and todo call sites DO pass -KeepSubStep',
477+
keeps.length === 2,
478+
`expected 2 -KeepSubStep call sites, found ${keeps.length}`);
479+
}
480+
379481
// ---------------------------------------------------------------------------
380482
// summary
381483
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)