Skip to content

fix(integrations): dispatch kiro-cli headless and install hyphenated prompts - #4798

Open
kartsan03 wants to merge 17 commits into
github:mainfrom
kartsan03:fix/kiro-cli-dispatch
Open

kartsan03 wants to merge 17 commits into
github:mainfrom
kartsan03:fix/kiro-cli-dispatch

Conversation

@kartsan03

@kartsan03 kartsan03 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4797: Kiro CLI workflow dispatch and prompt names.

Dispatch. Workflow steps with integration: kiro-cli ran kiro-cli -p "<prompt>", inherited from MarkdownIntegration.build_exec_args(). Kiro CLI rejects that with error: unexpected argument '-p' found, exit 2.

KiroCliIntegration.build_exec_args() now builds kiro-cli chat --no-interactive --trust-all-tools [--model M] [--output-format stream-json] <prompt>, following kiro-cli chat --help on Kiro CLI 2.26.0:

  • Without --trust-all-tools, headless mode denies every write and still exits 0. It plays the role of Copilot's --yolo and Cursor's --force.
  • Kiro has no json output format, so output_json maps to stream-json.
  • SPECKIT_INTEGRATION_KIRO_CLI_EXTRA_ARGS goes before the prompt.

Prompt names. Kiro CLI rejects /speckit.constitution and runs /speckit-constitution. So, like Junie (#4073) and Cline:

Migration.

  • specify integration upgrade kiro-cli replaces the core speckit.*.md prompts through the manifest. A modified one stops the upgrade until --force.
  • Each registration pass (use, switch, upgrade of the active integration) writes an extension's hyphenated prompts. It then removes each dotted prompt whose replacement exists, the step Qoder's skills migration uses (_retire_legacy_flat_extension_commands, [bug-fix] Fix qodercli-skills-migration: migrate QodercliIntegration to SkillsIntegration #4205). A command the manifest still declares but that isn't written keeps its old prompt and stays tracked, so extension remove still deletes it.
  • Upgrade refuses the rename, before changing files, in two cases:
    • presets have commands registered for Kiro;
    • an extension has registered a prompt where a core prompt goes, as with an alias such as speckit-plan, which Spec Kit 1.0.7 and earlier accepted.

Name collisions. speckit.foo.bar-baz and speckit.foo-bar.baz both become speckit-foo-bar-baz.

  • Install and update reject a command or alias that writes the same file as a core command or another installed extension once dots become hyphens. A command's own alias may share its file. An installed extension whose manifest can't be read counts with the names the registry tracks for it.

  • Projects can predate that check, with a pair like the one above or an alias such as plan from 1.0.7 and earlier. Registration for Kiro CLI and Qoder then skips the extension through the per-extension error path from register_enabled_extensions_for_agent has no per-extension error isolation: one failing extension silently drops the rest #2950:

    • nothing is written or retired;
    • the registry is unchanged;
    • the warning names the other writer.

    Once one of the pair is removed, the other migrates.

  • Extension removal keeps a core command's file only when the integration manifest tracks it. It also deletes the extension's old flat files, which Qoder keeps in .qoder/commands.

Detection. specify check and specify init accepted a bare kiro, which launches Kiro IDE by default. Kiro IDE 1.2.4 exits 0 on the headless argv without running anything. Detection now looks for kiro-cli only, like dispatch, the workflow preflights and the devcontainer.

Docs. The Kiro row in docs/reference/integrations.md covers names, dispatch, detection, the upgrade refusals and older collisions. docs/reference/extensions.md covers the install refusal and older collisions on Kiro CLI and Qoder CLI. The row drops "Alias: --integration kiro", because specify init --integration kiro is rejected.

Headless Kiro still drops text after /speckit-<name>. The existing prose fallback covers that (#1926).

Real runs.

  • [Bug]: kiro-cli workflow dispatch exits 2, and Kiro doesn't expand dotted /speckit.* prompt names #4797's workflow on Kiro CLI 2.26.0. main fails as above. This branch expands /speckit-constitution, writes .specify/memory/constitution.md and ends with Status: completed.
  • A 1.1.0 project with git ends with 15 speckit-*.md prompts in each of these cases:
    • a plain upgrade;
    • with Kiro secondary, the upgrade and then integration use kiro-cli;
    • with a preset, preset remove, the upgrade and preset add, which leaves the override in speckit-plan.md.
  • A 1.1.0 project with speckit.foo.bar-baz (plus speckit.foo.other) and speckit.foo-bar.baz. At 3bcee7e the upgrade lost foo's body and deleted both dotted prompts. Here both extensions are skipped, the dotted prompts are byte-identical, and foo migrates after extension remove foo-bar.
  • A 1.0.7 project with alias speckit-plan, dev-installed so the prompt is a symlink. At 3bcee7e the upgrade overwrote the core prompt and extension remove deleted it. Here the upgrade is refused, and after extension remove old it writes a regular core speckit-plan.md. With alias plan instead, extension remove old keeps the core prompt.
  • A 0.16.5 Qoder project with the same pair. On main the upgrade wrote one body into the shared skill and deleted both old commands. Here both extensions are skipped, extension remove foo-bar deletes its old command, and the next upgrade migrates foo.

Testing

  • Tested locally with uv run specify --help

  • Ran existing tests with uv sync && uv run pytest

  • Tested with a sample project (if applicable)

  • tests/integrations/test_integration_kiro_cli.py: argv, names, dispatch, hook note, handoffs, and the IDE launcher.

  • tests/specify_cli/integrations/test_command_upgrade.py:

    • migration;
    • the preset and core-alias refusals;
    • failed re-registration and tracking;
    • use, switch and enable after the rename;
    • collisions: a pair left unregistered, then migrated; a disabled or unreadable owner; enable; alias plan; a Qoder pair, then removing one; a command's own alias; force reinstall.
  • tests/test_extensions.py: install rejection, including an owner whose manifest can't be read.

  • On main's source, 27 of these tests fail and the Kiro module fails to import.

  • Each guard has a test that fails without it:

    • registration skip off: 6 collision tests fail;
    • no upgrade refusal, or removal judging core files by name: the speckit-plan test fails;
    • no core files kept on removal: the plan test fails;
    • no fallback for unreadable manifests: the unreadable-owner install test fails;
    • no removal of old flat files: the Qoder test fails.
  • Full suite: 9485 passed, 265 skipped (Linux, Python 3.13). ruff check src tests (0.15.0) and markdownlint: clean.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Claude Code (Claude Opus 5.5, autonomous agent mode) was used to investigate Kiro CLI and Kiro IDE, run the reproductions, and write the code changes, the tests and this description.

@kartsan03
kartsan03 requested a review from mnriem as a code owner September 30, 2026 09:13
@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Sep 30, 2026
@mnriem
mnriem requested a balanced review from Copilot September 30, 2026 14:00

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.

Copilot review overview

🟡 Changes recommended

The implementation is sound, but its docstring incorrectly claims dotted prompt names are expanded.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Fixes Kiro CLI workflow dispatch by using its supported headless chat interface.

Changes:

  • Builds kiro-cli chat --no-interactive --trust-all-tools arguments.
  • Supports model, stream-JSON output, and extra arguments.
  • Adds regression tests for generated arguments.
File Description
src/​specify_cli/​integrations/​kiro_cli/​__init__.py Implements Kiro CLI headless dispatch.
tests/​integrations/​test_integration_kiro_cli.py Tests dispatch arguments and options.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/integrations/kiro_cli/__init__.py Outdated
Kiro CLI runs /name from .kiro/prompts/name.md only when the name has
no dots, so the installed /speckit.plan is rejected as an unrecognized
slash command. Install speckit-<command>.md and dispatch
/speckit-<command>, following the Junie and Cline integrations, so
workflow steps run the prompt instead of relying on the model to find
the file. Upgrade stale-removes the old dotted prompts.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Pushed 86afed7, which adds the second half of #4797: Kiro prompts are now hyphenated. The assessment on #4797 says the argv change alone is not a complete fix, and I agree. With only 88c9c13, a workflow step sends /speckit.constitution, and Kiro answers that it "is not a built-in Kiro CLI slash command". The step then passes only because the model searches for and reads .kiro/prompts/speckit.constitution.md on its own.

What 86afed7 changes

  • Prompts install as .kiro/prompts/speckit-<command>.md, and dispatch sends /speckit-<command>. This follows Junie (Integrate Junie with dot-to-hyphen behavior and command formatting  #4073) and Cline:

    • format_kiro_command_name and the command_filename, build_command_invocation and invoke_separator = "-" overrides, so shared templates and scripts render /speckit-plan.
    • format_name in registrar_config, so extension and preset prompts get the same names. The bundled git extension installs speckit-git-commit.md.
    • The hook note and handoff rewrite. The hook note uses the per-instruction check from fix(cline): stop unrelated prose from suppressing the hook command note #4150, not the older whole-document check.
  • This also resolves the Copilot finding. The build_exec_args docstring now says a /speckit-* input runs the prompt file, which is true once the files are hyphenated.

  • New tests:

    • the formatter, filenames and invocation;
    • a CommandStep dispatch test asserting the exact Kiro argv with /speckit-constitution;
    • the hook note and handoffs;
    • hyphenated overrides of the base Markdown inventory tests;
    • test_upgrade_replaces_dotted_kiro_prompts.

    12 of these fail with the previous kiro_cli/__init__.py.

  • docs/reference/integrations.md: the Kiro row now says prompts are hyphenated and why.

Answers to the assessment's open questions

  1. Output format. Workflow command steps call dispatch_command() with the default stream=True, and prompt steps pass output_json=False. So Kiro gets no --output-format and prints text, which the runner streams without parsing. If a caller does ask for JSON, output_json=True maps to --output-format stream-json (JSON Lines), since Kiro has no plain json format.
  2. --trust-all-tools. It's always added. Headless Kiro can't ask for approval, so without it every write is denied while the run still exits 0. A workflow step would then report success with nothing written. Copilot's --yolo and Cursor's --force do the same job in their integrations.
  3. Migration. specify integration upgrade kiro-cli stale-removes the dotted prompts through the existing manifest contract.
    • I ran it on a project initialized with the code before this PR. Output: "Removed 10 stale file(s) from previous install", and all 10 prompts are now speckit-*.md.
    • If a dotted prompt was modified, the upgrade stops and lists it, and the file stays until --force is used. test_upgrade_replaces_dotted_kiro_prompts covers both cases.
    • Kiro dispatch exited 2 before this PR, so nothing that worked before depended on the dotted names being dispatched.
  4. Versions. I tested only Kiro CLI 2.26.0, the current stable download, and added no version gate.

Real run (Kiro CLI 2.26.0, specify workflow run with a speckit.constitution step and a shell step)

  • With this commit, Kiro expands the prompt itself. The model's first tool calls are the prompt's own steps: the extensions.yml hook check and resolve-template.sh. It never opens .kiro/prompts, and the run ends Status: completed.
  • One limitation is unchanged: headless Kiro drops any text after /speckit-<name>. I tried it on the same line and on the next line, and neither reached the model. So a step's input.args still don't arrive. This is the same Kiro limitation the existing prose fallback covers ([Bug]: $ARGUMENTS placeholder not substituted in Kiro CLI — file-based prompts don't support arguments #1926).

The full suite passes: 8771 passed, 251 skipped. ruff check src tests is clean.

One correction: 88c9c13 is missing the Assisted-by: trailer. The same agent made it, and 86afed7 carries the trailer.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent wrote the code, tests and this comment, and ran the Kiro CLI and upgrade checks above.

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.

Copilot review overview

🟡 Changes recommended

The description explicitly excludes the prompt-renaming work that the diff implements.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (1)

Comment thread src/specify_cli/integrations/kiro_cli/__init__.py
@kartsan03 kartsan03 changed the title fix(integrations): dispatch kiro-cli through chat --no-interactive fix(integrations): dispatch kiro-cli headless and install hyphenated prompts Sep 30, 2026
@kartsan03

Copy link
Copy Markdown
Contributor Author

I updated the title and description for Copilot's scope finding. They now cover the prompt rename, the integration upgrade migration of the old speckit.*.md files, and the docs row, and they say Fixes #4797 because both halves are in this PR. The code is unchanged since 86afed7.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent rewrote the PR title and description and wrote this comment.

test_upgrade_replaces_dotted_kiro_prompts restored the edited prompt
with write_text(), which writes CRLF on Windows. Integration files are
written as LF bytes, so the restored file no longer matched its
manifest hash and the second upgrade was still blocked as modified
(pytest on windows-latest). Read and write the prompt as bytes.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Fixed the Windows pytest failure in cb898ce. The problem was in the test I added, not the Kiro change.

test_upgrade_replaces_dotted_kiro_prompts edits .kiro/prompts/speckit.plan.md to check that upgrade stops on a modified prompt, then restores it. It restored the file with write_text(), which writes CRLF on Windows. write_file_and_record() writes LF bytes, so the restored file no longer matched its manifest hash, and the second upgrade was still blocked. The test now reads and writes the prompt as bytes.

The macOS 3.13 job was cancelled by fail-fast after the Windows failure; it didn't fail itself. The upgrade and Kiro test modules pass locally (75).


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent diagnosed the CI failure from the job logs, fixed the test and wrote this comment.

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.

Copilot review overview

🔵 Needs a closer look

Upgrade leaves legacy dotted extension and preset prompts behind, and the unrestricted headless permission mode is undocumented.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Migrate existing Kiro prompts during same-directory upgrades

src/​specify_cli/​integrations/​kiro_cli/​__init__.py:69

Existing extension and preset prompts are not migrated. On upgrade, this formatter makes re-registration write new hyphenated files, but command_upgrade.py:314-345 only unregisters old registrations when the command directory changes; Kiro's directory remains .kiro/prompts. Those artifacts are tracked outside the integration manifest, so their old speckit.*.md copies remain beside the replacements. Add same-directory naming-migration cleanup before re-registration and cover an upgrade with an installed extension and preset.

Low severity Document Kiro headless workflows' automatic tool approval

docs/​reference/​integrations.md:34

Document that headless Kiro workflow dispatch always passes --trust-all-tools, which auto-approves tool use. This is a security-relevant runtime default introduced by this PR; unlike the MiniMax row below, the current Kiro row only describes prompt naming and argument substitution, so users cannot discover the permission behavior from the integration reference.

@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

Upgrade only unregistered enabled extension commands when the command
directory changed. Kiro keeps .kiro/prompts but renamed its files, and
extension prompts are tracked in the extension registry rather than the
manifest, so upgrading a project with the git extension left the five
speckit.git.*.md prompts beside the new speckit-git-*.md ones. Treat a
same-directory rename of the core command files like a directory change,
so the existing cleanup removes them before re-registration.

Also document that headless Kiro dispatch passes --trust-all-tools.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Addressed the latest Copilot review in 270f740.

Extension prompts on upgrade. Confirmed and fixed. I initialized a project with the code before this PR and added the bundled git extension, then ran specify integration upgrade kiro-cli on the branch. The 10 core prompts were replaced, but the five speckit.git.*.md prompts stayed beside the new speckit-git-*.md ones. That happened because upgrade only calls _unregister_enabled_extension_commands_for_agent() when the command directory changes. _command_file_names_changed() now also triggers it when the core command files were renamed inside the same directory. unregister_commands() already removes both the formatted name and the raw registered name, so the existing cleanup deletes the dotted files before re-registration. The real upgrade now leaves 15 speckit-*.md prompts and no dotted ones. test_upgrade_replaces_dotted_kiro_prompts now installs the git extension under the old naming and fails without the fix.

Preset prompts. I didn't reproduce a leftover here. With the bundled lean preset installed under the old naming, upgrade --force (the preset overrides make a plain upgrade stop as "modified", same as on main) left the lean content in the hyphenated core prompts and no dotted copies.

--trust-all-tools. The Kiro row in docs/reference/integrations.md now says headless dispatch runs kiro-cli chat --no-interactive --trust-all-tools, which auto-approves every tool call, and why.

The full suite passes: 8771 passed, 251 skipped. ruff check src tests and markdownlint are clean.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent reproduced the upgrade cases, made the change and wrote this comment.

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.

Copilot review overview

🟡 Changes recommended

Filename migration can leave stale preset prompts and may misclassify ordinary command inventory changes as renames.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (1)

Comment thread src/specify_cli/integrations/_command_upgrade_layout.py Outdated
Comment thread src/specify_cli/integrations/command_upgrade.py Outdated
@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

_command_file_names_changed() now needs a removed command file and an
added one with the same name up to "."/"-" separators, so a release that
adds one command and drops another no longer unregisters extension
commands. When the rename does happen on the active integration, preset
commands are unregistered before re-registration too, so dotted preset
prompts such as speckit.fakeext.cmd.md don't survive the upgrade.

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Both findings from the latest Copilot pass are addressed in 569a1bd.

Rename detection. _command_file_names_changed() no longer treats "some files added, some removed" as a rename. It only fires when a removed command file and an added one have the same name once . and - are treated as the same separator, as with speckit.plan.md → speckit-plan.md. A release that adds speckit.new.md and drops speckit.old.md no longer unregisters extension commands. test_command_file_names_changed_needs_a_rename covers the rename case, the add-and-drop case, and an add-only case.

Preset commands. When that rename happens on the active integration, upgrade now calls _unregister_presets_for_agent() before the existing _register_presets_for_agent(), the same pairing switch uses. test_upgrade_replaces_dotted_kiro_prompts now installs a preset with a custom speckit.fakeext.cmd command under the old naming. After the upgrade, it checks that no speckit.*.md prompt is left and that speckit-fakeext-cmd.md has the preset content. Without the change, that test fails with speckit.fakeext.cmd.md still in .kiro/prompts.

tests/specify_cli/integrations, tests/integrations and tests/specify_cli/presets pass (3668 passed, 6 skipped), and ruff is clean on the changed files.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent reproduced both findings, made the change and wrote this comment.

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.

Copilot review overview

🟡 Changes recommended

Filename-change detection incorrectly treats unrelated skill additions and removals as renames.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread src/specify_cli/integrations/_command_upgrade_layout.py Outdated
@kartsan03

Copy link
Copy Markdown
Contributor Author

Addressed Copilot's "previously missed" finding on ed42004 in 234bbd2. I reproduced it on a project made by the released 1.0.13 with the git extension, then installed Claude and made it the default, so Kiro was secondary. On ed42004, specify integration upgrade kiro-cli renamed the 10 core prompts but left the 5 speckit.git.*.md prompts, because upgrade re-registers extensions only for the active integration (#2948). integration use kiro-cli or integration switch kiro-cli then wrote the 5 speckit-git-*.md prompts beside them. A later upgrade no longer saw a rename, so the dotted ones stayed for good. An extension that was disabled during the upgrade and enabled later ended up the same way.

What changed

  • Kiro now declares its dotted prompts as legacy flat command files (legacy_flat_command_dir = ".kiro/prompts"). ExtensionManager._retire_legacy_flat_extension_commands() retires them, the same step Qoder's skills migration ([bug-fix] Fix qodercli-skills-migration: migrate QodercliIntegration to SkillsIntegration #4205) uses: after a registration pass writes an extension command's new file, it removes the old one. It runs on every pass (use, switch, and upgrade of the active integration), so it no longer depends on the upgrade detecting the rename. It only handles names that pass just wrote and whose replacement exists, so an extension that can't be re-registered keeps its old prompt and its registry entry.
  • That step assumed the replacement is a SKILL.md. It now uses the registrar's output path, and it skips a name that is its own replacement. In Kiro a dot-free alias such as speckit-git-c is its own replacement, and without that check its new prompt would be deleted.
  • The upgrade-only _retire_renamed_command_files() from cc91560 is removed, so integrations/_helpers.py matches main again. Rename detection now feeds only the preset guard, which also refuses the rename when Kiro is upgraded as a secondary integration.
  • I merged main first, because Register extension commands and skills for generic integration #4785 and feat(extensions): select exact catalog releases #4726 changed extensions/__init__.py.

Tests

  • test_activating_kiro_after_secondary_upgrade_retires_dotted_prompts (use and switch) and test_enabling_extension_after_kiro_rename_retires_its_dotted_prompts fail on ed42004 (the dotted git prompts remain) and pass now.
  • test_kiro_prompt_named_without_dots_is_not_retired fails when the same-path check is removed.
  • The existing Kiro migration tests and the Qoder test pass unchanged.

Real runs (offline, project made by 1.0.13 as above):

ed42004 234bbd2
Kiro secondary: upgrade kiro-cli, then use kiro-cli or switch kiro-cli 15 speckit-*.md plus 5 speckit.git.*.md, still there after another upgrade 15 speckit-*.md, no dotted prompts
Kiro active, git disabled during upgrade, then extension enable git and upgrade 5 speckit.git.*.md remain 15 speckit-*.md, no dotted prompts
  • Failure path: with the git extension's extension.yml corrupted before use kiro-cli, its 5 dotted prompts stay and the registry still tracks them.
  • Presets: with lean registered for Kiro on 1.0.13 and Claude as the default, integration upgrade kiro-cli --force is refused before any file changes. preset remove lean, the upgrade, use kiro-cli and preset add lean then leave 15 speckit-*.md prompts with lean's content.

Full suite: 9046 passed, 250 skipped. ruff check src tests (0.15.0) is clean. The Migration line and the test list in the description are updated.

@mnriem this is ready for another Copilot pass.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent reproduced the finding, made the change, ran the checks above and wrote this comment.

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.

Copilot review overview

🟡 Changes recommended

A failed extension re-registration can leave a legacy dotted prompt untracked and orphaned.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread tests/specify_cli/integrations/test_command_upgrade.py
… missing

register_commands skips a missing source and returns only the names it
wrote. The command-mode registry update then replaced that agent's
registered_commands entry with the short list, or dropped the entry
when nothing was written. The prompt stayed on disk, and extension
remove no longer deleted it. The same drop hit a hyphenated prompt an
earlier pass had already written, and an alias from that source.

Keep a previously registered name when this pass did not write it and
the manifest still declares it. A name the manifest no longer declares
is not kept, because removal deletes the formatted path and another
extension may now own that file. Retirement is unchanged: only names
written this pass, and only once that pass's replacement exists.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Addressed Copilot's finding on 234bbd2 in 3bcee7e.

The test on that commit corrupts extension.yml, so get_extension() returns None and registration never runs. With a valid manifest and a missing command source, register_commands returns only the names it wrote. The command-mode update replaced registered_commands["kiro-cli"] with that list, or removed the entry when the list was empty. The prompt stayed on disk and extension remove no longer deleted it.

What changed

  • A previously registered name stays when this pass did not write it and the manifest still declares it. Nothing that was never registered is added.
  • A name the manifest no longer declares is not kept. extension remove deletes the formatted path, so a stale speckit.git.commit would unlink speckit-git-commit.md after another extension had taken that file.
  • Retirement is unchanged. It still runs only for names written this pass, and only once that pass's replacement file exists. A file already at the new path is not treated as this pass's write.

Tests. The first three fail on 234bbd2.

  • test_upgrade_keeps_tracking_when_one_kiro_command_source_is_missing: one git source deleted, the dotted prompt and its name stay, the commands that were written are renamed, and extension remove git --force deletes the leftover dotted prompt and leaves speckit-plan.md.
  • test_upgrade_keeps_tracking_when_every_kiro_command_source_is_missing: every source deleted, the kiro-cli entry is not removed, and remove deletes every dotted extension prompt.
  • test_upgrade_keeps_tracking_when_a_rewritten_kiro_source_disappears: after a successful upgrade, deleting the source keeps speckit-git-commit.md and the alias speckit-git-c.md tracked, and remove deletes both.
  • test_upgrade_drops_a_kiro_command_the_manifest_no_longer_declares: removing speckit.git.commit from the manifest drops that name and its alias.

Full suite: 9050 passed, 250 skipped (Linux, Python 3.13). ruff check src tests (0.15.0) is clean.

@mnriem Please re-run Copilot on 3bcee7e. The fork workflows will need approval before CI runs.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent wrote the code, the tests and this comment.

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.

Copilot review overview

🟡 Changes recommended

Hyphenation can merge distinct valid extension command names, causing prompt overwrites and unsafe cleanup.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread src/specify_cli/integrations/kiro_cli/__init__.py
@mnriem

mnriem commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

format_kiro_command_name replaces dots and is not injective.
speckit.foo.bar-baz and speckit.foo-bar.baz both become
speckit-foo-bar-baz, and so can another command's alias. Registration
overwrote one prompt, retired both dotted files, and removing either
extension deleted the shared replacement.

Install rejects a new command or alias that hyphenates onto another
command or a core prompt. A command's own alias may reuse its
hyphenated name, because both render the same body. A pair that is
already installed is left in place: registration warns, does not write
either prompt, and does not retire either dotted file, and the
extension's other commands still migrate. Enable refuses before it
flips the flag. Removal writes the remaining owner's source back into
the shared file when that source can be read, then deletes only the
removed command's own file. A name the manifest drops is rewritten the
same way, including when the other owner is disabled. Presets skip a
colliding command, including the reconcile pass, while an exact core
or extension override still writes. Skills follow the same rule.

The whole upgrade still exits 0. Preserving the shared file was the
alternative to refusing the migration, and the core rename does not
depend on one colliding extension. A collision check that cannot read
generic settings does not replace that enable error, and the filtered
command list is passed only when a name was dropped, so one failing
extension still does not abort the rest.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Hyphenation is not injective, so speckit.foo.bar-baz and speckit.foo-bar.baz both become speckit-foo-bar-baz, and an alias can land on the same file.

Install now rejects that pair, including an alias of one command that hyphenates onto another command or onto a core prompt. A command's own alias may still reuse its hyphenated name, because both render the same body. speckit.foo.plan stays legal: it is not the core plan command.

A pair that is already installed is left in place. Registration warns, does not write either prompt, and does not retire either dotted file. The extension's other commands still migrate, and the core rename still runs. I did not fail the whole upgrade. The review asked to refuse the migration or otherwise preserve ownership, and preserve keeps the core rename independent of one colliding extension. extension enable does refuse, before it flips the flag.

Removal writes the remaining owner's source back into the shared file when that source can be read, then deletes only the removed command's own file. If the source cannot be read, the shared file stays. Dropping a command the manifest no longer declares does the same rewrite, including when the other owner is disabled and this pass does not register it. A file only the dropped command owned is still deleted.

Preset registration skips a colliding command, and the reconcile step no longer writes it afterwards. An exact override of a core command, or of an extension command's own name, still writes. Skills follow the same preserve-and-rewrite rule.

use and switch still do not run integration.setup, so a dotted core prompt such as speckit.plan.md is renamed by integration upgrade, not by selecting Kiro.

Full suite: 9063 passed, 250 skipped (Linux, Python 3.13). ruff check src tests (0.15.0) is clean.

@mnriem Please re-run Copilot on 62732e8. The fork workflows will need approval before CI runs.


Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent wrote the code, the tests and this comment.

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.

Copilot review overview

🟡 Changes recommended

Shared skill restoration only updates the active agent, leaving historical agent mirrors with removed-extension content.

Review effort: Balanced
Findings: 2 High severity

Open (2)

Comment thread src/specify_cli/extensions/__init__.py Outdated
@mnriem

mnriem commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

main's github#4823 pins the zip entry time in the catalog test archive helper,
so this branch's copy of that fix (ed42004) is dropped in favour of
main's version of tests/specify_cli/workflows/test_catalog_versions.py.

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
…riting

62732e8 kept extension commands that hyphenate to one prompt
(speckit.foo.bar-baz and speckit.foo-bar.baz) by rewriting the shared
file from whichever owner remained, across removal, enable, presets and
skills. That rewriting is replaced by rules that leave such files alone.

Install and update still reject a command or alias that writes the same
file as a core command or another installed extension once dots become
hyphens. For a pair installed before that check, or an alias such as
plan from Spec Kit 1.0.7 and earlier, registration for Kiro CLI and
Qoder skips the extension through the per-extension error path from
github#2950. It writes and retires nothing, the registry stays as it was, and
the warning names the other writer. Once one extension of a pair is
removed, the other migrates on the next pass.

An alias such as speckit-plan has its prompt where the renamed core
speckit.plan goes, so integration upgrade refuses the rename while one
is registered, as it does for preset overrides. Extension removal keeps
a core command's file only when the integration manifest tracks it:
before the rename it deletes that alias's own prompt, so a dev-mode
symlink is not left dangling.

Presets and extension enable are back to main's code.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Pushed 8570f0a. It replaces the collision handling from 62732e8 instead of patching it.

62732e8 kept colliding commands working by rewriting the shared prompt from whichever owner remained, in removal, enable, presets and skills. Each path needed its own ownership rules, and the skill restoration missed the other agents' directories (r4185677037). The new rule never writes a shared file:

  • Install and update still reject names that write the same file once dots become hyphens.
  • A pair installed before that check is not registered for Kiro CLI or Qoder. Nothing is written, retired or untracked, and the warning names the other writer. The other extension migrates once one of them is removed.
  • integration upgrade refuses the rename while an older alias such as speckit-plan already has its prompt where the core prompt goes, as it does for preset overrides.
  • Extension removal keeps a core command's file only when the integration manifest tracks it.
  • presets/_manager_commands.py and extensions/command_enable.py are back to main. The source diff against main went from +1341/−46 in 9 files to +611/−32 in 7.

I also merged main. #4823 already pins the zip entry time, so this branch's _archive() change is gone.

Projects created with Spec Kit 1.1.0 and 1.0.7 were upgraded on this branch (details in the description):

  • Colliding pair (1.1.0): at 3bcee7e the upgrade lost one extension's body. Here both stay byte-identical.
  • speckit-plan alias (1.0.7): at 3bcee7e the upgrade overwrote the core prompt and extension remove then deleted it. Here the upgrade is refused first, and after the extension is removed it writes the core prompt.

Full suite: 9484 passed, 265 skipped. ruff and markdownlint: clean.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent made the change, ran the upgrade comparisons above and wrote this comment.

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.

Copilot review overview

🟡 Changes recommended

Qoder legacy cleanup and unreadable-extension collision ownership remain unsafe.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity

Open (3)
Resolved since last review (1)

Comment thread src/specify_cli/extensions/__init__.py
Comment thread src/specify_cli/extensions/__init__.py
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

…oder commands

The install check built its name map from readable manifests only, so an
installed extension whose extension.yml can't be read did not count, and a
colliding install could overwrite its files. Fall back to the command names
the registry tracks for it.

Registration leaves a skipped extension's old flat commands in place. For
Qoder they sit in .qoder/commands, outside the registrar's directory, so
extension removal left them behind. Removal now deletes the tracked old
flat files too (never a core command's own), reusing the registration-time
retirement step without its replacement check.

Refs github#4797

Assisted-by: Claude Code (model: claude-opus-5-5, autonomous)
@kartsan03

Copy link
Copy Markdown
Contributor Author

Pushed fdb8d75 with fixes for both findings from the last review:

  • An installed extension whose manifest can't be read now counts at install and update, with the command names the registry tracks for it (r4194890103).
  • Extension removal deletes the extension's tracked old flat commands. A Qoder extension that registration skipped no longer leaves .qoder/commands/*.md behind, and a core command's own old file is never removed (r4194890191).

Real run with a 0.16.5 Qoder project holding speckit.foo.bar-baz and speckit.foo-bar.baz:

  • On main, the upgrade merged them into one skill and deleted both old commands.
  • Here, both are skipped, extension remove foo-bar deletes its old command, and the next upgrade migrates foo.

Full suite: 9485 passed, 265 skipped. ruff: clean.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous). The agent made the change, ran the Qoder comparison above and wrote this comment.

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.

Copilot review overview

🔵 Needs a closer look

The cross-cutting migration and cleanup logic warrants final human validation despite extensive regression coverage.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)

@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

@kartsan03

Copy link
Copy Markdown
Contributor Author

Ready for review as is. The last Copilot pass on fdb8d75 raised no new findings and resolved the two from the previous round. I replied on the remaining collision thread (r4184204489) with the fix and resolved it, so no Copilot threads are open.

For the closer look Copilot asked for, the cross-cutting parts are:

  • Install and update: _validate_install_conflicts rejects a command or alias that hyphenates to a core command's file or another extension's file. Extensions whose manifest can't be read count by their registered names.
  • Registration (register_enabled_extensions_for_agent → _shared_command_files): for Kiro CLI and Qoder, an older extension that shares a file is skipped through the existing per-extension error path (register_enabled_extensions_for_agent has no per-extension error isolation: one failing extension silently drops the rest #2950). Nothing is written or retired, and the warning names the other writer.
  • Upgrade (command_upgrade.py): the Kiro rename is refused while presets have Kiro commands, or while an old alias such as speckit-plan owns a core prompt's new file name.
  • Removal (ExtensionManager.remove): core files are kept only when the integration manifest tracks them, and the extension's old flat files are deleted.

Real runs on projects from 1.1.0, 1.0.7 and 0.16.5 (Qoder) are in the description, each compared with the previous behaviour. Full suite: 9485 passed, 265 skipped.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: kiro-cli workflow dispatch exits 2, and Kiro doesn't expand dotted /speckit.* prompt names

3 participants