fix(agent-context): discover nested plan.md in scoped layouts (#3024)#3301
Conversation
…#3024) The agent-context updater only looked for plan.md one level deep (specs/*/plan.md), so scoped layouts created via SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) were never picked up and no plan reference was written into the context file. Recurse into specs/ in both the bash (rglob) and PowerShell (-Recurse) scripts. In the PowerShell script, also replace [System.IO.Path]::GetRelativePath, which is .NET Core 2.1+ only and throws under Windows PowerShell 5.1 (.NET Framework); the exception was swallowed by the surrounding try/catch, leaving the plan path empty on 5.1 even when a plan was found. Compute the project-relative path by stripping the root prefix instead. Add regression tests for both scripts covering nested discovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n-discovery # Conflicts: # extensions/agent-context/scripts/bash/update-agent-context.sh # extensions/agent-context/scripts/powershell/update-agent-context.ps1
There was a problem hiding this comment.
Pull request overview
Fixes the bundled agent-context extension’s plan auto-discovery so it can find plan.md in nested/scoped spec layouts (e.g. specs/<scope>/<feature>/plan.md) and correctly write a plan reference into the managed context section.
Changes:
- Bash updater: switches plan discovery from one-level globbing to recursive discovery under
specs/. - PowerShell updater: switches to recursive discovery and replaces a
.NET-version-sensitive relative-path call with PS 5.1–compatible prefix stripping. - Adds regression tests ensuring nested
specs/scope/001-feature/plan.mdis discovered by both updaters.
Show a summary per file
| File | Description |
|---|---|
| tests/extensions/test_extension_agent_context.py | Adds regression tests covering nested/scoped plan discovery for bash and PowerShell updaters. |
| extensions/agent-context/scripts/powershell/update-agent-context.ps1 | Updates plan discovery to recurse under specs/ and uses PS 5.1–compatible path relativization logic. |
| extensions/agent-context/scripts/bash/update-agent-context.sh | Updates plan discovery to recurse under specs/ for scoped layouts. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Low
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Address Copilot review feedback on github#3301: - bash updater: the mtime fallback filtered candidates lexically via relative_to() on the *unresolved* path, so a plan reached through a specs/ symlink pointing outside the project could be selected and emit an in-project-looking path. Resolve each candidate and keep only those whose resolved path stays under root before picking the newest. - test: the nested-plan PowerShell regression targets a Windows PowerShell 5.1 (.NET Framework) failure mode, but ran whatever POWERSHELL resolved to (prefers pwsh). Prefer powershell.exe on Windows so the 5.1-only compat fix is actually exercised. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…into fix/3024-nested-plan-discovery # Conflicts: # tests/extensions/test_extension_agent_context.py
|
Addressed both Copilot findings:
Merged in the Copilot Autofix commit for #2 and reconciled it onto the helper-based approach to avoid duplicating the subprocess call. Both nested-plan tests pass locally (bash + Windows PowerShell 5.1). |
Auto-detection now recurses (`specs/**/plan.md`) to support nested scoped layouts created via SPECIFY_FEATURE_DIRECTORY (github#3024). The update command doc still described the old one-level `specs/*/plan.md` glob, which could mislead users troubleshooting plan detection. Addresses Copilot review feedback on PR github#3301. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Please address Copilot feedback |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@mnriem kindly review changes |
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (1)
extensions/agent-context/scripts/powershell/update-agent-context.ps1:447
- The recursive plan scan can still select a plan reached via a junction/symlinked directory under
specs/that actually resolves outside the project root, because the new logic only does a lexical$ProjectRootprefix strip on$candidate.FullName. The bash implementation explicitly resolves and validates containment before accepting a candidate; PowerShell already hasResolve-ContextPath+Test-IsSubPathfor reparse-point-safe path resolution, so it should use the same approach here and skip candidates that resolve outside the project root.
$specsDir = Join-Path $ProjectRoot 'specs'
# Recurse (rather than the old one-level specs/*/plan.md scan) so scoped
# layouts created via SPECIFY_FEATURE_DIRECTORY, e.g.
# specs/<scope>/<feature>/plan.md, are still discovered when
# feature.json is absent (#3024).
$candidate = Get-ChildItem -Path $specsDir -Filter 'plan.md' -File -Recurse -ErrorAction SilentlyContinue |
Sort-Object LastWriteTime -Descending |
Select-Object -First 1
if ($candidate) {
# GetRelativePath is .NET 5+ only; strip prefix manually for PS 5.1 compat.
# Use case-insensitive comparison on Windows only (matches common.ps1 pattern).
$fullPath = $candidate.FullName.Replace('\', '/')
$normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/'
$cmp = if ([System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT) { [System.StringComparison]::OrdinalIgnoreCase } else { [System.StringComparison]::Ordinal }
if ($fullPath.StartsWith($normRoot, $cmp)) {
$PlanPath = $fullPath.Substring($normRoot.Length)
} else {
$PlanPath = $fullPath
}
}
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Low
…n-discovery # Conflicts: # extensions/agent-context/commands/speckit.agent-context.update.md # extensions/agent-context/scripts/bash/update-agent-context.sh # extensions/agent-context/scripts/powershell/update-agent-context.ps1
|
Thank you! |
Summary
Fixes #3024. The agent-context updater discovered
plan.mdonly one level deep (specs/*/plan.md), so scoped layouts created viaSPECIFY_FEATURE_DIRECTORY— e.g.specs/<scope>/<feature>/plan.md— were never found, and no plan reference was written into the context file.Changes
update-agent-context.sh):specs.glob("*/plan.md")→specs.rglob("plan.md").update-agent-context.ps1): one-levelGet-ChildItem→-Recurse.[System.IO.Path]::GetRelativePath(...)is .NET Core 2.1+ only and throws under Windows PowerShell 5.1 (.NET Framework). The exception was swallowed by the surroundingtry/catch, leaving$PlanPathempty on 5.1 even when a plan was found — so nested discovery silently produced no plan reference on the most common Windows shell. Replaced with a direct root-prefix strip, matching the script's existing path helpers.Tests
Added regression tests for both scripts (
test_bash_script_discovers_nested_plan,test_powershell_script_discovers_nested_plan) asserting aspecs/scope/001-feature/plan.mdis discovered — which the old one-level glob would miss. The PowerShell test fails without theGetRelativePathfix and passes with it.Verified locally: both new tests pass. Two unrelated bash tests in the same module (
rejects_symlink_escape,deduplicates_context_files_in_order) fail only under Git-for-Windows MSYS bash and reproduce on cleanmain— pre-existing environment artifacts, not touched by this change.