diff --git a/extensions/agent-context/extension.yml b/extensions/agent-context/extension.yml index 8c5c189757..055f0c5754 100644 --- a/extensions/agent-context/extension.yml +++ b/extensions/agent-context/extension.yml @@ -3,7 +3,7 @@ schema_version: "1.0" extension: id: agent-context name: "Coding Agent Context" - version: "1.0.3" + version: "1.0.4" description: "Manages coding agent context/instruction files (e.g., CLAUDE.md, copilot-instructions.md) with project-specific plan references and configurable markers" author: spec-kit-core repository: https://github.com/github/spec-kit diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 91d067cc41..2cc550d538 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -106,38 +106,175 @@ function Test-ConfigObject { return $false } +function Get-LinkTarget { + param( + [Parameter(Mandatory = $true)][string]$Path + ) + + # Read a link's target. Get-Item is the Windows PowerShell 5.1 + # path, but its cmdlets normalize backslashes on Unix, so a link + # whose path contains a backslash cannot be read that way; .NET's + # LinkTarget (pwsh 7+ on Unix) preserves Unix filename semantics. + # Returns $null when the target cannot be read, so the caller + # treats the link as unresolvable instead of following a wrong path. + if ($Path.Contains('\') -and [System.Environment]::OSVersion.Platform -ne [System.PlatformID]::Win32NT) { + # Use the .NET lookup first here, not only after a provider + # exception: when the normalized spelling also exists, Get-Item + # succeeds on that entry and returns the wrong target, so the + # fallback below is never reached. A $null result means the + # literal path has no readable target; return it rather than + # falling through to the provider, which would resolve the + # normalized spelling. + $linkTargetProperty = [System.IO.FileInfo].GetProperty('LinkTarget') + if ($null -ne $linkTargetProperty) { + try { + return $linkTargetProperty.GetValue([System.IO.FileInfo]::new($Path)) + } catch { + return $null + } + } + } + try { + $item = Get-Item -LiteralPath $Path -Force -ErrorAction Stop + $target = $item.Target + if ($target -is [System.Array]) { + $target = $target[0] + } + return $target + } catch { + try { + $linkTargetProperty = [System.IO.FileInfo].GetProperty('LinkTarget') + if ($null -ne $linkTargetProperty) { + return $linkTargetProperty.GetValue([System.IO.FileInfo]::new($Path)) + } + } catch { + # Cannot read the target; fall through to $null. + } + return $null + } +} + function Resolve-ContextPath { param( [Parameter(Mandatory = $true)][string]$Root, - [Parameter(Mandatory = $true)][string]$RelativePath + # Optional, not mandatory: a link that targets a filesystem + # root resolves to an empty relative path, and a mandatory + # string parameter rejects the empty string at bind time, + # aborting the whole fallback. Empty means the base directory. + [string]$RelativePath = '', + [string]$Base = $null, + [int]$Depth = 10 ) - $rootFull = [System.IO.Path]::GetFullPath($Root) - $segments = $RelativePath -split '/' - $resolved = $rootFull + # Resolve $RelativePath segment-by-segment from $Base (the project + # root by default; a link's parent dir when following a link + # target), following one link per segment. '..' pops the last + # already-resolved component instead of being collapsed lexically: + # a symlinked parent must win over the lexical spelling, so + # plans/alias/../current.md (alias -> archive/nested) reaches + # archive/current.md, not plans/current.md. Link chains are bounded + # by $Depth so a cycle terminates; the caller re-runs until the + # result stabilizes. + if (-not $Base) { + $Base = $Root + } + $resolved = [System.IO.Path]::GetFullPath($Base) + + # Split only on the host platform's separators: on Unix a backslash + # is a valid filename character, not a separator, so splitting on it + # would turn one filename into two components and let a symlink + # target escape the containment check. + $separators = [char[]]@( + [System.IO.Path]::DirectorySeparatorChar, + [System.IO.Path]::AltDirectorySeparatorChar + ) + foreach ($segment in $RelativePath.Split($separators)) { + # Skip only empty components and '.': a whitespace-only + # component is a valid Unix directory name, so it must be + # preserved — discarding it lets a later '..' validate + # against a path the write never follows. + if ([string]::IsNullOrEmpty($segment) -or $segment -eq '.') { + continue + } - foreach ($segment in $segments) { - if ([string]::IsNullOrWhiteSpace($segment) -or $segment -eq '.') { + if ($segment -eq '..') { + # Parent of the resolved path, clamped at the filesystem + # root (GetDirectoryName returns null above it). Never + # collapse '..' against an unresolved spelling. + # GetDirectoryName, not Split-Path: the cmdlet normalizes + # backslashes on Unix, so the parent of a backslash-named + # directory would be wrong. + # Require the preceding component to be an existing + # directory first: a missing or non-directory component + # followed by '..' must not be popped lexically, or a + # broken link rewrites to an existing file and gets + # selected by mistake. Reject the resolution instead. + $precedingAttributes = $null + try { + $precedingAttributes = [System.IO.File]::GetAttributes($resolved) + } catch { + return $null + } + if (($precedingAttributes -band [System.IO.FileAttributes]::Directory) -eq 0) { + return $null + } + $parent = [System.IO.Path]::GetDirectoryName($resolved) + if ($parent) { + $resolved = $parent + } continue } - $candidate = [System.IO.Path]::GetFullPath((Join-Path $resolved $segment)) - if (Test-Path -LiteralPath $candidate) { - $item = Get-Item -LiteralPath $candidate -Force - if ($item.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { - $target = $item.Target - if ($target -is [System.Array]) { - $target = $target[0] + # Concatenate, do not Join-Path: the cmdlet normalizes + # backslashes on Unix, turning one filename into two components. + $candidate = [System.IO.Path]::GetFullPath($resolved + [System.IO.Path]::DirectorySeparatorChar + $segment) + # File.GetAttributes, not Test-Path/Get-Item: the cmdlets + # normalize backslashes on Unix and cannot see a path that + # contains one. GetAttributes reports the link itself. + $attributes = $null + try { + $attributes = [System.IO.File]::GetAttributes($candidate) + } catch { + # Missing or unreadable: not a link, keep walking. + } + if ($null -ne $attributes -and ($attributes -band [System.IO.FileAttributes]::ReparsePoint)) { + if ($Depth -le 0) { + # Depth exhausted: the link cannot be followed, so the + # rest of the path cannot be resolved reliably — a + # later '..' would collapse against the unresolved + # spelling and validate a path the write never follows. + # Return unresolved instead of walking on. + return $null + } + $target = Get-LinkTarget -Path $candidate + if ($target) { + # Per-candidate total link-follow budget: each + # followed link spends one unit, shared across + # that candidate's recursive calls and fallback + # passes alike. The per-chain depth bound alone + # cannot stop a structure that re-expands the + # same targets (L1 -> L0/L0/..., L2 -> L1/L1/...), + # so total work is capped here; when it is spent, + # the candidate cannot be resolved and the null + # propagates up the chain for the caller to reject. + if ($script:LinkFollowBudgetRemaining -le 0) { + return $null } - if ($target) { - if ([System.IO.Path]::IsPathRooted($target)) { - $candidate = [System.IO.Path]::GetFullPath($target) - } else { - $candidate = [System.IO.Path]::GetFullPath( - (Join-Path (Split-Path -Parent $candidate) $target) - ) - } + $script:LinkFollowBudgetRemaining-- + if ([System.IO.Path]::IsPathRooted($target)) { + # Absolute target: walk its components from the + # filesystem root, not from the project root. + $targetRoot = [System.IO.Path]::GetPathRoot($target) + $followed = Resolve-ContextPath -Root $Root -RelativePath $target.Substring($targetRoot.Length) -Base $targetRoot -Depth ($Depth - 1) + } else { + # Relative target: walk its components from the + # link's parent dir ($resolved here). + $followed = Resolve-ContextPath -Root $Root -RelativePath $target -Base $resolved -Depth ($Depth - 1) + } + if ($null -eq $followed) { + return $null } + $candidate = $followed } } $resolved = $candidate @@ -166,6 +303,30 @@ function Test-IsSubPath { $pathFull.StartsWith($rootFull + [System.IO.Path]::DirectorySeparatorChar, $comparison) } +function ConvertTo-SlashPath { + param( + [Parameter(Mandatory = $true)][string]$Path + ) + + # Normalize '\' to '/' for string prefix comparisons on Windows + # only: on Unix a backslash is a valid filename character and must + # be preserved, or a plan under a backslash-named directory is + # resolved at a path that does not exist. + if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { + return $Path.Replace('\', '/') + } + return $Path +} + +# Link follows allowed per resolution unit (one candidate, or one +# context file), shared across that unit's recursion and fallback +# passes. The per-chain depth bound alone cannot stop a structure +# that re-expands the same targets, so total work is capped; when it +# is spent, the unit cannot be resolved. Scoped per unit: a rejected +# candidate must not consume the budget of later ones. +$script:LinkFollowBudget = 1000 +$script:LinkFollowBudgetRemaining = $script:LinkFollowBudget + $ErrorActionPreference = 'Stop' $DefaultStart = '' $DefaultEnd = '' @@ -361,8 +522,29 @@ foreach ($ContextFile in $ContextFiles) { Write-Warning "agent-context: context files must not contain '..' path segments; got '$ContextFile'." exit 1 } - $resolvedTarget = Resolve-ContextPath -Root $ProjectRoot -RelativePath $ContextFile - if (-not (Test-IsSubPath -Root $ProjectRoot -Path $resolvedTarget)) { + # Fresh budget per context file: the budget is scoped to one + # resolution unit, not shared across unrelated paths. + $script:LinkFollowBudgetRemaining = $script:LinkFollowBudget + # Resolve the complete link chain before the containment check: + # a single Resolve-ContextPath pass stops at its depth bound and + # returns a partially resolved path, so an in-project spelling can + # pass containment while the later write follows the remaining + # links outside the project. Re-run until the result stabilizes, + # like the mtime fallback; a chain that never stabilizes (or + # leaves the project) is rejected. + $cmp = if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { [System.StringComparison]::OrdinalIgnoreCase } else { [System.StringComparison]::Ordinal } + $resolvedTarget = [System.IO.Path]::GetFullPath((Join-Path $ProjectRoot $ContextFile)) + $stabilized = $false + for ($depth = 0; $depth -lt 10; $depth++) { + $relPath = ConvertTo-SlashPath -Path $resolvedTarget + $normRoot = (ConvertTo-SlashPath -Path $ProjectRoot).TrimEnd('/') + '/' + if (-not $relPath.StartsWith($normRoot, $cmp)) { break } + $next = Resolve-ContextPath -Root $ProjectRoot -RelativePath $relPath.Substring($normRoot.Length) + if ($null -eq $next) { break } + if ([string]::Equals($next, $resolvedTarget, $cmp)) { $stabilized = $true; break } + $resolvedTarget = $next + } + if (-not $stabilized -or $null -eq $resolvedTarget -or -not (Test-IsSubPath -Root $ProjectRoot -Path $resolvedTarget)) { Write-Warning "agent-context: context file path resolves outside the project root; got '$ContextFile'." exit 1 } @@ -430,15 +612,126 @@ if (-not $PlanPath) { # layouts created via SPECIFY_FEATURE_DIRECTORY, e.g. # specs///plan.md, are still discovered when # feature.json is absent (#3024). - $candidate = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue | - Sort-Object LastWriteTime -Descending | - Select-Object -First 1 + # Mirror the bash/python mtime fallback (#5222): follow file links + # and keep only candidates whose fully resolved target is a + # regular file inside the project. Resolve the complete link + # chain and check containment before reporting: a link to a file + # outside the project must not be reported at its in-project + # link path. Eligibility is decided from target metadata only: + # the old probe opened each link, which blocks forever on a + # Unix named pipe with no writer. Sort by the resolved + # target's mtime, like p.stat().st_mtime in the twins: + # GetLastWriteTime follows links on Windows but reports the + # link's own time on Linux, so call it on the resolved path. + # Use case-insensitive comparison on Windows only (matches + # common.ps1 pattern). + $cmp = if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { [System.StringComparison]::OrdinalIgnoreCase } else { [System.StringComparison]::Ordinal } + $resolvedCandidates = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue | + ForEach-Object { + # Fresh budget per candidate: the budget is scoped + # to this candidate's recursion and fallback + # passes, so a rejected candidate cannot suppress + # later ones. + $script:LinkFollowBudgetRemaining = $script:LinkFollowBudget + $resolved = $_.FullName + # Resolve-ContextPath follows link chains up to its + # depth bound; re-run until the result stabilizes, + # so a chain longer than the bound (or a cycle) + # never stabilizes and is rejected below. + $stabilized = $false + for ($depth = 0; $depth -lt 10; $depth++) { + $relPath = ConvertTo-SlashPath -Path $resolved + $normRoot = (ConvertTo-SlashPath -Path $ProjectRoot).TrimEnd('/') + '/' + if (-not $relPath.StartsWith($normRoot, $cmp)) { break } + $next = Resolve-ContextPath -Root $ProjectRoot -RelativePath $relPath.Substring($normRoot.Length) + # A null result means the shared link-follow + # budget is spent: the chain cannot be resolved, + # so the candidate is rejected below. + if ($null -eq $next) { break } + # Platform-aware equality: PowerShell's -eq is + # case-insensitive even on Unix, so a link to a + # case-only-differing path would stop the loop + # before $resolved is updated. + if ([string]::Equals($next, $resolved, $cmp)) { $stabilized = $true; break } + $resolved = $next + } + # Reject candidates whose chain never stabilized + # within the pass bound: $resolved may still hold an + # unresolved directory link (possibly leading outside + # the project), which the lexical containment check + # below cannot see. + if (-not $stabilized) { return } + # Skip candidates that resolve outside the project. + if (-not (Test-IsSubPath -Root $ProjectRoot -Path $resolved)) { return } + try { + if ($resolved.Contains('\') -and [System.Environment]::OSVersion.Platform -ne [System.PlatformID]::Win32NT) { + # Retrieve metadata for the literal Unix path + # before using the provider lookup: Get-Item + # normalizes backslashes, so when the + # normalized spelling also exists it reports + # that entry's type instead of this file's — + # a directory at the normalized spelling + # would discard a valid regular plan. .NET + # preserves Unix filename semantics. + $attributes = [System.IO.File]::GetAttributes($resolved) + if ($attributes -band [System.IO.FileAttributes]::Directory) { return } + if ($attributes -band [System.IO.FileAttributes]::ReparsePoint) { return } + # No provider metadata is available here: .NET + # reports no Unix file type, and the provider + # cannot see the path. The regular-file check + # below still applies: test -f stats the + # literal path directly. Nothing in the fallback + # opens the file, so a special file here is a + # wrong plan reference, not a hang. + $target = [System.IO.FileInfo]::new($resolved) + } else { + $target = Get-Item -LiteralPath $resolved -Force -ErrorAction Stop + } + } catch [System.IO.FileNotFoundException] { return } + catch [System.IO.DirectoryNotFoundException] { return } + catch [System.Management.Automation.ItemNotFoundException] { return } + # Directories (including links to directories) are not plans. + if ($target -is [System.IO.DirectoryInfo]) { return } + # Still a link: dangling, or a target that could not be + # resolved (e.g. through a non-directory). + if ($target.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { return } + # Special files (named pipe, socket, device): reject from + # metadata instead of opening them. `test -f` stats the + # resolved target and is true only for a regular file; + # it never opens the file, so a named pipe with no + # writer cannot hang the update. It is the same check + # the bash twin uses, and it is POSIX-identical on + # Linux and macOS. UnixMode (pwsh 7+ on Unix) is not + # sufficient on its own: the fifo-link tests failed on + # macOS CI while passing on Linux. Windows + # PowerShell 5.1 has no such files as link targets and + # no test command, so the check runs on Unix only; if + # test is unavailable, fall back to the UnixMode letter. + if ([System.Environment]::OSVersion.Platform -ne [System.PlatformID]::Win32NT) { + $isRegularFile = $null + try { + $null = & test -f $resolved 2>$null + $isRegularFile = ($LASTEXITCODE -eq 0) + } catch { + $isRegularFile = $null + } + if ($null -eq $isRegularFile) { + $unixMode = $target.PSObject.Properties['UnixMode'] + if ($null -ne $unixMode -and $unixMode.Value -is [string] -and $unixMode.Value.Length -gt 0 -and $unixMode.Value[0] -ne '-') { return } + } elseif (-not $isRegularFile) { + return + } + } + [PSCustomObject]@{ + ResolvedPath = $resolved + Mtime = [System.IO.File]::GetLastWriteTime($resolved) + } + } + $candidate = $resolvedCandidates | Sort-Object Mtime -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 } + $fullPath = ConvertTo-SlashPath -Path $candidate.ResolvedPath + $normRoot = (ConvertTo-SlashPath -Path $ProjectRoot).TrimEnd('/') + '/' if ($fullPath.StartsWith($normRoot, $cmp)) { $PlanPath = $fullPath.Substring($normRoot.Length) } else { diff --git a/extensions/catalog.json b/extensions/catalog.json index 10ad367461..f515917cd0 100644 --- a/extensions/catalog.json +++ b/extensions/catalog.json @@ -6,7 +6,7 @@ "agent-context": { "name": "Coding Agent Context", "id": "agent-context", - "version": "1.0.3", + "version": "1.0.4", "description": "Manages coding agent context/instruction files (e.g., CLAUDE.md, copilot-instructions.md) with project-specific plan references and configurable markers", "author": "spec-kit-core", "repository": "https://github.com/github/spec-kit", diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index fe21dc4c11..0edc170855 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -29,6 +29,7 @@ BASH, EXT_DIR, POWERSHELL, + WINDOWS_POWERSHELL, _bash_posix_path, _bundled_script_env, ) @@ -65,10 +66,12 @@ def run_python(project_root: Path, *args: str) -> subprocess.CompletedProcess: ) -def run_powershell(project_root: Path, *args: str) -> subprocess.CompletedProcess: +def run_powershell( + project_root: Path, *args: str, powershell: str | None = None +) -> subprocess.CompletedProcess: return subprocess.run( [ - POWERSHELL, + powershell or POWERSHELL, "-NoProfile", "-ExecutionPolicy", "Bypass", @@ -845,6 +848,502 @@ def test_python_mtime_fallback_finds_nested_plan_matches_powershell( assert b"at specs/backend/001-nested/plan.md" in (repo_b / "AGENTS.md").read_bytes() +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.parametrize("has_regular_plan", [False, True], ids=["alone", "with-plan"]) +@pytest.mark.parametrize( + "candidate_kind", + [ + "regular-file", + "directory", + "dangling-link", + "non-directory-link", + "directory-link", + "file-link", + "escaping-link", + "fifo-link", + ], +) +def test_powershell_mtime_fallback_selects_only_in_project_files( + tmp_path: Path, has_regular_plan: bool, candidate_kind: str +) -> None: + """PowerShell mtime fallback must match the bash/python #5222 fix (#5282). + + Mirrors test_mtime_fallback_selects_only_in_project_files for the + PowerShell twin: only candidates whose fully resolved target is a + regular file inside the project are eligible, the reported path is the + resolved project-relative path, and ordering uses the target's mtime. + Links to special files (e.g. a Unix named pipe) are rejected from + target metadata, without opening them: opening a pipe with no writer + would hang the whole context update. + + Runs under WINDOWS_POWERSHELL so the 5.1-specific link resolution is + exercised on Windows instead of deferring it to CI (Copilot review on + #5283). + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + expected_plan = "" + if has_regular_plan: + expected_plan = "specs/backend/001-valid/plan.md" + plan = repo / expected_plan + plan.parent.mkdir(parents=True) + plan.write_text("# older plan\n", encoding="utf-8") + os.utime(plan, (1_600_000_000, 1_600_000_000)) + + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + if candidate_kind == "regular-file": + candidate.write_text("# newer plan\n", encoding="utf-8") + os.utime(candidate, (1_700_000_000, 1_700_000_000)) + expected_plan = "specs/999-candidate/plan.md" + elif candidate_kind == "directory": + candidate.mkdir() + os.utime(candidate, (1_700_000_000, 1_700_000_000)) + else: + target = repo / "plans/current.md" + if candidate_kind == "escaping-link": + target = tmp_path / "outside.md" + if candidate_kind == "non-directory-link": + target.parent.parent.mkdir(parents=True, exist_ok=True) + target.parent.write_text("not a directory\n", encoding="utf-8") + elif candidate_kind == "directory-link": + target.mkdir(parents=True) + elif candidate_kind == "fifo-link": + target.parent.mkdir(parents=True, exist_ok=True) + try: + os.mkfifo(target) + except (AttributeError, OSError, NotImplementedError) as exc: + pytest.skip(f"named pipes unavailable: {exc}") + elif candidate_kind != "dangling-link": + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("# linked plan\n", encoding="utf-8") + try: + candidate.symlink_to( + target, target_is_directory=candidate_kind == "directory-link" + ) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + if candidate_kind not in ("dangling-link", "non-directory-link", "fifo-link"): + os.utime(target, (1_700_000_000, 1_700_000_000)) + if candidate_kind == "file-link": + # Valid file links are selected at the resolved target path, + # like the twins. + expected_plan = "plans/current.md" + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + expected = ( + "\n" + "For additional context about technologies to be used, project structure,\n" + "shell commands, and other important information, read the current plan\n" + ) + if expected_plan: + expected += f"at {expected_plan}\n" + expected += "\n" + assert (repo / "AGENTS.md").read_bytes() == expected.encode("utf-8") + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_mtime_fallback_sorts_by_target_mtime(tmp_path: Path) -> None: + """PowerShell mtime fallback must sort by the target's mtime, not the link's. + + Regression for the Copilot review on #5283: GetLastWriteTime() on the + link path reports the link's own mtime on Linux, so a newly created + link to an older target outranked a newer regular plan. The sort key + must come from the fully resolved target. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + regular = repo / "specs/backend/001-valid/plan.md" + regular.parent.mkdir(parents=True) + regular.write_text("# newer regular plan\n", encoding="utf-8") + os.utime(regular, (1_650_000_000, 1_650_000_000)) + + target = repo / "plans/current.md" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("# older linked plan\n", encoding="utf-8") + os.utime(target, (1_600_000_000, 1_600_000_000)) + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to(target) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + try: + # Newer link, older target: only the target's mtime may decide. + os.utime(candidate, (1_700_000_000, 1_700_000_000), follow_symlinks=False) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"link mtime unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at specs/backend/001-valid/plan.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_mtime_fallback_resolves_dotdot_through_symlinked_parent( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must resolve '..' after a symlinked parent. + + Regression for the Copilot review on #5283: Resolve-ContextPath + collapsed '..' in a link target before resolving the symlinked + parent, so plans/alias/../current.md (alias -> archive/nested) + wrongly produced plans/current.md instead of archive/current.md. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + nested = repo / "plans" / "archive" / "nested" + nested.mkdir(parents=True) + target = repo / "plans" / "archive" / "current.md" + target.write_text("# linked plan\n", encoding="utf-8") + os.utime(target, (1_700_000_000, 1_700_000_000)) + alias = repo / "plans" / "alias" + try: + alias.symlink_to("archive/nested", target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to("../../plans/alias/../current.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at plans/archive/current.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="case-only paths collide on Windows") +def test_powershell_mtime_fallback_selects_case_only_link_target( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must select a case-only link target on Unix. + + Regression for the Copilot review on #5283: PowerShell's -eq is + case-insensitive even on Unix, so a link from specs/001/plan.md to + Specs/001/plan.md stopped the resolution loop before $resolved was + updated, and the ReparsePoint filter then rejected the only + candidate. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + target_dir = repo / "Specs" / "001" + target_dir.mkdir(parents=True, exist_ok=True) + link_dir = repo / "specs" / "001" + link_dir.mkdir(parents=True, exist_ok=True) + if target_dir.resolve() == link_dir.resolve(): + pytest.skip("filesystem is case-insensitive") + target = target_dir / "plan.md" + target.write_text("# linked plan\n", encoding="utf-8") + os.utime(target, (1_700_000_000, 1_700_000_000)) + candidate = link_dir / "plan.md" + try: + candidate.symlink_to(target) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at Specs/001/plan.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_mtime_fallback_rejects_depth_limit_exhaustion( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must reject chains the loop cannot finish. + + Regression for the Copilot review on #5283: when the ten-pass + resolution loop exhausts without stabilizing, $resolved can still + hold an unresolved directory link, and the lexical containment + check then accepts a file the OS resolves through the remaining + chain. A chain of 100 directory links needs more follows than the + loop performs, so the candidate must be rejected and no plan + selected. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + real = repo / "plans" / "real" + real.mkdir(parents=True) + plan = real / "plan.md" + plan.write_text("# deep plan\n", encoding="utf-8") + os.utime(plan, (1_700_000_000, 1_700_000_000)) + prev = real + for i in range(100, 0, -1): + link = repo / f"l{i}" + try: + link.symlink_to(prev, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + prev = link + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to("../../l1/plan.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + expected = ( + "\n" + "For additional context about technologies to be used, project structure,\n" + "shell commands, and other important information, read the current plan\n" + "\n" + ) + assert (repo / "AGENTS.md").read_bytes() == expected.encode("utf-8") + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="backslash is a separator on Windows") +def test_powershell_context_file_backslash_target_stays_outside( + tmp_path: Path, +) -> None: + """PowerShell context-file check must preserve Unix filename semantics. + + Regression for the Copilot review on #5283: Resolve-ContextPath split + symlink targets on both '/' and '\\', so a context-file symlink to + /tmp/project\\other/AGENTS.md (a single Unix filename containing a + backslash) was checked as /tmp/project/other/AGENTS.md, which sits + inside the project root, while the later write followed the original + symlink and modified the external file. The external file must be + unchanged. + """ + outside_dir = tmp_path / "project\\other" + outside_dir.mkdir() + external = outside_dir / "AGENTS.md" + external.write_text("# external\n", encoding="utf-8") + + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + try: + (repo / "AGENTS.md").symlink_to(external) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 1, result.stdout + result.stderr + # Write-Warning reaches stdout under pwsh -File in this harness. + assert "outside the project root" in result.stdout + assert external.read_bytes() == b"# external\n" + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="backslash is a separator on Windows") +def test_powershell_mtime_fallback_preserves_backslash_in_plan_directory( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must keep backslashes in Unix filenames. + + Regression for the Copilot review on #5283: the fallback normalized + every '\\' to '/' before resolving candidates, so a plan under a + directory whose name contains a backslash was resolved at a path + that does not exist and silently skipped. The plan must be selected + with the backslash preserved in the reported path. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + plan = repo / "specs" / "we\\ird" / "plan.md" + plan.parent.mkdir(parents=True) + plan.write_text("# plan\n", encoding="utf-8") + os.utime(plan, (1_700_000_000, 1_700_000_000)) + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at specs/we\\ird/plan.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_mtime_fallback_rejects_exponential_link_expansion( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must bound total link-follow work. + + Regression for the Copilot review on #5283: the recursion limit + bounded nesting but not total work, so links like L0 -> ., L1 -> + L0/L0/..., L2 -> L1/L1/... re-expand the same targets and resolution + grows exponentially, stalling the updater on an untrusted checkout. + A plan link into this structure must be rejected quickly, with no + plan selected. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + # L0 -> ., L1 -> 8x L0, L2 -> 8x L1, L3 -> 8x L2, L4 -> 8x L3. + # Resolving a link into L4 costs ~4,700 follows without a shared + # budget; the budget must cut it off after a small fixed number. + try: + (repo / "L0").symlink_to(".", target_is_directory=True) + for level in range(1, 5): + (repo / f"L{level}").symlink_to( + "/".join([f"L{level - 1}"] * 8), target_is_directory=True + ) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + # A real plan at the root: the exploding chain resolves here, so + # without a budget the candidate is selected; with one it is not. + root_plan = repo / "plan.md" + root_plan.write_text("# root plan\n", encoding="utf-8") + os.utime(root_plan, (1_700_000_000, 1_700_000_000)) + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to("../../L4/plan.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + started = time.monotonic() + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + elapsed = time.monotonic() - started + + assert elapsed < 20, f"link resolution took {elapsed:.1f}s" + assert result.returncode == 0, result.stderr + assert result.stderr == "" + expected = ( + "\n" + "For additional context about technologies to be used, project structure,\n" + "shell commands, and other important information, read the current plan\n" + "\n" + ) + assert (repo / "AGENTS.md").read_bytes() == expected.encode("utf-8") + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="backslash is a separator on Windows") +def test_powershell_mtime_fallback_uses_literal_backslash_link_target( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must read a backslash link's own target. + + Regression for the Copilot review on #5283: on Unix, Get-Item + normalizes literal backslashes in a link's path, so when the + normalized spelling also exists it returns that entry's Target + instead of this link's. A plan link under a backslash-named + directory must resolve to its own target, not the target of the + link at the normalized spelling. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + current = repo / "plans" / "current.md" + current.parent.mkdir(parents=True) + current.write_text("# newer plan\n", encoding="utf-8") + os.utime(current, (1_700_000_000, 1_700_000_000)) + other = repo / "plans" / "other.md" + other.write_text("# older plan\n", encoding="utf-8") + os.utime(other, (1_600_000_000, 1_600_000_000)) + literal = repo / "specs" / "we\\ird" / "plan.md" + literal.parent.mkdir(parents=True) + try: + # Two levels up from specs/we\ird (one backslash-named + # directory): the normalized spelling specs/we/ird needs three. + literal.symlink_to("../../plans/current.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + normalized = repo / "specs" / "we" / "ird" / "plan.md" + normalized.parent.mkdir(parents=True) + try: + normalized.symlink_to("../../../plans/other.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at plans/current.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_mtime_fallback_rejected_chain_does_not_suppress_valid_link( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must scope the link budget per candidate. + + Regression for the Copilot review on #5283: the link-follow budget + was shared by unrelated candidates, so once an earlier candidate + consumed it — here the expanding chain — every later valid file + link returned null and was discarded too, making selection depend + on enumeration order rather than target mtime. The chain candidate + must be enumerated first for the bug to show: the valid link is + created first and the chain candidate second, so on filesystems + that enumerate newest-first the chain comes first, and the chain + directory also sorts first alphabetically. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + target = repo / "plans" / "current.md" + target.parent.mkdir(parents=True) + target.write_text("# valid plan\n", encoding="utf-8") + os.utime(target, (1_700_000_000, 1_700_000_000)) + valid = repo / "specs" / "999-valid" / "plan.md" + valid.parent.mkdir(parents=True) + try: + valid.symlink_to("../../plans/current.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + # L0 -> ., L1 -> 8x L0, L2 -> 8x L1, L3 -> 8x L2, L4 -> 8x L3. + try: + (repo / "L0").symlink_to(".", target_is_directory=True) + for level in range(1, 5): + (repo / f"L{level}").symlink_to( + "/".join([f"L{level - 1}"] * 8), target_is_directory=True + ) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + root_plan = repo / "plan.md" + root_plan.write_text("# root plan\n", encoding="utf-8") + os.utime(root_plan, (1_700_000_000, 1_700_000_000)) + chain = repo / "specs" / "000-chain" / "plan.md" + chain.parent.mkdir(parents=True) + try: + chain.symlink_to("../../L4/plan.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + started = time.monotonic() + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + elapsed = time.monotonic() - started + + assert elapsed < 20, f"link resolution took {elapsed:.1f}s" + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at plans/current.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="backslash is a separator on Windows") +def test_powershell_mtime_fallback_backslash_plan_with_directory_at_normalized_spelling( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must check the literal backslash path. + + Regression for the Copilot review on #5283: the literal-path + metadata check ran only when Get-Item reported the candidate + missing, but on Unix Get-Item normalizes backslashes, so when the + normalized spelling also exists it reports that entry's type + instead — a directory at the normalized spelling discarded a + valid regular plan. Both spellings are present here; the literal + regular file must be selected. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + plan = repo / "specs" / "we\\ird" / "plan.md" + plan.parent.mkdir(parents=True) + plan.write_text("# plan\n", encoding="utf-8") + os.utime(plan, (1_700_000_000, 1_700_000_000)) + (repo / "specs" / "we" / "ird" / "plan.md").mkdir(parents=True) + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at specs/we\\ird/plan.md" in (repo / "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") @@ -863,3 +1362,192 @@ def test_python_upsert_matches_powershell(tmp_path: Path) -> None: assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_context_file_rejects_root_link_target( + tmp_path: Path, +) -> None: + """PowerShell context-file check must survive a link to the filesystem root. + + Regression for the Copilot review on #5283: a link whose target is + the filesystem root passes an empty relative path to the recursive + resolver, and a mandatory string parameter rejects the empty string + at bind time, aborting the script with a binding error instead of + the containment rejection. The root is outside the project, so the + run must fail with the "outside the project root" warning. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + try: + (repo / "AGENTS.md").symlink_to("/") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 1, result.stdout + result.stderr + # Write-Warning reaches stdout under pwsh -File in this harness. + assert "outside the project root" in result.stdout + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.skipif(os.name == "nt", reason="whitespace-only names are trimmed on Windows") +def test_powershell_mtime_fallback_preserves_whitespace_directory( + tmp_path: Path, +) -> None: + """PowerShell mtime fallback must keep whitespace-only directory names. + + Regression for the Copilot review on #5283: the link-target walk + discarded whitespace-only components, although they are valid Unix + directory names. A plan reached through such a directory must be + selected at its real path, not resolved at the path with the + component dropped. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + target = repo / "plans" / " " / "current.md" + target.parent.mkdir(parents=True) + target.write_text("# linked plan\n", encoding="utf-8") + os.utime(target, (1_700_000_000, 1_700_000_000)) + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to("../../plans/ /current.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + assert b"at plans/ /current.md" in (repo / "AGENTS.md").read_bytes() + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +@pytest.mark.parametrize( + "ancestor_kind", ["missing", "non-directory"], ids=["missing", "non-directory"] +) +def test_powershell_mtime_fallback_rejects_dotdot_after_bad_ancestor( + tmp_path: Path, ancestor_kind: str +) -> None: + """PowerShell mtime fallback must not pop '..' past a bad ancestor. + + Regression for the Copilot review on #5283: a missing or + non-directory component followed by '..' was removed lexically, so + a broken plan link was rewritten to an existing file and + incorrectly selected. The candidate must be rejected and no plan + selected, even though plans/current.md exists. + """ + repo = make_project(tmp_path / "project", context_file="AGENTS.md") + current = repo / "plans" / "current.md" + current.parent.mkdir(parents=True) + current.write_text("# existing plan\n", encoding="utf-8") + os.utime(current, (1_700_000_000, 1_700_000_000)) + if ancestor_kind == "non-directory": + (repo / "plans" / "file.txt").write_text("not a directory\n", encoding="utf-8") + ancestor = "file.txt" + else: + ancestor = "missing" + candidate = repo / "specs/999-candidate/plan.md" + candidate.parent.mkdir(parents=True) + try: + candidate.symlink_to(f"../../plans/{ancestor}/../current.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + expected = ( + "\n" + "For additional context about technologies to be used, project structure,\n" + "shell commands, and other important information, read the current plan\n" + "\n" + ) + assert (repo / "AGENTS.md").read_bytes() == expected.encode("utf-8") + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_context_file_rejects_unresolved_deep_chain( + tmp_path: Path, +) -> None: + """PowerShell context-file check must reject chains past the depth limit. + + Regression for the Copilot review on #5283: when link resolution + reached its depth limit, the resolver returned a partially resolved + path, and the context-file caller checked that path only once, so + an in-project spelling passed containment while the later write + followed the remaining links outside the project. A chain of 100 + directory links to an outside directory must be rejected and the + outside file left unchanged. + """ + outside_dir = tmp_path / "outside" + outside_dir.mkdir() + external = outside_dir / "AGENTS.md" + external.write_text("# external\n", encoding="utf-8") + + repo = make_project(tmp_path / "project", context_file="l1/AGENTS.md") + prev = outside_dir + for i in range(100, 0, -1): + link = repo / f"l{i}" + try: + link.symlink_to(prev, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + prev = link + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 1, result.stdout + result.stderr + # Write-Warning reaches stdout under pwsh -File in this harness. + assert "outside the project root" in result.stdout + assert external.read_bytes() == b"# external\n" + + +@pytest.mark.skipif(not WINDOWS_POWERSHELL, reason="no PowerShell available") +def test_powershell_context_file_rejects_depth_exhaustion_before_dotdot( + tmp_path: Path, +) -> None: + """PowerShell context-file check must not collapse '..' past the depth limit. + + Regression for the Copilot review on #5283: when link resolution + reached its recursion depth limit, the resolver stopped following + links but kept walking, so a '..' after the exhausted link + collapsed against the unresolved spelling and the result stabilized + at an in-project path while the later write followed the original + chain outside the project. A chain of eleven links whose tenth + target ends in '..' must be rejected and the outside file left + unchanged. + + The eleventh link points at a directory beside the test tree, so + following the chain can never loop back into the project (a target + under the test tree would make directory enumeration recurse + forever through the chain). + """ + elsewhere = tmp_path.parent / (tmp_path.name + "-elsewhere") + outside_dir = elsewhere / "outside" + outside_dir.mkdir(parents=True) + external = elsewhere / "AGENTS.md" + external.write_text("# external\n", encoding="utf-8") + + repo = make_project(tmp_path / "project", context_file="l1/AGENTS.md") + for i in range(1, 11): + link = repo / f"l{i}" + try: + if i < 10: + link.symlink_to(f"l{i + 1}") + else: + link.symlink_to("l11/..") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + try: + (repo / "l11").symlink_to(outside_dir, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) + + assert result.returncode == 1, result.stdout + result.stderr + # Write-Warning reaches stdout under pwsh -File in this harness. + assert "outside the project root" in result.stdout + assert external.read_bytes() == b"# external\n"