feat: platform refactor — spec, decomposition, config, managed copier#90
feat: platform refactor — spec, decomposition, config, managed copier#90
Conversation
…ase audit - Landscape analysis: OpenSpec patterns (schema DAGs, context injection, multi-tool delivery, verification dimensions, migration) with clear differentiation from MAP's hard-gate philosophy - Per-phase codebase analysis with concrete file paths, LOC counts, and gap tables for all 7 implementation phases - Artifact inventory: 25 .map/ artifacts catalogued with producer/consumer and schema coverage assessment - Claude Code coupling map: portable vs platform-specific components - Implementation priority graph with dependency ordering and quick wins - 10 open questions enriched with codebase evidence
…dules Extract 1422 lines (53%) from the monolithic __init__.py (2692→1270 lines) into focused submodules while maintaining full backward compatibility: - cli_ui.py: StepTracker, interactive selectors, banner display - delivery/agent_generator.py: 7 fallback agent content generators - delivery/file_copier.py: file copy/install functions (agents, commands, hooks, etc.) - config/settings.py: global/project permissions management - config/mcp.py: MCP server configuration and .mcp.json handling Quick wins included: - Add BLUEPRINT_SCHEMA to schemas.py (blueprint.json was parsed without validation) - Add validate_artifact() and load_and_validate() utilities to schemas.py - Support jsonschema Draft7/4 fallback for older environments All re-exports preserved in __init__.py — existing imports continue to work. Tests updated: mock paths adjusted for new module locations. 24 new decomposition tests + 88 template sync tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduce MapConfig dataclass with 23 configurable fields covering workflow profiles, policy thresholds, safety guardrails, pruner settings, and delivery options. Config is loaded from .map/config.yaml with graceful fallback to defaults when file is missing or malformed. - Add config/project_config.py with MapConfig, load_map_config(), generate_default_config(), write_default_config() - Wire write_default_config() into `mapify init` command - Update safety-guardrails.py hook to read config overrides from .map/config.yaml (dangerous_file_patterns, dangerous_commands, safe_path_prefixes) with fallback to hardcoded defaults - Sync hook template to .claude/hooks/ - Add 13 tests for config system (load, generate, write, edge cases) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw shutil.copy2() with copy_managed_file() that injects metadata headers (generated_by, mapify_version, template_hash) and detects user modifications during upgrade. - Add delivery/managed_file_copier.py with metadata injection for .md (HTML comment), .py (comment), .json (_map_managed key) - Add drift detection: compares content hash against stored template_hash - Auto-backup drifted files to .bak before overwriting - Integrate into all file_copier.py functions (agents, commands, references, hooks, configs) with optional DriftReport parameter - Update upgrade() to collect drift reports and show warnings with backup locations for modified files - Add 29 tests covering hashing, injection, extraction, roundtrips, drift detection, and backup creation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix load_and_validate returning data instead of None on invalid input - Deduplicate get_templates_dir (delegate from __init__.py to file_copier) - Use timestamped backup names to prevent collision on repeated upgrades - Unify Console instances (import from cli_ui instead of creating new) - Fix extract_metadata .py reconstruction (positional split, not search) - Add MapConfig type validation (wrong YAML types fall back to defaults) - Simplify no-op setdefault calls in settings.py - Remove unused mcp_section from create_reflector_content - Remove duplicate in-function imports from file_copier._copy_map_path - Add spec link to README docs table - Add 4 tests: guardrails config override, type coercion, backup collision, load_and_validate with invalid data
There was a problem hiding this comment.
Pull request overview
Phase 1 of the platform refactor: decomposes the monolithic mapify_cli/__init__.py into clearer submodules, introduces a project-level .map/config.yaml system, and adds drift-aware “managed” copying to make upgrade safer and more transparent.
Changes:
- Extracts CLI UI, delivery, and config concerns into
cli_ui.py,delivery/, andconfig/with backward-compatible re-exports. - Adds drift-aware managed copying (hashing + backups) and updates
upgrade()to report drifted files. - Introduces
.map/config.yaml(MapConfig) and teaches safety guardrails hooks to read overrides; addsBLUEPRINT_SCHEMA+ schema utilities.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_mapify_cli.py | Adjusts assertions/mocks for managed metadata + refactor paths |
| tests/test_managed_file_copier.py | New unit tests for managed copier / drift + backups |
| tests/test_decomposition.py | New tests for decomposition re-exports + config/schema additions |
| src/mapify_cli/templates/hooks/safety-guardrails.py | Adds .map/config.yaml override loading for guardrail constants |
| .claude/hooks/safety-guardrails.py | Keeps repo dev hook in sync with shipped template hook |
| src/mapify_cli/schemas.py | Adds schema validation helpers + BLUEPRINT_SCHEMA |
| src/mapify_cli/delivery/managed_file_copier.py | Implements metadata injection, drift detection, backups |
| src/mapify_cli/delivery/file_copier.py | Routes installs/upgrades through managed copier + drift report aggregation |
| src/mapify_cli/delivery/agent_generator.py | Extracted fallback agent content generators |
| src/mapify_cli/delivery/init.py | Delivery package re-exports |
| src/mapify_cli/config/settings.py | Extracted settings/permissions utilities |
| src/mapify_cli/config/project_config.py | Adds .map/config.yaml loader + default config generation |
| src/mapify_cli/config/mcp.py | Extracted MCP config + .mcp.json merge logic |
| src/mapify_cli/config/init.py | Config package re-exports |
| src/mapify_cli/cli_ui.py | Extracted banner/UI selection + step tracker |
| src/mapify_cli/init.py | Decomposition wiring + upgrade drift reporting + config creation on init |
| README.md | Links the new platform spec |
| docs/MAP_PLATFORM_SPEC.md | Adds the detailed platform refactor roadmap/spec |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DANGEROUS_FILE_PATTERNS = _config.get( | ||
| "dangerous_file_patterns", _DEFAULT_DANGEROUS_FILE_PATTERNS | ||
| ) | ||
| DANGEROUS_COMMANDS = _config.get("dangerous_commands", _DEFAULT_DANGEROUS_COMMANDS) | ||
| SAFE_PATH_PREFIXES = _config.get("safe_path_prefixes", _DEFAULT_SAFE_PATH_PREFIXES) | ||
|
|
||
|
|
||
| def is_safe_path(path: str) -> bool: | ||
| """Check if path is in known safe directory.""" | ||
| return any(path.startswith(prefix) for prefix in SAFE_PATH_PREFIXES) |
There was a problem hiding this comment.
Config overrides are used without validating types. If .map/config.yaml sets safe_path_prefixes to a string (or contains non-string entries), any(path.startswith(prefix) for prefix in SAFE_PATH_PREFIXES) will iterate characters / raise TypeError, which can either break the hook or unintentionally mark broad paths as safe. Please validate/coerce overrides (e.g., ensure these are list[str], else fall back to defaults) for safe_path_prefixes, dangerous_file_patterns, and dangerous_commands before assigning the module-level constants.
| DANGEROUS_FILE_PATTERNS = _config.get( | ||
| "dangerous_file_patterns", _DEFAULT_DANGEROUS_FILE_PATTERNS | ||
| ) | ||
| DANGEROUS_COMMANDS = _config.get("dangerous_commands", _DEFAULT_DANGEROUS_COMMANDS) | ||
| SAFE_PATH_PREFIXES = _config.get("safe_path_prefixes", _DEFAULT_SAFE_PATH_PREFIXES) | ||
|
|
||
|
|
||
| def is_safe_path(path: str) -> bool: | ||
| """Check if path is in known safe directory.""" | ||
| return any(path.startswith(prefix) for prefix in SAFE_PATH_PREFIXES) |
There was a problem hiding this comment.
Config overrides are used without validating types. If .map/config.yaml sets safe_path_prefixes to a string (or contains non-string entries), any(path.startswith(prefix) for prefix in SAFE_PATH_PREFIXES) will iterate characters / raise TypeError, which can either break the hook or unintentionally mark broad paths as safe. Please validate/coerce overrides (e.g., ensure these are list[str], else fall back to defaults) for safe_path_prefixes, dangerous_file_patterns, and dangerous_commands before assigning the module-level constants.
| first_newline = content.index("\n") + 1 | ||
| shebang = content[:first_newline] | ||
| rest = content[first_newline:] |
There was a problem hiding this comment.
inject_metadata() assumes a shebang line always contains a newline (content.index("\n")). If a file is just a single-line shebang (no trailing newline), this raises ValueError and breaks installs/upgrades. Consider using content.find("\n") and handling -1 (treat entire content as the shebang line), or use splitlines(keepends=True) to preserve the first line safely.
| first_newline = content.index("\n") + 1 | |
| shebang = content[:first_newline] | |
| rest = content[first_newline:] | |
| first_newline = content.find("\n") | |
| if first_newline == -1: | |
| # File is just a single-line shebang with no trailing newline | |
| shebang = content | |
| rest = "" | |
| else: | |
| first_newline += 1 | |
| shebang = content[:first_newline] | |
| rest = content[first_newline:] |
| # Validate type: check that YAML value matches expected type | ||
| expected_type = type(getattr(defaults, key)) | ||
| if not isinstance(value, expected_type): | ||
| logger.warning( | ||
| "Config key '%s' expects %s, got %s (%r). Using default.", | ||
| key, | ||
| expected_type.__name__, | ||
| type(value).__name__, | ||
| value, | ||
| ) | ||
| continue | ||
| config_dict[key] = value |
There was a problem hiding this comment.
load_map_config() requires YAML values to be the exact same runtime type as the default (e.g., confidence_threshold must be float). In YAML it's common to write confidence_threshold: 1 (parsed as int), which is semantically valid but will be rejected and silently fall back to defaults. Consider allowing safe coercions (e.g., int→float, str→Path if ever added) or using the dataclass field annotations to drive validation/coercion instead of type(getattr(defaults, key)).
| # Small delay to ensure different timestamp | ||
| time.sleep(1.1) | ||
|
|
||
| # Second upgrade — must NOT overwrite first backup | ||
| result2 = copy_managed_file(src, dest, "3.7.0") |
There was a problem hiding this comment.
This test uses time.sleep(1.1) to force backup timestamps to differ, which makes the test suite slower and potentially flaky under heavy CI load. Consider making copy_managed_file() accept an injectable clock/now() function (or monkeypatch the module's datetime) so the test can advance time deterministically without sleeping.
| if ext == ".md": | ||
| header = f"<!-- {_MANAGED_TAG}: {meta_json} -->\n" | ||
| return header + content | ||
|
|
There was a problem hiding this comment.
For Markdown templates that rely on YAML frontmatter (agents/commands begin with ---), prepending an HTML comment header means the file no longer starts with frontmatter. That can break downstream consumers that require frontmatter at the first byte/line. Consider detecting existing frontmatter and injecting MAP-managed fields inside it (or placing the managed marker immediately after the closing ---) instead of always prepending a comment.
- Remove duplicate `from datetime import datetime, timezone` inside copy_managed_file() (already imported at module level) - Add try/except around dest.write_text() to gracefully handle write failures instead of crashing mid-upgrade - Add `from __future__ import annotations` to file_copier.py for consistent Python 3.10 compatibility with union type syntax Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e errors build_handoff_bundle and build_review_handoff read markdown/JSON artifacts that may contain raw control characters (ANSI escapes, null bytes from test output). These chars pass through Python json.dumps correctly but can corrupt bash variable expansion before reaching jq. - Add _sanitize_for_json() to strip U+0000-U+001F (except \n \r \t) - Use errors="replace" on read_text to handle non-UTF-8 files gracefully - Add OSError catch to prevent traceback on stdout from corrupted files - Use ensure_ascii=True in json.dumps for handoff CLI output
map-plan created step_state.json with field names "current_state" and "current_subtask", but workflow-gate.py reads "current_step_phase" and "current_subtask_id". This mismatch caused the gate to see empty phase and block all edits after map-plan, requiring manual state patching. - Fix map-plan.md step_state template: use current_step_phase and current_subtask_id to match gate expectations - Fix map-tdd.md: add workflow_status check before resume, explicit guidance for COMPLETE/INITIALIZED states requiring reinit - Add _sanitize_for_json to map_step_runner.py handoff functions to prevent jq parse errors from control chars in artifact content
Summary
Platform refactor Phase 1 implementation based on OpenSpec analysis and codebase audit.
docs/MAP_PLATFORM_SPEC.md) — 1156-line refactor roadmap with OpenSpec landscape analysis, per-phase codebase audit (25 artifacts inventoried, triple redundancy mapped, Claude coupling assessed), implementation priority graph, and 10 open questions with evidence__init__.py— reduced from 2692 to ~1200 lines by extractingcli_ui.py,delivery/package (agent_generator, file_copier, managed_file_copier),config/package (project_config, settings, mcp).map/config.yamlwithMapConfigdataclass, type validation, graceful YAML fallback, safety-guardrails hook reads config overrides.bakbackups, per-format metadata injection (.md/.py/.json),DriftReportfor upgrade safetyBLUEPRINT_SCHEMAadded,load_and_validatecorrectness fix (returnNoneon invalid data)Key metrics
Test plan
mapify initon a fresh project produces working setup (11 agents, 12 commands, 8 hooks, config.yaml with metadata injection)mapify upgradeon existing project creates timestamped backups for drifted files (verified:actor.md.20260323T122751.bak).map/config.yamlwith customsafe_path_prefixesoverrides hook behavior (verified: custom paths safe, defaultsrc/not safe when overridden)