8000 fix(installer): remove the QUIC block helper and its sudoers rule on uninstall (T93) by minimal-foundry[bot] · Pull Request #2651 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content

fix(installer): remove the QUIC block helper and its sudoers rule on uninstall (T93) - #2651

Merged
norrietaylor merged 3 commits into
mainfrom
agent/2631-remove-the-quic-block-helper-and-its-sudoers-rul
Oct 27, 2026
Merged

norrietaylor merged 3 commits into
mainfrom
agent/2631-remove-the-quic-block-helper-and-its-sudoers-rul

Conversation

@minimal-foundry
Copy link
Copy Markdown
Contributor

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_found check name /usr/local/lib/minimal/quic443-leaf and /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

line exit tail
just test-installer finalize_install_uninstall 0 d.slice ok - the printed remedies remove usr/local/lib/minimal ok - the boot unit is disabled before anything is removed ok - the remedy removes the classifier table ok - uninstall with an unrecorded classifier tree exits 0 ok - an unrecorded classifier tree is not advised for removal ok

Verify

min task run agent-verify passed ([tasks.verify]).

Closes #2631

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).
@coderabbitai
coderabbitai Bot commented Oct 26, 2026 •
Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 500811ae-6011-41dc-aa78-0c6720f5dc53

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@minimal-foundry
minimal-foundry Bot commented Oct 26, 2026 •
Copy link
Copy Markdown
Contributor Author

Approved: every review thread resolved at 72a1c7498440.

@norrietaylor resolved the last thread, which releases the approval this review had withheld.

Nothing to do.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 2 findings, 2 inline.

Panel: 2 of 3 personas ran (correctness, security); did not complete: supply-chain.

Comment thread scripts/install.sh Resolved
Comment thread scripts/install.sh Resolved
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.
@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 1 finding, 1 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread scripts/install.sh Resolved
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 1 of 10 on the runtime's review findings, pushed as bcaf9fb05b27.

line exit tail
just test-installer finalize_install_uninstall 0 nit is disabled before anything is removed ok - the remedy removes the classifier table ok - uninstall with an unrecorded classifier tree exits 0 ok - an unrecorded classifier tree is not advised for removal ok - every printed remedy ran (0 line(s)) ok - an unrecorded classifier tree stays

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — scripts/install.sh: On a yes, uninstall now checks the helper and its sudoers rule after min finalize-install --undo and prints the manual root remedy if either is left. Adding them to resolver.rs is outside this task's files.
  • 👍 valid — PRRT_kwDOPdfcxM6wfseD: The helper is now its own list item, 'the QUIC block helper and its sudoers rule ()', with its own remedy line. I also removed the requirement ID from the comment. (not posted on the thread: not an open thread on the pull request)

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.
@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 2 of 10 on the runtime's review findings, pushed as 72a1c7498440.

line exit tail
just test-installer finalize_install_uninstall 0 nit is disabled before anything is removed ok - the remedy removes the classifier table ok - uninstall with an unrecorded classifier tree exits 0 ok - an unrecorded classifier tree is not advised for removal ok - every printed remedy ran (0 line(s)) ok - an unrecorded classifier tree stays

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — scripts/install.sh:608: On a yes, uninstall now removes quic443-leaf and its sudoers rule with sudo rm -f. It prints the manual remedy only if one is left. The yes path has no test because the harness has no tty.

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 min finalize-install --undo (resolver::undo_command) to remove the two files. That code is outside this task's layers.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot added the review:approved minimal-review approved the current head label Oct 27, 2026
@norrietaylor norrietaylor left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@norrietaylor norrietaylor changed the title T93 — Remove the QUIC block helper and its sudoers rule on uninstall fix(installer): remove the QUIC block helper and its sudoers rule on uninstall (T93) Oct 27, 2026
@norrietaylor norrietaylor left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@norrietaylor
norrietaylor added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 03125f8 Oct 27, 2026
39 checks passed
@norrietaylor
norrietaylor deleted the agent/2631-remove-the-quic-block-helper-and-its-sudoers-rul branch October 27, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:approved minimal-review approved the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

T93 — Remove the QUIC block helper and its sudoers rule on uninstall

1 participant

0