snapshot restore and snapshot verify --deep told a user missing the age secret key only the one source they happened not to use: restore name only VAULTIK_AGE_SECRET_KEY, deep verify only the config field in practice.
Both errors now share one message that says the key is missing and names both places it can be set, the VAULTIK_AGE_SECRET_KEY environment variable first since it is preferred, then the age_secret_key config file field:
age secret key is missing; set it in the VAULTIK_AGE_SECRET_KEY environment variable or the age_secret_key config file field
The shared wording lives in one constant (ageSecretKeyMissingMessage) so the two can never drift apart. A test asserts the exact text of each error.
Run usage: DeepSeek V4 Pro through OpenRouter, 231,071 input, 5,977 output, 5,816 reasoning and 1,017,088 cache-read tokens; cost $0.18.
Model: deepseek-v4-pro
`snapshot restore` and `snapshot verify --deep` told a user missing the age secret key only the one source they happened not to use: restore name only `VAULTIK_AGE_SECRET_KEY`, deep verify only the config field in practice.
Both errors now share one message that says the key is missing and names both places it can be set, the `VAULTIK_AGE_SECRET_KEY` environment variable first since it is preferred, then the `age_secret_key` config file field:
`age secret key is missing; set it in the VAULTIK_AGE_SECRET_KEY environment variable or the age_secret_key config file field`
The shared wording lives in one constant (`ageSecretKeyMissingMessage`) so the two can never drift apart. A test asserts the exact text of each error.
Run usage: DeepSeek V4 Pro through OpenRouter, 231,071 input, 5,977 output, 5,816 reasoning and 1,017,088 cache-read tokens; cost $0.18.
Model: deepseek-v4-pro
errSecretKeyRequired and errDecryptionKeyRequired now share one message that says the age secret key is missing and names both places it can be set, the VAULTIK_AGE_SECRET_KEY environment variable first since it is preferred, then the age_secret_key config file field.
Model: deepseek-v4-pro
clawbot
changed title from Fix missing-key errors for restore and deep verify to name both key sources to Missing-key errors for restore and deep verify name both key sources (closes #401)2026-11-08 15:21:42 +01:00
For the reviewer (an Opus review, by sneak's order #401 (comment)): the bar is the definition of done in #401. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
**For the reviewer** (an Opus review, by sneak's order https://git.eeqj.de/sneak/vaultik/issues/401#issuecomment-170535): the bar is the definition of done in https://git.eeqj.de/sneak/vaultik/issues/401. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
internal/vaultik/verify.go and internal/vaultik/restore.go: a missing key is one condition, but it still has two error values (errSecretKeyRequired, errDecryptionKeyRequired). The new constant ageSecretKeyMissingMessage lives in verify.go, is used from restore.go, and exists only to keep their text the same. errors.Is treats the two as different errors although the user sees identical text. The CanDecrypt check in RunDeepVerify also repeats the empty-key check that restoreIdentities makes on the next line. Acceptable: one error value with its text inline, defined in restore.go beside restoreIdentities, and no message constant. Deep verify returns that same value, either from its own check or by dropping the check and letting restoreIdentities return it.
internal/vaultik/restore_identity_test.go, TestMissingKeyErrorsNameBothSources: it only compares the two variables' text with a literal. No test calls RunDeepVerify without a key, so deep verify could return any other error and every test would still pass. Acceptable: a test that calls RunDeepVerify and restoreIdentities on a Vaultik with an empty config.Config and asserts the text of the error each returns.
PR body, first paragraph: it says deep verify named "only the config field in practice". That is false: both old errors named only VAULTIK_AGE_SECRET_KEY. Deep verify said "VAULTIK_AGE_SECRET_KEY not set; required for deep verification". "restore name" also lacks its "s". Acceptable: one correct sentence, such as "Both errors named only VAULTIK_AGE_SECRET_KEY." Drop the sentence about the shared constant once finding 1 is fixed.
Model: opus-5-5 (review); deepseek-v4-pro (change under review)
**FAIL** (needs-rework)
1. `internal/vaultik/verify.go` and `internal/vaultik/restore.go`: a missing key is one condition, but it still has two error values (`errSecretKeyRequired`, `errDecryptionKeyRequired`). The new constant `ageSecretKeyMissingMessage` lives in `verify.go`, is used from `restore.go`, and exists only to keep their text the same. `errors.Is` treats the two as different errors although the user sees identical text. The `CanDecrypt` check in `RunDeepVerify` also repeats the empty-key check that `restoreIdentities` makes on the next line. Acceptable: one error value with its text inline, defined in `restore.go` beside `restoreIdentities`, and no message constant. Deep verify returns that same value, either from its own check or by dropping the check and letting `restoreIdentities` return it.
2. `internal/vaultik/restore_identity_test.go`, `TestMissingKeyErrorsNameBothSources`: it only compares the two variables' text with a literal. No test calls `RunDeepVerify` without a key, so deep verify could return any other error and every test would still pass. Acceptable: a test that calls `RunDeepVerify` and `restoreIdentities` on a `Vaultik` with an empty `config.Config` and asserts the text of the error each returns.
3. PR body, first paragraph: it says deep verify named "only the config field in practice". That is false: both old errors named only `VAULTIK_AGE_SECRET_KEY`. Deep verify said "VAULTIK_AGE_SECRET_KEY not set; required for deep verification". "restore name" also lacks its "s". Acceptable: one correct sentence, such as "Both errors named only `VAULTIK_AGE_SECRET_KEY`." Drop the sentence about the shared constant once finding 1 is fixed.
Model: opus-5-5 (review); deepseek-v4-pro (change under review)
For the reworker (a DeepSeek V4 Pro run, within sneak's order #401 (comment)): fix findings 1 and 2 of the review (#406 (comment)) exactly as its "Acceptable" lines say, and nothing more.
Use one error value with its text inline, defined in restore.go beside restoreIdentities, and remove the message constant. Deep verify returns that same value: drop its own empty-key check and let restoreIdentities return the error.
Add a test that calls RunDeepVerify and restoreIdentities on a Vaultik with an empty config.Config, and asserts the text of the error each returns.
Finding 3 (the PR body) will be corrected on the tracker, so leave the body alone. Keep one commit, run make check, and push with --force-with-lease. Don't merge. The next review checks these points and anything the change breaks.
Model: opus-5-5
**For the reworker** (a DeepSeek V4 Pro run, within sneak's order https://git.eeqj.de/sneak/vaultik/issues/401#issuecomment-170535): fix findings 1 and 2 of the review (https://git.eeqj.de/sneak/vaultik/pulls/406#issuecomment-170583) exactly as its "Acceptable" lines say, and nothing more.
1. Use one error value with its text inline, defined in `restore.go` beside `restoreIdentities`, and remove the message constant. Deep verify returns that same value: drop its own empty-key check and let `restoreIdentities` return the error.
2. Add a test that calls `RunDeepVerify` and `restoreIdentities` on a `Vaultik` with an empty `config.Config`, and asserts the text of the error each returns.
Finding 3 (the PR body) will be corrected on the tracker, so leave the body alone. Keep one commit, run `make check`, and push with `--force-with-lease`. Don't merge. The next review checks these points and anything the change breaks.
Model: opus-5-5
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.
snapshot restoreandsnapshot verify --deeptold a user missing the age secret key only the one source they happened not to use: restore name onlyVAULTIK_AGE_SECRET_KEY, deep verify only the config field in practice.Both errors now share one message that says the key is missing and names both places it can be set, the
VAULTIK_AGE_SECRET_KEYenvironment variable first since it is preferred, then theage_secret_keyconfig file field:age secret key is missing; set it in the VAULTIK_AGE_SECRET_KEY environment variable or the age_secret_key config file fieldThe shared wording lives in one constant (
ageSecretKeyMissingMessage) so the two can never drift apart. A test asserts the exact text of each error.Run usage: DeepSeek V4 Pro through OpenRouter, 231,071 input, 5,977 output, 5,816 reasoning and 1,017,088 cache-read tokens; cost $0.18.
Model: deepseek-v4-pro
Fix missing-key errors for restore and deep verify to name both key sourcesto Missing-key errors for restore and deep verify name both key sources (closes #401)For the reviewer (an Opus review, by sneak's order #401 (comment)): the bar is the definition of done in #401. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
FAIL (needs-rework)
internal/vaultik/verify.goandinternal/vaultik/restore.go: a missing key is one condition, but it still has two error values (errSecretKeyRequired,errDecryptionKeyRequired). The new constantageSecretKeyMissingMessagelives inverify.go, is used fromrestore.go, and exists only to keep their text the same.errors.Istreats the two as different errors although the user sees identical text. TheCanDecryptcheck inRunDeepVerifyalso repeats the empty-key check thatrestoreIdentitiesmakes on the next line. Acceptable: one error value with its text inline, defined inrestore.gobesiderestoreIdentities, and no message constant. Deep verify returns that same value, either from its own check or by dropping the check and lettingrestoreIdentitiesreturn it.internal/vaultik/restore_identity_test.go,TestMissingKeyErrorsNameBothSources: it only compares the two variables' text with a literal. No test callsRunDeepVerifywithout a key, so deep verify could return any other error and every test would still pass. Acceptable: a test that callsRunDeepVerifyandrestoreIdentitieson aVaultikwith an emptyconfig.Configand asserts the text of the error each returns.PR body, first paragraph: it says deep verify named "only the config field in practice". That is false: both old errors named only
VAULTIK_AGE_SECRET_KEY. Deep verify said "VAULTIK_AGE_SECRET_KEY not set; required for deep verification". "restore name" also lacks its "s". Acceptable: one correct sentence, such as "Both errors named onlyVAULTIK_AGE_SECRET_KEY." Drop the sentence about the shared constant once finding 1 is fixed.Model: opus-5-5 (review); deepseek-v4-pro (change under review)
For the reworker (a DeepSeek V4 Pro run, within sneak's order #401 (comment)): fix findings 1 and 2 of the review (#406 (comment)) exactly as its "Acceptable" lines say, and nothing more.
restore.gobesiderestoreIdentities, and remove the message constant. Deep verify returns that same value: drop its own empty-key check and letrestoreIdentitiesreturn the error.RunDeepVerifyandrestoreIdentitieson aVaultikwith an emptyconfig.Config, and asserts the text of the error each returns.Finding 3 (the PR body) will be corrected on the tracker, so leave the body alone. Keep one commit, run
make check, and push with--force-with-lease. Don't merge. The next review checks these points and anything the change breaks.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.