Skip to content

Feat/agent context instructions preset - #4389

Open
TheovanKraay wants to merge 4 commits into
github:mainfrom
TheovanKraay:feat/agent-context-instructions-preset
Open

Feat/agent context instructions preset#4389
TheovanKraay wants to merge 4 commits into
github:mainfrom
TheovanKraay:feat/agent-context-instructions-preset

Conversation

@TheovanKraay

Copy link
Copy Markdown

Description

Part of #4200, and a follow-up to the discussion on #4259.

This is the delivery mechanism the maintainer proposed on #4259: instead of an extension manifest field that changes the agent's always-on behavior implicitly on install, always-on rules are delivered through an explicit, opt-in preset over agent-context. Core validates metadata only; the opt-in agent-context extension owns the writes; nothing touches the agent's context unless the user explicitly enables a preset.

  • Presets (src/specify_cli/presets/__init__.py): preset.yml may declare provides.instructions (list of { file, description? }), path-safe. An instructions-only preset (no templates) is valid. Metadata validation only.
  • agent-context (all three twins): each installed and enabled preset's instruction block is composed into the managed section as a namespaced <!-- SPECKIT PRESET:<id> START/END --> sub-block. The Python twin is the single source of truth (--emit-preset-blocks); the bash and PowerShell twins delegate to it for byte-identical output. Path-unsafe, non-UTF-8, and marker-colliding payloads are skipped fail-closed. The PowerShell twin warns when no Python 3 with PyYAML is available and presets are installed, instead of silently omitting the blocks.
  • presets/example-always-on-rules/: example instructions-only preset.
  • Docs: docs/reference/presets.md, presets/PUBLISHING.md.

Ownership and consent: the preset is the standalone unit (specify preset add / disable / remove), so installing an extension does not by itself change the agent. Enabling the preset is the explicit opt-in. An extension author can also distribute the preset together with the extension and agent-context as a bundle, so specify bundle install sets everything up in one previewable, consented step; that uses the existing bundle mechanism and lives with the extension, so it is out of scope here.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

New tests/extensions/test_preset_instructions.py (16) and preset-parity tests in tests/extensions/test_update_agent_context_python_parity.py (Python-vs-Bash on CI, Python-vs-PowerShell locally, non-ASCII payload). No regressions across the preset and extension suites (771 passed, 144 skipped). Verified end to end via both the Python and PowerShell twins: specify init -> specify preset add --dev presets/example-always-on-rules -> specify extension add --dev extensions/agent-context -> agent-context update composes the block into .github/copilot-instructions.md.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Implemented with GitHub Copilot (agentic): preset validation, the agent-context composition and its bash/powershell twins, the example preset, the tests, and the docs were written with AI assistance and reviewed by me.

TheovanKraay added 2 commits September 1, 2026 13:27
…ns via agent-context (github#4200)

Alternative to the extension-manifest provides.instructions approach, per the github#4200 discussion: deliver always-on rules through an EXPLICIT, opt-in preset instead of a side effect of installing an extension.

- presets: preset.yml may declare provides.instructions (list of {file[, description]}); an instructions-only preset (no templates) is valid; entries are path-safe. Core validates metadata only.

- agent-context: update_agent_context.py composes each installed + ENABLED preset's instruction block into the managed section as a namespaced <!-- SPECKIT PRESET:<id> START/END --> sub-block (deterministic order; path-unsafe, non-UTF-8, and marker-colliding payloads skipped fail-closed).

- presets/example-always-on-rules/: example instructions-only preset.

- tests: preset validation + agent-context composition (16 tests). Verified end to end via the real CLI: specify init -> preset add -> extension add agent-context -> update composes the block into .github/copilot-instructions.md.

Follow-ups once the shape is agreed: bash/powershell twins for the composer, and an optional opt-in prompt during extension add for extensions that bundle such a preset.
…omposition, plus docs

- python twin gains --emit-preset-blocks; bash and powershell twins delegate to it so all three compose the same namespaced SPECKIT PRESET blocks byte-identically (single source of truth). ps1 warns when no Python 3 + PyYAML is available and presets are installed, instead of silently omitting the blocks.

- parity tests: Python vs Bash (POSIX CI) and Python vs PowerShell (passes locally), including a non-ASCII payload.

- docs: document provides.instructions in docs/reference/presets.md and presets/PUBLISHING.md.

Verified end to end via both the Python and PowerShell twins: specify init -> preset add -> extension add agent-context -> update composes the block into .github/copilot-instructions.md.
@TheovanKraay
TheovanKraay requested a review from mnriem as a code owner September 1, 2026 14:36
Copilot AI balanced review requested due to automatic review settings September 1, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Registry path containment and manifest validation gaps must be resolved before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds opt-in preset-provided always-on instructions composed by the agent-context extension.

Changes:

  • Validates provides.instructions in preset manifests.
  • Composes enabled preset rules across Python, Bash, and PowerShell scripts.
  • Adds an example preset, documentation, and parity tests.
File summaries
File Description
src/specify_cli/presets/__init__.py Adds instruction metadata validation.
extensions/agent-context/scripts/python/update_agent_context.py Collects and renders preset rules.
extensions/agent-context/scripts/bash/update-agent-context.sh Delegates composition to Python.
extensions/agent-context/scripts/powershell/update-agent-context.ps1 Delegates composition with dependency warnings.
tests/extensions/test_preset_instructions.py Tests validation and composition.
tests/extensions/test_update_agent_context_python_parity.py Tests cross-script output parity.
docs/reference/presets.md Documents always-on instructions.
presets/PUBLISHING.md Documents manifest syntax.
presets/example-always-on-rules/preset.yml Defines an example preset.
presets/example-always-on-rules/instructions/best-practices.md Supplies example rules.
Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +263 to +280
blocks: list[tuple[str, str]] = []
for preset_id in sorted(reg["presets"]):
meta = reg["presets"][preset_id]
if not isinstance(meta, dict) or not meta.get("enabled", True):
continue
manifest = presets_dir / preset_id / "preset.yml"
if not manifest.is_file():
continue
try:
with open(manifest, "r", encoding="utf-8") as fh:
pdata = yaml.safe_load(fh)
except Exception:
continue
provides = pdata.get("provides") if isinstance(pdata, dict) else None
instructions = provides.get("instructions") if isinstance(provides, dict) else None
if not isinstance(instructions, list):
continue
preset_root = (presets_dir / preset_id).resolve()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. The registry is untrusted input, so joining the preset id straight onto .specify/presets let an absolute key or a ../separator id (or a symlinked directory) point the manifest read outside the presets root.

Fixed in f660e11: the collector now rejects any id that is not a simple name (must match ^[a-z0-9][a-z0-9._-]*$, so no separators, .., or absolute/drive forms) and, after resolving, confirms the preset directory still lives inside the resolved presets root via relative_to before it opens preset.yml. A crafted key or symlink is skipped instead of read.

Added test_unsafe_registry_preset_id_skipped, which injects ../../evil and /abs-evil into the registry and asserts the good preset still composes while nothing from the escaped paths appears in the section.

Comment on lines +508 to +513
if has_instructions:
instructions = provides["instructions"]
if not isinstance(instructions, list):
raise PresetValidationError(
"Invalid provides.instructions: expected a list"
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. An empty provides.instructions list passed validation but contributed nothing, which is inconsistent with how we already reject empty templates.

Fixed in f660e11: validation now raises provides.instructions must not be empty when the list is present but empty, mirroring the empty-templates rejection. Added test_empty_instructions_list_rejected.

Comment thread src/specify_cli/presets/__init__.py Outdated
Comment on lines +527 to +537
normalized = os.path.normpath(file_path)
if (
file_path.startswith("/")
or "\\" in file_path
or os.path.isabs(normalized)
or normalized.startswith("..")
):
raise PresetValidationError(
f"Invalid instruction file path '{file_path}': "
"must be a relative path within the preset directory"
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the ad-hoc check let non-portable forms through (empty, whitespace, ., directory-only, and Windows drive-relative like C:rules.md).

Fixed in f660e11: instruction file paths now go through the shared relative_extension_path_violation policy in _utils.py, the same one the extension path validation uses, so all of those forms are rejected at metadata-validation time with a specific reason. Added a parametrized test_instruction_path_non_portable_rejected covering empty, whitespace, surrounding whitespace, ., directory-only, drive-relative, and backslash forms.

TheovanKraay pushed a commit to TheovanKraay/spec-kit-cosmosdb that referenced this pull request Sep 1, 2026
…p bundle

Pairs with github/spec-kit#4389 (preset provides.instructions + agent-context composition). Delivers the extension's always-on best-practice rules through an EXPLICIT opt-in preset over agent-context, instead of implicitly on extension install.

- cosmosdb-rules/: preset declaring provides.instructions -> the compact Cosmos rule block (same content as the shipped .github/copilot-instructions.md). Enabling the preset composes it into the agent context file; disabling/removing drops it.

- bundle.yml: composes the cosmosdb extension + cosmosdb-rules preset + agent-context so 'specify bundle install' sets up the extension and its always-on rules in one step.

- .extensionignore: exclude the preset and bundle from the extension package (they are separate primitives).

Verified: specify preset add cosmosdb-rules + agent-context update composes the Cosmos rules into .github/copilot-instructions.md; specify bundle build produces a valid distributable artifact.
TheovanKraay pushed a commit to TheovanKraay/spec-kit-cosmosdb that referenced this pull request Sep 1, 2026
Pairs with github/spec-kit#4389 (preset provides.instructions + agent-context composition). Delivers the extension's always-on best-practice rules through an EXPLICIT opt-in preset over agent-context, instead of implicitly on extension install.

- cosmosdb-rules/: preset declaring provides.instructions -> the compact Cosmos rule block (same content as the shipped .github/copilot-instructions.md). Enabling the preset composes it into the agent context file; disabling/removing drops it.

- .extensionignore: exclude the preset from the extension package (separate primitive).

Verified: specify preset add cosmosdb-rules + agent-context update composes the Cosmos rules into .github/copilot-instructions.md.
TheovanKraay pushed a commit to TheovanKraay/spec-kit-cosmosdb that referenced this pull request Sep 1, 2026
Pairs with github/spec-kit#4389 (preset provides.instructions + agent-context composition). Delivers the extension's always-on best-practice rules through an EXPLICIT opt-in preset over agent-context, instead of implicitly on extension install.

- cosmosdb-rules/: preset declaring provides.instructions -> the compact Cosmos rule block (same content as the shipped .github/copilot-instructions.md). Enabling the preset composes it into the agent context file; disabling/removing drops it.

- .github/workflows/release-preset.yml: on release, attach a preset-rooted cosmosdb-rules-<tag>.zip asset so users can 'specify preset add cosmosdb-rules --from <asset-url>' (the repo source archive can't resolve the nested preset).

- README + preset README: exact opt-in install steps and the release-asset URL.

- .extensionignore: exclude the preset from the extension package.

Verified end to end locally (spec-kit core branch + preset-rooted release zip served over HTTP): preset add --from downloads and installs, and agent-context composes the Cosmos rules into .github/copilot-instructions.md.
…ector containment

PR github#4389 Copilot review:

- collector (update_agent_context.py): the registry is untrusted; reject preset ids that are not simple names (no separators, '..', or absolute/drive forms) and confirm the resolved preset dir stays inside .specify/presets before opening the manifest, so a crafted key or symlink can't read a manifest/payload outside it.

- presets/__init__.py: reject an empty provides.instructions list (mirrors the empty-templates rejection); validate instruction file paths with the shared relative_extension_path_violation policy so empty/whitespace/'.'/directory-only/Windows drive-relative (C:rules.md)/backslash forms are rejected at metadata-validation time.

- tests: empty-list, non-portable-path (7 forms), and unsafe-registry-id cases (25 preset-instruction tests; 780 passed across preset+extension suites).
Copilot AI review requested due to automatic review settings September 1, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Instruction payloads need size limits, and the refresh lifecycle and traversal regression test need clarification or strengthening.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/reference/presets.md:219

  • This reads as though composition happens as soon as agent-context is installed and the preset is enabled, but the implementation only recomposes when the update command/hook next runs. In particular, adding or enabling a preset and then working outside a Spec Kit workflow leaves its rules absent indefinitely. Document that refresh requirement for add/enable as well as disable/remove, and point users to the update command.
This is opt-in and owned by the `agent-context` extension: nothing is written unless `agent-context` is installed and the preset is enabled. When both hold, `agent-context` composes each enabled preset's block into the routed context file (for example `.github/copilot-instructions.md`) inside a namespaced `<!-- SPECKIT PRESET:<id> START/END -->` block, and drops it again on `preset disable`/`remove` at the next refresh. Enabling the preset is the explicit opt-in; installing an extension does not by itself change the agent's context.
  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Balanced

if not target.is_file():
continue
try:
text = target.read_text(encoding="utf-8").strip()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. The composed managed section is re-sent as agent context on every request, so an oversized instruction file (a bundled archive member or an unbounded --dev source) could bloat it without bound.

Fixed in a899821: added a deliberately small budget in the collector. Any single file over a per-file cap (32 KiB) is skipped with a warning, and the on-disk size is checked with stat() before the file is read so a huge member is never allocated into memory. A running aggregate cap (64 KiB across all presets) stops composition once reached, and each skip logs which preset and why.

Boundary coverage added: test_instruction_file_at_limit_included (a file exactly at the per-file cap is kept, since the check is strictly greater), test_oversized_instruction_file_skipped (over-cap file skipped, other presets still compose), and test_aggregate_instruction_budget_enforced (three under-cap presets where the third crosses the aggregate cap and is dropped in id order). The bash/ps1 twins delegate to this collector via --emit-preset-blocks, so the budget applies uniformly.

Comment on lines +299 to +300
reg["presets"]["../../evil"] = {"version": "1.0.0", "enabled": True}
reg["presets"]["/abs-evil"] = {"version": "1.0.0", "enabled": True}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the previous version only injected registry keys, so collection stopped at manifest.is_file() and the test would have passed even without the guards.

Fixed in a899821: the test now materializes a real out-of-root preset (a preset.yml plus an instruction payload tagged PWNED_TRAVERSAL_PAYLOAD) at exactly the location the resolved ../../evil key points at, and asserts that payload is not composed. So if the id-format or containment guard is removed, collection resolves and reads it and the test fails.

I also added test_symlinked_preset_dir_escaping_root_skipped: a valid simple id (linked) whose directory is a symlink pointing outside .specify/presets. That case passes the id regex, so it specifically exercises the resolved-containment check; it asserts the out-of-root payload (PWNED_SYMLINK_PAYLOAD) is not composed, and skips only where the platform disallows symlink creation.

…aversal test

PR github#4389 Copilot review round 4:

- update_agent_context.py: the composed managed section is re-sent as agent context on every request, so an oversized preset instruction file (a bundled archive member or an unbounded --dev source) could bloat it without bound. Add a deliberately small budget: skip+warn any single file over a per-file cap (32 KiB, checked via on-disk size before reading so a huge member is never allocated), and stop composing once a running aggregate cap (64 KiB across all presets) is reached.

- tests: the unsafe-registry-id test now materializes a real out-of-root preset (manifest + payload) at the location the resolved '../../evil' key points at, and adds a symlinked-preset-dir-escaping-root case, so both tests fail if the id/containment guards are removed. Add per-file at-limit (included), oversized (skipped), and aggregate-budget (later preset skipped) boundary tests. 29 preset-instruction tests; 784 passed across preset+extension suites.
Copilot AI review requested due to automatic review settings September 2, 2026 13:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Shell wrappers currently suppress composer failures and can erase previously composed instruction blocks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

presets/example-always-on-rules/preset.yml:7

  • This 242-character description violates the preset publishing checklist’s “under 200 characters” rule (presets/PUBLISHING.md:92). Since this preset is the new reference example, shorten it so it models the documented publishing contract.
    src/specify_cli/presets/init.py:529
  • This shared validator’s traversal reason says the path must remain within the “extension directory” (src/specify_cli/_utils.py:95-96), so preset authors now receive an incorrect extension-specific error for unsafe instruction paths. Make the shared wording package-neutral or translate it to “preset directory” here.
  • Files reviewed: 10/10 changed files
  • Comments generated: 3
  • Review effort level: Balanced

# Always-on instruction blocks contributed by enabled presets (#4200).
# Delegated to the python twin's --emit-preset-blocks so all three twins emit
# byte-identical block text from a single implementation.
_PRESET_BLOCKS="$("$_python" "$_SCRIPT_DIR/../python/update_agent_context.py" --emit-preset-blocks --marker-start "$MARKER_START" --marker-end "$MARKER_END" 2>/dev/null || true)"
Comment on lines +493 to +504
$prevOutEnc = [Console]::OutputEncoding
try {
[Console]::OutputEncoding = [System.Text.Encoding]::UTF8
$emitted = (& $pyForBlocks $pyTwin --emit-preset-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 }
}
Comment thread presets/PUBLISHING.md
Comment on lines +79 to +81
instructions: # Optional: always-on rule blocks composed
- file: "instructions/best-practices.md" # by the opt-in agent-context extension
description: "Always-on engineering rules"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants