feat(extensions): let extensions contribute always-on instructions - #4259
feat(extensions): let extensions contribute always-on instructions#4259TheovanKraay wants to merge 4 commits into
Conversation
…ithub#4200) Adds a provides.instructions capability so an extension can ship a compact always-on rule block that reaches the agent without any command/hook invocation. Ownership per maintainer decision: core validates the metadata only; the opt-in agent-context extension composes and owns the agent-file writes (namespaced <!-- SPECKIT EXT:<id> START/END --> blocks, per-agent routing, enable/disable/remove lifecycle). No agent-file writes when agent-context is not installed. core: accept+validate provides.instructions (path-safe, instructions-only extension allowed), expose .instructions. agent-context: compose enabled extensions' instruction blocks into the routed context file; bash/ps1 twins delegate to the python twin's --emit-extension-blocks for byte-identical output. Tests: tests/extensions/test_extension_instructions.py (13). Evidence: extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md.
TheovanKraay
changed the title
There was a problem hiding this comment.
Pull request overview
Adds opt-in, always-on extension instructions composed through agent-context.
Changes:
- Validates
provides.instructionsmanifests. - Composes enabled extensions’ instruction blocks across script variants.
- Adds validation/composition tests and supporting evidence.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/extensions/__init__.py |
Adds manifest validation and accessor. |
extensions/agent-context/scripts/python/update_agent_context.py |
Collects and renders instruction blocks. |
extensions/agent-context/scripts/bash/update-agent-context.sh |
Delegates block rendering to Python. |
extensions/agent-context/scripts/powershell/update-agent-context.ps1 |
Delegates block rendering to Python. |
tests/extensions/test_extension_instructions.py |
Tests validation and composition. |
extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md |
Documents the prototype and evidence. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _collect_extension_instruction_blocks(project_root: str) -> list[tuple[str, str]]: | ||
| """Collect always-on instruction blocks from installed + enabled extensions. |
There was a problem hiding this comment.
Thanks, this is a fair observation on timing. To be precise, cleanup is implemented, it is just eventually-consistent rather than instantaneous: a disabled or removed extension's block is dropped on the next agent-context refresh (there is a passing test for that), and in the normal flow that refresh fires via the after_specify and after_plan hooks. What is deferred is an immediate trigger on extension add or remove. Implementing that would require core to invoke agent-context on lifecycle events, which crosses the separation agreed in #4200 (core validates metadata only and knows nothing about agent-context). I would rather keep that boundary clean and leave the auto-trigger as a follow-up owned by agent-context. I have clarified the trigger and lifecycle model in INSTRUCTIONS-POC-EVIDENCE.md (commit 0e46935).
| for ext_id, content in _collect_extension_instruction_blocks(project_root): | ||
| lines.append("") | ||
| lines.append(f"<!-- SPECKIT EXT:{ext_id} START -->") | ||
| lines.append(content) | ||
| lines.append(f"<!-- SPECKIT EXT:{ext_id} END -->") |
There was a problem hiding this comment.
Fixed in 0e46935. Instruction payloads that embed a managed-section marker (the outer markers or a per-extension SPECKIT EXT marker) are now rejected before rendering, so _upsert_section can no longer treat an embedded marker as the section end and strand content on disable or remove. Added a test that composes a payload containing the end marker and asserts it is skipped while the base section stays well formed.
| try: | ||
| parts.append(target.read_text(encoding="utf-8").strip()) | ||
| except OSError: | ||
| continue |
There was a problem hiding this comment.
Fixed in 0e46935. The collector now catches (OSError, UnicodeDecodeError) and skips the entry, consistent with the fail-closed behavior, so a non-UTF-8 instruction file no longer crashes the refresh. Added a test with an invalid UTF-8 file that asserts it is skipped while a valid sibling extension still composes.
| 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 |
There was a problem hiding this comment.
Fixed in 0e46935. The evidence doc now leads with the verified install-path figure of +0.123 (22 wins, 0 ties, 2 losses, n=24), which is bare vs the exact block this mechanism writes. The +0.142 was a distinct earlier pilot with a shorter hand-distilled payload and is now labelled as such rather than presented as the same result. Stale suite counts were refreshed at the same time.
| # 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 |
There was a problem hiding this comment.
Fixed in 0e46935. The emit-blocks interpreter probe now runs import sys, yaml so a python3 without PyYAML is not selected, matching the config-parse probe above. The bash twin already imported yaml.
| - **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. |
There was a problem hiding this comment.
Fixed in 0e46935. provides.instructions is now documented in EXTENSION-API-REFERENCE.md and EXTENSION-DEVELOPMENT-GUIDE.md, including the schema, the path-safety rules, the opt-in agent-context behavior, and the lifecycle semantics, and it is added to the at-least-one-of capability list.
…on payloads; ps1 PyYAML probe; docs + evidence Copilot PR review (github#4259) fixes: - update_agent_context.py: reject instruction payloads that embed a managed-section marker (outer markers or per-extension SPECKIT EXT markers) so _upsert_section cannot strand content on disable/remove; catch UnicodeDecodeError (not just OSError) so a non-UTF-8 file is skipped instead of crashing the refresh. Markers threaded through the collector/render helpers. - update-agent-context.ps1: the emit-blocks interpreter probe now requires 'import yaml' (mirrors the config-parse probe) so a python3 without PyYAML is not selected. - EXTENSION-API-REFERENCE.md + EXTENSION-DEVELOPMENT-GUIDE.md: document provides.instructions (schema, path rules, opt-in agent-context behavior + lifecycle). - INSTRUCTIONS-POC-EVIDENCE.md: use the verified +0.123 (22/0/2, n=24) install-path figure and mark the earlier +0.142 as a distinct pilot; refresh suite counts; clarify the trigger/lifecycle model. - tests: add marker-collision-skip and non-UTF-8-skip cases (15 passing).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
extensions/agent-context/scripts/python/update_agent_context.py:493
- This composition only runs when the update script is invoked. The extension add/remove/enable/disable paths only refresh native events, and
agent-contextregisters onlyafter_specify/after_planhooks, so adding an instructions extension leaves the context file unchanged until a user or agent invokes a command/hook; disable/remove likewise leaves stale rules until then. That contradicts the PR's “without any command or hook being invoked” guarantee and #4200's install/lifecycle acceptance criteria. Add an agent-context-owned extension-lifecycle refresh and cover add, enable, disable, remove, and update end to end.
extension_blocks = _render_extension_block_lines(project_root, marker_start, marker_end)
section = _build_section(marker_start, marker_end, plan_path, extension_blocks)
| 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}" | ||
| ) |
There was a problem hiding this comment.
Fixed in d7dc2e7. The validator now rejects a present non-string description on a provides.instructions entry, matching the documented schema and the command, template, and script validators. Added a validation test for it.
| 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"], |
There was a problem hiding this comment.
Fixed in d7dc2e7. Added an installed-instructions fixture to tests/extensions/test_update_agent_context_python_parity.py and a parity test that composes an instructions extension and compares the resulting context bytes for Python vs Bash and Python vs PowerShell, including a non-ASCII payload. The bash comparison is gated to POSIX CI like the other parity tests; the PowerShell comparison passes locally.
…t parity test for composed instructions - __init__.py: reject a present non-string provides.instructions[].description, matching the docs and the command/template/script validators. - test_update_agent_context_python_parity.py: add an installed-instructions fixture and compare Python/Bash/PowerShell resulting context bytes incl. a non-ASCII payload (bash gated to POSIX CI; PowerShell parity verified locally). - test_extension_instructions.py: add non-string-description validation test.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
extensions/agent-context/INSTRUCTIONS-POC-EVIDENCE.md:58
- This count is stale: the file contains 14 test functions, and the three-case parametrization produces 16 collected test cases.
`tests/extensions/test_extension_instructions.py` (13 tests, all passing):
| and a disabled or removed extension's block is dropped on the next refresh. A fully | ||
| automatic trigger on `extension add`/`remove` would need an extension-lifecycle hook point | ||
| in core (none exists today — the event system covers agent-runtime events only), so that is | ||
| deliberately left as a follow-up owned by `agent-context`. |
There was a problem hiding this comment.
This is the same lifecycle point as the earlier comment on the collector, where I replied in more detail. In short: cleanup is implemented as eventually-consistent (a disabled or removed extension's block drops on the next agent-context refresh, which fires via the after_specify and after_plan hooks). An immediate trigger on extension add, remove, enable, or disable would require core to invoke agent-context on lifecycle events, which crosses the core/agent-context separation agreed in #4200 (core validates metadata only). I would rather keep that boundary clean and leave the auto-trigger, including orphan cleanup when agent-context itself is removed, as a follow-up owned by agent-context. The evidence doc now describes the trigger and lifecycle model accurately rather than implying instant delivery.
| # 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)" |
There was a problem hiding this comment.
Good catch, fixed in 9af864c. The emitter (--emit-extension-blocks) now accepts --marker-start and --marker-end, and the bash twin forwards the configured MARKER_START and MARKER_END, so a payload containing a configured custom end marker is rejected too, not only the default SPECKIT markers. Added a custom-marker byte-parity test (POSIX CI) plus a unit test asserting a custom-end-marker payload is rejected.
| $prevOutEnc = [Console]::OutputEncoding | ||
| try { | ||
| [Console]::OutputEncoding = [System.Text.Encoding]::UTF8 | ||
| $emitted = (& $pyForBlocks $pyTwin --emit-extension-blocks 2>$null | Out-String) |
There was a problem hiding this comment.
Fixed in 9af864c. The PowerShell twin now forwards the configured MarkerStart and MarkerEnd to the emitter, so custom-marker collisions are rejected the same way. Added a PowerShell custom-marker byte-parity test, which passes locally.
| if ($LASTEXITCODE -eq 0) { $pyForBlocks = $candidate; break } | ||
| } catch { } | ||
| } | ||
| if ($pyForBlocks -and (Test-Path -LiteralPath $pyTwin)) { |
There was a problem hiding this comment.
Fixed in 9af864c. When no Python 3 with PyYAML is on PATH and other extensions are installed, the PowerShell twin now writes a clear warning that provides.instructions blocks were not composed, instead of silently reporting success. I kept the single Python emitter as the source of truth rather than reimplementing the registry, manifest, path-safety, and marker logic natively in three languages. Note the bash twin already requires Python for its own upsert, so it cannot silently omit the blocks; this gap was PowerShell-only.
…PowerShell has no Python for composition - emitter (--emit-extension-blocks) now accepts --marker-start/--marker-end; bash and ps1 twins forward their configured markers so collision-rejection uses the SAME markers the upsert uses (fixes custom-marker payload collisions, not just default SPECKIT markers). - ps1: when no Python 3 + PyYAML is on PATH but other extensions are installed, warn that provides.instructions blocks were not composed instead of silently writing only the base section (bash already requires Python for its upsert, so it cannot silently omit). - tests: custom-marker forwarding + collision-rejection unit tests; custom-marker byte-parity tests for bash (POSIX CI) and PowerShell (passes locally).
Description
Part of #4200.
Adds a
provides.instructionscapability so a Spec Kit extension can ship a compact,always-on rule block that reaches the agent without any command or hook being invoked —
the reachability gap discussed in #4200. Ownership follows what we agreed on in the thread:
agent-contextextension composes enabled extensions' blocks into the routedagent file (
.github/copilot-instructions.md,CLAUDE.md, …) as namespaced<!-- SPECKIT EXT:<id> START/END -->blocks, and owns routing, refresh, update, and removal.agent-contextisn't installed, nothing touches any agent file. The key is additive anda no-op on cores without this change.
Why it's needed: today nothing delivers an extension's guidance automatically on install in a way
that survives a hands-off agent run. This adds that always-on channel, scoped to an opt-in extension.
Efficacy (same rule block + conformance metric throughout):
0.843, B always-on0.986,C rules in an invoked command
0.989, D same command never invoked0.831.C − B = +0.003,payload present for B/C and absent for D — so the effect is reachability (present vs absent), not
obedience.
22 wins / 0 ties / 2 losses. The block that lands in
copilot-instructions.mdon a real installis byte-identical to the one measured.
One test not completed (for transparency): a behavioral A/B on an internal at-scale agent-execution
harness is currently inconclusive due to an agent-runtime regression unrelated to this change — the
agent crashed on startup on every attempt (both arms, incl. bare), confirmed by an untouched baseline
image failing identically. In-container delivery of the block was verified separately. That datapoint
is pending a healthy agent build and is not reported as a result.
Testing
uv run specify --helpuv sync && uv run pytestNew tests
tests/extensions/test_extension_instructions.py: 13 passed (validation accept/reject,composition, disable-cleanup, multi-extension ordering, path-unsafe skip, no-op without
agent-context,emit mode). Full suite on this branch, rebased on current
main: 6916 passed, 415 skipped, 0 failed.bash/powershell/pythonemit paths verified byte-identical. End-to-end verified in a sampleproject: after
specify extension add <ext> --dev+specify extension add /agent-context --dev, thenamespaced block appears in
.github/copilot-instructions.md, and enable/disable/remove clean up correctly.AI Disclosure
Implemented with GitHub Copilot (agentic) — the core validation, the
agent-contextcompose logic andits bash/powershell/python twins, and the test suite were written with AI assistance and reviewed by me.
The empirical evaluation (the pre-check and the +0.123 conformance measurement) was also run and analyzed
with AI assistance.