Bundle ops + atomic SetBundle (credstore §1.5.1/§2.1)

[rianjs]

Tracks Jira INT-431 (child of epic INT-310). Third discrete unit of cli-common; builds on #3 (Store + memory backend, merged in #4).

Scope

• ListBundle(profile) ([]string, error) — sorted keys under a profile (config show, pre-write, config clear, migration conflict detection). Not allowlist-gated.
• DeleteBundle(profile) ([]string, error) — removes every key under the profile; idempotent (empty → (nil, nil)). Not allowlist-gated (§1.7 clears all).
• SetBundle(profile, kv, ...SetOpt) (Result, error) per §1.5.1: validate-all → (no-overwrite: fail on any existing, name conflicts, nothing written) / (overwrite: snapshot existing) → write-all → on mid-bundle failure restore-from-snapshot (pre-existing) or delete (new) → zero snapshot before return → Result{Written,Restored,Deleted}; rollback failures surfaced explicitly.
• Extend internal backend interface with key enumeration; deterministic sorted iteration + Result ordering.
• §2.1 test seam: unexported memoryBackend failure-injection hook (same-package tests only) to drive mid-bundle rollback paths.
• Tests: list/delete (incl. empty/idempotent), no-overwrite conflict, overwrite snapshot+restore on induced failure, new-key delete-rollback, snapshot clearing, rollback-failure surfacing, allowlist on SetBundle only.

Out of scope (later INT-310 units)

Real OS backends + selection (PR4), Linux fail-closed (PR5), redaction (PR6), migration helpers (PR7), config show/clear surfaces, v0.1.0.

[rianjs]

INT-431 — cli-common: bundle ops + atomic SetBundle (credstore §1.5.1/§2.1)

Epic: INT-310. GitHub: #5. Builds on INT-430
(Store + in-memory backend, merged in PR #4). Standard: §2.1 (bundle API),
§1.5.1 (SetBundle atomicity contract), §1.5.2 (allowlist scope), §1.7
(config clear removes the whole bundle), §1.3 (Item.Key mapping).

Goal

The three bundle operations that later units depend on:
ListBundle (config show / pre-write / migration conflict detection),
DeleteBundle (config clear), and the atomic-ish SetBundle (init
multi-key write). Extends the internal backend interface with key
enumeration — the extension INT-430's review anticipated. No OS-touching
code; in-memory backend only.

One PR off feat/INT-431-bundle-ops. Files: extend credstore/store.go
and credstore/memory.go; new credstore/bundle_test.go.

Backend interface extension

Add one method (the only reshaping; per the INT-430 Codex note that
"bundle work may extend the interface"):

listKeys() ([]string, error) // all full Item.Keys "<profile>/<key>"; ErrStoreClosed if closed

memory: snapshot map keys under b.mu; nil map → ErrStoreClosed.

Internal helper splitItemKey(itemKey) (profile, key string) — inverse of
joinItemKey; exactly one / by construction (segments are
validSegment), so strings.Cut.

API

type Result struct {
    Written   []string // keys written this call (bare, sorted)
    Restored  []string // pre-existing keys restored to prior value on rollback
    Deleted   []string // newly-written keys removed on rollback
    Untouched []string // target keys never written (after the failure point)
}

func (s *Store) ListBundle(profile string) ([]string, error)
func (s *Store) DeleteBundle(profile string) ([]string, error)
func (s *Store) SetBundle(profile string, kv map[string]string, opts ...SetOpt) (Result, error)

Behavior

All three: take s.mu, return ErrStoreClosed if closed (consistent
with INT-430; s.be is nil post-close so the guard must precede backend
use), validate profile syntax via validSegment →
*RefError{Segment:"profile"}.

• ListBundle: listKeys(), keep those with prefix profile+"/",
  strip prefix, sort, return. Empty profile → (nil, nil). Not
  allowlist-gated — it reports stored reality (config show / migration
  need every key, §1.5.2 gates writes/deletes only, not reads).
• DeleteBundle: list as above, delete each (sorted), collect bare
  keys deleted. Empty → (nil, nil) (idempotent, §1.7.2). Not
  allowlist-gated — config clear removes every key in the bundle
  regardless of the current allowlist (§1.7). On a mid-way delete
  error: return the keys deleted so far + an error naming the failed key
  (delete is best-effort for config clear; not transactional).
• SetBundle (the §1.5.1 contract):
  1. len(kv)==0 → (Result{}, nil) (nothing to do).
  2. Validate all before any write: every key validSegment +
     checkAllowed (allowlist is enforced here — §1.5.2 lists
     SetBundle). First failure (sorted order, deterministic) → (Result{}, err), nothing written.
  3. Sorted target order; build Item.Keys.
  4. Pre-write state: for each target, exists.
     • no WithOverwrite: if any exist → (Result{}, err) where err
       wraps ErrExists and names all conflicting keys; nothing written.
     • WithOverwrite: snapshot get of every currently-existing
       target into an in-memory map (prior values) — before any write, so
       rollback restores values, not just deletes (§1.5.1).
  5. Write in sorted order: set(itemKey, value, true).
  6. On set failure at key K: roll back the keys already written —
     snapshot member → set(restore prior); new key → delete. Then
     best-effort clear+drop the snapshot. Result{Written:nil, Restored:…, Deleted:…, Untouched: K plus the not-yet-attempted}.
     Error wraps the underlying failure, names K. Any rollback step
     that itself fails is surfaced: the error explicitly states the
     keyring may be inconsistent and lists the affected keys (§1.5.1).
  7. Success: Result{Written: all (sorted)}, others nil; best-effort
     clear+drop snapshot; (Result, nil).

Snapshot is call-scoped; cleared (best-effort string overwrite, then
dropped) before return. As in INT-430, the doc says best-effort — no
false memory-zeroization claim for Go strings.

§2.1 test seam

Unexported memoryBackend.setHook func(itemKey string) error. set
consults it (if non-nil) before writing and returns its error. Set only
by same-package tests to drive mid-bundle rollback paths. ~3 LOC in
production memory.go; this is the §2.1-sanctioned test seam ("exposed as
test seams so each CLI's … tests can drive both paths"), not speculative.

Tests (bundle_test.go, table-driven, -race)

• ListBundle: empty → (nil,nil); multiple keys sorted; only the
  queried profile (no cross-profile bleed); invalid profile → *RefError;
  after Close → ErrStoreClosed; disallowed-but-stored key still listed
  (not gated).
• DeleteBundle: empty idempotent; removes all under profile, leaves
  other profiles; returns sorted deleted keys; not allowlist-gated;
  closed → ErrStoreClosed.
• SetBundle happy path: multi-key write, Result.Written sorted; keys
  then Get-able.
• No-overwrite conflict: one pre-existing target → error errors.Is ErrExists, names the key, nothing written (other targets absent
  afterward).
• Overwrite: pre-existing values replaced; Result.Written all.
• Induced mid-bundle failure via setHook:
  • new keys only → all written-so-far deleted, Result.Deleted set,
    Written nil, error names failing key, store has none of them.
  • mix of pre-existing (overwrite) + new → pre-existing restored to
    prior value, new ones deleted; Result.Restored/Deleted correct.
  • rollback-failure surfacing: setHook fails the write and a
    restore; error explicitly flags inconsistent state + affected keys.
• Allowlist: SetBundle with a disallowed key → errors.Is ErrKeyNotAllowed, nothing written; ListBundle/DeleteBundle not
  gated.
• Snapshot hygiene: after a rolled-back overwrite, a fresh Get returns
  the original value (proves snapshot captured pre-write state).

Verification

go build ./..., go test -race ./..., golangci-lint run, gofmt -l
(empty), tidy gate — all clean locally and in CI (still stdlib-only).
Each test asserts a concrete sentinel/value/Result, not "works".

Out of scope (later INT-310 units)

Real OS backends + auto/env/config selection (PR4), Linux fail-closed
(PR5), redaction helpers (PR6), migration helpers (PR7), config
show/clear command surfaces, v0.1.0 tag.