-
Notifications
You must be signed in to change notification settings - Fork 11.8k
feat(extensions): let extensions contribute always-on instructions #4259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
746678d
0e46935
d7dc2e7
9af864c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # 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. | ||
|
|
||
| **Triggers / lifecycle.** The agent needs no command invocation to *receive* the rules — | ||
| they live in the always-on context file. Composition and refresh are performed by | ||
| `agent-context` itself: its `speckit.agent-context.update` command and its `after_specify` | ||
| / `after_plan` hooks, so the block is written during normal setup, before the agent runs, | ||
| 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`. | ||
|
|
||
| ## 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. | ||
|
Comment on lines
+21
to
+24
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| - **`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: | ||
|
|
||
| ``` | ||
| <!-- SPECKIT EXT:<id> START --> | ||
| …rule block… | ||
| <!-- SPECKIT EXT:<id> END --> | ||
| ``` | ||
|
|
||
| - **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 lift is about **delivery/reachability**, not content or instruction weighting: the | ||
| delivered payload is the same rule block whether it arrives always-on or via a command, so | ||
| when it is present the measured conformance gain carries over by construction. Two | ||
| measurements, same conformance metric, 2 models × 4 languages × 3 complexity (n=24): | ||
|
|
||
| - **This mechanism's exact output.** Bare vs the block this install path actually writes to | ||
| `.github/copilot-instructions.md`, captured byte-for-byte: **+0.123 mean best-practice | ||
| conformance, 22 wins / 0 ties / 2 losses** (both losses tiny, on a near-ceiling model). | ||
| This is the verified, install-path-accurate figure. | ||
| - **Earlier distilled-block pilot** (a shorter, hand-distilled rule block — a *distinct* | ||
| experiment with a *distinct* payload): **+0.142 mean** over bare, vs +0.10 for the same | ||
| content delivered as on-demand commands. Kept for context, not the headline number. | ||
|
|
||
| ## 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 (rebased on current `main`): `pytest` → **6916 passed, 415 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -457,6 +457,56 @@ $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 that can import PyYAML | ||
| # (the emitter imports yaml). Skips the Windows Store 'python3' alias stub | ||
| # and any interpreter without PyYAML, mirroring the config-parse probe above. | ||
| try { | ||
| & $candidate -c "import sys, yaml; sys.exit(0 if sys.version_info[0] == 3 else 1)" 2>$null | Out-Null | ||
| if ($LASTEXITCODE -eq 0) { $pyForBlocks = $candidate; break } | ||
| } catch { } | ||
| } | ||
| if (-not $pyForBlocks) { | ||
| # The base section is written natively below, but extension-contributed | ||
| # always-on instruction blocks are composed by the Python emitter only. If no | ||
| # Python 3 + PyYAML is on PATH and other extensions are installed (any of which | ||
| # may declare provides.instructions), warn instead of silently dropping them. | ||
| $registryPath = Join-Path $ProjectRoot '.specify/extensions/.registry' | ||
| if (Test-Path -LiteralPath $registryPath) { | ||
| try { | ||
| $reg = Get-Content -LiteralPath $registryPath -Raw -Encoding UTF8 | ConvertFrom-Json | ||
| $others = @($reg.extensions.PSObject.Properties | Where-Object { | ||
| $_.Name -ne 'agent-context' -and $_.Value.enabled -ne $false | ||
| }) | ||
| if ($others.Count -gt 0) { | ||
| [Console]::Error.WriteLine("agent-context: Python 3 with PyYAML not found; extension always-on instruction blocks (provides.instructions) were NOT composed. Base context section written. Install PyYAML (pip install pyyaml) or expose a Python 3 on PATH to include them.") | ||
| } | ||
| } catch { } | ||
| } | ||
| } | ||
| if ($pyForBlocks -and (Test-Path -LiteralPath $pyTwin)) { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| # 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 --marker-start $MarkerStart --marker-end $MarkerEnd 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" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.