feat(deploy): Mode C 一鍵部屬 hybrid orchestrator(deploy.ps1 + scripts\lib\* + tests) - #124
Conversation
從 /superpowers:brainstorming 收斂出 deploy.ps1(Mode C hybrid) 的設計: preflight + auto-fix + 依賴順序啟 host-native conversion → Kit → docker compose,完成後印可診斷 summary。包含 module 分工、退出碼語意、A/B/C 三層 safety 紅線、Volume 對齊方案 A、三層測試策略與 acceptance criteria。 下一步:由 writing-plans 接力產出 implementation plan。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
從 /superpowers:writing-plans 接力 design spec(c900bab)收斂出 13 個 bite-sized task,每 task TDD 一 module: Task 1 scripts\tests\test-helpers.ps1 共用 assert helpers Task 2 lib\deploy-report.ps1 + test Task 3 lib\preflight-docker.ps1 + test Task 4 lib\preflight-host-native.ps1 + test Task 5 lib\preflight-env.ps1 + test Task 6 lib\preflight-ports.ps1 + test Task 7 lib\preflight-volume-alignment.ps1 + test Task 8 lib\host-native-launcher.ps1 + test Task 9 lib\kit-log-probe.ps1 + test Task 10 scripts\deploy.ps1 orchestrator(整合所有 lib + Phase 0-5) Task 11 scripts\tests\test-deploy-dryrun.ps1 integration Task 12 docs\runbooks\one-click-deploy-smoke.md Layer 3 smoke Task 13 GitNexus re-index + PR 每個 task 內含完整 PowerShell code(無 placeholder)、test cases、commit 指令。Self-Review 已通過 spec coverage + placeholder scan + type consistency 三項。 同時微調 spec §9.1 / §9.2 把「Pester」字眼改為「repo 風格(純 PowerShell + Assert-* helper)」:repo Pester 是 3.4.0 老版,實際既有 測試(test-pr-review-agent.ps1)都用純 PowerShell + assert helpers。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Repo 沿用 test-pr-review-agent.ps1 純 PowerShell 測試風格(不引入 Pester)。 這個檔把 Assert-True / Assert-Equal / Assert-Throws / New-TestSandbox / Remove-TestSandbox / Write-TestPass / Write-TestFail 抽出共用,供 deploy.ps1 系列 module 的 test scripts dot-source 使用。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Write-DeployTag(-Tag ok|fix|ask|skip|warn|fail -Message X -LogPath Y) 統一 deploy.ps1 / scripts\lib\* 的輸出格式。同步寫進 deploy.log 與 stdout, 供 Final Summary 連回。Write-DeployHeader 印階段分隔。 對應 spec §8.1 (Output Format) 與 §8.3 (落地物 deploy.log)。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-DockerEnvironment 回傳結構化 audit:cliVersion / composeV2 / engineRunning / envFile / ok。所有外部 CLI 都透過可注入 scriptblock 參數,test 內 fake 各種狀態(docker missing / engine down / .env fallback)。 對應 spec §5.2 module list 與 §6.2 audit result 的 docker 區塊。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-HostNativeEnvironment 偵測 .venv (OK | MISSING | WRONG_VERSION)、 Kit launcher path (OK | MISSING_PATH)、nvidia-smi (OK | MISSING)。 Python probe / NvidiaSmi probe 都接 scriptblock 注入。 對應 spec §5.2 與 §6.2 hostNative 區塊。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Get-EnvKeyList / Get-EnvAudit / Test-EnvFiles 對三個目標檔(root .env、 bim-review-coordinator/.env、.env.web-plane.host-kit)各自跟對應 .example 比對 missing key list。Invariant:.env 已有的 key 一律不出現在 missing (Phase 2 才會 append 預設值,不覆寫實值,符合 spec §7.3 紅線)。 對應 spec §5.2 與 §6.2 envFiles 區塊、§7.1 .env missing-key merge 行為。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
[string] cast 把 $null 轉成 "",if ($null -ne $EnvContent) 始終 true, Test 3 / 4 的「不傳 -EnvContent」場景仍會建 .env file 導致 envExists= true,assert 失敗。改用 IsNullOrEmpty 判斷 caller 意圖。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-PortAvailability 對 docker (8004/5173) + host-native (49100/49101/47998) 五個 port 做 listen owner audit。Get-PidsFromRunDir 讀 scripts\.run\*.pid 判斷 PID 是不是我們上次啟動的 (ourPidFile=true) — 區分 Phase 2 安全 自動清 stale PID vs Phase 3 互動問 'kill 陌生 PID'。 對應 spec §5.2、§6.2 ports 區塊、§7.1/7.2 區別。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PowerShell automatic variable $pid 是 read-only(current process PID), 任何 param($pid) 都會 throw "Cannot overwrite variable pid because it is read-only or constant"。Test-PortAvailability 的 ProcessNameLookup default 與 test fake callback 都踩到。改名 $procId。 同步修 plan(Task 6 preflight-ports 與 Task 8 host-native-launcher 的 GetProcessFn callback 範例)避免後續 task subagent 重踩。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-VolumeAlignment 實作 spec §7.4 方案 A:Ground truth = .env.web-plane.host-kit 的 RUNTIME_STORAGE_ROOT,host-native conversion-service 在 Phase 4a 啟動前反向對齊。leaf 必須是 'storage'(否則 host-native conversion-service 寫死的 Resolve-ConversionWorkDir 看不到 storage/ 子目錄)。 status: ALIGNED | MISSING_KEY | WRONG_LEAF。相對路徑以 RepoRoot 為基底 resolve 成絕對路徑。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Assert-Equal 的 -Expected 是 Mandatory,傳 $null 會 "Cannot bind null to mandatory parameter"。把 'no path' assertion 改用 Assert-True ($null -eq …) 寫法。同步修 plan Task 7 範例。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-AlreadyRunning / Remove-StalePidFile / Start-HostNativeService / Wait-HostNativeHealth 抽自 start-all.ps1 既有邏輯,讓 deploy.ps1 共用。 Start-HostNativeConversion / Start-HostNativeKit 是包好的 high-level 入口:前者反向對齊 RUNTIME_STORAGE_ROOT(spec §7.4 方案 A),後者 帶 -ResetUser(memory webrtc-no-video-reset-user-recovery)。 start-all.ps1 本身不動 — 抽 lib 的目的是 deploy.ps1 復用,不 refactor 既有入口(spec §4 / §5.3 Invariant 3)。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test-KitReadyFromLog scan scripts\.run\bim-streaming-server.log for 'Application started' / 'launching Linux Kit' / 'Streaming started'. Wait-KitReady poll-loop combines log keyword + signaling port LISTEN with 90s default timeout (Kit 啟動本來就慢)。Best-effort:timeout 時返回 ready=false,deploy.ps1 視 -StrictPostVerify 決定 warn / fail。 對應 spec §5.2 kit-log-probe、§6.3 Phase 5 Step 6、Decision Summary #5。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
整合 scripts\lib\* 八個 module,實作 spec §6 完整 Phase 0-5 流程: - Phase 1 read-only preflight + audit JSON - Phase 2 auto-fix(.venv / .env missing-key / volume alignment append / stale PID / 建目錄 / 容器衝突 rm / 第一次 docker build / -Pull) - Phase 3 互動 guard(陌生 PID 佔 port / .venv WRONG_VERSION) - Phase 4 嚴格順序啟動(4a conversion → 4b Kit → 4c docker compose) - Phase 5 post-start verify(best-effort,默認 warn 不 fail) - Final Summary 成功/失敗各印對應指引 退出碼 0/1/2/3/4/5 對齊 spec §6.3。 不改 start-all.ps1 / start-web-plane-docker.ps1 / start-runtime-manager-docker.ps1 (spec §4 / §13 acceptance criteria #10)。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
驗證 spec §9.2 -DryRun 不動真實狀態、印 Phase 1 audit、退 0、不進 Phase 4/5。同時驗 deploy-audit.json 落地(spec §8.3)。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… capture 兩個 PowerShell 陷阱: 1. $Args 是 automatic variable(unmatched args array),param($Args) 不會 bind 傳入值,docker @Args 退化成 docker 沒參數 → 印 help 文,version regex match 失敗,deploy.ps1 誤判 docker missing。改名 $ArgList。 同時 Out-String 把 array of lines 攤平給 -match 用(否則 $Matches 不 populate)。 2. test-deploy-dryrun 用 2>&1 capture 只抓 error stream,Write-Host 走 Information stream,內容跑 fly-through 到 host 但 $output 是空。改 *>&1 抓所有 stream。 全 9 個 test file regression 確認綠。同步修 plan 範例。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7 步驟手動 smoke runbook,對齊 spec §9.3。第一次合進 main 前操作員 跑一次蓋章在 Smoke Pass Log。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
兩個跑實機才暴露的 bug:
1. \$ErrorActionPreference = 'Stop' 在 PowerShell 5.1 對 native command
(docker compose rm/build/up)的 stderr 進度訊息('Container ... Stopping')
會 promote 成 terminating error,Phase 2 中途 crash。改 'Continue',
保留 native cmd stderr 不污染流程,exit code 用 \$LASTEXITCODE 主動檢。
2. .env.web-plane.host-kit Copy-Item from .example 後,\$resolvedEnvFile
仍指 .example,後續 RUNTIME_STORAGE_ROOT append / docker rm/build/up
都會用 .example 而非真檔。Copy 後立刻 re-resolve + re-audit volume。
Layer 2 dry-run regression check 仍綠。
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Omniverse Kit 啟動完成時 log 寫的是小寫 'app ready'(實機驗證):
[15.937s] [ext: ezplus.bim_review_stream_streaming-0.1.0] startup
[19.982s] app ready
之前 kit-log-probe 的 keyword list 沒含這個,Phase 4b 判定 Kit not ready
即使 :49100 已 LISTEN。加 'app ready' 為首位 keyword(其他三個保留以容
Linux Kit / 不同 launcher 版本)。
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
子 script 內 \$ErrorActionPreference='Stop' + docker compose up 進度寫 stderr → PowerShell 5.1 把 'Container ... Creating' 印成 NativeCommandError 紅字 trace,嚇到使用者(雖然實際 docker 仍 sync wait 沒真 fail)。 改用 Start-Process new process + RedirectStandardOutput/Error 隔離,讓 子 script 的 stderr 進 docker-compose-up.err.log 而非污染父流程。 父用 $proc.ExitCode 判定。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rwarders 兩個 Phase 3 互動 guard bug 一起修: 1. Phase 2 docker compose rm 改變 docker container 與 wslrelay/com.docker.backend 等 port forwarder 狀態,但 Phase 3 用 Phase 1 的 stale $ports 資料來問 互動。Phase 3 開頭 re-audit ports。 2. Docker Desktop 在 Windows 用 wslrelay.exe / com.docker.backend.exe / docker.exe / vpnkit.exe / vpnkit-bridge.exe 做 container → host port forward,這些不該被當「陌生 PID」要求 user kill。加 whitelist 跳過。 實際情境:user 已啟過 docker container,Phase 1 看到 :8004 被 wslrelay 佔 就標 [ask],Phase 3 問「kill PID 23492 (wslrelay.exe)? (y/N)」誤導 user。 正確做法是讓 Phase 2 docker rm 自然清掉 forwarder,Phase 3 不該問。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
兩個 bug 一起修讓 idempotent re-run 真的 work(5 秒 + Phase 4 全 skip + Phase 5 verify 全 200): 1. preflight-ports.ps1:Get-PidsFromRunDir 沿 ParentProcessId 遞迴展開 wrapper 的子孫 PID。host-native-launcher 用 powershell.exe wrapper spawn 真 process 是 grand-child(:49100 owner = kit.exe / :49101 = python.exe), wrapper PID file 內只有 powershell.exe wrapper,不展開子孫的話 wrapper alive 但 kit.exe child 被誤判為「陌生 process」,Phase 3 互動 prompt 要 user kill 自己的 Kit。 Resolve-PortStatus 同步把 portPid 強轉 [int],避免 Get-NetTCPConnection 回的 UInt32 與 hashtable Int32 key type mismatch 讓 ContainsKey 永遠 回 false。 2. deploy.ps1:Phase 2 docker rm/build + Phase 4c docker up 加 webPlaneRunning conditional。`docker compose ps --status running -q coordinator viewer` 回兩個 container id → web-plane 已 running → 三段都 [skip]。否則仍走 原本 rm + build + up 路徑(冷啟動)。 也修 Print-FinalSummary 的 Next/recover 指令文字(stop-all.ps1 沒有 -SkipCoordinator/-SkipViewer 參數,改成 docker compose down + stop-all)。 實機驗證: - 冷啟動 deploy.ps1:1m 42s 全綠 - idempotent re-run:5 秒,Phase 4 全 skip,Phase 5 verify 全 200 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Step 1 cold start + Step 2 first deploy(1m 42s 全綠)+ Step 5 idempotent re-run(5s + Phase 4 全 skip + verify 全 200)已實機驗。Step 3/4/6/7 需要人類目視 / 手動操作 Docker Desktop,留 PR review 階段補驗 + 後續 回來補蓋章。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 20 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds a complete hybrid one-click deployment system for Windows with NVIDIA GPU support, implementing a five-phase orchestration flow (preflight audit → auto-fix → interactive guard → service startup → health verification) through a main ChangesHybrid Deployment Orchestration System
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new one-click Mode C (hybrid) deployment entrypoint (scripts/deploy.ps1) that orchestrates preflight checks, safe auto-fixes, interactive safety prompts, ordered service startup (conversion → Kit → Docker web-plane), and post-start verification. It adds a small PowerShell “library” layer (scripts/lib/*.ps1) plus a suite of PowerShell tests (scripts/tests/test-*.ps1) and supporting design/plan/runbook documentation.
Changes:
- Add
scripts/deploy.ps1orchestrator implementing phased hybrid deployment with structured logging and exit codes. - Add 8 reusable PowerShell modules under
scripts/lib/*for reporting, preflight audits, host-native launching, and Kit readiness probing. - Add PowerShell unit/integration tests under
scripts/tests/*plus design/plan/runbook docs for the deployment workflow.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/runbooks/one-click-deploy-smoke.md | Adds manual Layer 3 smoke checklist and a recorded smoke run log. |
| docs/superpowers/plans/2026-05-26-one-click-deploy.md | Implementation plan documenting phases, modules, and testing strategy. |
| docs/superpowers/specs/2026-05-26-one-click-deploy-design.md | Design spec for Mode C one-click deploy architecture, safety boundaries, and acceptance criteria. |
| scripts/deploy.ps1 | New hybrid deployment orchestrator with Phase 1–5 flow, logs, and exit codes. |
| scripts/lib/deploy-report.ps1 | Shared tagged output/logging helpers for deploy reporting. |
| scripts/lib/host-native-launcher.ps1 | Shared process lifecycle helpers to start/health-check host-native conversion and Kit. |
| scripts/lib/kit-log-probe.ps1 | Kit readiness detection via log keyword scan + port listen probing. |
| scripts/lib/preflight-docker.ps1 | Read-only Docker/Compose/env-file audit with injectable probes for tests. |
| scripts/lib/preflight-env.ps1 | Read-only missing-key audit for env files vs corresponding .example files. |
| scripts/lib/preflight-host-native.ps1 | Read-only host-native toolchain audit (.venv, Kit launcher path, nvidia-smi). |
| scripts/lib/preflight-ports.ps1 | Read-only port occupancy audit with PID-file attribution and descendant PID expansion. |
| scripts/lib/preflight-volume-alignment.ps1 | Read-only audit of RUNTIME_STORAGE_ROOT path alignment and leaf validation. |
| scripts/tests/test-deploy-dryrun.ps1 | Integration-style test for deploy.ps1 -DryRun output and audit artifact generation. |
| scripts/tests/test-deploy-report.ps1 | Unit tests for deploy-report tag output + logging behavior. |
| scripts/tests/test-helpers.ps1 | Shared test assert helpers and temp sandbox helpers (non-Pester style). |
| scripts/tests/test-host-native-launcher.ps1 | Unit tests for launcher PID handling and health-check polling logic. |
| scripts/tests/test-kit-log-probe.ps1 | Unit tests for log keyword detection and Wait-KitReady polling behavior. |
| scripts/tests/test-preflight-docker.ps1 | Unit tests for Docker preflight audit logic with injected fakes. |
| scripts/tests/test-preflight-env.ps1 | Unit tests for env missing-key audit across the three target env files. |
| scripts/tests/test-preflight-host-native.ps1 | Unit tests for host-native audit outcomes (missing/wrong version/missing path). |
| scripts/tests/test-preflight-ports.ps1 | Unit tests for port availability + PID-file attribution logic. |
| scripts/tests/test-preflight-volume-alignment.ps1 | Unit tests for volume alignment statuses (ALIGNED/MISSING_KEY/WRONG_LEAF). |
Comments suppressed due to low confidence (2)
scripts/deploy.ps1:90
- Print-FinalSummary assumes each *.pid file has non-empty content and calls .Trim() unconditionally. If a PID file is empty/corrupt (or Get-Content returns $null), this will throw while printing the summary and can mask the real failure cause. Guard against null/empty content before trimming/printing (and ideally handle parse failures gracefully).
foreach ($pidFile in Get-ChildItem -LiteralPath $RunDir -Filter '*.pid' -ErrorAction SilentlyContinue) {
$procId = (Get-Content $pidFile.FullName | Select-Object -First 1).Trim()
Write-Host " > $($pidFile.BaseName) PID $procId"
}
docs/superpowers/specs/2026-05-26-one-click-deploy-design.md:449
- The spec says
-DryRunmust not write files ("不寫檔"), but the current implementation writesscripts\.run\deploy-audit.jsonanddeploy.logduring Phase 1 even under -DryRun (and the integration test asserts this). Please align the spec with the actual behavior (e.g., allow writing logs/audit artifacts under scripts.run during -DryRun), or adjust the implementation/tests to truly be no-write.
`-DryRun` 不能動真實狀態(不啟 process、不動 docker、不寫檔)。
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Set-StrictMode -Version Latest | ||
| # Continue(非 Stop):docker / docker compose 進度寫 stderr,PowerShell 5.1 native | ||
| # command 對 stderr 在 Stop policy 下會被 promote 成 terminating error。我們改用 | ||
| # $LASTEXITCODE 主動檢查,native cmd stderr 只當訊息看。 | ||
| $ErrorActionPreference = 'Continue' | ||
| $script:DeployStart = Get-Date |
| if ($hostNative.venv -eq 'WRONG_VERSION') { | ||
| $prompt = '.venv has wrong Python version (<3.11). Recreate? (will delete .venv) (y/N)' | ||
| if ($Force) { | ||
| Write-DeployTag -Tag 'fix' -Message "$prompt -> y (--Force)" -LogPath $LogPath | Out-Null | ||
| Remove-Item -LiteralPath (Join-Path $RepoRoot '.venv') -Recurse -Force | ||
| & python -m venv (Join-Path $RepoRoot '.venv') | ||
| } else { | ||
| Write-DeployTag -Tag 'ask' -Message $prompt -LogPath $LogPath | Out-Null | ||
| $response = Read-Host 'y/N' | ||
| if ($response -match '^[Yy]') { | ||
| Remove-Item -LiteralPath (Join-Path $RepoRoot '.venv') -Recurse -Force | ||
| & python -m venv (Join-Path $RepoRoot '.venv') |
| Push-Location $RepoRoot | ||
| try { docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile pull } finally { Pop-Location } |
| docker = $docker | ||
| hostNative = $hostNative | ||
| envFiles = $envFiles | ||
| ports = $ports | ||
| volume = $volume | ||
| envFileUsed = $resolvedEnvFile |
| $result = Test-DockerEnvironment ` | ||
| -DockerCommand { param($Args) "Docker version 27.0.3, build x" } ` | ||
| -ComposeCommand { param($Args) "Docker Compose version v2.29.0" } ` | ||
| -EngineProbe { param($Args) @{ ExitCode = 0; Stdout = '{"ServerVersion":"27.0.3"}' } } ` | ||
| -RepoRoot (New-TestSandbox -Prefix 'preflight-docker') | ||
|
|
||
| Assert-True ($result.cliVersion -ne $null) 'cliVersion populated' | ||
| Assert-True ($result.composeV2 -eq $true) 'composeV2 true' | ||
| Assert-True ($result.engineRunning -eq $true) 'engineRunning true' | ||
| Write-TestPass 'happy path returns full audit' | ||
|
|
||
| # Test 2: docker CLI 不在 → cliVersion=null + 整體 ok=false | ||
| $result = Test-DockerEnvironment ` | ||
| -DockerCommand { throw 'docker not found' } ` | ||
| -ComposeCommand { param($Args) '' } ` | ||
| -EngineProbe { param($Args) @{ ExitCode = 1; Stdout = '' } } ` | ||
| -RepoRoot (New-TestSandbox -Prefix 'preflight-docker') | ||
|
|
||
| Assert-True ($null -eq $result.cliVersion) 'cliVersion null when docker absent' | ||
| Assert-True ($result.ok -eq $false) 'overall ok=false' | ||
| Write-TestPass 'docker missing flagged' | ||
|
|
||
| # Test 3: engine 沒跑 → engineRunning=false | ||
| $result = Test-DockerEnvironment ` | ||
| -DockerCommand { param($Args) "Docker version 27.0.3" } ` | ||
| -ComposeCommand { param($Args) "Docker Compose version v2.29.0" } ` | ||
| -EngineProbe { param($Args) @{ ExitCode = 1; Stdout = '' } } ` | ||
| -RepoRoot (New-TestSandbox -Prefix 'preflight-docker') | ||
|
|
||
| Assert-True ($result.engineRunning -eq $false) 'engineRunning=false when engine probe non-zero' | ||
| Write-TestPass 'engine not running flagged' |
| $result = Test-PortAvailability -RepoRoot (New-TestSandbox -Prefix 'preflight-ports') ` | ||
| -PortLookup { param($port) $null } ` | ||
| -ProcessNameLookup { param($procId) $null } | ||
| Assert-True ($result.docker.Count -eq 2) 'docker has 2 ports' | ||
| Assert-True ($result.hostNative.Count -eq 3) 'hostNative has 3 ports' | ||
| foreach ($p in @($result.docker; $result.hostNative)) { | ||
| Assert-Equal 'FREE' $p.status "port $($p.port) FREE" | ||
| } | ||
| Write-TestPass 'all ports free' |
| Assert-Throws { | ||
| Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path (New-TestSandbox) 'd.log') | ||
| } 'unknown tag throws' | ||
| Write-TestPass 'unknown tag throws' |
| "volumeAlignment": { | ||
| "runtimeStorageRoot": null, | ||
| "leaf": null, | ||
| "status": "MISSING_KEY" | ||
| } | ||
| } |
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6cc7f8d70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (-not $docker.cliVersion) { $hardFails += 'docker_cli_missing' } | ||
| if (-not $docker.composeV2) { $hardFails += 'docker_compose_v2_missing' } | ||
| if (-not $docker.engineRunning) { $hardFails += 'docker_engine_not_running' } | ||
| if (-not $docker.envFile) { $hardFails += 'env_file_missing_entirely' } | ||
| if ($hostNative.nvidiaDriver -eq 'MISSING') { $hardFails += 'nvidia_smi_missing' } | ||
| if ($hostNative.kitLauncher -eq 'MISSING_PATH'){ $hardFails += 'kit_launcher_missing' } |
There was a problem hiding this comment.
deploy.ps1 exposes partial-start flags (-SkipDocker, -SkipKit, -SkipConversion), but Phase 1 still unconditionally appends Docker and host-native failures to $hardFails. In practice, -SkipDocker still fails on missing Docker/compose/engine/env, and -SkipKit -SkipConversion still fails on missing NVIDIA/launcher, so the script exits in Phase 1 before the requested reduced mode can run. Gate these hard-fail conditions by the corresponding skip flags so partial deploy/recovery flows actually work.
Useful? React with 👍 / 👎.
| if ($Force) { | ||
| Write-DeployTag -Tag 'fix' -Message "$prompt -> y (--Force)" -LogPath $LogPath | Out-Null | ||
| Remove-Item -LiteralPath (Join-Path $RepoRoot '.venv') -Recurse -Force | ||
| & python -m venv (Join-Path $RepoRoot '.venv') |
There was a problem hiding this comment.
In the WRONG_VERSION branch, .venv is deleted and recreated, but the python -m venv call is not validated (unlike the Phase 2 venv creation path). If recreation fails (missing interpreter, permission/path issues), deployment continues into Phase 4 and produces misleading downstream failures instead of a clear Phase 3 stop. Add exit-code/error checks (and ideally the same dependency bootstrap as Phase 2) immediately after this command.
Useful? React with 👍 / 👎.
| } elseif ($webPlaneRunning) { | ||
| Write-DeployTag -Tag 'skip' -Message 'Phase 4c docker compose: coordinator + viewer already running' -LogPath $LogPath | Out-Null | ||
| } else { |
There was a problem hiding this comment.
When -Build is used and coordinator/viewer are already running, Phase 2 can build new images, but Phase 4c still short-circuits on the stale $webPlaneRunning flag and skips up, so containers are never recreated onto the new image. This makes -Build ineffective for hot reruns where users expect rebuilt code to be applied.
Useful? React with 👍 / 👎.
| $env:STREAMING_CONVERSION_WORK_DIR = $parentRoot | ||
| $env:STREAMING_CONVERSION_HOST = '127.0.0.1' | ||
| $env:STREAMING_CONVERSION_PORT = "$Port" |
There was a problem hiding this comment.
Phase 2 installs conversion dependencies into .venv, but the conversion launcher only sets host/port/workdir and never pins STREAMING_CONVERSION_PYTHON to that venv interpreter. The downstream runner defaults to global python, so on machines where only the venv has required packages (e.g., uvicorn), Phase 4a reliably times out despite successful bootstrap.
Useful? React with 👍 / 👎.
| Write-Host ' > open http://127.0.0.1:8004/ui (coordinator UI / WebRTC entry)' | ||
| Write-Host ' > tail scripts\.run\bim-streaming-server.log -Wait' | ||
| Write-Host ' > stop all:' | ||
| Write-Host ' docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file .env.web-plane.host-kit down' |
There was a problem hiding this comment.
The final summary always prints docker compose ... --env-file .env.web-plane.host-kit down even when deploy actually ran with a different env file (for example .env.web-plane.host-kit.example or a custom -EnvFile). In those cases the provided recovery command can fail or target the wrong project, making operational cleanup misleading.
Useful? React with 👍 / 👎.
| -Name 'bim-streaming-conversion-service' ` | ||
| -WorkingDirectory (Join-Path $RepoRoot 'bim-streaming-server') ` | ||
| -FilePath 'powershell.exe' ` | ||
| -ArgumentList @('-NoExit','-ExecutionPolicy','Bypass','-NoProfile','-File',$launcher) ` |
There was a problem hiding this comment.
Start-HostNativeConversion launches PowerShell with -NoExit, so the wrapper process can stay alive even after the conversion script exits/crashes. Because Test-AlreadyRunning only checks whether the PID in *.pid is alive, later deploy runs may incorrectly skip Phase 4a as "already running" while no conversion API is actually serving.
Useful? React with 👍 / 👎.
| Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue | ||
| } else { |
There was a problem hiding this comment.
Phase 3 uses Stop-Process -ErrorAction SilentlyContinue for occupied ports, but does not check whether the process actually stopped before continuing. If termination fails (for example due to insufficient privileges), deploy proceeds with ports still occupied and later fails in less actionable places; Phase 3 should re-check liveness/port state and fail immediately with a clear message.
Useful? React with 👍 / 👎.
| $exitCode = $LASTEXITCODE | ||
|
|
||
| # Test 1: 退 0 | ||
| Assert-Equal 0 $exitCode '-DryRun exit 0' |
There was a problem hiding this comment.
test-deploy-dryrun.ps1 executes the real deploy script and asserts exit code 0, but deploy.ps1 -DryRun still performs Phase 1 hard-fail checks (Docker/engine/NVIDIA/env). On environments without those prerequisites, this test fails even though the script behavior is expected there, making the test non-deterministic across developer/CI hosts.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
scripts/tests/test-preflight-env.ps1 (2)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as previous files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-preflight-env.ps1` around lines 1 - 7, The file scripts/tests/test-preflight-env.ps1 should be saved with a UTF-8 BOM so non-ASCII characters are handled consistently; update the file encoding (that contains the dot-sourced lines for 'test-helpers.ps1' and the modulePath assignment and dot-sourcing of preflight-env.ps1) to UTF-8 with BOM using your editor or a save-encoding step in your CI/tooling so the script starts with the BOM marker.
9-14: 💤 Low valueComment is slightly misleading.
The comment states "用 IsNullOrEmpty 區分「沒給」vs「給空字串」" (use IsNullOrEmpty to distinguish "not provided" vs "empty string"), but the code actually treats both cases identically: neither creates a file.
Consider updating the comment to reflect the actual behavior:
# 注意:[string] cast 會把 $null 轉成 "",所以用 IsNullOrEmpty 統一處理 $null 與空字串 # (Note: [string] cast converts $null to "", so use IsNullOrEmpty to handle both $null and empty string identically)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-preflight-env.ps1` around lines 9 - 14, The comment for New-EnvSandbox is misleading about IsNullOrEmpty distinguishing "not provided" vs "empty string"; update the comment near the function and the param list (New-EnvSandbox, param $EnvContent) to state that casting to [string] converts $null to "" and IsNullOrEmpty therefore treats $null and empty string identically, e.g., replace the existing line with a note like "注意:[string] cast 會把 $null 轉成 "",所以用 IsNullOrEmpty 統一處理 $null 與空字串" (and include the English equivalent if desired).scripts/lib/preflight-volume-alignment.ps1 (2)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as previous files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-volume-alignment.ps1` around lines 1 - 7, This file contains non-ASCII (Chinese) comments and should be saved with a UTF-8 BOM so editors and Windows PowerShell correctly recognize the encoding; reopen scripts/lib/preflight-volume-alignment.ps1 and save it encoded as "UTF-8 with BOM" (or add an explicit BOM) so the header/comments (e.g., the lines above Set-StrictMode -Version Latest) are preserved and displayed correctly.
65-69: 💤 Low valueEmpty catch block could be more explicit.
While the comment "留原值" (keep original value) explains the intent, consider making the fallback more explicit for clarity:
try { $resolved = [System.IO.Path]::GetFullPath($resolved) } catch { # GetFullPath failed - keep original value and continue # Downstream checks will report the path as-is }Alternatively, if observability is important, you could track normalization failures in the return object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-volume-alignment.ps1` around lines 65 - 69, The empty catch should explicitly state the fallback behavior and optionally record failure: update the catch block after the [System.IO.Path]::GetFullPath($resolved) call to include a clear comment like “GetFullPath failed - keep original $resolved and continue” and optionally add minimal observability (e.g., set a flag or add a note to the return object) so downstream code can detect normalization failures; reference the $resolved variable and the GetFullPath call when making the change.scripts/tests/test-helpers.ps1 (2)
14-16: 💤 Low valueArray truthiness logic may produce unexpected results with $null.
The current implementation checks
$Condition -contains $false, but this doesn't catch$nullor other falsy values. For example,@($true, $null)would pass the assertion even though$nullis falsy in PowerShell.Consider using
-contains $falsetogether with a check for$null, or refactor to check that all elements are explicitly truthy:if ($Condition -is [array]) { $Condition = ($Condition.Count -gt 0) -and -not ($Condition | Where-Object { -not $_ }) }This ensures all elements evaluate to
$true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-helpers.ps1` around lines 14 - 16, The array-truthiness branch for $Condition currently only checks for $false and misses $null/other falsy values; update the block that handles "if ($Condition -is [array])" so $Condition becomes true only when Count -gt 0 and no element is falsy (e.g., replace the -not ($Condition -contains $false) test with a check that there are no elements matching {-not $_} such as using -not ($Condition | Where-Object { -not $_ }) or an equivalent All-true predicate), keeping the change scoped to the $Condition array-handling code path.
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
The file contains Traditional Chinese comments but lacks a BOM (Byte Order Mark). While this may work in your current environment, adding a UTF-8 BOM ensures consistent character encoding interpretation across different editors and systems, especially when files are opened on machines with different default encodings.
To add BOM in PowerShell:
$content = Get-Content -Path test-helpers.ps1 -Raw [System.IO.File]::WriteAllText((Resolve-Path test-helpers.ps1), $content, (New-Object System.Text.UTF8Encoding $true))Or configure your editor to save with UTF-8 BOM.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-helpers.ps1` around lines 1 - 7, The file contains Traditional Chinese comments but lacks a UTF‑8 BOM; update the file encoding to UTF‑8 with BOM so editors and systems reliably interpret the non-ASCII text. Open the file containing the top-level statements (e.g., Set-StrictMode and $ErrorActionPreference) and rewrite or save it using UTF‑8 with BOM (either via your editor settings or by programmatically reading the file and writing it back with a UTF8Encoding that emits a BOM) so the file is stored with the BOM present.scripts/lib/preflight-env.ps1 (1)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern. The file contains Traditional Chinese comments and should have UTF-8 BOM for consistent encoding interpretation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-env.ps1` around lines 1 - 7, This file contains Traditional Chinese comments at the top (before the Set-StrictMode -Version Latest line) and should be saved with a UTF-8 BOM so non-ASCII readers interpret encoding correctly; open the script (the header comment block including the lines starting "# scripts\lib\preflight-env.ps1" through the comment lines) and re-save it using UTF-8 with BOM (or prepend the UTF-8 BOM bytes) so the file encoding is explicitly BOM-marked while leaving the content and the Set-StrictMode -Version Latest line unchanged.scripts/lib/preflight-host-native.ps1 (1)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as previous files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-host-native.ps1` around lines 1 - 7, This file contains non-ASCII characters in the header and should be saved with a UTF-8 BOM to avoid encoding issues; open the script (containing Set-StrictMode and the Test-HostNativeEnvironment function) in your editor/CI step and re-save or transcode the file as UTF-8 with BOM (or remove/replace the non-ASCII characters) so the header comment is preserved correctly across tools that expect a BOM.scripts/tests/test-preflight-docker.ps1 (2)
10-12: 💤 Low valueParameter name
$Argsmay cause confusion.While technically valid in a
param()block, naming a parameter$Argscan be confusing since PowerShell has an automatic$Argsvariable. The parameters are declared but unused in these test mocks, so consider either:
- Removing the unused parameters (since mocks return static strings), or
- Renaming to
$ArgListor$Argumentsto match the module's convention and avoid confusion-DockerCommand { "Docker version 27.0.3, build x" } ` -ComposeCommand { "Docker Compose version v2.29.0" } ` -EngineProbe { @{ ExitCode = 0; Stdout = '{"ServerVersion":"27.0.3"}' } } `🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-preflight-docker.ps1` around lines 10 - 12, The mock script blocks DockerCommand, ComposeCommand, and EngineProbe declare unused parameters named $Args which can be confused with PowerShell's automatic $Args; remove the param() declarations (or rename to $ArgList/$Arguments) and return the static values directly so DockerCommand returns the version string, ComposeCommand returns the compose string, and EngineProbe returns the hashtable @{ ExitCode = 0; Stdout = '{"ServerVersion":"27.0.3"}' } as shown in the review.
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as previous files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test-preflight-docker.ps1` around lines 1 - 7, This test script contains non-ASCII content and should be saved with a UTF-8 BOM so PowerShell reads characters correctly; reopen scripts/tests/test-preflight-docker.ps1 (and any referenced helper files like test-helpers.ps1 and scripts/lib/preflight-docker.ps1 if they contain non-ASCII text) and re-save them with "UTF-8 with BOM" encoding (or add the BOM header) using your editor or an encoding-aware save command to ensure correct parsing at runtime.scripts/lib/preflight-docker.ps1 (1)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as test-helpers.ps1. The file contains Traditional Chinese comments and should have UTF-8 BOM for consistent encoding interpretation across systems.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-docker.ps1` around lines 1 - 7, The file contains Traditional Chinese comments (the header lines and the Set-StrictMode -Version Latest statement) but lacks a UTF-8 BOM; re-save this script with UTF-8 encoding with BOM so non-ASCII characters are interpreted consistently across platforms, e.g., open the file containing the header comments and the Set-StrictMode -Version Latest line and change the file encoding to "UTF-8 with BOM" (or use an editor/CLI tool to write a BOM) without altering the script contents.scripts/lib/preflight-ports.ps1 (1)
1-7: 💤 Low valueConsider adding UTF-8 BOM for non-ASCII characters.
Same BOM encoding concern as previous files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/preflight-ports.ps1` around lines 1 - 7, This file contains non-ASCII Chinese characters in the header comment and should be saved with a UTF-8 BOM so PowerShell correctly interprets them; open scripts/lib/preflight-ports.ps1 (look for the header comment and the Set-StrictMode -Version Latest line), change the file encoding to "UTF-8 with BOM" (or re-save via your editor/CI tooling with a BOM) and commit the re-encoded file so Windows/PowerShell won't misread the non-ASCII characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-26-one-click-deploy-design.md`:
- Line 46: Update the spec text that describes the orchestrator size to match
the current implementation: change the phrase "100-150 行薄 orchestrator" to
either "模組化 orchestrator（目前約 490 行）" or remove the hard line-count limit; edit
the same phrasing at the other occurrence referenced (line 108) as well; ensure
you update the description that mentions deploy.ps1 and scripts\lib\*.ps1 so the
docs reflect the actual deploy.ps1 size and modularization rather than the
outdated 100–150 line claim.
- Line 538: Update the Acceptance Criteria text that currently reads "Pester 8 個
module test 全綠" to instead state "純 PowerShell + Assert-* helpers 8 個 module
test 全綠" (or similar wording that uses "純 PowerShell + Assert-* helpers") so it
matches the earlier statement that the repo does not use Pester; ensure you
replace that exact phrase found in the Acceptance Criteria section.
- Line 15: Several fenced code blocks in the document are missing language
markers (MD040); locate the bare code fences (``` ) around the examples (the
blocks noted in the review) and add the appropriate language tag (e.g.,
powershell, json, or text) after the opening backticks so each block becomes
```powershell / ```json / ```text as appropriate to the snippet content,
ensuring lint passes and readability is improved.
In `@scripts/deploy.ps1`:
- Around line 413-425: The WRONG_VERSION branch recreates the virtualenv (checks
hostNative.venv, uses Remove-Item and & python -m venv) but does not reinstall
dependencies, leaving host-native without required packages; after both places
where the script creates the .venv (both the $Force and interactive branches
that call & python -m venv) invoke the same requirements-install logic used
elsewhere in the script (the existing pip install -r ... sequence), reuse or
extract that installation into a shared routine and call it after venv creation,
and if the pip install fails return/exit with the same ExitCode 2 as the
original requirements-install flow so failures are propagated.
- Around line 182-184: The hard-fail checks should respect the CLI flags by only
adding 'nvidia_smi_missing' when conversion is not skipped and only adding
'kit_launcher_missing' when kit steps are not skipped: update the conditions
around $hostNative.nvidiaDriver and $hostNative.kitLauncher so they include
checks like (-not $SkipConversion) for the NVIDIA check and (-not $SkipKit) for
the kit launcher check before appending to $hardFails; leave the $volume.status
/ 'runtime_storage_root_wrong_leaf' check unchanged.
- Around line 398-404: After calling Stop-Process on $sp.pid (both places where
Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue is used), verify
the PID was actually terminated: attempt to Get-Process -Id $sp.pid (or
Wait-Process with a short timeout) and if the process still exists or retrieval
fails due to permissions, write a failure DeployTag via Write-DeployTag (include
the PID and the error/reason), and either retry or abort the deploy flow so
Phase 4 does not continue with an inaccurate state; update the Stop-Process call
sites and the subsequent logging to perform this existence check and branch on
success/failure accordingly.
- Around line 365-366: After running the docker compose pull command, check
$LASTEXITCODE and treat non-zero as failure: capture the exit code immediately
after the try/finally block that runs "docker compose -f
compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile
pull", and if $LASTEXITCODE -ne 0, write an error (e.g., Write-Error or
Write-Host) and Exit with that code (or throw) so the script stops and does not
increment $fixActions; ensure Pop-Location still runs (keep the finally) but
perform the exit/check before the line that increments $fixActions.
In `@scripts/tests/test-deploy-report.ps1`:
- Around line 38-40: The test creates a sandbox inside Assert-Throws via
New-TestSandbox causing leftover temp dirs; refactor so you call New-TestSandbox
before the Assert-Throws, capture the sandbox path in a variable, then call
Write-DeployTag inside Assert-Throws using that path, and ensure you wrap the
test in try/finally (or use try/catch/finally) to always call Remove-TestSandbox
with the captured sandbox path in the finally block to guarantee cleanup;
reference the functions Assert-Throws, New-TestSandbox, Remove-TestSandbox and
Write-DeployTag when making these changes.
In `@scripts/tests/test-preflight-ports.ps1`:
- Around line 9-17: The test creates a sandbox inline via New-TestSandbox when
calling Test-PortAvailability but never removes it; change the test to capture
the sandbox handle (e.g. assign New-TestSandbox to a variable), pass that
sandbox/root into Test-PortAvailability, and ensure you call Remove-TestSandbox
on that handle at the end (preferably in a try/finally so Remove-TestSandbox
runs even if assertions fail); reference the New-TestSandbox call, the
Test-PortAvailability invocation and add a Remove-TestSandbox cleanup step tied
to the created sandbox.
---
Nitpick comments:
In `@scripts/lib/preflight-docker.ps1`:
- Around line 1-7: The file contains Traditional Chinese comments (the header
lines and the Set-StrictMode -Version Latest statement) but lacks a UTF-8 BOM;
re-save this script with UTF-8 encoding with BOM so non-ASCII characters are
interpreted consistently across platforms, e.g., open the file containing the
header comments and the Set-StrictMode -Version Latest line and change the file
encoding to "UTF-8 with BOM" (or use an editor/CLI tool to write a BOM) without
altering the script contents.
In `@scripts/lib/preflight-env.ps1`:
- Around line 1-7: This file contains Traditional Chinese comments at the top
(before the Set-StrictMode -Version Latest line) and should be saved with a
UTF-8 BOM so non-ASCII readers interpret encoding correctly; open the script
(the header comment block including the lines starting "#
scripts\lib\preflight-env.ps1" through the comment lines) and re-save it using
UTF-8 with BOM (or prepend the UTF-8 BOM bytes) so the file encoding is
explicitly BOM-marked while leaving the content and the Set-StrictMode -Version
Latest line unchanged.
In `@scripts/lib/preflight-host-native.ps1`:
- Around line 1-7: This file contains non-ASCII characters in the header and
should be saved with a UTF-8 BOM to avoid encoding issues; open the script
(containing Set-StrictMode and the Test-HostNativeEnvironment function) in your
editor/CI step and re-save or transcode the file as UTF-8 with BOM (or
remove/replace the non-ASCII characters) so the header comment is preserved
correctly across tools that expect a BOM.
In `@scripts/lib/preflight-ports.ps1`:
- Around line 1-7: This file contains non-ASCII Chinese characters in the header
comment and should be saved with a UTF-8 BOM so PowerShell correctly interprets
them; open scripts/lib/preflight-ports.ps1 (look for the header comment and the
Set-StrictMode -Version Latest line), change the file encoding to "UTF-8 with
BOM" (or re-save via your editor/CI tooling with a BOM) and commit the
re-encoded file so Windows/PowerShell won't misread the non-ASCII characters.
In `@scripts/lib/preflight-volume-alignment.ps1`:
- Around line 1-7: This file contains non-ASCII (Chinese) comments and should be
saved with a UTF-8 BOM so editors and Windows PowerShell correctly recognize the
encoding; reopen scripts/lib/preflight-volume-alignment.ps1 and save it encoded
as "UTF-8 with BOM" (or add an explicit BOM) so the header/comments (e.g., the
lines above Set-StrictMode -Version Latest) are preserved and displayed
correctly.
- Around line 65-69: The empty catch should explicitly state the fallback
behavior and optionally record failure: update the catch block after the
[System.IO.Path]::GetFullPath($resolved) call to include a clear comment like
“GetFullPath failed - keep original $resolved and continue” and optionally add
minimal observability (e.g., set a flag or add a note to the return object) so
downstream code can detect normalization failures; reference the $resolved
variable and the GetFullPath call when making the change.
In `@scripts/tests/test-helpers.ps1`:
- Around line 14-16: The array-truthiness branch for $Condition currently only
checks for $false and misses $null/other falsy values; update the block that
handles "if ($Condition -is [array])" so $Condition becomes true only when Count
-gt 0 and no element is falsy (e.g., replace the -not ($Condition -contains
$false) test with a check that there are no elements matching {-not $_} such as
using -not ($Condition | Where-Object { -not $_ }) or an equivalent All-true
predicate), keeping the change scoped to the $Condition array-handling code
path.
- Around line 1-7: The file contains Traditional Chinese comments but lacks a
UTF‑8 BOM; update the file encoding to UTF‑8 with BOM so editors and systems
reliably interpret the non-ASCII text. Open the file containing the top-level
statements (e.g., Set-StrictMode and $ErrorActionPreference) and rewrite or save
it using UTF‑8 with BOM (either via your editor settings or by programmatically
reading the file and writing it back with a UTF8Encoding that emits a BOM) so
the file is stored with the BOM present.
In `@scripts/tests/test-preflight-docker.ps1`:
- Around line 10-12: The mock script blocks DockerCommand, ComposeCommand, and
EngineProbe declare unused parameters named $Args which can be confused with
PowerShell's automatic $Args; remove the param() declarations (or rename to
$ArgList/$Arguments) and return the static values directly so DockerCommand
returns the version string, ComposeCommand returns the compose string, and
EngineProbe returns the hashtable @{ ExitCode = 0; Stdout =
'{"ServerVersion":"27.0.3"}' } as shown in the review.
- Around line 1-7: This test script contains non-ASCII content and should be
saved with a UTF-8 BOM so PowerShell reads characters correctly; reopen
scripts/tests/test-preflight-docker.ps1 (and any referenced helper files like
test-helpers.ps1 and scripts/lib/preflight-docker.ps1 if they contain non-ASCII
text) and re-save them with "UTF-8 with BOM" encoding (or add the BOM header)
using your editor or an encoding-aware save command to ensure correct parsing at
runtime.
In `@scripts/tests/test-preflight-env.ps1`:
- Around line 1-7: The file scripts/tests/test-preflight-env.ps1 should be saved
with a UTF-8 BOM so non-ASCII characters are handled consistently; update the
file encoding (that contains the dot-sourced lines for 'test-helpers.ps1' and
the modulePath assignment and dot-sourcing of preflight-env.ps1) to UTF-8 with
BOM using your editor or a save-encoding step in your CI/tooling so the script
starts with the BOM marker.
- Around line 9-14: The comment for New-EnvSandbox is misleading about
IsNullOrEmpty distinguishing "not provided" vs "empty string"; update the
comment near the function and the param list (New-EnvSandbox, param $EnvContent)
to state that casting to [string] converts $null to "" and IsNullOrEmpty
therefore treats $null and empty string identically, e.g., replace the existing
line with a note like "注意:[string] cast 會把 $null 轉成 "",所以用 IsNullOrEmpty 統一處理
$null 與空字串" (and include the English equivalent if desired).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac691764-0e4d-4ba8-b93d-4c869ec7520d
📒 Files selected for processing (22)
docs/runbooks/one-click-deploy-smoke.mddocs/superpowers/plans/2026-05-26-one-click-deploy.mddocs/superpowers/specs/2026-05-26-one-click-deploy-design.mdscripts/deploy.ps1scripts/lib/deploy-report.ps1scripts/lib/host-native-launcher.ps1scripts/lib/kit-log-probe.ps1scripts/lib/preflight-docker.ps1scripts/lib/preflight-env.ps1scripts/lib/preflight-host-native.ps1scripts/lib/preflight-ports.ps1scripts/lib/preflight-volume-alignment.ps1scripts/tests/test-deploy-dryrun.ps1scripts/tests/test-deploy-report.ps1scripts/tests/test-helpers.ps1scripts/tests/test-host-native-launcher.ps1scripts/tests/test-kit-log-probe.ps1scripts/tests/test-preflight-docker.ps1scripts/tests/test-preflight-env.ps1scripts/tests/test-preflight-host-native.ps1scripts/tests/test-preflight-ports.ps1scripts/tests/test-preflight-volume-alignment.ps1
|
|
||
| 成功標準: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
請為 fenced code block 補上語言標記
這些 code fence 缺少語言標記（MD040），建議補上 powershell / json / text，避免 lint 噪音並提升可讀性。
Also applies to: 80-80, 106-106, 138-138, 263-263, 288-288, 312-312, 337-337, 388-388
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-26-one-click-deploy-design.md` at line 15,
Several fenced code blocks in the document are missing language markers (MD040);
locate the bare code fences (``` ) around the examples (the blocks noted in the
review) and add the appropriate language tag (e.g., powershell, json, or text)
after the opening backticks so each block becomes ```powershell / ```json /
```text as appropriate to the snippet content, ensuring lint passes and
readability is improved.
|
|
||
| brainstorming 過程拍板: | ||
|
|
||
| 1. **Approach B**:`deploy.ps1` 是薄 orchestrator(100-150 行),所有檢查 / 修復 / 啟動邏輯分到 `scripts\lib\*.ps1` modules。Module 全部 read-only(或在明確 phase 才動手),便於 Pester 單測。 |
There was a problem hiding this comment.
請將 orchestrator 行數描述改為符合現況實作
這裡寫「100-150 行薄 orchestrator」，但本 PR 的實作已是約 490 行；規格若不更新，會直接誤導後續驗收與重構決策。建議改成「模組化 orchestrator（目前約 490 行）」或移除具體行數上限。
As per coding guidelines, 「When wiki conflicts with implementation, prioritize implementation and update wiki accordingly」.
Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-26-one-click-deploy-design.md` at line 46,
Update the spec text that describes the orchestrator size to match the current
implementation: change the phrase "100-150 行薄 orchestrator" to either "模組化
orchestrator（目前約 490 行）" or remove the hard line-count limit; edit the same
phrasing at the other occurrence referenced (line 108) as well; ensure you
update the description that mentions deploy.ps1 and scripts\lib\*.ps1 so the
docs reflect the actual deploy.ps1 size and modularization rather than the
outdated 100–150 line claim.
| 3. ✅ `-DryRun` 不動任何真實狀態,可預覽 fix plan | ||
| 4. ✅ `-Build` 強制 rebuild docker image | ||
| 5. ✅ `.env` 已有 key 的實值在所有路徑下都不被覆寫 | ||
| 6. ✅ Layer 1 Pester 8 個 module test 全綠 |
There was a problem hiding this comment.
Acceptance Criteria 的測試框架名稱需修正一致
此處寫「Pester 8 個 module test 全綠」，但前文（Line 386）已明確說 repo 不用 Pester。請統一為「純 PowerShell + Assert-* helpers」。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-26-one-click-deploy-design.md` at line 538,
Update the Acceptance Criteria text that currently reads "Pester 8 個 module test
全綠" to instead state "純 PowerShell + Assert-* helpers 8 個 module test 全綠" (or
similar wording that uses "純 PowerShell + Assert-* helpers") so it matches the
earlier statement that the repo does not use Pester; ensure you replace that
exact phrase found in the Acceptance Criteria section.
| if ($hostNative.nvidiaDriver -eq 'MISSING') { $hardFails += 'nvidia_smi_missing' } | ||
| if ($hostNative.kitLauncher -eq 'MISSING_PATH'){ $hardFails += 'kit_launcher_missing' } | ||
| if ($volume.status -eq 'WRONG_LEAF') { $hardFails += 'runtime_storage_root_wrong_leaf' } |
There was a problem hiding this comment.
Hard fail 條件應尊重 -SkipKit / -SkipConversion
Line 182-184 目前即使使用者明確 -SkipKit 或 -SkipConversion，仍可能因 kit_launcher_missing / nvidia_smi_missing 直接失敗，和參數語意衝突。
建議修正
-if ($hostNative.nvidiaDriver -eq 'MISSING') { $hardFails += 'nvidia_smi_missing' }
-if ($hostNative.kitLauncher -eq 'MISSING_PATH'){ $hardFails += 'kit_launcher_missing' }
+if ((-not $SkipConversion -or -not $SkipKit) -and $hostNative.nvidiaDriver -eq 'MISSING') {
+ $hardFails += 'nvidia_smi_missing'
+}
+if ((-not $SkipKit) -and $hostNative.kitLauncher -eq 'MISSING_PATH') {
+ $hardFails += 'kit_launcher_missing'
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($hostNative.nvidiaDriver -eq 'MISSING') { $hardFails += 'nvidia_smi_missing' } | |
| if ($hostNative.kitLauncher -eq 'MISSING_PATH'){ $hardFails += 'kit_launcher_missing' } | |
| if ($volume.status -eq 'WRONG_LEAF') { $hardFails += 'runtime_storage_root_wrong_leaf' } | |
| if ((-not $SkipConversion -or -not $SkipKit) -and $hostNative.nvidiaDriver -eq 'MISSING') { | |
| $hardFails += 'nvidia_smi_missing' | |
| } | |
| if ((-not $SkipKit) -and $hostNative.kitLauncher -eq 'MISSING_PATH') { | |
| $hardFails += 'kit_launcher_missing' | |
| } | |
| if ($volume.status -eq 'WRONG_LEAF') { $hardFails += 'runtime_storage_root_wrong_leaf' } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'deploy.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.ps1` around lines 182 - 184, The hard-fail checks should
respect the CLI flags by only adding 'nvidia_smi_missing' when conversion is not
skipped and only adding 'kit_launcher_missing' when kit steps are not skipped:
update the conditions around $hostNative.nvidiaDriver and
$hostNative.kitLauncher so they include checks like (-not $SkipConversion) for
the NVIDIA check and (-not $SkipKit) for the kit launcher check before appending
to $hardFails; leave the $volume.status / 'runtime_storage_root_wrong_leaf'
check unchanged.
| try { docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile pull } finally { Pop-Location } | ||
| $fixActions++ |
There was a problem hiding this comment.
docker compose pull 缺少失敗檢查
Line 365-366 執行 pull 後不檢查 $LASTEXITCODE，pull 失敗會被當成成功流程繼續，容易造成後續啟動失敗且訊號延遲。
建議修正
Push-Location $RepoRoot
try { docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile pull } finally { Pop-Location }
+$pullExit = $LASTEXITCODE
+if ($pullExit -ne 0) {
+ Write-DeployTag -Tag 'fail' -Message "docker compose pull failed (exit=$pullExit)" -LogPath $LogPath | Out-Null
+ Print-FinalSummary -ExitCode 2 -FailedPhase 'Phase 2 (docker pull)'
+ exit 2
+}
$fixActions++📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile pull } finally { Pop-Location } | |
| $fixActions++ | |
| try { docker compose -f compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile pull } finally { Pop-Location } | |
| $pullExit = $LASTEXITCODE | |
| if ($pullExit -ne 0) { | |
| Write-DeployTag -Tag 'fail' -Message "docker compose pull failed (exit=$pullExit)" -LogPath $LogPath | Out-Null | |
| Print-FinalSummary -ExitCode 2 -FailedPhase 'Phase 2 (docker pull)' | |
| exit 2 | |
| } | |
| $fixActions++ |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'deploy.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.ps1` around lines 365 - 366, After running the docker compose
pull command, check $LASTEXITCODE and treat non-zero as failure: capture the
exit code immediately after the try/finally block that runs "docker compose -f
compose.runtime-manager.yml -f compose.host-kit.yml --env-file $resolvedEnvFile
pull", and if $LASTEXITCODE -ne 0, write an error (e.g., Write-Error or
Write-Host) and Exit with that code (or throw) so the script stops and does not
increment $fixActions; ensure Pop-Location still runs (keep the finally) but
perform the exit/check before the line that increments $fixActions.
| Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue | ||
| } else { | ||
| Write-DeployTag -Tag 'ask' -Message $prompt -LogPath $LogPath | Out-Null | ||
| $response = Read-Host 'y/N' | ||
| if ($response -match '^[Yy]') { | ||
| Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue | ||
| Write-DeployTag -Tag 'fix' -Message "killed PID $($sp.pid)" -LogPath $LogPath | Out-Null |
There was a problem hiding this comment.
強制停止 stranger PID 後需驗證是否真的成功
Line 398/403 用 Stop-Process ... -ErrorAction SilentlyContinue 後直接前進；若因權限或 PID 失效未成功停止，Phase 4 會在更晚失敗且訊息不精準。
建議修正
-Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue
+Stop-Process -Id $sp.pid -Force -ErrorAction SilentlyContinue
+Start-Sleep -Milliseconds 300
+if (Get-Process -Id $sp.pid -ErrorAction SilentlyContinue) {
+ Write-DeployTag -Tag 'fail' -Message "failed to stop PID $($sp.pid)" -LogPath $LogPath | Out-Null
+ Print-FinalSummary -ExitCode 3 -FailedPhase 'Phase 3 (kill process failed)'
+ exit 3
+}🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'deploy.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.ps1` around lines 398 - 404, After calling Stop-Process on
$sp.pid (both places where Stop-Process -Id $sp.pid -Force -ErrorAction
SilentlyContinue is used), verify the PID was actually terminated: attempt to
Get-Process -Id $sp.pid (or Wait-Process with a short timeout) and if the
process still exists or retrieval fails due to permissions, write a failure
DeployTag via Write-DeployTag (include the PID and the error/reason), and either
retry or abort the deploy flow so Phase 4 does not continue with an inaccurate
state; update the Stop-Process call sites and the subsequent logging to perform
this existence check and branch on success/failure accordingly.
| if ($hostNative.venv -eq 'WRONG_VERSION') { | ||
| $prompt = '.venv has wrong Python version (<3.11). Recreate? (will delete .venv) (y/N)' | ||
| if ($Force) { | ||
| Write-DeployTag -Tag 'fix' -Message "$prompt -> y (--Force)" -LogPath $LogPath | Out-Null | ||
| Remove-Item -LiteralPath (Join-Path $RepoRoot '.venv') -Recurse -Force | ||
| & python -m venv (Join-Path $RepoRoot '.venv') | ||
| } else { | ||
| Write-DeployTag -Tag 'ask' -Message $prompt -LogPath $LogPath | Out-Null | ||
| $response = Read-Host 'y/N' | ||
| if ($response -match '^[Yy]') { | ||
| Remove-Item -LiteralPath (Join-Path $RepoRoot '.venv') -Recurse -Force | ||
| & python -m venv (Join-Path $RepoRoot '.venv') | ||
| } else { |
There was a problem hiding this comment.
重建 .venv 後未安裝相依套件
Line 417-425 在 WRONG_VERSION 分支只重建 .venv，但沒有重跑 pip install -r ...。這會讓 Phase 4 以缺套件狀態啟動 host-native 服務。
建議重用 Line 214-225 的 requirements 安裝流程（或抽成共用函式）並在失敗時沿用 ExitCode 2。
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'deploy.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.ps1` around lines 413 - 425, The WRONG_VERSION branch
recreates the virtualenv (checks hostNative.venv, uses Remove-Item and & python
-m venv) but does not reinstall dependencies, leaving host-native without
required packages; after both places where the script creates the .venv (both
the $Force and interactive branches that call & python -m venv) invoke the same
requirements-install logic used elsewhere in the script (the existing pip
install -r ... sequence), reuse or extract that installation into a shared
routine and call it after venv creation, and if the pip install fails
return/exit with the same ExitCode 2 as the original requirements-install flow
so failures are propagated.
| Assert-Throws { | ||
| Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path (New-TestSandbox) 'd.log') | ||
| } 'unknown tag throws' |
There was a problem hiding this comment.
避免在 Assert-Throws 內建立未清理的 sandbox
Line 39 目前會建立一次 New-TestSandbox 但沒有 Remove-TestSandbox，長期跑測試容易累積暫存目錄。建議把 sandbox 提前建立並在 finally 清理。
建議修正
-Assert-Throws {
- Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path (New-TestSandbox) 'd.log')
-} 'unknown tag throws'
+$sandbox = New-TestSandbox -Prefix 'deploy-report'
+try {
+ Assert-Throws {
+ Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path $sandbox 'd.log')
+ } 'unknown tag throws'
+}
+finally { Remove-TestSandbox -Path $sandbox }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert-Throws { | |
| Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path (New-TestSandbox) 'd.log') | |
| } 'unknown tag throws' | |
| $sandbox = New-TestSandbox -Prefix 'deploy-report' | |
| try { | |
| Assert-Throws { | |
| Write-DeployTag -Tag 'unknown' -Message 'x' -LogPath (Join-Path $sandbox 'd.log') | |
| } 'unknown tag throws' | |
| } | |
| finally { Remove-TestSandbox -Path $sandbox } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'test-deploy-report.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/tests/test-deploy-report.ps1` around lines 38 - 40, The test creates
a sandbox inside Assert-Throws via New-TestSandbox causing leftover temp dirs;
refactor so you call New-TestSandbox before the Assert-Throws, capture the
sandbox path in a variable, then call Write-DeployTag inside Assert-Throws using
that path, and ensure you wrap the test in try/finally (or use
try/catch/finally) to always call Remove-TestSandbox with the captured sandbox
path in the finally block to guarantee cleanup; reference the functions
Assert-Throws, New-TestSandbox, Remove-TestSandbox and Write-DeployTag when
making these changes.
| $result = Test-PortAvailability -RepoRoot (New-TestSandbox -Prefix 'preflight-ports') ` | ||
| -PortLookup { param($port) $null } ` | ||
| -ProcessNameLookup { param($procId) $null } | ||
| Assert-True ($result.docker.Count -eq 2) 'docker has 2 ports' | ||
| Assert-True ($result.hostNative.Count -eq 3) 'hostNative has 3 ports' | ||
| foreach ($p in @($result.docker; $result.hostNative)) { | ||
| Assert-Equal 'FREE' $p.status "port $($p.port) FREE" | ||
| } | ||
| Write-TestPass 'all ports free' |
There was a problem hiding this comment.
補上 Test 1 sandbox 清理，避免測試殘留目錄。
Line 9 目前直接在參數中建立 sandbox，沒有對應 Remove-TestSandbox，會造成暫存資料累積。
建議修正
-# Test 1: 全 FREE
-$result = Test-PortAvailability -RepoRoot (New-TestSandbox -Prefix 'preflight-ports') `
- -PortLookup { param($port) $null } `
- -ProcessNameLookup { param($procId) $null }
-Assert-True ($result.docker.Count -eq 2) 'docker has 2 ports'
-Assert-True ($result.hostNative.Count -eq 3) 'hostNative has 3 ports'
-foreach ($p in @($result.docker; $result.hostNative)) {
- Assert-Equal 'FREE' $p.status "port $($p.port) FREE"
-}
-Write-TestPass 'all ports free'
+# Test 1: 全 FREE
+$sandbox = New-TestSandbox -Prefix 'preflight-ports'
+try {
+ $result = Test-PortAvailability -RepoRoot $sandbox `
+ -PortLookup { param($port) $null } `
+ -ProcessNameLookup { param($procId) $null }
+ Assert-True ($result.docker.Count -eq 2) 'docker has 2 ports'
+ Assert-True ($result.hostNative.Count -eq 3) 'hostNative has 3 ports'
+ foreach ($p in @($result.docker; $result.hostNative)) {
+ Assert-Equal 'FREE' $p.status "port $($p.port) FREE"
+ }
+ Write-TestPass 'all ports free'
+}
+finally { Remove-TestSandbox -Path $sandbox }🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'test-preflight-ports.ps1'
(PSUseBOMForUnicodeEncodedFile)
[warning] 10-10: The parameter 'port' has been declared but not used.
(PSReviewUnusedParameter)
[warning] 11-11: The parameter 'procId' has been declared but not used.
(PSReviewUnusedParameter)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/tests/test-preflight-ports.ps1` around lines 9 - 17, The test creates
a sandbox inline via New-TestSandbox when calling Test-PortAvailability but
never removes it; change the test to capture the sandbox handle (e.g. assign
New-TestSandbox to a variable), pass that sandbox/root into
Test-PortAvailability, and ensure you call Remove-TestSandbox on that handle at
the end (preferably in a try/finally so Remove-TestSandbox runs even if
assertions fail); reference the New-TestSandbox call, the Test-PortAvailability
invocation and add a Remove-TestSandbox cleanup step tied to the created
sandbox.
實機 dispatch IFC conversion job 失敗:
error.code = invalid_ifc_input
error.message = local IFC path is outside storage_root:
C:\Repos\active\iot\AI-BIM-governance\storage\ifc-cache\...\source.ifc
ifc2usdc_powershell_adapter.py 用 env var STORAGE_ROOT(line 75-76)做
dispatch payload host_local_path 必須在其下的 sandbox 根,沒設就 fall
back 到 Path.cwd()(= bim-streaming-server 子目錄)。coordinator 給的
host_local_path = <RuntimeStorageRoot>\ifc-cache\<job>\source.ifc,
所以 STORAGE_ROOT 必須對齊 RuntimeStorageRoot,IFC path 才落在 sandbox 內。
之前只設 STREAMING_CONVERSION_WORK_DIR(launcher 用,不是 storage_root)。
加 STORAGE_ROOT = RuntimeStorageRoot 讓對齊真的生效。
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
PR #124 的 deploy.ps1 + scripts\lib\* 是行為 / repo-boundary 變更, pr-review-agent gate 要求 OpenSpec change id 或 documented exception。 這次補建 OpenSpec change(proposal / design / tasks / specs/one-click-deploy-hybrid/spec.md), 讓 PR 通過 CI gate。 內容: - proposal:Mode C hybrid 一鍵部屬的「為何 / 變更內容 / 能力 / 影響」 - design:Phase 0-5 流程 / 退出碼 / Volume 對齊方案 A / 12 個 PowerShell 5.1 trap - tasks:9 章節對應實際 commit history,9.1-9.7 全綠,9.8 PR review 補 - specs/one-click-deploy-hybrid/spec.md:9 個 capability requirement + 對應 scenario `openspec validate add-one-click-deploy-hybrid` 本機 pass。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de2472749a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (-not $docker.cliVersion) { $hardFails += 'docker_cli_missing' } | ||
| if (-not $docker.composeV2) { $hardFails += 'docker_compose_v2_missing' } | ||
| if (-not $docker.engineRunning) { $hardFails += 'docker_engine_not_running' } | ||
| if (-not $docker.envFile) { $hardFails += 'env_file_missing_entirely' } |
There was a problem hiding this comment.
Phase 1 hard-fails on -not $docker.envFile, but $docker.envFile only reflects auto-discovery of .env.web-plane.host-kit{.example} and ignores a valid user-supplied -EnvFile (which is already resolved into $resolvedEnvFile at lines 108-110). As a result, deploy.ps1 -EnvFile <custom-file> can exit with env_file_missing_entirely even when the custom env file exists and should be used, making the advertised custom env-file mode unusable in repos/environments without the default host-kit env filenames.
Useful? React with 👍 / 👎.
| if ($hostNative.venv -eq 'MISSING') { | ||
| Write-DeployTag -Tag 'fix' -Message 'creating .venv via python -m venv' -LogPath $LogPath | Out-Null | ||
| & python -m venv (Join-Path $RepoRoot '.venv') |
There was a problem hiding this comment.
The Phase 2 .venv bootstrap runs unconditionally when hostNative.venv is MISSING, even if both -SkipKit and -SkipConversion are set. In that docker-only mode, missing Python or pip install failures still abort deploy in Phase 2, although no host-native process will be started in Phase 4. This makes the skip flags ineffective for reduced-mode recovery/startup paths and should be gated to only run when at least one host-native stage is enabled.
Useful? React with 👍 / 👎.
| if (-not [int]::TryParse($raw.Trim(), [ref]$procId)) { return $false } | ||
| return ($null -ne (& $GetProcessFn $procId)) |
There was a problem hiding this comment.
Test-AlreadyRunning treats any live process ID from *.pid as the target service, without checking process identity. If a stale pidfile survives and that PID is later reused by an unrelated process, Phase 4 will incorrectly skip starting conversion/Kit as “already running,” leaving required services down. This is a real false-positive path on long-lived hosts and should validate executable/command-line (or listening port) before returning true.
Useful? React with 👍 / 👎.
| if (-not (Probe-Url -Name 'coordinator' -Url 'http://127.0.0.1:8004/health')) { $verifyFails += 'coordinator' } | ||
| if (-not (Probe-Url -Name 'viewer' -Url 'http://127.0.0.1:5173')) { $verifyFails += 'viewer' } |
There was a problem hiding this comment.
Phase 5 always verifies coordinator/viewer on 8004 and 5173, but Phase 4c starts containers with ports coming from the selected env file (COORDINATOR_PORT / VIEWER_PORT). When a custom env file changes either port, healthy services are marked unreachable and -StrictPostVerify incorrectly fails deployment with exit 5. The probe targets need to follow the resolved env-derived host ports used by compose.
Useful? React with 👍 / 👎.
| ) | ||
|
|
||
| $dockerPorts = @(8004, 5173) | ||
| $hostNativePorts = @(49100, 49101, 47998) |
There was a problem hiding this comment.
Port preflight is hardcoded to host-native ports 49100 and 47998, even though deploy.ps1 accepts -KitSignalPort and -KitMediaPort and launches Kit with those values. In custom-port runs, Phase 1/3 audit the wrong ports, so real conflicts on the requested Kit ports are missed and startup later fails in Phase 4b instead of being caught and handled by the interactive guard.
Useful? React with 👍 / 👎.
| foreach ($pidFile in Get-ChildItem -LiteralPath $RunDir -Filter '*.pid' -ErrorAction SilentlyContinue) { | ||
| $procId = (Get-Content $pidFile.FullName | Select-Object -First 1).Trim() | ||
| Write-Host " > $($pidFile.BaseName) PID $procId" |
There was a problem hiding this comment.
The failure summary prints every scripts\.run\*.pid entry without checking process liveness, so stale pidfiles are reported as if they are still running. This contradicts the summary intent (What might be running) and sends operators toward incorrect cleanup/debug paths after a failed deploy. Re-check each PID before printing it in the failure list.
Useful? React with 👍 / 👎.
…ms-spectator-capacity；project-risks-mitigation 保留 active - 兩個 change 對應實作已在 main (PR #124 hybrid orchestrator、PR #133 LAN viewer CORS + multi spectator handoff、後續 hotfix 9f4d541 / 4752ceb / 8c84fe9)，task 9.8 / 4.4 收尾並備註人工驗證來源。 - archive 同步 spec delta：新增 one-click-deploy-hybrid spec，更新 docker-web-plane-host-native-kit / local-coordinator-ifc-ready-intake-boundary / multi-artifact-kit-routing / runtime-verification-task-status。 - project-risks-mitigation 加 Archive Hold 備註，三項原因：5 個 ADDED Requirements 缺 SHALL/MUST 不過 strict validate、對應實作未進 main、2.1/2.2/2.3 follow-up 仍 pending；保留 active 等實作 PR。 - openspec validate --all --strict：31 spec/change 通過，僅 project-risks-mitigation 預期 fail。
Summary
新建
scripts\deploy.ps1+scripts\lib\*.ps1(8 個 module)+scripts\tests\test-*.ps1(9 個 test file),讓 Mode C(web-plane Docker + host-native Kit)一條指令冷啟到 demo-ready。完全不動start-all.ps1/start-web-plane-docker.ps1/start-runtime-manager-docker.ps1/compose.*.yml(spec §4 / §13 acceptance #10)。Spec / Plan:
docs/superpowers/specs/2026-05-26-one-click-deploy-design.md(commitc900bab)docs/superpowers/plans/2026-05-26-one-click-deploy.md(commitc45d27e)架構
8 個 lib module:
deploy-report/preflight-docker/preflight-host-native/preflight-env/preflight-ports/preflight-volume-alignment/host-native-launcher/kit-log-probe。退出碼
0 / 1 / 2 / 3 / 4 / 5(對齊 spec §6.3,Phase 4 內部 stage 在 log 標stage=4a/4b/4c)。實機驗證(本機 Windows + NVIDIA GPU)
1m 42s全綠,Phase 1-5 全[ok],Kit 抓app ready,verify 全 200readyState=4+ 影像 > 0)5s,Phase 2 全[skip],Phase 4 全[skip],verify 全 200-Buildforce rebuild蓋章紀錄見
docs/runbooks/one-click-deploy-smoke.mdSmoke Pass Log。Layer 1 unit tests + Layer 2 integration
9 個 test file 全綠(repo 風格:純 PowerShell +
Assert-*helpers,不引入 Pester):GitNexus impact
跑
npx gitnexus analyze --embeddings重 index 後:detect_changes scope=compare base=main:Scope
git diff --name-only main...HEAD(22 個檔):start-all.ps1/start-runtime-manager-docker.ps1/start-web-plane-docker.ps1/compose.*.yml完全沒動。Out of scope(留 v2 / follow-up PR)
.github/workflows/deploy-unit-test.yml)[ask]後 Phase 3 才 whitelist 過濾 — cosmetic,可改成 Phase 1 直接[skip]已知 fix iterations(實機暴露的 PowerShell trap)
$pidautomatic variable(param($pid)不 bind):rename$procId$Argsautomatic variable:rename$ArgListAssert-Equal $null Expected:用Assert-True ($null -eq …)寫法[string] $param = \$nullcast 成 empty string:用IsNullOrEmpty判斷\$ErrorActionPreference='Stop'+ native cmd stderr promotion:改ContinueWrite-Host走 Information stream:test 用*>&1抓所有 streamStart-Process隔離 PowerShell processapp ready(小寫):加入 keyword listGet-PidsFromRunDir沿 ParentProcessId 遞迴展開子孫Get-NetTCPConnection.OwningProcessUInt32 vs hashtable Int32 key type mismatch:統一[int]castTest plan(reviewer 補驗)
readyState=4+ 影像尺寸 > 0).\scripts\deploy.ps1 -Build確認 force rebuild.\scripts\deploy.ps1確認 Phase 1[fail]退 1start-all.ps1/start-web-plane-docker.ps1/start-runtime-manager-docker.ps1/compose.*.yml完全沒動(git diff main...HEAD --stat不含這些路徑)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation