Repository navigation
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f6dd1c. Configure here.
| if err != nil { | ||
| return n, &net.OpError{Op: "read", Net: "vsock", Err: err} | ||
| } | ||
| return n, err |
There was a problem hiding this comment.
Read wraps EOF as OpError
Low Severity
vmConn.Read wraps every os.File.Read failure, including bare io.EOF, in net.OpError. Standard net.Conn implementations leave io.EOF unwrapped, and gRPC/HTTP2 compare with == io.EOF for a clean close. Guest disconnects can be treated as transport errors instead of orderly EOF.
Reviewed by Cursor Bugbot for commit 8f6dd1c. Configure here.
b2a5fae to
a54ec4a
Compare
|
Real native Darwin GuestService2222 QA passed in an isolated cold-booted OCI guest using the current shared binary, user-launched at UID501 only. Host shared client verified Root daemon was not installed: guest noninteractive sudo is unavailable and no privileged provisioning was performed. System image capability remained false. These results prove native transport/user-level exec/files/cancellation and shutdown admission, not normal API system-agent installation/readiness or successful graceful root shutdown. Those remain gates. Original guest/API/storage untouched; isolated QA VM ended stopped. |
a1ed358 to
243ce9a
Compare
|
Simplification update at 243ce9a: common command setup/process-group cancellation and bounded wait/drain/final-status handling for both TTY modes. Permanent tests cover pre-cancelled starts and failed output sends cancelling/reaping commands. Full agent/client race suites passed 3 runs; combined post-restack focused suites also passed 3 runs. Base changed to feat/macos-oci-images so the image rollback fix and bundle consolidation are inherited once rather than duplicated. Root provisioning remains undone. All PRs remain draft. This is source-level/synthetic validation, not a new live boot or production build proof. Default Codex independent review was attempted but is still blocked by authentication (HTTP401). Published atomically after checking reviewer heads with explicit per-ref force-with-lease; prior heads preserved locally. |
|
Root QA follow-up pushed as ae3685d; desktop restacked atomically with explicit leases. Manually installed shared system-role agent survived isolated normal-API cold boots. Repeated race-instrumented authenticated API tests verified UID0 exec (both TTY modes), root-owned0600 file copy round trips, and descendant cancellation. Live testing reproduced and fixed Darwin readiness using the nonexistent /bin/true; /usr/bin/true now probes successfully and normal GET persists GuestAgentReadyAt without a Linux workload marker. Fixed premature shutdown fallback after a lost Darwin RPC reply: configured grace period is retained while still requiring owned VMM exit; Linux policy is unchanged. Final normal API stop accepted the RPC and confirmed VMM exit/storage closure without forced fallback. Corrected stale opt-in test calls left after runtime simplification. Focused instances/API/agent/client tests pass race,count3; default independent autoreview remains401-blocked. Manual QA provisioning is not automatic image installation or end-to-end build/publication proof. PR remains draft. |
- Persist macOS guest-agent readiness on the public read path by hydrating boot markers for macOS instances too. - Kill the command's process group on cancellation or timeout so shell descendants do not outlive the command. - Bound how long a cancelled exec waits for output to drain, so a client that stopped reading cannot hold the handler open. - Hold the fork lock across Darwin vsock accept so an accepted descriptor is not inherited by a concurrent fork before it is marked close-on-exec.
The guest agent runs commands as root, as the Linux agent does, and the host API is the only authorization boundary. Exec timeout ends the command; it does not bound a healthy output stream.
- Send the final exit code under the same bound as command output, so a client that stopped reading cannot hold the handler past a timeout. - Kill the command's process group from the context rather than only through exec.Cmd.Cancel, which is not called once the direct child has exited. - Make the drain bound a server field instead of a package variable, so tests do not mutate shared state read by handlers that outlive them.
Hydration and persistence duplicated the missing-marker checks, the serial log parse, marker assignment and the readiness probe. Extract them into one applyBootMarkers routine. Hydration keeps its scan throttling and rescan bookkeeping; persistence keeps its save and metrics.
- A command that exits 0 while a background child still holds its output returns exec.ErrWaitDelay after the wait delay. That is a normal exit, so report its status instead of failing the RPC. - A command that hits its deadline reports 124 (GNU timeout convention). The group kill leaves a process state behind, so the old check never fired. - Share exit-code selection between the TTY and non-TTY paths. - Define StoredMetadata.GuestAgentEnabled once and use it in exec, cp, stop, vsock and boot-marker code instead of five copies of the expression. - Make the websocket exec cancel function required and drop the nil checks. - Remove the unreachable empty-command checks and the redundant Cwd guard.
- Run exec under one cancellable context. The stream-send error path and caller disconnect cancel the same context, and killGroupOnDone is the only place the process group is killed. This drops the second context and the command cancel hooks, which both killed the same group. - Keep the already-cancelled error before start, as before. - Define the default ready-file path once per OS in ready_linux.go and ready_darwin.go instead of three copies across transport files. - Remove PR-status wording from the guest-agent doc. It described this patch rather than the code and would rot after merge.
Runs on every host with fixture capabilities instead of skipping off Apple silicon.
… rules - runBounded runs a call on its own goroutine under the drain bound. The command wait, output drain and exit-code send each repeated the goroutine, channel and bound by hand. - StoredMetadata.programMarkerSettled states the macOS rule that no workload start marker is awaited. Hydration and the agent probe gate use it. - Boot markers and the metrics readiness check read GuestAgentEnabled instead of the raw skip flag. For Linux records these are identical. - requestDarwinShutdown and its policy test move to untagged files, so the shutdown policy runs on Linux CI. Only the euid and command stay darwin-only. - The two stalled-client exec tests share one helper.
ae3685d to
c336b30
Compare
This branch has not been deployed
1 participant
Stack
PR 3/6 of macOS guest integration; based on #498. Draft foundation for Darwin GuestService, readiness and networking. It does not depend on the OCI implementation in #499.
Implemented
Unimplementedfor Darwin network identity reconfiguration; no NAT/static-IP/policy parity claim.guest_agentimage declaration enables normal exec/copy vsock paths and graceful-stop attempts; existing templates remain unmanaged and can explicitly disable integration.Validation
Tests use an in-memory gRPC transport, temporary files and short-lived test commands. Shutdown policy tests stub the command: no actual host/guest shutdown, agent installation or live API deployment occurred.
Remaining draft gates
No VM images, credentials, private notes or benchmark evidence are included. The live benchmark guest and its prototype agents remain untouched.