diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c4d90ac..a0373944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- `mapify init --provider codex` on a project that already has `.map/scripts/` + now refreshes the shipped runtime scripts with the same policy as the Claude + provider (overwrite, `.bak.` on drift, project-added files untouched) + instead of skipping the whole directory — so `mapify _update` delivers + runtime fixes to Codex-only installs. A symlinked `.map` or `.map/scripts` + is rejected with a plain `Error:` line before any write. (#461) + ## [3.30.0] - 2026-09-15 ### Added diff --git a/src/mapify_cli/__init__.py b/src/mapify_cli/__init__.py index 74d59fa3..26294955 100755 --- a/src/mapify_cli/__init__.py +++ b/src/mapify_cli/__init__.py @@ -1495,13 +1495,18 @@ def require_feature_gitignore(merger: Any, feature: str) -> None: raise typer.Exit(1) from exc if provider == "codex": - # Codex provider: install .agents/.codex files + .map/scripts/ (skip-if-exists) + # Codex provider: install .agents/.codex files + .map/scripts/ + # (.map/scripts policy is documented on create_codex_files) from mapify_cli.delivery.providers import CodexProvider tracker.add("create-codex", "Create Codex files") tracker.start("create-codex") codex_provider = CodexProvider() - counts = codex_provider.install(project_path) + try: + counts = codex_provider.install(project_path) + except RuntimeError as exc: + console.print(f"[red]Error:[/red] {exc}") + raise typer.Exit(1) from exc total = sum(counts.values()) tracker.complete("create-codex", f"{total} files") diff --git a/src/mapify_cli/delivery/codex_copier.py b/src/mapify_cli/delivery/codex_copier.py index 589fcce2..07733144 100644 --- a/src/mapify_cli/delivery/codex_copier.py +++ b/src/mapify_cli/delivery/codex_copier.py @@ -15,6 +15,9 @@ from typing import Any from mapify_cli.delivery.file_copier import ( + _IGNORED_TEMPLATE_NAMES, + _IGNORED_TEMPLATE_SUFFIXES, + _copy_map_path, _extract_requires_block, _get_version, _load_template_skill_catalog, @@ -65,14 +68,12 @@ def _copy_tree( """ count = 0 dst_dir.mkdir(parents=True, exist_ok=True) - ignored_names = {"__pycache__", ".DS_Store"} - ignored_suffixes = {".pyc", ".pyo"} for src_file in src_dir.rglob("*"): if not src_file.is_file(): continue - if any(part in ignored_names for part in src_file.parts): + if any(part in _IGNORED_TEMPLATE_NAMES for part in src_file.parts): continue - if src_file.suffix in ignored_suffixes: + if src_file.suffix in _IGNORED_TEMPLATE_SUFFIXES: continue rel = src_file.relative_to(src_dir) target = dst_dir / rel @@ -90,6 +91,20 @@ def _copy_tree( _EXEC_SUFFIXES = frozenset((".py", ".sh")) +def _first_symlink_component(root: Path, dest: Path) -> Path | None: + """Return the first component of *dest* below *root* that is a symlink. + + Checked component by component (``.map``, then ``.map/scripts``) so a + linked ancestor cannot redirect the runtime install outside *root*. + """ + current = root + for part in dest.relative_to(root).parts: + current = current / part + if current.is_symlink(): + return current + return None + + def _managed_codex_hook_names(hooks_dir_src: Path) -> frozenset[str]: """Names of the scripts mapify ships into .codex/hooks/ — the MAP-owned set.""" if not hooks_dir_src.is_dir(): @@ -250,9 +265,11 @@ def create_codex_files(project_path: Path) -> dict[str, int]: Watched files (skills, agents, config, AGENTS.md, hooks) are installed fence-aware so a re-install preserves any user content below the fence; hooks.json is merged without MAP metadata because Codex validates top-level - keys strictly; .map/scripts is MAP-owned (fenced=False, skip-if-exists). + keys strictly; .map/scripts is MAP-owned and refreshed exactly like the + Claude provider's tree (``_copy_map_path``: shipped scripts are overwritten, + a drifted managed copy is backed up to ``.bak.`` first, project-added + files are left alone). - Skips .map/scripts/ if the directory already exists. Never creates or modifies any .claude/ path. Args: @@ -264,6 +281,8 @@ def create_codex_files(project_path: Path) -> dict[str, int]: """ templates_dir = get_templates_dir() codex_templates = templates_dir / "codex" + map_scripts_src = templates_dir / "map" / "scripts" + map_scripts_dst = project_path / ".map" / "scripts" empty_counts: dict[str, int] = { "skills": 0, @@ -277,6 +296,14 @@ def create_codex_files(project_path: Path) -> dict[str, int]: if not codex_templates.exists(): return empty_counts + if map_scripts_src.exists(): + link = _first_symlink_component(project_path, map_scripts_dst) + if link is not None: + raise RuntimeError( + f"{link} is a symbolic link; mapify will not install .map/scripts " + "through it. Replace the link with a real directory and re-run." + ) + counts: dict[str, int] = dict(empty_counts) codex_dir = project_path / ".codex" agents_dir = project_path / ".agents" @@ -391,19 +418,11 @@ def create_codex_files(project_path: Path) -> dict[str, int]: counts["docs"] += 1 # ------------------------------------------------------------------ - # 6. .map/scripts/ — skip-if-exists (do not overwrite user scripts) - # MAP-owned: install fenced=False (no fence) when absent. + # 6. .map/scripts/ — MAP-owned, same policy as the Claude provider: + # shipped scripts are refreshed (.bak. on drift), project-added + # files are never touched. # ------------------------------------------------------------------ - map_scripts_dst = project_path / ".map" / "scripts" - if not map_scripts_dst.exists(): - map_scripts_src = templates_dir / "map" / "scripts" - if map_scripts_src.exists(): - counts["scripts"] = _copy_tree( - map_scripts_src, - map_scripts_dst, - version, - fenced=False, - executable_suffixes=_EXEC_SUFFIXES, - ) + if map_scripts_src.exists(): + counts["scripts"] = _copy_map_path(map_scripts_src, map_scripts_dst, version) return counts diff --git a/tests/test_mapify_cli.py b/tests/test_mapify_cli.py index a35c47d4..f59b6ac9 100644 --- a/tests/test_mapify_cli.py +++ b/tests/test_mapify_cli.py @@ -49,6 +49,10 @@ check_and_update, ) from mapify_cli.delivery import create_map_tools +from mapify_cli.delivery.file_copier import ( + _IGNORED_TEMPLATE_NAMES, + _IGNORED_TEMPLATE_SUFFIXES, +) from mapify_cli.install_manifest import read_manifest from mapify_cli.update_install import ( InstallKind, @@ -4982,6 +4986,17 @@ def codex_project(self, tmp_path): ) return tmp_path + @staticmethod + def _delivered_tree_inventory(root: Path) -> set[Path]: + """Return files copied by the Codex tree copier's standard filters.""" + return { + path.relative_to(root) + for path in root.rglob("*") + if path.is_file() + and not any(part in _IGNORED_TEMPLATE_NAMES for part in path.parts) + and path.suffix not in _IGNORED_TEMPLATE_SUFFIXES + } + # ------------------------------------------------------------------ # # AC-1: .agents/skills/map-plan/SKILL.md created # # ------------------------------------------------------------------ # @@ -5080,36 +5095,136 @@ def test_ac05_creates_config_and_agents(self, codex_project): ) # ------------------------------------------------------------------ # - # AC-6: .map/scripts/ installed (or skipped if already present) # + # AC-6: complete provider inventory and Claude-parity script refresh # # ------------------------------------------------------------------ # - def test_ac06_map_scripts_installed_or_skipped(self, codex_project, tmp_path): - """AC-6: .map/scripts/ installed when absent, pre-existing files preserved.""" + def test_ac06_clean_install_has_complete_provider_inventory(self, codex_project): + """AC-6: a clean Codex install contains every shipped provider file.""" + templates = get_templates_dir() map_scripts = codex_project / ".map" / "scripts" + agents_skills = codex_project / ".agents" / "skills" + codex_dir = codex_project / ".codex" + + expected_scripts = self._delivered_tree_inventory(templates / "map" / "scripts") + expected_skills = self._delivered_tree_inventory(templates / "codex" / "skills") + expected_codex = self._delivered_tree_inventory(templates / "codex") + expected_codex = { + path + for path in expected_codex + if path.parts[0] not in {"skills", "references"} + and path != Path("AGENTS.md") + } + + assert Path("map_step_runner.py") in expected_scripts + assert self._delivered_tree_inventory(map_scripts) == expected_scripts + assert self._delivered_tree_inventory(agents_skills) == expected_skills + assert self._delivered_tree_inventory(codex_dir) == expected_codex + + def test_ac06_existing_map_scripts_are_refreshed_like_claude(self, tmp_path): + """AC-6: a partial .map/scripts is completed and stale shipped scripts refreshed. + + Same policy as the Claude provider's ``_copy_map_path``: shipped names are + overwritten (a drifted managed copy gets a ``.bak.`` first), files the + template does not ship are never touched. + """ templates_scripts = get_templates_dir() / "map" / "scripts" - if templates_scripts.exists() and any(templates_scripts.iterdir()): - assert map_scripts.exists(), ( - ".map/scripts/ must exist when template provides scripts" - ) + expected_scripts = self._delivered_tree_inventory(templates_scripts) + shipped_runner = (templates_scripts / "map_step_runner.py").read_bytes() - # Verify skip-if-exists: pre-existing custom scripts survive codex init - project2 = tmp_path / "skip_test" - project2.mkdir() - scripts_dir = project2 / ".map" / "scripts" + project = tmp_path / "partial_install" + project.mkdir() + scripts_dir = project / ".map" / "scripts" scripts_dir.mkdir(parents=True) + stale_script = scripts_dir / "map_step_runner.py" + stale_bytes = b"#!/usr/bin/env python3\n# stale pre-upgrade runtime\n" + stale_script.write_bytes(stale_bytes) custom_script = scripts_dir / "custom.py" - custom_script.write_text("# user custom script\n") + custom_bytes = b"# user custom script\n" + custom_script.write_bytes(custom_bytes) runner2 = CliRunner() - os.chdir(project2) - result = runner2.invoke( - app, ["init", ".", "--provider", "codex", "--no-git", "--force"] - ) + os.chdir(project) + init_args = ["init", ".", "--provider", "codex", "--no-git", "--force"] + result = runner2.invoke(app, init_args) assert result.exit_code == 0, f"init failed: {result.output}" - assert custom_script.exists(), ( - ".map/scripts/custom.py must survive codex init (skip-if-exists)" + refreshed = stale_script.read_bytes() + assert refreshed != stale_bytes, "stale shipped script must be refreshed" + # The managed install injects a MAP-MANAGED header right after the + # shebang, so compare the shipped body below the shebang line. + assert b"MAP-MANAGED" in refreshed + assert shipped_runner.split(b"\n", 1)[1].strip() in refreshed, ( + "refreshed script must carry the shipped runtime body" + ) + assert custom_script.read_bytes() == custom_bytes + assert self._delivered_tree_inventory(scripts_dir) == expected_scripts | { + Path("custom.py") + } + + # A user edit on the managed copy is backed up, then refreshed (drift). + stale_script.write_bytes(refreshed + b"\n# local edit on managed copy\n") + result = runner2.invoke(app, init_args + ["--refresh-existing"]) + assert result.exit_code == 0, f"refresh failed: {result.output}" + assert b"local edit on managed copy" not in stale_script.read_bytes() + backups = sorted(scripts_dir.glob("map_step_runner.py.*.bak")) + assert len(backups) == 1, "drifted managed script must be backed up once" + assert b"local edit on managed copy" in backups[0].read_bytes() + assert custom_script.read_bytes() == custom_bytes + + def test_ac06_symlinked_map_scripts_is_rejected_without_external_writes( + self, tmp_path + ): + """AC-6: runtime install must never follow a project scripts symlink.""" + project = tmp_path / "symlinked_scripts" + project.mkdir() + external_scripts = tmp_path / "external_scripts" + external_scripts.mkdir() + sentinel = external_scripts / "sentinel.bin" + sentinel.write_bytes(b"external-runtime-sentinel\x00\xff") + + def snapshot() -> dict[Path, bytes]: + return { + path.relative_to(external_scripts): path.read_bytes() + for path in external_scripts.rglob("*") + if path.is_file() + } + + before = snapshot() + + scripts_dir = project / ".map" / "scripts" + scripts_dir.parent.mkdir() + try: + scripts_dir.symlink_to(external_scripts, target_is_directory=True) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"directory symlinks unavailable: {exc}") + + runner2 = CliRunner() + os.chdir(project) + result = runner2.invoke( + app, + [ + "init", + ".", + "--provider", + "codex", + "--no-git", + "--mcp", + "none", + "--force", + ], ) - assert custom_script.read_text() == "# user custom script\n" + + # A plain `Error:` line and exit 1 -- no uncaught traceback (typer.Exit). + assert result.exit_code == 1 + assert isinstance(result.exception, SystemExit) + flat_output = " ".join(result.output.split()) # rich wraps at 80 cols + assert "Error:" in flat_output + assert "is a symbolic link" in flat_output + assert "Replace the link with a real directory" in flat_output + assert "Traceback" not in flat_output + assert scripts_dir.is_symlink() + after = snapshot() + assert after == before + assert set(after) == {Path("sentinel.bin")} # ------------------------------------------------------------------ # # AC-7: Default init (no --provider) creates .claude/, not .codex/ # @@ -5119,9 +5234,10 @@ def test_ac07_default_init_unchanged(self, tmp_path): """AC-7: 'init .' without --provider must create .claude/ and not .codex/.""" local_runner = CliRunner() os.chdir(tmp_path) - result = local_runner.invoke( - app, ["init", ".", "--no-git", "--mcp", "none", "--force"] - ) + with mock.patch("mapify_cli.configure_global_permissions"): + result = local_runner.invoke( + app, ["init", ".", "--no-git", "--mcp", "none", "--force"] + ) assert result.exit_code == 0, f"Default init failed:\n{result.output}" assert (tmp_path / ".claude").exists(), ( ".claude/ must exist for default provider" @@ -5477,7 +5593,9 @@ def test_ac18_hooks_match_codex_canonical_tools(self, codex_project): matchers = [entry.get("matcher") for entry in pre_tool_use] assert "Bash|apply_patch" in matchers combined = next( - entry for entry in pre_tool_use if entry.get("matcher") == "Bash|apply_patch" + entry + for entry in pre_tool_use + if entry.get("matcher") == "Bash|apply_patch" ) commands = [hook["command"] for hook in combined["hooks"]] assert any("workflow-gate.py" in command for command in commands) @@ -5579,10 +5697,13 @@ def test_ac18b_hooks_json_merges_existing_project_hooks(self, tmp_path): for hook in entry.get("hooks", []) if isinstance(hook, dict) ] - assert sum( - ".codex/hooks/workflow-gate.py" in command - for command in all_pre_commands - ) == 1 + assert ( + sum( + ".codex/hooks/workflow-gate.py" in command + for command in all_pre_commands + ) + == 1 + ) assert any( entry.get("matcher") == "Read" and entry["hooks"][0]["command"] == "echo read" @@ -5704,9 +5825,7 @@ def test_ac20_workflow_gate_blocks_during_restricted(self, codex_project): ) assert hook_output.get("hookEventName") == "PreToolUse" - def test_ac20b_codex_safety_hook_blocks_apply_patch_secret( - self, codex_project - ): + def test_ac20b_codex_safety_hook_blocks_apply_patch_secret(self, codex_project): """Codex apply_patch paths pass through the shared sensitive-file gate.""" safety_script = codex_project / ".codex" / "hooks" / "safety-guardrails.py" payload = json.dumps( @@ -5714,9 +5833,7 @@ def test_ac20b_codex_safety_hook_blocks_apply_patch_secret( "hook_event_name": "PreToolUse", "tool_name": "apply_patch", "tool_input": { - "command": "*** Begin Patch\n" - "*** Update File: .env\n" - "*** End Patch\n" + "command": "*** Begin Patch\n*** Update File: .env\n*** End Patch\n" }, } ) @@ -5735,9 +5852,7 @@ def test_ac20b_codex_safety_hook_blocks_apply_patch_secret( assert output["hookEventName"] == "PreToolUse" assert output["permissionDecision"] == "deny" - def test_ac20c_codex_compaction_context_uses_session_start( - self, codex_project - ): + def test_ac20c_codex_compaction_context_uses_session_start(self, codex_project): """Post-compaction context uses SessionStart's supported context channel.""" branch_dir = codex_project / ".map" / "default" branch_dir.mkdir(parents=True, exist_ok=True) @@ -5756,9 +5871,7 @@ def test_ac20c_codex_compaction_context_uses_session_start( proc = subprocess.run( [sys.executable, str(hook)], - input=json.dumps( - {"hook_event_name": "SessionStart", "source": "compact"} - ), + input=json.dumps({"hook_event_name": "SessionStart", "source": "compact"}), capture_output=True, text=True, cwd=codex_project, @@ -5776,7 +5889,9 @@ def test_ac20c2_memory_hooks_use_installed_runtime_from_isolated_python( ): """SessionStart and Stop memory work without importing from the project.""" hooks_dir = codex_project / ".codex" / "hooks" - mapify_executable = Path(__file__).resolve().parents[1] / ".venv" / "bin" / "mapify" + mapify_executable = ( + Path(__file__).resolve().parents[1] / ".venv" / "bin" / "mapify" + ) assert mapify_executable.is_file() env = os.environ.copy() env.pop("PYTHONPATH", None) @@ -5844,13 +5959,11 @@ def test_ac20c2_memory_hooks_use_installed_runtime_from_isolated_python( check=False, ) assert session.returncode == 0, session.stderr - context = json.loads(session.stdout)["hookSpecificOutput"][ - "additionalContext" - ] + context = json.loads(session.stdout)["hookSpecificOutput"]["additionalContext"] assert "isolated runtime recall marker" in context - assert "from mapify_cli" not in ( - hooks_dir / "map-memory-session.py" - ).read_text(encoding="utf-8") + assert "from mapify_cli" not in (hooks_dir / "map-memory-session.py").read_text( + encoding="utf-8" + ) def test_ac20d_codex_bash_write_is_phase_gated(self, codex_project): """A shell redirection cannot bypass the RESEARCH mutation gate.""" @@ -5930,9 +6043,7 @@ def _run_codex_gate(codex_project, command: str, phase: str = "RESEARCH") -> str "true\nprintf x >> src.py", ], ) - def test_ac20d2_explicit_bash_targets_are_phase_gated( - self, codex_project, command - ): + def test_ac20d2_explicit_bash_targets_are_phase_gated(self, codex_project, command): """Every explicit write target, on any line, goes through the phase gate.""" assert self._run_codex_gate(codex_project, command) == "deny" @@ -5942,13 +6053,13 @@ def test_ac20d2_explicit_bash_targets_are_phase_gated( # The orchestrator's own commands, verbatim from the shipped skills. "SUBTASK_ID=$(jq -r '.current_subtask_id' \".map/default/step_state.json\")", "NEXT_STEP=$(python3 .map/scripts/map_orchestrator.py get_next_step)", - "MAP_CONTEXT=$(python3 .map/scripts/map_step_runner.py build_context_block \"$BRANCH\" \"$SUBTASK_ID\")", + 'MAP_CONTEXT=$(python3 .map/scripts/map_step_runner.py build_context_block "$BRANCH" "$SUBTASK_ID")', "TEST_OUTPUT=$(pytest --tb=short 2>&1) || true", "TEST_OUTPUT=$(go test ./... 2>&1) || true", "pytest -q", "make test 2>/dev/null", "date -u +%Y-%m-%dT%H:%M:%SZ", - "printf '%s' \"$RESEARCH_FINDINGS\" | python3 .map/scripts/map_step_runner.py save_research \"$BRANCH\" \"$SUBTASK_ID\"", + 'printf \'%s\' "$RESEARCH_FINDINGS" | python3 .map/scripts/map_step_runner.py save_research "$BRANCH" "$SUBTASK_ID"', # Writes into .map/ (exempt) and outside the repo (orthogonal). "printf x > .map/default/notes.md", "printf x > /tmp/map-safe.txt", @@ -5965,8 +6076,8 @@ def test_ac20d3_orchestrator_bash_is_allowed_in_every_phase( @pytest.mark.parametrize( "command", [ - 'python3 -c "open(\'src.py\', \'w\').write(\'x\')"', - 'ruby -e "File.write(\'src.py\', \'x\')"', + "python3 -c \"open('src.py', 'w').write('x')\"", + "ruby -e \"File.write('src.py', 'x')\"", 'echo "$(touch src.py)"', "cat <(touch src.py)", ], @@ -5991,9 +6102,7 @@ def test_ac20e_configured_hook_command_runs_without_git(self, codex_project): proc = subprocess.run( ["bash", "-lc", command], - input=json.dumps( - {"tool_name": "Bash", "tool_input": {"command": "pwd"}} - ), + input=json.dumps({"tool_name": "Bash", "tool_input": {"command": "pwd"}}), capture_output=True, text=True, cwd=codex_project, @@ -6011,9 +6120,7 @@ def test_ac20e_configured_hook_command_runs_without_git(self, codex_project): 'echo unsafe > "src".py', ], ) - def test_ac20e2_expanded_bash_targets_fail_closed( - self, codex_project, command - ): + def test_ac20e2_expanded_bash_targets_fail_closed(self, codex_project, command): """Variable/glob targets cannot be misclassified as orthogonal.""" branch_dir = codex_project / ".map" / "default" branch_dir.mkdir(parents=True, exist_ok=True) @@ -6027,13 +6134,7 @@ def test_ac20e2_expanded_bash_targets_fail_closed( encoding="utf-8", ) (branch_dir / "blueprint.json").write_text( - json.dumps( - { - "subtasks": [ - {"id": "ST-1", "affected_files": ["src.py"]} - ] - } - ), + json.dumps({"subtasks": [{"id": "ST-1", "affected_files": ["src.py"]}]}), encoding="utf-8", ) hook = codex_project / ".codex" / "hooks" / "workflow-gate.py" @@ -6055,9 +6156,10 @@ def test_ac20e2_expanded_bash_targets_fail_closed( ) assert proc.returncode == 0 - assert json.loads(proc.stdout)["hookSpecificOutput"][ - "permissionDecision" - ] == "deny" + assert ( + json.loads(proc.stdout)["hookSpecificOutput"]["permissionDecision"] + == "deny" + ) def test_ac20f_stop_dispatcher_runs_handlers_in_order(self, codex_project): """Codex Stop serializes scrub, validation, tokens, then memory."""