From 13d3e224ab201bcfade53a2ddab8715e0b87f80a Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 01:41:23 +0000 Subject: [PATCH 1/8] fix(agent-context): skip non-file plan candidates in PowerShell fallback Port the bash/python mtime-fallback fix from #5222 to the PowerShell twin: probe each symlinked candidate by opening it, so dangling links, links to directories, and links through non-directories are skipped, and sort by the target mtime. Bump agent-context 1.0.3 to 1.0.4. Fixes #5282 --- extensions/agent-context/extension.yml | 2 +- .../powershell/update-agent-context.ps1 | 26 +++++- extensions/catalog.json | 2 +- ...test_update_agent_context_python_parity.py | 86 +++++++++++++++++++ 4 files changed, 113 insertions(+), 3 deletions(-) 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..938fdb4098 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -430,8 +430,32 @@ if (-not $PlanPath) { # layouts created via SPECIFY_FEATURE_DIRECTORY, e.g. # specs///plan.md, are still discovered when # feature.json is absent (#3024). + # Mirror the bash/python mtime fallback (#5222): follow file links + # and keep only candidates whose target is a regular file. -File + # already excludes directories (including links to directories), + # but a dangling link still passes it, so probe each link target + # by opening it: the open follows the link and fails for + # dangling links, links to directories, and links through + # non-directories. Only links are probed, so plain files are + # never opened. Sort by the target's mtime, like + # p.stat().st_mtime in the twins: GetLastWriteTime follows links + # on Windows; on Linux it reports the link's own time, matching + # the previous behavior there. $candidate = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue | - Sort-Object LastWriteTime -Descending | + Where-Object { + if ($_.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { + try { + $probe = [System.IO.File]::OpenRead($_.FullName) + $probe.Dispose() + $true + } catch { + $false + } + } else { + $true + } + } | + Sort-Object { [System.IO.File]::GetLastWriteTime($_.FullName) } -Descending | Select-Object -First 1 if ($candidate) { # GetRelativePath is .NET 5+ only; strip prefix manually for PS 5.1 compat. 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..4cfe33734e 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -845,6 +845,92 @@ 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 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", + ], +) +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 target is a regular file are + eligible, and ordering uses the target's mtime. Two PowerShell-specific + differences from the twins, both inherent to Windows PowerShell 5.1: + + - A file link is reported at its in-project link path, not the resolved + target path: 5.1 has no link-target resolution API (ResolveLinkTarget + is .NET 6+), so the twins' resolved path cannot be reproduced here. + - The escaping-link case (target outside the project) is not covered: + without link-target resolution the containment skip from #5222 cannot + be ported, so the script reports the in-project link path instead. + That gap is documented in the fix; the bash/python test covers it. + """ + 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 == "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 != "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 == "file-link": + # Valid file links are selected, at the in-project link path + # (5.1 cannot resolve the target path like the twins do). + os.utime(target, (1_700_000_000, 1_700_000_000)) + expected_plan = "specs/999-candidate/plan.md" + + result = run_powershell(repo) + + 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 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 f91dce3e3a5017146ef1b83b884de0658636e9d7 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 03:10:35 +0000 Subject: [PATCH 2/8] fix(agent-context): resolve link chains and sort by target mtime in PowerShell fallback Address the Copilot review on #5283: the mtime fallback now resolves the complete link chain via Resolve-ContextPath, skips candidates that resolve outside the project and reports the resolved project-relative path, rejects special files from target metadata instead of opening links (a named pipe with no writer hung the update), sorts by the resolved target's mtime, and propagates unrelated I/O failures instead of silently reporting an older plan. The mtime fallback tests now run under WINDOWS_POWERSHELL so the 5.1-specific path is exercised on Windows. --- .../powershell/update-agent-context.ps1 | 80 +++++++++++------ ...test_update_agent_context_python_parity.py | 89 +++++++++++++++---- 2 files changed, 122 insertions(+), 47 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 938fdb4098..3b6492ce77 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -431,38 +431,62 @@ if (-not $PlanPath) { # specs///plan.md, are still discovered when # feature.json is absent (#3024). # Mirror the bash/python mtime fallback (#5222): follow file links - # and keep only candidates whose target is a regular file. -File - # already excludes directories (including links to directories), - # but a dangling link still passes it, so probe each link target - # by opening it: the open follows the link and fails for - # dangling links, links to directories, and links through - # non-directories. Only links are probed, so plain files are - # never opened. Sort by the target's mtime, like - # p.stat().st_mtime in the twins: GetLastWriteTime follows links - # on Windows; on Linux it reports the link's own time, matching - # the previous behavior there. - $candidate = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue | - Where-Object { - if ($_.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { - try { - $probe = [System.IO.File]::OpenRead($_.FullName) - $probe.Dispose() - $true - } catch { - $false - } - } else { - $true + # 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 { + $resolved = $_.FullName + # Resolve-ContextPath follows one link per path segment, + # so a link whose target is itself a link needs another + # pass; bound the loop so a link cycle terminates. + for ($depth = 0; $depth -lt 10; $depth++) { + $relPath = $resolved.Replace('\', '/') + $normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/' + if (-not $relPath.StartsWith($normRoot, $cmp)) { break } + $next = Resolve-ContextPath -Root $ProjectRoot -RelativePath $relPath.Substring($normRoot.Length) + if ($next -eq $resolved) { break } + $resolved = $next + } + # Skip candidates that resolve outside the project. + if (-not (Test-IsSubPath -Root $ProjectRoot -Path $resolved)) { return } + try { + $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. UnixMode (pwsh 7+ on + # Unix) reports the Unix file-type letter; Windows + # PowerShell 5.1 has no such files as link targets and + # the property is absent there, so nothing is skipped. + $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 } + [PSCustomObject]@{ + ResolvedPath = $resolved + Mtime = [System.IO.File]::GetLastWriteTime($resolved) } - } | - Sort-Object { [System.IO.File]::GetLastWriteTime($_.FullName) } -Descending | - Select-Object -First 1 + } + $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('\', '/') + $fullPath = $candidate.ResolvedPath.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 { diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 4cfe33734e..82abe15e04 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,7 +848,7 @@ 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 POWERSHELL, reason="no PowerShell available") +@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", @@ -856,6 +859,8 @@ def test_python_mtime_fallback_finds_nested_plan_matches_powershell( "non-directory-link", "directory-link", "file-link", + "escaping-link", + "fifo-link", ], ) def test_powershell_mtime_fallback_selects_only_in_project_files( @@ -864,17 +869,16 @@ def test_powershell_mtime_fallback_selects_only_in_project_files( """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 target is a regular file are - eligible, and ordering uses the target's mtime. Two PowerShell-specific - differences from the twins, both inherent to Windows PowerShell 5.1: - - - A file link is reported at its in-project link path, not the resolved - target path: 5.1 has no link-target resolution API (ResolveLinkTarget - is .NET 6+), so the twins' resolved path cannot be reproduced here. - - The escaping-link case (target outside the project) is not covered: - without link-target resolution the containment skip from #5222 cannot - be ported, so the script reports the in-project link path instead. - That gap is documented in the fix; the bash/python test covers it. + 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 = "" @@ -896,11 +900,19 @@ def test_powershell_mtime_fallback_selects_only_in_project_files( 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") @@ -910,13 +922,14 @@ def test_powershell_mtime_fallback_selects_only_in_project_files( ) except (OSError, NotImplementedError) as exc: pytest.skip(f"symlinks unavailable: {exc}") - if candidate_kind == "file-link": - # Valid file links are selected, at the in-project link path - # (5.1 cannot resolve the target path like the twins do). + if candidate_kind not in ("dangling-link", "non-directory-link", "fifo-link"): os.utime(target, (1_700_000_000, 1_700_000_000)) - expected_plan = "specs/999-candidate/plan.md" + 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) + result = run_powershell(repo, powershell=WINDOWS_POWERSHELL) assert result.returncode == 0, result.stderr assert result.stderr == "" @@ -931,6 +944,44 @@ def test_powershell_mtime_fallback_selects_only_in_project_files( 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 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 659e88ac034d0b0425f95eb1ec3a05f50b11e46f Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 03:59:05 +0000 Subject: [PATCH 3/8] fix(agent-context): resolve link targets component-wise in PowerShell fallback Resolve-ContextPath collapsed '..' in link targets before resolving their symlinked parents, used case-insensitive -eq for the resolution loop, and accepted candidates when the loop exhausted without stabilizing. Walk link targets segment by segment, compare with the platform-aware comparison, and reject candidates whose chain never stabilizes. --- .../powershell/update-agent-context.ps1 | 68 +++++++--- ...test_update_agent_context_python_parity.py | 120 ++++++++++++++++++ 2 files changed, 173 insertions(+), 15 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 3b6492ce77..49ccd4d0c1 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -109,20 +109,43 @@ function Test-ConfigObject { function Resolve-ContextPath { param( [Parameter(Mandatory = $true)][string]$Root, - [Parameter(Mandatory = $true)][string]$RelativePath + [Parameter(Mandatory = $true)][string]$RelativePath, + [string]$Base = $null, + [int]$Depth = 10 ) - $rootFull = [System.IO.Path]::GetFullPath($Root) - $segments = $RelativePath -split '/' - $resolved = $rootFull - - foreach ($segment in $segments) { + # 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) + + foreach ($segment in ($RelativePath -split '[/\\]')) { if ([string]::IsNullOrWhiteSpace($segment) -or $segment -eq '.') { continue } + if ($segment -eq '..') { + # Parent of the resolved path, clamped at the filesystem + # root (Split-Path returns empty above it). Never collapse + # '..' against an unresolved spelling. + $parent = Split-Path -Parent $resolved + if ($parent) { + $resolved = $parent + } + continue + } + $candidate = [System.IO.Path]::GetFullPath((Join-Path $resolved $segment)) - if (Test-Path -LiteralPath $candidate) { + if ($Depth -gt 0 -and (Test-Path -LiteralPath $candidate)) { $item = Get-Item -LiteralPath $candidate -Force if ($item.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { $target = $item.Target @@ -131,11 +154,14 @@ function Resolve-ContextPath { } if ($target) { if ([System.IO.Path]::IsPathRooted($target)) { - $candidate = [System.IO.Path]::GetFullPath($target) + # Absolute target: walk its components from the + # filesystem root, not from the project root. + $targetRoot = [System.IO.Path]::GetPathRoot($target) + $candidate = Resolve-ContextPath -Root $Root -RelativePath $target.Substring($targetRoot.Length) -Base $targetRoot -Depth ($Depth - 1) } else { - $candidate = [System.IO.Path]::GetFullPath( - (Join-Path (Split-Path -Parent $candidate) $target) - ) + # Relative target: walk its components from the + # link's parent dir ($resolved here). + $candidate = Resolve-ContextPath -Root $Root -RelativePath $target -Base $resolved -Depth ($Depth - 1) } } } @@ -447,17 +473,29 @@ if (-not $PlanPath) { $resolvedCandidates = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue | ForEach-Object { $resolved = $_.FullName - # Resolve-ContextPath follows one link per path segment, - # so a link whose target is itself a link needs another - # pass; bound the loop so a link cycle terminates. + # 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 = $resolved.Replace('\', '/') $normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/' if (-not $relPath.StartsWith($normRoot, $cmp)) { break } $next = Resolve-ContextPath -Root $ProjectRoot -RelativePath $relPath.Substring($normRoot.Length) - if ($next -eq $resolved) { 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 { diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 82abe15e04..98f6ddb92c 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -982,6 +982,126 @@ def test_powershell_mtime_fallback_sorts_by_target_mtime(tmp_path: Path) -> None 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 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 29ad13a33e5854d6a8bc9979d788540fac92f171 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 05:10:08 +0000 Subject: [PATCH 4/8] fix(agent-context): keep Unix backslashes and bound link follows in PowerShell fallback --- .../powershell/update-agent-context.ps1 | 150 +++++++++++++++--- ...test_update_agent_context_python_parity.py | 113 +++++++++++++ 2 files changed, 243 insertions(+), 20 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 49ccd4d0c1..b08de5b48f 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -106,6 +106,37 @@ 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. + 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, @@ -128,41 +159,75 @@ function Resolve-ContextPath { } $resolved = [System.IO.Path]::GetFullPath($Base) - foreach ($segment in ($RelativePath -split '[/\\]')) { + # 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)) { if ([string]::IsNullOrWhiteSpace($segment) -or $segment -eq '.') { continue } if ($segment -eq '..') { # Parent of the resolved path, clamped at the filesystem - # root (Split-Path returns empty above it). Never collapse - # '..' against an unresolved spelling. - $parent = Split-Path -Parent $resolved + # 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. + $parent = [System.IO.Path]::GetDirectoryName($resolved) if ($parent) { $resolved = $parent } continue } - $candidate = [System.IO.Path]::GetFullPath((Join-Path $resolved $segment)) - if ($Depth -gt 0 -and (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) + if ($Depth -gt 0) { + # 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)) { + $target = Get-LinkTarget -Path $candidate if ($target) { + # Shared total link-follow budget: each followed + # link spends one unit, across 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 + } + $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) - $candidate = Resolve-ContextPath -Root $Root -RelativePath $target.Substring($targetRoot.Length) -Base $targetRoot -Depth ($Depth - 1) + $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). - $candidate = Resolve-ContextPath -Root $Root -RelativePath $target -Base $resolved -Depth ($Depth - 1) + $followed = Resolve-ContextPath -Root $Root -RelativePath $target -Base $resolved -Depth ($Depth - 1) + } + if ($null -eq $followed) { + return $null } + $candidate = $followed } } } @@ -192,6 +257,27 @@ 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 +} + +# Total link follows allowed per run, shared across every +# Resolve-ContextPath call and fallback pass. Initialized here, before +# the first resolution below; the per-chain depth bound alone cannot +# stop a structure that re-expands the same targets. +$script:LinkFollowBudgetRemaining = 1000 + $ErrorActionPreference = 'Stop' $DefaultStart = '' $DefaultEnd = '' @@ -388,7 +474,7 @@ foreach ($ContextFile in $ContextFiles) { exit 1 } $resolvedTarget = Resolve-ContextPath -Root $ProjectRoot -RelativePath $ContextFile - if (-not (Test-IsSubPath -Root $ProjectRoot -Path $resolvedTarget)) { + if ($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 } @@ -479,10 +565,14 @@ if (-not $PlanPath) { # never stabilizes and is rejected below. $stabilized = $false for ($depth = 0; $depth -lt 10; $depth++) { - $relPath = $resolved.Replace('\', '/') - $normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/' + $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 @@ -502,7 +592,27 @@ if (-not $PlanPath) { $target = Get-Item -LiteralPath $resolved -Force -ErrorAction Stop } catch [System.IO.FileNotFoundException] { return } catch [System.IO.DirectoryNotFoundException] { return } - catch [System.Management.Automation.ItemNotFoundException] { return } + catch [System.Management.Automation.ItemNotFoundException] { + # Get-Item normalizes backslashes on Unix, so a + # resolved path containing one is reported missing + # even when it exists. Re-check with .NET, which + # preserves Unix filename semantics. + if ($resolved.Contains('\') -and [System.Environment]::OSVersion.Platform -ne [System.PlatformID]::Win32NT) { + try { + $attributes = [System.IO.File]::GetAttributes($resolved) + } catch { return } + if ($attributes -band [System.IO.FileAttributes]::Directory) { return } + if ($attributes -band [System.IO.FileAttributes]::ReparsePoint) { return } + # No UnixMode check is possible here: .NET + # reports no Unix file type, and the provider + # cannot see the path. 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 { + 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 @@ -523,8 +633,8 @@ if (-not $PlanPath) { $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. - $fullPath = $candidate.ResolvedPath.Replace('\', '/') - $normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/' + $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/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 98f6ddb92c..42d5e3025d 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -1102,6 +1102,119 @@ def test_powershell_mtime_fallback_rejects_depth_limit_exhaustion( 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 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 e4165bb36c81a9fac3f6a9f3278fc8d184a86d81 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 06:23:46 +0000 Subject: [PATCH 5/8] fix(agent-context): use literal Unix paths and per-candidate link budget in PowerShell fallback --- .../powershell/update-agent-context.ps1 | 73 +++++++--- ...test_update_agent_context_python_parity.py | 129 ++++++++++++++++++ 2 files changed, 180 insertions(+), 22 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index b08de5b48f..8917a64480 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -117,6 +117,23 @@ function Get-LinkTarget { # 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 @@ -202,10 +219,11 @@ function Resolve-ContextPath { if ($null -ne $attributes -and ($attributes -band [System.IO.FileAttributes]::ReparsePoint)) { $target = Get-LinkTarget -Path $candidate if ($target) { - # Shared total link-follow budget: each followed - # link spends one unit, across recursive calls and - # fallback passes alike. The per-chain depth bound - # alone cannot stop a structure that re-expands the + # 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 @@ -272,11 +290,14 @@ function ConvertTo-SlashPath { return $Path } -# Total link follows allowed per run, shared across every -# Resolve-ContextPath call and fallback pass. Initialized here, before -# the first resolution below; the per-chain depth bound alone cannot -# stop a structure that re-expands the same targets. -$script:LinkFollowBudgetRemaining = 1000 +# 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 = '' @@ -473,6 +494,9 @@ foreach ($ContextFile in $ContextFiles) { Write-Warning "agent-context: context files must not contain '..' path segments; got '$ContextFile'." exit 1 } + # Fresh budget per context file: the budget is scoped to one + # resolution unit, not shared across unrelated paths. + $script:LinkFollowBudgetRemaining = $script:LinkFollowBudget $resolvedTarget = Resolve-ContextPath -Root $ProjectRoot -RelativePath $ContextFile if ($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'." @@ -558,6 +582,11 @@ if (-not $PlanPath) { $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, @@ -589,18 +618,16 @@ if (-not $PlanPath) { # Skip candidates that resolve outside the project. if (-not (Test-IsSubPath -Root $ProjectRoot -Path $resolved)) { return } try { - $target = Get-Item -LiteralPath $resolved -Force -ErrorAction Stop - } catch [System.IO.FileNotFoundException] { return } - catch [System.IO.DirectoryNotFoundException] { return } - catch [System.Management.Automation.ItemNotFoundException] { - # Get-Item normalizes backslashes on Unix, so a - # resolved path containing one is reported missing - # even when it exists. Re-check with .NET, which - # preserves Unix filename semantics. if ($resolved.Contains('\') -and [System.Environment]::OSVersion.Platform -ne [System.PlatformID]::Win32NT) { - try { - $attributes = [System.IO.File]::GetAttributes($resolved) - } catch { return } + # 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 UnixMode check is possible here: .NET @@ -610,9 +637,11 @@ if (-not $PlanPath) { # wrong plan reference, not a hang. $target = [System.IO.FileInfo]::new($resolved) } else { - return + $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 diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 42d5e3025d..b4649ed316 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -1215,6 +1215,135 @@ def test_powershell_mtime_fallback_rejects_exponential_link_expansion( 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") From 5953f1b2b3de677dbaf97461b7fac19774d11a1b Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 15:21:39 +0000 Subject: [PATCH 6/8] fix(agent-context): reject special-file plan targets on macOS via test -f The mtime fallback rejected named-pipe link targets through the UnixMode property, which proved insufficient on macOS: the fifo-link tests failed on macOS CI while passing on Linux. test -f stats the resolved target and is true only for a regular file, never opens it, and is POSIX-identical on Linux and macOS, matching the bash twin. UnixMode remains as the fallback when test is unavailable. --- .../powershell/update-agent-context.ps1 | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 8917a64480..45135c92b8 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -630,9 +630,11 @@ if (-not $PlanPath) { $attributes = [System.IO.File]::GetAttributes($resolved) if ($attributes -band [System.IO.FileAttributes]::Directory) { return } if ($attributes -band [System.IO.FileAttributes]::ReparsePoint) { return } - # No UnixMode check is possible here: .NET + # No provider metadata is available here: .NET # reports no Unix file type, and the provider - # cannot see the path. Nothing in the fallback + # 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) @@ -648,12 +650,32 @@ if (-not $PlanPath) { # 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. UnixMode (pwsh 7+ on - # Unix) reports the Unix file-type letter; Windows + # 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 - # the property is absent there, so nothing is skipped. - $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 } + # 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) From 56730150e5d19873cc65f0dd427de5c233f38d57 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 15:55:01 +0000 Subject: [PATCH 7/8] fix(agent-context): reject unresolved links and keep whitespace names in PowerShell fallback Copilot round 6 on #5283: the recursive link-target walk discarded whitespace-only components (valid Unix directory names) and collapsed '..' past missing or non-directory ancestors, letting validation accept an in-project path while the later write follows the link outside the project. The context-file caller also checked a partially resolved path only once, so a chain past the depth limit passed containment. And a link to the filesystem root aborted the resolver with a binding error on the empty relative path. Resolve-ContextPath now preserves whitespace-only components, requires the preceding component to be an existing directory before applying '..', and accepts an empty relative path as the base directory. The context-file check re-runs resolution until it stabilizes, like the mtime fallback, and rejects chains that never stabilize. The FIFO finding is already covered by the test -f fix on the current head. Five new PowerShell regression tests, red on the old code and green on the new: root link target, whitespace directory, '..' after a missing ancestor, '..' after a non-directory ancestor, and an unresolved 100-link context-file chain to an outside directory. --- .../powershell/update-agent-context.ps1 | 48 +++++- ...test_update_agent_context_python_parity.py | 140 ++++++++++++++++++ 2 files changed, 184 insertions(+), 4 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 45135c92b8..195c04d963 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -157,7 +157,11 @@ function Get-LinkTarget { 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 ) @@ -185,7 +189,11 @@ function Resolve-ContextPath { [System.IO.Path]::AltDirectorySeparatorChar ) foreach ($segment in $RelativePath.Split($separators)) { - if ([string]::IsNullOrWhiteSpace($segment) -or $segment -eq '.') { + # 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 } @@ -196,6 +204,20 @@ function Resolve-ContextPath { # 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 @@ -497,8 +519,26 @@ foreach ($ContextFile in $ContextFiles) { # Fresh budget per context file: the budget is scoped to one # resolution unit, not shared across unrelated paths. $script:LinkFollowBudgetRemaining = $script:LinkFollowBudget - $resolvedTarget = Resolve-ContextPath -Root $ProjectRoot -RelativePath $ContextFile - if ($null -eq $resolvedTarget -or -not (Test-IsSubPath -Root $ProjectRoot -Path $resolvedTarget)) { + # 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 } diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index b4649ed316..ff763d6075 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -1362,3 +1362,143 @@ 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" From a44fd016f72f50cc366ec72f2a31c62ab823a5f9 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Fri, 6 Nov 2026 16:33:56 +0000 Subject: [PATCH 8/8] fix(agent-context): return unresolved at the link depth limit in PowerShell fallback --- .../powershell/update-agent-context.ps1 | 84 ++++++++++--------- ...test_update_agent_context_python_parity.py | 49 +++++++++++ 2 files changed, 94 insertions(+), 39 deletions(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 195c04d963..2cc550d538 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -228,47 +228,53 @@ function Resolve-ContextPath { # 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) - if ($Depth -gt 0) { - # 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. + # 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 } - if ($null -ne $attributes -and ($attributes -band [System.IO.FileAttributes]::ReparsePoint)) { - $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 - } - $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 + $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 + } + $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 diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index ff763d6075..0edc170855 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -1502,3 +1502,52 @@ def test_powershell_context_file_rejects_unresolved_deep_chain( # 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"