From 051e42465b9c4a100237a918806216bb77af9b79 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Wed, 20 May 2026 17:15:30 -0500 Subject: [PATCH 1/2] validation(scaffold,companyos): reject regular-file target with clear message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an operator typo'd socrates init my-project --base ~/notes.txt or otherwise passed a path that exists as a regular file, scaffold() and scaffold_companyos() previously hit confusing late-stage errors: - scaffold() got "NotADirectoryError: [Errno 20] Not a directory" from inside the FILES touch loop — after target.mkdir(exist_ok=overwrite) silently no-op'd on the existing-as-file path. - scaffold_companyos() blew up inside any(target.iterdir()) because Path.iterdir on a file raises NotADirectoryError mid-condition. Both now check `target.is_file()` up-front and raise a clean NotADirectoryError with an actionable message. The file is untouched. Tests added (3): - scaffold() rejects file target — no side effects - scaffold(file, overwrite=True) STILL rejects (overwrite is for empty dirs, not for silently clobbering user files) - scaffold_companyos() symmetric guard 150/150 tests pass; ruff + mypy clean. --- src/socrates120x/companyos.py | 13 +++++++++++- src/socrates120x/scaffold.py | 9 +++++++++ tests/test_companyos.py | 14 +++++++++++++ tests/test_scaffold.py | 37 +++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/socrates120x/companyos.py b/src/socrates120x/companyos.py index 468adc3..360db25 100644 --- a/src/socrates120x/companyos.py +++ b/src/socrates120x/companyos.py @@ -27,7 +27,18 @@ def scaffold_companyos(target: Path, *, overwrite: bool = False) -> list[Path]: - """Create the CompanyOS macro layer at *target*. Returns files written.""" + """Create the CompanyOS macro layer at *target*. Returns files written. + + Raises NotADirectoryError if *target* is an existing regular file — + previously `any(target.iterdir())` cascaded into NotADirectoryError + from inside the boolean short-circuit, with a confusing traceback. + Reject explicitly with an actionable message. + """ + if target.exists() and target.is_file(): + raise NotADirectoryError( + f"Cannot scaffold CompanyOS into a regular file: {target}. " + f"Pass a directory path (it will be created if missing)." + ) if target.exists() and not overwrite and any(target.iterdir()): raise FileExistsError( f"Refusing to scaffold CompanyOS into non-empty path: {target}" diff --git a/src/socrates120x/scaffold.py b/src/socrates120x/scaffold.py index 190ca8d..8952c09 100644 --- a/src/socrates120x/scaffold.py +++ b/src/socrates120x/scaffold.py @@ -57,7 +57,16 @@ def scaffold(target: Path, *, overwrite: bool = False) -> list[Path]: Returns the list of files created (or that already existed). Raises FileExistsError if *target* already exists and ``overwrite`` is False. + Raises NotADirectoryError if *target* already exists as a regular file — + previously the call cascaded into a confusing "Cannot create directory" + error from inside the FILES/DIRS loop. Catch it up-front so the operator + gets an actionable message before any side effects. """ + if target.exists() and target.is_file(): + raise NotADirectoryError( + f"Cannot scaffold into a regular file: {target}. " + f"Pass a directory path (it will be created if missing)." + ) if target.exists() and not overwrite: raise FileExistsError(f"Refusing to overwrite existing path: {target}") diff --git a/tests/test_companyos.py b/tests/test_companyos.py index dc9dd1c..22f4518 100644 --- a/tests/test_companyos.py +++ b/tests/test_companyos.py @@ -49,3 +49,17 @@ def test_companyos_allows_empty_dir(tmp_path: Path) -> None: # Should succeed against an empty pre-existing dir. scaffold_companyos(target) assert (target / "AGENTS.md").is_file() + + +def test_companyos_rejects_file_target(tmp_path) -> None: + """Symmetric guard with scaffold(): passing a regular file path must + fail up-front, not midway through the per-file write loop.""" + import pytest + + from socrates120x.companyos import scaffold_companyos + + file_path = tmp_path / "co.txt" + file_path.write_text("operator's notes", encoding="utf-8") + with pytest.raises(NotADirectoryError, match="regular file"): + scaffold_companyos(file_path) + assert file_path.read_text(encoding="utf-8") == "operator's notes" diff --git a/tests/test_scaffold.py b/tests/test_scaffold.py index c54f28b..f2e2075 100644 --- a/tests/test_scaffold.py +++ b/tests/test_scaffold.py @@ -31,3 +31,40 @@ def test_scaffold_overwrite_flag_allows_reentry(tmp_path: Path) -> None: scaffold(target, overwrite=True) for f in FILES: assert (target / f).is_file() + + +# --------------------------------------------------------------------------- +# Target-is-a-file rejection +# (validation/scaffold-rejects-file-target) +# --------------------------------------------------------------------------- + + +def test_scaffold_rejects_file_target_with_clear_message(tmp_path) -> None: + """If the operator passes a path that is an existing regular file, + fail up-front with NotADirectoryError, not a confusing mid-scaffold + error from inside the directory-creation loop.""" + import pytest + + from socrates120x.scaffold import scaffold + + file_path = tmp_path / "not-a-dir.txt" + file_path.write_text("hi", encoding="utf-8") + with pytest.raises(NotADirectoryError, match="regular file"): + scaffold(file_path) + # And nothing got created next to it. + assert not (file_path.parent / "not-a-dir.txt" / "planning").exists() + + +def test_scaffold_overwrite_true_still_rejects_file_target(tmp_path) -> None: + """overwrite=True is for replacing an EMPTY directory; it must NOT + silently overwrite a regular file (which would be data loss).""" + import pytest + + from socrates120x.scaffold import scaffold + + file_path = tmp_path / "important.txt" + file_path.write_text("user content I would hate to lose", encoding="utf-8") + with pytest.raises(NotADirectoryError): + scaffold(file_path, overwrite=True) + # File must be untouched. + assert file_path.read_text(encoding="utf-8") == "user content I would hate to lose" From 5bc304a547e9adf6eb9c3bbb37065a8bb5698426 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Wed, 20 May 2026 18:25:35 -0500 Subject: [PATCH 2/2] validation: catch NotADirectoryError in _cmd_companyos too (#cli) Iter 12 added NotADirectoryError to scaffold_companyos but _cmd_companyos only caught FileExistsError, so passing a regular file path crashed with a stacktrace instead of returning 2 + a clean error message. Catch both errors in the same except clause. Test added: socrates companyos returns 2 with 'error:' on stderr and leaves the file untouched. --- src/socrates120x/cli.py | 5 ++++- tests/test_companyos.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/socrates120x/cli.py b/src/socrates120x/cli.py index dadf51d..1fe4a71 100644 --- a/src/socrates120x/cli.py +++ b/src/socrates120x/cli.py @@ -585,7 +585,10 @@ def _cmd_companyos(args: argparse.Namespace) -> int: target: Path = args.path.expanduser().resolve() try: written = scaffold_companyos(target) - except FileExistsError as e: + except (FileExistsError, NotADirectoryError) as e: + # scaffold_companyos can now raise NotADirectoryError when the + # target exists as a regular file. Catch it here so the CLI + # surfaces a clean error instead of a Python stacktrace. print(f"error: {e}", file=sys.stderr) return 2 print(f"Scaffolded CompanyOS at: {target}") diff --git a/tests/test_companyos.py b/tests/test_companyos.py index 22f4518..f46b71d 100644 --- a/tests/test_companyos.py +++ b/tests/test_companyos.py @@ -63,3 +63,19 @@ def test_companyos_rejects_file_target(tmp_path) -> None: with pytest.raises(NotADirectoryError, match="regular file"): scaffold_companyos(file_path) assert file_path.read_text(encoding="utf-8") == "operator's notes" + + +def test_cli_companyos_handles_file_target_gracefully(tmp_path, capsys) -> None: + """CLI entry point must catch NotADirectoryError too, not just FileExistsError. + Pre-fix the new validation in scaffold_companyos crashed the CLI with a + stacktrace because _cmd_companyos only caught FileExistsError.""" + from socrates120x.cli import main + + file_path = tmp_path / "operators-notes.txt" + file_path.write_text("user content", encoding="utf-8") + rc = main(["companyos", str(file_path)]) + assert rc == 2 + err = capsys.readouterr().err + assert "error:" in err + # The actual file content is untouched. + assert file_path.read_text(encoding="utf-8") == "user content"