Cut macOS widgets over to the shared renderer - #47
Conversation
The macOS cutover from the shared-renderer plan (docs/macos-memory-handoff.md on feat/macos-memory-work): weaverd owns one render host — the only Metal-owning process — and every weaverd-spawned widget renders through it device-lessly. - weaver-widget gains --render-host <bootstrap-name>: the process becomes the shared renderer (native_sdk_appkit_render_host_run) and never loads a widget bundle. - weaverd spawns the host at startup and supervises it like a widget: crash -> respawn after a 1 s backoff; widgets keep their retained frames and reconnect on their own (their existing 1/5/30 s retry cadence). Widget environments get NATIVE_SDK_GPU_SHARED_RENDERER=1 and the host's bootstrap name. The automation seam keeps the legacy in-process renderer (its verdicts sample the widget's own drawable). - native-sdk pinned to weaver-main b72da2d2 (shared-renderer slices 1-3: IOSurface presenter, render host + mach protocol, device-less client). Acceptance (Mac15,6, isolated-HOME weaverd, 8 registered clock widgets): every widget 32.2-35.8 MB phys_footprint with ZERO owned-unmapped-graphics regions (in-process baseline: 125.3 MB with the 85 MB arena each); 31-minute hold flat within ±0.3 MB per widget, host non-increasing (168.5 -> 158.9 MB, slope -165 KB/min); host killed -> weaverd restarted it in 1 s and the widgets resumed live rendering, verified by window-capture diff. Runtime test-platform-services and host test suites pass.
…d-renderer slices)
Greptile SummaryThis PR moves weaverd-managed macOS widgets to a supervised shared renderer. It adds render-host mode to No product defect was established from the available execution results. T-Rex validation blockedMissing tool: Zig — the required native lifecycle checks could not run because this Linux worker does not have Confidence Score: 5/5
 T-Rex LogsWhat T-Rex did
Reviews (2): Last reviewed commit: "Reap the render host before dropping its..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 4 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 Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe runtime adds a macOS render-host command. The macOS host supervises one shared render process, restarts it after failure, passes its configuration to non-automation widgets, and stops it during shutdown. The Native SDK audit reference is updated. ChangesShared render-host integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HostMainLoop
participant macos_host.zig
participant RenderHostProcess
participant NativeSDKAppKit
HostMainLoop->>macos_host.zig: supervise shared render host
macos_host.zig->>RenderHostProcess: spawn with bootstrap name
RenderHostProcess->>NativeSDKAppKit: start AppKit render host
macos_host.zig->>RenderHostProcess: send SIGTERM during shutdown
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@host/src/macos_host.zig`:
- Around line 613-618: Update stopRenderHost to preserve the recovery marker
until the render host has exited: send the bounded TERM signal, wait with the
same TERM/KILL timeout and waitpid sequence used by stopSlot, then remove the
child marker and clear render_host_process only after reaping succeeds. Reuse
stopSlot’s existing lifecycle behavior rather than introducing a separate
shutdown strategy.
In `@runtime/src/main.zig`:
- Around line 1204-1206: Update the usage output in the argument-validation
block around the dev/render-host parsing so each accepted command form is
printed separately, with <widget-directory> attached only to the applicable
form. Emit the --render-host usage form only on macOS builds, while preserving
the existing dev and regular widget-directory forms.
🪄 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 Plus
Run ID: a13c3e73-7254-4960-a6c9-3185df81e9a7
📒 Files selected for processing (4)
host/src/macos_host.zigruntime/native-sdkruntime/src/main.zigscripts/release-audit.mjs
| const dev = args.len == 3 and std.mem.eql(u8, args[1], "--dev"); | ||
| if ((!dev and args.len != 2) or (dev and args.len != 3)) { | ||
| std.debug.print("usage: weaver-widget [--dev] <widget-directory>\n", .{}); | ||
| std.debug.print("usage: weaver-widget [--dev | --render-host <bootstrap-name>] <widget-directory>\n", .{}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the render-host usage grammar.
Because the render-host branch returns before widget-directory parsing, it accepts only --render-host <bootstrap-name>. Line 1206 places <widget-directory> outside the alternative and advertises an invalid command. The message also advertises this macOS-only option on non-macOS builds.
Print separate usage forms and include the render-host form only on macOS.
Proposed fix
- std.debug.print("usage: weaver-widget [--dev | --render-host <bootstrap-name>] <widget-directory>\n", .{});
+ const usage = if (builtin.os.tag == .macos)
+ "usage: weaver-widget <widget-directory> | weaver-widget --dev <widget-directory> | weaver-widget --render-host <bootstrap-name>\n"
+ else
+ "usage: weaver-widget <widget-directory> | weaver-widget --dev <widget-directory>\n";
+ std.debug.print(usage, .{});📝 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.
| const dev = args.len == 3 and std.mem.eql(u8, args[1], "--dev"); | |
| if ((!dev and args.len != 2) or (dev and args.len != 3)) { | |
| std.debug.print("usage: weaver-widget [--dev] <widget-directory>\n", .{}); | |
| std.debug.print("usage: weaver-widget [--dev | --render-host <bootstrap-name>] <widget-directory>\n", .{}); | |
| const dev = args.len == 3 and std.mem.eql(u8, args[1], "--dev"); | |
| if ((!dev and args.len != 2) or (dev and args.len != 3)) { | |
| const usage = if (builtin.os.tag == .macos) | |
| "usage: weaver-widget <widget-directory> | weaver-widget --dev <widget-directory> | weaver-widget --render-host <bootstrap-name>\n" | |
| else | |
| "usage: weaver-widget <widget-directory> | weaver-widget --dev <widget-directory>\n"; | |
| std.debug.print(usage, .{}); |
🤖 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 `@runtime/src/main.zig` around lines 1204 - 1206, Update the usage output in
the argument-validation block around the dev/render-host parsing so each
accepted command form is printed separately, with <widget-directory> attached
only to the applicable form. Emit the --render-host usage form only on macOS
builds, while preserving the existing dev and regular widget-directory forms.
There was a problem hiding this comment.
Fixed: teardown now escalates exactly like widget teardown (TERM, 1.5 s reap window, KILL, blocking reap) and removes the recovery marker only after the process is dead and reaped; the usage string now shows the two argument shapes honestly.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline runtime/src/main.zig --items all --type function --match main
sed -n '1190,1225p' runtime/src/main.zigLength of output: 2377
@SunkenInTime, the argument shapes are now correct.
One part of the finding remains. Non-macOS builds still print weaver-widget --render-host <bootstrap-name>, but the parser accepts that option only when builtin.os.tag == .macos.
Please conditionally include the render-host usage form on macOS. I will mark the comment addressed after that change.
You are interacting with an AI system.
Review is right that teardown removed the marker, sent a best-effort TERM, and forgot the pid — a TERM-ignoring host would outlive its marker and the next weaverd could not find the orphan. Teardown now escalates exactly like widget teardown (TERM, 1.5 s reap window, KILL, blocking reap) and removes the marker only after the process is dead. Also fixes the usage string to show the two argument shapes honestly.
|
@coderabbitai The usage string is now comptime-conditional: non-macOS builds print only the widget-directory form, matching what the parser accepts. |
1 participant
The weaver half of slice 4 (native half: budget tripwire in SunkenInTime/native#24, at 5/5). This makes the shared renderer the macOS default for weaverd-owned widgets — the cutover the whole plan (docs/macos-memory-handoff.md on feat/macos-memory-work) builds to.
What changes
weaver-widget --render-host <bootstrap-name>: the process becomes the shared renderer — the only Metal-owning process on the desktop — and never loads a widget bundle.NATIVE_SDK_GPU_SHARED_RENDERER=1+ the host's bootstrap name. The automation seam keeps the legacy in-process renderer (its correctness verdicts sample the widget's own drawable) — converting automation is future work, deliberately not smuggled in here.b72da2d2(shared-renderer slices 1–3).Acceptance receipts (Mac15,6, macOS 26.5.2, isolated-HOME weaverd, 8 registered clock widgets)
kill -9on the host → weaverd log names the restart, respawn in 1 s → widgets reconnected and resumed live rendering (window-capture diff shows the clock ticking)ps -E: both variables present on weaverd-spawned widgetstest-platform-servicesand hostzig build testpassFull evidence trail (Phase 1 spike gates, per-slice receipts, drift CSVs) lives in
docs/macos-memory-handoff.mdonfeat/macos-memory-workand.zig-cache/macos-memory/receipts on this machine.Follow-ups (named, not smuggled)
weaver dev/ standalone launches keep in-process rendering (no weaverd, no host) by design.Summary by CodeRabbit
New Features
Bug Fixes