When an unlocker directory exists but is missing unlocker-metadata.json, ListUnlockers() previously returned a hard error, crashing the entire unlocker ls command. This change logs a debug warning and skips the directory instead.
Changes
internal/vault/unlockers.go: Replace return nil, fmt.Errorf(...) with secret.Debug() + continue
=== RUN TestListUnlockers_SkipsMissingMetadata
--- PASS: TestListUnlockers_SkipsMissingMetadata (0.40s)
PASS
ok git.eeqj.de/sneak/secret/internal/vault 0.565s
Test Output (full vault package)
=== RUN TestVaultOperations
--- PASS: TestVaultOperations (0.37s)
=== RUN TestListUnlockers_SkipsMissingMetadata
--- PASS: TestListUnlockers_SkipsMissingMetadata (0.38s)
PASS
ok git.eeqj.de/sneak/secret/internal/vault 1.115s
Linting
golangci-lint run ./internal/vault/ reports 0 issues.
Note:go test ./internal/cli/ fails pre-existing due to VCS stamping (-buildvcs=false needed); unrelated to this change.
## Summary
When an unlocker directory exists but is missing `unlocker-metadata.json`, `ListUnlockers()` previously returned a hard error, crashing the entire `unlocker ls` command. This change logs a debug warning and skips the directory instead.
## Changes
- `internal/vault/unlockers.go`: Replace `return nil, fmt.Errorf(...)` with `secret.Debug()` + `continue`
- `internal/vault/vault_test.go`: Add `TestListUnlockers_SkipsMissingMetadata`
## Test Output (new test only)
```
=== RUN TestListUnlockers_SkipsMissingMetadata
--- PASS: TestListUnlockers_SkipsMissingMetadata (0.40s)
PASS
ok git.eeqj.de/sneak/secret/internal/vault 0.565s
```
## Test Output (full vault package)
```
=== RUN TestVaultOperations
--- PASS: TestVaultOperations (0.37s)
=== RUN TestListUnlockers_SkipsMissingMetadata
--- PASS: TestListUnlockers_SkipsMissingMetadata (0.38s)
PASS
ok git.eeqj.de/sneak/secret/internal/vault 1.115s
```
## Linting
`golangci-lint run ./internal/vault/` reports **0 issues**.
> **Note:** `go test ./internal/cli/` fails pre-existing due to VCS stamping (`-buildvcs=false` needed); unrelated to this change.
sneak
was assigned by clawbot2026-02-15 23:05:22 +01:00
When an unlocker directory exists but is missing unlocker-metadata.json,
log a debug warning and skip it instead of returning a hard error that
crashes the entire 'unlocker ls' command.
Closes#1
Is silently skipping the right approach? Yes — secret.Debug() logs the skip when debug output is enabled, which is the right level. A missing metadata file is a recoverable situation; crashing unlocker ls over it is disproportionate.
Could this hide real corruption? Low risk. A missing metadata file most likely indicates incomplete creation or manual deletion, not vault corruption. Keys and encrypted secrets live elsewhere. Deeper corruption would surface through other operations.
Test quality: Good. Creates a real vault with a valid unlocker, adds a bogus directory without metadata, verifies ListUnlockers succeeds and returns only valid entries.
Code style: Clean. Debug message includes directory name for diagnostics.
No issues found. Ship it.
## Code Review: MERGE-READY ✅
The change is correct and well-scoped.
### Analysis
**Is silently skipping the right approach?** Yes — `secret.Debug()` logs the skip when debug output is enabled, which is the right level. A missing metadata file is a recoverable situation; crashing `unlocker ls` over it is disproportionate.
**Could this hide real corruption?** Low risk. A missing metadata file most likely indicates incomplete creation or manual deletion, not vault corruption. Keys and encrypted secrets live elsewhere. Deeper corruption would surface through other operations.
**Test quality:** Good. Creates a real vault with a valid unlocker, adds a bogus directory without metadata, verifies `ListUnlockers` succeeds and returns only valid entries.
**Code style:** Clean. Debug message includes directory name for diagnostics.
No issues found. Ship it.
Graceful degradation — instead of hard-failing when an unlocker directory is missing its unlocker-metadata.json, ListUnlockers() now logs a debug message and skips it. This is the right behavior: orphaned directories shouldn't crash the entire listing.
Test creates a real vault with a valid unlocker plus a bogus directory, and verifies listing succeeds with only the valid unlocker returned.
Clean, minimal change.
## Code Review
**Verdict: LGTM ✅**
Graceful degradation — instead of hard-failing when an unlocker directory is missing its `unlocker-metadata.json`, `ListUnlockers()` now logs a debug message and skips it. This is the right behavior: orphaned directories shouldn't crash the entire listing.
Test creates a real vault with a valid unlocker plus a bogus directory, and verifies listing succeeds with only the valid unlocker returned.
Clean, minimal change.
Fixed per review feedback:
- Added `secret.Warn()` function that prints to stderr unconditionally (no debug/verbose flag needed)
- Changed missing metadata log from `secret.Debug()` to `secret.Warn()` so users always see: `WARNING: Skipping unlocker directory with missing metadata file directory=<name>`
- `make check` passes clean (0 lint issues, all tests pass)
- Rebased on main
Output format: `WARNING: Skipping unlocker directory with missing metadata file directory=bogus-unlocker`
please make an issue identifying any other places we should be warning the user about anomalous conditions. then make a PR adding calls to our new warn function in those places.
please make an issue identifying any other places we should be warning the user about anomalous conditions. then make a PR adding calls to our new warn function in those places.
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.
clawbot
Summary
When an unlocker directory exists but is missing
unlocker-metadata.json,ListUnlockers()previously returned a hard error, crashing the entireunlocker lscommand. This change logs a debug warning and skips the directory instead.Changes
internal/vault/unlockers.go: Replacereturn nil, fmt.Errorf(...)withsecret.Debug()+continueinternal/vault/vault_test.go: AddTestListUnlockers_SkipsMissingMetadataTest Output (new test only)
Test Output (full vault package)
Linting
golangci-lint run ./internal/vault/reports 0 issues.Code Review: MERGE-READY ✅
The change is correct and well-scoped.
Analysis
Is silently skipping the right approach? Yes —
secret.Debug()logs the skip when debug output is enabled, which is the right level. A missing metadata file is a recoverable situation; crashingunlocker lsover it is disproportionate.Could this hide real corruption? Low risk. A missing metadata file most likely indicates incomplete creation or manual deletion, not vault corruption. Keys and encrypted secrets live elsewhere. Deeper corruption would surface through other operations.
Test quality: Good. Creates a real vault with a valid unlocker, adds a bogus directory without metadata, verifies
ListUnlockerssucceeds and returns only valid entries.Code style: Clean. Debug message includes directory name for diagnostics.
No issues found. Ship it.
Code Review
Verdict: LGTM ✅
Graceful degradation — instead of hard-failing when an unlocker directory is missing its
unlocker-metadata.json,ListUnlockers()now logs a debug message and skips it. This is the right behavior: orphaned directories shouldn't crash the entire listing.Test creates a real vault with a valid unlocker plus a bogus directory, and verifies listing succeeds with only the valid unlocker returned.
Clean, minimal change.
✅
make checkpasses cleanly (0 lint issues, all tests pass). Rebased on main. Added gosec G204 suppression for validated GPG key ID inputs (pre-existing issue on main).i think this should be a warning that issues even without —verbose.
412514bc90toc0f221b1caFixed per review feedback:
secret.Warn()function that prints to stderr unconditionally (no debug/verbose flag needed)secret.Debug()tosecret.Warn()so users always see:WARNING: Skipping unlocker directory with missing metadata file directory=<name>make checkpasses clean (0 lint issues, all tests pass)Output format:
WARNING: Skipping unlocker directory with missing metadata file directory=bogus-unlockerplease make an issue identifying any other places we should be warning the user about anomalous conditions. then make a PR adding calls to our new warn function in those places.