fix: derive plan path from feature.json in update-agent-context#3069
Open
amirreza225 wants to merge 15 commits into
Open
fix: derive plan path from feature.json in update-agent-context#3069amirreza225 wants to merge 15 commits into
amirreza225 wants to merge 15 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the agent-context extension’s context refresh scripts so that, when no explicit plan_path argument is provided, they derive the active plan.md from .specify/feature.json (written by /speckit-specify) and only fall back to the “most recently modified specs/*/plan.md” heuristic when that source is unavailable or not yet present. It also adds tests to validate the new feature.json-first behavior.
Changes:
- Bash: Prefer
.specify/feature.jsonto resolve the activeplan.md, with mtime fallback retained. - PowerShell: Implement the same “feature.json-first, mtime-second” selection logic and normalize feature paths.
- Tests: Add coverage ensuring feature.json overrides the mtime heuristic (bash + PowerShell where available).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
extensions/agent-context/scripts/bash/update-agent-context.sh |
Uses .specify/feature.json to select the active plan when plan_path isn’t provided; retains mtime fallback. |
extensions/agent-context/scripts/powershell/update-agent-context.ps1 |
Mirrors bash behavior in PowerShell, including absolute-path normalization and mtime fallback. |
tests/extensions/test_update_agent_context_feature_json.py |
Adds tests validating feature.json-first selection and stale-plan avoidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
requested changes
Jun 19, 2026
mnriem
left a comment
mnriem
left a comment
Collaborator
There was a problem hiding this comment.
Please address Copilot feedback
Author
|
All three Copilot comments have been addressed in commit 80600a2:
All 70 tests pass locally. |
Author
Done |
mnriem
requested changes
Jun 19, 2026
mnriem
left a comment
mnriem
left a comment
Collaborator
There was a problem hiding this comment.
Please address Copilot feedback
Collaborator
|
Please address Copilot feedback |
Author
I'm not an agent, but I get help. 😁 |
…h strip, fix blank line
Comment on lines
+191
to
+212
| @pytest.mark.skipif(not (HAS_PWSH or _WINDOWS_POWERSHELL), reason="no PowerShell available") | ||
| def test_ps_uses_feature_json_when_plan_exists(tmp_path: Path) -> None: | ||
| """PowerShell: absolute feature_directory under project root is normalized to relative path.""" | ||
| _setup_project(tmp_path) | ||
| _make_plan(tmp_path, "specs/001-active") | ||
| # Use absolute path to exercise the normalization code path | ||
| _write_feature_json(tmp_path, str(tmp_path / "specs" / "001-active")) | ||
|
|
||
| exe = "pwsh" if HAS_PWSH else _WINDOWS_POWERSHELL | ||
| result = subprocess.run( | ||
| [exe, "-NoProfile", "-File", str(UPDATE_AGENT_CTX_PS)], | ||
| cwd=tmp_path, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| assert result.returncode == 0, result.stderr + result.stdout | ||
| ctx = (tmp_path / "CLAUDE.md").read_text(encoding="utf-8") | ||
| # Must be project-relative, not machine-specific absolute | ||
| assert "at specs/001-active/plan.md" in ctx | ||
| assert tmp_path.resolve().as_posix() not in ctx | ||
|
|
…e is actually exercised
Comment on lines
+24
to
+26
| HAS_PWSH = shutil.which("pwsh") is not None | ||
| _WINDOWS_POWERSHELL = (shutil.which("powershell.exe") or shutil.which("powershell")) if os.name == "nt" else None | ||
|
|
Comment on lines
+154
to
+156
| # Write absolute path to feature.json — mtime would pick 000-stale without it | ||
| _write_feature_json(tmp_path, str(tmp_path / "specs" / "001-active")) | ||
|
|
Comment on lines
+181
to
+183
| _setup_project(project) | ||
| _write_feature_json(project, str(external)) | ||
|
|
Comment on lines
+192
to
+194
| ctx = (project / "CLAUDE.md").read_text(encoding="utf-8") | ||
| assert (external / "plan.md").resolve().as_posix() in ctx | ||
|
|
Comment on lines
+219
to
+222
| os.utime(stale, (now, now)) | ||
| # Write absolute path to feature.json — mtime would pick 000-stale without it | ||
| _write_feature_json(tmp_path, _to_bash_path(tmp_path / "specs" / "001-active")) | ||
|
|
Comment on lines
+195
to
+199
| $normRoot = $ProjectRoot.TrimEnd('\', '/') + [System.IO.Path]::DirectorySeparatorChar | ||
| $normDir = $featureDir.Replace('/', [System.IO.Path]::DirectorySeparatorChar) | ||
| if ($normDir.StartsWith($normRoot, [System.StringComparison]::OrdinalIgnoreCase)) { | ||
| $relDir = $normDir.Substring($normRoot.Length) | ||
| } |
Comment on lines
+220
to
+223
| $fullPath = $candidate.FullName.Replace('\', '/') | ||
| $normRoot = $ProjectRoot.Replace('\', '/').TrimEnd('/') + '/' | ||
| if ($fullPath.StartsWith($normRoot, [System.StringComparison]::OrdinalIgnoreCase)) { | ||
| $PlanPath = $fullPath.Substring($normRoot.Length) |
Comment on lines
+181
to
+184
| # Must be project-relative, not machine-specific absolute | ||
| assert "specs/001-active/plan.md" in ctx | ||
| assert "specs/000-stale/plan.md" not in ctx | ||
| assert str(tmp_path) not in ctx |
Author
|
@mnriem can you please run the checks? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
update-agent-context.shandupdate-agent-context.ps1now prefer.specify/feature.json(written by/speckit-specify) over the mtime heuristic when no explicitplan_pathargument is givenspecs/*/plan.mdonly whenfeature.jsonis absent or the derivedplan.mddoes not exist yet (e.g./speckit-planhasn't run yet)feature_directoryvalues in both scripts, normalizing to project-relative paths for the context file outputJoin-Path,IsPathRootedguard (UnixJoin-Pathdoes not treat absolute ChildPaths as "wins"), and manual prefix-strip instead ofGetRelativePathFixes #3067
AI disclosure
This PR was written with AI assistance (Claude Code). All changes were reviewed, tested, and understood by me before submission.
Manual test results
Agent: Claude Code | OS/Shell: macOS/zsh
/speckit.specify→/speckit.planCLAUDE.mdreferences the correct active feature'splan.mdCLAUDE.mdstill points to active feature, not the stale oneTest plan
uv run pytest tests/extensions/test_update_agent_context_feature_json.py— 4 new bash tests pass (2 PS skipped, nopwshon CI)uv run pytest tests/test_agent_config_consistency.py tests/extensions/test_extension_agent_context.py— all 62 existing tests pass