Skip to content

Commit 8152248

Browse files
antosubashclaude
andauthored
fix(cli): tolerate safe leftovers in smpy new dest (#148) (#150)
`smpy new --dest .` was refusing any non-empty destination, so the common `gh repo create myapp && cd myapp && smpy new myapp --dest .` flow blew up the moment `.git/`, `.gitignore`, or `README.md` was present. Closes #148. Scaffolding now tolerates a small allowlist (`SAFE_PRESERVED_NAMES`: `.git/`, `.gitignore`, `.gitattributes`, `README*`, `LICENSE*`, `COPYING`, `CHANGELOG.md`, `CONTRIBUTING.md`, `CODE_OF_CONDUCT.md`, `.claude/`, `.vscode/`, `.idea/`, `.editorconfig`, `.DS_Store`) at the scaffold target. When the scaffold would write over a tolerated pre-existing file, the scaffold's copy is skipped — the user's file wins. Any other pre-existing entry still raises `FileExistsError`, naming the colliding files so the user knows what to move aside. `create_workspace` / `create_host` accept a `preserve_existing` set and return the list of preserved paths; `create_app_project` returns `(host_dir, preserved)`; `new.py` surfaces a `Preserved existing files` notice listing what was skipped so users can merge content in by hand. Three regression tests in test_cli_new_dest_tolerance.py cover: workspace mode tolerates a fresh `git init` layout (and user files survive), flat mode does the same, and unrelated files still hard-fail without being touched. https://claude.ai/code/session_01K84RjsX1ToXorMyuaxNBT6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent a466da6 commit 8152248

4 files changed

Lines changed: 194 additions & 41 deletions

File tree

‎framework/cli/simple_module_cli/app_project.py‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from simple_module_cli.catalog import CATALOG, PRESETS, expand_deps
2828
from simple_module_cli.recipes import RECIPES, ScaffoldCtx
2929
from simple_module_cli.scaffolding import (
30+
SAFE_PRESERVED_NAMES,
3031
_module_to_pypi_name,
3132
create_host,
3233
create_module,
@@ -97,38 +98,39 @@ def create_app_project(
9798
tenancy: bool = False,
9899
selected: Sequence[str] | None = None,
99100
flat: bool = False,
100-
) -> Path:
101+
) -> tuple[Path, list[Path]]:
101102
"""Greenfield ``simple-module new`` scaffold.
102103
103-
In workspace mode (the default), lays down a uv + npm workspace at
104-
``target/`` with the host under ``target/host/`` and a sample module
105-
under ``target/modules/hello/``. In flat mode (``flat=True``), keeps
106-
the legacy single-host layout: host files at ``target/`` with no
107-
``modules/`` directory or workspace plumbing.
104+
Workspace mode (default) lays down a uv + npm workspace at ``target/``
105+
with the host under ``target/host/`` and a sample module under
106+
``target/modules/hello/``. Flat mode keeps the legacy single-host layout.
107+
Tolerates ``SAFE_PRESERVED_NAMES`` at ``target`` (leftovers from
108+
``git init`` / ``gh repo create`` / IDE setup); other pre-existing
109+
entries raise ``FileExistsError``.
108110
109-
Generates a secret, picks a DB URL, rewrites the host's
110-
``pyproject.toml`` / the relevant ``package.json`` to pin exact
111-
framework versions, and applies any matching post-scaffold recipes
112-
(e.g. the ``background_tasks`` recipe drops a Celery worker stack).
113-
114-
Returns the host directory — ``target`` in flat mode, ``target/host``
115-
in workspace mode.
111+
Returns ``(host_dir, preserved)`` — the host directory (``target`` in
112+
flat mode, ``target/host`` in workspace mode) and the paths whose
113+
scaffold copy was skipped because the user already had one.
116114
"""
117-
if target.exists() and any(target.iterdir()):
118-
raise FileExistsError(
119-
f"Destination {target} already exists and is non-empty; "
120-
"choose a new path or remove its contents first."
121-
)
122-
123115
chosen = list(selected) if selected is not None else list(PRESETS["standard"])
124116
resolved, _added = expand_deps(chosen)
125117

126118
display_names = [to_pascal_case(CATALOG[m].display) for m in resolved]
127119
host_dir = target if flat else target / "host"
120+
preserved: list[Path] = []
128121
if not flat:
129-
target.mkdir(parents=True, exist_ok=True)
130-
create_workspace(target, name=name)
131-
create_host(host_dir, name=name, modules=display_names, framework_version=_FRAMEWORK_VERSION)
122+
preserved.extend(
123+
create_workspace(target, name=name, preserve_existing=SAFE_PRESERVED_NAMES)
124+
)
125+
preserved.extend(
126+
create_host(
127+
host_dir,
128+
name=name,
129+
modules=display_names,
130+
framework_version=_FRAMEWORK_VERSION,
131+
preserve_existing=SAFE_PRESERVED_NAMES if flat else frozenset(),
132+
)
133+
)
132134
if not flat:
133135
_strip_workspace_owned_files(host_dir)
134136

@@ -180,7 +182,7 @@ def create_app_project(
180182
if recipe_key is not None and recipe_key in RECIPES:
181183
RECIPES[recipe_key].apply(target, ctx)
182184

183-
return host_dir
185+
return host_dir, preserved
184186

185187

186188
def _strip_workspace_owned_files(host_dir: Path) -> None:

‎framework/cli/simple_module_cli/new.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ def new_project(
111111
raise typer.Exit(code=1) from None
112112

113113
try:
114-
host_dir = create_app_project(
114+
host_dir, preserved = create_app_project(
115115
target,
116116
name=name,
117117
db=db_final,
@@ -125,6 +125,14 @@ def new_project(
125125

126126
typer.echo(f"Created app '{name}' at {target}")
127127
typer.echo(f"Modules: {', '.join(resolved)}")
128+
if preserved:
129+
typer.echo(
130+
"\nPreserved existing files (scaffold's versions were skipped — "
131+
"merge by hand if you want their contents):"
132+
)
133+
for path in preserved:
134+
rel = path.relative_to(target) if path.is_relative_to(target) else path
135+
typer.echo(f" {rel}")
128136
typer.echo("\nNext steps:")
129137
typer.echo(f" cd {target}")
130138
if no_install:

‎framework/cli/simple_module_cli/scaffolding.py‎

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,28 @@
2828
validate_scaffold_name,
2929
)
3030

31-
__all__ = ["create_host", "create_module", "create_workspace"]
31+
__all__ = [
32+
"SAFE_PRESERVED_NAMES",
33+
"create_host",
34+
"create_module",
35+
"create_workspace",
36+
]
3237

3338
logger = logging.getLogger(__name__)
3439

3540
_TEMPLATES_PACKAGE = "simple_module_cli.templates"
3641
_PACKAGE_PATH_TOKEN = "__PACKAGE__"
3742

43+
# Pre-existing entries we tolerate at a scaffold target — typical leftovers
44+
# from ``git init`` / ``gh repo create`` / IDE setup.
45+
SAFE_PRESERVED_NAMES = frozenset(
46+
{".git", ".gitignore", ".gitattributes", ".editorconfig", ".DS_Store"}
47+
| {".claude", ".vscode", ".idea"}
48+
| {"README", "README.md", "README.rst"}
49+
| {"LICENSE", "LICENSE.md", "LICENSE.txt", "COPYING"}
50+
| {"CHANGELOG.md", "CONTRIBUTING.md", "CODE_OF_CONDUCT.md"}
51+
)
52+
3853

3954
def _module_to_pypi_name(name: str) -> str:
4055
return f"simple_module_{name.lower()}"
@@ -50,12 +65,21 @@ def _iter_template_files(template_root: Path):
5065
yield path
5166

5267

53-
def _require_empty_dest(dest: Path) -> None:
54-
if dest.exists() and any(dest.iterdir()):
55-
raise FileExistsError(
56-
f"Destination {dest} already exists and is non-empty. "
57-
"Choose a new path or remove the contents first."
58-
)
68+
def _require_empty_dest(dest: Path, *, preserve_existing: frozenset[str] = frozenset()) -> None:
69+
"""Refuse a non-empty destination unless every top-level entry is allowed.
70+
71+
``preserve_existing`` is matched against the *name* of each top-level entry,
72+
so callers can permit common pre-existing files (``.git``, ``README.md``,
73+
...) without silently overwriting unrelated user content.
74+
"""
75+
if dest.exists():
76+
unexpected = sorted(p.name for p in dest.iterdir() if p.name not in preserve_existing)
77+
if unexpected:
78+
raise FileExistsError(
79+
f"Destination {dest} exists and contains files that would collide "
80+
f"with the scaffold: {', '.join(unexpected)}. "
81+
"Move them aside or choose another path."
82+
)
5983
dest.mkdir(parents=True, exist_ok=True)
6084

6185

@@ -71,13 +95,20 @@ def _apply_template_files(
7195
substitutions: Mapping[str, str],
7296
*,
7397
path_rewrites: Mapping[str, str] | None = None,
74-
) -> None:
98+
preserve_existing: frozenset[str] = frozenset(),
99+
) -> list[Path]:
100+
"""Write template files into ``dest``; return paths skipped to preserve the user's copy."""
101+
preserved: list[Path] = []
75102
for src in _iter_template_files(src_root):
76103
rel_str = str(src.relative_to(src_root))
77104
for old, new in (path_rewrites or {}).items():
78105
rel_str = rel_str.replace(old, new)
79106
rel_str = rel_str.removesuffix(".tpl")
80107
target = dest / rel_str
108+
top = Path(rel_str).parts[0] if rel_str else ""
109+
if top in preserve_existing and target.exists():
110+
preserved.append(target)
111+
continue
81112
target.parent.mkdir(parents=True, exist_ok=True)
82113
if src.suffix == ".tpl":
83114
text = src.read_text(encoding="utf-8")
@@ -86,32 +117,41 @@ def _apply_template_files(
86117
target.write_text(text, encoding="utf-8")
87118
else:
88119
shutil.copy2(src, target)
120+
return preserved
89121

90122

91123
def create_workspace(
92124
dest: Path,
93125
name: str,
94126
template_root: Path | None = None,
95-
) -> Path:
96-
"""Materialize the workspace-root shell at ``dest``.
127+
*,
128+
preserve_existing: frozenset[str] = frozenset(),
129+
) -> list[Path]:
130+
"""Materialize the workspace-root shell at ``dest``; return preserved paths.
97131
98132
Lays down the top-level ``pyproject.toml`` (uv workspace), ``package.json``
99133
(npm workspace), ``Makefile`` (delegates to host), ``.env.example``,
100134
``.gitignore``, and ``README.md``. Does NOT create the host or any
101135
modules — those go under ``dest/host`` and ``dest/modules/`` afterwards.
136+
137+
``preserve_existing`` lists top-level entry names that may already exist
138+
in ``dest``; the scaffold's copy is skipped and the preserved path is
139+
included in the returned list. Other pre-existing entries raise
140+
``FileExistsError``.
102141
"""
103142
dest = Path(dest)
104-
dest.mkdir(parents=True, exist_ok=True)
105-
_apply_template_files(
143+
_require_empty_dest(dest, preserve_existing=preserve_existing)
144+
preserved = _apply_template_files(
106145
_resolve_template_root("workspace", template_root),
107146
dest,
108147
{
109148
"{{HOST_NAME}}": validate_scaffold_name(name),
110149
"{{HOST_PYPI_NAME}}": to_kebab_case(name),
111150
},
151+
preserve_existing=preserve_existing,
112152
)
113153
logger.info("Scaffolded workspace root at %s", dest)
114-
return dest
154+
return preserved
115155

116156

117157
def create_host(
@@ -120,11 +160,17 @@ def create_host(
120160
modules: Sequence[str],
121161
template_root: Path | None = None,
122162
framework_version: str = "*",
123-
) -> Path:
163+
*,
164+
preserve_existing: frozenset[str] = frozenset(),
165+
) -> list[Path]:
166+
"""Scaffold a host project at ``dest``; return preserved pre-existing paths.
167+
168+
``preserve_existing`` semantics match :func:`create_workspace`.
169+
"""
124170
dest = Path(dest)
125-
_require_empty_dest(dest)
171+
_require_empty_dest(dest, preserve_existing=preserve_existing)
126172
module_dep_lines = "\n".join(f' "{_module_to_pypi_name(m)}>=0.1,<1.0",' for m in modules)
127-
_apply_template_files(
173+
preserved = _apply_template_files(
128174
_resolve_template_root("host", template_root),
129175
dest,
130176
{
@@ -133,11 +179,12 @@ def create_host(
133179
"{{MODULE_DEPS}}": module_dep_lines,
134180
"{{FRAMEWORK_VERSION}}": framework_version,
135181
},
182+
preserve_existing=preserve_existing,
136183
)
137184
logger.info(
138185
"Scaffolded host '%s' at %s (modules: %s)", name, dest, ", ".join(modules) or "<none>"
139186
)
140-
return dest
187+
return preserved
141188

142189

143190
def create_module(
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
"""Regression tests for ``smpy new`` against a non-empty destination.
2+
3+
Issue #148: scaffolding into a directory that already contains a fresh
4+
``git init`` (or ``gh repo create``) layout — ``.git/``, ``.gitignore``,
5+
``README.md``, ``LICENSE`` — has to succeed without clobbering the user's
6+
files. Arbitrary pre-existing entries must still hard-fail so we don't
7+
overwrite real work.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from pathlib import Path
13+
14+
from simple_module_cli.cli import app
15+
from typer.testing import CliRunner
16+
17+
18+
def test_sm_new_tolerates_git_init_leftovers_in_dest(tmp_path: Path) -> None:
19+
"""``smpy new --dest .`` after ``git init`` / ``gh repo create`` must succeed,
20+
even with ``.git/``, ``.gitignore``, ``README.md``, ``LICENSE`` pre-existing —
21+
the scaffold preserves them and writes everything else."""
22+
runner = CliRunner()
23+
target = tmp_path / "demo"
24+
target.mkdir()
25+
(target / ".git").mkdir()
26+
user_gitignore = "*.pyc\n"
27+
(target / ".gitignore").write_text(user_gitignore)
28+
user_readme = "# demo (user-authored)\n"
29+
(target / "README.md").write_text(user_readme)
30+
(target / "LICENSE").write_text("MIT\n")
31+
32+
result = runner.invoke(
33+
app,
34+
["new", "demo", "--yes", "--db", "sqlite", "--no-install", "--dest", str(target)],
35+
)
36+
37+
assert result.exit_code == 0, result.output
38+
assert (target / ".git").is_dir()
39+
assert (target / ".gitignore").read_text() == user_gitignore
40+
assert (target / "README.md").read_text() == user_readme
41+
assert (target / "LICENSE").read_text() == "MIT\n"
42+
assert (target / "host" / "pyproject.toml").is_file()
43+
assert (target / "modules" / "hello").is_dir()
44+
assert "Preserved existing files" in result.output
45+
assert ".gitignore" in result.output
46+
assert "README.md" in result.output
47+
48+
49+
def test_sm_new_flat_tolerates_git_init_leftovers(tmp_path: Path) -> None:
50+
"""Flat mode (host-at-target) must also tolerate the safe allowlist,
51+
since `smpy new --flat --dest .` is the same flow against a fresh
52+
`git init`."""
53+
runner = CliRunner()
54+
target = tmp_path / "demo"
55+
target.mkdir()
56+
(target / ".git").mkdir()
57+
(target / ".gitignore").write_text("# user\n")
58+
59+
result = runner.invoke(
60+
app,
61+
[
62+
"new",
63+
"demo",
64+
"--yes",
65+
"--flat",
66+
"--db",
67+
"sqlite",
68+
"--no-install",
69+
"--dest",
70+
str(target),
71+
],
72+
)
73+
74+
assert result.exit_code == 0, result.output
75+
assert (target / ".gitignore").read_text() == "# user\n"
76+
assert (target / "pyproject.toml").is_file()
77+
78+
79+
def test_sm_new_still_refuses_unrelated_files_in_dest(tmp_path: Path) -> None:
80+
"""Tolerance is *narrow* — arbitrary pre-existing files (anything not in
81+
the safe allowlist) must still be a hard error so users don't accidentally
82+
scaffold over real work."""
83+
runner = CliRunner()
84+
target = tmp_path / "demo"
85+
target.mkdir()
86+
(target / "my_notes.md").write_text("don't clobber me")
87+
88+
result = runner.invoke(
89+
app,
90+
["new", "demo", "--yes", "--db", "sqlite", "--no-install", "--dest", str(target)],
91+
)
92+
93+
assert result.exit_code != 0
94+
output = result.output + (result.stderr or "")
95+
assert "my_notes.md" in output
96+
assert (target / "my_notes.md").read_text() == "don't clobber me"

0 commit comments

Comments
 (0)