Skip to content

hygiene round: typed dial error, narrowed catches, layout and ownership - #130

Merged
CMGS merged 10 commits into
mainfrom
review/hygiene-2026-09-03
Sep 3, 2026
Merged

CMGS merged 10 commits into
mainfrom
review/hygiene-2026-09-03

Conversation

@CMGS

@CMGS CMGS commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

A hygiene round over the Go, Rust and Python sources: one bug fix with a
regression test, review commits that only lower comment and allocation counts,
and two cuts.

Commits

fix(python): typed error for a refused connectionsocket.create_connection
sat outside the try that wraps the rest of dial_agent, so a refused,
unreachable, or DNS-failed dial leaked a raw OSError while every other failure
came back typed. Wrapped as ProtocolError carrying the cause; the test dials a
closed port and fails on the parent commit with ConnectionRefusedError.

review(python): narrow the background-thread catches; one-line comments
the proxy dial, the pump_out suppress and _feed_stdin's suppress caught bare
Exception, hiding programming errors in daemon threads; narrowed to
(SandboxError, OSError), which is exactly what the code below them raises.

review(go): layout, one func type, WalkDir, logger name, test comments
retryArchiveDelete logged under its sibling's name; func(NodeState) bool was
spelled twice in mesh and claimFollow carried two inline func types in a
250-character signature, all now named types; tarInto moves to
filepath.WalkDir so a symlink or device entry costs no stat; payload types move
ahead of the types they serve.

review(rust): ownership on the silkd request path and review(rust): fewer
allocations on the git and session paths
— see the table below.

review(rust): layout, shared cmdline tokenizer, one-line docssysutil.rs
free functions sorted public above private as a pure move, each functional
group's internal order preserved and every // SAFETY: line travelling with its
block; cfg::parse and cfg::debug_requested shared one cmdline tokenizer.

cut(init): switch_root reuses move_mountswitch_root spelled
mount(\".\", \"/\", None, libc::MS_MOVE, None) inline, which is exactly
move_mount(\".\", \"/\"). Same syscall, same error string, since both go through
the one mount helper that formats it. A dedup, not a line cut.

cut(init): drop the debug-flag divergence testparse and
debug_requested no longer hand-roll separate cmdline walks, so the test that
cross-checked them is redundant. Its one unique case, that the last
sandbox.debug occurrence wins for parse, moves into parse_debug_forms.

review: one-line docs and WHY comments (STE 101) — every multi-line comment
across silkd/src and boot/init/src condensed to one line or deleted where it
restated the code. 630 comment lines to 304. Five multi-line blocks survive, each
because a contract genuinely does not fit one line: the wire-protocol module doc
in proto.rs, the kernel ip= positional field order in cfg.rs, and the
module docs of lsp.rs, tree.rs and the test harness. Every // SAFETY: block
is untouched.

Allocations removed on the silkd request path

Site Before After
parse_file_line path a Vec, a join String and a second String one String, truncated in place
GitFileStatus XY codes two String per changed file two char
apply_config a String from a literal per git run borrowed constant
Response::Info.version one String per info call borrowed constant
ProcInfo.state one String per process per ps borrowed constant
Io::converse a format String per command a cleared, reused buffer
tree::pull PathBuf plus OsString, plus a third when the parent is empty borrowed from the request path
tree::drain always copies out of the buffer moves it when valid UTF-8
fs::write_atomic a String from display() &Path threaded through
fs::scan_dir two allocations per directory entry one
lsp::manifest_dir one String per manifest read borrowed constant

The wire is unchanged. char and Cow<'static, str> serialize exactly as the
strings they replace: the Rust and Go fixture suites each pin exactly 61 frames
over protocol/wire/fixtures/v1 and both pass, and git_e2e asserts on the JSON
a real silkd emits. write_atomic taking &Path also drops a latent bug, since
display() would have mangled a non-UTF-8 path.

Gates

macOS: cargo fmt --check, cargo clippy --all-targets -- -D warnings and
cargo test clean in silkd (13 suites, 107 tests) and boot/init (15);
GOWORK=off make go-lint prints ten 0 issues. lines with fmt --diff silent;
asl and GOOS=linux asl find nothing in all five Go modules;
go test -race -count=1 ./... passes in each; ruff check clean and 155 pytest
tests pass.

Linux (rust:1.98.0, --platform linux/arm64, repo root mounted so the fixtures
resolve): silkd fmt, clippy and all 107 tests green; boot/init the same with 16
tests, one of them Linux-gated.

ruff format --check reports the same 8 files as main. Reformatting them is
unrelated churn.

@CMGS
CMGS merged commit e124a0d into main Sep 3, 2026
4 checks passed
@CMGS
CMGS deleted the review/hygiene-2026-09-03 branch September 3, 2026 07:21
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