[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Sweep of five audited top-level files in crates/socket-patch-cli/src: json_envelope.rs, update_notifier.rs, main.rs, interrupt.rs and path_scope.rs. It applies the coverage-row notes from the architecture ledger. The aim is fewer wrappers, docs that are true of main, and tests that check what their names claim.

Changes

update_notifier.rs

• Inlined in_ci, upgrade_command and the grace_budget wrapper (each had one caller).
• SOCKET_UPDATE_TIMEOUT_MS and SOCKET_UPDATE_GRACE_MS were parsed by two copies of the same code. Both now go through one millis_or, which replaces grace_budget_from. Dropping the .filter(!empty) changes nothing, because "".parse() already fails.
• GuardCtx, SkipReason, should_check and GuardCtx::capture are now private (only this file uses them).
• Trimmed the 16-line flaky-history doc down to a 4-line why comment.
• Test notice_is_two_prefixed_lines_with_a_green_version checks the exact plain string and the exact colored string. Before, it ran 4 .contains checks that the equality assert already covered, plus a bare "has an escape" check.

main.rs / interrupt.rs

• restore_default_sigpipe and its non-unix stub moved into interrupt's unix install(). The order is unchanged: SIGPIPE is reset first, then the handlers are installed, all before any output. The doc now says the reset is unconditional.

json_envelope.rs

• Deleted the pub use re-export of the sidecar types (nothing imports through it) and the test-only GcReport::to_value.
• usage_error now prints through ui::print_json, which writes the same bytes.
• usage_error_json is now private.
• Removed ~60 comment lines: v5 history, bare issue numbers, a stale "package, diff, blob" list, stale Removed/Verified scopes (now matching the CLI_CONTRACT vocabulary table), the section banner, and "the prior test only checked…" narration.
• Tests:
  • Five tests claimed to cover "every" action, command or status but listed variants by hand and missed some (RolledBack, Rebuilt, Update, 8 of the 10 statuses). They now share the complete ACTIONS and COMMANDS tables, and every_status_serializes_camel_case covers all 10 statuses. It replaces status_serializes_camel_case and special_statuses_serialize_camel_case.
  • Deleted 4 duplicates:
    • record_keeps_summary_in_sync, covered by each_action_bumps_exactly_its_own_counter;
    • old_uuid_omitted_when_unset and skipped_reason_does_not_leak_into_error_field, covered by skipped_event_omits_uuid_and_files;
    • status_serializes_camel_case.
  • 12 from_str(&to_pretty_json()) and from_str(&to_string()) round trips became to_value.

path_scope.rs (tests only; PathScope::matches's one production caller is in rollback.rs, which #1864 holds)

• bound_scope_matches_like_the_per_call_form compared bind().matches against matches, which is implemented as bind().matches, so it could not fail. It now asserts the expected outcomes directly.
• Deleted matching_is_purely_textual, a duplicate of the directory-pattern test.
• Added relative_cwd_strips_relative_candidates for the --cwd . / ./node_modules/x case the cwd field exists for.
• Fixed the stale nested/* comment.

Red→green

These mutations were applied to committed code and then reverted. Each turns the named tests red:

• Making Summary::bump's RolledBack arm bump rebuilt fails each_action_bumps_exactly_its_own_counter. The old test listed neither variant.
• Renaming Command::Update to selfUpdate fails every_command_serializes_to_its_contract_tag and usage_error_json_is_a_full_envelope_for_every_command. Neither old list had Update.
• Painting the notice's version yellow (33) instead of green fails notice_is_two_prefixed_lines_with_a_green_version. The old test only checked for an escape.

Size

+195 / −447 across 5 files. Production code goes from 1,486 to 1,422 lines (−64), and test code from 1,036 to 848 lines (−188).

Behavior

None. Output bytes, JSON, exit codes and signal handling are unchanged.

Verification

• cargo clippy --workspace --all-features -- -D warnings is clean.
• cargo test -p socket-patch-cli --lib passes 936/936.
• The update suite passes 113 (it includes update_notifier_e2e), and the cli suite passes 68.
• interrupt.rs is cfg(unix) code, but it only gained the SIGPIPE line that used to run in main. The Windows path is untouched.

/code-review high

Six findings. Three are fixed:

• the doc now says the SIGPIPE reset is unconditional;
• the module doc names hosted-bundle as the exception to the envelope;
• millis_or's doc no longer calls SOCKET_UPDATE_TIMEOUT_MS a "test knob".

Three are not fixed:

• Public API narrowed (GuardCtx/SkipReason/sidecar re-export/GcReport::to_value): socket_patch_cli's lib only serves its own binary and tests, and nothing in the workspace uses these items.
• Core's UpdateTimeouts::from_env parses SOCKET_UPDATE_TIMEOUT_MS too: this predates the PR (audit A09/C19). The two parses differ: core caps connect and the notifier does not. Unifying them would change behavior, so it stays for the structural row.
• in_ci inlined and the 500 ms literal: in_ci had no test before this PR either, and the 500 ms literal now sits next to its why comment.

Risk

Low. The changes are local, and the touched test suites are green.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeSzcBKdmvUrvNDZi76

---

Note

Low Risk
Refactor and test/doc changes within the CLI crate; PR asserts unchanged JSON bytes, exit codes, and signal handling, with broad test coverage still green.

Overview
This is a behavior-neutral cleanup across five socket-patch-cli modules: fewer one-off helpers, docs aligned with main, and tests that actually cover the full JSON contract tables.

Startup / signals: Unix SIGPIPE handling moves from main into interrupt::install() (still before any output), alongside the existing lock-cleanup signal handlers.

JSON envelope: Trims stale module docs, drops unused GcReport::to_value and sidecar type re-exports, routes usage_error through ui::print_json, and tightens tests with shared ACTIONS/COMMANDS tables so every action, command, and status serializes as documented (including RolledBack, Update, etc.).

Update notifier: Inlines single-use helpers, unifies SOCKET_UPDATE_*_MS parsing via millis_or, narrows GuardCtx/SkipReason/should_check to crate-private, and strengthens the notice formatting test.

Path scope: Test-only fixes—replaces a tautological bind-vs-matches check, adds --cwd . / ./node_modules/… coverage, and corrects glob comments.

^{Reviewed by Cursor Bugbot for commit 37dc766. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: 12f6ad0400b231e28e6a04b52c5a32a8bebe1fab
• CI: 82/82 green on this head (32 passed, 50 skipped). Mergeable, no conflicts.
• Bugbot: reviewed this head; no findings.
• Reviewers: there is no behavior change, but SIGPIPE reset moved from main into interrupt::install() (unix), and GuardCtx/SkipReason/sidecar re-exports were made private. Check that no downstream user of the socket_patch_cli lib needs those.
• Slack announcement could not be sent this run (no Slack send tool available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A tidy-up of five socket-patch-cli files with no behavior change. It inlines one-caller helpers, folds the two millisecond env-knob parsers into one millis_or, moves the Unix SIGPIPE reset from main into interrupt::install(), narrows some pub items to private, and replaces hand-picked test lists with complete ACTIONS/COMMANDS/status tables so the "every variant" tests really cover every variant.

Risk: low. Production changes are small and mechanical. The JSON bytes are the same (ui::print_json is the same to_string_pretty + println!), and the SIGPIPE reset still runs first, before any output. Nothing outside these files used the items that became private (I checked with git grep across crates/).

Look here

• interrupt.rs:53: SIGPIPE reset now at the top of the Unix install(). main calls it first thing, so the order is unchanged. Windows and other targets never did this.
• update_notifier.rs:250: millis_or now parses both SOCKET_UPDATE_TIMEOUT_MS and SOCKET_UPDATE_GRACE_MS. Dropping the !is_empty filter is safe because "".parse() already fails.
• update_notifier.rs:305: the upgrade hint is still computed only after the skip/early-return checks.
• json_envelope.rs:707: the shared ACTIONS/COMMANDS tables now include RolledBack, Rebuilt and Update, which the old lists missed.
• path_scope.rs:293: the new --cwd . relative-candidate test.

Verified

• Read the full diff (5 files, +195/−447). CHANGELOG.md is untouched.
• Ran cargo test -p socket-patch-cli --lib -- json_envelope update_notifier path_scope locally on this head: 42/42 pass.
• CI is green on this head (ci-ok and clippy pass). It is mergeable and has no review threads. Bugbot reviewed this head and found nothing.

Changes I made: none.

Open questions (not blocking)

• The sweep dropped one assertion (scope(&["*"]) matches /proj/anything) along with its stale comment. I did not check whether another test still covers a bare * pattern; if none does, the coverage loss is small.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Re the final review's open question: no other test covered a bare * pattern. The sweep dropped that assertion by accident along with the stale comment next to it. 37dc766 restores it (scope(&["*"]) matches /proj/anything), and the path_scope tests pass 12/12 locally. This push re-runs CI, and auto-merge will wait for it.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (head 37dc766)

What it does. A tidy-up of five socket-patch-cli files. It doesn't change what the program does.

• It inlines one-caller helpers and folds the two millisecond env parsers into millis_or.
• It moves the Unix SIGPIPE reset from main into interrupt::install() and makes unused pub items private.
• It swaps hand-picked test lists for complete tables, so the "every variant" tests really cover every variant.

Since the last brief (12f6ad0), one test-only commit restores the bare * path-scope assertion. That answers the earlier open question: no other test covered it.

Risk: low. Every runtime change is a mechanical inline, or a move that keeps the call order. Serde attributes and field names are untouched, so the JSON output is identical. Envelope test coverage went up.

Look here

• interrupt.rs:53: the SIGPIPE reset, now at the top of the Unix install(). main calls it first, so it still runs before any output. Non-Unix is unchanged.
• update_notifier.rs:250 and :268: millis_or and the default timeouts (2000 ms fetch, 500 ms grace). Dropping the !is_empty filter is safe because "".parse() already fails.
• json_envelope.rs:675: usage_error now prints through ui::print_json, which produces the same bytes. Exit code is still 2.
• json_envelope.rs:707: the shared ACTIONS/COMMANDS tables now include RolledBack and Update.
• path_scope.rs:228: the restored bare * assertion.

Verified

• Read the full diff (5 files, +197/−447). CHANGELOG.md is untouched.
• Each deleted test is still covered by a remaining one.
• cargo test -p socket-patch-cli --lib -- json_envelope update_notifier path_scope interrupt on this head: 42/42 pass.
• cargo clippy -p socket-patch-cli --lib --bins -- -D warnings: clean.
• CI on this head: 30/30 green, 19 path-skipped. Mergeable, with no review threads.
• Bugbot reviewed 12f6ad0 clean and has been asked to review this head.

Changes I made: none.

Nits (not blocking): the Skipped doc near json_envelope.rs:482 has a short dangling line after the reflow.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 37dc766. Configure here.}