Repository navigation
Fix windows prune - #48
Merged
Merged
Conversation
On Windows, mdl import erwin reported 'imported 52 entities' but left the model dir EMPTY — a silent data-loss bug affecting every importer (erwin, reverse, OSI all go through write_model). Root cause: _prune_stale deletes .yaml files under the owned dirs that this write did not produce, comparing each on-disk path against the set. It built the comparison key with str(path.relative_to(root)), which stringifies with the OS separator — BACKSLASH on Windows — while holds forward-slash keys from dump()'s f-strings. So on Windows every just-written file (logical\entities\e.yaml) failed the check (logical/entities/e.yaml) and was immediately unlinked, then the emptied dirs were rmdir'd. macOS/Linux use '/' on both sides, so it never triggered — which is why 939 tests and the Linux Docker check missed it. Fix: compare with as_posix() so the separator is normalised on every OS, and keep dump()'s recorded rel POSIX-style too. Regression test simulates the Windows separator directly (PureWindowsPath), so this class is caught on any OS. A real windows-latest CI job is a follow-up. CLI 0.6.12.
A full audit after the import data-loss bug found the whole class of POSIX-only assumptions that break on Windows. This fixes them. CRITICAL (data loss / unusable on Windows): - core/state.py: the SAME prune-separator bug as the writer. save() compared str(relative_to()) (backslash) against forward-slash _shard_rel keys, so every shard was deleted on every save, wiping the 3-way-merge base. Fixed with as_posix(). - core/commands.py dir_fingerprint: summed raw byte st_size, so a CRLF checkout differed from the LF size the client held, firing StaleModelError on every edit (canvas/LSP read-only on Windows). Now hashes newline-normalised content. Plus cli/collab.py's generated .gitattributes pins eol=lf on model YAML, .mdl/lock.yaml, decisions, state. - emit-dbt/emitter.py: two entities mapping to the same dbt model name (or case-only differing, the same file on a case-insensitive FS) silently overwrote each other's .sql and broke ref(). Now raises loudly. HIGH: - One shared hardened filename slug (new core/fsnames.py) reused by writer and commands: strips Windows-illegal chars, trailing dot/space, reserved device names (CON/NUL/COMn/LPTn), with case-fold collision dedup. Replaces three weaker _slug copies. - ~9 subprocess(text=True) calls (git/gh/dbt) now pass encoding=utf-8, errors=replace: Windows decoded output as cp1252, garbling non-ASCII branches, PR titles, git identity. - vscode/driftDiagnostics.ts: document.fileName (backslash) vs the CLI report's POSIX i.file with endsWith made drift quick-fixes silently vanish on Windows. Normalised. LOW: encoding on the remaining plain write_text; chatParticipant/extension use path.join / path.sep. Tests: fsnames, state prune, CRLF-insensitive fingerprint, dbt name collision. 942 pytest, ruff clean, all extension gates, Docker clean-install green. Windows CI is a deferred follow-up (the whole class is invisible to a POSIX-only test matrix).
dbose
added a commit
that referenced
this pull request
Sep 28, 2026
PR #48 fixed driftDiagnostics.ts (drift quick-fixes silently vanished on Windows because document.fileName's backslash path was compared against the CLI report's POSIX path), plus path.join/path.sep in chatParticipant and extension, but left the extension version at 0.3.16. Bump so the shipped fix carries a version.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix critical Windows data-loss bugs and harden the codebase for portability
Summary
mdl import erwinreported "imported 52 entities" on Windows but left the model directory empty — a silent data-loss bug. Investigating it revealed a whole class of POSIX-only assumptions that break on Windows. This PR fixes the originating bug and then does a full audit-driven hardening pass across Python and TypeScript.None of these were caught by the 942-test suite or the Docker check — both run on POSIX (forward-slash) filesystems, so the entire bug class is invisible to them. New regression tests simulate the Windows path separator (
PureWindowsPath) so they catch it on any OS.CRITICAL — data loss / editing unusable on Windows
mdl_reverse.writer._prune_stale)._prune_stalecompared each on-disk path built withstr(relative_to())(backslash on Windows) against a forward-slash "just wrote" set. On Windows nothing matched, so every freshly-written entity was pruned as "stale" and the emptied dirs removed. The summary line prints the in-memory count, masking it. Affects all importers (erwin, reverse, OSI).mdl_core.state.py—GenerationState.save()deleted the entire.mdl/statestore (the 3-way-merge base) on every save on Windows, silently corrupting generation state.dir_fingerprintsummed raw byte size; a Git-for-Windows (autocrlf) CRLF checkout differs from the LF size the client holds, firingStaleModelError/ HTTP 409 on every edit. Now hashes newline-normalised content, and the generated.gitattributespinseol=lfon tool-owned files.emit-dbt). Two entities mapping to the same name (or differing only in case — the same file on a case-insensitive FS) clobbered one another's.sqland brokeref(). Now raises loudly.HIGH — wrong behavior on Windows
mdl_core.fsnames): strips Windows-illegal chars, trailing dot/space, and reserved device names (CON/PRN/NUL/COM1-9/LPT1-9), with case-fold collision dedup. Replaces three weaker_slugcopies; the command engine's could inject a path separator or crashopen()on a user-typed name.subprocess(text=True)calls (git/gh/dbt) now passencoding="utf-8", errors="replace"— Windows decoded them as cp1252, garbling a non-ASCII git identity (e.g. "José" → wrong branch names and audit attribution), PR titles, and dbt output.driftDiagnostics.tscompareddocument.fileName(backslash) against the CLI report's POSIX path withendsWith, so drift quick-fixes silently disappeared on Windows. Normalised to forward slashes.LOW
Added
encodingto remaining plainwrite_textcalls;chatParticipant/extensionusepath.join/path.sep.Tests
New:
test_writer_prune_windows,test_state_prune_windows,test_fingerprint_newlines(LF/CRLF produce identical fingerprints),test_fsnames(illegal chars, reserved names, case dedup),test_model_name_collision. 942 pytest passed, ruff clean, all 6 extension gates green, Docker clean-install green.Notes
encoding="utf-8", subprocess uses argv lists (noshell=True), temp/home/env viatempfile/Path.home(),_force_utf8_outputfor stdout, and the extension'sresolve.ts(findsmdl.exeon locked-down Windows).windows-latestCI job is the durable guard against this class (deferred). ThePureWindowsPathsimulation tests are the interim.CLI
modelith-dbtbump to 0.6.12.