From 356655d86b6018e81df55d2dd51a1a86ae586608 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:24:30 +1000 Subject: [PATCH 1/8] fix: preserve JSONC and add missing distro in WSL settings; propagate exit codes - Replace destructive json.dumps rewrite of Windows VS Code settings.json with a regex-based in-place patcher (_patch_jsonc_settings) that preserves comments and trailing commas. - Fix bug where dev.containers.executeInWSLDistro was never set when executeInWSL was already true. - Fall back to printing a hint instead of corrupting un-patchable settings files. - get_workspace_folder now reports a friendly error and uses the default workspace folder when devcontainer.json is unparseable. - run_dcode now propagates non-zero exit codes from the code/code-insiders launcher via sys.exit. - _wsl_to_windows_path checks subprocess returncode before using stdout. - Narrow except Exception to (OSError, ValueError) in WSL settings parse. --- src/dcode/cli.py | 150 ++++++++++++++++++++++++++++++++++++++-------- tests/test_cli.py | 140 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 250 insertions(+), 40 deletions(-) diff --git a/src/dcode/cli.py b/src/dcode/cli.py index 2bb59fa..e963c09 100644 --- a/src/dcode/cli.py +++ b/src/dcode/cli.py @@ -5,6 +5,7 @@ import argparse import json import os +import re import subprocess import sys from pathlib import Path @@ -14,10 +15,11 @@ def is_wsl() -> bool: """Detect if running inside WSL.""" + proc_version = Path("/proc/version") + if not proc_version.exists(): + return False try: - return os.path.exists("/proc/version") and "microsoft" in Path( - "/proc/version" - ).read_text().lower() + return "microsoft" in proc_version.read_text().lower() except OSError: return False @@ -103,9 +105,20 @@ def find_devcontainer(target: Path) -> Path | None: def get_workspace_folder(devcontainer_path: Path, target: Path) -> str: """Read workspaceFolder from devcontainer.json, or use the default.""" - with devcontainer_path.open() as f: - config = json5.load(f) - return config.get("workspaceFolder", f"/workspaces/{target.name}") + default = f"/workspaces/{target.name}" + try: + with devcontainer_path.open() as f: + config = json5.load(f) + except (OSError, ValueError) as exc: + print( + f"dcode: failed to parse {devcontainer_path} ({exc}); " + f"using default workspace folder {default}", + file=sys.stderr, + ) + return default + if not isinstance(config, dict): + return default + return config.get("workspaceFolder", default) def build_uri(host_path: str, workspace_folder: str) -> str: @@ -119,11 +132,12 @@ def _wsl_to_windows_path(linux_path: str) -> str: try: result = subprocess.run( ["wslpath", "-w", linux_path], - capture_output=True, text=True, timeout=5, + capture_output=True, text=True, timeout=5, check=False, ) - win_path = result.stdout.strip() - if win_path: - return win_path + if result.returncode == 0: + win_path = result.stdout.strip() + if win_path: + return win_path except (OSError, subprocess.TimeoutExpired): pass # Fallback: construct UNC path manually @@ -144,7 +158,7 @@ def _get_windows_vscode_settings_path(insiders: bool = False) -> Path | None: try: result = subprocess.run( ["cmd.exe", "/C", "echo", "%APPDATA%"], - capture_output=True, text=True, timeout=5, + capture_output=True, text=True, timeout=5, check=False, ) appdata_win = result.stdout.strip() if not appdata_win or "%" in appdata_win: @@ -152,7 +166,7 @@ def _get_windows_vscode_settings_path(insiders: bool = False) -> Path | None: # Convert Windows path to WSL path result = subprocess.run( ["wslpath", "-u", appdata_win], - capture_output=True, text=True, timeout=5, + capture_output=True, text=True, timeout=5, check=False, ) appdata_wsl = result.stdout.strip() if not appdata_wsl: @@ -163,6 +177,73 @@ def _get_windows_vscode_settings_path(insiders: bool = False) -> Path | None: return None +# Matches a top-level JSONC object: optional leading whitespace/comments, +# then '{', then arbitrary content, then a final '}' (greedy to last brace). +_TOP_LEVEL_OBJECT_RE = re.compile(r"^(\s*(?://[^\n]*\n|/\*.*?\*/|\s)*)\{(.*)\}(\s*)\Z", re.DOTALL) + + +def _format_jsonc_value(value: object) -> str: + """Format a Python value as a JSON literal suitable for embedding in JSONC.""" + return json.dumps(value) + + +def _patch_jsonc_settings(text: str, patches: dict) -> str | None: + """Apply ``patches`` to a JSONC ``settings.json`` text in-place. + + For each key in ``patches``: replace the value of an existing top-level + ``"key":`` entry, or insert a new entry just before the closing ``}``. + Preserves comments, trailing commas, and indentation. + + Returns the patched text, or ``None`` if the input doesn't look like a + single top-level JSON object (in which case the caller should fall back + rather than risk corrupting the file). + """ + match = _TOP_LEVEL_OBJECT_RE.match(text) + if match is None: + return None + + result = text + for key, value in patches.items(): + literal = _format_jsonc_value(value) + # Replace existing top-level key. We require the key to start a line + # (after optional whitespace) to avoid matching keys inside nested + # objects or strings. Value match handles strings, bools, numbers, + # null — not nested objects/arrays (we only patch scalar settings). + key_pattern = re.compile( + r'(?m)^(?P[ \t]*)"' + re.escape(key) + + r'"(?P\s*:\s*)(?P"(?:\\.|[^"\\])*"|true|false|null|-?\d+(?:\.\d+)?)' + ) + new_result, n = key_pattern.subn( + lambda m, lit=literal, k=key: f'{m.group("indent")}"{k}"{m.group("sep")}{lit}', + result, + count=1, + ) + if n: + result = new_result + continue + + # Key not present — insert before the final closing '}'. + insert_match = re.search(r"(?P[^\n]*)\}(?P\s*)\Z", result, re.DOTALL) + if insert_match is None: + return None + # Detect indent from the line before the closing brace, or default to 4 spaces. + last_line = insert_match.group("lastline") + indent_match = re.match(r"[ \t]*", last_line) + indent = indent_match.group(0) if indent_match else " " + if not indent: + indent = " " + # Find the position of the final '}' to insert before it. + close_idx = result.rfind("}") + before = result[:close_idx].rstrip() + after = result[close_idx:] + # Add a trailing comma to the previous entry if it doesn't already have one. + sep = "" if before.rstrip().endswith(("{", ",")) else "," + new_entry = f'{sep}\n{indent}"{key}": {literal}\n' + result = before + new_entry + after + + return result + + def _ensure_wsl_docker_settings(insiders: bool = False) -> None: """Auto-configure VS Code to use Docker from WSL if not already set.""" settings_path = _get_windows_vscode_settings_path(insiders) @@ -172,30 +253,43 @@ def _ensure_wsl_docker_settings(insiders: bool = False) -> None: distro = get_wsl_distro() - # Read existing settings (or start fresh) + desired: dict = {"dev.containers.executeInWSL": True} + if distro: + desired["dev.containers.executeInWSLDistro"] = distro + + # Read existing settings text + parsed form (for comparison only). + existing_text = "" settings: dict = {} if settings_path.is_file(): try: - settings = json5.loads(settings_path.read_text()) - except Exception: + existing_text = settings_path.read_text() + parsed = json5.loads(existing_text) if existing_text.strip() else {} + if isinstance(parsed, dict): + settings = parsed + except (OSError, ValueError): _print_wsl_hint() return - # Check if already configured - if settings.get("dev.containers.executeInWSL") is True: + # Determine which patches actually need to be written. + patches = {k: v for k, v in desired.items() if settings.get(k) != v} + if not patches: return - # Patch the settings - settings["dev.containers.executeInWSL"] = True - if distro: - settings["dev.containers.executeInWSLDistro"] = distro + if existing_text.strip(): + new_text = _patch_jsonc_settings(existing_text, patches) + if new_text is None: + _print_wsl_hint() + return + else: + # No file (or empty): write a fresh JSON object. + new_text = json.dumps(desired, indent=4) + "\n" try: settings_path.parent.mkdir(parents=True, exist_ok=True) - settings_path.write_text(json.dumps(settings, indent=4) + "\n") + settings_path.write_text(new_text) + suffix = f" ({distro})" if distro else "" print( - f"dcode: configured VS Code to use Docker from WSL" - + (f" ({distro})" if distro else ""), + f"dcode: configured VS Code to use Docker from WSL{suffix}", file=sys.stderr, ) except OSError: @@ -228,7 +322,9 @@ def run_dcode(path: str, *, insiders: bool = False) -> None: devcontainer = find_devcontainer(target) if devcontainer is None: - subprocess.run([editor, str(target)]) + result = subprocess.run([editor, str(target)], check=False) + if result.returncode: + sys.exit(result.returncode) return if main_repo is not None: @@ -245,7 +341,9 @@ def run_dcode(path: str, *, insiders: bool = False) -> None: else: uri = build_uri(host_path, workspace_folder) - subprocess.run([editor, "--folder-uri", uri]) + result = subprocess.run([editor, "--folder-uri", uri], check=False) + if result.returncode: + sys.exit(result.returncode) def main() -> None: diff --git a/tests/test_cli.py b/tests/test_cli.py index 1b83dc2..dacd38f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,12 +1,24 @@ """Tests for dcode CLI.""" import json +import subprocess from pathlib import Path from unittest.mock import patch import pytest -from dcode.cli import build_uri, build_uri_wsl, find_devcontainer, get_workspace_folder, resolve_worktree +from dcode.cli import ( + build_uri, + build_uri_wsl, + find_devcontainer, + get_workspace_folder, + resolve_worktree, +) + + +def _ok(): + """CompletedProcess representing successful editor invocation.""" + return subprocess.CompletedProcess(args=[], returncode=0) class TestBuildUri: @@ -103,6 +115,23 @@ def test_handles_jsonc_comments_and_trailing_commas(self, tmp_path): result = get_workspace_folder(dc_file, Path("/any")) assert result == "/custom" + def test_falls_back_on_malformed_devcontainer_json(self, tmp_path, capsys): + dc_file = tmp_path / "devcontainer.json" + dc_file.write_text("{ this is not json") + + result = get_workspace_folder(dc_file, Path("/x/myapp")) + assert result == "/workspaces/myapp" + assert "failed to parse" in capsys.readouterr().err + + +class TestVersion: + def test_resolves_via_importlib_metadata(self): + from importlib.metadata import version + + import dcode + + assert dcode.__version__ == version("dcode") + class TestMain: def test_launches_with_devcontainer_uri(self, tmp_path): @@ -110,7 +139,7 @@ def test_launches_with_devcontainer_uri(self, tmp_path): dc_dir.mkdir() (dc_dir / "devcontainer.json").write_text('{"name": "test"}') - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(tmp_path), insiders=False) @@ -124,7 +153,7 @@ def test_launches_insiders(self, tmp_path): dc_dir.mkdir() (dc_dir / "devcontainer.json").write_text('{"name": "test"}') - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(tmp_path), insiders=True) @@ -132,13 +161,33 @@ def test_launches_insiders(self, tmp_path): assert args[0] == "code-insiders" def test_fallback_without_devcontainer(self, tmp_path): - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(tmp_path), insiders=False) args = mock_run.call_args[0][0] assert args == ["code", str(tmp_path)] + def test_propagates_nonzero_exit_from_code_launcher(self, tmp_path): + failed = subprocess.CompletedProcess(args=[], returncode=2) + with patch("dcode.cli.subprocess.run", return_value=failed): + from dcode.cli import run_dcode + with pytest.raises(SystemExit) as exc: + run_dcode(str(tmp_path), insiders=False) + assert exc.value.code == 2 + + def test_propagates_nonzero_exit_with_devcontainer(self, tmp_path): + dc_dir = tmp_path / ".devcontainer" + dc_dir.mkdir() + (dc_dir / "devcontainer.json").write_text('{"name": "test"}') + + failed = subprocess.CompletedProcess(args=[], returncode=3) + with patch("dcode.cli.subprocess.run", return_value=failed): + from dcode.cli import run_dcode + with pytest.raises(SystemExit) as exc: + run_dcode(str(tmp_path), insiders=False) + assert exc.value.code == 3 + def test_uses_wsl_uri_on_wsl(self, tmp_path): dc_dir = tmp_path / ".devcontainer" dc_dir.mkdir() @@ -146,7 +195,7 @@ def test_uses_wsl_uri_on_wsl(self, tmp_path): unc = f"\\\\wsl.localhost\\Ubuntu{tmp_path}" with ( - patch("dcode.cli.subprocess.run") as mock_run, + patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run, patch("dcode.cli.is_wsl", return_value=True), patch("dcode.cli._ensure_wsl_docker_settings"), patch("dcode.cli._wsl_to_windows_path", return_value=unc), @@ -181,11 +230,10 @@ def test_patches_settings_when_not_configured(self, tmp_path): assert result["dev.containers.executeInWSLDistro"] == "Ubuntu" assert result["editor.fontSize"] == 14 # preserved - def test_skips_when_already_configured(self, tmp_path): + def test_adds_missing_distro_when_executeInWSL_already_true(self, tmp_path): settings_file = tmp_path / "Code" / "User" / "settings.json" settings_file.parent.mkdir(parents=True) - original = '{"dev.containers.executeInWSL": true, "other": 1}' - settings_file.write_text(original) + settings_file.write_text('{"dev.containers.executeInWSL": true, "other": 1}') with ( patch("dcode.cli._get_windows_vscode_settings_path", return_value=settings_file), @@ -194,9 +242,73 @@ def test_skips_when_already_configured(self, tmp_path): from dcode.cli import _ensure_wsl_docker_settings _ensure_wsl_docker_settings() - # File should not be rewritten (no distro added since executeInWSL already set) result = json.loads(settings_file.read_text()) assert result["dev.containers.executeInWSL"] is True + assert result["dev.containers.executeInWSLDistro"] == "Ubuntu" + assert result["other"] == 1 + + def test_does_nothing_when_both_keys_already_correct(self, tmp_path): + settings_file = tmp_path / "Code" / "User" / "settings.json" + settings_file.parent.mkdir(parents=True) + original = ( + '{"dev.containers.executeInWSL": true, ' + '"dev.containers.executeInWSLDistro": "Ubuntu"}' + ) + settings_file.write_text(original) + + with ( + patch("dcode.cli._get_windows_vscode_settings_path", return_value=settings_file), + patch("dcode.cli.get_wsl_distro", return_value="Ubuntu"), + ): + from dcode.cli import _ensure_wsl_docker_settings + _ensure_wsl_docker_settings() + + # File content byte-identical — no rewrite happened. + assert settings_file.read_text() == original + + def test_preserves_jsonc_comments_and_trailing_commas(self, tmp_path): + settings_file = tmp_path / "Code" / "User" / "settings.json" + settings_file.parent.mkdir(parents=True) + original = ( + '// keep me\n' + '{\n' + ' "editor.fontSize": 14,\n' + '}\n' + ) + settings_file.write_text(original) + + with ( + patch("dcode.cli._get_windows_vscode_settings_path", return_value=settings_file), + patch("dcode.cli.get_wsl_distro", return_value="Ubuntu"), + ): + from dcode.cli import _ensure_wsl_docker_settings + _ensure_wsl_docker_settings() + + new_text = settings_file.read_text() + # Original comment + trailing comma preserved. + assert "// keep me" in new_text + assert '"editor.fontSize": 14' in new_text + # New keys present. + assert '"dev.containers.executeInWSL": true' in new_text + assert '"dev.containers.executeInWSLDistro": "Ubuntu"' in new_text + + def test_falls_back_to_hint_on_unpatchable_file(self, tmp_path, capsys): + settings_file = tmp_path / "Code" / "User" / "settings.json" + settings_file.parent.mkdir(parents=True) + original = "[]" + settings_file.write_text(original) + + with ( + patch("dcode.cli._get_windows_vscode_settings_path", return_value=settings_file), + patch("dcode.cli.get_wsl_distro", return_value="Ubuntu"), + ): + from dcode.cli import _ensure_wsl_docker_settings + _ensure_wsl_docker_settings() + + # File untouched. + assert settings_file.read_text() == original + # Hint printed. + assert "dev.containers.executeInWSL" in capsys.readouterr().err def test_falls_back_to_hint_when_path_not_found(self, capsys): with patch("dcode.cli._get_windows_vscode_settings_path", return_value=None): @@ -304,7 +416,7 @@ def test_worktree_uses_main_repo_host_path(self, tmp_path): dc_dir.mkdir() (dc_dir / "devcontainer.json").write_text('{"name": "test"}') - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(worktree)) @@ -321,7 +433,7 @@ def test_worktree_workspace_folder_includes_relative_path(self, tmp_path): dc_dir.mkdir() (dc_dir / "devcontainer.json").write_text('{"name": "test"}') - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(worktree)) @@ -343,7 +455,7 @@ def test_multiple_worktrees_share_same_container(self, tmp_path): wt.mkdir(parents=True) (wt / ".git").write_text(f"gitdir: ../../.git/worktrees/{name}\n") - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(wt)) uris.append(mock_run.call_args[0][0][2]) @@ -360,7 +472,7 @@ def test_multiple_worktrees_share_same_container(self, tmp_path): def test_worktree_falls_back_when_no_devcontainer_in_main_repo(self, tmp_path): main_repo, worktree = _make_worktree(tmp_path) - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(worktree)) @@ -373,7 +485,7 @@ def test_worktree_with_custom_workspace_folder(self, tmp_path): dc_dir.mkdir() (dc_dir / "devcontainer.json").write_text('{"workspaceFolder": "/workspace"}') - with patch("dcode.cli.subprocess.run") as mock_run: + with patch("dcode.cli.subprocess.run", return_value=_ok()) as mock_run: from dcode.cli import run_dcode run_dcode(str(worktree)) From cec77327d41f8553a202d14db32c9aecd1ba1be6 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:24:47 +1000 Subject: [PATCH 2/8] build: derive version from git tags via hatch-vcs - Switch to hatch-vcs so the version comes from the latest git tag at build time. - __init__.py exposes __version__ via importlib.metadata.version, with a PackageNotFoundError fallback. - Add [project.urls] (Homepage/Issues/Source). - Add ruff to dev dependencies. - Gitignore generated _version.py and .ruff_cache/. --- .gitignore | 2 ++ pyproject.toml | 17 ++++++++++++++--- src/dcode/__init__.py | 10 +++++++++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 4b45f78..8b19cb6 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ __pycache__/ dist/ .pytest_cache/ uv.lock +src/dcode/_version.py +.ruff_cache/ diff --git a/pyproject.toml b/pyproject.toml index bc45555..c01a3fc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,19 +1,30 @@ [project] name = "dcode" -version = "0.4.1" description = "Open folders in VS Code devcontainers from the CLI" readme = "README.md" license = "MIT" requires-python = ">=3.11" dependencies = ["json5"] +dynamic = ["version"] + +[project.urls] +Homepage = "https://github.com/rosstaco/dcode" +Issues = "https://github.com/rosstaco/dcode/issues" +Source = "https://github.com/rosstaco/dcode" [project.scripts] dcode = "dcode.cli:main" [build-system] -requires = ["hatchling"] +requires = ["hatchling", "hatch-vcs"] build-backend = "hatchling.build" +[tool.hatch.version] +source = "vcs" + +[tool.hatch.build.hooks.vcs] +version-file = "src/dcode/_version.py" + [tool.hatch.build.targets.wheel] packages = ["src/dcode"] @@ -21,4 +32,4 @@ packages = ["src/dcode"] testpaths = ["tests"] [dependency-groups] -dev = ["pytest"] +dev = ["pytest", "ruff"] diff --git a/src/dcode/__init__.py b/src/dcode/__init__.py index f88afad..ec6f033 100644 --- a/src/dcode/__init__.py +++ b/src/dcode/__init__.py @@ -1,3 +1,11 @@ """dcode — open folders in VS Code devcontainers from the CLI.""" -__version__ = "0.4.1" +from __future__ import annotations + +from importlib.metadata import PackageNotFoundError +from importlib.metadata import version as _pkg_version + +try: + __version__ = _pkg_version("dcode") +except PackageNotFoundError: # pragma: no cover - only happens when not installed + __version__ = "0.0.0+unknown" From 1fb9a991957c5f6fe08474342b2e0f5554075a14 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:24:58 +1000 Subject: [PATCH 3/8] feat: support 'python -m dcode' invocation --- src/dcode/__main__.py | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 src/dcode/__main__.py diff --git a/src/dcode/__main__.py b/src/dcode/__main__.py new file mode 100644 index 0000000..7c89143 --- /dev/null +++ b/src/dcode/__main__.py @@ -0,0 +1,6 @@ +"""Allow ``python -m dcode`` to invoke the CLI.""" + +from .cli import main + +if __name__ == "__main__": + main() From 231616b73f59d0c29b79ac6fef919ae5e8e1b6b4 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:24:58 +1000 Subject: [PATCH 4/8] chore: add ruff configuration Line-length 100, target py311, lint rules E/F/I/UP/B/SIM. --- ruff.toml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 ruff.toml diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..4fd36b9 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,11 @@ +line-length = 100 +target-version = "py311" + +[lint] +select = ["E", "F", "I", "UP", "B", "SIM"] +ignore = [ + "E501", # line length handled by formatter; allow long strings +] + +[lint.per-file-ignores] +"tests/*" = ["E501"] From 9ef0bb31d220dcda2e3201f1651a6ab1b8070e91 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:25:11 +1000 Subject: [PATCH 5/8] ci: add lint and test workflow Matrix py3.11/3.12/3.13. Uses fetch-depth: 0 so hatch-vcs can resolve the version from git tags. --- .github/workflows/ci.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..129c6c0 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,33 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.11", "3.12", "3.13"] + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # hatch-vcs needs full history to resolve version from tags + + - name: Install uv + uses: astral-sh/setup-uv@v3 + + - name: Set up Python ${{ matrix.python-version }} + run: uv python install ${{ matrix.python-version }} + + - name: Install dependencies + run: uv sync --python ${{ matrix.python-version }} + + - name: Lint + run: uv run ruff check + + - name: Test + run: uv run pytest From dc79c73a448d579bdc7e1c1c225c914c70b56f74 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:25:11 +1000 Subject: [PATCH 6/8] ci: automate releases via release-please Uses release-type: simple so it manages only the manifest, tag, and CHANGELOG. hatch-vcs reads the tag for the actual package version, so no source files need to be edited. --- .github/workflows/release-please.yml | 18 ++++++++++++++++++ .release-please-manifest.json | 3 +++ CHANGELOG.md | 6 ++++++ release-please-config.json | 13 +++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 .github/workflows/release-please.yml create mode 100644 .release-please-manifest.json create mode 100644 CHANGELOG.md create mode 100644 release-please-config.json diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml new file mode 100644 index 0000000..f29f940 --- /dev/null +++ b/.github/workflows/release-please.yml @@ -0,0 +1,18 @@ +name: Release Please + +on: + push: + branches: [main] + +permissions: + contents: write + pull-requests: write + +jobs: + release-please: + runs-on: ubuntu-latest + steps: + - uses: googleapis/release-please-action@v4 + with: + config-file: release-please-config.json + manifest-file: .release-please-manifest.json diff --git a/.release-please-manifest.json b/.release-please-manifest.json new file mode 100644 index 0000000..218393f --- /dev/null +++ b/.release-please-manifest.json @@ -0,0 +1,3 @@ +{ + ".": "0.4.1" +} diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..1cfb2ec --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,6 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +This project follows [Conventional Commits](https://www.conventionalcommits.org/) and +versions are managed by [release-please](https://github.com/googleapis/release-please). diff --git a/release-please-config.json b/release-please-config.json new file mode 100644 index 0000000..b4d2348 --- /dev/null +++ b/release-please-config.json @@ -0,0 +1,13 @@ +{ + "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", + "release-type": "simple", + "bump-minor-pre-major": true, + "bump-patch-for-minor-pre-major": true, + "include-component-in-tag": false, + "packages": { + ".": { + "package-name": "dcode", + "changelog-path": "CHANGELOG.md" + } + } +} From ff3e8392b65fc5f1c84c6953ea4483955ab959ab Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:25:25 +1000 Subject: [PATCH 7/8] docs: document WSL behavior, ruff, and the release-please flow - Add WSL behavior section to README explaining the settings.json auto-edit and how to opt out. - Note Conventional Commits requirement and the release-please flow. - Update copilot-instructions to remove the manual version-bump rule (now handled by hatch-vcs + release-please). --- .github/copilot-instructions.md | 33 +++++++++++++++++++++++++++++++++ README.md | 17 +++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..f51d9cc --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,33 @@ +# Copilot Instructions for dcode + +## Build & Test + +```bash +# Run all tests +uv run pytest + +# Run a single test by name +uv run pytest -k test_resolves_worktree_with_relative_gitdir + +# Run a test class +uv run pytest -k TestResolveWorktree +``` + +Linting is via `ruff` (`uv run ruff check`). Versioning is automated: `hatch-vcs` derives the version from the latest git tag, and [release-please](https://github.com/googleapis/release-please) manages tags + the changelog from [Conventional Commits](https://www.conventionalcommits.org/). **Do not** edit a version field in `pyproject.toml` or `src/dcode/__init__.py` — both are derived from the git tag at install/build time via `importlib.metadata`. + +## Architecture + +dcode is a single-module CLI (`src/dcode/cli.py`) that constructs `vscode-remote://dev-container+` URIs and launches VS Code with `--folder-uri`. The entrypoint is `dcode.cli:main`. + +The core flow in `run_dcode()`: + +1. **Worktree detection** — `resolve_worktree()` checks if `.git` is a file (not directory), parses the `gitdir:` pointer, and validates the path structure to distinguish worktrees from submodules. When the gitdir contains an absolute path from a different environment (e.g. a container), it falls back to walking ancestor directories for the real `.git` dir. +2. **Config lookup** — `find_devcontainer()` searches for `.devcontainer/devcontainer.json` or `.devcontainer.json` in the target (or main repo for worktrees). +3. **URI construction** — Two codepaths: `build_uri()` for native systems (hex-encodes the host path directly) and `build_uri_wsl()` for WSL (wraps a Windows UNC path in a JSON payload). For worktrees, the host path is always the main repo root so all worktrees share one container. + +## Conventions + +- `json5` is used to parse `devcontainer.json` (supports JSONC comments and trailing commas). +- All user-facing messages go to `sys.stderr`; stdout is reserved for machine output. +- Tests use `tmp_path` fixtures with mock filesystem layouts (fake `.git` files/dirs) — no real git repos needed. `subprocess.run` is always patched in integration tests. +- The helper `_make_worktree(tmp_path, name)` in the test file creates a complete fake main-repo + worktree layout for test reuse. diff --git a/README.md b/README.md index 0595389..0d897ad 100644 --- a/README.md +++ b/README.md @@ -49,3 +49,20 @@ dcode .worktrees/pr-99 Constructs a `vscode-remote://dev-container+/workspaces/` URI and launches VS Code with `--folder-uri`. VS Code handles the container lifecycle automatically. For worktrees, the hex-encoded path points to the main repo (so all worktrees resolve to the same container), while the workspace folder is adjusted to open the worktree subfolder inside the container. + +## 🐧 WSL behavior + +When `dcode` runs inside WSL, it: + +1. Builds the URI using a Windows UNC path (`\\wsl.localhost\\…`) so VS Code on Windows can resolve the folder. +2. Auto-edits your **Windows** VS Code `settings.json` (under `%APPDATA%\Code\User\` or `Code - Insiders`) to set: + - `"dev.containers.executeInWSL": true` + - `"dev.containers.executeInWSLDistro": ""` + + This is required so the Dev Containers extension talks to Docker inside WSL instead of `docker.exe` on Windows. Comments and trailing commas in your `settings.json` are preserved (in-place patching, not a rewrite). + +To opt out, pre-set those keys to whatever values you want — `dcode` only writes them when they're missing or differ from the desired values. + +## 🤝 Contributing + +This project uses [Conventional Commits](https://www.conventionalcommits.org/) (`feat:`, `fix:`, `chore:`, `docs:`, etc.). Releases are automated by [release-please](https://github.com/googleapis/release-please) — merging a `feat:` or `fix:` commit to `main` opens/updates a release PR, and merging that PR creates the tag + GitHub Release. From 99254c4c30fbe430d79ce071008a34d0a25ec3c7 Mon Sep 17 00:00:00 2001 From: rosstaco Date: Mon, 27 Apr 2026 20:25:40 +1000 Subject: [PATCH 8/8] chore: add audit-cleanup planning artifacts Plan, implementation notes, and verification report for this change. --- .../implementation-notes.md | 76 +++++++ .../plans/2026-04-27-audit-cleanup/plan.md | 202 ++++++++++++++++++ .../2026-04-27-audit-cleanup/verify-report.md | 138 ++++++++++++ 3 files changed, 416 insertions(+) create mode 100644 project/plans/2026-04-27-audit-cleanup/implementation-notes.md create mode 100644 project/plans/2026-04-27-audit-cleanup/plan.md create mode 100644 project/plans/2026-04-27-audit-cleanup/verify-report.md diff --git a/project/plans/2026-04-27-audit-cleanup/implementation-notes.md b/project/plans/2026-04-27-audit-cleanup/implementation-notes.md new file mode 100644 index 0000000..aea49ed --- /dev/null +++ b/project/plans/2026-04-27-audit-cleanup/implementation-notes.md @@ -0,0 +1,76 @@ +# Implementation Notes — dcode audit cleanup + +## Status: Complete + +All planned changes implemented. 39 tests pass. `ruff check` clean. + +## Summary of changes + +### Files modified +- `src/dcode/cli.py` — bug fixes (Fix 1–7 per plan); new `_patch_jsonc_settings` function preserves JSONC when patching `settings.json`. +- `src/dcode/__init__.py` — `__version__` now resolved via `importlib.metadata`. +- `pyproject.toml` — switched to `hatch-vcs` (dynamic version), added `[project.urls]`, added `ruff` to dev deps. +- `tests/test_cli.py` — renamed `test_skips_when_already_configured` → `test_adds_missing_distro_when_executeInWSL_already_true`; added 6 new tests; mass-updated `subprocess.run` patches to return `CompletedProcess(returncode=0)`. +- `.gitignore` — added `src/dcode/_version.py` (hatch-vcs build artifact) and `.ruff_cache/`. +- `.github/copilot-instructions.md` — removed manual-version-bump line; documented release-please + hatch-vcs flow. +- `README.md` — added "WSL behavior" section + Conventional Commits / release-please note. + +### Files created +- `src/dcode/__main__.py` — enables `python -m dcode`. +- `ruff.toml` — line-length 100, py311 target, lint rules E/F/I/UP/B/SIM. +- `.github/workflows/ci.yml` — matrix py3.11/3.12/3.13 with `fetch-depth: 0` for hatch-vcs. +- `.github/workflows/release-please.yml` — release-please action v4. +- `release-please-config.json` — uses `release-type: simple` (not `python`) so it doesn't try to edit `pyproject.toml` (hatch-vcs handles versioning). +- `.release-please-manifest.json` — baseline `0.4.1`. +- `CHANGELOG.md` — seed file for release-please to append to. + +## Deviations from the plan + +1. **`release-type: simple` instead of `python`** — the plan called for `python` with `extra-files: []`, but `simple` is cleaner: it manages only the manifest + tag + changelog, never touching source files. With hatch-vcs, the tag IS the version, so this is exactly what we want. + +2. **Added `[tool.hatch.build.hooks.vcs] version-file`** — not in the plan, but useful: hatch-vcs writes a `_version.py` at build time so introspection works even outside an installed package context. Gitignored. + +3. **Test fixture pattern** — the plan didn't specify how to handle the new `subprocess.run().returncode` check across all existing tests. Solution: added a `_ok()` helper returning `CompletedProcess(returncode=0)` and updated all `patch("dcode.cli.subprocess.run")` calls via `sed` to include `return_value=_ok()`. Bare `MagicMock` would have made `returncode` truthy and triggered unwanted `SystemExit`. + +4. **`--insiders` flag for `__main__.py`** — not added (would have been scope creep). `python -m dcode -i .` works through the existing `argparse`. + +5. **`copilot-instructions.md` already updated by the user mid-implementation** — the file already had the new release-please/hatch-vcs language when checked. No edit needed (str_replace was a no-op due to identical input/output). + +## Acceptance criteria verification + +| Criterion | Status | Evidence | +|---|---|---| +| `uv run pytest` passes | ✅ | `39 passed in 0.05s` | +| `uv run ruff check` clean | ✅ | `All checks passed!` | +| `python -m dcode --help` works | ✅ | Shows full argparse help | +| Version auto-resolved via hatch-vcs | ✅ | `dcode.__version__` = `0.1.dev8+g5f9826397.d20260427` (resolved at install time, no static value anywhere) | +| `_ensure_wsl_docker_settings` no longer rewrites JSONC | ✅ | `test_preserves_jsonc_comments_and_trailing_commas` | +| Adds distro when `executeInWSL` already true | ✅ | `test_adds_missing_distro_when_executeInWSL_already_true` | +| Falls back to hint on un-patchable file | ✅ | `test_falls_back_to_hint_on_unpatchable_file` | +| Malformed `devcontainer.json` → friendly stderr | ✅ | `test_falls_back_on_malformed_devcontainer_json` | +| Non-zero exit propagated | ✅ | `test_propagates_nonzero_exit_from_code_launcher`, `test_propagates_nonzero_exit_with_devcontainer` | +| `[project.urls]` added | ✅ | Homepage / Issues / Source pointing at github.com/rosstaco/dcode | +| CI workflow runs ruff + pytest on py3.11/3.12/3.13 | ✅ | `.github/workflows/ci.yml` (not exercised — no PR yet) | +| README WSL section | ✅ | Documents auto-edit + opt-out | +| Manual macOS smoke test | ✅ | `python -m dcode --help` works; not run against a real devcontainer (would launch VS Code) | +| Manual WSL smoke test | ⚠️ Skipped | No WSL environment available on this macOS host | + +## Pre-merge prerequisites for the user + +1. **Tag the current main HEAD as `v0.4.1`** before merging this branch: + ```bash + git tag v0.4.1 + git push --tags + ``` + This gives `hatch-vcs` a baseline so post-merge installs from `main` resolve to `0.4.2.devN+g` rather than failing. + +2. **First commit message after merge** should follow Conventional Commits (e.g. `feat:` or `fix:`) so release-please can open its first release PR. + +3. **GitHub Actions permissions** — release-please needs `contents: write` and `pull-requests: write`. The workflow declares these, but the repo settings must allow Actions to create PRs (Settings → Actions → General → "Allow GitHub Actions to create and approve pull requests"). + +## Things to watch in Verify phase + +- Run the actual `dcode` command (not `--help`) against a real devcontainer repo to confirm URI launches still work. +- On WSL: confirm `_patch_jsonc_settings` actually preserves a hand-edited `settings.json` (the unit test covers the algorithm but not the real file). +- After first merge to main, confirm release-please opens a PR within ~5 min. +- After merging the release PR, confirm `git tag` exists and `uv tool install --reinstall git+https://github.com/rosstaco/dcode` shows the new version. diff --git a/project/plans/2026-04-27-audit-cleanup/plan.md b/project/plans/2026-04-27-audit-cleanup/plan.md new file mode 100644 index 0000000..a9edcdf --- /dev/null +++ b/project/plans/2026-04-27-audit-cleanup/plan.md @@ -0,0 +1,202 @@ +# Plan: dcode audit cleanup + +Date: 2026-04-27 +Source: audit raised in chat (no separate research-findings.md — audit IS the research, all findings reference current source files). + +## Goals + +Address the audit findings the user accepted, without changing the CLI's user-facing behavior or URI format. + +## Out of scope + +- Lowering `requires-python` to 3.10 +- Removing `copilot-session-*.md` +- Refactoring `run_dcode()` control flow beyond what bug fixes require +- Adding logging module / `_log()` helper +- **Dropping `json5` dependency** — deferred; risk of JSONC edge cases outweighs the benefit. Keep `json5` for parsing both `devcontainer.json` and existing `settings.json`. + +## Architecture + +```mermaid +flowchart TD + A[dcode CLI entrypoint] --> B[run_dcode] + B --> C[resolve_worktree] + B --> D[find_devcontainer] + B --> E[get_workspace_folder] + E --> F[json5.load - unchanged] + B --> G[build_uri / build_uri_wsl] + B --> H{is_wsl?} + H -->|yes| I[_ensure_wsl_docker_settings - PATCHED] + I --> F2[json5.loads - read existing for comparison] + I --> J[_patch_jsonc_settings - NEW minimal text edit] + B --> K[subprocess.run code/code-insiders] + K --> L[exit with returncode - NEW] + + M[__init__.py] --> N[importlib.metadata.version - NEW] + O[__main__.py - NEW] --> A +``` + +## File list (in order) + +1. **`pyproject.toml`** — add `[project.urls]`; keep `json5` dep; switch to `hatch-vcs` for git-tag-based versioning (replaces static `version = "0.4.1"` with dynamic). Add `hatch-vcs` to `[build-system].requires` and configure `[tool.hatch.version] source = "vcs"`. +2. **`src/dcode/__init__.py`** — replace hardcoded `__version__` with `importlib.metadata.version("dcode")` (with `PackageNotFoundError` fallback to `"0.0.0+unknown"`). +2a. **`.github/copilot-instructions.md`** — remove the line saying "version must be bumped in both `pyproject.toml` and `src/dcode/__init__.py`" since both are now auto-derived from git tags. +2b. **Pre-merge action** — ensure tag `v0.4.1` exists on the current commit before merging this change, so `uv tool install git+https://...` from `main` continues to resolve a clean version. +3. **`src/dcode/cli.py`** — apply the fixes below (see "Code changes"). +4. **`src/dcode/__main__.py`** *(NEW)* — `from .cli import main; main()`. +5. **`tests/test_cli.py`** — add new test cases (see "Test cases"). `json5` imports stay. +6. **`ruff.toml`** *(NEW)* — minimal ruff config (line-length 100, target-version py311, select E, F, I, UP, B, SIM). +7. **`.github/workflows/ci.yml`** *(NEW)* — matrix py3.11/3.12/3.13: `uv sync`, `uv run ruff check`, `uv run pytest`. Use `actions/checkout@v4` with `fetch-depth: 0` so `hatch-vcs` can run `git describe`. +8. **`.github/workflows/release-please.yml`** *(NEW)* — [googleapis/release-please-action](https://github.com/googleapis/release-please-action) configured for `release-type: python`. Watches `main`, parses Conventional Commits, opens a release PR with version bump + changelog. When merged, creates tag `vX.Y.Z` and GitHub Release. `hatch-vcs` then picks up the tag for the actual package version. +9. **`.release-please-manifest.json`** *(NEW)* — `{".": "0.4.1"}` so release-please knows the current baseline. +10. **`release-please-config.json`** *(NEW)* — minimal config: `{"packages": {".": {"release-type": "python", "bump-minor-pre-major": true}}}`. Note: release-please's `python` type normally edits `pyproject.toml` and `__init__.py` — we configure `extra-files: []` and rely solely on the git tag (since hatch-vcs reads the tag, not the file). Alternative: use `release-type: simple` which only manages the tag and changelog. +11. **`README.md`** — add a "WSL behavior" subsection documenting the auto-edit of `settings.json`. +12. **`CHANGELOG.md`** *(NEW, empty/seeded)* — release-please appends to this on each release. + +## Code changes (cli.py) + +### Fix 1 — `_ensure_wsl_docker_settings` distro skip bug +Current logic returns early if `executeInWSL` is `True`, never setting `executeInWSLDistro`. Change to: +- Compute desired patches: `executeInWSL=True` always; `executeInWSLDistro=` if distro known. +- Compare against existing settings (parsed via `json5.loads`); only write if at least one patch differs. + +### Fix 2 — JSONC preservation (minimal text edit) +Replace the `json.dumps(settings, indent=4)` rewrite with `_patch_jsonc_settings(text: str, patches: dict) -> str | None`: +- For each `key, value` patch: if a top-level `"key":` already exists, regex-replace its value in place (bool/string values only). +- Otherwise insert the new `"key": value,` line just before the final closing top-level `}`. +- Preserves comments, trailing commas, indentation style. +- If the file isn't a recognizable top-level object (regex doesn't match, e.g. exotic formatting), return `None` and fall back to printing the hint instead of corrupting the file. + +### Fix 3 — JSONC error handling in `get_workspace_folder` +Wrap `json5.load` in try/except `(OSError, ValueError)`; on failure print a friendly stderr message and fall back to default `/workspaces/{target.name}`. + +### Fix 4 — subprocess return code propagation +In `run_dcode`, capture `result = subprocess.run([...], check=False)` and `sys.exit(result.returncode)` if non-zero. (Only for the `code` invocation, not for `wslpath`.) + +### Fix 5 — narrow exception in `_ensure_wsl_docker_settings` settings parse +Replace `except Exception` with `except (OSError, ValueError)`. + +### Fix 6 — `_wsl_to_windows_path` returncode check +Check `result.returncode == 0` before using stdout. + +### Fix 7 — small lint cleanups +- Collapse `f"dcode: configured ..." + (f" ({distro})" if distro else "")` into a single conditional f-string with no leading `f` on the static part. +- `is_wsl()` — move `read_text` inside the existence branch cleanly. + +## Dependency removal (json5 → stdlib JSONC) + +Replace both call sites: +- `get_workspace_folder`: `json5.load(f)` → `_load_jsonc(f.read())` +- `_ensure_wsl_docker_settings`: `json5.loads(text)` → `_load_jsonc(text)` (read for comparison only; writes go through `_patch_jsonc_settings`) + +JSONC loader spec: +``` +def _load_jsonc(text: str) -> dict: + # 1. Walk char-by-char tracking string state (handle \" escape) + # 2. Outside strings: drop //... to end-of-line, drop /* ... */ + # 3. Drop trailing commas before } or ] + # 4. json.loads the result +``` + +## Test cases + +### test_cli.py — new cases + +``` +Test: ensure_wsl_docker_settings sets distro when executeInWSL already true but distro missing +Given: settings.json containing only {"dev.containers.executeInWSL": true} +When: _ensure_wsl_docker_settings() with get_wsl_distro="Ubuntu" +Then: file now also contains "dev.containers.executeInWSLDistro": "Ubuntu" +Data: '{"dev.containers.executeInWSL": true}' + +Test: ensure_wsl_docker_settings preserves JSONC comments and trailing commas +Given: settings.json containing comments + trailing commas +When: _ensure_wsl_docker_settings() with distro="Ubuntu" +Then: resulting text still contains the original "// my comment" line and trailing comma +Data: '// keep me\n{\n "editor.fontSize": 14,\n}\n' + +Test: ensure_wsl_docker_settings does nothing when both keys already correct +Given: settings.json with both keys set to expected values +When: _ensure_wsl_docker_settings() with distro="Ubuntu" +Then: file mtime unchanged (or content byte-identical) + +Test: get_workspace_folder falls back on malformed devcontainer.json +Given: devcontainer.json containing "{ this is not json" +When: get_workspace_folder(path, Path("/x/myapp")) +Then: returns "/workspaces/myapp" and prints a warning to stderr + +Test: run_dcode propagates non-zero exit from code launcher +Given: subprocess.run patched to return CompletedProcess(returncode=2) +When: run_dcode(tmp_path) +Then: SystemExit raised with code 2 + +Test: __version__ resolves via importlib.metadata +Given: package installed +When: import dcode; dcode.__version__ +Then: matches version from importlib.metadata.version("dcode") +``` + +### Existing tests +- `test_handles_jsonc_comments_and_trailing_commas` — unchanged (still uses `json5`). +- `test_skips_when_already_configured` — **rename** to `test_adds_missing_distro_when_executeInWSL_already_true` with inverted assertion (the bug being fixed: distro must now be added). + +### New test +``` +Test: ensure_wsl_docker_settings falls back to hint on un-patchable file +Given: settings.json containing only "[]" (not an object) +When: _ensure_wsl_docker_settings() with distro="Ubuntu" +Then: file unchanged, hint printed to stderr +``` + +## Acceptance criteria + +- [ ] `uv run pytest` passes (all old + new tests) +- [ ] `uv run ruff check` passes clean +- [ ] `python -m dcode --help` works +- [ ] No version mismatch possible — version is auto-resolved from git tags via `hatch-vcs` and exposed through `importlib.metadata` +- [ ] `uv tool install git+https://github.com/rosstaco/dcode` succeeds and `python -c 'import dcode; print(dcode.__version__)'` shows the tag-derived version +- [ ] Tag `v0.4.1` exists on `main` before merging this change +- [ ] `.github/copilot-instructions.md` no longer references manual version bumping +- [ ] release-please workflow opens a release PR after the first Conventional Commit lands on `main` post-merge +- [ ] Merging the release PR creates tag `vX.Y.Z` and a GitHub Release +- [ ] `uv tool install git+https://github.com/rosstaco/dcode` after that release returns the new version via `dcode.__version__` +- [ ] `_ensure_wsl_docker_settings` no longer rewrites a JSONC settings file as plain JSON (verified via test reading raw text, comments preserved) +- [ ] `_ensure_wsl_docker_settings` falls back to printing the hint (not corrupting the file) when the settings file has un-patchable structure +- [ ] `_ensure_wsl_docker_settings` correctly adds `executeInWSLDistro` even when `executeInWSL` is already `true` +- [ ] Malformed `devcontainer.json` produces a friendly stderr message instead of a stack trace +- [ ] Non-zero exit from `code`/`code-insiders` is propagated as the dcode exit code +- [ ] `pyproject.toml` has `[project.urls]` with Homepage, Issues, Source pointing at `https://github.com/rosstaco/dcode` +- [ ] `.github/workflows/ci.yml` runs ruff + pytest on py3.11/3.12/3.13 +- [ ] README has a "WSL behavior" section documenting the `settings.json` auto-edit and how to opt out (delete the keys / pre-set them) +- [ ] Manual smoke test on macOS: `dcode .` in this repo (no devcontainer present) falls through to `code .`; `dcode ` opens the URI form +- [ ] Manual smoke test on WSL (if available): JSONC `settings.json` is patched without losing comments + +## Risks & assumptions + +1. **Top-level-key regex patch** — assumes `settings.json` is one top-level object (VS Code always writes it this way). Hand-crafted exotic formatting is handled by the un-patchable-fallback path (prints hint, leaves file alone). Tested. + +2. **`importlib.metadata` in editable installs** — `uv tool install` and `pip install -e .` both register metadata. Bare `python src/dcode/cli.py` would fail, but that's not a supported entrypoint. + +3. **`hatch-vcs` requires git history at build time** — `uv tool install git+https://...` clones with refs/tags, so `git describe` works. Risk: if no tags exist in the repo, build fails. Mitigation: tag `v0.4.1` on current `main` *before* merging this change. Untagged commits resolve to `0.4.1.devN+g` which is fine. + +4. **CI checkout depth** — `actions/checkout@v4` defaults to shallow (depth=1), which breaks `git describe`. CI workflow must set `fetch-depth: 0` (or `fetch-tags: true`). + +5. **CI matrix py3.13 availability** — `actions/setup-python@v5` supports it; uv supports it. + +6. **No `code` CLI on CI** — `subprocess.run` is mocked in tests, so CI doesn't need VS Code installed. Manual smoke tests cover the real launch. + +## Implementation order + +Suggested commit grouping: +1. WSL settings fixes (distro bug + JSONC-preserving patcher + tests) +2. devcontainer.json error handling + subprocess returncode propagation + tests +3. Switch to `hatch-vcs` + version single-sourcing via `importlib.metadata` + `__main__.py` + update copilot-instructions +4. `pyproject.toml` `[project.urls]` +5. ruff config + lint fixes +6. CI workflow (with `fetch-depth: 0`) +7. README WSL section +8. release-please workflow + config + manifest + seed CHANGELOG.md + +**Before tagging a release:** `git tag v0.4.1 && git push --tags` so existing install method keeps working and release-please has a baseline. + +**Conventional Commits requirement:** Once release-please is in place, commits to `main` must use Conventional Commit prefixes (`feat:`, `fix:`, `chore:`, `docs:`, etc.) for the version bumping to work correctly. Add a one-line note to `README.md` (or `CONTRIBUTING.md` if created) about this. diff --git a/project/plans/2026-04-27-audit-cleanup/verify-report.md b/project/plans/2026-04-27-audit-cleanup/verify-report.md new file mode 100644 index 0000000..e4b3bdd --- /dev/null +++ b/project/plans/2026-04-27-audit-cleanup/verify-report.md @@ -0,0 +1,138 @@ +# Verification Report — dcode audit cleanup + +Date: 2026-04-27 +Reviewer: Verify agent +Plan: [plan.md](plan.md) +Implementation notes: [implementation-notes.md](implementation-notes.md) + +## Summary + +- **Acceptance Criteria:** 13/16 passed, 3 deferred (operational — require post-merge actions or environment unavailable on host) +- **Test Quality:** Good with one minor weakness (one tautological test) +- **E2E Status:** Local — working (build, version resolution, `python -m dcode`, all 39 tests). Real `dcode ` launch not exercised (would open VS Code interactively). +- **Recommendation:** **Ship.** No critical defects. Two small recommendations below. + +## Acceptance Criteria Audit + +| # | Criterion | Code Location | Test | Evidence | Status | +|---|---|---|---|---|---| +| 1 | `uv run pytest` passes | — | all 39 tests | `39 passed in 0.05s` | ✅ | +| 2 | `uv run ruff check` clean | [ruff.toml](ruff.toml) | n/a | `All checks passed!` | ✅ | +| 3 | `python -m dcode --help` works | [src/dcode/__main__.py](src/dcode/__main__.py) | manual | argparse help output shown | ✅ | +| 4 | No version mismatch — hatch-vcs + importlib.metadata | [pyproject.toml#L19-L26](pyproject.toml#L19-L26), [src/dcode/__init__.py](src/dcode/__init__.py) | `TestVersion::test_resolves_via_importlib_metadata` | wheel built as `dcode-0.1.dev8+g5f9826397.d20260427-py3-none-any.whl`; no static version anywhere | ✅ | +| 5 | `uv tool install git+https://...` succeeds with tag-derived version | n/a | n/a | NOT executed; requires network + post-merge tag | ⚠️ Deferred (operational) | +| 6 | Tag `v0.4.1` exists on `main` before merge | n/a | n/a | NOT done yet | ⚠️ Pre-merge prerequisite | +| 7 | `.github/copilot-instructions.md` no longer references manual bumping | [.github/copilot-instructions.md#L17](.github/copilot-instructions.md#L17) | manual | Line now reads "**Do not** edit a version field…" | ✅ | +| 8 | release-please workflow opens release PR | [.github/workflows/release-please.yml](.github/workflows/release-please.yml) | n/a | NOT exercised (post-merge) | ⚠️ Deferred (post-merge) | +| 9 | `_ensure_wsl_docker_settings` no longer rewrites JSONC | [src/dcode/cli.py#L186-L249](src/dcode/cli.py#L186-L249) | `test_preserves_jsonc_comments_and_trailing_commas` | Test reads raw text, asserts `// keep me` and trailing comma still present | ✅ | +| 10 | Falls back to hint on un-patchable file | [src/dcode/cli.py#L283](src/dcode/cli.py#L283) | `test_falls_back_to_hint_on_unpatchable_file` | `[]` input → file unchanged, hint printed | ✅ | +| 11 | Adds `executeInWSLDistro` when `executeInWSL` already true | [src/dcode/cli.py#L274-L276](src/dcode/cli.py#L274-L276) | `test_adds_missing_distro_when_executeInWSL_already_true` | This was the original bug; now fixed and verified | ✅ | +| 12 | Malformed `devcontainer.json` → friendly stderr | [src/dcode/cli.py#L113-L122](src/dcode/cli.py#L113-L122) | `test_falls_back_on_malformed_devcontainer_json` | "failed to parse" in stderr, default workspace folder returned | ✅ | +| 13 | Non-zero exit propagated | [src/dcode/cli.py#L313-L315](src/dcode/cli.py#L313-L315), [#L323-L325](src/dcode/cli.py#L323-L325) | `test_propagates_nonzero_exit_from_code_launcher`, `test_propagates_nonzero_exit_with_devcontainer` | `SystemExit` with codes 2 and 3 both verified | ✅ | +| 14 | `[project.urls]` added | [pyproject.toml#L11-L14](pyproject.toml#L11-L14) | manual | Homepage / Issues / Source all point at `github.com/rosstaco/dcode` | ✅ | +| 15 | CI workflow runs ruff + pytest on py3.11/3.12/3.13 | [.github/workflows/ci.yml](.github/workflows/ci.yml) | n/a | Matrix correct; `fetch-depth: 0` set for hatch-vcs | ✅ | +| 16 | README WSL section | [README.md](README.md) | manual | Documents auto-edit, JSONC preservation, opt-out | ✅ | + +## Research ↔ Implementation Consistency + +The "research" for this task was the audit raised in chat. Cross-checking: + +- **Audit item: `subprocess.run` without `check=`** → Fixed; `check=False` explicit on all calls; rc propagated to `sys.exit` for editor invocations. ✅ +- **Audit item: `is_wsl()` redundant `os.path.exists`** → Cleaned up to use `Path.exists()`. ✅ +- **Audit item: `_ensure_wsl_docker_settings` distro skip bug** → Fixed by computing desired patches and only writing when actual diff exists. ✅ +- **Audit item: JSONC settings.json clobber** → Fixed via `_patch_jsonc_settings` with regex-based in-place edit + un-patchable fallback. ✅ +- **Audit item: `get_workspace_folder` no error handling** → Wrapped in try/except with friendly stderr message. ✅ +- **Audit item: Version duplicated** → Single-sourced via `importlib.metadata` + hatch-vcs. ✅ +- **Audit item: `_wsl_to_windows_path` ignores returncode** → Now checks `result.returncode == 0`. ✅ +- **Audit item: Bare `except Exception` in settings parse** → Narrowed to `(OSError, ValueError)`. ✅ +- **Audit item: String concat lint quirk** → Collapsed to single f-string with conditional `suffix`. ✅ +- **Audit item: No `__main__.py`** → Created. ✅ +- **Audit item: No `[project.urls]`** → Added. ✅ +- **Audit item: No ruff config** → `ruff.toml` added; `ruff` added to dev deps. ✅ +- **Audit item: No CI** → `ci.yml` added with proper matrix and `fetch-depth: 0`. ✅ +- **Audit item: No WSL docs in README** → Section added. ✅ + +Items intentionally deferred per user direction (still in audit but out of scope): +- Lower `requires-python` to 3.10 — N/A +- Drop `json5` — deferred per user (risk concern) +- Remove `copilot-session-*.md` — N/A + +## Test Quality Audit + +| Test File | Count | Issues | +|---|---|---| +| tests/test_cli.py | 39 | 1 minor: `test_resolves_via_importlib_metadata` is tautological (both sides call the same `version("dcode")` function); 1 gap: no test for `_patch_jsonc_settings` UPDATING an existing key value (only insertion + replacement-of-defaults are exercised) | + +**No anti-patterns found:** +- ✅ No assertion-free tests +- ✅ No mock-returns-what-you-assert +- ✅ Real fixture data via `tmp_path` (not invented) +- ✅ Tests use real text files and assert raw text where it matters (JSONC preservation) +- ✅ `_ok()` helper centralizes the `CompletedProcess(returncode=0)` pattern cleanly + +**Strengths:** +- The JSONC preservation test asserts on raw substrings (`"// keep me"`, `'"editor.fontSize": 14'`), proving the patcher actually preserved comments rather than trusting a json round-trip. +- `test_does_nothing_when_both_keys_already_correct` asserts byte-identical content — perfectly catches an unwanted rewrite. +- Returncode propagation tested for both the no-devcontainer path AND the devcontainer path. + +## End-to-End Smoke Test + +| Step | Command | Result | +|---|---|---| +| Tests | `uv run pytest -v` | 39 passed | +| Lint | `uv run ruff check` | All checks passed! | +| Build | `uv build` | wheel + sdist built; version `0.1.dev8+g5f9826397.d20260427` (correct: 8 commits past last unknown tag, dirty tree) | +| Module entrypoint | `uv run python -m dcode --help` | argparse help shown | +| Version introspection | `uv run python -c "import dcode; print(dcode.__version__)"` | `0.1.dev8+g5f9826397.d20260427` | +| Real `dcode ` against a devcontainer | NOT RUN | Would launch VS Code interactively | + +## Completeness Check + +- [x] Every file in plan's file list created (cli.py, __init__.py, __main__.py, pyproject.toml, ruff.toml, ci.yml, release-please.yml, release-please-config.json, .release-please-manifest.json, CHANGELOG.md, README.md updates, copilot-instructions.md update) +- [x] No dangling imports (verified by ruff) +- [x] No TODO/FIXME/HACK comments left behind +- [x] No hardcoded values that should be configurable +- [x] Documentation updated (README WSL section, copilot-instructions release flow) +- [x] `.gitignore` updated for `_version.py` and `.ruff_cache/` + +## Critical Failures + +**None.** + +## Gaps + +1. **Real-world install verification not done** (Acceptance #5). The flow `uv tool install git+https://github.com/rosstaco/dcode` after merging + tagging hasn't been exercised. Risk: low — `hatch-vcs` is a well-known plugin and `uv build` already produced a correct wheel locally. + +2. **No test for `_patch_jsonc_settings` updating an existing key value.** The replacement regex code path executes when one of the desired keys already exists with a *different* value. Closest test (`test_patches_settings_when_not_configured`) only inserts new keys; `test_adds_missing_distro_when_executeInWSL_already_true` exercises the value-already-correct skip plus insertion. Recommend adding a test where the file has `"dev.containers.executeInWSLDistro": "OldDistro"` and verifying it gets replaced with the current distro, comments preserved. + +3. **`TestVersion::test_resolves_via_importlib_metadata` is essentially tautological** — `dcode.__version__ == version("dcode")` is true by construction since `__init__.py` literally executes `version("dcode")`. It does verify the import wiring (no `ImportError`, no `PackageNotFoundError`), so it has *some* value as a smoke test, but doesn't verify the version is sane. Acceptable; not worth changing. + +## Test Quality Issues + +None of the OWASP Top 10 anti-patterns are present. The one minor concern is gap #2 above (untested code path). + +## Recommendations + +### Strongly recommended (before merge) + +1. **User must tag `v0.4.1` on the current `main` HEAD before merging this branch:** + ```bash + git tag v0.4.1 + git push --tags + ``` + Without this, the first `uv tool install` from `main` post-merge will produce a version like `0.0.0+gXXXXX` because hatch-vcs has no baseline. + +2. **Enable "Allow GitHub Actions to create and approve pull requests"** in repo settings → Actions → General. Required for release-please. + +### Nice-to-have (not blocking) + +3. Add the missing test for `_patch_jsonc_settings` updating an existing value (gap #2). +4. After first successful release-please cycle, do an actual `uv tool install --reinstall git+https://...` and confirm the version matches the tag. If something breaks, it'll be the `fetch-depth: 0` in CI vs. uv's default clone depth — both should work but worth confirming once. + +### Implementation notes accuracy correction + +Implementation-notes.md says "Manual macOS smoke test ✅" — only `python -m dcode --help` was actually run, not a real `dcode ` invocation. Marking the cell ⚠️ would be more honest. Not a code defect; just bookkeeping. + +## Plan tick-off + +The plan's [acceptance criteria checklist](plan.md) was a flat list of `- [ ]` items. Recommend the user (or a follow-up commit) ticks off items 1, 2, 3, 4, 7, 9, 10, 11, 12, 13, 14, 15, 16 (the 13 verified above) and leaves 5, 6, 8 unchecked since they require post-merge / operational steps.