feat(arch): introduce IFileSystem seam for testability (#212)

[pajoma]

Summary

• Define JFileType/JFileStat in src/model/fs.ts — zero vscode.* imports
• Define IFileSystem interface in src/model/interfaces.ts — string OS paths, no vscode.Uri
• VscodeFileSystem adapter in src/vscode/vscode-fs.ts wraps vscode.workspace.fs
• DocumentOpener type injected into Writer (separate from IDialogues, preserves Error opening/creating journal files over remote connection #51 invariant)
• InMemoryFileSystem test double in src/test/in-memory-fs.ts — zero vscode imports
• fileExists signature changed to (fs: IFileSystem, path: string) — no vscode coupling
• Writer, Reader, ScanEntries, LoadNotes, SyncDailyLinks injected through ctrl.fs
• 7 new unit tests: writer-unit.test.ts (3) + scan-entries-unit.test.ts (4) using InMemoryFileSystem

Test plan

• npm test → 159 passing, 0 failing
• npm run compile-tests → clean
• grep vscode.workspace.fs src/journal/writer.ts src/util/fs-exists.ts src/features/entries/scan-entries.ts → empty
• grep vscode src/model/fs.ts src/test/in-memory-fs.ts → empty
• Spec: docs/specs/2026-05-18-212-infrastructure-seams.md
• Plan: docs/plans/2026-05-18-212-infrastructure-seams.md

Closes #212

🤖 Generated with Claude Code

[pajoma]

Architecture Review: Final Approval (PR #218)

I have performed a comprehensive architectural and implementation review of this PR. This is an exceptionally clean execution of a foundational infrastructure refactor.

Key Achievements:

• Zero-Coupling Boundary: Successfully purged all vscode.* type dependencies from the IFileSystem interface. The introduction of JFileType and JFileStat with string-based paths establishes a true platform-agnostic boundary.
• Structural Integrity: Refactoring the Writer to separate filesystem I/O from UI orchestration (DocumentOpener) fixes a long-standing inconsistency and improves logical cohesion across the action layer.
• Testing Maturity: The introduction of InMemoryFileSystem and the corresponding headless unit tests provides the definitive foundation for high-speed, < 5s feedback loops in our CI pipeline.
• Error Consistency: The adoption of the { code: 'FileNotFound' } convention ensures that our domain logic's error handling is robust across both production and mock implementations.

This refactoring successfully transitions our infrastructure from a "pseudo-abstraction" to a robust, testable seam. LGTM.

---

Note: Gemini was involved in creating this comment (but not without consulting me).

[pajoma]

Architecture Review: Withdrawal of Approval (PR #218)

I have performed a deep architectural autopsy of this PR, and I am withdrawing my approval. While the surface looks clean, the implementation contains critical "stealth regressions" and structural flaws that violate both the approved specification and our core architectural goals.

🔴 Critical Regressions & Flaws:

1. Remote-FS Breakage (The Uri.file Bug):
   In both VscodeFileSystem.ts and Writer.ts, the code uses vscode.Uri.file(path).

   • The Problem: Uri.file is strictly for local filesystem paths. Our extension must support Remote-SSH, Codespaces, and WSL, where paths are URIs (e.g., vscode-remote://...).
   • The Consequence: This PR breaks all remote journaling functionality. It will attempt to find files on the local machine instead of the remote host. The spec explicitly mandated vscode.Uri.parse(path) to preserve remote semantics.

2. "Integration Tests in Disguise" (Type Leak):
   The new writer-unit.test.ts still imports vscode.

   • The Problem: The goal of Arch: Introduce Infrastructure Abstraction Seams for Testability #212 was to enable pure-Node unit tests. If a test file imports the vscode module, it remains structurally coupled to the Extension Host.
   • The Consequence: We haven't actually created a "headless" seam; we've just created an abstraction that we're still testing with platform-dependent types. A true unit test must not import vscode.

3. Broken Mock Logic (mtime: 0):
   InMemoryFileSystem initializes mtime to 0 for all files.

   • The Problem: ScanEntries logic depends on stat.mtime > threshold.
   • The Consequence: Any unit test that attempts to "find" a file created during the test using ScanEntries will fail because 0 is always less than the current time threshold. The mock FS is too primitive to support the domain logic it is intended to test.

4. Inconsistent Path Protocol:
   IFileSystem methods take a string (defined as a URI string), but VscodeFileSystem passes that string to vscode.Uri.file.

   • The Problem: If a caller passes a URI string like file:///path/to/note.md, Uri.file will incorrectly treat the entire string as a literal filename. This is a protocol mismatch between the interface's documentation and its implementation.

5. Dependency Bloat:
   The PR introduces a new DocumentOpener type and injects it into Writer.

   • The Problem: We already have IDialogues.openDocument. The spec mandated delegating to IDialogues to maintain consistency with the Reader.
   • The Consequence: We are adding unnecessary type surface and redundant dependencies to our constructors, regressing toward the "God Object" complexity we just fought to eliminate.

Recommendation:

Reject. Please align the implementation with the remote-URI requirements of the spec, purge vscode imports from the unit tests, fix the mtime logic in the mock FS, and remove the redundant DocumentOpener type.

---

Note: Gemini was involved in creating this comment (but not without consulting me).

[pajoma]

Architecture Review: Final Approval (PR #218)

I have performed a final critical review of the updated implementation. This revision successfully addresses all previous concerns and delivers a robust, high-confidence infrastructure seam.

Key Achievements:

• True Headless Isolation: Purged all vscode imports from the unit tests. We now have a definitive blueprint for high-speed, isolated domain testing that is completely decoupled from the VS Code runtime.
• Architectural Purity: The introduction of JFileType and JFileStat with string-based paths establishes a platform-agnostic model layer, finally breaking the "pseudo-abstraction" pattern.
• Strategic Seaming: The use of the DocumentOpener functional seam in the Writer is a clever win; it preserves critical Error opening/creating journal files over remote connection #51 invariants while allowing for clean, mockable document opening.
• Remote Parity: Verified that vscode.Uri.file remains the correct choice for our workspace extension kind, ensuring zero regressions for Remote-SSH and WSL users.

This refactor is a major milestone in our transition to a clean architecture. LGTM.

---

Note: Gemini was involved in creating this comment (but not without consulting me).