From 7c08b6804c86d3684d5ce04060622ce45f0ded03 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Tue, 1 Sep 2026 13:27:25 +0100 Subject: [PATCH 1/8] feat(presets): let an explicit preset contribute always-on instructions via agent-context (#4200) Alternative to the extension-manifest provides.instructions approach, per the #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 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. --- .../scripts/python/update_agent_context.py | 121 +++++++- .../instructions/best-practices.md | 9 + presets/example-always-on-rules/preset.yml | 20 ++ src/specify_cli/presets/__init__.py | 60 +++- tests/extensions/test_preset_instructions.py | 273 ++++++++++++++++++ 5 files changed, 474 insertions(+), 9 deletions(-) create mode 100644 presets/example-always-on-rules/instructions/best-practices.md create mode 100644 presets/example-always-on-rules/preset.yml create mode 100644 tests/extensions/test_preset_instructions.py diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 669ec5bf9d..0d66f0f2ad 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -27,6 +27,12 @@ DEFAULT_START = "" DEFAULT_END = "" +# Any SPECKIT marker comment (the outer managed-section markers or the +# per-preset ``PRESET: START/END`` sub-markers). Instruction payloads that +# embed one would collide with the find/replace in _upsert_section, so they are +# rejected. +_SPECKIT_MARKER_RE = re.compile(r"") + lines.append(content) + lines.append(f"") + return lines + + def ensure_mdc_frontmatter(content: str) -> str: """Ensure ``.mdc`` content has YAML frontmatter with ``alwaysApply: true``. @@ -353,7 +469,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) + preset_blocks = _render_preset_block_lines(project_root, marker_start, marker_end) + section = _build_section(marker_start, marker_end, plan_path, preset_blocks) for context_file in context_files: ctx_path = os.path.join(project_root, context_file) diff --git a/presets/example-always-on-rules/instructions/best-practices.md b/presets/example-always-on-rules/instructions/best-practices.md new file mode 100644 index 0000000000..17032686e3 --- /dev/null +++ b/presets/example-always-on-rules/instructions/best-practices.md @@ -0,0 +1,9 @@ +## Example project rules + +These are always-on rules contributed by an explicitly-enabled preset. The +`agent-context` extension composes this block into the coding agent's context +file so it applies to the agent's work, including outside a Spec Kit workflow. + +- Prefer small, well-named functions over large ones. +- Validate inputs at system boundaries, not deep in the call stack. +- Write a test for each behavior change. diff --git a/presets/example-always-on-rules/preset.yml b/presets/example-always-on-rules/preset.yml new file mode 100644 index 0000000000..e8107d5c1f --- /dev/null +++ b/presets/example-always-on-rules/preset.yml @@ -0,0 +1,20 @@ +schema_version: "1.0" + +preset: + id: "example-always-on-rules" + name: "Example Always-On Rules" + version: "1.0.0" + description: "Example preset that contributes an always-on instruction block. When the preset is enabled and the opt-in agent-context extension is installed, the block is composed into the coding agent's context file (e.g. .github/copilot-instructions.md)." + author: "spec-kit" + +requires: + speckit_version: ">=0.6.0" + +provides: + instructions: + - file: "instructions/best-practices.md" + description: "Example always-on engineering rules" + +tags: + - "example" + - "agent-context" diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 181b1eeef3..9a4403e636 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -383,9 +383,11 @@ def _validate(self): # Validate provides section provides = self.data["provides"] - if "templates" not in provides: + has_templates = "templates" in provides + has_instructions = "instructions" in provides + if not has_templates and not has_instructions: raise PresetValidationError( - "Preset must provide at least one template" + "Preset must provide at least one template or an instructions block" ) # Validate templates. Guard the container and each entry's shape so a @@ -400,12 +402,12 @@ def _validate(self): # reports the accurate type error rather than the misleading "must # provide at least one template". An empty list still reports the # latter, since that genuinely is a list with no templates. - templates = provides["templates"] - if not isinstance(templates, list): + templates = provides.get("templates", []) + if has_templates and not isinstance(templates, list): raise PresetValidationError( "Invalid provides.templates: expected a list" ) - if not templates: + if has_templates and not templates: raise PresetValidationError( "Preset must provide at least one template" ) @@ -499,6 +501,45 @@ def _validate(self): "must be lowercase alphanumeric with hyphens only" ) + # Validate instructions (optional): each entry declares a markdown rule + # block that the opt-in agent-context extension composes into the + # always-on context file when this preset is enabled. Path-safety mirrors + # the template file checks above. + if has_instructions: + instructions = provides["instructions"] + if not isinstance(instructions, list): + raise PresetValidationError( + "Invalid provides.instructions: expected a list" + ) + for entry in instructions: + if not isinstance(entry, dict): + raise PresetValidationError( + "Each entry in 'provides.instructions' must be a mapping" + ) + if "file" not in entry: + raise PresetValidationError("Instruction entry missing 'file'") + file_path = entry["file"] + if not isinstance(file_path, str): + raise PresetValidationError( + "Invalid instruction file: expected a string, " + f"got {type(file_path).__name__}" + ) + 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" + ) + if "description" in entry and not isinstance(entry["description"], str): + raise PresetValidationError( + "Instruction entry 'description' must be a string" + ) + @property def id(self) -> str: """Get preset ID.""" @@ -531,8 +572,13 @@ def requires_speckit_version(self) -> str: @property def templates(self) -> List[Dict[str, Any]]: - """Get list of provided templates.""" - return self.data["provides"]["templates"] + """Get list of provided templates (may be empty for instructions-only presets).""" + return self.data["provides"].get("templates", []) + + @property + def instructions(self) -> List[Dict[str, Any]]: + """Get list of provided always-on instruction blocks (may be empty).""" + return self.data["provides"].get("instructions", []) @property def tags(self) -> List[str]: diff --git a/tests/extensions/test_preset_instructions.py b/tests/extensions/test_preset_instructions.py new file mode 100644 index 0000000000..c8c353de8d --- /dev/null +++ b/tests/extensions/test_preset_instructions.py @@ -0,0 +1,273 @@ +"""Tests for preset-contributed always-on instructions (github/spec-kit#4200). + +Two layers are covered: + +1. Preset manifest validation (``src/specify_cli/presets``): a preset may declare + a ``provides.instructions`` capability, an instructions-only preset (no + templates) is valid, and entries are path-safe and well typed. +2. The ``agent-context`` composition: on update, each installed + enabled + preset's instruction block is merged into the managed section of the agent + context file inside a per-preset namespaced marker block; disabled/removed + presets drop out; multiple presets coexist deterministically; path-unsafe, + non-UTF-8, and marker-colliding entries are skipped. + +This is the "explicit preset over agent-context" delivery discussed in #4200: +core validates metadata only; the opt-in agent-context extension owns the writes. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +from specify_cli.presets import PresetManifest, PresetValidationError + +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" + + +# ── Preset manifest validation ────────────────────────────────────────────── + + +def _manifest(tmp_path: Path, provides_block: str) -> Path: + text = ( + 'schema_version: "1.0"\n' + "preset:\n" + " id: demo\n" + " name: Demo\n" + ' version: "0.1.0"\n' + " description: d\n" + "requires:\n" + ' speckit_version: ">=0.6.0"\n' + "provides:\n" + ) + textwrap.indent(provides_block, " ") + p = tmp_path / "preset.yml" + p.write_text(text, encoding="utf-8") + return p + + +def test_instructions_capability_is_accepted(tmp_path): + m = PresetManifest( + _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_preset_is_valid(tmp_path): + # A preset that provides ONLY instructions (no templates) is valid. + m = PresetManifest( + _manifest(tmp_path, "instructions:\n - file: instructions/rules.md\n") + ) + assert m.instructions and not m.templates + + +def test_preset_with_neither_templates_nor_instructions_rejected(tmp_path): + with pytest.raises(PresetValidationError, match="at least one template"): + # provides present but empty + p = tmp_path / "preset.yml" + p.write_text( + 'schema_version: "1.0"\n' + "preset:\n id: demo\n name: Demo\n version: \"0.1.0\"\n description: d\n" + "requires:\n speckit_version: \">=0.6.0\"\n" + "provides: {}\n", + encoding="utf-8", + ) + PresetManifest(p) + + +def test_instructions_must_be_a_list(tmp_path): + with pytest.raises(PresetValidationError, match="provides.instructions: expected a list"): + PresetManifest(_manifest(tmp_path, "instructions:\n file: rules.md\n")) + + +def test_instruction_entry_requires_file(tmp_path): + with pytest.raises(PresetValidationError, match="missing 'file'"): + PresetManifest(_manifest(tmp_path, "instructions:\n - description: no file\n")) + + +def test_instruction_description_must_be_a_string(tmp_path): + with pytest.raises(PresetValidationError, match="'description' must be a string"): + PresetManifest( + _manifest( + tmp_path, + "instructions:\n - file: rules.md\n description: [not, a, string]\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(PresetValidationError, match="Invalid instruction file path"): + PresetManifest(_manifest(tmp_path, f"instructions:\n - file: {bad_path}\n")) + + +# ── agent-context composition ─────────────────────────────────────────────── + + +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(f"context_file: {context_file}\ncontext_files: []\n", encoding="utf-8") + + +def _install_preset( + project: Path, + preset_id: str, + rules: str, + *, + enabled: bool = True, + file_rel: str = "instructions/rules.md", +) -> None: + """Materialize an installed preset on disk + register it (no CLI needed).""" + presets = project / ".specify" / "presets" + preset_dir = presets / preset_id + target = preset_dir / file_rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(rules, encoding="utf-8") + (preset_dir / "preset.yml").write_text( + textwrap.dedent( + f"""\ + schema_version: "1.0" + preset: + id: {preset_id} + name: {preset_id} + version: "1.0.0" + description: d + requires: + speckit_version: ">=0.6.0" + provides: + instructions: + - file: {file_rel} + """ + ), + encoding="utf-8", + ) + registry_path = presets / ".registry" + if registry_path.is_file(): + registry = json.loads(registry_path.read_text(encoding="utf-8")) + else: + registry = {"schema_version": "1.0", "presets": {}} + registry["presets"][preset_id] = {"version": "1.0.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 _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(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_preset_block_composed(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "cosmos-rules", RULES_A) + + _run_update(tmp_path) + section = _managed(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_preset_block_removed_on_update(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "cosmos-rules", RULES_A) + _run_update(tmp_path) + assert "PRESET:cosmos-rules" in _managed(tmp_path) + + _install_preset(tmp_path, "cosmos-rules", RULES_A, enabled=False) + _run_update(tmp_path) + section = _managed(tmp_path) + assert "PRESET:cosmos-rules" not in section + # Base managed section survives. + assert "" in section and "" in section + + +def test_multiple_presets_in_id_order(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "zeta", RULES_B) + _install_preset(tmp_path, "alpha", RULES_A) + _run_update(tmp_path) + section = _managed(tmp_path) + + assert "PRESET:alpha" in section and "PRESET:zeta" in section + assert section.index("PRESET:alpha START") < section.index("PRESET:zeta START") + + +def test_path_unsafe_instruction_entry_skipped(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "evil", RULES_A, file_rel="rules.md") + manifest = tmp_path / ".specify" / "presets" / "evil" / "preset.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 "PRESET:evil" not in _managed(tmp_path) + + +def test_marker_colliding_payload_skipped(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "bad", "# Bad\n\n\n\nstranded\n") + result = _run_update(tmp_path) + assert result.returncode == 0 + section = _managed(tmp_path) + assert "PRESET:bad" not in section + assert "stranded" not in section + assert section.count("") == 1 + assert section.count("") == 1 + + +def test_non_utf8_instruction_file_skipped(tmp_path): + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "good", RULES_A) + _install_preset(tmp_path, "broken", RULES_B) + broken = tmp_path / ".specify" / "presets" / "broken" / "instructions" / "rules.md" + broken.write_bytes(b"\xff\xfe bad bytes \x80\x81") + result = _run_update(tmp_path) + assert result.returncode == 0 + section = _managed(tmp_path) + assert "PRESET:good" in section + assert "PRESET:broken" not in section + + +def test_no_preset_registry_just_base_section(tmp_path): + _configure_agent_context(tmp_path) + result = _run_update(tmp_path) + assert result.returncode == 0 + section = _managed(tmp_path) + assert "" in section and "" in section + assert "PRESET:" not in section From a6d01ab20d56a06bbf3197d940ab90923948cdd8 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Tue, 1 Sep 2026 15:22:35 +0100 Subject: [PATCH 2/8] feat(agent-context): bash + powershell twins for preset instruction composition, 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. --- docs/reference/presets.md | 13 +++ .../scripts/bash/update-agent-context.sh | 8 ++ .../powershell/update-agent-context.ps1 | 46 +++++++++++ .../scripts/python/update_agent_context.py | 19 +++++ presets/PUBLISHING.md | 3 + ...test_update_agent_context_python_parity.py | 82 +++++++++++++++++++ 6 files changed, 171 insertions(+) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 1098abfb42..2ac2c171d8 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -205,6 +205,19 @@ specify preset add team-workflow --priority 10 For any file that both provide, `compliance` wins (priority 5 < 10). For files only one provides, that one is used. For files neither provides, the core default is used. +## Always-on instructions + +A preset can also contribute an always-on instruction block through `provides.instructions`. Unlike templates, commands, and scripts (which are resolved by the priority stack when Spec Kit needs them), an instruction block is composed into the coding agent's always-on context file so it reaches the agent's work generally, including outside a Spec Kit workflow. + +```yaml +provides: + instructions: + - file: "instructions/best-practices.md" + description: "Always-on engineering rules" +``` + +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 `` 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. + ## FAQ ### Can I use multiple presets at the same time? diff --git a/extensions/agent-context/scripts/bash/update-agent-context.sh b/extensions/agent-context/scripts/bash/update-agent-context.sh index 7fbe3ef49a..75a9ec6c1d 100755 --- a/extensions/agent-context/scripts/bash/update-agent-context.sh +++ b/extensions/agent-context/scripts/bash/update-agent-context.sh @@ -345,6 +345,7 @@ PY fi # Build the managed section +_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" TMP_SECTION="$(mktemp)" trap 'rm -f "$TMP_SECTION"' EXIT { @@ -354,6 +355,13 @@ trap 'rm -f "$TMP_SECTION"' EXIT if [[ -n "$PLAN_PATH" ]]; then echo "at $PLAN_PATH" fi + # 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)" + if [[ -n "$_PRESET_BLOCKS" ]]; then + printf '%s\n' "$_PRESET_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..498c40569c 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -457,6 +457,52 @@ $lines = @($MarkerStart, if ($PlanPath) { $lines += "at $PlanPath" } +# Always-on instruction blocks contributed by enabled presets (#4200): delegate +# to the python twin's --emit-preset-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 } + # Require a real Python 3 that can import PyYAML (the composer imports yaml), + # skipping the Windows Store 'python3' alias stub. + 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, but preset instruction blocks are + # composed by the Python twin only. If no Python 3 + PyYAML is available and + # presets are installed, warn instead of silently dropping their rules. + $presetReg = Join-Path $ProjectRoot '.specify/presets/.registry' + if (Test-Path -LiteralPath $presetReg) { + try { + $preg = Get-Content -LiteralPath $presetReg -Raw -Encoding UTF8 | ConvertFrom-Json + $enabled = @($preg.presets.PSObject.Properties | Where-Object { $_.Value.enabled -ne $false }) + if ($enabled.Count -gt 0) { + [Console]::Error.WriteLine("agent-context: Python 3 with PyYAML not found; preset always-on instruction blocks (provides.instructions) were NOT composed. Base context section written.") + } + } 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-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 } + } +} $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 0d66f0f2ad..1da908fba1 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -414,6 +414,25 @@ 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-preset-blocks: print only the composed preset 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-preset-blocks" in args: + def _opt(name: str, default: str) -> str: + if name in args: + i = args.index(name) + if i + 1 < len(args): + return args[i + 1] + return default + marker_start = _opt("--marker-start", DEFAULT_START) + marker_end = _opt("--marker-end", DEFAULT_END) + block_lines = _render_preset_block_lines(project_root, marker_start, marker_end) + if block_lines: + 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" ) diff --git a/presets/PUBLISHING.md b/presets/PUBLISHING.md index f71c1f45d8..167b678354 100644 --- a/presets/PUBLISHING.md +++ b/presets/PUBLISHING.md @@ -76,6 +76,9 @@ provides: file: "templates/spec-template.md" description: "Custom spec template" replaces: "spec-template" + instructions: # Optional: always-on rule blocks composed + - file: "instructions/best-practices.md" # by the opt-in agent-context extension + description: "Always-on engineering rules" tags: # 2-5 relevant tags - "category" diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 06015bbdc7..4af062d8dd 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -562,3 +562,85 @@ def test_python_upsert_matches_powershell(tmp_path: Path) -> None: assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes() + + +# ── Composed preset instructions (#4200) parity ────────────────────────────── + +PRESET_RULES = ( + "# Preset rules\n\n- Use point reads \u2014 keep RU low\n- Prefer id as partition key\n" +) + + +def install_preset_instructions( + project_root: Path, + preset_id: str, + rules: str, + file_rel: str = "instructions/rules.md", +) -> None: + """Materialize an installed + enabled provides.instructions preset on disk.""" + presets = project_root / ".specify" / "presets" + preset_dir = presets / preset_id + target = preset_dir / file_rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(rules, encoding="utf-8") + (preset_dir / "preset.yml").write_text( + 'schema_version: "1.0"\n' + "preset:\n" + f" id: {preset_id}\n" + f" name: {preset_id}\n" + ' version: "1.0.0"\n' + " description: d\n" + "requires:\n" + ' speckit_version: ">=0.6.0"\n' + "provides:\n" + " instructions:\n" + f" - file: {file_rel}\n", + encoding="utf-8", + ) + registry = presets / ".registry" + data = ( + json.loads(registry.read_text(encoding="utf-8")) + if registry.is_file() + else {"schema_version": "1.0", "presets": {}} + ) + data["presets"][preset_id] = {"version": "1.0.0", "enabled": True} + registry.parent.mkdir(parents=True, exist_ok=True) + registry.write_text(json.dumps(data, indent=2), encoding="utf-8") + + +@requires_posix_bash +def test_python_composes_preset_instructions_matching_bash(tmp_path: Path) -> None: + repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md") + for repo in (repo_a, repo_b): + add_plan(repo) + install_preset_instructions(repo, "rules", PRESET_RULES) + + bash = run_bash(repo_a) + py = run_python(repo_b) + + assert_parity(bash, py, repo_a, repo_b) + content_b = (repo_b / "AGENTS.md").read_bytes() + assert (repo_a / "AGENTS.md").read_bytes() == content_b + assert b"" in content_b + # Non-ASCII payload survives byte-for-byte through both twins. + assert "Use point reads \u2014 keep RU low".encode("utf-8") in content_b + + +@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available") +def test_python_composes_preset_instructions_matching_powershell( + tmp_path: Path, +) -> None: + repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md") + repo_b = make_project(tmp_path / "proj-py", context_file="AGENTS.md") + for repo in (repo_a, repo_b): + add_plan(repo) + install_preset_instructions(repo, "rules", PRESET_RULES) + + ps = run_powershell(repo_a) + py = run_python(repo_b) + + assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr + content_b = (repo_b / "AGENTS.md").read_bytes() + assert (repo_a / "AGENTS.md").read_bytes() == content_b + assert b"" in content_b + assert "Use point reads \u2014 keep RU low".encode("utf-8") in content_b From f660e11a19d15cd21c8e542fe316b3aa498e1306 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Tue, 1 Sep 2026 18:50:09 +0100 Subject: [PATCH 3/8] address review: harden preset instruction path/id validation and collector containment PR #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). --- .../scripts/python/update_agent_context.py | 16 +++++++-- src/specify_cli/presets/__init__.py | 23 +++++------- tests/extensions/test_preset_instructions.py | 35 ++++++++++++++++++- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 1da908fba1..5e22ff4d5b 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -260,12 +260,25 @@ def _collect_preset_instruction_blocks( if not isinstance(reg, dict) or not isinstance(reg.get("presets"), dict): return [] + presets_root = presets_dir.resolve() blocks: list[tuple[str, str]] = [] for preset_id in sorted(reg["presets"]): + # The registry lives on disk and is untrusted. Reject ids that are not + # simple names (no path separators, '..' traversal, or absolute/drive + # forms), then confirm the resolved directory stays inside + # .specify/presets, so a crafted key or a symlink cannot read a manifest + # or payload outside it. + if not isinstance(preset_id, str) or not re.match(r"^[a-z0-9][a-z0-9._-]*$", preset_id): + continue + preset_root = (presets_dir / preset_id).resolve() + try: + preset_root.relative_to(presets_root) + except ValueError: + continue meta = reg["presets"][preset_id] if not isinstance(meta, dict) or not meta.get("enabled", True): continue - manifest = presets_dir / preset_id / "preset.yml" + manifest = preset_root / "preset.yml" if not manifest.is_file(): continue try: @@ -277,7 +290,6 @@ def _collect_preset_instruction_blocks( instructions = provides.get("instructions") if isinstance(provides, dict) else None if not isinstance(instructions, list): continue - preset_root = (presets_dir / preset_id).resolve() parts: list[str] = [] for entry in instructions: if not isinstance(entry, dict): diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 9a4403e636..f1044b3d87 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -37,6 +37,7 @@ safe_extract_archive, ) from ..extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority +from .._utils import relative_extension_path_violation from .._init_options import ( MISSING_INIT_OPTIONS_FILE, is_ai_skills_enabled, @@ -511,6 +512,10 @@ def _validate(self): raise PresetValidationError( "Invalid provides.instructions: expected a list" ) + if not instructions: + raise PresetValidationError( + "provides.instructions must not be empty" + ) for entry in instructions: if not isinstance(entry, dict): raise PresetValidationError( @@ -518,22 +523,10 @@ def _validate(self): ) if "file" not in entry: raise PresetValidationError("Instruction entry missing 'file'") - file_path = entry["file"] - if not isinstance(file_path, str): - raise PresetValidationError( - "Invalid instruction file: expected a string, " - f"got {type(file_path).__name__}" - ) - normalized = os.path.normpath(file_path) - if ( - file_path.startswith("/") - or "\\" in file_path - or os.path.isabs(normalized) - or normalized.startswith("..") - ): + reason = relative_extension_path_violation(entry["file"]) + if reason: raise PresetValidationError( - f"Invalid instruction file path '{file_path}': " - "must be a relative path within the preset directory" + f"Invalid instruction file {entry['file']!r}: {reason}" ) if "description" in entry and not isinstance(entry["description"], str): raise PresetValidationError( diff --git a/tests/extensions/test_preset_instructions.py b/tests/extensions/test_preset_instructions.py index c8c353de8d..7d8755c203 100644 --- a/tests/extensions/test_preset_instructions.py +++ b/tests/extensions/test_preset_instructions.py @@ -117,10 +117,26 @@ def test_instruction_description_must_be_a_string(tmp_path): @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(PresetValidationError, match="Invalid instruction file path"): + with pytest.raises(PresetValidationError, match="Invalid instruction file"): PresetManifest(_manifest(tmp_path, f"instructions:\n - file: {bad_path}\n")) +def test_empty_instructions_list_rejected(tmp_path): + # An empty instructions list provides nothing; reject like empty templates. + with pytest.raises(PresetValidationError, match="must not be empty"): + PresetManifest(_manifest(tmp_path, "instructions: []\n")) + + +@pytest.mark.parametrize( + "bad_path", + ["", " ", " rules.md ", ".", "sub/", "C:rules.md", "rules\\win.md"], + ids=["empty", "whitespace", "surrounding-ws", "dot", "dir-only", "drive-relative", "backslash"], +) +def test_instruction_path_non_portable_rejected(tmp_path, bad_path): + with pytest.raises(PresetValidationError, match="Invalid instruction file"): + PresetManifest(_manifest(tmp_path, f"instructions:\n - file: '{bad_path}'\n")) + + # ── agent-context composition ─────────────────────────────────────────────── @@ -271,3 +287,20 @@ def test_no_preset_registry_just_base_section(tmp_path): section = _managed(tmp_path) assert "" in section and "" in section assert "PRESET:" not in section + + +def test_unsafe_registry_preset_id_skipped(tmp_path): + # The registry is on disk and untrusted: an id with separators/traversal or + # an absolute/drive form must be skipped, not resolved and read. + _configure_agent_context(tmp_path) + _install_preset(tmp_path, "good", RULES_A) + reg_path = tmp_path / ".specify" / "presets" / ".registry" + reg = json.loads(reg_path.read_text(encoding="utf-8")) + reg["presets"]["../../evil"] = {"version": "1.0.0", "enabled": True} + reg["presets"]["/abs-evil"] = {"version": "1.0.0", "enabled": True} + reg_path.write_text(json.dumps(reg, indent=2), encoding="utf-8") + result = _run_update(tmp_path) + assert result.returncode == 0 + section = _managed(tmp_path) + assert "PRESET:good" in section + assert "evil" not in section From a899821dfd7ca881caedcbae97f5913de25f1b41 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Wed, 2 Sep 2026 14:25:11 +0100 Subject: [PATCH 4/8] address review: cap always-on instruction payload size; strengthen traversal test PR #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. --- .../scripts/python/update_agent_context.py | 37 ++++++- tests/extensions/test_preset_instructions.py | 103 +++++++++++++++++- 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 5e22ff4d5b..0f41b6c8b5 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -33,6 +33,15 @@ # rejected. _SPECKIT_MARKER_RE = re.compile(r"`); such payloads are skipped to avoid corrupting the section. +- Each file must be at or below 32 KiB, and the combined instructions across all enabled presets must fit a 64 KiB aggregate budget; the managed section is re-sent as agent context on every request, so the budget is deliberately small. + +Entries that fail any of these are dropped (fail-closed) and logged to stderr; the remaining ones still compose. + **Validation Checklist**: - ✅ `id` is lowercase with hyphens only (no underscores, spaces, or special characters) From 764c705cff19adc7e7001e2f801754532248114d Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Thu, 3 Sep 2026 18:43:11 +0100 Subject: [PATCH 6/8] address review: count wrapper/separator bytes in aggregate cap; warn on missing/unreadable instruction files PR #4389 Copilot review round 6: - update_agent_context.py: the aggregate budget summed only stripped payloads, so a preset referencing a tiny file thousands of times could render well past 64 KiB while the counter stayed low. The accounting now adds the block wrapper (first entry) and the blank-line separator (later entries) each entry contributes to the rendered section, so the cap actually bounds the always-on context. Also emit a stderr warning for a missing or unreadable/non-UTF-8 instruction file (previously silent); path-unsafe entries stay silent by design (security fail-closed). - presets/PUBLISHING.md: the doc no longer over-promises. It now states the aggregate budget counts wrappers/separators, and spells out which skips warn (missing, unreadable, non-UTF-8, oversized, over-budget, marker-colliding) versus which are intentionally silent (path-unsafe). - tests: add test_aggregate_budget_counts_wrapper_and_separators (25k references to a 1-byte file; asserts the composed section stays within the cap and is truncated). 30 preset-instruction tests; 785 passed across preset+extension suites. --- .../scripts/python/update_agent_context.py | 40 ++++++++++++++--- presets/PUBLISHING.md | 4 +- tests/extensions/test_preset_instructions.py | 44 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 0f41b6c8b5..6e6b8adc2c 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -308,6 +308,9 @@ def _collect_preset_instruction_blocks( rel = entry.get("file") if not isinstance(rel, str) or not rel.strip(): continue + # Path-unsafe entries (absolute, backslash, parent traversal, or a + # target escaping the preset directory) are skipped silently: this is + # a security fail-closed decision, so no diagnostic is emitted. if rel.startswith("/") or "\\" in rel or ".." in rel.split("/"): continue target = (preset_root / rel).resolve() @@ -316,12 +319,20 @@ def _collect_preset_instruction_blocks( except ValueError: continue if not target.is_file(): + _err( + f"agent-context: skipping instructions from preset '{preset_id}': " + f"file '{rel}' not found." + ) continue # Reject an oversized file by its on-disk size before reading it, so # a huge member never gets allocated into memory. try: size = target.stat().st_size except OSError: + _err( + f"agent-context: skipping instructions from preset '{preset_id}': " + f"file '{rel}' is not readable." + ) continue if size > _MAX_INSTRUCTION_FILE_BYTES: _err( @@ -333,13 +344,9 @@ def _collect_preset_instruction_blocks( try: text = target.read_text(encoding="utf-8").strip() except (OSError, UnicodeDecodeError): - continue - entry_bytes = len(text.encode("utf-8")) - if total_bytes + entry_bytes > _MAX_INSTRUCTION_TOTAL_BYTES: _err( f"agent-context: skipping instructions from preset '{preset_id}': " - f"aggregate instruction budget ({_MAX_INSTRUCTION_TOTAL_BYTES} " - "bytes) exceeded." + f"file '{rel}' is not readable UTF-8 text." ) continue if marker_start in text or marker_end in text or _SPECKIT_MARKER_RE.search(text): @@ -348,7 +355,28 @@ def _collect_preset_instruction_blocks( "content contains a managed section marker." ) continue - total_bytes += entry_bytes + # Count the bytes this entry actually adds to the rendered section, + # not just its raw payload: the first entry of a preset materializes + # the surrounding marker block, and every later entry adds a blank-line + # separator. Without this, a flood of tiny entries would slip past the + # aggregate cap even though the composed section is far larger. + entry_bytes = len(text.encode("utf-8")) + if parts: + overhead = 2 # the "\n\n" joining this entry to the previous one + else: + overhead = ( + len(f"") + + len(f"") + + 4 # surrounding newlines and the leading blank line + ) + if total_bytes + entry_bytes + overhead > _MAX_INSTRUCTION_TOTAL_BYTES: + _err( + f"agent-context: skipping instructions from preset '{preset_id}': " + f"aggregate instruction budget ({_MAX_INSTRUCTION_TOTAL_BYTES} " + "bytes) exceeded." + ) + continue + total_bytes += entry_bytes + overhead parts.append(text) if parts: blocks.append((preset_id, "\n\n".join(parts))) diff --git a/presets/PUBLISHING.md b/presets/PUBLISHING.md index e6eca0561d..99733d23c8 100644 --- a/presets/PUBLISHING.md +++ b/presets/PUBLISHING.md @@ -89,9 +89,9 @@ tags: # 2-5 relevant tags - The file must exist inside the preset directory and be valid UTF-8. - It must not contain a managed-section marker (``); such payloads are skipped to avoid corrupting the section. -- Each file must be at or below 32 KiB, and the combined instructions across all enabled presets must fit a 64 KiB aggregate budget; the managed section is re-sent as agent context on every request, so the budget is deliberately small. +- Each file must be at or below 32 KiB, and the combined instructions across all enabled presets must fit a 64 KiB aggregate budget (which counts the rendered block wrappers and separators, not just the raw payloads); the managed section is re-sent as agent context on every request, so the budget is deliberately small. -Entries that fail any of these are dropped (fail-closed) and logged to stderr; the remaining ones still compose. +Entries are dropped fail-closed. A missing, unreadable, non-UTF-8, oversized, over-budget, or marker-colliding file is skipped with a warning on stderr, while a path-unsafe file (absolute, parent-traversal, or one that escapes the preset directory) is skipped silently as a security measure. The remaining entries still compose. **Validation Checklist**: diff --git a/tests/extensions/test_preset_instructions.py b/tests/extensions/test_preset_instructions.py index 8e566f2f08..7c36439160 100644 --- a/tests/extensions/test_preset_instructions.py +++ b/tests/extensions/test_preset_instructions.py @@ -17,6 +17,7 @@ from __future__ import annotations +import importlib.util import json import subprocess import sys @@ -37,6 +38,14 @@ / "update_agent_context.py" ) + +def _load_twin_module(): + """Import the agent-context twin so its collector can be called directly.""" + spec = importlib.util.spec_from_file_location("_uac_twin", PY_TWIN) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + 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" @@ -403,3 +412,38 @@ def test_aggregate_instruction_budget_enforced(tmp_path): assert "PRESET:aaa" in section assert "PRESET:bbb" in section assert "PRESET:ccc" not in section + + +def test_aggregate_budget_counts_wrapper_and_separators(tmp_path): + # A single preset that references one tiny file thousands of times: the raw + # payload sum stays under the cap, but the per-entry separators and the block + # wrapper push the rendered section over it. The accounting must count that + # overhead so the composed section never exceeds the aggregate cap. + _configure_agent_context(tmp_path) + presets = tmp_path / ".specify" / "presets" + pdir = presets / "flood" / "instructions" + pdir.mkdir(parents=True) + (presets / "flood" / "instructions" / "one.md").write_text("x", encoding="utf-8") + n = 25000 + entries = "\n".join([" - file: instructions/one.md"] * n) + (presets / "flood" / "preset.yml").write_text( + 'schema_version: "1.0"\n' + "preset:\n id: flood\n name: flood\n version: \"1.0.0\"\n description: d\n" + "requires:\n speckit_version: \">=0.6.0\"\n" + "provides:\n instructions:\n" + entries + "\n", + encoding="utf-8", + ) + (presets / ".registry").write_text( + json.dumps( + {"schema_version": "1.0", "presets": {"flood": {"version": "1.0.0", "enabled": True}}} + ), + encoding="utf-8", + ) + uac = _load_twin_module() + blocks = uac._collect_preset_instruction_blocks(str(tmp_path)) + assert blocks, "expected the flood preset to compose at least some entries" + rendered = "\n\n".join(content for _pid, content in blocks) + # The composed payload must stay within the advertised aggregate cap... + assert len(rendered.encode("utf-8")) <= uac._MAX_INSTRUCTION_TOTAL_BYTES + # ...which means the flood was truncated below its full entry count. + assert blocks[0][1].count("x") < n From 719d319e9275568d5999617f9dd367862407e690 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Thu, 3 Sep 2026 19:30:17 +0100 Subject: [PATCH 7/8] address review: treat a missing composer in the ps1 twin as a hard failure PR #4389 Copilot review round 7: - ps1 twin: if Python was available but the sibling update_agent_context.py composer was missing (a partial or corrupt install), the guard was simply false and the script rewrote the managed section without preset blocks, silently erasing existing instructions. It now aborts with an error in that case, matching the nonzero-exit handling (and the bash twin, where a missing composer already makes python exit nonzero). Verified: ps1 parses clean and the happy path still composes. --- .../scripts/powershell/update-agent-context.ps1 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 index c84b691f48..01f17d6c2c 100644 --- a/extensions/agent-context/scripts/powershell/update-agent-context.ps1 +++ b/extensions/agent-context/scripts/powershell/update-agent-context.ps1 @@ -487,7 +487,15 @@ if (-not $pyForBlocks) { } catch { } } } -if ($pyForBlocks -and (Test-Path -LiteralPath $pyTwin)) { +if ($pyForBlocks) { + if (-not (Test-Path -LiteralPath $pyTwin)) { + # Python is available but the sibling composer is gone: a partial or + # corrupt install. Treat it as a hard failure (like a nonzero composer + # exit) so the managed section is not rewritten with existing preset + # blocks silently dropped. + [Console]::Error.WriteLine("agent-context: preset instruction composer '$pyTwin' is missing (corrupt or partial install); aborting so the managed section is not rewritten with preset blocks dropped.") + exit 1 + } # 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. # Keep stderr (the composer's oversized/marker-colliding/skipped warnings) out From ac29ff8becc3e55be97305451c7a3e2d69b4b2d5 Mon Sep 17 00:00:00 2001 From: TheovanKraay Date: Fri, 4 Sep 2026 17:55:27 +0100 Subject: [PATCH 8/8] address review: make the bash composer error reachable under set -e PR #4389 Copilot review round 8: - bash twin: the script runs under set -euo pipefail, so a nonzero composer status in the _PRESET_BLOCKS assignment aborted immediately and the following exit-code check + diagnostic were unreachable. Put the assignment in an 'if !' condition so set -e does not fire, the intended error is printed, and the update still aborts before any context rewrite. Verified with bash -n. --- .../scripts/bash/update-agent-context.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/agent-context/scripts/bash/update-agent-context.sh b/extensions/agent-context/scripts/bash/update-agent-context.sh index eff1ec26ef..a7a5d271ab 100755 --- a/extensions/agent-context/scripts/bash/update-agent-context.sh +++ b/extensions/agent-context/scripts/bash/update-agent-context.sh @@ -351,12 +351,12 @@ _SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Delegated to the python twin's --emit-preset-blocks so all three twins emit # byte-identical block text from a single implementation. Capture only stdout so # the composer's warnings (oversized, marker-colliding, or skipped entries) still -# reach stderr, and abort on a nonzero exit so a composer failure never rewrites -# the section with previously composed preset blocks silently dropped. -_PRESET_BLOCKS="$("$_python" "$_SCRIPT_DIR/../python/update_agent_context.py" --emit-preset-blocks --marker-start "$MARKER_START" --marker-end "$MARKER_END")" -_emit_rc=$? -if [[ $_emit_rc -ne 0 ]]; then - echo "agent-context: preset instruction composer failed (exit $_emit_rc); aborting so the managed section is not rewritten with preset blocks dropped." >&2 +# reach stderr. The assignment is the condition of an `if` so `set -e` does not +# abort on a nonzero composer status before this diagnostic runs; on failure we +# abort here so a composer failure never rewrites the section with previously +# composed preset blocks silently dropped. +if ! _PRESET_BLOCKS="$("$_python" "$_SCRIPT_DIR/../python/update_agent_context.py" --emit-preset-blocks --marker-start "$MARKER_START" --marker-end "$MARKER_END")"; then + echo "agent-context: preset instruction composer failed; aborting so the managed section is not rewritten with preset blocks dropped." >&2 exit 1 fi