fix(deploy)：rebuild 先停部署區 locker 服務再 git clean -fdx（解 EINVAL） - #268
Conversation
rebuild-test-deploy 在 clean 前先以 deploy-zone scripts\.run pidfile 為界 停掉 kit / conversion / governance 三個 host-native 服務,釋放 _build 下的 鎖定 log handle,避免 git clean -fdx 撞 EINVAL(Invalid argument)。 - 新增 $ServiceStopper 注入 seam(對齊 $CommandRunner/$DeployRunner null-default); 預設分支 dot-source host-native-launcher.ps1 呼叫 Stop-HostNativeService。 - 停服務為 best-effort(失敗只 WARNING 不 abort);真正安全網是 clean 3 次重試, 最後一次仍失敗才 re-throw,續走既有 env-restore catch。 - guardrail:只碰本部署區 pidfile 記錄的 PID-tree,不觸及 hub.exe 等區外行程。 - 測試:新增 stop-順序(三服務皆停且皆在 clean 之前)+ clean 暫時性失敗重試; 並修既有 cleanFailureRunner 為 idempotent(-ErrorAction SilentlyContinue), 否則新重試迴圈會讓最終錯誤變成 missing-file 而非 git 輸出。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKZSJPSihwExrC1QQBZv8v
記錄 git clean -fdx EINVAL 根因(存活的 kit.exe 鎖 _build log)、 pre-clean stop + retry 設計、治理護欄(只停部署區自己 pidfile 的服務、 不碰 hub.exe)、驗證與 impact。滿足 pr-review-agent 對 code 變更的 formal spec evidence 要求。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKZSJPSihwExrC1QQBZv8v
📝 WalkthroughWalkthroughAdds a mitigation for intermittent ChangesRebuild Cleanup Lock Mitigation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Rebuild as Invoke-TestDeployRebuild
participant Stopper as ServiceStopper/Stop-HostNativeService
participant Git as git clean -fdx
participant Deploy as Deploy step
Rebuild->>Stopper: stop bim-streaming-server
Rebuild->>Stopper: stop bim-streaming-conversion-service
Rebuild->>Stopper: stop governance-service
Rebuild->>Git: attempt 1: clean -fdx
Git-->>Rebuild: fail (Invalid argument)
Rebuild->>Rebuild: sleep 1s
Rebuild->>Git: attempt 2: clean -fdx
Git-->>Rebuild: success
Rebuild->>Deploy: proceed to deploy
Deploy-->>Rebuild: exit code
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: 57c1b6cdbe
ℹ️ 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".
| $deployZoneRunDir = Join-Path $deployRoot 'scripts\.run' | ||
| foreach ($serviceName in @('bim-streaming-server', 'bim-streaming-conversion-service', 'governance-service')) { | ||
| try { | ||
| & $effectiveServiceStopper $serviceName $deployZoneRunDir | Out-Null |
There was a problem hiding this comment.
When scripts\.run\*.pid survives from a previous crashed deploy and Windows has since reused that PID, this new pre-clean path passes the stale value straight to Stop-HostNativeService; that helper only reads the integer pidfile and recursively calls Stop-Process -Force, without checking that the process command line/executable is under $deployRoot or is one of these services. Because the rebuild now invokes it automatically before every clean, a stale pidfile can kill an unrelated process tree; validate the PID belongs to the deployment/service or discard stale pidfiles before stopping it.
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: 3
🧹 Nitpick comments (1)
scripts/tests/test-rebuild-test-deploy.ps1 (1)
317-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the deploy-zone run directory in the stopper test.
This only proves service names and ordering. A regression that passed the wrong run directory into
-ServiceStopperwould still pass, even though the implementation guardrail is deploy-zone-scoped pidfiles.Suggested assertion
$stoppedServices = New-Object 'System.Collections.Generic.List[string]' + $stopRunDirs = New-Object 'System.Collections.Generic.List[string]' $script:stoppedServices = $stoppedServices + $script:stopRunDirs = $stopRunDirs $stopOrderStopper = { param([string] $ServiceName, [string] $ServiceRunDir) $script:stopOrderLog.Add("stop:$ServiceName") | Out-Null $script:stoppedServices.Add($ServiceName) | Out-Null + $script:stopRunDirs.Add($ServiceRunDir) | Out-Null }.GetNewClosure() @@ + $expectedRunDir = Join-Path $stopOrderRoot 'scripts\.run' + foreach ($runDir in $stopRunDirs) { + Assert-Equal $expectedRunDir $runDir 'pre-clean stop uses the deploy-zone run dir' + }Also applies to: 329-337
🤖 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-rebuild-test-deploy.ps1` around lines 317 - 321, The stopper test only verifies service names and ordering, so it can miss a regression where the wrong run directory is passed into -ServiceStopper. Update the test around stopOrderStopper to assert that the ServiceRunDir parameter matches the expected deploy-zone run directory for each stopped service, using the existing stopOrderLog/stoppedServices setup to keep the current ordering checks intact.
🤖 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/rebuild-stop-locker-before-clean.md`:
- Around line 7-10: Add a language tag to the fenced log snippet in the
rebuild-stop-locker-before-clean spec so markdownlint stops flagging it. Update
the existing log fence to use an appropriate language identifier for
shell/output-style logs, and keep the rest of the content unchanged.
- Around line 45-47: The Impact section in rebuild-stop-locker-before-clean.md
should not present GitNexus as the authority; rephrase the note so GitNexus
detect-changes is described only as supporting validation, while the
implementation/source code remains the source of truth. Update the wording in
the Impact text to align with the repository’s source-of-truth order and keep
the same context around Invoke-TestDeployRebuild,
scripts/dev/rebuild-test-deploy.ps1, and the optional pre-clean stop/retry
behavior.
In `@scripts/lib/rebuild-test-deploy.ps1`:
- Around line 298-299: The new warning/retry output in rebuild-test-deploy.ps1
still uses bare Write-Host, which should be routed through
scripts/lib/StructLog.psm1 instead. Update the warning/retry paths around the
best-effort stop and clean-retry handling to use the existing structured logging
helpers already available in the script, keeping the same message content but
emitting it through StructLog for consistent script boundary logging. Refer to
the rebuild-test-deploy flow and the stop/retry logic so the new messages are
handled the same way as the rest of the structured output.
---
Nitpick comments:
In `@scripts/tests/test-rebuild-test-deploy.ps1`:
- Around line 317-321: The stopper test only verifies service names and
ordering, so it can miss a regression where the wrong run directory is passed
into -ServiceStopper. Update the test around stopOrderStopper to assert that the
ServiceRunDir parameter matches the expected deploy-zone run directory for each
stopped service, using the existing stopOrderLog/stoppedServices setup to keep
the current ordering checks intact.
🪄 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: b61073bd-b566-490d-9d12-c668cf736dcf
📒 Files selected for processing (3)
docs/superpowers/specs/rebuild-stop-locker-before-clean.mdscripts/lib/rebuild-test-deploy.ps1scripts/tests/test-rebuild-test-deploy.ps1
| ``` | ||
| warning: failed to remove bim-streaming-server/_build/.../logs/Kit/kit_*.log: Invalid argument | ||
| git clean -fdx failed with exit code 1 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language tag to the fenced log snippet.
This new code fence is currently missing a language, so markdownlint will keep flagging it.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 7-7: 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/rebuild-stop-locker-before-clean.md` around lines 7 -
10, Add a language tag to the fenced log snippet in the
rebuild-stop-locker-before-clean spec so markdownlint stops flagging it. Update
the existing log fence to use an appropriate language identifier for
shell/output-style logs, and keep the rest of the content unchanged.
Source: Linters/SAST tools
| ## Impact | ||
|
|
||
| `Invoke-TestDeployRebuild` 的呼叫者為 `scripts/dev/rebuild-test-deploy.ps1` 包裝器與本測試；新增參數為可選（預設 null），不破壞既有簽章；pre-clean stop 與 retry 皆為附加行為。Blast radius：LOW。（本機 GitNexus 索引 stale，以上為推理；CI pr-review-agent 之 GitNexus detect-changes 為權威 impact 檢查。） |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid calling GitNexus the authority in a docs spec.
That sentence elevates a tooling result above the repo’s documented source-of-truth order. Rephrase it as supporting validation, with implementation remaining authoritative.
As per coding guidelines, "Files within the docs/ directory must not be treated as authoritative sources for program code behavior or API specifications" and "MUST align with source of truth order: code > contracts > AGENTS boundary > wiki (per root AGENTS.md §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 `@docs/superpowers/specs/rebuild-stop-locker-before-clean.md` around lines 45 -
47, The Impact section in rebuild-stop-locker-before-clean.md should not present
GitNexus as the authority; rephrase the note so GitNexus detect-changes is
described only as supporting validation, while the implementation/source code
remains the source of truth. Update the wording in the Impact text to align with
the repository’s source-of-truth order and keep the same context around
Invoke-TestDeployRebuild, scripts/dev/rebuild-test-deploy.ps1, and the optional
pre-clean stop/retry behavior.
Source: Coding guidelines
| Write-Host "[rebuild-test-deploy] WARNING best-effort stop of '$serviceName' failed (continuing to clean-retry): $($_.Exception.Message)" | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use structured logging for the new warning/retry paths.
These added messages extend bare Write-Host usage in a scripts/**/*.ps1 file. Please route them through scripts/lib/StructLog.psm1 so rebuild output stays consistent with the scripting boundary.
As per coding guidelines, "Use scripts/lib/StructLog.psm1 for structured logging output; do not replace with bare Write-Host calls".
Also applies to: 314-315
🤖 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/rebuild-test-deploy.ps1` around lines 298 - 299, The new
warning/retry output in rebuild-test-deploy.ps1 still uses bare Write-Host,
which should be routed through scripts/lib/StructLog.psm1 instead. Update the
warning/retry paths around the best-effort stop and clean-retry handling to use
the existing structured logging helpers already available in the script, keeping
the same message content but emitting it through StructLog for consistent script
boundary logging. Refer to the rebuild-test-deploy flow and the stop/retry logic
so the new messages are handled the same way as the rest of the structured
output.
Source: Coding guidelines
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes an intermittent git clean -fdx ... Invalid argument (EINVAL) failure in rebuild-test-deploy.ps1 -Build. The root cause is that a kit.exe (and sibling host-native services) from a prior rebuild keeps a locked handle on gitignored _build/.../logs/Kit/*.log files, so git clean -fdx cannot unlink them and the rebuild aborts before deploy.ps1 runs. The fix stops the deploy-zone's own pidfile-tracked services before the clean, and wraps the clean in a bounded retry to absorb the transient handle-release race. This sits entirely within the test-deploy rebuild helper and leaves the deploy.ps1 golden path untouched.
Changes:
- Insert a best-effort, pidfile-scoped pre-clean stop of
bim-streaming-server,bim-streaming-conversion-service, andgovernance-service(via a new injectable-ServiceStopperseam) betweengit reset --hardandgit clean -fdx. - Wrap
git clean -fdxin a 3-attempt retry with ~1s backoff that re-throws on the final failure into the existing env-restore catch. - Add tests asserting stop-before-clean ordering and clean-retry-to-deploy, make the existing
cleanFailureRunneridempotent, and document the design/guardrails in a new spec.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/lib/rebuild-test-deploy.ps1 | Adds -ServiceStopper seam, pre-clean best-effort service stop, and bounded clean retry within the existing try/catch. |
| scripts/tests/test-rebuild-test-deploy.ps1 | Adds stop-order and clean-retry assertions; makes the existing clean-failure mock idempotent for the retry loop. |
| docs/superpowers/specs/rebuild-stop-locker-before-clean.md | New design spec documenting the problem, sequence, guardrails, and verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,47 @@ | |||
| # Spec：rebuild-test-deploy 於 git clean 前先停部署區 locker 服務 | |||
PR #281（rebuild-test-deploy 排除 Kit 執行期 log 跳過 EINVAL 幽靈鎖） 改了 scripts/ 底下的程式碼，依治理規則需要正式 spec 佐證，先前 直接開 PR 漏補；補上 docs/superpowers/specs/2026-07-02-rebuild- clean-exclude-kit-logs.md，內容涵蓋問題/設計/驗證/impact，並連結 回前一輪 #268 的 rebuild-stop-locker-before-clean.md spec。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GqZ6RWEhngbgUULeewjy1
* fix(deploy)：rebuild-test-deploy 排除 Kit 執行期 log 路徑跳過 EINVAL 幽靈鎖 git clean -fdx 清除 bim-streaming-server/_build/**/logs 下的 Kit 執行期 log 檔時，偶發撞上查無擁有行程的孤兒 OS handle（Defender/host-native 服務皆已排除嫌疑，60 秒輪詢仍鎖死），3 次重試機制吃不掉這種長時間鎖。 這些 log 純屬診斷輸出、非建置狀態，故從 clean 排除而非繼續重試。 已用真實鎖檔情境端到端驗證：套用前兩次卡在同一檔案並丟出例外； 套用後完整跑過 rebuild-test-deploy 進入 deploy.ps1 Phase 1/2。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GqZ6RWEhngbgUULeewjy1 * chore：觸發 PR body 重新檢查（補 Deploy Path Verification 表格） Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GqZ6RWEhngbgUULeewjy1 * docs(specs)：補正式 spec 消 pr-review-agent missing_openspec blocker PR #281（rebuild-test-deploy 排除 Kit 執行期 log 跳過 EINVAL 幽靈鎖） 改了 scripts/ 底下的程式碼，依治理規則需要正式 spec 佐證，先前 直接開 PR 漏補；補上 docs/superpowers/specs/2026-07-02-rebuild- clean-exclude-kit-logs.md，內容涵蓋問題/設計/驗證/impact，並連結 回前一輪 #268 的 rebuild-stop-locker-before-clean.md spec。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GqZ6RWEhngbgUULeewjy1 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
摘要
rebuild-test-deploy.ps1 -Build偶發卡在git clean -fdx ... Invalid argument：上一輪重建起的kit.exe仍存活、鎖住部署區bim-streaming-server/_build/.../logs/Kit/kit_*.log，clean 刪不掉 → Windows EINVAL → 重建在 deploy 前 abort。（log 中.env的刪除／還原是Save/Restore-TestDeployEnvSnapshot既有設計，非本問題來源。）本 PR 在
git clean -fdx之前先停部署區自己 pidfile 記錄的三個 host-native 服務（Kit／conversion／governance）以釋放 log handle，並對 clean 加重試吸收瞬時鎖競速；附治理護欄（只停部署區自己的服務、不碰hub.exe等區外行程）。設計依據見docs/superpowers/specs/rebuild-stop-locker-before-clean.md。變更
scripts/lib/rebuild-test-deploy.ps1：pre-cleanStop-HostNativeService（best-effort）+ retry-only-clean（3×、約 1s backoff、最後仍失敗才 re-throw）+-ServiceStopper注入 seam。scripts/tests/test-rebuild-test-deploy.ps1：新增 stop-順序與 clean-重試斷言；修既有cleanFailureRunner為 idempotent（-ErrorAction SilentlyContinue）。docs/superpowers/specs/rebuild-stop-locker-before-clean.md：正式設計依據。Deploy Path Verification
治理護欄
scripts\.run\*.pid記錄的 PID-tree」；Stop-HostNativeService無 pidfile 即 no-op；hub.exe（Omniverse，位於C:\Users\...\ov\pkg，非部署區底下、非三服務子進程）永不被停。deploy.ps1golden path 未動。🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
git cleanfailures caused by locked files.