You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(agent-context): skip non-file plan candidates in PowerShell fallback - #5283
PR #5222 fixed plan-candidate discovery in the bash and Python implementations of the agent-context extension, but the PowerShell twin was left behind. This ports that fix to update-agent-context.ps1.
What broke: the mtime fallback picked the newest specs/**/plan.md with no check on what the candidate actually is. A dangling plan.md symlink, or one pointing through a non-directory, was selected and written into the context file as the current plan.
What changed: link candidates are now probed by opening them, so only links whose target is a readable regular file are kept. Sorting uses the target mtime, like p.stat().st_mtime in the twins. Extension version bumped 1.0.3 to 1.0.4 with the catalog kept in sync.
How verified: new regression test test_powershell_mtime_fallback_selects_only_in_project_files covers regular files, directories, dangling links, non-directory links, directory links, and file links. It fails on main (4 cases) and passes with this change (12/12). Full parity files pass (148 passed, 5 skipped), agent config consistency passes, ruff clean, and the extension version guard passes. The test runs under PowerShell 7.5 on Linux here; Windows PowerShell 5.1 validation is left to CI, since no Windows host is available in this environment.
Two known limits, both from Windows PowerShell 5.1 having no link-target resolution API: a file link is reported at its in-project link path rather than the resolved target path the twins emit, and the escaping-link containment skip from #5222 could not be ported, so that case stays covered by the bash/python test only.
Port the bash/python mtime-fallback fix from github#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.
Fixesgithub#5282
…owerShell fallback
Address the Copilot review on github#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.
… 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.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Critical symlink-resolution flaws can select incorrect plans, bypass containment checks to modify external files, and cause exponential processing that stalls the updater.
A link target such as ../../plans/not-a-directory/../current.md, where not-a-directory is a regular file, is reduced to plans/current.md even though filesystem lookup should fail. If that file exists, the fallback selects it as the plan instead of rejecting the invalid link. Validate intermediate components before consuming .., skip invalid candidates without discarding valid ones, and add this case to the non-directory-link regressions.
Reject unresolved link chains before processing parent segments
If a directory-link chain exceeds $Depth before a .. segment, this removes an unresolved component and can select the wrong file. For example, twelve links ending at plans/archive/nested make plans/alias01/../current.md resolve to plans/current.md; the outer loop then stabilizes on that incorrect path. Propagate incomplete resolution and reject it before consuming further segments, with a regression for a link chain followed by ...
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unix backslash paths can cause containment validation to inspect a different target from the one written, while a shared link-follow budget can suppress later valid plans.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Link resolution mishandles whitespace-only path components and accepts partially resolved paths at its depth limit, allowing containment checks to pass before context files are written outside the project.
…t -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.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The context-file containment check can accept long symlink chains that escape the project, allowing subsequent writes to overwrite a file outside the project.
… in PowerShell fallback
Copilot round 6 on github#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.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The resolver mishandles .. after exhausting its link-recursion limit, allowing an incorrect path to appear in-project and context-file writes to modify files outside the project.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because path-resolution edge cases in symlinked checkouts and malformed link targets can reject valid links or select broken ones.
Addressed. The latest Copilot finding (depth-limit handling of .. in link resolution) is fixed in a44fd01, with a regression test covering depth exhaustion followed by ... The earlier rounds are fixed in the previous pushes as well.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mnriem
added
the
triage-must-have
Fixes #5282
PR #5222 fixed plan-candidate discovery in the bash and Python implementations of the agent-context extension, but the PowerShell twin was left behind. This ports that fix to
update-agent-context.ps1.What broke: the mtime fallback picked the newest
specs/**/plan.mdwith no check on what the candidate actually is. A danglingplan.mdsymlink, or one pointing through a non-directory, was selected and written into the context file as the current plan.What changed: link candidates are now probed by opening them, so only links whose target is a readable regular file are kept. Sorting uses the target mtime, like
p.stat().st_mtimein the twins. Extension version bumped 1.0.3 to 1.0.4 with the catalog kept in sync.How verified: new regression test
test_powershell_mtime_fallback_selects_only_in_project_filescovers regular files, directories, dangling links, non-directory links, directory links, and file links. It fails on main (4 cases) and passes with this change (12/12). Full parity files pass (148 passed, 5 skipped), agent config consistency passes, ruff clean, and the extension version guard passes. The test runs under PowerShell 7.5 on Linux here; Windows PowerShell 5.1 validation is left to CI, since no Windows host is available in this environment.Two known limits, both from Windows PowerShell 5.1 having no link-target resolution API: a file link is reported at its in-project link path rather than the resolved target path the twins emit, and the escaping-link containment skip from #5222 could not be ported, so that case stays covered by the bash/python test only.