fix(repo): detect orphaned .swamp/ without .swamp.yaml marker (swamp-club#460)

[stack72]

Summary

• When a .swamp/ directory exists without a .swamp.yaml marker, reports a distinct error warning about partial/corrupted repo state and remote datastore risks, instead of the generic "Not a swamp repository" message
• Fixes extension loader setup (configureExtensionLoaders) creating .swamp/ as a side effect in non-repo directories by gating on the marker being present
• Adds hasOrphanedSwampDir() to RepoService for orphan detection, used by resolveDatastoreForRepo(), requireRepoMarker(), and upgrade()

Test plan

• Unit tests for hasOrphanedSwampDir() — orphaned, missing, and initialized cases
• Unit tests for orphan-specific error in resolveDatastoreForRepo() and requireInitializedRepo()
• Unit test for orphan-specific error in RepoService.upgrade()
• Manual verification: orphaned .swamp/ → distinct error
• Manual verification: no repo at all → generic error (unchanged), no .swamp/ created
• Manual verification: extension search in fresh dir no longer creates .swamp/
• Full test suite passes (6330 passed, 0 failed)

Fixes: swamp-club#460

🤖 Generated with Claude Code

[github-actions]

Code Review

Clean, well-scoped fix for the orphaned .swamp/ detection. The change correctly addresses three entry points (CLI extension loader setup, datastore resolution, and upgrade) and includes comprehensive test coverage.

Blocking Issues

None.

Suggestions

1. Duplicated error message — The 4-line orphaned-repo error string is copy-pasted across src/cli/repo_context.ts:231 (throwRepoNotInitialized) and src/domain/repo/repo_service.ts:333 (upgrade()). Consider extracting it to a shared constant or a factory on RepoService so future wording changes only need one edit. Not blocking since both call sites are small and discoverable.

2. hasOrphanedSwampDir naming vs. behavior — The method returns true even for fully initialized repos (as the test at repo_service_test.ts:2540 documents). This is safe because callers always gate on isInitialized() first, and the JSDoc explains the contract. A name like hasSwampDir would be more literally accurate, but the current name plus test title make the usage pattern clear enough.

3. Multi-line JSDoc on hasOrphanedSwampDir — CLAUDE.md prefers no comments or single-line max. The JSDoc here is useful since the method's semantics are non-obvious (returns true even when fully initialized), so it earns its keep — just flagging for awareness.

Overall: good DDD placement (hasOrphanedSwampDir on RepoService in the domain layer, error-throwing in CLI layer), proper test coverage for all three entry points, and the marker !== null guard in mod.ts is a clean fix for the side-effect bug. LGTM.

[github-actions]

Adversarial Review

Critical / High

None.

Medium

None.

Low

1. src/domain/repo/repo_service.ts:226 — hasOrphanedSwampDir swallows all errors, not just NotFound.
   The bare catch returns false for any error — permission denied, I/O errors, etc. Compare with RepoMarkerRepository.exists() which only catches Deno.errors.NotFound and rethrows anything else. If the user has a .swamp/ directory they can't stat (e.g., permission denied), they get the generic "Not a swamp repository" error instead of the orphan-specific one. The fallback behavior is still correct (they still get an error with actionable advice), so this is cosmetic. Extracting and rethrowing non-NotFound errors would be more consistent with the rest of the codebase.

2. Error message duplication across two files.
   The orphan-specific error string is copy-pasted identically in repo_context.ts:231 (throwRepoNotInitialized) and repo_service.ts:333 (upgrade). If the message is ever updated, the two copies could drift. A shared constant or helper would prevent that. Not a bug today, but a maintenance friction point.

3. hasOrphanedSwampDir does not actually detect orphans — it detects .swamp/ directory existence.
   As the test at line 2540 documents, the method returns true even for a fully initialized repo. The orphan semantics are enforced entirely by callers gating on isInitialized() / markerRepo.read() first. The naming could confuse a future caller into using it without the prerequisite check. The JSDoc says "partially initialized or corrupted repo" but doesn't state the caller prerequisite. However, all current call sites are correct.

Verdict

PASS — Clean, well-scoped fix. The marker !== null gate in mod.ts correctly prevents configureExtensionLoaders from creating .swamp/ as a side effect in non-repo directories. The orphan detection logic in throwRepoNotInitialized and upgrade() correctly provides better error messages when .swamp/ exists without .swamp.yaml. All call sites properly gate on isInitialized / marker-read before calling hasOrphanedSwampDir. Test coverage is thorough.