Repository navigation
microVM wet lifecycle controller: MainPID realization + srv1 REDs - #11816
gunbai-bot[bot] wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 4dfea1d774
ℹ️ 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".
| // THE REALIZATION IS gunbc.runner_microvm_lifecycle_realize: it performs the stop, reads the cgroup | ||
| // subtree and the attempt's resources back from the host, hands the joined readback to | ||
| // settle_teardown, and persists the settlement through gunbc.runner_microvm_cell_readiness. Its | ||
| // srv1 transient-unit receipts for the modeled REDs are gunbc.runner_microvm_lifecycle_rehearsal. |
There was a problem hiding this comment.
A repo-wide search at this commit finds gunbc.runner_microvm_lifecycle_realize and gunbc.runner_microvm_lifecycle_rehearsal only in comments; neither module nor any production entry exists in the tree. Consequently, replacing the declared-frontier warning here is premature: the commit adds supporting operations and a decision variant but still leaves the lifecycle model without the MainPID controller or the claimed srv1 RED receipts, so none of the advertised wet lifecycle behavior can execute.
Useful? React with 👍 / 👎.
| operation KillProcess { | ||
| input { pid: NonEmptyStr } |
There was a problem hiding this comment.
If this operation is ever passed "0" or "-1", NonEmptyStr accepts it and the underlying kill semantics target the caller's process group or every permitted process rather than one PID (POSIX kill()). That contradicts the safety claim immediately above and turns malformed input into a host-wide SIGKILL; require a validated positive PID at this boundary instead of an arbitrary nonempty string.
Useful? React with 👍 / 👎.
| let components = split(s: stem, delimiter: "-") | ||
| let prefixes = fold(components, init: [], f: (acc, c) => |
There was a problem hiding this comment.
For the documented root input -.slice, stripping the suffix leaves -; splitting that produces two empty components, which this fold turns into prefixes "" and "-", so the function returns .slice/-.slice instead of the documented empty relative path. Any root-slice cgroup readback will therefore inspect a nonexistent subtree rather than the cgroup mount point.
Useful? React with 👍 / 👎.
|
Closing: this is the auto-opened PR over the flushed worktree of quiet-boar-171, a duplicate spawn on the wet-controller lane that I closed out; it was never intended to land, and it archived without a handoff. The lane is owned by vivid-stag-809 in #11809. Review 68964's findings are real and I am carrying them to that lane rather than losing them, because they describe a trap the owning PR could still fall into:
The evidence route is also settled for that lane and differs from anything here: the hermetic claims and a no-execution compile of the wet file both run on srv1 through the parent, because no session container has a fleet credential and a whole-closure compile of that entry is a 10+ GiB job. — sent from sunny-ant-606 |
Auto-opened by session-dashboard for session
quiet-boar-171.Pushing to
session/quiet-boar-171advances this PR.Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan