mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(kiro-cli): replace literal $ARGUMENTS with prose fallback (#2482)
* fix(kiro-cli): replace literal $ARGUMENTS with prose fallback Kiro CLI file-based prompts do not natively substitute any argument placeholder (kirodotdev/Kiro#4141, kiro.dev/docs/cli manage-prompts), so the literal "$ARGUMENTS" set in KiroCliIntegration.registrar_config["args"] reached the model verbatim and broke the prompt — every parameterized SpecKit command under Kiro CLI was unusable. Replace the placeholder with a prose fallback that instructs the model to take its argument from the user's next message, mirroring the convention used by other integrations whose target CLI lacks native argument injection. Add two regression tests in TestKiroCliIntegration: - test_rendered_prompts_do_not_contain_raw_arguments - test_rendered_prompts_contain_kiro_arg_placeholder and override the inherited test_registrar_config so it does not require args == "$ARGUMENTS". Fixes #1926 * test(kiro-cli): tighten args regression guard + document quirk Address review feedback on PR #2482. Two changes that bracket the original bug fix from both sides — code AND documentation: 1. Test layer (Copilot finding at lines 27, 56) The previous test_registrar_config asserted only that args != "$ARGUMENTS" and that args is truthy. That would silently pass if a future change swapped $ARGUMENTS for $INPUT, {{userMessage}}, <args>, or any other unsubstituted placeholder syntax — defeating the regression guard for issue #1926. Replace with a dual-layer guard: - test_registrar_config_args_is_exact_prose_fallback pins args to the imported _KIRO_ARG_FALLBACK constant. Wording drift now requires a deliberate paired commit (production constant + test). - test_registrar_config_args_does_not_look_like_a_placeholder_token is an independent regression guard built on a 7-pattern regex set covering Bash ($X, ${X}, ${X:-default}), Mustache/Handlebars/Jinja ({{X}}, {{{X}}}), Liquid/Jinja control ({% %}), Python str.format / .NET ({0}, {var}), angle-bracket (<X>), and Windows (%X%). Patterns are anchored to the full string so legitimate prose mentioning a placeholder ("the {{magic}} of placeholders") is not flagged. Also fix the line-56 tautology by importing _KIRO_ARG_FALLBACK directly into test_rendered_prompts_contain_kiro_arg_placeholder, instead of reading the constant back from registrar_config["args"]. The test now verifies the FALLBACK STRING reaches the rendered output, independent of the integration's own config staying correct. 2. Docs layer (mnriem CHANGES_REQUESTED) The Kiro CLI row in docs/reference/integrations.md only documented its alias. Update the notes column to lead with the limitation — Kiro CLI does not substitute $ARGUMENTS in file-based prompts, so Spec Kit ships a prose fallback at render time — with inline links to upstream Kiro "Manage prompts" docs and issue #1926. Style follows the Pi row ("limitation first, alias preserved at end"). Refs #1926
This commit is contained in:
Eldar Shlomi
@@ -23,7 +23,7 @@ The Specify CLI supports a wide range of AI coding agents. When you run `specify
|
||||
| [Junie](https://junie.jetbrains.com/) | `junie` | |
|
||||
| [Kilo Code](https://github.com/Kilo-Org/kilocode) | `kilocode` | |
|
||||
| [Kimi Code](https://code.kimi.com/) | `kimi` | Skills-based integration; supports `--migrate-legacy` for dotted→hyphenated directory migration |
|
||||
| [Kiro CLI](https://kiro.dev/docs/cli/) | `kiro-cli` | Alias: `--integration kiro` |
|
||||
| [Kiro CLI](https://kiro.dev/docs/cli/) | `kiro-cli` | Kiro CLI does not substitute `$ARGUMENTS` in file-based prompts, so Spec Kit ships a prose fallback at render time (see [Manage prompts](https://kiro.dev/docs/cli/chat/manage-prompts/) and issue [#1926](https://github.com/github/spec-kit/issues/1926)). Alias: `--integration kiro` |
|
||||
| [Lingma](https://lingma.aliyun.com/) | `lingma` | Skills-based integration; skills are installed automatically |
|
||||
| [Mistral Vibe](https://github.com/mistralai/mistral-vibe) | `vibe` | |
|
||||
| [opencode](https://opencode.ai/) | `opencode` | |
|
||||
|
||||
@@ -3,6 +3,14 @@
|
||||
from ..base import MarkdownIntegration
|
||||
|
||||
|
||||
# Kiro CLI file-based prompts do NOT support any argument-substitution syntax,
|
||||
# so a raw "$ARGUMENTS" token would reach the model verbatim and break the
|
||||
# prompt (issue #1926, kirodotdev/Kiro#4141). Use a prose fallback so the
|
||||
# rendered prompt instructs the model to take its argument from the user's
|
||||
# next message.
|
||||
_KIRO_ARG_FALLBACK = "(the user will provide the argument in this conversation)"
|
||||
|
||||
|
||||
class KiroCliIntegration(MarkdownIntegration):
|
||||
key = "kiro-cli"
|
||||
config = {
|
||||
@@ -15,7 +23,7 @@ class KiroCliIntegration(MarkdownIntegration):
|
||||
registrar_config = {
|
||||
"dir": ".kiro/prompts",
|
||||
"format": "markdown",
|
||||
"args": "$ARGUMENTS",
|
||||
"args": _KIRO_ARG_FALLBACK,
|
||||
"extension": ".md",
|
||||
}
|
||||
context_file = "AGENTS.md"
|
||||
|
||||
@@ -1,10 +1,41 @@
|
||||
"""Tests for KiroCliIntegration."""
|
||||
|
||||
import os
|
||||
import re
|
||||
|
||||
from specify_cli.integrations import get_integration
|
||||
from specify_cli.integrations.kiro_cli import _KIRO_ARG_FALLBACK
|
||||
from specify_cli.integrations.manifest import IntegrationManifest
|
||||
|
||||
from .test_integration_base_markdown import MarkdownIntegrationTests
|
||||
|
||||
|
||||
# Regex shapes that indicate a value is a placeholder token, not prose.
|
||||
# Covers Bash ($VAR, ${VAR}, ${VAR:-default}), Mustache/Handlebars/Jinja
|
||||
# ({{var}}, {{{var}}}), Liquid/Jinja control ({% ... %}), Python str.format /
|
||||
# .NET ({var}, {0}), angle-bracket (<var>), and Windows-style (%VAR%).
|
||||
# Anchored to the FULL STRING so legitimate prose mentioning a placeholder
|
||||
# (e.g. "the {{magic}} of placeholders") is not flagged. The Liquid pattern
|
||||
# is anchored to the START so multi-tag templates fire while mid-sentence
|
||||
# {%-quotation does not.
|
||||
_PLACEHOLDER_TOKEN_PATTERNS = (
|
||||
re.compile(r"^\$\w+$"), # $ARGUMENTS, $args
|
||||
re.compile(r"^\$\{\w+(?:[:\-+?][^}]*)?\}$"), # ${ARGS}, ${ARGS:-default}
|
||||
re.compile(r"^\{\{\{?\s*\w+(\s*[|.][^}]*)?\s*\}?\}\}$"), # {{var}} {{{var}}} {{x|y}}
|
||||
re.compile(r"^\{%"), # {% if x %}{{ x }}{% endif %}
|
||||
re.compile(r"^<\w+>$"), # <args>
|
||||
re.compile(r"^%\w+%$"), # %USERNAME%
|
||||
re.compile(r"^\{(?:\d+|[a-zA-Z_]\w*)(?:[.\[][^}]*)?(?:![rsa])?(?::[^}]*)?\}$"), # {0}, {var}, {0:>5}
|
||||
)
|
||||
|
||||
|
||||
def _looks_like_placeholder_token(value: str) -> bool:
|
||||
"""Return True if *value* matches a known placeholder-token shape."""
|
||||
if not value:
|
||||
return False
|
||||
return any(p.search(value) for p in _PLACEHOLDER_TOKEN_PATTERNS)
|
||||
|
||||
|
||||
class TestKiroCliIntegration(MarkdownIntegrationTests):
|
||||
KEY = "kiro-cli"
|
||||
FOLDER = ".kiro/"
|
||||
@@ -12,6 +43,85 @@ class TestKiroCliIntegration(MarkdownIntegrationTests):
|
||||
REGISTRAR_DIR = ".kiro/prompts"
|
||||
CONTEXT_FILE = "AGENTS.md"
|
||||
|
||||
def test_registrar_config(self):
|
||||
"""Override base assertion: kiro-cli uses a prose fallback for args
|
||||
because Kiro CLI file-based prompts do not natively substitute
|
||||
``$ARGUMENTS`` (see issue #1926 / kirodotdev/Kiro#4141). The
|
||||
regression-guard load is carried by the two layer tests below
|
||||
(exact-fallback + placeholder-shape rejection)."""
|
||||
i = get_integration(self.KEY)
|
||||
assert i.registrar_config["dir"] == self.REGISTRAR_DIR
|
||||
assert i.registrar_config["format"] == "markdown"
|
||||
assert i.registrar_config["extension"] == ".md"
|
||||
|
||||
def test_registrar_config_args_is_exact_prose_fallback(self):
|
||||
"""Layer 1 — pin the exact fallback so wording drift requires a
|
||||
deliberate paired commit (production constant + test update)."""
|
||||
i = get_integration(self.KEY)
|
||||
assert i.registrar_config["args"] == _KIRO_ARG_FALLBACK, (
|
||||
f"args drifted from the pinned fallback constant. "
|
||||
f"Got: {i.registrar_config['args']!r}; expected: {_KIRO_ARG_FALLBACK!r}. "
|
||||
f"If the wording change is intentional, update _KIRO_ARG_FALLBACK and "
|
||||
f"this test together."
|
||||
)
|
||||
|
||||
def test_registrar_config_args_does_not_look_like_a_placeholder_token(self):
|
||||
"""Layer 2 — independent regression guard: even if someone bypasses
|
||||
layer-1 by changing both constant and test, the value still must not
|
||||
look like ANY placeholder token shape ($X, ${X}, {{X}}, <X>, %X%, {0},
|
||||
{% %}). Catches the class of regression Copilot called out: a swap
|
||||
from $ARGUMENTS to $INPUT or {{userMessage}} would fail this test
|
||||
even if it accidentally passed layer 1."""
|
||||
i = get_integration(self.KEY)
|
||||
args = i.registrar_config["args"]
|
||||
assert not _looks_like_placeholder_token(args), (
|
||||
f"registrar_config['args'] = {args!r} matches a known placeholder-"
|
||||
f"token shape — Kiro CLI does not substitute placeholders so this "
|
||||
f"would reach the model verbatim and break the prompt (issue #1926). "
|
||||
f"Use a prose fallback instead."
|
||||
)
|
||||
|
||||
def test_rendered_prompts_do_not_contain_raw_arguments(self, tmp_path):
|
||||
"""Rendered Kiro prompt files must NOT contain the raw ``$ARGUMENTS``
|
||||
token — Kiro CLI does not substitute it, so the literal would reach
|
||||
the model and break the prompt (issue #1926)."""
|
||||
integration = get_integration(self.KEY)
|
||||
manifest = IntegrationManifest(self.KEY, tmp_path)
|
||||
integration.setup(tmp_path, manifest, script_type="sh")
|
||||
|
||||
prompts_dir = tmp_path / self.REGISTRAR_DIR
|
||||
rendered = list(prompts_dir.glob("*.md"))
|
||||
assert rendered, "expected at least one rendered prompt file"
|
||||
|
||||
offenders = [
|
||||
p.name for p in rendered if "$ARGUMENTS" in p.read_text(encoding="utf-8")
|
||||
]
|
||||
assert offenders == [], (
|
||||
f"these rendered prompts still contain the raw $ARGUMENTS token: {offenders}"
|
||||
)
|
||||
|
||||
def test_rendered_prompts_contain_kiro_arg_placeholder(self, tmp_path):
|
||||
"""The chosen kiro-cli args fallback string must end up in at least
|
||||
one rendered prompt (proves substitution actually fired, not just
|
||||
that $ARGUMENTS was removed). Imports the fallback constant directly
|
||||
instead of reading the field back so the test stays independent of
|
||||
the integration's own config — even if the registrar_config['args']
|
||||
regresses, this test still verifies the FALLBACK STRING is in the
|
||||
rendered output."""
|
||||
integration = get_integration(self.KEY)
|
||||
manifest = IntegrationManifest(self.KEY, tmp_path)
|
||||
integration.setup(tmp_path, manifest, script_type="sh")
|
||||
|
||||
expected = _KIRO_ARG_FALLBACK
|
||||
prompts_dir = tmp_path / self.REGISTRAR_DIR
|
||||
contents = "\n".join(
|
||||
p.read_text(encoding="utf-8") for p in prompts_dir.glob("*.md")
|
||||
)
|
||||
assert expected in contents, (
|
||||
f"none of the rendered prompts contain the configured args fallback "
|
||||
f"({expected!r})"
|
||||
)
|
||||
|
||||
|
||||
class TestKiroAlias:
|
||||
"""--ai kiro alias normalizes to kiro-cli and auto-promotes."""
|
||||
|
||||
Reference in New Issue
Block a user