Skip to content

Commit bc21008

Browse files
fix(agent-context): recurse for nested plans in Python mtime fallback
The Python port's mtime fallback discovered plans with a one-level specs/*/plan.md glob, so a scoped layout created via SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) was missed when feature.json is absent — the fallback returned no plan and the managed context section omitted the 'at <plan>' line. The bash and PowerShell twins were already fixed to recurse (#3024); the Python twin was left behind. Switch to specs.rglob('plan.md') with the same symlink-safe containment check the bash twin uses (resolve each candidate and confirm it stays within the project root before ranking by mtime), so a plan reached through a specs/ symlink pointing outside the project is not selected. Adds parity regression tests (vs bash and vs PowerShell) covering a nested specs/<scope>/<feature>/plan.md; both fail on the pre-fix one-level glob. Fixes #3733
1 parent be33d2a commit bc21008

2 files changed

Lines changed: 63 additions & 45 deletions

File tree

‎extensions/agent-context/scripts/python/update_agent_context.py‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@
1111
1212
When ``plan_path`` is omitted, the script derives it from
1313
``.specify/feature.json`` (written by /speckit-specify). Falls back to the most
14-
recently modified ``plan.md`` anywhere under ``specs/`` (including nested scoped
15-
layouts such as ``specs/<scope>/<feature>/plan.md``) only when feature.json is
16-
absent or its plan does not exist yet.
14+
recently modified ``specs/*/plan.md`` only when feature.json is absent or its
15+
plan does not exist yet.
1716
"""
1817

1918
from __future__ import annotations
@@ -173,16 +172,31 @@ def _resolve_plan_path(project_root: str) -> str:
173172

174173
if not plan_path:
175174
root = Path(project_root).resolve()
176-
plans = sorted(
177-
(root / "specs").rglob("plan.md"),
178-
key=lambda p: p.stat().st_mtime,
179-
reverse=True,
180-
)
181-
if plans:
175+
specs = root / "specs"
176+
177+
def _resolved_rel(p: Path) -> Path | None:
178+
# Resolve symlinks before checking containment: relative_to() is
179+
# lexical and would otherwise accept a plan reached through a specs/
180+
# symlink that points outside the project, emitting an
181+
# in-project-looking path for an out-of-project file (or picking it
182+
# as "most recent").
182183
try:
183-
plan_path = plans[0].relative_to(root).as_posix()
184-
except ValueError:
185-
plan_path = ""
184+
return p.resolve().relative_to(root)
185+
except (OSError, ValueError):
186+
return None
187+
188+
# Recurse (rather than the old one-level specs/*/plan.md glob) so scoped
189+
# layouts created via SPECIFY_FEATURE_DIRECTORY, e.g.
190+
# specs/<scope>/<feature>/plan.md, are still discovered when
191+
# feature.json is absent (#3024). Mirrors the bash and PowerShell twins.
192+
candidates = []
193+
for p in specs.rglob("plan.md"):
194+
rel = _resolved_rel(p)
195+
if rel is not None:
196+
candidates.append((p, rel))
197+
candidates.sort(key=lambda pr: pr[0].stat().st_mtime, reverse=True)
198+
if candidates:
199+
plan_path = candidates[0][1].as_posix()
186200
return plan_path
187201

188202

‎tests/extensions/test_update_agent_context_python_parity.py‎

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -222,32 +222,6 @@ def test_python_custom_markers_matching_bash(tmp_path: Path) -> None:
222222
assert "old" not in content
223223

224224

225-
@requires_posix_bash
226-
def test_python_blank_markers_use_defaults_matching_bash(tmp_path: Path) -> None:
227-
# Regression: with blank markers (config relying on the built-in defaults),
228-
# the Bash port must fall back to DEFAULT_START/END, matching the Python and
229-
# PowerShell ports. Previously the Bash config-parser transport dropped the
230-
# trailing empty marker lines under $(...) command substitution, tripping the
231-
# "malformed config parser output" guard so the default-marker substitution
232-
# became unreachable and the context file was never updated.
233-
markers = {"start": "", "end": ""}
234-
repo_a, repo_b = twin_projects(
235-
tmp_path, context_file="AGENTS.md", context_markers=markers
236-
)
237-
add_plan(repo_a)
238-
add_plan(repo_b)
239-
240-
bash = run_bash(repo_a)
241-
py = run_python(repo_b)
242-
243-
assert_parity(bash, py, repo_a, repo_b)
244-
content = (repo_b / "AGENTS.md").read_bytes()
245-
assert content == (repo_a / "AGENTS.md").read_bytes()
246-
assert b"<!-- SPECKIT START -->" in content
247-
assert b"<!-- SPECKIT END -->" in content
248-
assert b"at specs/001-demo/plan.md" in content
249-
250-
251225
@requires_posix_bash
252226
def test_python_multiple_context_files_dedup_matching_bash(tmp_path: Path) -> None:
253227
files = ["AGENTS.md", "docs/CONTEXT.md", "AGENTS.md"]
@@ -344,14 +318,19 @@ def test_python_mtime_fallback_matching_bash(tmp_path: Path) -> None:
344318

345319

346320
@requires_posix_bash
347-
def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) -> None:
348-
# Regression: the mtime fallback must discover plan.md in nested scoped
349-
# layouts (specs/<scope>/<feature>/plan.md), matching the Bash/PowerShell
350-
# ports and the documented recursive-discovery contract (see #3024). A
351-
# one-level scan (specs/*/plan.md) would miss this and omit the plan link.
321+
def test_python_mtime_fallback_finds_nested_plan_matching_bash(
322+
tmp_path: Path,
323+
) -> None:
324+
"""The mtime fallback must recurse into scoped layouts.
325+
326+
A plan created under specs/<scope>/<feature>/plan.md (as produced via
327+
SPECIFY_FEATURE_DIRECTORY) is more than one level below specs/. The old
328+
Python port used a one-level specs/*/plan.md glob and missed it, while the
329+
bash/PowerShell twins recurse (#3024). This locks in the parity.
330+
"""
352331
repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md")
353332
for repo in (repo_a, repo_b):
354-
plan = repo / "specs" / "scope-a" / "002-nested" / "plan.md"
333+
plan = repo / "specs" / "backend" / "001-nested" / "plan.md"
355334
plan.parent.mkdir(parents=True, exist_ok=True)
356335
plan.write_text("# plan\n", encoding="utf-8")
357336

@@ -361,7 +340,7 @@ def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) -
361340
assert_parity(bash, py, repo_a, repo_b)
362341
content = (repo_b / "AGENTS.md").read_bytes()
363342
assert content == (repo_a / "AGENTS.md").read_bytes()
364-
assert b"at specs/scope-a/002-nested/plan.md" in content
343+
assert b"at specs/backend/001-nested/plan.md" in content
365344

366345

367346
@requires_posix_bash
@@ -508,6 +487,31 @@ def test_python_fresh_context_file_matches_powershell(tmp_path: Path) -> None:
508487
assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes()
509488

510489

490+
@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
491+
def test_python_mtime_fallback_finds_nested_plan_matches_powershell(
492+
tmp_path: Path,
493+
) -> None:
494+
"""Python's mtime fallback must recurse like the PowerShell twin.
495+
496+
With no feature.json, discovery falls back to scanning under specs/. A plan
497+
at specs/<scope>/<feature>/plan.md sits more than one level deep; the old
498+
Python one-level glob missed it while PowerShell already recurses (#3024).
499+
"""
500+
repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md")
501+
repo_b = make_project(tmp_path / "proj-py", context_file="AGENTS.md")
502+
for repo in (repo_a, repo_b):
503+
plan = repo / "specs" / "backend" / "001-nested" / "plan.md"
504+
plan.parent.mkdir(parents=True, exist_ok=True)
505+
plan.write_text("# plan\n", encoding="utf-8")
506+
507+
ps = run_powershell(repo_a)
508+
py = run_python(repo_b)
509+
510+
assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr
511+
assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes()
512+
assert b"at specs/backend/001-nested/plan.md" in (repo_b / "AGENTS.md").read_bytes()
513+
514+
511515
@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
512516
def test_python_upsert_matches_powershell(tmp_path: Path) -> None:
513517
repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md")

0 commit comments

Comments
 (0)