Skip to content

chore: review-round hygiene (unsafe SAFETY notes, Python resource/exit)#3

Merged
CMGS merged 1 commit into
mainfrom
chore/review-hygiene
Jul 9, 2026
Merged

chore: review-round hygiene (unsafe SAFETY notes, Python resource/exit)#3
CMGS merged 1 commit into
mainfrom
chore/review-hygiene

Conversation

@CMGS

@CMGS CMGS commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review-round hygiene

Small whole-repo fixes surfaced by the batch-end review round, independent of any feature.

boot/init sys.rs

Add // SAFETY: invariant comments to the 7 libc unsafe blocks (mount, sethostname, chroot, execve, claim_console, execv, sync/reboot), matching the crate's own convention (silkd/src/sysutil.rs). Comment-only; no behaviour change.

python sandbox.py __exit__

Sandbox.__exit__ unconditionally suppressed every exception from close(), so with client.new() as sb: could never learn that a genuine release failed (auth expired, node down, quota). Now it surfaces a real release failure on a clean block and only suppresses while already unwinding another exception — matching close()'s own contract (it propagates non-404), Pty.__exit__/PortConn.__exit__, and the Go SDK.

python conn.py dial_agent

On the error path, sock.makefile("rb") incremented the socket's internal ref, so sock.close() alone left the fd lingering. Close the reader too when it was created.

Gates

cargo fmt --check + clippy -D warnings clean (mac + a rust:1 Linux container); ruff check clean and 101 pytest pass.

Whole-repo review fixes independent of any feature:
- boot/init sys.rs: add SAFETY invariant comments on the 7 libc unsafe blocks.
- python sandbox.py __exit__: surface a real release failure on a clean block
  instead of always suppressing it (matches close()/Pty/PortConn/the Go SDK).
- python conn.py dial_agent: close the makefile reader on the error path so the
  socket fd is released.
@CMGS CMGS merged commit 1ecc744 into main Jul 9, 2026
2 checks passed
@CMGS CMGS deleted the chore/review-hygiene branch July 9, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant