fix(kiro-cli): replace literal $ARGUMENTS with prose fallback#2482
fix(kiro-cli): replace literal $ARGUMENTS with prose fallback#2482eldar702 wants to merge 2 commits intogithub:mainfrom
Conversation
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 github#1926
There was a problem hiding this comment.
Pull request overview
This PR fixes Kiro CLI prompt rendering by replacing the literal "$ARGUMENTS" registrar args value with a prose fallback, preventing Kiro’s file-based prompts from shipping an un-substituted placeholder to the model (fixes #1926).
Changes:
- Update
KiroCliIntegration.registrar_config["args"]to a prose fallback string suitable for Kiro CLI. - Add/adjust integration tests to ensure rendered
.kiro/prompts/*.mdfiles no longer contain raw$ARGUMENTSand that substitution occurs. - Override the shared Markdown integration registrar-config assertion for
kiro-cli.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/kiro_cli/__init__.py |
Switches Kiro CLI args placeholder from "$ARGUMENTS" to a prose fallback constant. |
tests/integrations/test_integration_kiro_cli.py |
Updates/extends tests to validate Kiro prompt rendering and overrides the base registrar-config expectation for kiro-cli. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
tests/integrations/test_integration_kiro_cli.py:64
test_rendered_prompts_contain_kiro_arg_placeholderderivesexpectedfromintegration.registrar_config["args"], so it will pass even ifargsis changed later to a different (still-broken) placeholder token as long as that token is substituted into the rendered files. Consider asserting the specific prose fallback value you intend to ship (or a predicate that enforces “prose” rather than “placeholder token”) so the test fails if someone swaps back to another literal token that Kiro won’t substitute.
expected = integration.registrar_config["args"]
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})"
)
- Files reviewed: 2/2 changed files
- Comments generated: 1
| 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).""" | ||
| i = get_integration(self.KEY) | ||
| assert i.registrar_config["dir"] == self.REGISTRAR_DIR | ||
| assert i.registrar_config["format"] == "markdown" | ||
| assert i.registrar_config["args"] != "$ARGUMENTS" | ||
| assert i.registrar_config["args"] | ||
| assert i.registrar_config["extension"] == ".md" |
mnriem
left a comment
There was a problem hiding this comment.
This PR should also update docs/reference/integrations.md to document the argument limitation in the Kiro CLI notes column. Currently it only says Alias: --integration kiro. It should note that Kiro CLI file-based prompts do not support argument substitution and that $ARGUMENTS placeholders are replaced with a prose fallback at install time.
This is consistent with how other integrations document their quirks in that table (e.g., Goose noting YAML recipe format, Pi noting MCP limitations, skills-based integrations noting their layout).
Address review feedback on PR github#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 github#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 github#1926. Style follows the Pi row ("limitation first, alias preserved at end"). Refs github#1926
|
Thanks for both reviews! Pushed 1. Maintainer (mnriem) — docsUpdated the Kiro CLI row in
2. Copilot — test (lines 27 + 56)The previous
Also fixed the line-56 tautology — Why two tests instead of one?Different regression classes deserve different diagnostic messages:
Either alone leaves a gap. Together they're ~25 extra lines. Validation
|
Summary
Replace the literal
"$ARGUMENTS"value inKiroCliIntegration.registrar_config["args"]with a prose fallback string so that rendered Kiro CLI prompt files no longer ship a broken shell-style placeholder.Fixes #1926
Problem
When SpecKit prompts are invoked via Kiro CLI (
kiro-cli chat), the literal string$ARGUMENTSwas passed to the AI model unchanged. Every parameterized SpecKit slash-command under Kiro CLI was effectively unusable — the agent either errored out or asked the user to repeat their input.The integration code feeds
registrar_config["args"]straight intoprocess_template()(base.py:717-719), which then substitutes both{ARGS}and$ARGUMENTStokens in command bodies with that value. Setting it to"$ARGUMENTS"made the substitution a no-op, so the literal token remained in every rendered prompt.Solution
Kiro CLI file-based prompts do not natively support any argument-substitution syntax —
$ARGUMENTS,{{userMessage}}, and$INPUTare all delivered to the model verbatim (see kirodotdev/Kiro#4141 and kiro.dev/docs/cli/chat/manage-prompts). Switching to a different shell-style token would just ship a different broken literal.Instead, set
argsto a prose fallback that gets substituted into the rendered prompt and tells the model to take its argument from the user's next conversational message:This is the approach used by other integrations whose target CLI lacks native argument injection.
Test plan
test_rendered_prompts_do_not_contain_raw_arguments— callssetup(), walks every rendered.kiro/prompts/*.md, asserts$ARGUMENTSis not present. Fails onmain, passes after the fix.test_rendered_prompts_contain_kiro_arg_placeholder— asserts the new prose value is substituted in (proves substitution fired, not just that the placeholder was removed).test_registrar_configfromMarkdownIntegrationTestsoverridden inTestKiroCliIntegrationbecause the base assertionargs == "$ARGUMENTS"no longer holds for kiro-cli.pytestfull suite: 2788 passed, 34 skipped — zero regressions.Notes
This change was AI-assisted. The fix was selected after a 3-agent solution-design debate in which two alternative approaches (
{{userMessage}}and$INPUTplaceholder swaps) were rejected after web research confirmed Kiro file-based prompts do not substitute any token; the prose-fallback was the only approach that actually solves the bug. The deliberation also produced the regression-test pair that proves both directions —$ARGUMENTSmust be absent and the new value must be present — so we can't accidentally regress by changing the fallback to an empty string.