CLI UX Review

Blocking

None.

Suggestions

1. Duplicate log messages on successful update — When an update is pulled, users see two info-level messages back-to-back:

   • datastore_auto_update.ts:120: "Updating @swamp/s3-datastore 2026.03.15.1 → 2026.03.30.1" (before the pull)
   • resolve_datastore.ts:158: "Updated @swamp/s3-datastore 2026.03.15.1 → 2026.03.30.1" (after the pull)

   Only one message is needed. The pre-pull "Updating ..." in datastore_auto_update.ts is redundant since the caller already logs the result. Suggest removing the logger.info at datastore_auto_update.ts:120-123 and keeping only the post-pull "Updated ..." in resolve_datastore.ts.

2. No user control over auto-update behavior — There's no flag to opt out (e.g., --no-auto-update) or force an immediate check. Not blocking for v1, but worth tracking as the feature matures.

Verdict

PASS — the auto-update runs silently in the background, failures are swallowed, and the only user-visible output is the update log message. The duplicate messaging is minor noise and doesn't block.

Adversarial Review

Critical / High

None found.

Medium

1. Cache written before pull — failed pull suppresses retry for 24h (src/libswamp/extensions/datastore_auto_update.ts:108-113)

   The cache is updated at line 109-113 before pullExtension is called at line 124. If pullExtension throws (network timeout, disk full, checksum mismatch), the outer catch at line 131 returns null, but the cache already records checkedAt: now with the latest version. For the next 24 hours, isExtensionCheckStale() returns false and the update is never retried.

   Breaking scenario: User runs a command while their network is flaky. The registry check succeeds (getting latestVersion), but the archive download in pullExtension fails. The extension stays at the old version, and no retry happens until the cache expires 24 hours later.

   Suggested fix: Move the cache write to after the successful pull (between lines 124 and 126). For the "already at latest" path (line 115-117), the cache write at its current position is fine — split it into two: write-on-up-to-date before the early return, and write-on-successful-pull after pullExtension resolves. This way a pull failure allows an immediate retry on the next command.

2. Import from internal libswamp path violates architecture rule (src/cli/resolve_datastore.ts:44)

   maybeAutoUpdateDatastoreExtension is imported from ../libswamp/extensions/datastore_auto_update.ts, but CLAUDE.md requires CLI code to import libswamp functions from src/libswamp/mod.ts. The function is already exported from mod.ts (this PR adds it). The import should be:

   import { maybeAutoUpdateDatastoreExtension } from "../libswamp/mod.ts";

Low

3. Variable shadowing of resolvedRepoDir (src/cli/resolve_datastore.ts:75 and src/cli/resolve_datastore.ts:103)

   resolvedRepoDir is declared with const at line 75 (resolve(repoDir)) in the outer function scope, then declared again at line 103 inside the pullExtension callback with the same resolve(repoDir) expression. The values are identical so behavior is correct, but the inner declaration is unnecessary and could mask a future bug if repoDir were ever reassigned. Use the outer resolvedRepoDir directly in the closure.

4. Non-atomic cache file writes (src/infrastructure/persistence/extension_update_check_repository.ts:48-51)

   Deno.writeTextFile is not atomic — a crash mid-write leaves a truncated file. The read() method's catch-all means this self-heals (returns {}), so worst case is one extra registry check. Existing pattern in the codebase (update_check_cache.ts) has the same characteristic, so this is consistent.

Verdict

PASS — The core logic is sound and well-tested. The cache-before-pull ordering (Medium #1) is the most actionable finding — it causes a 24h retry blackout on pull failure. The import path (Medium #2) is a straightforward fix. Neither blocks the merge but both are worth addressing.