[Mikola Lysenko (mikolalysenko)]

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

Refs #1089 (the remove slice of child 6; the tracking issue stays open)

Root cause

The remove behaviour tests matched English sentences, so a wording
change could fail a test whose behaviour was still right. Every other
command suite (get, scan, vendor, apply, rollback, repair)
already keeps human output in a *_render.rs file and checks behaviour
through --json codes via common::envelope. remove had no render
file yet.

Change

• remove/remove_render.rs (new) holds the human-mode and PTY
  wording checks: 10 tests moved from remove_failure_paths.rs
  (including the PTY decline test). It also holds the human legs split
  out of three mixed tests: the hosted-only --preserve-state note, the
  detached --preserve-state dry-run preview, and the detached
  drift-keep lines.
• remove/remove_fixture.rs (new) holds the shared runner, the
  manifest and ledger writers, the hosted-redirect lock pair, the mock npm
  registry and the mixed drift-keep fixture. These used to be private
  helpers in remove_failure_paths.rs.
• remove/remove_failure_paths.rs keeps the --json behaviour tests.
  The only message reads left are the ones its header allows: a purl, a
  flag, a path, a remedy command, and the "manifest was not modified"
  promise. The manifest-backed --preserve-state JSON test now also
  checks that the lock is restored and the manifest entry is kept. Those
  checks used to live only in the human-mode test that moved.
• .github/workflows/ci.yml: this is Fix nightly Pipenv pylock legs broken by uv 0.13.2 #1639's uv pin
  (version: "0.13.1" on the Pipenv/pip setup-uv step), copied here
  so this PR can go green while main is red from uv 0.13.2. It changes
  nothing once Fix nightly Pipenv pylock legs broken by uv 0.13.2 #1639 merges.

Assertions of 4+-word sentences in remove_failure_paths.rs went from
11 to 1. The one left is the "manifest was not modified" promise. No
production code changes.

Testing

• cargo test -p socket-patch-cli --all-features --test remove: 103
  passed, 0 failed. Main has 100: three mixed human/JSON tests were each
  split into a JSON test and a render test, and no test was dropped.
• cargo fmt --all -- --check: clean.
• cargo clippy --locked --workspace --all-features -- -D warnings (the
  same command CI runs): clean. Running clippy with --tests reports only
  existing findings in tests/common/mod.rs and
  tests/prebuilt_common/mod.rs, none in files this PR touches.
• I ran only the changed suite, not the whole workspace: the PR touches
  one test binary and no library code.
• No cfg(windows)/cfg(unix) logic changed. The unix-only helpers keep
  their #[cfg(unix)], and std::path::Path is imported inside the
  unix-only pty module, so Windows builds have no unused import. For
  that reason I didn't dispatch a full-scope run.
• CI on 9fe4b7b is green: ci-ok, clippy, coverage, and every lean
  e2e leg, including the pipenv pylock leg. Bugbot found no issues.

CI cost

The workflow edit only pins a tool version. remove is still one test
binary in the same job, so a lean PR run is the same before and after:
about 28 Linux jobs, about 8 minutes.

/code-review high findings

• Fixed:
  • The --json preserve-state test now checks the restored lock and
    the kept manifest.
  • The duality test's message check matches main again.
  • Section headers and numbering are fixed.
  • The corrupt pre-v5 ledger test asserts no wording, so it stays in
    the behaviour file.
• Not fixed here: remove_duality_invariants.rs keeps its own copy of
  the hosted fixtures. They differ from remove_fixture.rs: its
  NpmRegistry takes a dist value. Merging the two belongs in a
  separate dedup slice.
• Not fixed here: the PTY runner is copied across render suites
  (rollback_render.rs, scan_tty.rs, interactive_prompts_e2e.rs, and
  now remove_render.rs). Moving it into common/pty_io.rs touches four
  suites and belongs in its own PR.

Follow-ups (still open on #1089)

cli_remove_silent.rs keeps its sentence checks for loud vs --silent
output. Whether silent/TTY suites count as render suites is still an
open question on #1089.

https://claude.ai/code/session_01S8t79pTCoiDJkY1YX6A7Vq

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] e2e (ubuntu-latest, e2e_vex_build, pipenv:: --ignored, 2026.8.0) failed on f8ded66 (job): uv pip install -r pylock.toml → "The target environment is not supported by the lockfile". This failure isn't from this PR, which only changes the remove test files. uv 0.13.2 broke this leg on main as well (nightly 39376690526, and the merge_group runs since 15:54).

I ported #1639's ci.yml uv pin (version: "0.13.1" on the Pipenv/pip setup-uv step) as 9fe4b7b. It does nothing once #1639 merges. I left out the disabled pipenv-compatibility.yml half. It doesn't change job count or minutes.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

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 9fe4b7b. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: 9fe4b7b5. Mergeable, no conflicts; CHANGELOG.md not touched.
• CI: 55/55 checks green on the head (36 success, 19 skipped by path filters), including the pipenv:: --ignored, 2026.8.0 leg.
• Bugbot: reviewed 9fe4b7b5, no findings; 0 open review threads.
• Reviewer focus: test-only move of remove human-output checks into remove_render.rs; the ci.yml edit is Fix nightly Pipenv pylock legs broken by uv 0.13.2 #1639's uv 0.13.1 pin and no-ops once Fix nightly Pipenv pylock legs broken by uv 0.13.2 #1639 merges.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Splits the remove CLI test suite (#1089). remove_failure_paths.rs keeps the --json behaviour tests. The human and PTY wording checks move to a new remove_render.rs, and the shared helpers move to remove_fixture.rs. Three tests that mixed a JSON leg and a human leg are each split in two. No production code changes.

Risk: low. Only tests change. I diffed every old test body against the new files and found no lost assertions.

Look here

• remove_failure_paths.rs:427:`` the manifest-backed --preserve-state JSON test now also checks the restored lock and the kept manifest.
• remove_failure_paths.rs:467:``` the new hosted-only JSON half of a split test.
• remove_render.rs:386 and :430: the render halves of the split tests.
• remove_fixture.rs: the helpers, now pub(crate). Bodies are identical to the old private copies, and the unix-only ones are still #[cfg(unix)].

Verified

• cargo test -p socket-patch-cli --all-features --test remove: 103/103 pass. The test count goes from 73 to 76 (+3 from the splits), as the PR says.
• Test-by-test comparison: 31 of the 38 old tests are byte-identical, 1 is renamed with the same body, and every assertion in the other 5 is still present in either the JSON half or the render half.
• Sentence checks (4+ words): 22 before, now 2 in the behaviour file and 20 in the render file, so none were dropped. The description's "11 to 1" uses a different counting rule.
• cargo clippy -p socket-patch-cli --all-targets -D warnings: no findings in the files this PR touches. It does fail on main from other test files, but CI's clippy job doesn't run --all-targets.
• CHANGELOG.md is untouched and the PR merges cleanly into main a54753a.
• CI: 55/55 green on 9fe4b7b (36 success, 19 skipped). Cursor Bugbot is green with no findings. 0 open threads.
• The CI uv-pin commit is already on main, so it adds no diff.

Changes I made: none.

Open questions (non-blocking)

• remove_cleanup_failures_warn_not_fatal (:1057) is in the behaviour file but still runs in human mode and checks stderr wording. A --json twin pinning three cleanup_failed warnings passes locally. It was the same before this PR, so it can be a follow-up unless you'd like it here.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] About the open question on remove_cleanup_failures_warn_not_fatal: I'm leaving it as it is in this PR. It was already like this on main, and adding a --json twin now would mean a new push to a green, approved-pending head. I'll do it as the next remove follow-up on #1089, along with the cli_remove_silent.rs question.

---

Generated by Claude Code