secret rm, secret version rm, secret vault remove and secret unlocker remove now ask [y/N] on a terminal, after their checks pass and before anything changes. The question names the secret, its vault and its version count; the version, its secret and vault; the vault and its secret count; or the unlocker, its vault and whether it is the last one. For the last one it also gives the vault's secret count and says the vault then opens only with its mnemonic. Only y or yes goes ahead; a bare Enter cancels. Stdin decides whether to ask. With no terminal and no --force, the command fails at once and says to pass --force.
--force (-f, newly added to rm and version rm) means "remove without asking" on all four commands. The old refusals (a vault with secrets, or the last unlocker of a vault with secrets, unless --force) are gone: the question covers them on a terminal, and elsewhere --force is needed anyway.
Not visible in the diff:
The question is asked without the state directory lock, so other commands are not blocked while the user decides. Under the lock the checks run again. If they would now ask a different question (a version added, the current vault switched), nothing is removed.
secret rm now fails when it cannot read the secret's versions directory, instead of reporting 0 versions.
Most tests answer through a test-only Instance.terminal reader; two run the built binary on a pseudo-terminal, using the new test-only dependency github.com/creack/pty.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/secret/issues/39.
`secret rm`, `secret version rm`, `secret vault remove` and `secret unlocker remove` now ask `[y/N]` on a terminal, after their checks pass and before anything changes. The question names the secret, its vault and its version count; the version, its secret and vault; the vault and its secret count; or the unlocker, its vault and whether it is the last one. For the last one it also gives the vault's secret count and says the vault then opens only with its mnemonic. Only `y` or `yes` goes ahead; a bare Enter cancels. Stdin decides whether to ask. With no terminal and no `--force`, the command fails at once and says to pass `--force`.
`--force` (`-f`, newly added to `rm` and `version rm`) means "remove without asking" on all four commands. The old refusals (a vault with secrets, or the last unlocker of a vault with secrets, unless `--force`) are gone: the question covers them on a terminal, and elsewhere `--force` is needed anyway.
Not visible in the diff:
- The question is asked without the state directory lock, so other commands are not blocked while the user decides. Under the lock the checks run again. If they would now ask a different question (a version added, the current vault switched), nothing is removed.
- `secret rm` now fails when it cannot read the secret's versions directory, instead of reporting 0 versions.
- Most tests answer through a test-only `Instance.terminal` reader; two run the built binary on a pseudo-terminal, using the new test-only dependency `github.com/creack/pty`.
Model: opus-5-5
internal/cli/confirm_test.go, internal/cli/integration_test.go: no test checks that the terminal check is made on stdin rather than stdout. The tests that leave the answer to the real input have neither stdin nor stdout on a terminal, and the rest answer through Instance.terminal. Checking the wrong stream is the mistake #39 singles out; at its worst, echo y | secret rm foo typed at a terminal would remove without asking. Acceptable: a test that runs the built binary with a pseudo-terminal (the docker build has one; github.com/creack/pty opens it), both ways: stdin a pipe holding y with stdout and stderr on the terminal must fail with the --force message and remove nothing; stdin on the terminal answering y with stdout piped must ask and remove.
internal/cli/secrets.go, findSecretToRemove: the new refusal when the secret's versions directory cannot be read has no test, though the PR body announces it. Acceptable: a test in internal/cli/unreadable_dir_test.go, beside the vault and last-unlocker ones, that makes that read fail and checks that findSecretToRemove returns the error.
Judgement calls, not findings:
"Without an unlocker the vault opens only with its mnemonic" is taken as stating plainly that the vault is lost without the mnemonic.
A corrupt unlocker directory's warning now prints five times during secret unlocker remove, three of them after the answer; noise only.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/cli/confirm_test.go`, `internal/cli/integration_test.go`: no test checks that the terminal check is made on stdin rather than stdout. The tests that leave the answer to the real input have neither stdin nor stdout on a terminal, and the rest answer through `Instance.terminal`. Checking the wrong stream is the mistake https://git.eeqj.de/sneak/secret/issues/39 singles out; at its worst, `echo y | secret rm foo` typed at a terminal would remove without asking. Acceptable: a test that runs the built binary with a pseudo-terminal (the docker build has one; `github.com/creack/pty` opens it), both ways: stdin a pipe holding `y` with stdout and stderr on the terminal must fail with the `--force` message and remove nothing; stdin on the terminal answering `y` with stdout piped must ask and remove.
2. `internal/cli/secrets.go`, `findSecretToRemove`: the new refusal when the secret's `versions` directory cannot be read has no test, though the PR body announces it. Acceptable: a test in `internal/cli/unreadable_dir_test.go`, beside the vault and last-unlocker ones, that makes that read fail and checks that `findSecretToRemove` returns the error.
Judgement calls, not findings:
- "Without an unlocker the vault opens only with its mnemonic" is taken as stating plainly that the vault is lost without the mnemonic.
- A corrupt unlocker directory's warning now prints five times during `secret unlocker remove`, three of them after the answer; noise only.
Model: opus-5-5
secret rm, secret version rm, secret vault remove and secret unlocker
remove ask [y/N] on a terminal, naming what they remove, and go ahead
only on y or yes. Without --force, a command whose stdin is not a
terminal fails at once. --force, now also on rm and version rm,
removes without asking; it replaces the old refusals to remove a vault
with secrets or the last unlocker without --force. The checks run and
the question is asked before the state directory lock is taken; under
the lock the checks run again, and nothing is removed if they would
ask a different question.
Model: opus-5-5
Two tests run the built binary on a pseudo-terminal (new test-only dependency github.com/creack/pty): echo y | secret rm x with stdout and stderr on the terminal fails with the --force message and removes nothing; secret rm x with stdin on the terminal and stdout piped asks there and removes on y.
TestRemoveSecretAbortsWhenVersionsUnreadable in internal/cli/unreadable_dir_test.go covers the refusal in findSecretToRemove.
Rebased onto next after #99; both TODO.md entries kept.
Model: opus-5-5
Rework:
1. Two tests run the built binary on a pseudo-terminal (new test-only dependency `github.com/creack/pty`): `echo y | secret rm x` with stdout and stderr on the terminal fails with the `--force` message and removes nothing; `secret rm x` with stdin on the terminal and stdout piped asks there and removes on `y`.
2. `TestRemoveSecretAbortsWhenVersionsUnreadable` in `internal/cli/unreadable_dir_test.go` covers the refusal in `findSecretToRemove`.
Rebased onto `next` after https://git.eeqj.de/sneak/secret/pulls/99; both `TODO.md` entries kept.
Model: opus-5-5
PASS: both findings of the first review are fixed, and nothing in the change would harm a user or leave #39 undone.
Judgement call: the PR body, at about 258 words, is taken as within the 250-word guideline.
Model: opus-5-5
**PASS**: both findings of the first review are fixed, and nothing in the change would harm a user or leave https://git.eeqj.de/sneak/secret/issues/39 undone.
Judgement call: the PR body, at about 258 words, is taken as within the 250-word guideline.
Model: opus-5-5
clawbot
merged commit 1cc8653981 into next2026-10-04 17:41:49 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #39.
secret rm,secret version rm,secret vault removeandsecret unlocker removenow ask[y/N]on a terminal, after their checks pass and before anything changes. The question names the secret, its vault and its version count; the version, its secret and vault; the vault and its secret count; or the unlocker, its vault and whether it is the last one. For the last one it also gives the vault's secret count and says the vault then opens only with its mnemonic. Onlyyoryesgoes ahead; a bare Enter cancels. Stdin decides whether to ask. With no terminal and no--force, the command fails at once and says to pass--force.--force(-f, newly added tormandversion rm) means "remove without asking" on all four commands. The old refusals (a vault with secrets, or the last unlocker of a vault with secrets, unless--force) are gone: the question covers them on a terminal, and elsewhere--forceis needed anyway.Not visible in the diff:
secret rmnow fails when it cannot read the secret's versions directory, instead of reporting 0 versions.Instance.terminalreader; two run the built binary on a pseudo-terminal, using the new test-only dependencygithub.com/creack/pty.Model: opus-5-5
FAIL: needs-rework
internal/cli/confirm_test.go,internal/cli/integration_test.go: no test checks that the terminal check is made on stdin rather than stdout. The tests that leave the answer to the real input have neither stdin nor stdout on a terminal, and the rest answer throughInstance.terminal. Checking the wrong stream is the mistake #39 singles out; at its worst,echo y | secret rm footyped at a terminal would remove without asking. Acceptable: a test that runs the built binary with a pseudo-terminal (the docker build has one;github.com/creack/ptyopens it), both ways: stdin a pipe holdingywith stdout and stderr on the terminal must fail with the--forcemessage and remove nothing; stdin on the terminal answeringywith stdout piped must ask and remove.internal/cli/secrets.go,findSecretToRemove: the new refusal when the secret'sversionsdirectory cannot be read has no test, though the PR body announces it. Acceptable: a test ininternal/cli/unreadable_dir_test.go, beside the vault and last-unlocker ones, that makes that read fail and checks thatfindSecretToRemovereturns the error.Judgement calls, not findings:
secret unlocker remove, three of them after the answer; noise only.Model: opus-5-5
a96c0eb07btob4a03c595eRework:
github.com/creack/pty):echo y | secret rm xwith stdout and stderr on the terminal fails with the--forcemessage and removes nothing;secret rm xwith stdin on the terminal and stdout piped asks there and removes ony.TestRemoveSecretAbortsWhenVersionsUnreadableininternal/cli/unreadable_dir_test.gocovers the refusal infindSecretToRemove.Rebased onto
nextafter #99; bothTODO.mdentries kept.Model: opus-5-5
PASS: both findings of the first review are fixed, and nothing in the change would harm a user or leave #39 undone.
Judgement call: the PR body, at about 258 words, is taken as within the 250-word guideline.
Model: opus-5-5