Repository navigation
fix(installer): remove the QUIC block helper and its sudoers rule on uninstall (T93) - #2651
Conversation
Uninstall now finds /usr/local/lib/minimal/quic443-leaf and /etc/sudoers.d/minimal-quic443-leaf with the classifier units, names them in its list, and its printed remedy removes both (T93, #2631).
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The design of record says nothing about uninstall, so this ruling is a PASS by absence. The change adds the QUIC block helper and its sudoers rule to the uninstall list and the printed remedy. The paths match the paths that install-host-classifier.sh installs. One sudo rm -f removes both files, so the remedy does not leave the sudoers rule without its helper. The new rehearsal checks the proof that the task states.
Ruled at 89130a3 against gominimal/arch@109541a48c31 by claude-opus-5-5.
Uninstall now names the QUIC block helper and its sudoers rule apart from the classifier units, with its own remedy line. On a yes at the prompt, min finalize-install --undo does not remove the two files, so uninstall prints the manual remedy when either one is still there.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The design of record is silent on uninstall, so this change passes by absence, as the ruling on 89130a32 did. The new compare lists the QUIC block helper and its sudoers rule as an item of their own. After --undo, a check prints the manual remedy if either file remains. That path fails closed. No conditions.
Ruled at bcaf9fb against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
Uninstall lists the QUIC block helper and its sudoers rule as their own item, for example 'the QUIC block helper and its sudoers rule (/usr/local/lib/minimal/quic443-leaf)'. The printed root remedy removes both files. On a yes at the prompt, uninstall prints that remedy if either file is still there after min finalize-install --undo. Notes from the agent: No test covers the interactive yes branch. The uninstall prompt tests [ -t 0 ], and the test harness runs the installer with stdin from /dev/null, so it cannot reach that branch. The real fix for that branch is to add the two paths to the undo script in crates/minimal/src/resolver.rs (linux_undo_command), but that file is outside this task's layers. Until then, the installer prints the manual remedy for them. I also changed the expected remedy-line count in the all-items scenario of case_finalize_install_uninstall from 5 to 6, because the helper now has its own line. |
min finalize-install --undo does not remove the helper or its sudoers rule. On a yes at the uninstall prompt, the installer now removes both files with sudo, and prints the manual remedy only if one is left.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The design of record has no line on uninstall or on sudoers drop-ins. This ruling is PASS by absence, the same as both earlier heads of this PR. The new yes branch removes the QUIC block helper and its sudoers rule. A failed removal prints the manual remedy, so the path fails closed. The non-interactive path still never elevates.
Ruled at 72a1c74 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 2 unresolved threads — minimal-foundry[bot] scripts/install.sh, minimal-foundry[bot] scripts/install.sh:608. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
Uninstall lists the QUIC block helper and its sudoers rule as an item of their own. The printed root remedy removes both files. On a yes at the terminal prompt, uninstall now removes both files with sudo. Notes from the agent: Round 2 changed only scripts/install.sh. On a yes at the prompt, the installer now runs sudo rm -f on each file that is present. It then checks that each file is gone, and prints the manual remedy only if one is left. The yes path has no test. The harness runs the installer with stdin from /dev/null, so the [ -t 0 ] prompt branch never runs, and no other item has a yes-path test. A test needs a pty driver such as script(1), which this harness does not use now. The other option is to teach |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of 72a1c7498440 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
There was a problem hiding this comment.
Own review at 72a1c74. Uninstall now finds the QUIC block helper and its sudoers rule as an item of their own and names them in the finalize-install list. Their printed remedy removes both. On a yes it removes them itself, because min finalize-install --undo does not. install_test.sh seeds both files, checks that the item is named, and applies the printed remedies, which leave neither behind.
Generated by Claude Code
There was a problem hiding this comment.
Babysitter approval at 72a1c74: every merge gate passes at this head (arch PASS at 72a1c74, zero open threads, every lane aggregator green). Squash title fix(installer): remove the QUIC block helper and its sudoers rule on uninstall (T93) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
T93 — Remove the QUIC block helper and its sudoers rule on uninstall
slice: S12
kind: task
covers: BEP-018
size: M
est: 20 code, 180 tests
executor: agent
difficulty: routine
model: standard
diagnostics: uninstall names the helper and its sudoers rule among the finalize-install files it lists.
observability: none: the installer is a shell script that writes no log.
layers:
- uninstall:
- scripts/install.sh: the uninstall list and its
_cls_foundcheck name/usr/local/lib/minimal/quic443-leafand/etc/sudoers.d/minimal-quic443-leaf(#2631)- scripts/install_test.sh: after the uninstall rehearsal neither file exists
test: just test-installer finalize_install_uninstall
produces: none
consumes: none
proof: Uninstall removes the QUIC block helper and its sudoers rule.
depends on:
- blocked by #2558
Generated by Claude Code
Proof
On a host where min finalize-install ran, the installer's uninstall now lists /usr/local/lib/minimal/quic443-leaf (when it is the first classifier file found) with the classifier units. Its printed root remedy removes that helper and /etc/sudoers.d/minimal-quic443-leaf. The uninstall rehearsal in install_test.sh shows that neither file exists after the remedies run.
Notes from the agent: The two paths join the classifier loop in maybe_remove_finalize_install, so they share the 'classifier units' list item and its 'sudo rm -f' remedy. The list item names only the first file it finds. A host that has only the helper and its rule sees the helper path, and the new rehearsal scenario pins that. dash is not installed on this box, so the dash pass of test-installer did not run. Nothing outside the layers was touched.
Tests
just test-installer finalize_install_uninstallVerify
min task run agent-verifypassed ([tasks.verify]).Closes #2631