mirror of
https://github.com/github/spec-kit.git
synced 2026-07-08 07:04:44 +08:00
Extract agent context updates into bundled agent-context extension (#2546)
* Initial plan * Extract agent context updates into bundled agent-context extension * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: address review comments on agent-context extension - bash: parse init-options.json with a single python3 invocation instead of three separate read_json_field calls, for parity with the PowerShell ConvertFrom-Json approach and to avoid divergent error semantics - bash: use parameter expansion to strip PROJECT_ROOT prefix from plan path instead of sed interpolation, avoiding special-character fragility - powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches the bash glob specs/*/plan.md (one level deep) — fixes cross-platform inconsistency with nested plan.md files - powershell: replace Substring+Length relative-path with [System.IO.Path]::GetRelativePath for robustness across case/PSDrive differences - __init__.py: move agent-context extension install to after save_init_options so init-options.json is present when hooks run - __init__.py: seed context_markers in init-options only when context_file is truthy; avoids noise for integrations without a context file - integrations/base.py: narrow blanket except Exception in _resolve_context_markers to ImportError / (OSError, ValueError) so unexpected bugs surface instead of being silently swallowed * fix: gate context_markers in _update_init_options_for_integration on context_file Apply the same gating logic used during `specify init`: only write context_markers to init-options.json when the integration actually has a context_file set. When switching to an integration without a context file the stale markers are removed, keeping the two init paths consistent. * fix: move context_file/context_markers from init-options.json to agent-context extension config * Potential fix for pull request finding 'Unused global variable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: clarify local import comment in agents.py * Fix remaining agent-context review findings * Fix follow-up agent-context review issues * Address review feedback: narrow except, improve PyYAML messaging, surface config-written note * Fix double-space in PyYAML install hint message * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Address latest agent-context review feedback * Harden bash config parse output handling * Clarify ImportError-only fallback comment * Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check * Remove redundant $Options = $null in PS1 catch block * Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix - Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands - Add deprecation warning (v0.12.0) in upsert_context_section() - Auto-install agent-context extension during specify init - Move context_file from init-options.json to agent-context extension config - Add tests: deprecation warning, corrupt config, constitution directives - Update file inventories across all integration tests * Address review: fix init ordering, test coverage, and hermes inventory - Move agent-context extension install after init-options.json is saved so skill registration can read ai_skills + integration key - Write extension config after install (avoids template overwriting context_file) - Fix test_defaults_when_markers_field_missing to truly test missing markers key - Update hermes tests to allow extension-installed agent-context skill * Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order - Move ensure_executable_scripts after agent-context extension install so extension scripts get execute bits set - Use preserve_markers=True on reinit to keep user-customized markers - Add Python 3 version check in PowerShell fallback (matching bash behavior) - Add sort_keys=False to yaml.safe_dump for stable config output * Address review: path traversal guards and docstring fix - Reject absolute paths and '..' segments in context_file in both bash and PowerShell scripts to prevent writes outside the project root - Fix docstring in _update_init_options_for_integration to accurately describe marker preservation behavior * Address review: strict enabled check, docstring, segment-level path traversal - Use 'is not False' for enabled check so only literal False disables - Update upsert_context_section docstring to mention disabled-extension return - Fix path traversal guards to check actual path segments, not substrings (allows filenames like 'notes..md' while rejecting '../' traversal) * Address review: UnicodeError handling, missing extension warning - Add UnicodeError to exception tuples in _load_agent_context_config and _resolve_context_markers so garbled UTF-8 config files fall back to defaults - Emit error (with reinstall command) instead of silent skip when bundled agent-context extension is not found during init * Address review: bash backslash traversal guard, wheel packaging - Reject backslash separators and Windows drive-letter paths in bash context_file validation (prevents traversal on Git-Bash/Windows) - Add extensions/agent-context to pyproject.toml force-include so the bundled extension is included in wheel builds * Address review: write extension config before init-options.json - Reorder writes in _update_init_options_for_integration so the agent-context extension config is updated first; if it fails, init-options.json remains consistent with the previous state --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
This commit is contained in:
Copilot
@@ -13,6 +13,7 @@ Provides:
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import shlex
|
||||
@@ -549,6 +550,91 @@ class IntegrationBase(ABC):
|
||||
lines.append(f"at {plan_path}")
|
||||
return "\n".join(lines)
|
||||
|
||||
@staticmethod
|
||||
def _agent_context_extension_enabled(project_root: Path) -> bool:
|
||||
"""Return whether the bundled ``agent-context`` extension is enabled.
|
||||
|
||||
The extension is the single source of truth for managing coding
|
||||
agent context/instruction files (e.g. ``CLAUDE.md``,
|
||||
``.github/copilot-instructions.md``).
|
||||
|
||||
Returns ``True`` (enabled) when:
|
||||
- the extension registry does not exist (legacy project, backwards
|
||||
compatibility), or
|
||||
- the registry has no ``agent-context`` entry (older project layout
|
||||
predating the extension), or
|
||||
- the entry is present and not explicitly disabled.
|
||||
|
||||
Returns ``False`` only when an entry exists with ``enabled: false``.
|
||||
"""
|
||||
registry_path = (
|
||||
project_root / ".specify" / "extensions" / ".registry"
|
||||
)
|
||||
if not registry_path.exists():
|
||||
return True
|
||||
try:
|
||||
data = json.loads(registry_path.read_text(encoding="utf-8"))
|
||||
except (OSError, ValueError, UnicodeError):
|
||||
return True
|
||||
if not isinstance(data, dict):
|
||||
return True
|
||||
extensions = data.get("extensions")
|
||||
if not isinstance(extensions, dict):
|
||||
return True
|
||||
entry = extensions.get("agent-context")
|
||||
if not isinstance(entry, dict):
|
||||
return True
|
||||
return entry.get("enabled", True) is not False
|
||||
|
||||
def _resolve_context_markers(self, project_root: Path) -> tuple[str, str]:
|
||||
"""Return the (start, end) context markers to use for *project_root*.
|
||||
|
||||
Reads ``context_markers.start`` / ``context_markers.end`` from the
|
||||
agent-context extension config
|
||||
(``.specify/extensions/agent-context/agent-context-config.yml``)
|
||||
when present. Falls back to the class-level constants
|
||||
``CONTEXT_MARKER_START`` / ``CONTEXT_MARKER_END`` when the file is
|
||||
missing, the section is absent, or the values are not non-empty
|
||||
strings.
|
||||
"""
|
||||
from .._console import console # local import to avoid cycles
|
||||
|
||||
start = self.CONTEXT_MARKER_START
|
||||
end = self.CONTEXT_MARKER_END
|
||||
config_path = (
|
||||
project_root
|
||||
/ ".specify"
|
||||
/ "extensions"
|
||||
/ "agent-context"
|
||||
/ "agent-context-config.yml"
|
||||
)
|
||||
try:
|
||||
raw = config_path.read_text(encoding="utf-8")
|
||||
cfg = yaml.safe_load(raw)
|
||||
except (OSError, UnicodeError, ValueError, yaml.YAMLError):
|
||||
return start, end
|
||||
markers = cfg.get("context_markers") if isinstance(cfg, dict) else None
|
||||
if isinstance(markers, dict):
|
||||
cm_start = markers.get("start")
|
||||
cm_end = markers.get("end")
|
||||
s_valid = isinstance(cm_start, str) and cm_start
|
||||
e_valid = isinstance(cm_end, str) and cm_end
|
||||
if not s_valid and cm_start is not None:
|
||||
console.print(
|
||||
f"[yellow]agent-context: ignoring invalid context_markers.start "
|
||||
f"({cm_start!r}), using default[/yellow]"
|
||||
)
|
||||
if not e_valid and cm_end is not None:
|
||||
console.print(
|
||||
f"[yellow]agent-context: ignoring invalid context_markers.end "
|
||||
f"({cm_end!r}), using default[/yellow]"
|
||||
)
|
||||
if s_valid:
|
||||
start = cm_start # type: ignore[assignment]
|
||||
if e_valid:
|
||||
end = cm_end # type: ignore[assignment]
|
||||
return start, end
|
||||
|
||||
def upsert_context_section(
|
||||
self,
|
||||
project_root: Path,
|
||||
@@ -557,34 +643,54 @@ class IntegrationBase(ABC):
|
||||
"""Create or update the managed section in the agent context file.
|
||||
|
||||
If the context file does not exist it is created with just the
|
||||
managed section. If it exists, the content between
|
||||
``<!-- SPECKIT START -->`` and ``<!-- SPECKIT END -->`` markers
|
||||
is replaced (or appended when no markers are found).
|
||||
managed section. If it exists, the content between the configured
|
||||
start/end markers (default ``<!-- SPECKIT START -->`` /
|
||||
``<!-- SPECKIT END -->``) is replaced, or appended when no markers
|
||||
are found. Markers are read from the agent-context extension config
|
||||
(``.specify/extensions/agent-context/agent-context-config.yml``)
|
||||
when present, falling back to the class-level constants.
|
||||
|
||||
Returns the path to the context file, or ``None`` when
|
||||
``context_file`` is not set.
|
||||
``context_file`` is not set or the ``agent-context`` extension is
|
||||
disabled.
|
||||
"""
|
||||
if not self.context_file:
|
||||
return None
|
||||
|
||||
if not self._agent_context_extension_enabled(project_root):
|
||||
return None
|
||||
|
||||
from .._console import console # local import to avoid cycles
|
||||
|
||||
console.print(
|
||||
"[yellow]Deprecation:[/yellow] Inline agent-context updates during "
|
||||
"integration setup will be disabled in v0.12.0. Context file "
|
||||
"management has moved to the bundled [bold]agent-context[/bold] "
|
||||
"extension. Run [cyan]specify extension disable agent-context[/cyan] "
|
||||
"to opt out early.",
|
||||
highlight=False,
|
||||
)
|
||||
|
||||
marker_start, marker_end = self._resolve_context_markers(project_root)
|
||||
|
||||
ctx_path = project_root / self.context_file
|
||||
section = (
|
||||
f"{self.CONTEXT_MARKER_START}\n"
|
||||
f"{marker_start}\n"
|
||||
f"{self._build_context_section(plan_path)}\n"
|
||||
f"{self.CONTEXT_MARKER_END}\n"
|
||||
f"{marker_end}\n"
|
||||
)
|
||||
|
||||
if ctx_path.exists():
|
||||
content = ctx_path.read_text(encoding="utf-8-sig")
|
||||
start_idx = content.find(self.CONTEXT_MARKER_START)
|
||||
start_idx = content.find(marker_start)
|
||||
end_idx = content.find(
|
||||
self.CONTEXT_MARKER_END,
|
||||
marker_end,
|
||||
start_idx if start_idx != -1 else 0,
|
||||
)
|
||||
|
||||
if start_idx != -1 and end_idx != -1 and end_idx > start_idx:
|
||||
# Replace existing section (include the end marker + newline)
|
||||
end_of_marker = end_idx + len(self.CONTEXT_MARKER_END)
|
||||
end_of_marker = end_idx + len(marker_end)
|
||||
# Consume trailing line ending (CRLF or LF)
|
||||
if end_of_marker < len(content) and content[end_of_marker] == "\r":
|
||||
end_of_marker += 1
|
||||
@@ -596,7 +702,7 @@ class IntegrationBase(ABC):
|
||||
new_content = content[:start_idx] + section
|
||||
elif end_idx != -1:
|
||||
# Corrupted: end marker without start — replace BOF through end marker
|
||||
end_of_marker = end_idx + len(self.CONTEXT_MARKER_END)
|
||||
end_of_marker = end_idx + len(marker_end)
|
||||
if end_of_marker < len(content) and content[end_of_marker] == "\r":
|
||||
end_of_marker += 1
|
||||
if end_of_marker < len(content) and content[end_of_marker] == "\n":
|
||||
@@ -630,20 +736,27 @@ class IntegrationBase(ABC):
|
||||
"""Remove the managed section from the agent context file.
|
||||
|
||||
Returns ``True`` if the section was found and removed. If the
|
||||
file becomes empty (or whitespace-only) after removal it is
|
||||
deleted.
|
||||
file becomes empty (or whitespace-only) after removal it is deleted.
|
||||
Markers are read from the agent-context extension config
|
||||
(``.specify/extensions/agent-context/agent-context-config.yml``)
|
||||
when present, falling back to the class-level constants.
|
||||
"""
|
||||
if not self.context_file:
|
||||
return False
|
||||
|
||||
if not self._agent_context_extension_enabled(project_root):
|
||||
return False
|
||||
|
||||
ctx_path = project_root / self.context_file
|
||||
if not ctx_path.exists():
|
||||
return False
|
||||
|
||||
marker_start, marker_end = self._resolve_context_markers(project_root)
|
||||
|
||||
content = ctx_path.read_text(encoding="utf-8-sig")
|
||||
start_idx = content.find(self.CONTEXT_MARKER_START)
|
||||
start_idx = content.find(marker_start)
|
||||
end_idx = content.find(
|
||||
self.CONTEXT_MARKER_END,
|
||||
marker_end,
|
||||
start_idx if start_idx != -1 else 0,
|
||||
)
|
||||
|
||||
@@ -654,7 +767,7 @@ class IntegrationBase(ABC):
|
||||
return False
|
||||
|
||||
removal_start = start_idx
|
||||
removal_end = end_idx + len(self.CONTEXT_MARKER_END)
|
||||
removal_end = end_idx + len(marker_end)
|
||||
|
||||
# Consume trailing line ending (CRLF or LF)
|
||||
if removal_end < len(content) and content[removal_end] == "\r":
|
||||
|
||||
Reference in New Issue
Block a user