Phase 3-A: Native macOS Spike 実機検証3項目 - #154
Merged
Merged
Conversation
Phase 3-A (3-2): validate whether a Java subprocess can launch under Hardened Runtime with ad-hoc code signing before committing to a process-management design for the native macOS spike. Adds JavaLaunchHarness (actor-based Process wrapper), a script that builds/bundles/signs the app with three entitlements variants (baseline, allow-jit, allow-unsigned-executable-memory), and runs `java -version` under each. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 3-A (3-1): scaffold two candidate implementations for a Floating Console Panel so the non-active glass degradation can be compared on real hardware before committing to one design. Pattern A: NonactivatingGlassPanel, an NSPanel bridge (.nonactivatingPanel + NSHostingView). Pattern B: SwiftUIWindowLevelSpike, a pure SwiftUI Window scene with .windowLevel(.floating). Both render the same GlassSpikeContent view and are reachable via `MCV_SPIKE=panel-nspanel` / `MCV_SPIKE=panel-window` on the App executable. This session has no WindowServer connection to capture screenshots, so the active/inactive glass comparison itself is deferred to the user running these on real hardware; results will be recorded in a follow-up commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 3-A (3-3): scaffold a dummy high-frequency log generator plus List/ScrollView rendering variants so real-hardware Instruments traces can determine which batching/virtualization strategy the production log stream view should use. DummyLogGenerator emits LogLine values via AsyncStream at a configurable rate. LogBatcher groups lines into fixed-size time windows as a candidate batching strategy, with unit tests covering same-window grouping, cross-window splitting, and empty input. LogStreamSpikeApp exposes both view variants via `MCV_SPIKE=log-stream` (`MCV_LOG_SPIKE_VARIANT=list|scroll`). This session cannot capture an Instruments .trace (no WindowServer connection), so the actual hitch/CPU measurement is deferred to the user running these on real hardware with swiftui-expert-skill/scripts/record_trace.py; results will be recorded in a follow-up commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Captures the 3-2 hardened runtime findings (Java subprocess launches successfully under ad-hoc signing with no additional entitlements) and documents the deferred user-side steps for 3-1 (screenshot comparison) and 3-3 (Instruments trace capture). Updates the risk table in native-macos-requirements.md §6 to reflect what Phase 3-A actually verified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughHardened Runtime下でのJavaプロセス起動検証（JavaLaunchHarness）、NSPanel/SwiftUIによるフローティングパネル表示スパイク、ログストリーム生成・バッチング・表示スパイクを新規実装した。エントリポイントを環境変数分岐でasync化し、entitlements plistと署名スクリプト、対応テスト、仕様書を追加した。 ChangesNative macOSスパイク検証一式
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Script as hardened-runtime-spike.sh
participant App
participant Main
participant JavaLaunchHarness
Script->>App: 各entitlementsでcodesign
Script->>App: MCV_SPIKE=hardened-runtime-java で実行
App->>Main: main() [async]
Main->>JavaLaunchHarness: launch(java, "-version")
JavaLaunchHarness-->>Main: JavaLaunchResult
Main-->>Script: 終了コード/出力を記録
Script->>Script: RESULTSのサマリ表示
sequenceDiagram
participant DummyLogGenerator
participant LogBatcher
participant LogStreamSpikeApp
participant LogStreamListView
LogStreamSpikeApp->>DummyLogGenerator: stream(linesPerSecond)
DummyLogGenerator-->>LogStreamListView: LogLine を yield
LogStreamListView->>LogStreamListView: 保持件数超過時に先頭削除
Note over LogBatcher: 別経路でLogLineをinterval窓でバッチ化
Poem
🚥 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Phase 3-A（native macOS spike）の「実機検証3項目」に向けて、検証用スキャフォールド/ハーネス/自動テストを追加し、結果記録とリスク表を更新するPRです（Core→App 起動経路もスパイク選択対応に拡張）。
Changes:
- NSPanel×glassEffect 劣化検証用のPanelスパイク実装とテストを追加
- 高頻度ログ描画の計測用ビュー/ログ生成/バッチャとテストを追加
- Hardened Runtime 下での Java 起動検証用ハーネス + 署名/実行スクリプト、ならびに結果ドキュメント/要件リスク表を追加更新
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/phase3a-spike-results.md | Phase 3-Aの実機検証結果・保留事項の記録を追加 |
| spec/native-macos-requirements.md | リスク表をPhase 3-Aの状況に合わせて更新 |
| apps/native-macos/Tests/CoreTests/PanelSpikeTests.swift | Panelスパイク構成のユニットテストを追加 |
| apps/native-macos/Tests/CoreTests/LogBatcherTests.swift | LogBatcherのユニットテストを追加 |
| apps/native-macos/Tests/CoreTests/JavaLaunchHarnessTests.swift | JavaLaunchHarnessのユニットテストを追加 |
| apps/native-macos/Sources/Core/Spikes/PanelSpike/SwiftUIWindowLevelSpike.swift | SwiftUIウィンドウレベル検証用スパイクAppを追加 |
| apps/native-macos/Sources/Core/Spikes/PanelSpike/NonactivatingGlassPanel.swift | non-activating NSPanel ブリッジ実装を追加 |
| apps/native-macos/Sources/Core/Spikes/PanelSpike/GlassSpikeContent.swift | glassEffect表示用の共通SwiftUIビューを追加 |
| apps/native-macos/Sources/Core/Spikes/LogSpike/LogStreamSpikeView.swift | List/ScrollView のログ高頻度描画スパイクViewを追加 |
| apps/native-macos/Sources/Core/Spikes/LogSpike/LogBatcher.swift | ログ行の時間窓バッチ化ユーティリティを追加 |
| apps/native-macos/Sources/Core/Spikes/LogSpike/DummyLogGenerator.swift | ダミーログ生成（AsyncStream）を追加 |
| apps/native-macos/Sources/Core/Spikes/JavaLaunchHarness.swift | Java起動（Process）ハーネスを追加 |
| apps/native-macos/Sources/App/Spikes/PanelSpikeRunner.swift | スパイク実行のApp側ランナーを追加 |
| apps/native-macos/Sources/App/Main.swift | 環境変数でスパイクを選択して起動するエントリに変更 |
| apps/native-macos/Scripts/hardened-runtime-spike.sh | Hardened Runtime + entitlements別の署名/実行スクリプトを追加 |
| apps/native-macos/Scripts/entitlements/spike-baseline.plist | baseline用entitlementsを追加 |
| apps/native-macos/Scripts/entitlements/spike-allow-unsigned-executable-memory.plist | unsigned executable memory等を許可するentitlementsを追加 |
| apps/native-macos/Scripts/entitlements/spike-allow-jit.plist | JIT許可entitlementsを追加 |
| .gitattributes | Linguist設定に変更を追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+27
|
|
||
| *.js |
Comment on lines
+20
to
+27
| for line in lines { | ||
| if line.timestamp - windowStart >= self.interval, !currentBatch.isEmpty { | ||
| batches.append(currentBatch) | ||
| currentBatch = [] | ||
| windowStart = line.timestamp | ||
| } | ||
| currentBatch.append(line) | ||
| } |
Comment on lines
+23
to
+25
| if self.lines.count > self.retainedLineCount { | ||
| self.lines.removeFirst(self.lines.count - self.retainedLineCount) | ||
| } |
Comment on lines
+54
to
+56
| if self.lines.count > self.retainedLineCount { | ||
| self.lines.removeFirst(self.lines.count - self.retainedLineCount) | ||
| } |
Comment on lines
+38
to
+42
| let stdoutData = try stdoutPipe.fileHandleForReading.readToEndCompat() | ||
| let stderrData = try stderrPipe.fileHandleForReading.readToEndCompat() | ||
|
|
||
| process.waitUntilExit() | ||
|
|
| | プラグイン解決ロジックの二重実装コスト | 低(現Spike範囲外) | Native版でプラグイン管理を実装する段階で再評価 | | ||
| | glass非アクティブ劣化 | 低 | 実機検証→Materialフォールバック(確定済み) | | ||
| | `.xcodeproj`なしによるXcode GUI機能の一部制約(Instruments連携等) | 低 | Xcodeは`Package.swift`を直接開けるため多くの機能は利用可能。制約が顕在化したら`.xcodeproj`併用を再検討 | | ||
| | glass非アクティブ劣化 | 低 | 実機検証→Materialフォールバック(確定済み)。Phase 3-Aでスキャフォールド実装済み、実機での目視判定は`spec/phase3a-spike-results.md`参照(検証待ち) | |
Two Important findings from code review: - JavaLaunchHarness.launch read stdout then stderr sequentially via Pipe.readToEnd(), the classic Process/Pipe deadlock pattern: a child that fills the OS pipe buffer on stderr before stdout closes would hang forever. Didn't manifest for `java -version` (trivial stderr-only output), but this harness is the basis for 3-7's real server launch. Fixed to read both pipes concurrently via `async let`. - LogStreamListView/ScrollView trimmed to exactly retainedLineCount on every single append, making Array.removeFirst the dominant cost at 1000 lines/sec rather than the List vs ScrollView rendering cost 3-3 is supposed to measure. Extracted LogLineBuffer, a shared hysteresis-based trim (only shifts once overshoot is exceeded), used by both view variants. Also adds the missing LogBatcher edge case tests (single line, identical timestamps) and LogLineBuffer trim-behavior tests flagged in review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
native.yml runs `swiftformat --lint` then `swiftlint --strict`. SwiftFormat's default trailingCommas rule requires a trailing comma on the last element of a multi-line collection; SwiftLint's default trailing_comma rule forbids one. Neither Phase 2's minimal files nor earlier Phase 3-A files had a multi-line literal to trigger this, so it went unnoticed locally (lefthook runs swiftformat then swiftlint --fix, so swiftlint's fix silently wins) until CI's lint job failed on Tests/CoreTests/LogBatcherTests.swift and Sources/App/Main.swift. Disables SwiftFormat's trailingCommas rule so SwiftLint's no-trailing-comma stance is the sole authority. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/native-macos/Tests/CoreTests/PanelSpikeTests.swift (1)
6-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
collectionBehaviorのアサーション追加を推奨。
NonactivatingGlassPanelはcollectionBehavior = [.canJoinAllSpaces, .fullScreenAuxiliary]を設定していますが、テストで検証されていません。このプロパティはフローティングパネルの期待動作に直結するため、アサーションを追加することを推奨します。✅ 提案するアサーション追加
`#expect`(panel.styleMask.contains(.nonactivatingPanel)) `#expect`(panel.isFloatingPanel) `#expect`(panel.level == .floating) + `#expect`(panel.collectionBehavior.contains(.canJoinAllSpaces)) + `#expect`(panel.collectionBehavior.contains(.fullScreenAuxiliary)) + `#expect`(panel.titleVisibility == .hidden) + `#expect`(panel.titlebarAppearsTransparent) }🤖 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 `@apps/native-macos/Tests/CoreTests/PanelSpikeTests.swift` around lines 6 - 12, The NonactivatingGlassPanel test currently verifies styleMask, isFloatingPanel, and level but misses collectionBehavior validation. Update nonactivatingGlassPanelConfiguration() in PanelSpikeTests to assert that NonactivatingGlassPanel.collectionBehavior includes the expected flags (.canJoinAllSpaces and .fullScreenAuxiliary), alongside the existing assertions, so the panel’s space and fullscreen behavior is covered by the test.apps/native-macos/Sources/Core/Spikes/LogSpike/LogStreamSpikeView.swift (1)
20-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
removeFirstのO(n)コストが高頻度で発生デフォルト（
linesPerSecond = 1000）の場合、バッファ満了後に1行追加するたびにremoveFirst(1)が呼ばれ、O(n)（n ≈ 5000）の要素シフトが毎秒1000回発生します。さらに、1行ごとのappendがSwiftUIの再描画をトリガーし、描画コストも増大します。同じディレクトリにLogBatcherが定義されテストされていますが、ビューでは未使用です。時間ベースでUI更新をバッチ化し、配列操作と再描画の頻度を下げることを推奨します。
♻️ 提案するバッチ化の修正（LogStreamListView）
.task { - for await line in await self.generator.stream(linesPerSecond: self.linesPerSecond) { - self.lines.append(line) - if self.lines.count > self.retainedLineCount { - self.lines.removeFirst(self.lines.count - self.retainedLineCount) - } + var buffer: [LogLine] = [] + let clock = ContinuousClock() + let flushInterval = Duration.milliseconds(16) + var lastFlush = clock.now + + for await line in await self.generator.stream(linesPerSecond: self.linesPerSecond) { + buffer.append(line) + if clock.now - lastFlush >= flushInterval, !buffer.isEmpty { + self.lines.append(contentsOf: buffer) + buffer.removeAll(keepingCapacity: true) + if self.lines.count > self.retainedLineCount { + self.lines.removeFirst(self.lines.count - self.retainedLineCount) + } + lastFlush = clock.now + } } }
LogStreamScrollView（行51-58）にも同様の修正を適用してください。Also applies to: 51-58
🤖 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 `@apps/native-macos/Sources/Core/Spikes/LogSpike/LogStreamSpikeView.swift` around lines 20 - 27, The log streaming views are updating per line, causing frequent O(n) buffer trimming with removeFirst and unnecessary SwiftUI redraws at high line rates. Update LogStreamSpikeView and LogStreamScrollView to batch incoming lines over time instead of appending each line immediately, and reuse the existing LogBatcher in the same directory to coalesce updates and retention trimming. Keep the retainedLineCount behavior intact, but move the array mutation and UI state updates into the batch flush path so rendering happens less often.
🤖 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 @.gitattributes:
- Line 27: The `*.js` entry in `.gitattributes` is a no-op because it has no
attribute assigned, so it does not affect Linguist. Update the `.gitattributes`
rule to either explicitly mark JavaScript as detectable with
`linguist-detectable=true` if you want it visible, or remove the `*.js` entry
entirely if there is no intended behavior change.
In `@apps/native-macos/Sources/Core/Spikes/JavaLaunchHarness.swift`:
- Around line 36-41: The stdout/stderr capture in JavaLaunchHarness.run is
vulnerable to deadlock because the two pipes are read sequentially and
waitUntilExit() is only reached after both reads succeed. Update the process
launch flow to read stdout and stderr concurrently (for example by using
separate asynchronous tasks around stdoutPipe and stderrPipe) and make sure
process.waitUntilExit() is always executed even if readToEndCompat() throws, so
the launched process is not left running or blocked.
In `@spec/phase3a-spike-results.md`:
- Around line 35-39: The markdown fence in this results snippet is missing a
language tag, which triggers MD040. Update the fenced block in the phase3a spike
results section to use the existing code block with a text language hint, or
remove the fence entirely; locate it by the visible spike result entries and
keep the content unchanged otherwise.
---
Nitpick comments:
In `@apps/native-macos/Sources/Core/Spikes/LogSpike/LogStreamSpikeView.swift`:
- Around line 20-27: The log streaming views are updating per line, causing
frequent O(n) buffer trimming with removeFirst and unnecessary SwiftUI redraws
at high line rates. Update LogStreamSpikeView and LogStreamScrollView to batch
incoming lines over time instead of appending each line immediately, and reuse
the existing LogBatcher in the same directory to coalesce updates and retention
trimming. Keep the retainedLineCount behavior intact, but move the array
mutation and UI state updates into the batch flush path so rendering happens
less often.
In `@apps/native-macos/Tests/CoreTests/PanelSpikeTests.swift`:
- Around line 6-12: The NonactivatingGlassPanel test currently verifies
styleMask, isFloatingPanel, and level but misses collectionBehavior validation.
Update nonactivatingGlassPanelConfiguration() in PanelSpikeTests to assert that
NonactivatingGlassPanel.collectionBehavior includes the expected flags
(.canJoinAllSpaces and .fullScreenAuxiliary), alongside the existing assertions,
so the panel’s space and fullscreen behavior is covered by the test.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0c3ebae-4285-49db-84c9-ca9b867cc28d
📒 Files selected for processing (19)
.gitattributesapps/native-macos/Scripts/entitlements/spike-allow-jit.plistapps/native-macos/Scripts/entitlements/spike-allow-unsigned-executable-memory.plistapps/native-macos/Scripts/entitlements/spike-baseline.plistapps/native-macos/Scripts/hardened-runtime-spike.shapps/native-macos/Sources/App/Main.swiftapps/native-macos/Sources/App/Spikes/PanelSpikeRunner.swiftapps/native-macos/Sources/Core/Spikes/JavaLaunchHarness.swiftapps/native-macos/Sources/Core/Spikes/LogSpike/DummyLogGenerator.swiftapps/native-macos/Sources/Core/Spikes/LogSpike/LogBatcher.swiftapps/native-macos/Sources/Core/Spikes/LogSpike/LogStreamSpikeView.swiftapps/native-macos/Sources/Core/Spikes/PanelSpike/GlassSpikeContent.swiftapps/native-macos/Sources/Core/Spikes/PanelSpike/NonactivatingGlassPanel.swiftapps/native-macos/Sources/Core/Spikes/PanelSpike/SwiftUIWindowLevelSpike.swiftapps/native-macos/Tests/CoreTests/JavaLaunchHarnessTests.swiftapps/native-macos/Tests/CoreTests/LogBatcherTests.swiftapps/native-macos/Tests/CoreTests/PanelSpikeTests.swiftspec/native-macos-requirements.mdspec/phase3a-spike-results.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spec/phase-tasks.md) の実機検証3項目に着手。3-2(Hardened Runtime下のJava起動)はこのセッション内で検証完了、3-1(NSPanel glass劣化)・3-3(高頻度ログ描画パフォーマンス)は実装・自動テストまで完了し、実機でのみ可能な操作(スクリーンショット撮影・Instrumentsトレース記録)をユーザーに依頼する形で保留spec/phase3a-spike-results.mdを新規作成し検証結果・保留事項を記録、spec/native-macos-requirements.md§6のリスク表を更新Test plan
swift build/swift testが全コミットで通ることを確認済み(8テスト green)hardened-runtime-spike.shを実行し、baseline/allow-jit/allow-unsigned-executable-memoryの3パターン全てexit 0を確認apps/native-macos/spec-assets/3-1/)を撮影record_trace.pyで取得native.yml) のlint/build/testジョブがgreenになることを確認Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01W6ZTictDPbmypEwnXQn8Zg
Summary by CodeRabbit
New Features
Tests
Documentation