From a50e71444f8380a6daec52e570a042b94fda904b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:41:29 +0000 Subject: [PATCH 1/4] Initial plan From 8e136f6c2d18add0236a90cefaf4c7ea9c9cbe71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:45:08 +0000 Subject: [PATCH 2/4] fix: replace placeholders, add path traversal checks, and strengthen empty-list tests Agent-Logs-Url: https://github.com/Project-Navi/navi-bootstrap/sessions/604f9871-000e-4f48-b728-1c2a85fe6c4c Co-authored-by: Fieldnote-Echo <202828230+Fieldnote-Echo@users.noreply.github.com> --- .claude/agents/pack-validator.md | 16 +++++++++------- src/navi_bootstrap/cli.py | 19 ++++++++++++++----- tests/test_hooks.py | 4 +++- tests/test_init.py | 1 + tests/test_validate.py | 4 +++- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/.claude/agents/pack-validator.md b/.claude/agents/pack-validator.md index 7a5311b..701de57 100644 --- a/.claude/agents/pack-validator.md +++ b/.claude/agents/pack-validator.md @@ -17,18 +17,20 @@ You are a pack-validation specialist for navi-bootstrap. Your job: confirm that 2. For each changed pack, run the full validation chain: ```bash - uv run nboot validate --spec nboot-spec.json - scratch=$(mktemp -d -t nboot-scratch-XXXX) - uv run nboot new "$scratch" - uv run nboot apply --spec nboot-spec.json --pack --target "$scratch" - uv run nboot diff --spec nboot-spec.json --pack --target "$scratch" + for PACK in $(git diff --name-only origin/main...HEAD | grep '^packs/' | cut -d'/' -f2 | sort -u); do + uv run nboot validate --spec nboot-spec.json + scratch=$(mktemp -d -t nboot-scratch-XXXX) + uv run nboot new "$scratch" + uv run nboot apply --spec nboot-spec.json --pack "$PACK" --target "$scratch" + uv run nboot diff --spec nboot-spec.json --pack "$PACK" --target "$scratch" + done ``` 3. Run the pack-specific test: ```bash - pack_snake=$(echo | tr '-' '_') - uv run pytest tests/test_${pack_snake}_pack.py -v 2>/dev/null || \ + pack_snake=$(echo "$PACK" | tr '-' '_') + uv run pytest tests/test_${pack_snake}_pack.py -v || \ uv run pytest tests/ -k "$pack_snake" -v ``` diff --git a/src/navi_bootstrap/cli.py b/src/navi_bootstrap/cli.py index 3c2f75f..64ff6ed 100644 --- a/src/navi_bootstrap/cli.py +++ b/src/navi_bootstrap/cli.py @@ -116,12 +116,20 @@ def render_cmd( if out is None: name = spec_data["name"] - if not name or "/" in name or "\\" in name: + stripped_name = name.strip() if isinstance(name, str) else "" + if ( + not stripped_name + or "/" in stripped_name + or "\\" in stripped_name + or ".." in stripped_name + or stripped_name.startswith(".") + ): raise click.ClickException( f"Unsafe spec name {name!r} cannot be used as output directory. " + "Name must be a non-empty single path segment without '.' or '..' components. " "Use --out to specify an explicit output path." ) - output_dir = Path(name) + output_dir = Path(stripped_name) else: output_dir = out @@ -459,12 +467,13 @@ def new( ) -> None: """Create a new Python project with operational infrastructure.""" # Validate name before using as path - if not name or "/" in name or "\\" in name or name.startswith("."): + stripped_name = name.strip() if name else "" + if not stripped_name or "/" in stripped_name or "\\" in stripped_name or ".." in stripped_name or stripped_name.startswith("."): raise click.ClickException( f"Unsafe project name {name!r}. " - "Names must not contain path separators or start with a dot." + "Names must not contain path separators, '..', or start with a dot." ) - output_dir = Path(name) + output_dir = Path(stripped_name) if output_dir.exists(): raise click.ClickException( f"Directory {name!r} already exists. nboot new is for greenfield projects only." diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 8dfadcc..fdb39fb 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -29,9 +29,11 @@ def test_reports_failures_without_stopping(self, mock_run: MagicMock, tmp_path: assert not results[0].success assert results[1].success - def test_empty_hooks(self, tmp_path: Path) -> None: + @patch("navi_bootstrap.hooks.subprocess.run") + def test_empty_hooks(self, mock_run: MagicMock, tmp_path: Path) -> None: results = run_hooks([], tmp_path) assert results == [] + mock_run.assert_not_called() @patch("navi_bootstrap.hooks.subprocess.run") def test_captures_output(self, mock_run: MagicMock, tmp_path: Path) -> None: diff --git a/tests/test_init.py b/tests/test_init.py index a64a8da..fcffe69 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -455,6 +455,7 @@ def test_detects_async_test_functions(self, tmp_path: Path) -> None: test_file = tests / "test_example.py" test_file.write_text("def test_sync(): pass\nasync def test_async(): pass\n") result = detect_test_info(tmp_path) + assert result["test_framework"] == "pytest" assert result["test_count"] == 2 diff --git a/tests/test_validate.py b/tests/test_validate.py index 104f683..78f1459 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -41,9 +41,11 @@ def test_warnings_accepted(self, mock_run: MagicMock, tmp_path: Path) -> None: assert len(results) == 1 assert results[0].passed - def test_empty_validations(self, tmp_path: Path) -> None: + @patch("navi_bootstrap.validate.subprocess.run") + def test_empty_validations(self, mock_run: MagicMock, tmp_path: Path) -> None: results = run_validations([], tmp_path) assert results == [] + mock_run.assert_not_called() @patch("navi_bootstrap.validate.subprocess.run") def test_skips_method_based_validations(self, mock_run: MagicMock, tmp_path: Path) -> None: From 9adb152d9569252cb0a741be68fb7c0e6656408c Mon Sep 17 00:00:00 2001 From: Nelson Spence Date: Sun, 26 Apr 2026 15:21:49 -0500 Subject: [PATCH 3/4] style: ruff-format the new-cmd validation if-chain (one-liner -> multi-line) The single-line if chain in new() (added in this PR) exceeds ruff-format's preferred wrapping. Auto-applying `uv run ruff format` to clear the `lint` (ruff format --check) and `quality-gate` failures on PR #52. No semantic change. 381 tests still pass, mypy strict clean, both ruff check and ruff format clean. --- src/navi_bootstrap/cli.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/navi_bootstrap/cli.py b/src/navi_bootstrap/cli.py index 64ff6ed..7ecc253 100644 --- a/src/navi_bootstrap/cli.py +++ b/src/navi_bootstrap/cli.py @@ -468,7 +468,13 @@ def new( """Create a new Python project with operational infrastructure.""" # Validate name before using as path stripped_name = name.strip() if name else "" - if not stripped_name or "/" in stripped_name or "\\" in stripped_name or ".." in stripped_name or stripped_name.startswith("."): + if ( + not stripped_name + or "/" in stripped_name + or "\\" in stripped_name + or ".." in stripped_name + or stripped_name.startswith(".") + ): raise click.ClickException( f"Unsafe project name {name!r}. " "Names must not contain path separators, '..', or start with a dot." From 765a9a3781cdb9adb94cf4b9fb5a9092be544658 Mon Sep 17 00:00:00 2001 From: Nelson Spence Date: Sun, 26 Apr 2026 15:32:52 -0500 Subject: [PATCH 4/4] fix: address Codex P1 + Copilot findings on PR #52 1. [Codex P1 + Copilot] pack-validator.md: pack-specific pytest step was outside the per-pack `for PACK ... done` loop, so $PACK held only the LAST iteration's value and earlier packs' tests were silently skipped. A regression in earlier-changed packs would slip through the validator. Moved the test inside the same loop body and renumbered the cross-cutting tests as step 3. 2. [Copilot] cli.py render_cmd error message: previous wording ('without dot or dot-dot components') didn't match the actual checks (`startswith('.')` rejects '.foo'; `'..' in name` substring rejects 'foo..bar'). Reworded to match the rules: 'must not start with .', 'must not contain .. or path separators'. Skipped from the same review batch: - Copilot's note that PR description claims apply/diff hardening: only render and new construct Path-from-name; apply/diff take --target as a Click Path. PR description should be updated but code is correct. 381 tests still pass, mypy strict clean, ruff + format clean. --- .claude/agents/pack-validator.md | 20 ++++++++++---------- src/navi_bootstrap/cli.py | 3 ++- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.claude/agents/pack-validator.md b/.claude/agents/pack-validator.md index 701de57..0b39ee2 100644 --- a/.claude/agents/pack-validator.md +++ b/.claude/agents/pack-validator.md @@ -14,7 +14,10 @@ You are a pack-validation specialist for navi-bootstrap. Your job: confirm that git diff --name-only origin/main...HEAD | grep '^packs/' | cut -d'/' -f2 | sort -u ``` -2. For each changed pack, run the full validation chain: +2. For each changed pack, run the full validation chain **including** its + pack-specific tests inside the same loop body — otherwise `pack_snake` + takes the value of the last iteration only and earlier packs' tests are + silently skipped: ```bash for PACK in $(git diff --name-only origin/main...HEAD | grep '^packs/' | cut -d'/' -f2 | sort -u); do @@ -23,18 +26,15 @@ You are a pack-validation specialist for navi-bootstrap. Your job: confirm that uv run nboot new "$scratch" uv run nboot apply --spec nboot-spec.json --pack "$PACK" --target "$scratch" uv run nboot diff --spec nboot-spec.json --pack "$PACK" --target "$scratch" - done - ``` - -3. Run the pack-specific test: - ```bash - pack_snake=$(echo "$PACK" | tr '-' '_') - uv run pytest tests/test_${pack_snake}_pack.py -v || \ - uv run pytest tests/ -k "$pack_snake" -v + # Pack-specific test for this iteration's pack + pack_snake=$(echo "$PACK" | tr '-' '_') + uv run pytest tests/test_${pack_snake}_pack.py -v || \ + uv run pytest tests/ -k "$pack_snake" -v + done ``` -4. Run cross-cutting tests that commonly break on pack changes: +3. Run cross-cutting tests that commonly break on pack changes: ```bash uv run pytest tests/test_engine.py tests/test_manifest.py tests/test_integration.py -v diff --git a/src/navi_bootstrap/cli.py b/src/navi_bootstrap/cli.py index 7ecc253..c685ffa 100644 --- a/src/navi_bootstrap/cli.py +++ b/src/navi_bootstrap/cli.py @@ -126,7 +126,8 @@ def render_cmd( ): raise click.ClickException( f"Unsafe spec name {name!r} cannot be used as output directory. " - "Name must be a non-empty single path segment without '.' or '..' components. " + "Name must be a non-empty single path segment, must not start with " + "'.', and must not contain '..' or path separators. " "Use --out to specify an explicit output path." ) output_dir = Path(stripped_name)