CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR touches only internal library functions (extractTopLevelRoot in layout.ts and its call site in doctor.ts). No commands, renderers, output formatters, help text, error messages, or JSON output shapes were changed. The fix silently corrects false-positive orphan reports for users with nested scoped extensions (e.g. @swamp/aws/iam), which is a transparent UX improvement — users simply stop seeing spurious warnings they should never have seen.

Code Review

No blocking issues found. This is a well-scoped, correct bug fix.

Summary

The fix replaces a hardcoded 2-segment assumption in extractTopLevelRoot (which assumed scoped names are always @scope/name) with using the actual extensionName segment count. This correctly handles 3+ segment nested scoped extensions like @swamp/aws/iam and @swamp/aws/s3, which previously resolved to the same root and cross-attributed each other's files as false orphans.

What I verified:

• extractTopLevelRoot is only called from doctor.ts:254 (plus tests) — the single call site is correctly updated to pass extensionName from the lockfile iteration
• The function is internal to libswamp (not exported via mod.ts), so the signature change has no public API impact
• New unit tests cover the 3-segment case directly, and a new integration-style test verifies sibling nested extensions don't cross-attribute orphans
• All existing tests are updated with the new parameter
• License headers present on all modified files
• The .catch(() => {}) cleanup pattern is correctly used in the new temp-dir test
• DDD: the change stays within the extensions bounded context; extractTopLevelRoot remains a pure stateless function — good domain design
• No any types, no fire-and-forget promises, no libswamp import boundary violations

Suggestions

1. The extensionName parameter is trusted to match the file path structure (i.e., the extension name segments should be a prefix of the path under pulled-extensions/). This is fine since the only caller iterates the lockfile map where keys and file paths are consistent by construction, but the implicit coupling is worth being aware of if more callers are added later.

Adversarial Review

Critical / High

None.

Medium

None.

Low

1. layout.ts:215 — Dropped empty-segment guard for bare-prefix file paths.
   The old code had if (segments.length === 0 || segments[0].length === 0) return null; which returned null when filePath was exactly .swamp/pulled-extensions/ (empty rest). The new code's guard segments.length < nameSegments.length does not catch this: "".split("/") yields [""] (length 1), and any valid extensionName also has ≥1 segments, so 1 < 1 is false, and the function returns .swamp/pulled-extensions/ instead of null. Not reachable in practice — lockfile files[] entries are always full file paths, never bare directory prefixes — but the semantic change exists. No action needed.

Verdict

PASS. The fix is correct and well-targeted. extensionName is always sourced from the same lockfile entry as the file paths (detectOrphanFiles iterates Object.entries(upstreamMap) and passes both from the same entry), so the trust placed on name/path consistency is justified. The new segment-count approach correctly generalises from the old hardcoded 2-segment assumption to N-segment scoped names. Tests cover the new 3-segment case, the existing 2-segment case, and the unscoped 1-segment case. Only caller updated, no public API surface affected (extractTopLevelRoot is not exported from mod.ts).