diff --git a/extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md b/extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md new file mode 100644 index 0000000000..53b1261fdf --- /dev/null +++ b/extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md @@ -0,0 +1,62 @@ +# Extension-contributed always-on instructions — prototype + evidence + +Prototype for [github/spec-kit#4200](https://github.com/github/spec-kit/issues/4200): +let an extension contribute an always-on instruction block that reaches the agent +without any command/hook invocation. Ownership follows the maintainer's decision: +**core validates the metadata only; the opt-in `agent-context` extension composes and +owns the agent-file writes.** With `agent-context` not installed, installing an +extension does not touch agent files. + +## What changed + +- **Core (`src/specify_cli/extensions/__init__.py`)** — accepts and validates a new + `provides: instructions:` capability (list of `{ file, description? }`), path-safe via + the existing `relative_extension_path_violation` guard, exposed as `.instructions`. + Core performs **no** agent-file writes. An instructions-only extension is valid. +- **`agent-context` (`scripts/python/update_agent_context.py`)** — on update, discovers + installed **and enabled** extensions (reads `.specify/extensions/.registry` + + each `extension.yml` directly, no CLI dependency), reads each `provides.instructions` + file, and merges it into the routed agent context file inside a per-extension + namespaced block: + + ``` + + …rule block… + + ``` + +- **bash / PowerShell twins** — delegate to the Python twin's new + `--emit-extension-blocks` mode, so all three produce **byte-identical** output from a + single implementation. + +## Efficacy + +The delivered payload is the **same rule block** measured in the delivery A/B. Installed +via this path, the block written to `.github/copilot-instructions.md` is **byte-identical** +to the always-on rule block that scored **+0.142 mean** best-practice conformance over bare +(vs +0.10 for the same content as on-demand commands), across 2 models × 4 languages × +3 complexity levels. Because the payload is identical, the measured lift carries over by +construction — this change is about **delivery/reachability**, not content or instruction +weighting. + +## Verification (automated) + +`tests/extensions/test_extension_instructions.py` (13 tests, all passing): + +- **Core validation** — `provides: instructions:` accepted; instructions-only extension is + valid; non-list rejected; entry missing `file` rejected; path traversal (`/abs`, `..`, + `sub/../../..`) rejected. +- **Composition** — enabled extension's block is written into the routed context file with + namespaced markers and byte-exact payload; disabling an extension removes its block on + the next update while leaving the base managed section intact; multiple extensions + coexist in deterministic id order; a path-unsafe manifest entry is skipped; **no agent + file is written when `agent-context` is not configured**; `--emit-extension-blocks` + emits the shared block text. + +Full suite: `pytest tests/extensions tests/test_extensions.py` → **673 passed, 146 skipped** +(the skips are the bash/pwsh cross-execution parity tests, which run on POSIX CI). + +Manual end-to-end (copilot integration) also confirmed: `specify extension add` a +`provides: instructions:` extension + `agent-context` → the rules appear in +`.github/copilot-instructions.md`; `disable`/`enable` remove/restore the block; a project +without `agent-context` gets no agent-file writes. diff --git a/extensions/agent-context/scripts/bash/update-agent-context.sh b/extensions/agent-context/scripts/bash/update-agent-context.sh index 7fbe3ef49a..195625bacd 100755 --- a/extensions/agent-context/scripts/bash/update-agent-context.sh +++ b/extensions/agent-context/scripts/bash/update-agent-context.sh @@ -17,6 +17,7 @@ set -euo pipefail PROJECT_ROOT="$(pwd)" +_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" EXT_CONFIG="$PROJECT_ROOT/.specify/extensions/agent-context/agent-context-config.yml" DEFAULT_START="" DEFAULT_END="" @@ -354,6 +355,13 @@ trap 'rm -f "$TMP_SECTION"' EXIT if [[ -n "$PLAN_PATH" ]]; then echo "at $PLAN_PATH" fi + # Extension-contributed always-on instruction blocks (github/spec-kit#4200). + # Delegated to the python twin's --emit-extension-blocks so all three twins + # emit byte-identical block text from a single implementation. + _EXT_BLOCKS="$("$_python" "$_SCRIPT_DIR/../python/update_agent_context.py" --emit-extension-blocks 2>/dev/null || true)" + if [[ -n "$_EXT_BLOCKS" ]]; then + printf '%s\n' "$_EXT_BLOCKS" + fi echo "$MARKER_END" } > "$TMP_SECTION" diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index 91d067cc41..332632c91f 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -457,6 +457,37 @@ $lines = @($MarkerStart, if ($PlanPath) { $lines += "at $PlanPath" } +# Extension-contributed always-on instruction blocks (github/spec-kit#4200): +# delegate to the python twin's --emit-extension-blocks so all three twins emit +# byte-identical block text from a single implementation. +$pyTwin = Join-Path (Join-Path (Join-Path $PSScriptRoot '..') 'python') 'update_agent_context.py' +$pyForBlocks = $null +foreach ($candidate in @($env:SPECKIT_PYTHON, 'python3', 'python')) { + if (-not $candidate) { continue } + if (-not (Get-Command $candidate -ErrorAction SilentlyContinue)) { continue } + # Verify the candidate is a real, runnable Python 3 (skips the Windows Store + # 'python3' alias stub, mirroring the config-parse detection above). + try { + & $candidate -c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)" 2>$null | Out-Null + if ($LASTEXITCODE -eq 0) { $pyForBlocks = $candidate; break } + } catch { } +} +if ($pyForBlocks -and (Test-Path -LiteralPath $pyTwin)) { + # Windows PowerShell decodes native-command stdout using the console code + # page; force UTF-8 so non-ASCII rule text (e.g. em-dashes) survives capture. + $prevOutEnc = [Console]::OutputEncoding + try { + [Console]::OutputEncoding = [System.Text.Encoding]::UTF8 + $emitted = (& $pyForBlocks $pyTwin --emit-extension-blocks 2>$null | Out-String) + } finally { + [Console]::OutputEncoding = $prevOutEnc + } + if ($emitted) { + $emitted = ($emitted -replace "`r`n", "`n") -replace "`r", "`n" + $emitted = $emitted.TrimEnd("`n") + foreach ($bl in ($emitted -split "`n")) { $lines += $bl } + } +} $lines += $MarkerEnd $Section = ($lines -join "`n") + "`n" diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 669ec5bf9d..6e51295f7e 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -201,7 +201,84 @@ def _resolved_rel(p: Path) -> Path | None: return plan_path -def _build_section(marker_start: str, marker_end: str, plan_path: str) -> str: +def _collect_extension_instruction_blocks(project_root: str) -> list[tuple[str, str]]: + """Collect always-on instruction blocks from installed + enabled extensions. + + Implements the agent-context side of github/spec-kit#4200: an extension that + declares ``provides.instructions`` gets its rule block composed into the + managed section. Reads ``.specify/extensions/.registry`` and each extension's + manifest directly, with no dependency on the Specify CLI (mirrors this + extension's by-design independence). Returns ``(extension_id, content)`` in + deterministic id order. Each referenced file must resolve inside its own + extension directory; anything else is skipped. Fails closed on a + present-but-unreadable registry so unregistered directories are never + admitted as enabled extensions. + """ + exts_dir = Path(project_root) / ".specify" / "extensions" + registry = exts_dir / ".registry" + if not registry.is_file(): + return [] + try: + import yaml + except ImportError: + return [] + try: + with open(registry, "r", encoding="utf-8") as fh: + reg = json.load(fh) + except (json.JSONDecodeError, OSError, UnicodeDecodeError): + return [] + if not isinstance(reg, dict) or not isinstance(reg.get("extensions"), dict): + return [] + + blocks: list[tuple[str, str]] = [] + for ext_id in sorted(reg["extensions"]): + meta = reg["extensions"][ext_id] + if not isinstance(meta, dict) or not meta.get("enabled", True): + continue + manifest = exts_dir / ext_id / "extension.yml" + if not manifest.is_file(): + continue + try: + with open(manifest, "r", encoding="utf-8") as fh: + data = yaml.safe_load(fh) + except Exception: + continue + provides = data.get("provides") if isinstance(data, dict) else None + instructions = provides.get("instructions") if isinstance(provides, dict) else None + if not isinstance(instructions, list): + continue + ext_root = (exts_dir / ext_id).resolve() + parts: list[str] = [] + for entry in instructions: + if not isinstance(entry, dict): + continue + rel = entry.get("file") + if not isinstance(rel, str) or not rel.strip(): + continue + if rel.startswith("/") or "\\" in rel or ".." in rel.split("/"): + continue + target = (ext_root / rel).resolve() + try: + target.relative_to(ext_root) + except ValueError: + continue + if not target.is_file(): + continue + try: + parts.append(target.read_text(encoding="utf-8").strip()) + except OSError: + continue + if parts: + blocks.append((ext_id, "\n\n".join(parts))) + return blocks + + +def _build_section( + marker_start: str, + marker_end: str, + plan_path: str, + extension_blocks: list[tuple[str, str]] | None = None, +) -> str: lines = [ marker_start, "For additional context about technologies to be used, project structure,", @@ -209,10 +286,28 @@ def _build_section(marker_start: str, marker_end: str, plan_path: str) -> str: ] if plan_path: lines.append(f"at {plan_path}") + # Extension-contributed always-on instruction blocks, each in its own + # namespaced sub-block so multiple extensions coexist and each can be + # regenerated or dropped independently on the next update. + lines.extend(extension_blocks or []) lines.append(marker_end) return "\n".join(lines) + "\n" +def _render_extension_block_lines(project_root: str) -> list[str]: + """Render the namespaced sub-block lines for all enabled extensions' + instruction blocks. Shared by _build_section and the --emit-extension-blocks + mode so the bash/PowerShell twins produce byte-identical output. + """ + lines: list[str] = [] + for ext_id, content in _collect_extension_instruction_blocks(project_root): + lines.append("") + lines.append(f"") + lines.append(content) + lines.append(f"") + return lines + + def ensure_mdc_frontmatter(content: str) -> str: """Ensure ``.mdc`` content has YAML frontmatter with ``alwaysApply: true``. @@ -298,6 +393,19 @@ def _upsert_section( def main(argv: list[str] | None = None) -> int: args = sys.argv[1:] if argv is None else argv project_root = os.getcwd() + + # --emit-extension-blocks: print only the composed extension instruction + # sub-block lines and exit. Used by the bash/PowerShell twins so all three + # produce identical output from this single implementation. Does not require + # the agent-context config (the twin already validated it before calling). + if "--emit-extension-blocks" in args: + block_lines = _render_extension_block_lines(project_root) + if block_lines: + # Write bytes with explicit \n so the bash/PowerShell twins receive + # identical separators regardless of OS text-mode newline translation. + sys.stdout.buffer.write("\n".join(block_lines).encode("utf-8")) + return 0 + ext_config = ( f"{project_root}/.specify/extensions/agent-context/agent-context-config.yml" ) @@ -353,7 +461,8 @@ def main(argv: list[str] | None = None) -> int: if not plan_path: plan_path = _resolve_plan_path(project_root) - section = _build_section(marker_start, marker_end, plan_path) + extension_blocks = _render_extension_block_lines(project_root) + section = _build_section(marker_start, marker_end, plan_path, extension_blocks) for context_file in context_files: ctx_path = os.path.join(project_root, context_file) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 3968e4fcbe..81ca3971c5 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -377,6 +377,12 @@ def _validate(self): commands = provides.get("commands", []) templates = provides.get("templates", []) scripts = provides.get("scripts", []) + # provides.instructions: always-on rule blocks an extension contributes to + # the agent's context file. Core only validates this metadata; the actual + # agent-file write is owned by the opt-in agent-context extension + # (github/spec-kit#4200). Installing an extension never mutates agent files + # when agent-context is absent. + instructions = provides.get("instructions", []) hooks = self.data.get("hooks") events = self.data.get("events") @@ -386,6 +392,8 @@ def _validate(self): raise ValidationError("Invalid provides.templates: expected a list") if "scripts" in provides and not isinstance(scripts, list): raise ValidationError("Invalid provides.scripts: expected a list") + if "instructions" in provides and not isinstance(instructions, list): + raise ValidationError("Invalid provides.instructions: expected a list") if "hooks" in self.data and not isinstance(hooks, dict): raise ValidationError("Invalid hooks: expected a mapping") if "events" in self.data: @@ -397,16 +405,40 @@ def _validate(self): has_events = bool(events) has_templates = bool(templates) has_scripts = bool(scripts) + has_instructions = bool(instructions) - if not has_commands and not has_hooks and not has_events and not has_templates and not has_scripts: + if ( + not has_commands + and not has_hooks + and not has_events + and not has_templates + and not has_scripts + and not has_instructions + ): raise ValidationError( "Extension must provide at least one command, hook, or event " - "(or a declared template/script)" + "(or a declared template/script/instructions block)" ) self._validate_provided_artifacts(templates, section="templates", singular="template") self._validate_provided_artifacts(scripts, section="scripts", singular="script") + # provides.instructions entries carry only a 'file' (they are not invoked, + # so unlike commands/templates they need no 'name'). Validate the path with + # the same shared safety policy used for command files. + for entry in instructions: + if not isinstance(entry, dict): + raise ValidationError( + "Each entry in 'provides.instructions' must be a mapping" + ) + if "file" not in entry: + raise ValidationError("Instruction entry missing 'file'") + reason = relative_extension_path_violation(entry["file"]) + if reason: + raise ValidationError( + f"Invalid instruction file {entry['file']!r}: {reason}" + ) + # Validate hook values (if present). # Each event is a single mapping or a list of mappings. if hooks: @@ -720,6 +752,11 @@ def scripts(self) -> List[Dict[str, Any]]: """Get list of declared scripts (provides.scripts).""" return self.data.get("provides", {}).get("scripts", []) + @property + def instructions(self) -> List[Dict[str, Any]]: + """Get list of declared always-on instruction blocks (provides.instructions).""" + return self.data.get("provides", {}).get("instructions", []) + @property def hooks(self) -> Dict[str, Any]: """Get hook definitions.""" diff --git a/tests/extensions/test_extension_instructions.py b/tests/extensions/test_extension_instructions.py new file mode 100644 index 0000000000..37f9f0ee29 --- /dev/null +++ b/tests/extensions/test_extension_instructions.py @@ -0,0 +1,258 @@ +"""Tests for extension-contributed always-on instructions (github/spec-kit#4200). + +Two layers are covered: + +1. Core manifest validation (``src/specify_cli/extensions``): the ``provides.instructions`` + capability is accepted, validated, and path-safe, and an instructions-only + extension is a valid extension. +2. The ``agent-context`` composition: on update, each installed + enabled extension's + instruction block is merged into the routed agent context file inside a + per-extension namespaced marker block, disabled/removed extensions drop out, + multiple extensions coexist deterministically, path-unsafe entries are skipped, + and nothing is written when agent-context is not configured. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +from specify_cli.extensions import ExtensionManifest, ValidationError + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +PY_TWIN = ( + PROJECT_ROOT + / "extensions" + / "agent-context" + / "scripts" + / "python" + / "update_agent_context.py" +) + +RULES_A = "# Rules A\n\n- Rule a1\n- Rule a2 with an em-dash \u2014 keep it\n" +RULES_B = "# Rules B\n\n- Rule b1\n" + + +# ── Core manifest validation ──────────────────────────────────────────────── + + +def _manifest(tmp_path: Path, provides_block: str) -> Path: + text = ( + 'schema_version: "1.0"\n' + "extension:\n" + " id: demo\n" + " name: Demo\n" + " version: \"0.1.0\"\n" + " description: d\n" + " author: a\n" + "requires:\n" + ' speckit_version: ">=0.6.0"\n' + "provides:\n" + ) + textwrap.indent(provides_block, " ") + p = tmp_path / "extension.yml" + p.write_text(text, encoding="utf-8") + return p + + +def test_instructions_capability_is_accepted(tmp_path): + m = ExtensionManifest( + _manifest( + tmp_path, + "instructions:\n - file: instructions/best-practices.md\n description: rules\n", + ) + ) + assert m.instructions == [ + {"file": "instructions/best-practices.md", "description": "rules"} + ] + + +def test_instructions_only_extension_is_valid(tmp_path): + # An extension that provides ONLY instructions (no command/hook) is valid. + m = ExtensionManifest( + _manifest(tmp_path, "instructions:\n - file: instructions/rules.md\n") + ) + assert m.instructions and not m.commands + + +def test_instructions_must_be_a_list(tmp_path): + with pytest.raises(ValidationError, match="provides.instructions: expected a list"): + ExtensionManifest(_manifest(tmp_path, "instructions:\n file: rules.md\n")) + + +def test_instruction_entry_requires_file(tmp_path): + with pytest.raises(ValidationError, match="missing 'file'"): + ExtensionManifest( + _manifest(tmp_path, "instructions:\n - description: no file here\n") + ) + + +@pytest.mark.parametrize( + "bad_path", + ["/abs/rules.md", "../escape.md", "sub/../../escape.md"], +) +def test_instruction_path_traversal_rejected(tmp_path, bad_path): + with pytest.raises(ValidationError, match="Invalid instruction file"): + ExtensionManifest( + _manifest(tmp_path, f"instructions:\n - file: {bad_path}\n") + ) + + +# ── agent-context composition ─────────────────────────────────────────────── + + +def _install_extension( + project: Path, + ext_id: str, + rules: str, + *, + enabled: bool = True, + file_rel: str = "instructions/rules.md", + declare_instructions: bool = True, +) -> None: + """Materialize an installed extension on disk + register it (no CLI needed).""" + exts = project / ".specify" / "extensions" + ext_dir = exts / ext_id + target = ext_dir / file_rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(rules, encoding="utf-8") + + provides = ( + f"provides:\n instructions:\n - file: {file_rel}\n" + if declare_instructions + else "provides:\n commands:\n - name: demo.noop\n file: cmd.md\n" + ) + (ext_dir / "extension.yml").write_text( + textwrap.dedent( + f"""\ + schema_version: "1.0" + extension: + id: {ext_id} + name: {ext_id} + version: "0.1.0" + description: d + author: a + requires: + speckit_version: ">=0.2.0" + """ + ) + + provides, + encoding="utf-8", + ) + + registry_path = exts / ".registry" + if registry_path.is_file(): + registry = json.loads(registry_path.read_text(encoding="utf-8")) + else: + registry = {"schema_version": "1.0", "extensions": {}} + registry["extensions"][ext_id] = {"version": "0.1.0", "enabled": enabled} + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text(json.dumps(registry, indent=2), encoding="utf-8") + + +def _configure_agent_context(project: Path, context_file: str = "AGENTS.md") -> None: + cfg = project / ".specify" / "extensions" / "agent-context" / "agent-context-config.yml" + cfg.parent.mkdir(parents=True, exist_ok=True) + cfg.write_text( + "context_file: {}\ncontext_files: []\n".format(context_file), + encoding="utf-8", + ) + + +def _run_update(project: Path) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(PY_TWIN)], + cwd=str(project), + capture_output=True, + text=True, + encoding="utf-8", + ) + + +def _managed_section(project: Path, context_file: str = "AGENTS.md") -> str: + p = project / context_file + return p.read_text(encoding="utf-8") if p.is_file() else "" + + +def test_enabled_extension_block_composed_into_context_file(tmp_path): + _configure_agent_context(tmp_path) + _install_extension(tmp_path, "cosmosdb", RULES_A) + + _run_update(tmp_path) + section = _managed_section(tmp_path) + + assert "" in section + assert "" in section + # Payload preserved byte-for-byte (including the em-dash). + assert RULES_A.strip() in section + + +def test_disabled_extension_block_is_removed_on_update(tmp_path): + _configure_agent_context(tmp_path) + _install_extension(tmp_path, "cosmosdb", RULES_A) + _run_update(tmp_path) + assert "EXT:cosmosdb" in _managed_section(tmp_path) + + # Flip enabled -> false and re-run: the block must disappear cleanly. + _install_extension(tmp_path, "cosmosdb", RULES_A, enabled=False) + _run_update(tmp_path) + section = _managed_section(tmp_path) + assert "EXT:cosmosdb" not in section + # Base managed section survives. + assert "" in section and "" in section + + +def test_multiple_extensions_coexist_in_id_order(tmp_path): + _configure_agent_context(tmp_path) + _install_extension(tmp_path, "zeta", RULES_B) + _install_extension(tmp_path, "alpha", RULES_A) + _run_update(tmp_path) + section = _managed_section(tmp_path) + + assert "EXT:alpha" in section and "EXT:zeta" in section + # Deterministic id ordering: alpha before zeta. + assert section.index("EXT:alpha START") < section.index("EXT:zeta START") + + +def test_path_unsafe_instruction_entry_is_skipped(tmp_path): + _configure_agent_context(tmp_path) + # Register an extension whose manifest points outside its dir; the composer + # must skip it rather than read an arbitrary file. + _install_extension(tmp_path, "evil", RULES_A, file_rel="rules.md") + manifest = tmp_path / ".specify" / "extensions" / "evil" / "extension.yml" + manifest.write_text( + manifest.read_text(encoding="utf-8").replace( + "- file: rules.md", "- file: ../../../../etc/passwd" + ), + encoding="utf-8", + ) + _run_update(tmp_path) + assert "EXT:evil" not in _managed_section(tmp_path) + + +def test_noop_when_agent_context_not_configured(tmp_path): + # No agent-context config present: the update must not write any agent file. + _install_extension(tmp_path, "cosmosdb", RULES_A) + result = _run_update(tmp_path) + assert result.returncode == 0 + assert not (tmp_path / "AGENTS.md").exists() + + +def test_emit_extension_blocks_mode(tmp_path): + # The --emit-extension-blocks mode is the single source of truth shared by the + # bash/PowerShell twins; it prints the namespaced block for enabled extensions. + _install_extension(tmp_path, "cosmosdb", RULES_A) + result = subprocess.run( + [sys.executable, str(PY_TWIN), "--emit-extension-blocks"], + cwd=str(tmp_path), + capture_output=True, + text=True, + encoding="utf-8", + ) + assert result.returncode == 0 + assert "" in result.stdout + assert RULES_A.strip() in result.stdout