feat(vaults): annotation UX improvements

[stack72]

Summary

Addresses five UX improvements for vault annotations identified in swamp-club#418:

• --note → --notes: Rename flag to match the data model field name (notes)
• Annotation in JSON output: vault annotate --json now includes the resulting annotation state, eliminating the need for a follow-up vault inspect call
• --clear validation: Error when --clear is combined with --url/--notes/--label/--remove-label (previously silently discarded)
• --remove-label <key>: New repeatable flag for removing individual labels without clearing all annotations
• secretExists optimization: Replace svc.get() (full AES-GCM decryption) with svc.list() membership check in annotate, inspect, and put deps

Closes swamp-club#418

Test plan

• deno check — type checking passes
• deno lint — no lint errors
• deno fmt --check — formatting correct
• deno run test — all 6169 tests pass (37 vault-specific tests including 7 new ones)
• deno run compile — binary compiles successfully
• npx tessl skill review — swamp-vault skill passes at 94%

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. No --remove-label example in --help (src/cli/commands/vault_annotate.ts): The command has .example() calls for URL+notes, labels, and --clear, but nothing for the new --remove-label flag. A new flag without a help example is harder to discover. Suggested addition:

   .example("Remove a label", "swamp vault annotate my-vault API_KEY --remove-label team")
   

2. Description body doesn't mention --remove-label (src/cli/commands/vault_annotate.ts, line 70): The description says "Use --clear to remove all annotations" but doesn't mention that individual labels can now be removed with --remove-label. A user skimming the description might miss this. Suggested addition to the description: "Use --remove-label to remove individual labels, or --clear to remove all annotations."

3. Log-mode output doesn't name which labels were removed: When --remove-label team is used, the output is Annotated API_KEY in vault my-vault (fields: labels). This is consistent with how other field updates are reported, but it doesn't distinguish between "added a label" and "removed label 'team'". Minor—the current behavior is internally consistent, so only worth changing if the other field updates also get more specific.

4. --note → --notes is a breaking change: Existing user scripts that pass --note will silently fail (Cliffy will likely error on unknown flag). The rename is correct—it aligns with the data model—but a note in the release changelog would help users who hit this. Not a CLI change needed, just a release comms callout.

Verdict

PASS — no blocking issues. The five UX improvements (flag rename, JSON annotation output, --clear validation, --remove-label, secretExists optimization) are all clean and correct. Error messages are clear and actionable. JSON output now includes the full annotation state, which eliminates the follow-up vault inspect call.

[github-actions]

Code Review

Clean, well-tested set of annotation UX improvements. DDD patterns are correctly applied throughout.

Blocking Issues

None.

Suggestions

1. Re-export VaultAnnotationData from src/libswamp/mod.ts: The new annotation: VaultAnnotationData | null field on the public VaultAnnotateData interface references a domain type that isn't re-exported through the libswamp barrel. Consumers that need to reference this type by name (e.g., for typed variables or function signatures) would have to import from the domain layer directly, violating the import boundary. TypeScript structural typing makes this work today, but an explicit re-export would be more robust.

2. secretExists via svc.list() + .includes(): The switch from svc.get() (AES-GCM decryption) to svc.list() (key enumeration) is a good improvement. For very large vaults, a dedicated svc.has(vaultName, key) method could avoid allocating the full key array — but that's a future optimization, not something this PR needs to address.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. Missing --clear mutual exclusivity validation in libswamp layer (src/libswamp/vaults/annotate.ts:203-217): The CLI layer (vault_annotate.ts:123-131) correctly validates that --clear cannot be combined with --url/--notes/--label/--remove-label, but the vaultAnnotate generator function has no such check. If input.clear is true alongside url: "foo", the generator silently enters the if (input.clear) branch at line 217 and ignores the other fields. Any future caller of the libswamp API that bypasses the CLI would not get the validation error. This is a defense-in-depth concern — the CLI catches it today, but the library should enforce its own invariants.

2. removeLabels on nonexistent annotation persists a vacuous annotation (src/libswamp/vaults/annotate.ts:251-268): If a user runs swamp vault annotate my-vault KEY --remove-label team on a secret that has no existing annotation, the code creates a new empty VaultAnnotation (via VaultAnnotation.create({})), applies removeLabels (a no-op on empty labels), and then persists the empty annotation via putAnnotation. The result is a saved annotation with only an updatedAt field — isEmpty() returns true but it occupies storage. Consider either skipping the write when the result is empty, or documenting that this is expected behavior.

Low

1. svc.list() performance characteristic change (annotate.ts:110, inspect.ts:77, put.ts:103): The old secretExists used svc.get() (single-key decrypt), replaced with svc.list() (fetch all key names). This avoids AES-GCM decryption per the stated goal, but trades it for O(n) key listing on backends with many secrets. For put.ts specifically, secretExists is called before every store operation — if a vault has thousands of keys, listing them all for a membership check could be slower than the old decrypt-one approach. Fine for current scale; worth keeping in mind if vault sizes grow.

2. --label X=Y --remove-label X ordering is implicit (annotate.ts:252-265): Labels are added via merge() first, then removeLabels is applied after. This means --label team=infra --remove-label team adds then removes — removal wins. The test at line 385 documents this behavior explicitly, which is good. The ordering is deterministic and reasonable, but could surprise users who expect the flags to conflict.

Verdict

PASS — Clean implementation with good test coverage (7 new tests covering the new paths). The new features are well-separated between domain, library, and CLI layers. The --clear validation gap and empty-annotation persistence are minor defense-in-depth concerns, not correctness bugs.