Skip to content

refactor(integrations): own invocation rendering - #4983

Open
mnriem wants to merge 7 commits into
github:mainfrom
mnriem:mnriem-integration-invocation-ownership
Open

mnriem wants to merge 7 commits into
github:mainfrom
mnriem:mnriem-integration-invocation-ownership

Conversation

@mnriem

@mnriem mnriem commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Description

Moves command naming and invocation rendering ownership into integration classes, using the behavior characterized by #4975 as the preservation boundary.

The shared init, command registrar, extension skill and hook paths, preset rendering, and integration runtime now resolve naming and invocation behavior through the active integration. Dollar-prefixed skills, Kimi's /skill: syntax, Cline/Forge/Junie formatting, and mode-sensitive Bob/Copilot behavior live with their owning integrations. The obsolete _invocation_style.py identity tables are removed, and extension code no longer imports concrete integration formatters.

Generated filenames, command-reference replacement, displayed and hook invocations, custom non-Spec-Kit hook commands, and dual-mode behavior remain unchanged. Identity checks intentionally retained are limited to onboarding guidance and companion/content artifact handling, which are outside this ownership change.

Testing

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

Exact commands and results:

  • uv sync --extra test --quiet — passed.
  • .venv/bin/python -m pytest tests/integrations/test_integration_characterization.py tests/integrations/test_base.py tests/integrations/test_registry.py tests/integrations/test_integration_alquimia.py tests/integrations/test_integration_docker_agent.py tests/integrations/test_integration_droid.py tests/integrations/test_integration_dsh.py tests/integrations/test_integration_zed.py tests/extensions/bug/test_bug_hooks.py tests/extensions/github/test_github_extension.py -q — 1,487 passed, 4 skipped.
  • .venv/bin/python -m pytest tests/test_extensions.py::TestHookInvocationRendering tests/integrations/test_integration_characterization.py tests/integrations/test_integration_cline.py tests/integrations/test_integration_forge.py tests/integrations/test_integration_junie.py tests/integrations/test_base.py -q — 219 passed.
  • .venv/bin/python -m pytest tests/integrations tests/specify_cli/integrations tests/extensions tests/specify_cli/extensions tests/specify_cli/presets tests/specify_cli/test_command_init.py tests/test_init_dir.py tests/test_init_dir_cli.py tests/test_extension_registration.py tests/test_extension_skills.py tests/test_extensions.py tests/test_presets.py tests/test_command_template_hooks.py -q — 6,255 passed, 14 skipped.
  • .venv/bin/python -m pytest tests/test_agent_config_consistency.py -q — 31 passed.
  • .venv/bin/python -m pytest tests -q — 12,581 passed, 101 skipped.
  • After rebasing onto current upstream/main: .venv/bin/python -m pytest tests/integrations/test_integration_characterization.py tests/test_extensions.py::TestHookInvocationRendering tests/test_agent_config_consistency.py -q — 56 passed.
  • git diff --check, .venv/bin/python -m compileall -q src/specify_cli, and uvx ruff check --select F <changed Python files> — passed.

The sample-project coverage is exercised by the table-driven specify init completion-guidance characterization tests for Command Code, Forge, Kimi, Copilot, and DSH.

AI Disclosure

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

AI disclosure: GitHub Copilot, powered by GPT-5.6 Sol, operated autonomously in autopilot mode with the runtime-default reasoning setting. It inspected the integration design and characterization baseline, implemented the production and test changes, ran focused and full validation, authored the commit, and drafted this pull request description.

Copilot AI max review requested due to automatic review settings October 15, 2026 19: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.

🔵 Needs a closer look

The refactor spans several rendering paths and includes a confirmed historical-skill restoration regression.

1 open finding
What changed in this PR

Moves command naming and invocation rendering into integration classes, aiming to preserve the delivery contracts characterized in #4975.

Changes:

  • Adds integration-owned naming, reference, and hook rendering methods.
  • Updates shared callers and removes centralized invocation-style tables.
  • Revises integration tests and design documentation.

AI disclosure: GitHub Copilot for the requester; model/settings unavailable, autonomous review and fix suggestions.

File Description
tests/​integrations/​test_integration_zed.py Renames style tests and updates comments.
tests/​integrations/​test_integration_dsh.py Clarifies the invocation-style docstring.
tests/​integrations/​test_integration_droid.py Tests integration-owned references.
tests/​integrations/​test_integration_docker_agent.py Tests native skill references.
tests/​integrations/​test_integration_alquimia.py Replaces centralized-helper assertions.
tests/​integrations/​test_base.py Tests integration-owned prefixes.
tests/​extensions/​github/​test_github_extension.py Derives dollar-prefix coverage from integrations.
src/​specify_cli/​presets/​_manager_skills.py Delegates preset skill references.
src/​specify_cli/​integrations/​zcode/​__init__.py Declares dollar-prefix skill style.
src/​specify_cli/​integrations/​vibe/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​rovodev/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​lingma/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​kimi/​__init__.py Declares the native /skill: prefix.
src/​specify_cli/​integrations/​junie/​__init__.py Owns hook command formatting.
src/​specify_cli/​integrations/​hermes/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​generic/​__init__.py Selects separators by output mode.
src/​specify_cli/​integrations/​forge/​__init__.py Owns hook command formatting.
src/​specify_cli/​integrations/​cursor_agent/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​command_code/​__init__.py Declares dollar-prefix skill style.
src/​specify_cli/​integrations/​codex/​__init__.py Declares dollar-prefix skill style.
src/​specify_cli/​integrations/​cline/​__init__.py Owns hook command formatting.
src/​specify_cli/​integrations/​claude/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​base.py Adds shared naming and rendering methods.
src/​specify_cli/​integrations/​alquimia/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integrations/​agy/​__init__.py Declares mode-gated skill style.
src/​specify_cli/​integration_runtime.py Delegates prefix selection.
src/​specify_cli/​extensions/​__init__.py Uses integration rendering for skills and hooks.
src/​specify_cli/​command_init.py Delegates displayed command references.
src/​specify_cli/​agents.py Delegates registered names and references.
src/​specify_cli/​_invocation_style.py Removes centralized identity tables.
design/​integration.md Documents rendering ownership.

🧠 Review effort: Max


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

Comment thread src/specify_cli/presets/_manager_skills.py
@mnriem mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Oct 15, 2026
Copilot AI max review requested due to automatic review settings October 16, 2026 21:12
@mnriem
mnriem force-pushed the mnriem-integration-invocation-ownership branch from 31591d7 to 4c79819 Compare October 16, 2026 21:12
@mnriem

mnriem commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review feedback in 4c79819c.

  • Historical preset skill restoration now derives skills mode from a target registrar that writes /SKILL.md.
  • Dual-layout integrations continue using the persisted project mode.
  • Added a Claude → Codex → command-mode Qwen removal regression that verifies restored /speckit-plan and $speckit-plan references.

Validation:

  • .venv/bin/python -m pytest tests/specify_cli/presets/test_manager_skills.py tests/specify_cli/presets/test_manager_commands.py tests/specify_cli/presets/test_manager.py -q — 213 passed.
  • After rebasing onto current upstream main, .venv/bin/python -m pytest tests/specify_cli/presets/test_manager_skills.py::TestPresetSkills::test_skill_switch_then_command_switch_remove_restores_skill_refs tests/integrations/test_integration_characterization.py -q — 14 passed.
  • git diff --check, Ruff fatal-error checks, and source compilation passed.

AI disclosure: Posted on behalf of Manfred Riem by GitHub Copilot using GPT-5.6 Sol in autonomous autopilot mode; implementation, test generation, validation, commit authoring, and comment drafting were AI-assisted.

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.

🔵 Needs a closer look

This cross-cutting integration refactor has unresolved adapter correctness issues and needs human review.

3 open issues
1 resolved since last review

Review effort: Max

Comment thread src/specify_cli/extensions/__init__.py Outdated
Comment on lines +144 to +148
invocation_prefix: str = "/"
"""Prefix used for rendered command references in the native UI."""

skills_invocation_only: bool = False
"""Whether the skills invocation style applies only in skills mode."""

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in a7370854. _validate_implementation() now requires invocation_prefix to be a non-empty string and skills_invocation_only to be a boolean. The invalid public-attribute matrix covers None, empty/non-string prefixes, and non-boolean mode flags, and design/integration.md documents both type requirements.

Validation: complete external-adapter suite 681 passed; hook/integration lifecycle suites 546 passed; post-rebase focused review regressions 17 passed.

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

Comment thread src/specify_cli/integrations/base.py Outdated
Copilot AI max review requested due to automatic review settings October 19, 2026 12:46
@mnriem
mnriem force-pushed the mnriem-integration-invocation-ownership branch from 4c79819 to a737085 Compare October 19, 2026 12:46
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5553926559 in a7370854:

  • pins external adapter lookup and hook rendering to HookExecutor's project scope, with fresh-process and interleaved-project regressions;
  • validates external invocation_prefix and skills_invocation_only attributes and documents their types;
  • preserves hyphenated installed skill names for runtime dispatch while retaining integration-owned prefixes.

Validation:

  • .venv/bin/python -m pytest tests/specify_cli/integrations/test_installed_adapters.py -q — 681 passed
  • .venv/bin/python -m pytest tests/test_extensions.py tests/test_extension_registration.py tests/specify_cli/integrations/test_command_install.py tests/specify_cli/integrations/test_command_use.py tests/specify_cli/integrations/test_command_switch.py -q — 546 passed
  • post-rebase focused review regression set — 17 passed
  • git diff --check and Ruff fatal checks — passed

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

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.

Comment thread src/specify_cli/agents.py
Comment thread src/specify_cli/integrations/base.py Outdated
Copilot AI max review requested due to automatic review settings October 19, 2026 14:04
@mnriem
mnriem force-pushed the mnriem-integration-invocation-ownership branch from a737085 to 45af679 Compare October 19, 2026 14:04
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5572004563 in 45af6793:

  • pins command unregistration to the target project's adapter registry for the complete cleanup operation, with an interleaved same-key adapter regression that protects unrelated files;
  • passes resolved output mode independently from command-reference separator through template rendering, preserving skills-only prefixes for adapters with custom separators;
  • retains the prior validation fix for invocation_prefix and skills_invocation_only in rebased commit 82cf3756.

Validation:

  • .venv/bin/python -m pytest tests/specify_cli/integrations/test_installed_adapters.py -q — 683 passed
  • affected integration rendering suites — 707 passed
  • command-template and unregister coverage — 178 passed
  • post-rebase focused review regressions — 21 passed
  • git diff --check, Ruff fatal checks, and source compilation — passed

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

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.

🔵 Needs a closer look

The cross-cutting rendering refactor retains project-isolation risks and needs final human review.

2 open issues
2 resolved since last review

Review effort: Max

Comment thread src/specify_cli/agents.py
Comment on lines +586 to +589
integration = get_integration(agent_name)
if integration is not None:
return integration.format_registered_command_name(
cmd_name, agent_config

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 63db5158. Artifact resolution now executes the integration-owned filename lookup inside project_integrations(project_root). Extension update backup, preflight candidate tracking, and rollback cleanup all use a shared project-scoped naming helper, so a later same-key adapter load cannot combine project A's registrar snapshot with project B's naming hook.

Added two interleaved-project regressions: artifact resolution snapshots A, loads B, and still resolves A's materialized command; a failed extension update loads B after A's registrar snapshot, removes A's managed command, and verifies rollback restores the original A-named file.

Validation: complete external-adapter suite 685 passed; artifact resolution suites 160 passed; extension update transaction suites 56 passed; post-rebase focused regressions 16 passed.

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

@mnriem mnriem added the author-awaiting Waiting on author response label Oct 19, 2026
Copilot AI max review requested due to automatic review settings October 19, 2026 14:38
@mnriem
mnriem force-pushed the mnriem-integration-invocation-ownership branch from 45af679 to 63db515 Compare October 19, 2026 14:38
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5572977507 in 63db5158:

  • scopes artifact-resolution command naming to the target project's integration registry;
  • scopes extension-update backup, candidate tracking, and rollback naming lookups to the target project;
  • adds interleaved same-key adapter regressions for artifact source resolution and failed-update restoration;
  • retains the prior adapter attribute validation fix in rebased commit 044e6e4c.

Validation:

  • .venv/bin/python -m pytest tests/specify_cli/integrations/test_installed_adapters.py -q — 685 passed
  • artifact resolution suites — 160 passed
  • extension update transaction suites — 56 passed
  • post-rebase focused review regressions — 16 passed
  • git diff --check, Ruff fatal checks, and source compilation — passed

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

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.

🔵 Needs a closer look

The cross-cutting rendering changes affect project-scoped adapters, destructive cleanup, and rollback paths that warrant final maintainer verification.

3 open issues

Review effort: Max

Comment thread src/specify_cli/extensions/__init__.py Outdated
Move registered command naming, command-reference rendering, and hook invocation formatting behind resolved integration classes while preserving existing mode-specific behavior.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Render restored SKILL.md references from the historical target registrar while retaining persisted mode for dual-layout integrations.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Validate public invocation adapter attributes, pin hook rendering to the executor project, and keep skill runtime dispatch aligned with installed hyphenated names.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Keep command removal scoped to the target project's adapter registry and pass resolved output mode independently from command-reference spelling.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Pin integration-owned command naming to the target project during artifact resolution and extension update backup and rollback paths.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Delegate extension skill placeholders to integration-owned and base fallback resolvers so grammar and override behavior stay aligned.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Copilot AI max review requested due to automatic review settings October 19, 2026 15:29
@mnriem
mnriem force-pushed the mnriem-integration-invocation-ownership branch from 63db515 to ecb79db Compare October 19, 2026 15:29
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5573445895 in ecb79db0:

  • extension skill command-reference replacement now delegates to integration.resolve_command_refs_for_mode();
  • the no-integration path delegates to IntegrationBase.resolve_command_refs();
  • added regression coverage for integration resolver overrides and the base fallback seam.

Validation:

  • .venv/bin/python -m pytest tests/test_extensions.py tests/test_extension_registration.py -q — 488 passed
  • integration command-reference coverage — 11 passed
  • post-rebase focused regressions — 3 passed
  • git diff --check, Ruff fatal checks, and source compilation — passed

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

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.

🔵 Needs a closer look

Cross-cutting rendering and adapter-lifecycle changes need maintainer review, and a core-template override inconsistency remains unresolved.

3 open issues
1 resolved since last review

Review effort: Max

Comment thread src/specify_cli/integrations/base.py Outdated
Route core template placeholders through integration-owned resolver seams while preserving explicit separator behavior for the default renderer.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef
Copilot AI max review requested due to automatic review settings October 19, 2026 16:58
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5574148923 in 40c34802:

  • core template placeholders now honor integration overrides of resolve_command_refs_for_mode();
  • inherited resolver behavior remains polymorphic for build_command_reference() overrides;
  • the default integration resolver accepts the core caller's explicit separator while retaining the integration-owned prefix;
  • unknown integrations continue through the static base fallback.

Validation:

  • affected core integration rendering suites — 714 passed
  • .venv/bin/python -m pytest tests/specify_cli/integrations/test_installed_adapters.py -q — 685 passed
  • post-rebase focused regressions — 5 passed
  • git diff --check, Ruff fatal checks, and source compilation — passed

AI disclosure: GitHub Copilot for the requester; GPT-5.6 Sol, autonomous implementation.

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.

🔵 Needs a closer look

Integration-owned naming affects project-scoped rendering, cleanup, and rollback, requiring final human review.

3 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Add regression tests for manager skill resolver delegation

src/​specify_cli/​extensions/​__init__.py:1641

The new tests at tests/test_extensions.py:3985-4062 call CommandRegistrar.register_commands_for_agent(), which delegates to agents.CommandRegistrar.register_commands() (extensions/__init__.py:4433), not this manager method. Existing manager tests cover built-in spellings but would not catch reverting this resolver delegation.

Add regression cases through ExtensionManager._register_extension_skills() or install_from_directory(..., register_commands=False), with Codex skills mode enabled. Check that a resolver override receives the full body and skills_enabled=True, and that a missing integration uses the base resolver with the configured separator. Assert the generated SKILL.md content in both cases. This covers the changed execution path required by CONTRIBUTING.md:227-248.

AI disclosure: GitHub Copilot for the requester; model/settings unavailable, autonomous review and suggestions.

1 resolved since last review

Review effort: Max

Comment on lines 111 to +112
skills_mode = integration.is_skills_mode(parsed_options, project_root)
return get_invocation_prefix(key, skills_mode)
return integration.invocation_prefix_for_mode(skills_mode)

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

author-awaiting Waiting on author response triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants