Repository navigation
feat(boot): Issue #70 — Step 3 Memory Restore を McpHealthCheck.psm1 でワイヤリング - #81
Conversation
…ワイヤリング - Start-ClaudeOS.ps1 に McpHealthCheck.psm1 を Import-Module - Invoke-StepMemoryRestore 関数を実装: - .mcp.json 未設定 → SKIP - memory server 未設定 → SKIP - 設定あり → OK (エントリ数・ファイルパスをレポート) - 例外 → FAIL - Step 3 のプレースホルダー呼び出しを Invoke-StepMemoryRestore に置換 - StartScripts.Tests.ps1: Step 3 を SKIP リストから除外し専用テストを追加 Closes #70 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 Walkthroughウォークスルー
Changes
予測される関連イシュー
推定コードレビュー工数🎯 3 (中程度) | ⏱️ 約25分 ポエム
🚥 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)
Comment |
There was a problem hiding this comment.
Pull request overview
Implements Boot Step 3 (“Memory Restore”) wiring in Start-ClaudeOS.ps1 using McpHealthCheck.psm1, and updates Pester tests to reflect that Step 3 is no longer a placeholder.
Changes:
- Import
McpHealthCheck.psm1and addInvoke-StepMemoryRestorefor Step 3 decisioning (SKIP/OK/FAIL). - Replace the previous Step 3 placeholder call with
Invoke-StepMemoryRestore. - Update
StartScripts.Tests.ps1to stop expecting Step 3 placeholder-SKIP and add a Step 3 execution assertion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| scripts/main/Start-ClaudeOS.ps1 | Adds Step 3 implementation and wires it into the boot flow via MCP health reporting + memory file reporting. |
| tests/StartScripts.Tests.ps1 | Updates dry-run flow assertions and adds a dedicated Step 3 “Memory Restore is executed” test. |
| function Invoke-StepMemoryRestore { | ||
| param([string]$Root) | ||
| Write-BootStep 3 'Memory Restore' | ||
| try { |
There was a problem hiding this comment.
The script header comment still states that Step 3 is a placeholder that emits SKIP (it lists steps 3/5/6/7/8 as placeholders). Since Step 3 is now implemented via Invoke-StepMemoryRestore, please update the top-of-file description to avoid misleading users and future maintainers.
| if (-not $report.configured) { | ||
| Write-Host ' [SKIP] .mcp.json not found' -ForegroundColor DarkGray | ||
| Write-Host '' | ||
| return @{ Step = 3; Name = 'Memory Restore'; Status = 'SKIP'; Detail = 'no mcp config' } | ||
| } | ||
|
|
||
| $memoryConn = @($report.connections | Where-Object { $_.kind -eq 'memory' }) | Select-Object -First 1 | ||
| if (-not $memoryConn) { | ||
| Write-Host ' [SKIP] memory server not configured in .mcp.json' -ForegroundColor DarkGray |
There was a problem hiding this comment.
The SKIP message hard-codes “.mcp.json not found”, but Get-McpHealthReport can be configured to look at a different config path via AI_STARTUP_MCP_CONFIG_PATH. Consider printing/reporting the actual $report.configPath (or similar) so the message remains accurate when a custom config path is used.
| if (-not $report.configured) { | |
| Write-Host ' [SKIP] .mcp.json not found' -ForegroundColor DarkGray | |
| Write-Host '' | |
| return @{ Step = 3; Name = 'Memory Restore'; Status = 'SKIP'; Detail = 'no mcp config' } | |
| } | |
| $memoryConn = @($report.connections | Where-Object { $_.kind -eq 'memory' }) | Select-Object -First 1 | |
| if (-not $memoryConn) { | |
| Write-Host ' [SKIP] memory server not configured in .mcp.json' -ForegroundColor DarkGray | |
| $configLabel = if ($report.configPath) { $report.configPath } else { '.mcp.json' } | |
| if (-not $report.configured) { | |
| Write-Host (' [SKIP] {0} not found' -f $configLabel) -ForegroundColor DarkGray | |
| Write-Host '' | |
| return @{ Step = 3; Name = 'Memory Restore'; Status = 'SKIP'; Detail = 'no mcp config' } | |
| } | |
| $memoryConn = @($report.connections | Where-Object { $_.kind -eq 'memory' }) | Select-Object -First 1 | |
| if (-not $memoryConn) { | |
| Write-Host (' [SKIP] memory server not configured in {0}' -f $configLabel) -ForegroundColor DarkGray |
| $memoryConn = @($report.connections | Where-Object { $_.kind -eq 'memory' }) | Select-Object -First 1 | ||
| if (-not $memoryConn) { | ||
| Write-Host ' [SKIP] memory server not configured in .mcp.json' -ForegroundColor DarkGray | ||
| Write-Host '' | ||
| return @{ Step = 3; Name = 'Memory Restore'; Status = 'SKIP'; Detail = 'no memory server' } | ||
| } |
There was a problem hiding this comment.
Invoke-StepMemoryRestore returns Status='OK' as long as a memory server entry exists in .mcp.json, even if that server is unavailable/unhealthy (e.g., command missing on Linux, or runtime probe/health status indicates failure). This can lead to a misleading boot summary where Memory Restore reports OK when it cannot actually run. Consider basing the status on the selected memory server’s availability/health (e.g., skip or fail with a clear detail when unavailable) and include the connection/server status in the returned Detail for diagnostics.
| $filePath = $env:CLAUDE_MEMORY_FILE_PATH | ||
| $entryCount = 0 | ||
| if ($filePath -and (Test-Path $filePath)) { | ||
| try { | ||
| $data = Get-Content $filePath -Raw -Encoding UTF8 | ConvertFrom-Json -ErrorAction Stop | ||
| $entryCount = if ($data.entities) { @($data.entities).Count } else { 0 } | ||
| } | ||
| catch { $entryCount = 0 } | ||
| } | ||
|
|
||
| $fileLabel = if ($filePath -and (Test-Path $filePath)) { | ||
| 'found ({0} entries)' -f $entryCount | ||
| } |
There was a problem hiding this comment.
The memory file existence checks use Test-Path $filePath (twice) without -LiteralPath / -PathType Leaf. Using -LiteralPath avoids wildcard expansion, and PathType Leaf avoids treating a directory as a valid “memory file”; also consider computing a single $fileExists flag to avoid repeating Test-Path.
| elseif ($filePath) { | ||
| 'configured (file not yet created)' | ||
| } | ||
| else { | ||
| 'CLAUDE_MEMORY_FILE_PATH not set' | ||
| } |
There was a problem hiding this comment.
PR description says that when CLAUDE_MEMORY_FILE_PATH is unset the output should show something like “configured (file not yet created)” as a normal case, but the implementation labels it “CLAUDE_MEMORY_FILE_PATH not set”. Please align the output behavior with the PR description (either adjust the message/logic or update the PR description).
v2.9.0 セクションを `[v2.9.0-dev] - 2026-04-08` から `[v2.9.0] - 2026-04-14 (STABLE)` に更新し、 ClaudeOS v7.5 の全成果 (Boot Sequence Step 3/7/9 完全実装、CodeRabbit 統合、 /team-onboarding、ループ時間最適化、Issue Sync 修正) を反映。 新規に `[v3.0.0] - Unreleased` セクションを追加し、Phase 4 残タスク (セキュリティ監査 / E2E テスト / リリースタグ) と Go/No-Go 基準を明文化。 - テスト数: 304 → 311 件に更新 - 追加 PR: #77, #79, #80, #81, #82, #83, #84, #86, #87, #88, #89 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
v2.9.0 セクションを `[v2.9.0-dev] - 2026-04-08` から `[v2.9.0] - 2026-04-14 (STABLE)` に更新し、 ClaudeOS v7.5 の全成果 (Boot Sequence Step 3/7/9 完全実装、CodeRabbit 統合、 /team-onboarding、ループ時間最適化、Issue Sync 修正) を反映。 新規に `[v3.0.0] - Unreleased` セクションを追加し、Phase 4 残タスク (セキュリティ監査 / E2E テスト / リリースタグ) と Go/No-Go 基準を明文化。 - テスト数: 304 → 311 件に更新 - 追加 PR: #77, #79, #80, #81, #82, #83, #84, #86, #87, #88, #89 Co-authored-by: 有藤 健太郎 <k-aritoh@mirai-const.co.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Start-ClaudeOS.ps1にMcpHealthCheck.psm1を追加インポートInvoke-StepMemoryRestore関数を新規実装 (Step 3).mcp.json未設定 →SKIPmemorykind サーバー未設定 →SKIPOK(エントリ数・ファイルパスをレポート)FAILInvoke-StepPlaceholder) をInvoke-StepMemoryRestoreに置換StartScripts.Tests.ps1: Step 3 を SKIP リストから除外し、Memory Restore 専用テストを追加Test plan
StartScripts.Tests.ps1—Step 3 が Memory Restore として実行されることが PASSStartScripts.Tests.ps1—Step 5/6/8 がプレースホルダー SKIPが PASS.mcp.jsonあり /memoryサーバー設定あり → Step 3 = OK.mcp.jsonなし /memoryサーバーなし → Step 3 = SKIP影響範囲
scripts/main/Start-ClaudeOS.ps1— Step 3 ロジック変更tests/StartScripts.Tests.ps1— テスト更新残課題
CLAUDE_MEMORY_FILE_PATH環境変数が未設定の場合は「configured (file not yet created)」表示(正常動作)Closes #70
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
テスト