fix: derive plan path from feature.json in update-agent-context (#3069)

* fix: derive plan path from feature.json in update-agent-context

When `plan_path` is omitted, prefer `.specify/feature.json`
(written by /speckit-specify) over the mtime heuristic. The
old approach picked the most recently modified `specs/*/plan.md`,
which could inject an unrelated plan into CLAUDE.md if another
spec's plan was touched after the active feature directory was
created but before its own plan.md existed.

Bash: handle both relative and absolute feature_directory values,
normalizing absolute paths back to project-relative for the
context file. Fall back to mtime only when feature.json is absent
or the derived plan.md does not yet exist.

PowerShell: same logic, PS 5.1-compatible (nested Join-Path,
IsPathRooted guard to avoid Unix Join-Path mis-joining absolute
ChildPaths, manual prefix-strip instead of GetRelativePath).

Fixes #3067

* fix: address Copilot review feedback on update-agent-context

- bash: add explicit encoding="utf-8" to feature.json open() call
- powershell: replace GetRelativePath (.NET 5+ only) with manual
  prefix-strip in mtime fallback for PS 5.1 compatibility
- tests: add coverage for absolute feature_directory values
  (under and outside PROJECT_ROOT)

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* test: replace time.sleep with os.utime and strengthen PS normalization assertion

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: normalize trailing slash and guard non-string feature_directory in PS script

* Fix: use .resolve().as_posix().

Valid. The PS tests run on Windows where str(tmp_path) uses backslashes, but the PS script normalizes output to forward slashes. Assertions like assert str(tmp_path) not in ctx become false negatives on Windows CI.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: use context manager for feature.json open() in bash heredoc

* test: add PS coverage for absolute feature_directory outside project root

* fix: guard null feature_directory, re-check empty after trailing-slash strip, fix blank line

* test: add stale plan to absolute-path tests so feature.json preference is actually exercised

* test: convert absolute paths to MSYS2 style for Git-for-Windows bash compatibility

* fix: revert PS test to native path, fix bash outside-root assertion for Git bash

* fix: use _to_bash_path in not-in assertion for Git bash Windows compat

* fix: add ConvertFrom-Json fallback in PS script, write test config as JSON

* fix: use OS-appropriate StringComparison in PS prefix-strip (matches common.ps1)

* fix: emit project-relative POSIX path from mtime fallback; use upstream test helpers

* fix: write config as JSON directly, drop _install_agent_context_config

* fix: normalize backslashes to forward slashes in feature_directory before path ops

* fix: treat drive-qualified paths (C:/...) as absolute after backslash normalization

* fix: resolve symlinks when computing relative plan path; use UTF8 encoding in PS ConvertFrom-Yaml path

* fix: use bash-side path for outside-root case to avoid WindowsPath backslashes

* fix: use .as_posix() instead of PurePosixPath() to avoid backslashes on native Windows Python

* fix: resolve ./.. segments in PS feature_directory via GetFullPath before relativizing

* fix: replace $IsWindows guard with OSVersion.Platform check for PS 5.1 StrictMode compat

* fix: guard empty relDir to avoid leading slash in PlanPath when feature_directory is project root

* fix: remove unused PurePosixPath import; fix stale PS comment after ConvertFrom-Json fallback was added

* fix: use cand.as_posix() for outside-root path instead of raw bash-side argv

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Amirreza Alibeigi
2026-06-26 19:07:44 +02:00
committed by GitHub
parent 5f9791b524
commit 49cc05384a
3 changed files with 359 additions and 29 deletions

View File

@@ -10,9 +10,9 @@
#
# Usage: update-agent-context.sh [plan_path]
#
# When `plan_path` is omitted, the script picks the most recently modified
# `specs/*/plan.md` if any exist, otherwise emits the section without a
# concrete plan path.
# When `plan_path` is omitted, the script derives it from `.specify/feature.json`
# (written by /speckit-specify). Falls back to the most recently modified
# `specs/*/plan.md` only when feature.json is absent or its plan does not exist yet.
set -euo pipefail
@@ -202,23 +202,78 @@ unset _cf_parts _seg
PLAN_PATH="${1:-}"
if [[ -z "$PLAN_PATH" ]]; then
# Pick the most recently modified plan.md one level deep (specs/<feature>/plan.md).
# Use find + sort by modification time to avoid ls/head fragility with
# spaces in paths or SIGPIPE from pipefail.
_plan_abs="$("$_python" - "$PROJECT_ROOT" <<'PY'
import sys, os
# Prefer .specify/feature.json (written by /speckit-specify) over mtime heuristic.
_feature_json="$PROJECT_ROOT/.specify/feature.json"
if [[ -f "$_feature_json" ]]; then
_feature_dir="$("$_python" - "$_feature_json" <<'PY'
import sys, json
try:
with open(sys.argv[1], encoding="utf-8") as fh:
d = json.load(fh)
val = d.get("feature_directory", "")
print(val if isinstance(val, str) else "")
except Exception:
print("")
PY
)"
# Normalize backslashes (written by PS on Windows) to forward slashes before path ops.
_feature_dir="$(printf '%s' "$_feature_dir" | tr '\\' '/')"
_feature_dir="${_feature_dir%/}"
if [[ -n "$_feature_dir" ]]; then
# feature_directory may be relative or absolute (absolute paths outside PROJECT_ROOT
# are preserved as-is by _persist_feature_json in common.sh).
# Also match drive-qualified paths (C:/...) written by PowerShell on Windows.
if [[ "$_feature_dir" == /* ]] || [[ "$_feature_dir" =~ ^[A-Za-z]:/ ]]; then
_candidate="$_feature_dir/plan.md"
else
_candidate="$PROJECT_ROOT/$_feature_dir/plan.md"
fi
if [[ -f "$_candidate" ]]; then
# Resolve symlinks before comparing so paths like /var/… vs /private/var/…
# (macOS) are treated as equivalent. Mirrors the mtime-fallback approach.
PLAN_PATH="$("$_python" - "$PROJECT_ROOT" "$_candidate" <<'PY'
import sys
from pathlib import Path
specs = Path(sys.argv[1]) / "specs"
root = Path(sys.argv[1]).resolve()
cand = Path(sys.argv[2]).resolve()
try:
print(cand.relative_to(root).as_posix())
except ValueError:
# Outside project root: emit the resolved path in POSIX form.
# as_posix() converts backslashes correctly on native Windows Python.
print(cand.as_posix())
PY
)"
fi
fi
fi
# Fall back to mtime only when feature.json is absent or its plan does not exist yet.
# Python emits a project-relative POSIX path directly to avoid bash prefix-strip
# issues with backslash paths on Windows (Git bash / MSYS2).
if [[ -z "$PLAN_PATH" ]]; then
_plan_rel="$("$_python" - "$PROJECT_ROOT" <<'PY'
import sys
from pathlib import Path
root = Path(sys.argv[1]).resolve()
specs = root / "specs"
plans = sorted(
specs.glob("*/plan.md"),
key=lambda p: p.stat().st_mtime,
reverse=True,
)
print(plans[0] if plans else "")
if plans:
try:
print(plans[0].relative_to(root).as_posix())
except ValueError:
print("")
else:
print("")
PY
)"
if [[ -n "$_plan_abs" ]]; then
PLAN_PATH="${_plan_abs#"$PROJECT_ROOT/"}"
if [[ -n "$_plan_rel" ]]; then
PLAN_PATH="$_plan_rel"
fi
fi
fi

View File

@@ -9,6 +9,10 @@
# .specify/extensions/agent-context/agent-context-config.yml
#
# Usage: update-agent-context.ps1 [plan_path]
#
# When `plan_path` is omitted, the script derives it from `.specify/feature.json`
# (written by /speckit-specify). Falls back to the most recently modified
# `specs/*/plan.md` only when feature.json is absent or its plan does not exist yet.
[CmdletBinding()]
param(
@@ -126,14 +130,26 @@ if (-not (Test-Path -LiteralPath $ExtConfig)) {
$Options = $null
if (Get-Command ConvertFrom-Yaml -ErrorAction SilentlyContinue) {
try {
$Options = Get-Content -LiteralPath $ExtConfig -Raw | ConvertFrom-Yaml -ErrorAction Stop
$Options = Get-Content -LiteralPath $ExtConfig -Raw -Encoding UTF8 | ConvertFrom-Yaml -ErrorAction Stop
} catch {
# fall through to Python fallback
# fall through to ConvertFrom-Json fallback
}
}
if ($null -eq $Options) {
# ConvertFrom-Yaml unavailable or failed; fall back to Python+PyYAML.
# ConvertFrom-Yaml unavailable or failed; try ConvertFrom-Json (no external deps,
# works when the config file is valid JSON, which is a subset of YAML).
try {
$raw = Get-Content -LiteralPath $ExtConfig -Raw -Encoding UTF8
$Options = $raw | ConvertFrom-Json -ErrorAction Stop
if (-not (Test-ConfigObject -Object $Options)) { $Options = $null }
} catch {
$Options = $null
}
}
if ($null -eq $Options) {
# ConvertFrom-Yaml/Json unavailable or failed; fall back to Python+PyYAML.
$pythonCmd = $null
$pythonCandidates = @()
if ($env:SPECKIT_PYTHON) {
@@ -280,21 +296,69 @@ if ($cm) {
}
if (-not $PlanPath) {
# Discover plan.md exactly one level deep (specs/<feature>/plan.md),
# matching the bash glob specs/*/plan.md. Wrap in try/catch so access errors under
# $ErrorActionPreference = 'Stop' don't abort the script.
try {
$specsDir = Join-Path $ProjectRoot 'specs'
$candidate = Get-ChildItem -Path $specsDir -Directory -ErrorAction SilentlyContinue |
ForEach-Object { Get-Item -LiteralPath (Join-Path $_.FullName 'plan.md') -ErrorAction SilentlyContinue } |
Where-Object { $_ } |
Sort-Object LastWriteTime -Descending |
Select-Object -First 1
if ($candidate) {
$PlanPath = [System.IO.Path]::GetRelativePath($ProjectRoot, $candidate.FullName).Replace('\','/')
# Prefer .specify/feature.json (written by /speckit-specify) over mtime heuristic.
$FeatureJson = Join-Path $ProjectRoot '.specify/feature.json'
if (Test-Path -LiteralPath $FeatureJson) {
try {
$fj = Get-Content -LiteralPath $FeatureJson -Raw -Encoding UTF8 | ConvertFrom-Json
$featureDir = $fj.feature_directory
if ($featureDir -isnot [string] -or -not $featureDir) {
$featureDir = $null
} else {
$featureDir = $featureDir.TrimEnd('\', '/')
}
if ($featureDir) {
# Join-Path on Unix does not treat absolute ChildPath as "wins"; check explicitly.
if ([System.IO.Path]::IsPathRooted($featureDir)) {
$candidatePlan = Join-Path $featureDir 'plan.md'
} else {
$candidatePlan = Join-Path (Join-Path $ProjectRoot $featureDir) 'plan.md'
}
if (Test-Path -LiteralPath $candidatePlan) {
# Resolve ./ .. segments before relativizing (mirrors bash Path.resolve()).
# GetFullPath is available in .NET Framework 4.x (PS 5.1 compatible).
$resolvedPlan = [System.IO.Path]::GetFullPath($candidatePlan)
$resolvedDir = [System.IO.Path]::GetDirectoryName($resolvedPlan)
$normRoot = $ProjectRoot.TrimEnd('\', '/') + [System.IO.Path]::DirectorySeparatorChar
$normDir = $resolvedDir.TrimEnd('\', '/') + [System.IO.Path]::DirectorySeparatorChar
$cmp = if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { [System.StringComparison]::OrdinalIgnoreCase } else { [System.StringComparison]::Ordinal }
if ($normDir.StartsWith($normRoot, $cmp)) {
$relDir = $normDir.Substring($normRoot.Length).TrimEnd('\', '/')
$PlanPath = if ($relDir) { $relDir.Replace('\', '/') + '/plan.md' } else { 'plan.md' }
} else {
$PlanPath = $resolvedPlan.Replace('\', '/')
}
}
}
} catch {
# Non-fatal: fall through to mtime heuristic.
}
}
# Fall back to mtime only when feature.json is absent or its plan does not exist yet.
if (-not $PlanPath) {
try {
$specsDir = Join-Path $ProjectRoot 'specs'
$candidate = Get-ChildItem -Path $specsDir -Directory -ErrorAction SilentlyContinue |
ForEach-Object { Get-Item -LiteralPath (Join-Path $_.FullName 'plan.md') -ErrorAction SilentlyContinue } |
Where-Object { $_ } |
Sort-Object LastWriteTime -Descending |
Select-Object -First 1
if ($candidate) {
# GetRelativePath is .NET 5+ only; strip prefix manually for PS 5.1 compat.
# Use case-insensitive comparison on Windows only (matches common.ps1 pattern).
$fullPath = $candidate.FullName.Replace('\', '/')
$normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/'
$cmp = if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { [System.StringComparison]::OrdinalIgnoreCase } else { [System.StringComparison]::Ordinal }
if ($fullPath.StartsWith($normRoot, $cmp)) {
$PlanPath = $fullPath.Substring($normRoot.Length)
} else {
$PlanPath = $fullPath
}
}
} catch {
# Non-fatal: continue without a plan path.
}
} catch {
# Non-fatal: continue without a plan path.
}
}

View File

@@ -0,0 +1,211 @@
"""Tests that update-agent-context.sh/.ps1 prefer feature.json over mtime."""
from __future__ import annotations
import json
import os
import time
from pathlib import Path
import pytest
from tests.conftest import requires_bash
from tests.extensions.test_extension_agent_context import (
BASH,
POWERSHELL,
_bash_posix_path,
_run_bash_agent_context_script,
_run_powershell_agent_context_script,
)
def _setup_project(root: Path, context_file: str = "CLAUDE.md") -> None:
"""Write agent-context extension config as JSON.
JSON is valid YAML so bash+PyYAML can parse it, and PowerShell's built-in
ConvertFrom-Json can parse it without needing powershell-yaml or Python.
Written directly as JSON (not via yaml.safe_dump) so the PS ConvertFrom-Json
fallback actually works on Windows CI.
"""
cfg_dir = root / ".specify" / "extensions" / "agent-context"
cfg_dir.mkdir(parents=True, exist_ok=True)
(cfg_dir / "agent-context-config.yml").write_text(
json.dumps({
"context_file": context_file,
"context_markers": {
"start": "<!-- SPECKIT START -->",
"end": "<!-- SPECKIT END -->",
},
}),
encoding="utf-8",
)
def _write_feature_json(root: Path, feature_directory: str) -> None:
specify_dir = root / ".specify"
specify_dir.mkdir(parents=True, exist_ok=True)
(specify_dir / "feature.json").write_text(
json.dumps({"feature_directory": feature_directory}),
encoding="utf-8",
)
def _make_plan(root: Path, feature_dir: str, content: str = "# plan\n") -> Path:
p = root / feature_dir / "plan.md"
p.parent.mkdir(parents=True, exist_ok=True)
p.write_text(content, encoding="utf-8")
return p
@requires_bash
def test_bash_uses_feature_json_when_plan_exists(tmp_path: Path) -> None:
"""feature.json points to the active feature; that plan.md is injected."""
_setup_project(tmp_path)
_make_plan(tmp_path, "specs/001-active")
_write_feature_json(tmp_path, "specs/001-active")
result = _run_bash_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/001-active/plan.md" in ctx
@requires_bash
def test_bash_ignores_newer_stale_plan_when_feature_json_present(tmp_path: Path) -> None:
"""An older spec's plan.md modified more recently must NOT win over feature.json."""
_setup_project(tmp_path)
active = _make_plan(tmp_path, "specs/001-active")
stale = _make_plan(tmp_path, "specs/000-stale")
now = time.time()
os.utime(active, (now - 10, now - 10))
os.utime(stale, (now, now))
_write_feature_json(tmp_path, "specs/001-active")
result = _run_bash_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/001-active/plan.md" in ctx
assert "specs/000-stale/plan.md" not in ctx
@requires_bash
def test_bash_falls_back_to_mtime_when_feature_json_absent(tmp_path: Path) -> None:
"""No feature.json → mtime fallback selects the most recently modified plan."""
_setup_project(tmp_path)
old = _make_plan(tmp_path, "specs/000-old")
newer = _make_plan(tmp_path, "specs/001-newer")
now = time.time()
os.utime(old, (now - 10, now - 10))
os.utime(newer, (now, now))
result = _run_bash_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/001-newer/plan.md" in ctx
@requires_bash
def test_bash_falls_back_to_mtime_when_plan_not_yet_created(tmp_path: Path) -> None:
"""feature.json exists but plan.md not yet written → fall back to mtime."""
_setup_project(tmp_path)
_make_plan(tmp_path, "specs/000-old")
_write_feature_json(tmp_path, "specs/001-new")
result = _run_bash_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/000-old/plan.md" in ctx
@requires_bash
def test_bash_absolute_feature_dir_under_project_root(tmp_path: Path) -> None:
"""Absolute feature_directory under PROJECT_ROOT → project-relative path in context."""
_setup_project(tmp_path)
active = _make_plan(tmp_path, "specs/001-active")
stale = _make_plan(tmp_path, "specs/000-stale")
now = time.time()
os.utime(active, (now - 10, now - 10))
os.utime(stale, (now, now))
# Write POSIX absolute path — mtime would pick 000-stale without feature.json
_write_feature_json(tmp_path, _bash_posix_path(tmp_path / "specs" / "001-active"))
result = _run_bash_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/001-active/plan.md" in ctx
assert "specs/000-stale/plan.md" not in ctx
assert _bash_posix_path(tmp_path) not in ctx
@requires_bash
def test_bash_absolute_feature_dir_outside_project_root(tmp_path: Path) -> None:
"""Absolute feature_directory outside PROJECT_ROOT → absolute path preserved in context."""
project = tmp_path / "project"
external = tmp_path / "external" / "001-feature"
project.mkdir()
external.mkdir(parents=True)
(external / "plan.md").write_text("# plan\n", encoding="utf-8")
_setup_project(project)
_write_feature_json(project, _bash_posix_path(external))
result = _run_bash_agent_context_script(project)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (project / "CLAUDE.md").read_text(encoding="utf-8")
assert _bash_posix_path(external) + "/plan.md" in ctx
@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
def test_ps_uses_feature_json_when_plan_exists(tmp_path: Path) -> None:
"""PowerShell: absolute feature_directory under project root is normalized to relative path."""
_setup_project(tmp_path)
active = _make_plan(tmp_path, "specs/001-active")
stale = _make_plan(tmp_path, "specs/000-stale")
now = time.time()
os.utime(active, (now - 10, now - 10))
os.utime(stale, (now, now))
# Native str() — PowerShell expects Windows-native paths, not MSYS2 /c/... form
_write_feature_json(tmp_path, str(tmp_path / "specs" / "001-active"))
result = _run_powershell_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "at specs/001-active/plan.md" in ctx
assert "specs/000-stale/plan.md" not in ctx
assert tmp_path.resolve().as_posix() not in ctx
@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
def test_ps_ignores_newer_stale_plan_when_feature_json_present(tmp_path: Path) -> None:
"""PowerShell: stale plan touched more recently must not win over feature.json."""
_setup_project(tmp_path)
active = _make_plan(tmp_path, "specs/001-active")
stale = _make_plan(tmp_path, "specs/000-stale")
now = time.time()
os.utime(active, (now - 10, now - 10))
os.utime(stale, (now, now))
_write_feature_json(tmp_path, "specs/001-active")
result = _run_powershell_agent_context_script(tmp_path)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8")
assert "specs/001-active/plan.md" in ctx
assert "specs/000-stale/plan.md" not in ctx
@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available")
def test_ps_absolute_feature_dir_outside_project_root(tmp_path: Path) -> None:
"""PowerShell: absolute feature_directory outside project root → absolute path preserved."""
project = tmp_path / "project"
external = tmp_path / "external" / "001-feature"
project.mkdir()
external.mkdir(parents=True)
(external / "plan.md").write_text("# plan\n", encoding="utf-8")
_setup_project(project)
_write_feature_json(project, str(external))
result = _run_powershell_agent_context_script(project)
assert result.returncode == 0, result.stderr + result.stdout
ctx = (project / "CLAUDE.md").read_text(encoding="utf-8")
assert external.resolve().as_posix() + "/plan.md" in ctx