Cross-cutting prep: jinja2 dep + host-paths.md contract + _locking.py primitive#148
Merged
Conversation
Pin reserved for downstream Phase C (Jinja-rendered templates) per Stefania-adaptation roadmap. No call-site wired in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents canonical MAP_* namespace, three reserved env vars (MAP_INVOKED_BY/MAP_BRANCH/MAP_SUBTASK_ID), live MAP_* registry with file:line citations, MAPIFY_* legacy status, `~/.map/locks|hooks/` layout, the closed six-state marker enum (INV-5), and a forward-ref to src/mapify_cli/_locking.py (ST-003 in same PR). Dual-copied to src/mapify_cli/templates/references/ to keep tests/test_template_sync.py green. No runtime wiring; new env vars are documented only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…l test suite
src/mapify_cli/_locking.py — stdlib-only fcntl.flock-based exclusive lock
with closed-enum state marker sidecar at ~/.map/locks/<name>.state.json.
Public surface: LockState (StrEnum: IN_PROGRESS/CREATED/UPDATED/SKIPPED/
TIMEOUT/ERROR), LockTimeoutError, LockSecurityError, flock_with_state
context manager yielding a StateWriter with .set(state).
Security boundaries: name regex ^[a-zA-Z0-9_-]{1,64}$ enforced before any
fs touch; lock fd opens with O_NOFOLLOW (ELOOP→LockSecurityError);
sidecar uses os.lstat-then-os.replace with atomic tmp file co-located in
~/.map/locks/ (INV-6, never /tmp). Lock dir 0o700, files 0o600.
Behavior: polling timeout (50 ms / D11, no SIGALRM) writes timeout marker
then raises LockTimeoutError. Exception inside `with` writes error marker
and bare-raises the original exception unchanged (HC-4).
tests/test_locking.py — covers AC-5 (a)-(g) plus AC-6 modes, INV-4
docstring sentence, INV-6 tmp prefix, name-validation. Real two-process
contention via subprocess.Popen (HC-3, threads forbidden). HOME is
monkeypatched per-test so the host's ~/.map/locks/ is never touched.
Return annotation is collections.abc.Generator[StateWriter, None, None]
rather than Iterator — Pyright deprecates @contextmanager + Iterator; both
satisfy AC-4's intent (iterator protocol, not ContextManager) and mypy.
make check: ruff + mypy clean; full pytest 1714 passed / 4 skipped / 0 failed.
pyright src/mapify_cli/_locking.py: 0/0/0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds cross-cutting preparation for future MAP workflow phases by introducing a Jinja2 runtime dependency, a canonical host-path/environment-variable contract, and a new file-locking primitive with tests.
Changes:
- Adds
jinja2>=3.1,<4and a smoke test for import/version coverage. - Introduces duplicated host-path/MAP environment reference docs for development and shipped templates.
- Adds
_locking.pywith flock-based state sidecars plus subprocess-backed tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds Jinja2 as a runtime dependency. |
tests/test_jinja2_dep.py |
Verifies Jinja2 is importable and meets the version floor. |
src/mapify_cli/_locking.py |
Adds the MAP lock/state sidecar primitive. |
tests/test_locking.py |
Covers locking behavior, contention, markers, symlink handling, and permissions. |
.claude/references/host-paths.md |
Adds the source reference contract for MAP host paths and env vars. |
src/mapify_cli/templates/references/host-paths.md |
Adds the shipped template copy of the same reference contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| |---|---|---|---| | ||
| | `MAP_DEBUG` | live | `src/mapify_cli/__init__.py:207` | Enables verbose debug logging across MAP CLI internals when set to a truthy value. | | ||
| | `MAP_MONITOR_HOTFIX` | live | `src/mapify_cli/templates/codex/hooks/workflow-gate.py:68` | Bypasses the monitor gate for emergency hotfix flows; must not be set in normal workflows. | | ||
| | `MAP_STRICT_SCOPE` | live | `src/mapify_cli/templates/map/scripts/map_step_runner.py:7137` | Enforces strict mutation-boundary validation; rejects Actor edits outside `affected_files`. | |
| |---|---|---|---| | ||
| | `MAP_DEBUG` | live | `src/mapify_cli/__init__.py:207` | Enables verbose debug logging across MAP CLI internals when set to a truthy value. | | ||
| | `MAP_MONITOR_HOTFIX` | live | `src/mapify_cli/templates/codex/hooks/workflow-gate.py:68` | Bypasses the monitor gate for emergency hotfix flows; must not be set in normal workflows. | | ||
| | `MAP_STRICT_SCOPE` | live | `src/mapify_cli/templates/map/scripts/map_step_runner.py:7137` | Enforces strict mutation-boundary validation; rejects Actor edits outside `affected_files`. | |
|
|
||
| ## (f) State Markers (Closed Enum) | ||
|
|
||
| The MAP step runner records exactly one of six state markers per subtask step: |
|
|
||
| ## (f) State Markers (Closed Enum) | ||
|
|
||
| The MAP step runner records exactly one of six state markers per subtask step: |
Comment on lines
+88
to
+90
| def set(self, state: LockState) -> None: | ||
| """Write *state* to the sidecar atomically.""" | ||
| _write_state_atomic(self.lock_root, self.name, state, self.pid) |
| # ---- 3. Open lock file (O_NOFOLLOW refuses symlinks) ------------------ | ||
| o_nofollow = getattr(os, "O_NOFOLLOW", 0) | ||
| try: | ||
| fd = os.open(str(lock_path), os.O_RDWR | os.O_CREAT | o_nofollow, 0o600) |
Comment on lines
+105
to
+106
| lock_root.mkdir(mode=0o700, parents=True, exist_ok=True) | ||
| # Do NOT chmod an already-existing dir — respect what the user set. |
Linux CI failure (test bug, not runtime):
- tests/test_locking.py asserted `not str(tmp).startswith("/tmp")` as
the INV-6 check. On Linux runners pytest's tmp_path is rooted at
/tmp/pytest-of-runner/..., so the legitimate `$HOME/.map/locks/`
layout falsely tripped the prefix scan; macOS passed because its
tmp lives in /private/var/folders/. The actual INV-6 invariant is
"tmp file shares the target sidecar's directory" (required for
os.replace atomicity). Replaced both assertions with the correct
`tmp.parent == lock_root` check.
Copilot findings (4 unique, 7 inline due to dual-copy):
1. host-paths.md registry omitted live MAP_REVIEW_PROMPT_BUDGET_TOKENS.
Added row citing src/.../map_step_runner.py:147,4577 (the
REVIEW_PROMPT_BUDGET_ENV consumer).
2. Section (f) said "the MAP step runner records these six markers"
but this PR wires no caller and the step runner's actual subtask
statuses are a different enum (pending |in_progress|complete|blocked).
Rephrased to attribute the six-marker enum to _locking.py's
sidecar writer and explicitly note no surface calls it yet.
3. StateWriter was @DataClass (mutable) and exposed publicly via the
contextmanager yield. A caller could mutate `writer.name` or
construct StateWriter directly with a traversal name to bypass the
regex. Two-layer fix:
- @DataClass(frozen=True) so FrozenInstanceError blocks mutation
- _write_state_atomic re-validates `name` on every write so direct
construction with a bad name still cannot escape the lock root
Two new tests cover both: test_state_writer_is_frozen,
test_direct_state_writer_with_bad_name_revalidated.
4. mode=0o600/0o700 only applied on file/dir CREATION; a pre-existing
lock dir at 0o755 or lock file at 0o644 kept its mode, despite the
module contract guaranteeing 0o700/0o600. Now:
- _ensure_lock_dir calls os.chmod(lock_root, 0o700) after mkdir
- flock_with_state calls os.fchmod(fd, 0o600) after os.open
Two new tests cover both: test_existing_lock_dir_mode_enforced,
test_existing_lock_file_mode_enforced (skipif win32).
make check: ruff + mypy clean; 1718 passed / 4 skipped / 0 failed.
host-paths.md byte-identical to its templates copy.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cross-cutting primitives for the Stefania-adaptation roadmap (Phases A and E will consume). Primitives only — no caller wired, no hook rewired in this PR.
274bc56) — addjinja2>=3.1,<4runtime dep +tests/test_jinja2_dep.pysmoke import-and-version-floor test. Paid-for-future-use until Phase C lands the first Jinja-rendered template.cb83278) — create.claude/references/host-paths.md(dual-copied tosrc/mapify_cli/templates/references/) documenting the canonicalMAP_*namespace, three reserved env vars (MAP_INVOKED_BY/MAP_BRANCH/MAP_SUBTASK_ID), the existing MAP_* registry with file:line citations,MAPIFY_TRANSCRIPT_PATHlegacy status,~/.map/locks|hooks/layout, the closed six-state marker enum (INV-5), and a forward-ref to_locking.py.79a554f) — implementsrc/mapify_cli/_locking.py(stdlib-onlyfcntl.flock+ state-marker sidecar at~/.map/locks/<name>.state.json) andtests/test_locking.pycovering AC-5 (a)-(g) including real two-process contention viasubprocess.Popen(not threads). Security boundaries: name regex enforced before any fs touch; lock fd opens withO_NOFOLLOW; sidecaros.lstat-checked before everyos.replace; tmp file co-located in~/.map/locks/(INV-6, never/tmp); modes 0o700 dir / 0o600 files.Scope discipline
Generator[StateWriter, None, None]instead ofIterator(Pyright deprecatesIterator+@contextmanager; both satisfy the spec's mypy intent — "iterator-protocol, not ContextManager")Deferred (not in this PR): wiring
MAP_INVOKED_BYinto hooks, calling_locking.pyfrom any workflow, renamingMAPIFY_TRANSCRIPT_PATH, Jinja-rendered templates, retrofittingstep_state.jsonwrites with locking.Test plan
make check— ruff clean, mypy clean (36 src files), full pytest 1714 passed / 4 skipped / 0 failedpython -m pyright src/mapify_cli/_locking.py— 0 errors / 0 warnings / 0 informationspytest tests/test_template_sync.py— 49 passed (host-paths.md dual-copy enforced)pytest tests/test_locking.py— 12 passed, including real two-process contentionpytest tests/test_jinja2_dep.py— 1 passeddiff -q .claude/references/host-paths.md src/mapify_cli/templates/references/host-paths.md— identical🤖 Generated with Claude Code