Skip to content

fix(agent-context): recurse for nested plans in Python mtime fallback - #3757

Merged
mnriem merged 2 commits into
github:mainfrom
Quratulain-bilal:fix/agent-context-py-nested-plans
Aug 6, 2026
Merged

fix(agent-context): recurse for nested plans in Python mtime fallback#3757
mnriem merged 2 commits into
github:mainfrom
Quratulain-bilal:fix/agent-context-py-nested-plans

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Fixes #3733

Problem

The agent-context Python port's mtime fallback discovered plans with a one-level glob:

(root / "specs").glob("*/plan.md")

So a scoped layout created via SPECIFY_FEATURE_DIRECTORYspecs/<scope>/<feature>/plan.md, which sits two levels below specs/ — was missed whenever feature.json is absent. The fallback returned no plan, and the managed context section omitted its at <plan> line.

The bash and PowerShell twins were already fixed to recurse in #3024 (their comments explicitly call out "rather than the old one-level specs/*/plan.md scan"); the Python twin was left behind, so the three drifted out of parity.

Fix

Switch to specs.rglob("plan.md") with the same symlink-safe containment check the bash twin uses: resolve each candidate and confirm it stays within the project root before ranking by mtime. This keeps a plan reached through a specs/ symlink pointing outside the project from being selected (the lexical relative_to alone would accept it).

Tests

Adds two parity regression tests covering a nested specs/<scope>/<feature>/plan.md with no feature.json:

  • vs bash (@requires_posix_bash, runs in CI)
  • vs PowerShell (runs on the Windows runner)

Both fail on the pre-fix one-level glob and pass after. Verified locally on Windows: the PowerShell-parity test fails without the fix (Python omits the nested plan, diverging from PowerShell) and passes with it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes nested plan discovery parity in the agent-context Python port.

Changes:

  • Recursively discovers plan.md files with symlink-safe containment.
  • Adds Bash and PowerShell parity tests for nested plans.
Show a summary per file
File Description
extensions/agent-context/scripts/python/update_agent_context.py Implements recursive plan discovery.
tests/extensions/test_update_agent_context_python_parity.py Adds nested-layout parity coverage.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread extensions/agent-context/scripts/python/update_agent_context.py
Comment thread extensions/agent-context/scripts/python/update_agent_context.py

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address Copilot feedback and fix test & lint errors

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (2)

extensions/agent-context/scripts/python/update_agent_context.py:195

  • The containment check runs only after rglob() has traversed the tree. When the top-level specs path itself is a symlink outside the project, rglob() follows that starting directory and recursively scans its external target before every candidate is rejected; for example, specs -> / can make this command walk most of the filesystem. Resolve and validate specs itself before starting recursion (and apply the same guard to the bash twin).
        candidates = []
        for p in specs.rglob("plan.md"):
            rel = _resolved_rel(p)

tests/extensions/test_update_agent_context_python_parity.py:367

  • Path.rglob() does not recurse into symlinked directories by default, so this candidate is never yielded and the test passes even if _resolved_rel() stops rejecting escaping plans. Symlink the plan.md file itself so rglob() discovers it and the new containment guard is actually exercised.
        (specs / "linked").symlink_to(outside.parent, target_is_directory=True)
        # Sanity: the plan really is reachable through the symlink.
        assert (specs / "linked" / "001-x" / "plan.md").is_file()
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

The Python port's mtime fallback discovered plans with a one-level
specs/*/plan.md glob, so a scoped layout created via
SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) was missed when
feature.json is absent — the fallback returned no plan and the managed
context section omitted the 'at <plan>' line. The bash and PowerShell twins
were already fixed to recurse (github#3024); the Python twin was left behind.

Switch to specs.rglob('plan.md') with the same symlink-safe containment check
the bash twin uses (resolve each candidate and confirm it stays within the
project root before ranking by mtime), so a plan reached through a specs/
symlink pointing outside the project is not selected.

Adds parity regression tests (vs bash and vs PowerShell) covering a nested
specs/<scope>/<feature>/plan.md; both fail on the pre-fix one-level glob.

Fixes github#3733
The recursive fallback resolves each candidate before the relative_to()
containment check, but nothing exercised that path. Add a parity test for a
plan reachable only through a specs/ symlink pointing outside the project:
relative_to() is lexical and would accept it, emitting an in-project-looking
path for an out-of-project file. Both the bash twin and the Python port skip
it, so the "at <plan>" line is omitted.

Also correct the module docstring, which still described the fallback as
scanning specs/*/plan.md one level deep.
@Quratulain-bilal
Quratulain-bilal force-pushed the fix/agent-context-py-nested-plans branch from 789648f to d71609d Compare July 28, 2026 23:27
@Quratulain-bilal

Copy link
Copy Markdown
Contributor Author

@merium Conflict resolved and pushed. Branch is now rebased on latest main. Please review!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (2)

extensions/agent-context/scripts/python/update_agent_context.py:186

  • This containment rule makes the Python port diverge from PowerShell for file symlinks. The PowerShell fallback still selects Get-ChildItem's lexical FullName after only a root-prefix check (update-agent-context.ps1:433-445), so specs/x/plan.md pointing outside is accepted there but rejected here. The repository requires the sh/ps/py variants to stay behaviorally equivalent; add the same resolved-target containment check to PowerShell (and a parity case) as part of this change.
                return p.resolve().relative_to(root)
            except (OSError, ValueError):

tests/extensions/test_update_agent_context_python_parity.py:367

  • This test does not exercise _resolved_rel: Path.rglob() does not recurse into directory symlinks by default, so both Python-backed scans omit this candidate even if the containment guard is removed. Use a regular directory containing a plan.md file symlink to an outside file; rglob will enumerate that path and the test will then fail without the new guard.
        (specs / "linked").symlink_to(outside.parent, target_is_directory=True)
        # Sanity: the plan really is reachable through the symlink.
        assert (specs / "linked" / "001-x" / "plan.md").is_file()
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@mnriem
mnriem self-requested a review August 6, 2026 19:32
@mnriem
mnriem merged commit 81d5cdb into github:main Aug 6, 2026
14 checks passed
@mnriem

mnriem commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: agent-context Python port misses nested plans (one-level glob), a parity regression of #3024

3 participants