Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 26 additions & 11 deletions extensions/agent-context/scripts/python/update_agent_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

When ``plan_path`` is omitted, the script derives it from
``.specify/feature.json`` (written by /speckit-specify). Falls back to the most
recently modified ``plan.md`` anywhere under ``specs/`` (including nested scoped
layouts such as ``specs/<scope>/<feature>/plan.md``) only when feature.json is
recently modified ``plan.md`` found anywhere under ``specs/`` scoped layouts
nest it as ``specs/<scope>/<feature>/plan.md`` only when feature.json is
absent or its plan does not exist yet.
"""

Expand Down Expand Up @@ -173,16 +173,31 @@ def _resolve_plan_path(project_root: str) -> str:

if not plan_path:
root = Path(project_root).resolve()
plans = sorted(
(root / "specs").rglob("plan.md"),
key=lambda p: p.stat().st_mtime,
reverse=True,
)
if plans:
specs = root / "specs"

def _resolved_rel(p: Path) -> Path | None:
# Resolve symlinks before checking containment: relative_to() is
# lexical and would otherwise accept a plan reached through a specs/
# symlink that points outside the project, emitting an
# in-project-looking path for an out-of-project file (or picking it
# as "most recent").
try:
plan_path = plans[0].relative_to(root).as_posix()
except ValueError:
plan_path = ""
return p.resolve().relative_to(root)
except (OSError, ValueError):
return None

# Recurse (rather than the old one-level specs/*/plan.md glob) so scoped
# layouts created via SPECIFY_FEATURE_DIRECTORY, e.g.
# specs/<scope>/<feature>/plan.md, are still discovered when
# feature.json is absent (#3024). Mirrors the bash and PowerShell twins.
candidates = []
Comment thread
Quratulain-bilal marked this conversation as resolved.
for p in specs.rglob("plan.md"):
rel = _resolved_rel(p)
if rel is not None:
candidates.append((p, rel))
candidates.sort(key=lambda pr: pr[0].stat().st_mtime, reverse=True)
if candidates:
plan_path = candidates[0][1].as_posix()
return plan_path


Expand Down
4 changes: 2 additions & 2 deletions src/specify_cli/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ def atomic_write_json(target_file: Path, payload: dict[str, Any]) -> None:

os.replace(temp_path, target_file)
except Exception:
if temp_path and temp_path.exists():
temp_path.unlink()
if temp_path:
temp_path.unlink(missing_ok=True)
raise

try:
Expand Down
3 changes: 1 addition & 2 deletions src/specify_cli/integrations/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,7 @@ def save(self) -> Path:
_ensure_safe_manifest_destination(self.project_root, path)
os.replace(temp_path, path)
finally:
if temp_path.exists():
temp_path.unlink()
temp_path.unlink(missing_ok=True)
return path

@classmethod
Expand Down
3 changes: 1 addition & 2 deletions src/specify_cli/shared_infra.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@ def _write_shared_bytes(
_ensure_safe_shared_destination(project_path, dest)
os.replace(temp_path, dest)
finally:
if temp_path.exists():
temp_path.unlink()
temp_path.unlink(missing_ok=True)


_BASH_FORMAT_COMMAND_RE = re.compile(
Expand Down
76 changes: 69 additions & 7 deletions tests/extensions/test_update_agent_context_python_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,19 @@ def test_python_mtime_fallback_matching_bash(tmp_path: Path) -> None:


@requires_posix_bash
def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) -> None:
# Regression: the mtime fallback must discover plan.md in nested scoped
# layouts (specs/<scope>/<feature>/plan.md), matching the Bash/PowerShell
# ports and the documented recursive-discovery contract (see #3024). A
# one-level scan (specs/*/plan.md) would miss this and omit the plan link.
def test_python_mtime_fallback_finds_nested_plan_matching_bash(
tmp_path: Path,
) -> None:
"""The mtime fallback must recurse into scoped layouts.

A plan created under specs/<scope>/<feature>/plan.md (as produced via
SPECIFY_FEATURE_DIRECTORY) is more than one level below specs/. The old
Python port used a one-level specs/*/plan.md glob and missed it, while the
bash/PowerShell twins recurse (#3024). This locks in the parity.
"""
repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md")
for repo in (repo_a, repo_b):
plan = repo / "specs" / "scope-a" / "002-nested" / "plan.md"
plan = repo / "specs" / "backend" / "001-nested" / "plan.md"
plan.parent.mkdir(parents=True, exist_ok=True)
plan.write_text("# plan\n", encoding="utf-8")

Expand All @@ -361,7 +366,39 @@ def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) -
assert_parity(bash, py, repo_a, repo_b)
content = (repo_b / "AGENTS.md").read_bytes()
assert content == (repo_a / "AGENTS.md").read_bytes()
assert b"at specs/scope-a/002-nested/plan.md" in content
assert b"at specs/backend/001-nested/plan.md" in content


@requires_posix_bash
def test_python_mtime_fallback_skips_plan_reached_through_escaping_symlink(
tmp_path: Path,
) -> None:
"""A plan reached via a specs/ symlink out of the project is not selected.

``relative_to()`` is lexical, so ``specs/linked/001-x/plan.md`` looks
in-project even when ``specs/linked`` points outside the tree. Resolving
before the containment check rejects it, so the fallback finds nothing and
the ``at <plan>`` line is omitted rather than naming an out-of-project file
with an in-project-looking path. Mirrors the bash twin's ``_resolved_rel``.
"""
repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md")
for repo in (repo_a, repo_b):
outside = repo.parent / f"outside-{repo.name}" / "001-x"
outside.mkdir(parents=True, exist_ok=True)
(outside / "plan.md").write_text("# plan\n", encoding="utf-8")
specs = repo / "specs"
specs.mkdir(parents=True, exist_ok=True)
(specs / "linked").symlink_to(outside.parent, target_is_directory=True)
# Sanity: the plan really is reachable through the symlink.
assert (specs / "linked" / "001-x" / "plan.md").is_file()

bash = run_bash(repo_a)
py = run_python(repo_b)

assert_parity(bash, py, repo_a, repo_b)
content = (repo_b / "AGENTS.md").read_bytes()
assert content == (repo_a / "AGENTS.md").read_bytes()
assert b"\nat " not in content


@requires_posix_bash
Expand Down Expand Up @@ -508,6 +545,31 @@ def test_python_fresh_context_file_matches_powershell(tmp_path: Path) -> None:
assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes()


@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
def test_python_mtime_fallback_finds_nested_plan_matches_powershell(
tmp_path: Path,
) -> None:
"""Python's mtime fallback must recurse like the PowerShell twin.

With no feature.json, discovery falls back to scanning under specs/. A plan
at specs/<scope>/<feature>/plan.md sits more than one level deep; the old
Python one-level glob missed it while PowerShell already recurses (#3024).
"""
repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md")
repo_b = make_project(tmp_path / "proj-py", context_file="AGENTS.md")
for repo in (repo_a, repo_b):
plan = repo / "specs" / "backend" / "001-nested" / "plan.md"
plan.parent.mkdir(parents=True, exist_ok=True)
plan.write_text("# plan\n", encoding="utf-8")

ps = run_powershell(repo_a)
py = run_python(repo_b)

assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr
assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes()
assert b"at specs/backend/001-nested/plan.md" in (repo_b / "AGENTS.md").read_bytes()


@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
def test_python_upsert_matches_powershell(tmp_path: Path) -> None:
repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md")
Expand Down