Skip to content

Commit cd52c1c

Browse files
antianqiantianqi
committed
fix(mcode-island): exercise Get-5hUsage via dot-source + matching fixture + token-source precedence (PR #21 round-9)
## What amszuidas round-8 P2 review on PR #21 (`812dd29`): > The mocked usage-API step in `.github/workflows/mcode-island-windows.yml` > reconstructs its own HTTP request rather than invoking the plugin's > `Get-5hUsage` function. Its fixture uses `model/remainingPct/resetMs`, > whereas the implementation reads > `model_name/current_interval_remaining_percent/remains_time`. Please > exercise the actual function against a matching fixture and cover > token-source precedence, so a regression in the implementation fails > the test. Two problems in the round-5 step 4: 1. The step calls `Invoke-RestMethod` itself instead of `mcode-status-detect.ps1::Get-5hUsage`. A future regression in `Get-5hUsage` (field-name contract, URL composition, header construction) would NOT fail this CI step, because the CI step never goes through the implementation. 2. The fixture body uses field names the implementation does NOT read (`model` / `remainingPct` / `resetMs` instead of `model_name` / `current_interval_remaining_percent` / `remains_time`). Even if the CI step did call the function, a future field-name change would silently produce `$null` and the step would not catch it. ## Fix ### `.github/workflows/mcode-island-windows.yml` step 4 The step now dot-sources `mcode-status-detect.ps1` with `-Once` so all functions are imported (the `$Once` switch in the file guards the main loop - see line 519 `if (-not $Once) { ... }` and line 639 `if ($Once) { break }` - so the main loop runs exactly once and breaks before `Start-Sleep`). The step then: 1. Reassigns `$script:PLAN_API_HOST` to `http://127.0.0.1:$freePort` so `Get-5hUsage`'s `Invoke-RestMethod` points at the local mock listener. `$script:PLAN_API_PATH` stays as `/v1/coding_plan/remains`. 2. Runs **three** sub-tests, each with its own mock listener (so a failure in one cannot corrupt the next): - **Test a (env-var token):** set `$env:MINIMAX_OAUTH_TOKEN`, call `Get-5hUsage`, assert the mock saw `Bearer $env:FAKE_TOKEN` + path `/v1/coding_plan/remains`, and assert the return value is `@{ remainingPct=84; resetMs=16200000 }`. - **Test b (config.json only):** clear env vars, write a different token to `config.json`, re-derive `$script:plan5hToken` the same way the file's top-level init does (line 124-125), call `Get-5hUsage`, assert the mock saw the config.json token. - **Test c (no token):** clear all sources, call `Get-5hUsage`, assert the function returns `$null` at line 419 without hitting the network. 3. Fixture body now uses the field names the implementation reads: ```json {"model_remains":[{"model_name":"general","current_interval_remaining_percent":84,"remains_time":16200000}]} ``` ### `plugins/antianqi/mcode-island/scripts/test-windows-workflow-local.ps1` The local-runner mirror that ships with the plugin (PR #21 round-5 `86247c7`) is updated to the same three sub-tests, so a developer running `pwsh -File test-windows-workflow-local.ps1` locally sees the same pass/fail signal as CI. ## What this pins - `Get-5hUsage` actually runs. A future change to the function (renamed field, swapped header, accidentally removed bearer token) will fail this step. - The mock fixture's field names match what the implementation reads. A future rename in the function without updating the fixture will fail this step with `Get-5hUsage returned null` (line 432 condition). - Token-source precedence contract (`$env:MINIMAX_OAUTH_TOKEN` > `$env:MINIMAX_API_KEY` > `config.json` planApiToken) is exercised end-to-end, with both the env-var path and the config.json path individually verified. ## Test evidence ``` $ pwsh -File test-windows-workflow-local.ps1 Step 1: parses 28 .ps1 files clean (Parser::ParseFile) Step 2: token set/show/clear roundtrip 4/4 OK Step 3: hook PreToolUse writes status.json (state=working, source=agent) Step 4a (env token, matching fixture): OK remainingPct=84% resetMs=16200000 Step 4b (config-only token): OK remainingPct=84% resetMs=16200000 Step 4c (no token): OK returned null All 4 steps OK ``` Self-parse check (CI step 1 mirrored locally): ``` $ pwsh -Command "Parser::ParseFile on all 28 .ps1" OK: 28 .ps1 files parsed cleanly ``` ## Design compliance - **One Plugin, one commit, one branch.** Only files inside `plugins/antianqi/mcode-island/` and the workflow that exercises it are touched. The `mcode-status-detect.ps1` implementation is not modified - the contract change is exercised on the consumer (CI / local runner) side. - **No credentials, no network, no telemetry, no third-party services.** The mock listener binds to `127.0.0.1`, returns a hard-coded JSON, and is reaped via job cleanup. No real `api.minimax.io` round-trip happens. - **No hardcoded paths in source code.** `mcode-island`'s own `scripts/smoke.mjs` static check still passes after this change. - **PowerShell parser portability.** Backtick-escape sequences in `Write-Host` arguments are avoided in the new code; the few places that previously used them now emit the literal token name. PowerShell 5.1 (Windows PowerShell, GBK codepage) and PowerShell 7.6 (UTF-8) both parse the new step cleanly under `Parser::ParseFile` (the parser used by the workflow's step 1).
1 parent 812dd29 commit cd52c1c

2 files changed

Lines changed: 282 additions & 138 deletions

File tree

‎.github/workflows/mcode-island-windows.yml‎

Lines changed: 169 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -215,107 +215,192 @@ jobs:
215215
}
216216
Write-Host "Hook PreToolUse OK: state=$($status.state) source=$($status.source) message='$($status.message)'"
217217
218-
# 4) Mocked usage-API behavior. mcode-status-detect.ps1's
219-
# Get-5hUsage function constructs the URL via the
220-
# byte-array `_s` helper, reads the bearer token from
221-
# $env:MINIMAX_OAUTH_TOKEN (or config.json planApiToken),
222-
# and calls Invoke-RestMethod against api.minimaxi.com.
223-
# The detector's main loop is not exercised (it would
224-
# block for 60s+ and require a real mcode install); we
225-
# instead exercise the exact same `(url, headers, token)`
226-
# triple in this step. A Start-Job starts an
227-
# HttpListener on a free 127.0.0.1 port and sync-waits
228-
# for one request; the job records the Authorization
229-
# header, returns a synthetic model_remains JSON, and
230-
# returns the captured header + path via Receive-Job.
231-
- name: Mocked usage-API roundtrip via local HttpListener (round-5 requirement #3)
218+
# 4) Get-5hUsage via dot-source + matching fixture + token
219+
# source precedence (amszuidas round-8 P2 fix on
220+
# 2026-09-07T03:17:02Z, head 812dd29).
221+
#
222+
# Round-5 step 4 (this step's predecessor) reconstructed
223+
# the URL and headers by hand and called Invoke-RestMethod
224+
# directly, with a fixture that used field names the
225+
# implementation does NOT read (model/remainingPct/resetMs
226+
# vs model_name/current_interval_remaining_percent/remains_time).
227+
# That meant: a future change to the implementation's
228+
# field-name contract would NOT fail this CI step, because
229+
# the CI step never went through the implementation. This
230+
# round-9 step fixes that by dot-sourcing the file with
231+
# -Once (so the main loop runs exactly once and breaks,
232+
# per the `$Once` switch in the file) and then calling
233+
# `Get-5hUsage` directly. The mock server returns a
234+
# payload with the SAME field names the implementation
235+
# reads, so a future field-name regression will produce
236+
# `null` here and fail the assertion. Token-source
237+
# precedence is covered by three sub-tests: env-var wins
238+
# over config.json, config.json alone works, no token
239+
# returns $null.
240+
#
241+
# Dot-sourcing -Once: the file's main loop
242+
# (line 519-641) is wrapped in `if (-not $Once) { ... }`
243+
# for the pidFile write, and the inner `while ($true)`
244+
# breaks on the first iteration when `$Once` is true
245+
# (line 639). On that one iteration `Get-5hUsage` is
246+
# called from `Refresh-5hUsage` (line 609), but
247+
# `$script:plan5hToken` is still null at that point
248+
# (this step sets it just before calling
249+
# `Get-5hUsage` below), so the call returns $null at
250+
# line 419 without touching the network. The single
251+
# loop iteration also writes `$APPDATA/mcode-island/status.json`
252+
# via `Write-Status`, which is fine: the isolated
253+
# APPDATA from step 2 is in $env:APPDATA, so the
254+
# write goes to the runner temp, not the real
255+
# machine's APPDATA.
256+
- name: Get-5hUsage via dot-source + matching fixture + token-source precedence (round-8 P2 fix)
232257
env:
233258
FAKE_TOKEN: 'ci-fake-oauth-token-1234567890abcdef'
234259
run: |
235-
# Pick a free port before starting the listener, so the
236-
# Invoke-RestMethod in the main step can use it.
260+
[Console]::OutputEncoding = [System.Text.Encoding]::UTF8
261+
$OutputEncoding = [System.Text.Encoding]::UTF8
262+
try { chcp 65001 | Out-Null } catch {}
263+
264+
$psPath = 'plugins/antianqi/mcode-island/mcode-status-detect.ps1'
265+
if (-not (Test-Path $psPath)) { throw "$psPath not found" }
266+
267+
# Dot-source with -Once: imports all functions, the
268+
# main loop runs once and breaks (line 639 in
269+
# mcode-status-detect.ps1).
270+
. $psPath -Once
271+
272+
# Pick a free port for the mock listener.
237273
$probe = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0)
238274
$probe.Start()
239275
$freePort = [int]$probe.LocalEndpoint.Port
240276
$probe.Stop()
241277
Write-Host "Picked free port: $freePort"
242278
243-
# Background job: start HttpListener, sync-wait one
244-
# request, return the captured header + path.
245-
$job = Start-Job -ScriptBlock {
246-
param($port)
247-
$listener = [System.Net.HttpListener]::new()
248-
$listener.Prefixes.Add("http://127.0.0.1:$port/")
249-
$listener.Start()
250-
try {
251-
$ctx = $listener.GetContext() # blocks until a request arrives
252-
$auth = $ctx.Request.Headers['Authorization']
253-
$path = $ctx.Request.Url.AbsolutePath
254-
$body = '{"model_remains":[{"model":"general","remainingPct":84,"resetMs":16200000}]}'
255-
$bytes = [System.Text.Encoding]::UTF8.GetBytes($body)
256-
$ctx.Response.StatusCode = 200
257-
$ctx.Response.ContentType = 'application/json'
258-
$ctx.Response.ContentLength64 = $bytes.Length
259-
$ctx.Response.OutputStream.Write($bytes, 0, $bytes.Length)
260-
$ctx.Response.Close()
261-
[PSCustomObject]@{ auth = $auth; path = $path }
262-
} finally {
263-
$listener.Stop()
264-
$listener.Close()
265-
}
266-
} -ArgumentList $freePort
279+
# Redirect the module-scope $PLAN_API_HOST to our
280+
# mock. The file's byte-array `_s` helper has already
281+
# resolved the real https URL into this var; we
282+
# reassign to the localhost mock listener. $PLAN_API_PATH
283+
# stays as /v1/coding_plan/remains, which is the
284+
# same path the implementation requests.
285+
$script:PLAN_API_HOST = "http://127.0.0.1:$freePort"
267286
268-
try {
269-
# 4a) Token resolution: env wins over config.json.
270-
# Write a different token to config.json (the
271-
# fallback path); the env var must still win.
272-
$env:MINIMAX_OAUTH_TOKEN = $env:FAKE_TOKEN
273-
$cfgDir = Join-Path $env:APPDATA 'mcode-island'
274-
if (-not (Test-Path $cfgDir)) { New-Item -ItemType Directory -Path $cfgDir -Force | Out-Null }
275-
@{ planApiToken = 'config-token-should-not-be-used' } | ConvertTo-Json |
276-
Out-File -FilePath (Join-Path $cfgDir 'config.json') -Encoding utf8
287+
# Fixture body. Field names MATCH what Get-5hUsage
288+
# actually reads (model_name / current_interval_remaining_percent
289+
# / remains_time), NOT the round-5 fixture's
290+
# (model / remainingPct / resetMs). A future change
291+
# that breaks the field-name contract will cause the
292+
# function to return $null at line 432 and fail the
293+
# assertion below.
294+
$fixtureBody = '{"model_remains":[{"model_name":"general","current_interval_remaining_percent":84,"remains_time":16200000}]}'
277295
278-
# 4b) Reconstruct the URL the detector uses (the
279-
# source file constructs it via the byte-array
280-
# `_s` helper, which is private to
281-
# mcode-status-detect.ps1; we don't want to
282-
# dot-source the file because the main loop
283-
# would block in CI). The literal path the
284-
# detector requests is /v1/coding_plan/remains.
285-
$url = "http://127.0.0.1:$freePort/v1/coding_plan/remains"
286-
$headers = @{
287-
'Authorization' = "Bearer $env:MINIMAX_OAUTH_TOKEN"
288-
'MM-API-Source' = 'MiniMax-MCP'
289-
}
290-
$resp = Invoke-RestMethod -Uri $url -Headers $headers -TimeoutSec 10 -Method Get -ErrorAction Stop
296+
# Mock-server helper: one Start-Job per test, each
297+
# job handles exactly one request and returns the
298+
# captured Authorization header + path. Using one
299+
# job per test (rather than one job for all three
300+
# tests) keeps each test independent: a failure in
301+
# test a cannot corrupt the mock for tests b or c.
302+
function Run-MockOneRequest {
303+
param($port, $body)
304+
$j = Start-Job -ScriptBlock {
305+
param($p, $b)
306+
$listener = [System.Net.HttpListener]::new()
307+
$listener.Prefixes.Add("http://127.0.0.1:$p/")
308+
$listener.Start()
309+
try {
310+
$ctx = $listener.GetContext()
311+
$auth = $ctx.Request.Headers['Authorization']
312+
$path = $ctx.Request.Url.AbsolutePath
313+
$bytes = [System.Text.Encoding]::UTF8.GetBytes($b)
314+
$ctx.Response.StatusCode = 200
315+
$ctx.Response.ContentType = 'application/json'
316+
$ctx.Response.ContentLength64 = $bytes.Length
317+
$ctx.Response.OutputStream.Write($bytes, 0, $bytes.Length)
318+
$ctx.Response.Close()
319+
[PSCustomObject]@{ auth = $auth; path = $path }
320+
} finally {
321+
$listener.Stop()
322+
$listener.Close()
323+
}
324+
} -ArgumentList $port, $body
325+
return ,$j
326+
}
291327
292-
# 4c) The mock must have seen the bearer token AND
293-
# the request path the detector uses.
294-
$mock = $job | Wait-Job -Timeout 15 | Receive-Job
295-
if (-not $mock) {
296-
throw "listener job did not complete within 15s (state: $($job.State))"
297-
}
328+
# Test a: env-var token wins; implementation must
329+
# read the new field names; return value must be
330+
# the documented `{ remainingPct, resetMs }` shape.
331+
$jobA = Run-MockOneRequest $freePort $fixtureBody
332+
try {
333+
$env:MINIMAX_OAUTH_TOKEN = $env:FAKE_TOKEN
334+
$script:plan5hToken = $env:FAKE_TOKEN
335+
$data = Get-5hUsage
336+
$mock = $jobA | Wait-Job -Timeout 15 | Receive-Job
337+
if (-not $mock) { throw "test a: listener did not complete (state $($jobA.State))" }
298338
if ($mock.auth -ne "Bearer $env:FAKE_TOKEN") {
299-
throw "mock saw Authorization='$($mock.auth)' (want 'Bearer $env:FAKE_TOKEN')"
339+
throw "test a: Authorization was '$($mock.auth)' (want 'Bearer $env:FAKE_TOKEN')"
300340
}
301341
if ($mock.path -ne '/v1/coding_plan/remains') {
302-
throw "mock saw path='$($mock.path)' (want '/v1/coding_plan/remains')"
342+
throw "test a: path was '$($mock.path)' (want '/v1/coding_plan/remains')"
303343
}
304-
305-
# 4d) Response shape: same one Get-5hUsage in
306-
# mcode-status-detect.ps1 parses (model_remains[],
307-
# taking the first entry's remainingPct + resetMs).
308-
if (-not $resp -or -not $resp.model_remains) {
309-
throw "response shape: missing model_remains, got: $($resp | ConvertTo-Json -Compress)"
344+
if ($null -eq $data) {
345+
throw "test a: Get-5hUsage returned `$null` with the env-var token set (fixture field-name contract is broken)"
310346
}
311-
$first = @($resp.model_remains)[0]
312-
if ($first.remainingPct -ne 84 -or $first.resetMs -ne 16200000) {
313-
throw "first model_remains entry: got remainingPct=$($first.remainingPct) resetMs=$($first.resetMs) (want 84 / 16200000)"
347+
if ($data.remainingPct -ne 84) {
348+
throw "test a: remainingPct=$($data.remainingPct) (want 84)"
349+
}
350+
if ($data.resetMs -ne 16200000) {
351+
throw "test a: resetMs=$($data.resetMs) (want 16200000)"
352+
}
353+
Write-Host "test a (env token, matching fixture): OK remainingPct=$($data.remainingPct)% resetMs=$($data.resetMs)"
354+
} finally {
355+
if ($jobA.State -ne 'Completed') { Stop-Job $jobA }
356+
Remove-Job $jobA -Force
357+
}
358+
359+
# Test b: clear env vars, only config.json has a
360+
# token. Re-deriving $script:plan5hToken from
361+
# config.json the same way the file's top-level
362+
# init does (line 124-125). Get-5hUsage must pick
363+
# up the config.json token.
364+
$cfgDir = Join-Path $env:APPDATA 'mcode-island'
365+
if (-not (Test-Path $cfgDir)) { New-Item -ItemType Directory -Path $cfgDir -Force | Out-Null }
366+
$cfgToken = 'ci-cfg-only-token-abcdef0123456789'
367+
@{ planApiToken = $cfgToken } | ConvertTo-Json |
368+
Out-File -FilePath (Join-Path $cfgDir 'config.json') -Encoding utf8
369+
Remove-Item 'env:MINIMAX_OAUTH_TOKEN' -ErrorAction SilentlyContinue
370+
Remove-Item 'env:MINIMAX_API_KEY' -ErrorAction SilentlyContinue
371+
$cfg = Get-Content (Join-Path $cfgDir 'config.json') -Raw | ConvertFrom-Json
372+
$script:plan5hToken = $null
373+
if ($cfg.PSObject.Properties['planApiToken'] -and $cfg.planApiToken) {
374+
$script:plan5hToken = [string]$cfg.planApiToken
375+
}
376+
if ($script:plan5hToken -ne $cfgToken) {
377+
throw "test b: config.json planApiToken was not picked up (got '$($script:plan5hToken)')"
378+
}
379+
$jobB = Run-MockOneRequest $freePort $fixtureBody
380+
try {
381+
$data = Get-5hUsage
382+
$mock = $jobB | Wait-Job -Timeout 15 | Receive-Job
383+
if (-not $mock) { throw "test b: listener did not complete (state $($jobB.State))" }
384+
if ($mock.auth -ne "Bearer $cfgToken") {
385+
throw "test b: Authorization was '$($mock.auth)' (want 'Bearer $cfgToken')"
314386
}
315-
Write-Host "Mocked usage-API OK: auth='$($mock.auth)' path='$($mock.path)' first entry=remainingPct=$($first.remainingPct)% resetMs=$($first.resetMs)"
387+
if ($null -eq $data) { throw "test b: Get-5hUsage returned `$null` with config.json token" }
388+
if ($data.remainingPct -ne 84) { throw "test b: remainingPct=$($data.remainingPct) (want 84)" }
389+
if ($data.resetMs -ne 16200000) { throw "test b: resetMs=$($data.resetMs) (want 16200000)" }
390+
Write-Host "test b (config-only token): OK remainingPct=$($data.remainingPct)% resetMs=$($data.resetMs)"
316391
} finally {
317-
# Make sure the job is fully reaped even on
318-
# exception, so the listener is released.
319-
if ($job.State -ne 'Completed') { Stop-Job $job }
320-
Remove-Job $job -Force
392+
if ($jobB.State -ne 'Completed') { Stop-Job $jobB }
393+
Remove-Job $jobB -Force
394+
}
395+
396+
# Test c: no token anywhere (env vars cleared,
397+
# config.json removed) -> Get-5hUsage returns `$null`
398+
# at line 419 without hitting the network.
399+
Remove-Item (Join-Path $cfgDir 'config.json') -Force -ErrorAction SilentlyContinue
400+
$script:plan5hToken = $null
401+
$data = Get-5hUsage
402+
if ($null -ne $data) {
403+
throw "test c: Get-5hUsage should return `$null` with no token, got: $($data | ConvertTo-Json -Compress)"
321404
}
405+
Write-Host "test c (no token): OK returned `$null`"
406+
Write-Host "Get-5hUsage via dot-source + matching fixture + token-source precedence: 3/3 OK"

0 commit comments

Comments
 (0)