From 75b4954f513fbc2cb8a97d2193fc8e1fd56ade53 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Mon, 27 Jul 2026 13:16:10 +0500 Subject: [PATCH 1/3] fix(agent-context): recurse for nested plans in Python mtime fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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///plan.md) was missed when feature.json is absent — the fallback returned no plan and the managed context section omitted the 'at ' 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///plan.md; both fail on the pre-fix one-level glob. Fixes #3733 --- .../scripts/python/update_agent_context.py | 33 ++++++++++---- ...test_update_agent_context_python_parity.py | 44 ++++++++++++++++--- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index fc8894ee14..9e2cfda953 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -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///plan.md, are still discovered when + # feature.json is absent (#3024). Mirrors the bash and PowerShell twins. + candidates = [] + 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 diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 969192eef3..a6b994c735 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -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///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///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") @@ -361,7 +366,7 @@ 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 @@ -508,6 +513,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///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") From 26964776f016f4c933a548fa25409420deb58bfb Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 28 Jul 2026 20:25:20 +0500 Subject: [PATCH 2/3] test(agent-context): cover symlink containment in the mtime fallback The recursive fallback resolves each candidate before the relative_to() containment check, but nothing exercised that path. Add a parity test for a plan reachable only through a specs/ symlink pointing outside the project: relative_to() is lexical and would accept it, emitting an in-project-looking path for an out-of-project file. Both the bash twin and the Python port skip it, so the "at " line is omitted. Also correct the module docstring, which still described the fallback as scanning specs/*/plan.md one level deep. --- .../scripts/python/update_agent_context.py | 4 +-- ...test_update_agent_context_python_parity.py | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 9e2cfda953..669ec5bf9d 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -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///plan.md``) only when feature.json is +recently modified ``plan.md`` found anywhere under ``specs/`` — scoped layouts +nest it as ``specs///plan.md`` — only when feature.json is absent or its plan does not exist yet. """ diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index a6b994c735..36e7fd4557 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -369,6 +369,38 @@ def test_python_mtime_fallback_finds_nested_plan_matching_bash( 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 `` 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 def test_python_prefers_feature_json_over_mtime_matching_bash(tmp_path: Path) -> None: repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md") From 1adf9bf25581c30de227803b4844e5dbab8c779b Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 28 Jul 2026 20:25:57 +0500 Subject: [PATCH 3/3] fix: use missing_ok for temp file cleanup to avoid masking errors --- src/specify_cli/_utils.py | 4 ++-- src/specify_cli/integrations/manifest.py | 3 +-- src/specify_cli/shared_infra.py | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 85b659d67b..b623de81af 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -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: diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py index ac799ebee6..504734c8f1 100644 --- a/src/specify_cli/integrations/manifest.py +++ b/src/specify_cli/integrations/manifest.py @@ -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 diff --git a/src/specify_cli/shared_infra.py b/src/specify_cli/shared_infra.py index 1c8d727d73..001a31e9e1 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -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(