github-featured/github--spec-kit
2
0
Fork 0
Code Issues Pull Requests Deployments Models Agent Notes Packages Projects Releases Wiki Activity
1,859 Commits 40 Branches 217 Tags
main
Commit Graph

77 Commits

Author SHA1 Message Date
github-actions[bot] fa19e1c68b [bug-fix] Fix qodercli-skills-migration: migrate QodercliIntegration to SkillsIntegration (#4205) 
* Fix qodercli-skills-migration: migrate QodercliIntegration to SkillsIntegration

Apply the remediation from the bug assessment on issue #4199.
Qoder IDE 1.24+ dropped .qoder/commands/ scanning in favour of the
skills layout (.qoder/skills/{skill-name}/SKILL.md). Migrated
QodercliIntegration from MarkdownIntegration to SkillsIntegration,
updating config[commands_subdir] to 'skills' and
registrar_config[dir] to '.qoder/skills' with extension '/SKILL.md'.
Updated tests to use SkillsIntegrationTests base mixin.

Refs #4199

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(qodercli): resolve failing skills-flag test and slash invocation

Builds on the qodercli->SkillsIntegration migration (PR #4205). Qoder IDE
1.24+ is always skills-based, so it should not expose a --skills toggle.
Override the inherited SkillsIntegrationTests.test_options_include_skills_flag
to skip (mirroring Grok/Zed/Droid) and add a test asserting no --skills
option, plus a requires_cli/name/multi_install_safe check.

Also add "qodercli" to ALWAYS_SLASH_AGENTS so hooks and next-steps render
the hyphenated /speckit-<name> invocation instead of the legacy dotted
/speckit.<name> form.

Fixes the single failing test reported for #4199.

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 43394151-ce2a-432d-9cc5-88f587d1b570

* fix(qodercli): migrate legacy extension commands

Retire old flat Qoder extension commands only after their replacement skills are successfully written. Cover old-layout upgrades and both slash invocation states, and update the integration reference path.

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

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

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Copilot-Session: 43394151-ce2a-432d-9cc5-88f587d1b570
 2026-08-20 11:03:23 -05:00
Manfred Riem fa3a5c5ce7 Clarify extension catalog trust model in docs, help, and messaging (#4177) 
* Clarify extension catalog trust model in docs, help, and messaging (#4176)

Extension catalog management gave no explanation of why the community
catalog is discovery-only, and the install-error text nudged users to
flip a discovery catalog to install_allowed — exactly the wrong move.

- Docs: add a "discovery-only vs. install sources" trust-model section,
  document `add --from <url>` as the lightweight vetted-install path, and
  stop implying you should make community installable.
- Help: expand the `catalog` app and `--install-allowed` help to state the
  vetting intent instead of bare mechanics.
- Messaging: rewrite the not-installable errors in `add`, `search`, and
  `info` to point at `--from` and self-curated catalogs, and to say
  explicitly not to flip a discovery-only catalog to install_allowed.
- `catalog list` now prints trust-model guidance when a discovery-only
  catalog is active.
- Tests cover the new list guidance (present/absent).

Deliberately does not add a verb to toggle install_allowed on an existing
catalog: discovery-only is a security boundary, not an inconvenience.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a86c498e-f129-4422-9983-d1a33513fd4d

* Address PR review: copy-pasteable install hint and accurate --from warning (#4176)

- The discovery-only "install directly" hint used the user-typed argument,
  which can be a display name with spaces (resolved via search) and would
  break when copied as a shell command. Emit the resolved catalog ID
  (ext_info['id']) instead. Added a regression test.
- The `--from` untrusted-source warning claimed the URL was "not listed in
  any of your configured extension catalogs", which is false for a URL
  copied from a discovery-only catalog — the exact flow this PR documents.
  Reword it to state the install is bypassing trusted (install-allowed)
  catalogs, which is accurate regardless of discovery-catalog membership.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a86c498e-f129-4422-9983-d1a33513fd4d

* Harden install hints against catalog-controlled IDs; expose archive URL (#4176)

Second review round on #4177.

Shell-safety: catalog entry IDs (especially from discovery-only catalogs)
are not validated during catalog merge, and rich.markup.escape only
neutralizes Rich markup, not shell metacharacters. A malicious ID like
`foo; rm -rf ~` was interpolated into the `specify extension add ... --from`
command we encourage the user to copy. Add `_command_safe_id`, which only
emits an ID matching the manifest rule `^[a-z0-9-]+$` (via
VALID_EXTENSION_ARTIFACT_NAME_PATTERN) and otherwise falls back to a literal
`<extension-id>` placeholder. Applied to every suggested command in `add`,
`search`, and `info`.

Discoverability: the documented `--from <archive-url>` flow gave no CLI path
to obtain the URL. `extension info` now prints the candidate `download_url`
for a discovery-only entry (clearly flagged as needing vetting), and the docs
show `extension info <name>` as the way to get the archive URL.

Tests cover the resolved-ID hint, the unsafe-ID neutralization, and pass the
full extensions + CLI suites (635).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a86c498e-f129-4422-9983-d1a33513fd4d

* Reject leading-hyphen catalog IDs; test info archive-URL branch (#4176)

Third review round on #4177.

_command_safe_id: an ID like `--force` satisfies the manifest character rule
`^[a-z0-9-]+$` but Typer parses a leading hyphen as an option rather than the
positional extension argument, so an untrusted catalog could still yield a
non-copyable or option-altering suggested command. Reject a leading hyphen and
fall back to the `<extension-id>` placeholder.

Tests: cover the new `extension info` discovery-only branch that surfaces the
candidate `download_url` (plus the no-URL fallback), and the leading-hyphen
rejection. Full extensions suite green (528).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a86c498e-f129-4422-9983-d1a33513fd4d

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a86c498e-f129-4422-9983-d1a33513fd4d
 2026-08-17 15:48:59 -05:00
Manfred Riem 229022943c feat(presets): list presets in resolution/precedence order (#4086) (#4104) 
`specify preset list` now sorts installed presets by (priority, id) so the
printed order matches the actual resolution/composition order used by
PresetRegistry.list_by_priority(). Lower priority number = higher precedence;
ties are broken alphabetically by preset id. Adds a header and footer note
clarifying the ordering, updates the presets reference docs, and adds tests.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-13 10:00:16 -05:00
WOLIKIMCHENG bd04776dcc Clarify custom checklist ownership and lifecycle (#4028) 
* docs: clarify custom checklist lifecycle

* docs: clarify implement checklist marker ownership

---------

Co-authored-by: root <kinsonnee@gmail.com>
 2026-08-11 11:09:56 -05:00
Ángel Peña 6aa9431b24 Add Command Code integration to spec-kit (#4019) 
* Add Command Code integration to spec-kit

Adds `command-code` as a built-in skills-based integration so Spec Kit can
be installed into Command Code. Command Code loads agent skills from
`.commandcode/skills/speckit-<name>/SKILL.md` and invokes them in chat as
`$speckit-<command>`.

- New `CommandCodeIntegration` (SkillsIntegration) writing to
  `.commandcode/skills/`; declared multi-install safe (static, isolated
  agent root).
- Register in `_register_builtins()` and the integration catalog.
- Add `command-code` to `DOLLAR_SKILLS_AGENTS` so next-steps guidance
  renders `$speckit-*` invocations.
- Tests: reuse `SkillsIntegrationTests` mixin plus a dollar-invocation
  next-steps test; registry completeness updated.
- Docs: README and docs/reference/integrations.md (supported agents +
  multi-install-safe table).

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Assisted-by: Command Code (autonomous)

* Fix issue template agent lists to include command-code

The runtime AGENT_CONFIG now includes command-code, but the GitHub issue
templates and the consistency test's expected key list were not updated,
failing test_issue_template_agent_lists_match_runtime_integrations.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Assisted-by: Command Code (autonomous)

---------

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
 2026-08-10 14:44:20 -05:00
Manfred Riem 40037b1aca feat(init): scaffold managed .specify/.gitignore (#4000) 
* feat(init): scaffold managed .specify/.gitignore

Write a manifest-tracked `.specify/.gitignore` during shared-infra install
so machine-local Spec Kit state stays out of version control while everything
else under `.specify/` remains shareable:

- `feature.json` — the current-feature pointer, rewritten on every feature
  switch (per-checkout state, not something to share).
- `extensions/*/local-config.yml` — per-machine extension config overrides.

The file is routed through the same overwrite/skip/preserve policy as shared
templates: `--force` refreshes it, user edits are preserved on re-init, and
uninstall removes it via the manifest. Addresses github/spec-kit#2304.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9

* docs: correct .specify/.gitignore uninstall claim

The file is tracked in the shared-infra manifest (speckit.manifest.json),
not the per-integration manifest that `specify integration uninstall` loads.
Shared infrastructure is deliberately preserved on uninstall
(see test_uninstall_preserves_shared_infra), so `.specify/.gitignore` is
left in place rather than removed. Reword the code comment and core.md note
to state the actual behavior; keep the true benefits (force-refresh and
preserve-on-edit).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9

* revert: drop manual CHANGELOG.md edit

CHANGELOG.md is auto-generated; do not hand-edit it.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9

* test: add .specify/.gitignore to integration file inventories

The complete-file-inventory tests assert an exact match of every file
produced by `specify init`. Now that shared infra scaffolds a managed
`.specify/.gitignore`, add it to the expected inventories so the exact-match
assertions pass on both sh and ps script types.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9
 2026-08-06 09:08:09 -05:00
Manfred Riem 0fbd99d594 feat(copilot): default integration to skills (#3976) 
* feat(copilot): default integration to skills

Make Copilot skills the default while retaining the commands layout behind --integration-options="--commands". Preserve historical project layouts and validate conflicting mode flags before switch teardown.

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

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

Copilot-Session: 930d846b-8921-44ef-9f45-3e77c036b6b5

* fix(copilot): preserve layout state during migration

Keep target integration options isolated from fallback state, prefer the Copilot manifest when resolving layouts, and update dispatch coverage for the skills-first default.

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

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

Copilot-Session: 930d846b-8921-44ef-9f45-3e77c036b6b5

---------

Copilot-Session: 930d846b-8921-44ef-9f45-3e77c036b6b5
 2026-08-04 06:15:40 -05:00
Manfred Riem e9ffc9d8e7 feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT (#3952) 
* feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT

Resolve the non-interactive/init default integration from the
SPECKIT_INTEGRATION_DEFAULT environment variable, fitting the existing
SPECKIT_INTEGRATION_* namespace. Falls back to the hardcoded "copilot"
default when unset, and warns to stderr (rather than silently falling
back) when the value is not a registered integration key.

Wires the resolver into specify init (interactive prompt default and
non-interactive fallback), the init workflow step, and the bundle init
default. Adds unit and CLI tests and documents the variable.

Closes #3939

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0

* test: cover env-var default wiring for picker, workflow step, and bundle

Address PR review: add regression tests so each SPECKIT_INTEGRATION_DEFAULT
wiring site cannot silently revert to the hardcoded constant.

- init.py: interactive picker receives the resolved key as default_key.
- workflow init step: no step/workflow default + env var drives output
  integration and argv.
- bundle _resolve_init_integration: env-var default applies when unspecified,
  while explicit override and manifest-declared integration still win.

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0
 2026-08-03 12:20:49 -05:00
Noor ul ain cf71d00dfe fix(workflows): reject a retry gate whose verdict enum forbids the reset value (#3912) 
A gate with `on_reject: retry` consumes a bound reject verdict before
pausing by resetting the named input to `""` (documented behaviour, so a
later resume prompts again). Every `resume()` re-resolves the persisted
inputs through `_coerce_input`.

Those two rules collide when the bound input declares an `enum` that does
not list `""`. The reset writes a value the input's own enum forbids, and
the run wedges:

    inputs:
      spec_verdict:
        type: string
        enum: [approve, reject]
    steps:
      - id: review
        type: gate
        options: [approve, reject]
        on_reject: retry
        verdict_input: spec_verdict

    $ specify workflow run wf --input spec_verdict=reject
    Status: paused
    $ specify workflow resume <run_id> --input note=b
    Error: Input 'spec_verdict' value '' not in allowed values:
           ['approve', 'reject'].

The workflow validates clean and the first run looks fine, so the failure
only appears at the second resume. It is also unrecoverable in practice:
`_resolve_inputs` re-coerces the whole persisted map, so *any* resume that
supplies an input dies on the stored `""`. Only a resume with no inputs at
all still works -- and that is precisely the call that cannot deliver a new
verdict, which is the one thing the retry cycle exists to allow.

Extend the existing `verdict_input` cross-check (which already confirms the
name is declared) to also require that a retry-bound input's `enum` admits
the reset sentinel, and report it with a fix hint. To do that, thread the
input *definitions* through `_validate_steps` instead of just their names.

Rejected the alternative of popping the key instead of writing `""`: that
lets the input's `default` flow back in on the next resume, so a gate the
user just rejected would silently auto-approve.

Docs: note the `enum` requirement next to the reset behaviour it follows
from.

Adds 4 validation tests for the new guard plus a characterization test that
drives the engine directly to pin the wedge it prevents.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Assisted-by: Claude Opus 5 (1M context)
 2026-07-31 09:53:25 -05:00
Manfred Riem a3e183d069 feat: support tar archives for installs (#3874) 
* feat: support tar archives for installs

Add secure .tar.gz and .tgz parity with ZIP installation for extensions, presets, and workflows, including full workflow package preservation.

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

* chore: clean rebased archive imports

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

* fix: preserve hardened archive install behavior

Keep malformed ZIP diagnostics, filesystem-independent manifest selection, and reserved workflow overlays consistent after adding generic archive support.

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

* fix: extract staged workflow archives by descriptor

Avoid reopening a held staging path on Windows while retaining authoritative-inode archive validation.

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

* fix: extract catalog archives from verified bytes

Use the already bounded and SHA-verified response bytes directly so Windows file-sharing semantics cannot affect archive detection.

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

* fix: address archive install review feedback

Preserve forced preset reinstalls, sniff suffixless workflow archives without weakening YAML limits, and restore prior workflow packages before failed-install cleanup.

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

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

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd

---------

Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd
 2026-07-31 07:23:00 -05:00
Markus Wondrak 675143591d feat: bind gate verdict to workflow input via verdict_input (#3725) 
* feat: bind gate verdict to workflow input via verdict_input

Add an optional `verdict_input` field to gate steps that lets an
external system supply a verdict through a declared workflow input
instead of an interactive TTY prompt.

When the referenced input carries a non-empty string value that
matches one of the gate's `options` (case-insensitive), the gate
auto-decides, records the matched spelling in `output.choice`, and
applies the existing `on_reject` / abort / skip / retry semantics.
If the value is present but does not match an option, or is a
non-string, the gate fails immediately with a clear error message.
When the input is absent, null, or empty, the gate falls back to
today's TTY-prompt-or-pause behaviour unchanged.

The engine now persists `result.error` alongside each step's status
and output so that failed-step error messages survive across runs.
The CLI (`workflow run` and `workflow resume`) surfaces these
persisted errors after a failed or aborted run. `validate_workflow`
cross-references `verdict_input` against the workflow's declared
inputs block and reports an error for undeclared names, consistent
with the existing `wait_for` id cross-check.

Closes discussion: https://github.com/github/spec-kit/discussions/3717

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix workflow JSON error payloads

Include persisted step errors in _workflow_run_payload so workflow run/resume/status --json all surface failure reasons consistently. Add JSON-path tests for failed and successful runs.

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(workflows): reject verdict inputs in fan-out

Fan-out items share workflow inputs and cannot safely consume a bound gate verdict. Reject verdict_input bindings during validation and at runtime while preserving unbound gates.

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

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

* fix: update workflow command handling

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

---------

Co-authored-by: Markus <markus@example.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-07-30 07:39:49 -05:00
Eric X Engstfeld 7fc5b236c8 feat: Add Alquimia AI integration (#2734) 
* Add alquimia-ai as new integration: https://alquimia.ai

* Fix test cases for alquimia-ai integration. Add alquimia-ai to workflow.yml

* Add install url to alquimia-ai integration

* Renamed alquimia-ai to alquimia (cli native denomination)

* Fix unit tests for alquimia integration

* Minor fix in alquimia integration

* Fix typos and copilot findings

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Update tests cases and lint formatting

* Final fixes

* Rename alquimia_ai to alquimia module integration

* Make cli optional for alquimiia integration

* resolve review comments

* Fix copilot review

* Minor fixes: naming, remove unused code

* Update tests cases. Fix issues

* Fix unit tests

* Add alquimia context to default agent-context extension. Update cli requirment to support workflows

* Fix hints (suggestion)

* Add Alquimia AI as agent in github issue template. Fix unit tests

* Address review comments. Update docs

* Update test cases

---------

Co-authored-by: Eric Engstfeld <ericengstfeld@Erics-MacBook-Pro.local>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-07-28 07:54:13 -05:00
Marsel Safin 683bfd00c9 fix: register extensions for the active integration only (#3459) 
* fix: register extensions for the active integration only

extension add registered commands for every detected agent, and
integration upgrade back-filled enabled extensions for non-active
integrations. Maintainer direction on #2948: treat the project as
single-active. Only the active integration gets extension artifacts;
use/switch rescaffold the target when the user selects it.

- extension add now routes through the all-agents pass restricted to
  the active integration (only_agent), keeping detection and
  missing-skills-dir recovery safeguards. Projects without recorded
  init-options fall back to detection-based registration.
- integration upgrade re-registers extensions only when upgrading the
  active integration, reversing the #2886 back-fill for non-active
  targets at maintainer request.

Fixes #2948

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

* fix: address review feedback on active-only extension registration

- Restrict the extension-add active-integration fallback to projects
  with no recorded active key at all. A recorded but unsupported key
  (e.g. "generic", deliberately excluded from AGENT_CONFIGS) no longer
  falls back to registering every detected agent.
- Apply the same single-active rule to preset command overrides:
  PresetManager._register_commands now scopes registration to the
  active integration via only_agent.
- Add PresetManager.register_enabled_presets_for_agent, mirroring
  ExtensionManager.register_enabled_extensions_for_agent, and call it
  from integration use/switch/upgrade (active only) alongside the
  existing extension re-registration so presets are rescaffolded on
  activation instead of being written for inactive integrations.

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

* fix: address second round of review feedback (priority order, fail-closed, docs)

- register_enabled_presets_for_agent now processes presets in reverse
  priority order (lowest-precedence first) so the highest-precedence
  preset is written last and actually wins after `integration use`
  rescaffolds two overlapping preset command overrides. Verified this
  reproduces the previously reported reversed-priority bug and that the
  fix resolves it.
- _register_commands_for_active_agent now checks for the "ai" key's
  presence separately from its value: a missing key still falls back to
  detection-based registration for all agents, but a recorded, malformed
  value (non-string or empty, e.g. [] or null) now fails closed
  (registers nothing) instead of being treated as "no active
  integration" or reaching AGENT_CONFIGS.get() with an unhashable key
  and raising TypeError.
- Updated docs/reference/presets.md and docs/reference/integrations.md
  to describe active-only preset/extension registration and clarify
  that `integration use`/`switch` is the activation point for
  installed extensions and presets, and that `upgrade` only
  re-registers them for the active integration.

Adds regression tests: two enabled presets overriding the same command
with different priorities (priority winner must survive `use`
rescaffolding), and a malformed recorded `ai` value ([]) for
`extension add`.

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

* fix: address third round of review feedback (multi-integration semantics)

Fixes five deeper active-only registration bugs surfaced by Copilot review
after 2486c08, all in the presets/extensions single-active integration
rule (#2948):

1. presets: _reconcile_composed_commands (run after install/remove)
   bypassed the active-only filter entirely, writing composition-winner
   command files for every detected non-skill agent via
   register_commands_for_non_skill_agents. Added an only_agent param to
   that registrar method (mirroring register_commands_for_all_agents)
   and threaded it through all 5 reconciliation call sites.

2. presets: `integration use copilot` with --skills (ai_skills: true)
   wrote both the static .agent.md command file AND the SKILL.md
   mirror for the same override. Mirrored the extension path's
   ai_skills guard in both _register_commands and the reconciliation
   pass: a command-backed active agent running in skills mode is
   excluded from non-skill command registration.

3. presets: registered_skills was a flat list, so switching between
   two skill-mode agents (e.g. Claude -> Codex) and then removing the
   preset only restored the currently active agent's directory,
   permanently orphaning the other. _unregister_skills now restores
   every existing skill-mode agent directory instead of only the
   active one.

4. extensions: load_init_options() collapses "no file" and "corrupted
   file" into the same {}, so the round-2 fail-closed fix didn't
   actually distinguish them. Added a shared
   resolve_active_agent_for_registration() helper in _init_options.py
   that checks file existence separately from parse success, returning
   a distinct sentinel for "file absent" vs None for "corrupted or
   invalid". extensions/__init__.py now uses this helper.

5. presets: same corruption-collapsing bug in _register_commands's
   active_agent resolution. Now uses the same shared helper as (4).

Adds regression tests for all five: reconciliation active-only
filtering, copilot --skills dual-write prevention, multi-skill-agent
switch+remove, and corrupted init-options fail-closed behavior for
both extension add and preset add. Each test was verified to fail
against the pre-fix code and pass with the fix.

Targeted (883) and full (3923 passed, 109 skipped) suites pass; ruff
check clean.

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

* fix: address fourth round of review feedback (skill registration provenance)

Replace the "enumerate every skill-mode directory and restore all of them"
approach from the previous round with precise per-agent provenance
tracking, per reviewer feedback that the enumerate-and-restore-everything
design was unsound:

- registered_skills changes from a flat List[str] to Dict[str, List[str]]
  (agent name -> skill names actually written), mirroring the shape
  registered_commands already uses. _register_skills now returns this
  per-agent mapping instead of a bare list, and every call site
  (register_enabled_presets_for_agent, install_from_directory, the
  _reconcile_skills "was this skill previously managed" check) is updated
  to read/merge the new shape. Legacy flat-list registry entries from
  before this change are still readable: writes self-migrate the format,
  and _normalize_registered_skills() handles the transitional read paths.

- _unregister_skills now restores exactly the agent directories recorded
  for a preset instead of guessing at every skill-mode integration that
  happens to exist on disk. This fixes two problems with the old
  enumerate-everything design: (1) it could silently overwrite or delete
  another preset's (or a user's) override in an agent directory the
  current preset never actually touched, and (2) it depended on
  transient per-process integration state (_skills_mode), which is unset
  in a fresh CLI invocation for mode-selectable integrations like Copilot
  --skills, permanently orphaning their overrides after a process
  restart. Registries written before this change (flat list, no agent
  provenance) fall back to best-effort restoration under only the
  currently active agent, matching the pre-existing guarantee level.

- Every directory resolved from persisted provenance is now validated
  through the project's shared symlink/containment guard
  (_ensure_safe_shared_directory) before any file in it is read, written,
  or removed, since restoration may target an agent that isn't currently
  active and its directory can't be assumed safe just because a name was
  recorded for it.

- _tracked_skill_agent_dirs() (the enumeration helper introduced last
  round) is removed; it's superseded by the provenance-based design.

Adds regression tests: a symlinked skills directory is rejected during
removal; removing one preset does not disturb a different preset's
override in another agent's directory; and a Copilot --skills
registration installed, then removed after switching agents in a fresh
PresetManager instance (simulating a new process), is still correctly
restored. Updates existing skill-registration assertions across
test_presets.py and test_integration_claude.py for the new per-agent
registry shape.

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

* fix: address fifth round of review feedback (symlink presence, rescaffold reconciliation, shared skills dir)

- _init_options.py: resolve_active_agent_for_registration() now treats a
  dangling init-options.json symlink as present (path.is_symlink() check
  alongside path.exists()), since Path.exists() follows symlinks and
  returns False for a broken one. Previously a broken symlink fell back
  to the legacy "no file" path and registered every detected agent
  instead of failing closed.
- presets/__init__.py (register_enabled_presets_for_agent): the
  integration use/switch rescaffold path now collects affected command
  names across all presets processed and runs
  _reconcile_composed_commands/_reconcile_skills once after the loop,
  matching install/remove. Previously rescaffolding wrote each preset's
  raw content directly with no follow-up reconciliation, so a
  project-level override (the highest-priority layer) could be clobbered
  by a lower-precedence preset after switching agents.
- presets/__init__.py (_unregister_skills): multiple integrations can
  share one physical skills directory (agy/codex/zed all resolve to
  .agents/skills). Provenance restoration now groups recorded agent
  entries by resolved directory and restores each physical directory
  exactly once, preferring the currently active agent's renderer when it
  owns that directory (otherwise any recorded owner, chosen
  deterministically). Previously each recorded agent key triggered its
  own restore pass against the same directory, with whichever agent was
  iterated last silently winning regardless of which agent was active.

Adds regression tests for each: a dangling init-options.json symlink
failing closed for both preset resolution and extension add; integration
use rescaffold preserving a project override over a lower-priority
preset; and a codex/agy shared-directory removal restoring the directory
exactly once in the active agent's format.

Targeted (tests/integrations/test_integration_subcommand.py,
tests/test_presets.py, tests/test_extensions.py,
tests/test_extension_skills.py,
tests/integrations/test_integration_opencode.py,
tests/integrations/test_integration_claude.py): 930 passed.
Full suite: 3930 passed, 109 skipped.
ruff check: clean on files touched by this change.

Refs #2948

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

* fix: guard skill subdirectories and active-agent scoping in preset reconciliation

Fix 4 issues from round-6 review of the active-only integration
registration work (#2948):

- remove(): removed_cmd_names only collected primary command names from
  registered_commands + manifest aliases, missing commands that were
  only ever registered via skills mode (ai_skills guard returns no
  command names for command-backed integrations in skills mode). This
  skipped reconciliation entirely when removing a higher-priority
  skills-mode preset, causing _unregister_skills() to fall back to
  core/extension content instead of the surviving lower-priority
  preset's override. Now every command template's primary name is
  added to removed_cmd_names unconditionally.

- _reconcile_composed_commands(): the "composed is None" branch (fires
  when no replace-strategy layer remains for a command, e.g. after
  removing a wrap/append preset's base) called unregister_commands()
  across every configured non-skill agent, ignoring only_agent. This
  deleted historical artifacts from integrations that were never active
  for the preset. Now filtered by only_agent like the rest of the file.

- Added _validate_skill_subdir() helper (reusing
  _ensure_safe_shared_directory/_validate_safe_shared_directory from
  shared_infra.py) and applied it at every site that reads or writes an
  individual skill subdirectory (_register_skills,
  _unregister_skills_in_dir, _reconcile_skills' override_skills
  restoration loop). _safe_skills_dir_for_agent only validated the
  parent skills directory; a symlinked leaf subdirectory (e.g.
  .claude/skills/speckit-specify) would slip past that check since
  is_dir()/exists() follow symlinks, letting write_text/rmtree operate
  through it to an arbitrary location outside the project.

Added regression tests: removing a higher-priority skills-only preset
restores the surviving lower-priority preset's content; composed-is-None
unregistration only touches the active agent; symlinked skill subdirectory
rejected on restore; symlinked skill subdirectory rejected on write.

Targeted (934) and full (3934 passed, 109 skipped) test suites and ruff
check pass clean.

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

* fix: persist command registration before fallible skills phase on rescaffold

Fix remaining round-6 review findings on the active-only integration
registration work (#2948):

- register_enabled_presets_for_agent(): registered_commands and
  registered_skills were merged and persisted together in a single
  registry.update() call after both the commands and skills phases ran.
  If _register_skills() raised, the per-preset try/except swallowed it
  before that update() call was reached, even though _register_commands()
  had already written a real command file to disk. That file became
  untracked, so preset removal could no longer clean it up.
  install_from_directory() already persists registered_commands
  immediately after the commands phase, before starting the independently
  fallible skills phase; rescaffold now does the same.

- test_presets.py: renamed a misleading claude_dir variable (pointing at
  Gemini's command directory) in
  test_composed_none_unregister_respects_active_agent to reuse the
  existing gemini_commands_dir variable already defined earlier in the
  same test.

Added regression test
test_rescaffold_persists_commands_before_fallible_skills_phase:
simulates a skills-phase failure during rescaffold and asserts the
command file already written to disk is still tracked in
registered_commands.

Verified all other round-6 findings (preset active-integration scoping,
preset reconciliation/remove paths, skills-mode switching, override
precedence during rescaffold, skill-subdirectory symlink safety) are
already addressed by prior commits in this branch; re-checked each
against current code before concluding no further change was needed.

Targeted (tests/test_presets.py, tests/test_extensions.py: 689 passed)
and full (3935 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: unregister stale opposite-mode preset artifact on same-agent skills toggle

Fix an Important gap in register_enabled_presets_for_agent() surfaced by
quality review (#2948): toggling ai_skills for the *same already-active*
command-backed agent (e.g. `integration upgrade copilot` after flipping
ai_skills, with copilot staying active throughout) left a stale artifact
from the previous mode behind, violating the command/skill mutual-
exclusion invariant this PR otherwise enforces.

- command -> skills: _register_commands()'s ai_skills guard makes the
  commands phase a no-op, but the previously-written command file (e.g.
  .agent.md) and its registered_commands[agent] entry were never cleaned
  up, so it lingered alongside the newly written SKILL.md.
- skills -> command: _get_skills_dir() stops resolving a skills directory
  once ai_skills is off, making the skills phase a no-op, but the
  previously-written SKILL.md and its registered_skills[agent] entry were
  never cleaned up, so it lingered alongside the newly (re)written command
  file.

register_enabled_presets_for_agent() now resolves once per call whether
agent_name is a command-backed integration (extension != "/SKILL.md") and
the current ai_skills state, then narrowly unregisters the stale opposite-
mode entry for that agent via the existing _unregister_commands /
_unregister_skills helpers before persisting updated tracking — mirroring
the same per-agent, per-preset isolation already used elsewhere in this
method. Native skill-only agents (claude, codex, ...) are unaffected:
they have no command/skill toggle, so registered_commands and
registered_skills legitimately co-exist for them by design. The trailing
reconciliation pass, project-override precedence, and per-preset
partial-failure isolation are all unchanged.

Added red-first regression tests exercising the real install +
register_enabled_presets_for_agent rescaffold path in both toggle
directions:
- test_rescaffold_toggle_command_to_skills_removes_stale_command_file
- test_rescaffold_toggle_skills_to_command_removes_stale_skill_file

Both failed against the prior code (stale artifact persisted / registry
still tracked it) and pass after the fix.

Targeted (tests/test_presets.py, tests/test_extensions.py: 691 passed)
and full (3937 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: migrate legacy flat-list registered_skills on rescaffold even when unchanged

Fix a valid finding from GitHub Copilot's review of HEAD b9d9053 (#2948):
register_enabled_presets_for_agent() normalizes a legacy flat-list
registered_skills value (predating per-agent provenance) to the
{agent_name: [...]} dict shape in memory via _normalize_registered_skills,
but the persistence check only compared the two *normalized* forms. When
the freshly rescaffolded skill names are identical to what the legacy
list already held — the common case, since nothing about the preset or
skill actually changed — that comparison is a no-op and registry.update()
is skipped, leaving the *raw* on-disk value as the un-migrated flat list.

A later switch to a different skill-mode agent and removal then follows
_unregister_skills's legacy best-effort path (restore only the currently
active agent's directory) instead of the per-agent provenance path,
permanently orphaning the first agent's override.

Fix: track the raw (pre-normalization) existing value and force
persistence whenever it's a non-empty list, independent of whether the
normalized content changed. Traced registered_commands for the same
class of bug: its registry value has always been Dict[str, List[str]]
(no legacy flat-list format ever existed for it — the existing
`if not isinstance(existing_commands, dict): existing_commands = {}`
guard is not a lossy migration path), so this fix stays scoped to
registered_skills only.

Added red-first regression test
test_rescaffold_migrates_legacy_flat_list_registered_skills: installs a
preset, overwrites its registry entry with a raw legacy flat list,
rescaffolds the *same* active agent with unchanged skill names, and
asserts the raw registry is migrated to per-agent dict form. Extends the
scenario with a switch to a second skill-mode agent and preset removal
to prove both agents' directories restore cleanly instead of orphaning
the first. Failed against the prior code (raw value stayed a list) and
passes after the fix.

Targeted (tests/test_presets.py, tests/test_extensions.py: 692 passed)
and full (3938 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reconcile before fallible skills phase, infer legacy skill provenance, and unregister stale extension artifacts on toggle

Three findings from the Copilot review on HEAD b9d9053/3a1e749:

1. `register_enabled_presets_for_agent()` only recorded a preset's command
   names into `affected_cmd_names` (the set later passed to
   `_reconcile_composed_commands`/`_reconcile_skills`) in the loop that ran
   *after* `_register_skills()`, inside the same per-preset `try` block. If
   `_register_skills` raised, the `except` caught it and `continue`d before
   that loop ever ran — so a preset whose commands phase already wrote real
   content to disk never got reconciled against the full priority stack,
   leaving its raw content in place instead of a project override or
   higher-precedence preset's content. Fix: record the manifest's command
   names immediately after the commands phase succeeds and persists, before
   calling the independently fallible `_register_skills()`.

2. The legacy flat-list `registered_skills` migration (added for the
   previous review round) attributed every name in the list to whichever
   agent was currently being (re)activated. If the first operation after
   upgrading from a pre-#2948 registry was a direct switch to a *different*
   skill-mode agent (e.g. a legacy Claude override, then `integration use
   codex` with no intervening Claude rescaffold), the migrated dict only
   recorded `{"codex": [...]}`, permanently losing Claude's actual
   provenance and orphaning its override on later removal. Fix: added
   `_infer_legacy_skill_provenance()`, which probes every configured
   skill-mode agent's directory (via the same safe, symlink-validated
   helpers already used for restore/removal) for a `SKILL.md` whose
   frontmatter records this exact preset as the owner
   (`metadata.source == "preset:<pack_id>"`). A name found under more than
   one directory is attributed to every matching agent (the preset may have
   been active while the user switched between several skill-mode agents
   before provenance tracking existed); names that can't be matched to any
   directory still fall back to the previously-active best-effort
   behaviour. Directory grouping for shared-path aliases (e.g.
   agy/codex/zed all resolving to `.agents/skills`) intentionally does not
   call `.resolve()` on the path, since doing so diverges from
   `project_root`'s own resolution state on platforms where a path
   component is itself a symlink (e.g. macOS's `/var` -> `/private/var`)
   and made every subsequent containment check spuriously fail.

3. `register_enabled_extensions_for_agent()` has the same command/skill
   mutual-exclusion gap the preset path had (fixed in a previous round):
   toggling `ai_skills` for the *same active* agent left the opposite
   mode's artifact behind. Command -> skills left the extension's
   `.agent.md` file and its `registered_commands[agent]` entry in place
   once `skills_mode_active` made the commands phase a no-op. Skills ->
   command left the extension's `SKILL.md` file in place, since an empty
   `_register_extension_skills()` result (because this agent's skills
   directory no longer resolves once `ai_skills` is off) was treated as
   "nothing to register" rather than "this was rendered here before and is
   now stale". This diverges from the preset path in one respect:
   `registered_skills` for extensions has always been a flat list with no
   per-agent provenance (extension skills are only ever rendered for the
   active agent, never per-preset-per-agent tracked), so the fix resolves
   ownership by checking which of the extension's tracked skill names
   still exist as directories under this specific agent's directory before
   removing them — mirroring the same technique `unregister_agent_artifacts`
   already uses for full agent deactivation, but scoped narrowly to firing
   only when a toggle is actually detected (`skills_mode_active` /
   `command_mode_active`), so a same-mode re-run never disturbs
   already-correct artifacts or a user's manual customizations.

Regression tests (all confirmed red before their respective fix, green
after):
- tests/test_presets.py::TestPresetSkills::test_rescaffold_reconciles_override_even_when_skills_phase_fails
- tests/test_presets.py::TestPresetSkills::test_rescaffold_legacy_flat_list_direct_switch_preserves_original_agent
- tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_command_to_skills_removes_stale_extension_command_file
- tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_skills_to_command_removes_stale_extension_skill_file

Verification: tests/test_presets.py + tests/test_extensions.py +
tests/test_extension_skills.py (753 passed), tests/integrations/ (1768
passed, 1 skipped), full suite `pytest tests -q` (3942 passed, 109
skipped), `ruff check` on changed files clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: broaden legacy skill provenance inference to command-backed agents

_infer_legacy_skill_provenance() only probed agents whose registrar
config statically declares extension == "/SKILL.md", excluding
command-backed agents (e.g. Copilot) that can also render preset
overrides as SKILL.md files when ai_skills is enabled. A real
preset-owned .github/skills/.../SKILL.md written while Copilot was the
active skills-mode agent was therefore never probed and got
misattributed entirely to whichever agent activated first after the
upgrade, permanently orphaning Copilot's override on later removal.

Broaden the candidate set to every configured integration
(CommandRegistrar.AGENT_CONFIGS), reusing the existing safe-path
helper (_safe_skills_dir_for_agent, itself built on the shared
_get_skills_dir resolver) rather than inventing new path-construction
logic. The existing preset-marker match (metadata.source ==
"preset:<pack_id>") continues to gate every attribution, so
command-mode agents that never rendered this preset's skill are not
falsely attributed.

Add red-first regression tests: a legacy flat-list entry owned by
Copilot in skills mode, switched directly to Claude with no
intervening Copilot rescaffold, now migrates to a per-agent dict
covering both agents, and removal restores both agents' files instead
of orphaning Copilot's override; plus a negative-case test confirming
a command-mode Copilot with no preset-owned skill marker is not
falsely attributed during the same migration.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: preserve extension skill tracking for mirrors in other agent dirs

The skills -> command toggle cleanup in
register_enabled_extensions_for_agent() recomputed the remaining
tracked registered_skills names by checking only the toggling agent's
own skills directory. Since registered_skills is a single flat list
shared across every agent an extension has ever been activated under
(skills are only ever rendered for the active agent, so there is no
per-agent registry key), a name whose mirror still existed under a
*different*, previously-active agent's directory was incorrectly
dropped from tracking as soon as the current agent's own copy was
removed. A later full removal only iterates registered_skills, so the
orphaned mirror under the other agent's directory was never found or
cleaned up.

Add _extension_owned_skill_names(), which re-verifies ownership across
every configured agent's skills directory (deduped by shared path) the
same way the existing _unregister_extension_skills() fallback scan
already does, keeping a name only when a SKILL.md with a matching
metadata.source == "extension:<id>" marker is found somewhere -
read-only, no directory creation, no symlink escape. Use it instead of
re-checking only the toggling agent's own directory when recomputing
what remains tracked after narrow stale-mirror cleanup.

Add a red-first regression test: Auggie is activated in skills mode
first (writing a mirror), then Copilot is activated in skills mode
(writing its own mirror for the same names), then Copilot toggles to
command mode. Before the fix, registered_skills lost both names
entirely even though Auggie's mirrors were untouched on disk; after
the fix tracking is preserved and a subsequent full removal correctly
cleans up Auggie's remaining mirrors too.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reject symlinked skills-directory escape in extension skill scans

_extension_owned_skill_names() and the fast/fallback paths of its
sibling _unregister_extension_skills() called skills_candidate.resolve()
and then checked children relative to that already-resolved candidate.
If the candidate directory itself (e.g. .gemini/skills) was a symlink
pointing outside the project root, both the resolve() call and the
subsequent containment check silently passed through the symlink
instead of rejecting it:

- _extension_owned_skill_names() would falsely attribute ownership to
  a marker-matching SKILL.md living outside the project.
- _unregister_extension_skills()'s fast path (an explicit skills_dir,
  as passed by the toggle-cleanup call site) and its fallback scan
  (used during full extension removal) would both shutil.rmtree() the
  external directory, deleting unrelated content outside the project.

Fix by validating the candidate directory itself with the existing
_validate_safe_shared_directory() shared-infra helper before any probe
or delete: it rejects a symlink at any path component (walking down
from the project root, including the final component) without ever
resolving through it, and is already used elsewhere in the codebase for
the same class of shared-directory containment check. Unsafe
candidates are skipped/refused rather than followed.

Add red-first security regression tests reproducing each of the three
call sites with a `.gemini/skills` symlink pointing at an external
directory containing a marker-matching SKILL.md and an unrelated
precious_file.txt: provenance inference must not attribute the name,
and both the explicit-skills_dir fast path and the None-skills_dir
fallback scan must leave the external directory and file untouched.
Existing valid shared/deduped directory tests (e.g. agy/amp/codex/zed
sharing .agents/skills) continue to pass, confirming legitimate shared
directories still clean up correctly.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix unscoped extension-skill removal and legacy preset provenance on direct remove

- _unregister_extension_skills(): omitting skills_dir now always triggers
  the full multi-directory fallback scan instead of narrowing to the
  currently active agent's directory. Previously, remove() (the only
  caller that omits skills_dir) would resolve the active agent's dir and
  take the scoped fast path, orphaning a previously-active second agent's
  extension skill mirror during full removal.

- PresetManager.remove(): infer legacy flat-list registered_skills
  provenance (reusing _infer_legacy_skill_provenance from the prior
  rescaffold fix) before invoking _unregister_skills, so a direct
  `preset remove` with no intervening rescaffold/switch also restores
  every previously-active agent's directory instead of only the
  currently active one.

Added regression tests:
- test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror
- test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Keep unregister_agent_artifacts scoped to its agent when directory is absent

ExtensionManager.unregister_agent_artifacts() converted its resolved
agent_skills_dir to None whenever that directory didn't exist, before
calling _unregister_extension_skills(). After 1d8f9e3, omitting
skills_dir means "genuinely unscoped removal": scan every configured
agent's directory, reserved for ExtensionManager.remove()'s full
project cleanup. Since unregister_agent_artifacts is agent-scoped (used
by switch to clean up the previous integration's artifacts), this
caused it to delete every other agent's live extension skill mirrors
whenever the target agent's own directory happened to be absent, e.g.
unregistering an agent that was never activated.

Fix: always pass the explicit, agent-scoped skills_dir, even when it
doesn't exist on disk, so the fast path is a safe no-op for an absent
directory instead of falling back to the all-agents scan. Registry
reconciliation (dropping removed names from the flat registered_skills
list) now only runs when the agent's directory actually exists, so an
absent directory can't be misread as "these names were removed
everywhere" and wipe tracking for mirrors that still legitimately live
under other agents' directories.

Added regression test:
- test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Preserve global skill tracking across agents in unregister_agent_artifacts

The present-directory branch of ExtensionManager.unregister_agent_artifacts()
recomputed "remaining" registered_skills only by checking whether each name
still existed under the just-cleaned agent's own directory. registered_skills
is a single flat list shared across every agent an extension was ever
activated under (skills are only ever rendered for the currently active
agent, so there's no per-agent registry key). Repro: auggie and copilot both
have mirrors for the same extension; unregister_agent_artifacts("auggie")
correctly removes auggie's own mirror, sees the names absent from auggie's
(now empty) directory, and stores an empty registered_skills list - even
though copilot's mirror is still live on disk and now untracked. A later full
remove() then reads an empty registry and leaves copilot's mirror orphaned.

Fix: after the agent-scoped cleanup, recompute remaining names with
_extension_owned_skill_names(), which scans every safe, configured agent
skills directory (not just the one just cleaned) and keeps a name only if a
marker-verified SKILL.md for this extension still exists somewhere. This is
the same helper already used for the analogous same-agent toggle-cleanup
case, so no new abstraction was introduced. Explicit per-agent cleanup,
marker ownership verification, and symlink/containment safety are unchanged.

Added regression test:
- test_unregister_agent_artifacts_preserves_tracking_for_other_agent_mirror

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Reconcile every historical agent on preset removal; validate child skill dirs

Fixes 3 findings from the Copilot review on HEAD 31c9b97 (#2948):

1. presets/__init__.py: remove()'s command reconciliation only recreated
   the surviving preset's content for the currently active agent, even
   though the removed preset's registered_commands could span multiple
   historical (now-inactive) agents recorded via prior rescaffolds. Now
   remove() captures every historical agent registered_commands actually
   targeted (before mutation) and passes it as extra_agents through
   _reconcile_composed_commands -> _register_for_non_skill_agents /
   _register_command_from_path -> registrar.register_commands_for_non_
   skill_agents, so the active-only restriction for install/use is
   preserved while post-removal reconciliation restores every touched
   directory.

2. presets/__init__.py: the analogous gap existed for skills. _unregister_
   skills() now returns {skills_dir: renderer_agent} for every directory it
   actually restored, and _reconcile_skills() accepts extra_skills_dirs to
   reconcile each of those directories (via a new apply_to_dir() helper),
   not only the currently active skills directory. _register_skills() gained
   optional target_dir/target_agent overrides (forcing
   create_missing_skills off for non-active directories) so a historical
   directory is only ever restored, never seeded with brand-new skills.

3. extensions/__init__.py: _extension_owned_skill_names() and both the
   fast and fallback paths of _unregister_extension_skills() validated only
   the parent skills_dir for symlink escape, then resolved
   skills_dir / skill_name and checked containment relative to that
   already-resolved parent. A per-skill child that is itself a symlink to
   a different, legitimate skill directory within the same (safe) root
   passed that containment check, so deleting/attributing through the
   symlink name could destroy or misattribute an unrelated skill reached
   only via the alias. All three call sites now run the shared
   _validate_safe_shared_directory() component-wise check against the full
   skills_dir / skill_name path (not just the parent) before any read or
   delete, rejecting a symlinked child outright rather than following it,
   even when its resolved target remains in-bounds.

Regression tests added (all confirmed red against pre-fix code, green
after):
- test_remove_reconciles_command_for_every_historical_agent
- test_remove_reconciles_skill_for_every_historical_agent
- test_extension_owned_skill_names_rejects_symlinked_child_skill_dir
- test_unregister_extension_skills_explicit_dir_rejects_symlinked_child
- test_unregister_extension_skills_fallback_rejects_symlinked_child

Tests: tests/test_presets.py (361), tests/test_extension_skills.py (69),
tests/test_extensions.py (338) all pass; tests/integrations (1768 passed,
1 skipped) pass; full suite 3902 passed / 74 skipped (90 pre-existing,
environment-only git-signing tests deselected — confirmed failing
identically on the pre-change baseline due to local 1Password SSH-agent
signing, unrelated to this change). ruff check clean on all changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Persist historical reconciliation ownership; defer destructive toggle cleanup; validate registry-provided skill names

Round 11 review findings (5 comments on HEAD ab6c28c), three root causes:

A) Historical-agent reconciliation wrote surviving content to disk but
   discarded the returned per-agent write map, so the preset's own
   registered_commands/registered_skills never learned about directories
   reconciliation restored on its behalf. A later removal of that same
   preset then orphaned those directories. Added
   _merge_pack_registered_commands/_merge_pack_registered_skills and wired
   them into _reconcile_composed_commands and _reconcile_skills's
   apply_to_dir so every actual write is merged back into the winning
   preset's registry metadata.

B) Command<->skills toggle on an already-active agent deleted the old
   artifact before the replacement registration ran, in both
   presets/__init__.py's register_enabled_presets_for_agent and
   extensions/__init__.py's register_enabled_extensions_for_agent. If the
   replacement step raised, both artifacts were lost. Deferred the
   destructive cleanup until after the replacement phase completes
   without raising (register-new-then-remove-old ordering); the mirror
   skills->command direction was already safe since the new command file
   is always registered unconditionally before any cleanup runs.

C) _unregister_skills_in_dir and _infer_legacy_skill_provenance joined a
   registry-provided (untrusted) skill name directly onto a directory
   before any name-shape validation. An absolute in-project name discards
   the intended parent directory entirely (Path's "/" operator drops the
   left side for an absolute right side), letting a corrupted registry
   entry escape the intended skills subtree while still resolving inside
   the project root - passing the existing containment/symlink check.
   Added a centralized _is_safe_registry_skill_name guard (rejecting
   non-strings, empty strings, absolute paths, multi-component paths, and
   "."/".." ) and applied it before every path join derived from
   registry-provided skill names in both functions. Also fixed
   _infer_legacy_skill_provenance's unmatched-name fallback, which
   previously still attributed rejected names to fallback_agent even
   after the loop skipped them.

Added red-first regressions for all three root causes, covering: a
two-preset historical-command-agent survivor scenario, an analogous
skill-agent survivor scenario, injected skills-phase failure during a
preset command->skills toggle and the extension equivalent, a direct
unit test of the new name-safety guard, an absolute-path escape attempt
against _unregister_skills_in_dir, and a false-attribution attempt
against _infer_legacy_skill_provenance.

Tests: tests/test_presets.py (367 passed), tests/test_extension_skills.py
+ tests/test_extensions.py (408 passed), tests/integrations (1768
passed, 1 skipped), full suite tests -q deselecting the pre-existing
1Password-signing-affected tests/extensions/git/test_git_extension.py
(3909 passed, 74 skipped, 90 deselected). ruff check clean on all
changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Verify replacement actually landed before retiring stale toggle artifacts

The command<->skills toggle cleanup added for #2948 deferred destructive
removal of the old-mode artifact until after the replacement registration
call completed without raising. That was necessary but not sufficient:
none of _register_skills(), _register_commands(),
register_commands_for_agent(), or _register_extension_skills() raise on
a missing source template, a safety-validation skip, or a corrupted
manifest entry — they simply return an empty or partial result. Treating
"did not raise" as "fully replaced" meant a stale artifact could still be
deleted (or its tracking dropped) even though its specific replacement
never actually landed, leaving neither artifact in place for that logical
command/skill.

Fix all four affected toggle directions by checking the replacement
call's actual return value before allowing any destructive step:

- presets command->skills (register_enabled_presets_for_agent): only
  unregister a stale command name once its corresponding skill name
  (via the existing _skill_names_for_command() helper) is confirmed
  present in the skills call's returned names for that agent; the
  remainder stays tracked and on disk.
- presets skills->command (register_enabled_presets_for_agent): only
  unregister a stale skill name once its corresponding command name is
  confirmed present in the commands call's returned names for that
  agent, using the same helper.
- extensions skills->command (register_enabled_extensions_for_agent):
  only remove a skill mirror once the matching command (mapped via the
  existing HookExecutor._skill_name_from_command() helper) is confirmed
  present in register_commands_for_agent's returned names.
- extensions command->skills (register_enabled_extensions_for_agent):
  only remove a deferred stale command once its matching skill name is
  confirmed present in _register_extension_skills()'s returned names.

All four reuse the existing command<->skill name-derivation helpers
rather than inventing new mapping logic. Registry tracking is updated to
retain exactly the unreplaced subset rather than being popped wholesale,
so partially-successful toggles leave correct, minimal tracking behind.

Added 8 new regression tests (4 presets, 4 extensions) covering both the
fully-empty and genuinely-partial result cases for each of the four
toggle directions, using real missing-source-file scenarios (not mocked
return values) to exercise the actual code paths. Confirmed red before
the fix and green after for all 8.

Focused (test_presets.py, test_extension_skills.py, test_extensions.py,
tests/integrations): 2551 passed, 1 skipped.
Full suite (tests, excluding the pre-existing environment-local
1Password-signing git-extension failures): 3917 passed, 74 skipped, 90
deselected.
ruff check: clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Retire alias command groups on toggle; scope preset cleanup to switched-away agent (#2948)

Fixes three current Copilot review findings on HEAD d0d152e:

1. Command->skills toggle cleanup only matched a stale command's own
   name against the returned replacement skill name. Aliases
   (CommandRegistrar tracks and returns primary + alias names flattened
   into one list) never have their own skill rendered -- only the
   primary command's skill is rendered -- so an alias's name could never
   match, leaving its command artifact and tracking behind forever even
   after the primary's replacement landed. Fixed identically in both
   presets (register_enabled_presets_for_agent) and extensions
   (register_enabled_extensions_for_agent): build a primary->alias
   mapping from the manifest, group stale names by primary, and
   retire/keep the whole group together based solely on whether the
   primary's skill replacement actually landed.

2. `integration switch` to a not-yet-installed target unregistered the
   old agent's extension artifacts but had no preset equivalent, so a
   preset's command overrides (including custom preset commands) and
   skill mirrors for the deactivated agent lingered as orphans. Added
   `PresetManager.unregister_agent_artifacts()`, mirroring
   `ExtensionManager.unregister_agent_artifacts()`: scoped strictly to
   the given agent, migrates a legacy flat-list `registered_skills`
   entry via existing on-disk provenance inference before removing
   anything (so other agents' real ownership is preserved rather than
   guessed or dropped), and guards against double-processing an
   artifact through both the commands and skills paths for native
   SKILL.md agents. Wired via a new `_unregister_presets_for_agent()`
   helper into the integration switch command's existing old-agent
   cleanup phase.

Added red-first regression tests:
- tests/test_presets.py: alias-group retire/keep/partial-multi-group
  tests for the command->skills toggle; unregister_agent_artifacts
  scoping tests for commands and legacy-list skill provenance.
- tests/test_extension_skills.py: alias-group retire/keep tests for the
  extension command->skills toggle.
- tests/integrations/test_integration_subcommand.py: end-to-end switch
  test proving a preset's custom command override is cleaned up when
  switching to a not-yet-installed integration, with tracking updated
  correctly and the new agent's registration unaffected.

All new tests confirmed red (AttributeError / orphaned file assertions)
before the fix and green after. Full suite: 3980 passed, 109 skipped.
ruff check clean on all changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: track reconciled extension artifacts

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

* Fix native skill preset reconciliation

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

* Fix shared native skill cleanup

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

* Fix partial preset rescaffold tracking

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

* Fix preset agent skill lifecycle

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

* Clarify preset removal reconciliation

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

* fix(integrations): address upgrade review feedback

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

* fix(presets): reconcile partial command writes

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

* fix: address active artifact cleanup review

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

* fix: defer preset skill cleanup to winning command

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

* fix: track reconciled and partial preset skills

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

* fix: reconcile project overrides to legacy skills

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

* fix: harden preset skill writes and rollback

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

* fix(presets): harden legacy skill restoration

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

* fix(presets): preserve non-owned legacy skills

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

* fix: validate reconciled skill paths

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

* fix(presets): preserve reconciled skill ownership

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

* fix(presets): clean reconciled agent skills

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

* fix: keep legacy cleanup project-local

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

* fix(presets): keep active agent's artifacts in its current mode on remove

A partially failed command<->skills toggle leaves stale tracking
(registered_commands or registered_skills) for the active agent, and
remove() replayed that history regardless of the agent's current mode:

- extra_agents re-admitted the active skills-mode agent into command
  reconciliation, recreating its command file from a surviving lower
  preset even though only_agent excluded it.
- _unregister_skills restored (and _reconcile_skills reapplied) a skill
  artifact for the active command-mode agent instead of deleting the
  preset-owned leftover.

The active agent's participation is now decided exclusively by its
current mode: reconciliation strips it from extra_agents, and removal
routes its stale skills through _delete_agent_preset_skills. Historical
replay still applies to inactive agents only (#2948).

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

* fix: filter uninstalled-extension commands in reconciliation; allow active-agent layout change with presets

Two follow-ups to the upstream-main merge:

- Preset reconciliation (_reconcile_composed_commands) now skips
  extension-scoped commands (speckit.<ext>.<cmd>) whose extension is not
  installed, at the single chokepoint every install/remove/rescaffold
  pass funnels through. Registration already refused them, so
  reconciliation could materialize files no registry entry tracks. The
  duplicated per-call-site filters collapse into one
  _extension_installed_for_command helper.

- The #3415 layout-change guard predates this PR's agent-scoped preset
  rescaffold: for the active integration, _register_presets_for_agent
  now re-registers enabled presets in the new layout and retires the
  old layout's stale files, so an active-agent command<->skills toggle
  proceeds and reconciles instead of being rejected. The guard still
  rejects non-active agents (no rescaffold runs for them) and still
  fails closed on an unreadable registry.
  _installed_presets_affecting_agent also understands the per-agent
  dict shape of registered_skills this PR writes, instead of raising
  'malformed'.

Regression tests: rescaffold with an uninstalled extension's command,
CLI-level legacy<->skills toggle with an installed preset (both
directions), secondary-agent rejection, and dict-shaped
registered_skills in the guard helper.

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

* fix: reject active layout change while a disabled preset owns artifacts

The post-upgrade preset rescaffold iterates enabled presets only, and a
disabled preset's artifacts are deliberately frozen until removal, so an
active-agent command<->skills layout change cannot reconcile them.
_installed_presets_affecting_agent now reports each preset's enabled
state and the guard rejects the migration while any affected preset is
disabled, with re-enable/remove guidance. Enabled presets and non-active
rejection behave as before.

Regression test: disabled preset blocks the toggle untouched; re-enabling
unblocks it and reconciles.

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

* docs: replace placeholder prefix in two safety comments

Comment-only: spell out why skill deletion is restricted to
project-local directories (flat/legacy provenance cannot prove
home-directory ownership) instead of an undefined placeholder word.

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

* docs: correct guard-helper docstring to active-only registration model

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

* fix: fail closed on non-list values in per-agent preset provenance

A dict-shaped registered_skills/registered_commands entry with a
non-list value (e.g. null) left ownership undecidable but read as "no
artifacts", letting a layout-changing upgrade proceed on a malformed
registry. Validate values are lists and raise
_PresetRegistryUnreadableError otherwise, matching the guard's
fail-closed contract. Unit test covers both fields.

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

* fix: drop eager extension unregister on layout-changing upgrade

Unregistering the agent's extension artifacts before re-registration
deleted files and registry tracking up front, so a failed or partial
re-registration left the extension with no artifacts at all. Retirement
of each opposite-mode artifact already belongs to
register_enabled_extensions_for_agent's deferred toggle cleanup, which
removes an old artifact only after its replacement is confirmed. Also
keeps disabled extensions consistent with disabled presets: artifacts
stay frozen in place with intact tracking.

Regression test corrupts the installed extension manifest so
re-registration fails, then asserts the old-layout artifacts and their
registry tracking survive the upgrade.

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

* fix: rescaffold fallback integration after failed switch rollback

When Phase 2 of a switch fails, rollback restores another installed
integration as the default via _set_default_integration but never
re-registered extensions or presets for it. Under active-only
registration the fallback may never have received any artifacts (it
was installed while another integration was active), and Phase 1
already unregistered the outgoing agent's artifacts — leaving the
restored default unusable. Rescaffold both extensions and presets
(best-effort) after the fallback default is successfully restored.

Regression test: secondary codex install with the git extension, a
failing switch to generic, then asserts codex ends up with registered
extension artifacts after rollback.

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

* docs: explain load-bearing pre-create loop in _reconcile_skills

The per-skill _validate_skill_subdir(create=True) loop looks like dead
code (its result is unused), but it re-creates the tracked skill
subdirectories that _unregister_skills just deleted so
_register_skills's only-overwrite-existing gate passes during a
historical-directory restore. Removing it fails
test_skill_reconciliation_preserves_per_directory_names.

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

* fix: preserve dashed-description skill tracking

Use the shared frontmatter parser when verifying surviving extension skill mirrors so delimiter substrings cannot hide provenance metadata.

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

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

* fix: skip absent extension skills during reconciliation

Filter extension-scoped commands before skill reconciliation so historical preset tracking and project overrides cannot recreate artifacts for uninstalled extensions.

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

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

* fix: preserve partial native skill cleanup

Coordinate native-skill command cleanup with registered skill coverage per agent and command so partial rescaffolds cannot orphan preset artifacts.

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

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-07-27 14:09:40 -05:00
WOLIKIMCHENG ae82c74339 fix(kilocode): install commands under .kilo/commands (#3672) 
* fix(kilocode): write commands to .kilo/commands

* fix: guard Kilo legacy command migration

---------

Co-authored-by: root <kinsonnee@gmail.com>
 2026-07-24 11:05:32 -05:00
Manfred Riem 781a14a6d2 docs: clarify shell-step interpolation safety (#3719) 
* docs: clarify shell-step interpolation safety

Shell step `run` fields are executed by the system shell and `{{ ... }}`
expressions are substituted as raw, unquoted text. Document that untrusted
sources — workflow `inputs.*` and prior-step output, including AI-generated
`prompt` output — must be quoted, enum-constrained, validated, or gated before
they reach a `run` field.

- docs/reference/workflows.md: add an "Interpolation and shell safety" section.
- workflows/README.md: add a warning under the Shell Steps example, link to the
  new section, and quote the `inputs.project_dir` example.
- workflows/PUBLISHING.md: strengthen the interpolation guidance and call out
  prior-step/agent output as untrusted.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: correct shell-step interpolation guidance

Address review feedback that the previous wording over-promised. Clarify that
none of the mitigations neutralise a hostile interpolated value:

- Quoting is not a security boundary — there is no shell-escaping filter, and a
  value containing the matching quote can break out. Present quoting as
  correctness handling for already-constrained values only.
- Remove the "pass data via environment or files" guidance: ShellStep has no
  `env` mapping (it only copies the process environment and sets
  SPECKIT_WORKFLOW_DIR), so that transport does not exist.
- Drop the claim that routing through a command/prompt step validates or safely
  binds a value; it does not.
- Correct the gate guidance: a gate renders only its own message/show_file and
  does not inspect, resolve, or sanitise the following step. Authors must
  surface the exact command/data in the gate themselves, and approval does not
  neutralise an injectable interpolation.

Frame constraining values at the source (enum/allowlist) as the only reliable
control, and keeping unconstrained values out of `run` fields entirely.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: remove unsafe interpolation from example and gate guidance

Address further review feedback:

- workflows/README.md: the shell example interpolated an unconstrained path
  into shell source, which contradicted the warning beneath it. Shell steps
  already run from the project root, so drop the `cd '{{ inputs.project_dir }}'`
  prefix and model a plain `run: "npm test"` with no interpolation.
- docs/reference/workflows.md: GateStep prints `message` verbatim with no
  control-character stripping (stripping applies only to `show_file` path and
  contents), so recommending that authors surface untrusted data in `message`
  was itself unsafe — agent/caller output could inject terminal escapes to
  alter or hide the prompt. Direct authors to keep `message` to trusted text
  and surface untrusted material via `show_file`, whose contents are stripped.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: use correct prompt-step output key in example

A `prompt` step stores agent-generated text under `output.stdout`, not
`output.value`, so the example expression `{{ steps.plan.output.value }}`
would resolve to None. Reference `output.stdout` so the example correctly
demonstrates untrusted agent output flowing into a shell step.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835
 2026-07-24 10:40:26 -05:00
Ali jawwad 391cc0dff8 fix(integrations): declare PiIntegration multi_install_safe (#3652) 
* fix(integrations): declare PiIntegration multi_install_safe

PiIntegration writes only to its isolated, static root .pi/prompts,
disjoint from every other integration, yet never declared
multi_install_safe — so it inherited the IntegrationBase default False,
leaving `specify integration status` in a permanent unsafe-multi-install
ERROR state when pi is co-installed alongside another agent.

Add `multi_install_safe = True`, mirroring the isolated MarkdownIntegration
cohort (qwen, shai, qodercli) and the kiro-cli #3471 fix. The parametrized
registry isolation contracts auto-include pi and pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(integrations): list pi in the multi-install-safe reference table

Declaring PiIntegration multi_install_safe means the reference table in
docs/reference/integrations.md (which states it lists all currently
declared multi-install-safe integrations) should include it. Add the
alphabetized pi row with its .pi/prompts isolation path.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-07-24 09:48:30 -05:00
Ali jawwad e9dfe900f6 fix(integrations): declare OmpIntegration multi_install_safe (#3650) 
* fix(integrations): declare OmpIntegration multi_install_safe

OmpIntegration is a plain MarkdownIntegration whose files live only under
its isolated, static root .omp/commands/, disjoint from every other
integration. But it never declared multi_install_safe, so it inherited the
IntegrationBase default False — leaving `specify integration status` in a
permanent unsafe-multi-install ERROR state whenever omp is co-installed
alongside another agent, with no acknowledgment path.

Add `multi_install_safe = True`, mirroring the isolated MarkdownIntegration
cohort (qwen, shai, qodercli, junie, kilocode) and the kiro-cli #3471 fix.
The parametrized registry isolation contracts auto-include omp once the flag
is set and pass (.omp/commands is isolated and its manifest disjoint).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(integrations): list omp in the multi-install-safe reference table

Declaring OmpIntegration multi_install_safe means the reference table in
docs/reference/integrations.md (which states it lists all currently
declared multi-install-safe integrations) should include it. Add the
alphabetized omp row with its .omp/commands isolation path.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-07-23 11:07:40 -05:00
Quratulain-bilal e4cfa4c19c fix(integrations): declare kiro-cli multi-install safe (#3477) 
* tidy kiro-cli multi-install-safe declaration + add docs row and test

kiro-cli is declared multi_install_safe (it uses a fully isolated .kiro/
root, .kiro/prompts command dir, a stable '.' separator, and a dedicated
manifest — #3471). main ended up with the flag assigned twice in the class
body; this drops the bare duplicate and keeps a single declaration with the
comment explaining why it's safe.

also adds the missing kiro-cli row to the multi-install-safe table in
docs/reference/integrations.md, and a test asserting the flag is set (the
registry contract tests already enforce the actual path isolation against
every other safe integration).

* address review: drop agent-file column and duplicate test

per maintainer guidance on #3477: the Isolation table listed each
integration's agent-context file (AGENTS.md, CLAUDE.md, etc.) alongside
its command dir, which contradicted the safety definition (Copilot flagged
kiro-cli/codex both mapping to AGENTS.md). those context files are owned by
the optional agent-context extension, not by multi_install_safe — the flag
governs only the command directory + manifest.

- renamed the column to 'Command directory' and removed the agent-file
  entries, so it lists only what each integration actually manages
- reworded the definition to note agent-context is a separate concern and
  is not multi-install safe
- removed the duplicate test_declared_multi_install_safe (the existing
  test_declares_multi_install_safe already asserts the same thing)

* address review: keep agent-root requirement; reframe agent-context targeting

- Restore 'static, unique agent root' alongside command directory in the
  multi-install-safe definition — base.py and test_registry.py
  (test_safe_integrations_have_distinct_agent_roots) enforce both.
- Reframe the agent-context note: multi_install_safe is an integration-level
  declaration about command/skill paths, so describe context-file targeting
  as independent of it rather than calling the extension 'not multi-install
  safe'. The extension can even sync multiple anchors via context_files.

* address review: hoist multi_install_safe to top of class

Move the multi_install_safe = True declaration to the top of
KiroCliIntegration (right after key) so it is visible at the exact spot
the diff touched. The flag was never actually removed — it was declared
once further down the class — but placing it at the top makes the opt-in
unmistakable in review and keeps the single declaration. Verified the
integration still resolves multi_install_safe is True; 24 kiro-cli tests
pass.
 2026-07-23 07:41:07 -05:00
Manfred Riem a5560fcf13 docs(scripts): document the 'py' script type and sh/ps migration plan (#3284) (#3653) 
* docs(scripts): document the 'py' script type and sh/ps migration plan (#3284)

Bring remaining docs up to date with the Python (`py`) workflow-script
variant introduced in #3277, and record the retention/deprecation plan
for the shell variants.

- AGENTS.md: document the `scripts:` frontmatter (sh/ps/py), clarify the
  `{SCRIPT}` placeholder resolution, and add a "Script Types and
  Migration" section (why py is recommended, defaults, phased sh/ps
  deprecation path). Note the Python agent-context variant.
- docs/quickstart.md, docs/local-development.md: mention the `py` variant
  and `--script sh|ps|py`.
- docs/reference/integrations.md: add `py` to the `--script` rows for
  install/switch/upgrade.
- .devcontainer/devcontainer.json: auto-approve `.specify/scripts/python/`.

Closes #3284.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 298d6ec2-a330-49bc-9394-fe2b77f25ff3

* docs(scripts): address review — accurate paths, prompt behavior, scoped py claims

Addresses the review on PR #3653:

- AGENTS.md: fix the agent-context Python path to its real
  `extensions/agent-context/scripts/python/` location.
- AGENTS.md: qualify that only templates that invoke a helper script
  carry `scripts:` frontmatter (constitution/specify do not).
- AGENTS.md: narrow the availability claim — `py` covers the core
  command templates; the bundled extensions ship Python scripts on disk
  but their command templates still invoke shell variants, so `--script
  py` does not yet route extension commands to Python.
- AGENTS.md / quickstart / local-development: describe the interactive
  prompt vs. non-interactive OS default instead of "auto-selects".
- local-development: add `--script py` to the wrong-script-type fix.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 298d6ec2-a330-49bc-9394-fe2b77f25ff3

* docs(scripts): trim deprecation timeline, correct parity/scope claims (review)

Addresses the second review on PR #3653:

- Remove the speculative four-phase deprecation timeline from AGENTS.md.
  A forward-looking removal schedule is roadmap content, not contributor
  guidance, and its phases lacked an actionable adoption signal. Replace
  it with the concrete contributor parity rule plus a one-line
  current-posture note pointing removal work to the #3277 epic.
- Stop stating dual-maintenance as already eliminated: reframe "single
  source of truth" as the intended direction, noting all three variants
  are still maintained in parallel today.
- Correct the parity-coverage claim: Python ports have output-parity
  tests where the contract is stdout-based and unit tests elsewhere,
  rather than every file being compared to every shell counterpart.
- Scope the `scripts:` frontmatter rule to core command templates and
  note the agent-context/git extension templates don't use it yet.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 298d6ec2-a330-49bc-9394-fe2b77f25ff3

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-07-22 11:46:11 -05:00
Nurfitra Pujo Santiko 470ac5b6e6 feat: add Factory Droid CLI integration (#822) (#3587) 
Adds a skills-based integration for the Factory Droid CLI alongside the
existing Claude/Codex skills agents. The integration scaffolds
`.factory/skills/speckit-*` directories and documents the install step
in the devcontainer post-create script via the official npm
distribution (`npm install -g droid`), which matches the layout of every
other CLI install block above and avoids executing an unverified remote
shell installer. The integration is also added to the user-facing
supported-agent table in `docs/reference/integrations.md` so the new key
is discoverable from the published documentation, as required by the
"Updating this documentation" guideline in AGENTS.md.

Operator-supplied extra args via `SPECKIT_INTEGRATION_DROID_EXTRA_ARGS`
are appended after the canonical Spec Kit flags so the canonical flags
are always present in argv. The Factory Droid CLI parser uses
last-wins duplicate-flag semantics (verified empirically against
droid 0.175.0), so a later operator-supplied value may override the
canonical one — this is a deliberate inversion of the cursor-agent /
opencode / codex ordering.

Includes:
- `src/specify_cli/integrations/droid/__init__.py` (subpackage)
- `tests/integrations/test_integration_droid.py` (46 tests, including
  regression coverage for the no-trailing-newline frontmatter fusion
  bug, idempotent skill injection, and env-var path resolution)
- `integrations/catalog.json` entry + `updated_at` bump
- Alphabetical registration in `src/specify_cli/integrations/__init__.py`
  and `tests/integrations/test_registry.py`
- Devcontainer Droid install block via the npm distribution
  (`npm install -g droid`), replacing the earlier curl-based installer
- User-facing supported-agent table row in
  `docs/reference/integrations.md` (key `droid`, `.factory/skills/`
  layout, `/speckit-<command>` invocation)
- `AGENT_CONFIG` entry and matching alphabetical entries in
  `tests/test_agent_config_consistency.py` (`ISSUE_TEMPLATE_AGENT_KEYS`)
  and the three issue-template dropdowns (`agent_request.yml`,
  `bug_report.yml`, `feature_request.yml`) so
  `test_issue_template_agent_lists_match_runtime_integrations` keeps
  the runtime/template surfaces synchronized

Closes #822

Assisted-by: Droid (oracle-reviewer)

Assisted-by: Droid (model: MiniMax M3, autonomous)

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
 2026-07-22 07:49:48 -05:00
Ali jawwad 4f4d19ba93 docs(core): document the 'py' (Python) --script type in the init option table (#3625) 
'specify init --script' accepts sh, ps, or py (init.py + SCRIPT_TYPE_CHOICES in
_agent_config.py), and 'specify init --script py' scaffolds Python scripts. The
core.md option reference listed only 'sh|ps', so a user following the canonical
docs never learns about --script py. Update the option row to sh|ps|py.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-07-22 07:34:43 -05:00
WOLIKIMCHENG b6b3ec49d9 docs: clarify hook priority validation semantics (#3594) 
* docs: clarify hook priority validation semantics

* docs: clarify stored hook priority normalization

---------

Co-authored-by: root <kinsonnee@gmail.com>
 2026-07-22 05:28:02 -05:00
Davide Barletta 74662cffad feat: update Bob integration to skills-based layout for Bob 2.0 (#3415) 
* feat: update Bob integration to skills-based layout for Bob 2.0

Bob 2.0 replaces the command-based workflow (.bob/commands/*.md) with
a skills-based layout (.bob/skills/speckit-<name>/SKILL.md), matching
the pattern used by Claude Code, Codex, and other skills-first agents.

- Switch BobIntegration from MarkdownIntegration to SkillsIntegration
- Update folder/dir from .bob/commands to .bob/skills
- Change extension from .md to /SKILL.md (skills layout)
- Add --skills option (default: True) consistent with Codex pattern
- Update tests to inherit from SkillsIntegrationTests (28 tests pass)
- Bump catalog entry to version 2.0.0 with updated description

Assisted-by: IBM Bob (model: claude-sonnet-4-5, autonomous)

* PR comments fix: keep old Bob 1 commands till next release

* Copilot suggested change

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* feat(bob): address copilot comments, make skills layout default, demote legacy commands to opt-in

* fix(bob): honor legacy_commands in ai_skills persistence and add bob to ALWAYS_SLASH_AGENTS

- init.py: suppress ai_skills=True when --legacy-commands is passed so
  extensions and presets target .bob/commands, not .bob/skills
- _invocation_style.py: add 'bob' to ALWAYS_SLASH_AGENTS so init next-steps
  and hook invocations always show /speckit-<name> (skills is the default
  layout; no ai_skills flag required)

* Copilot suggestion

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(bob): extend IntegrationBase directly to avoid false isinstance(SkillsIntegration)

- bob/__init__.py: switch BobIntegration base from SkillsIntegration to
  IntegrationBase; add _BobSkillsHelper for skills-mode delegation; set
  invoke_separator='-' explicitly; set _skills_mode flag in setup() so
  consumers can derive the effective mode without isinstance checks
- _helpers.py: replace isinstance(integration, SkillsIntegration) guard
  with getattr(_skills_mode) so legacy-commands mode does not persist
  ai_skills=True
- _invocation_style.py: remove 'bob' from ALWAYS_SLASH_AGENTS — Bob 2.0
  skills are invoked via natural language, not /skill-name slash commands
- integrations/catalog.json: advance updated_at to 2026-07-15

* fix(lint): remove unused SkillsIntegration import from _helpers.py

* Copilot suggested change

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* feat(bob): add bob skills integration with registrar-based mode detection

* address 3 comments from copilot

* feat(bob): update registrar config to use legacy commands layout

* fix lint

* Suggested fix from Copilot

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix pr comment

* fix pr comment

* fix pr comment

* refactor(bob): resolve skills mode via base-class hooks + fix command-ref separators

Rework the dual-mode handling introduced for Bob 2.0 so an integration's
internal representation never leaks into shared init/install/upgrade code,
and fix the legacy command-reference separator surfaced in review.

Base-class contract:
- Add IntegrationBase.is_skills_mode(parsed_options) — the single hook the
  shared machinery consults to decide whether to persist ai_skills and render
  skill invocations. SkillsIntegration returns True; Copilot honors --skills /
  self._skills_mode; Bob returns `not legacy_commands`.
- Add IntegrationBase.invoke_separator_for_mode(skills_enabled) — resolves the
  command-ref separator from a project's persisted mode for registration paths
  that only have the ai_skills flag (no CLI parsed_options). Default is
  behavior-preserving; Bob maps skills->"-", legacy->".".
- BobIntegration stays on IntegrationBase (mirroring Copilot, the other
  dual-mode agent) and delegates setup() to internal _BobSkillsHelper /
  _BobMarkdownHelper. Removes the _skills_mode method and all
  isinstance(SkillsIntegration) / callable(_skills_mode) probing from
  _helpers.py and init.py.

Fix legacy separator (review feedback): CommandRegistrar.register_commands and
PresetManager._resolve_skill_command_refs previously read the single static
AGENT_CONFIGS[key]["invoke_separator"], so legacy .bob/commands/ extension and
preset command refs rendered /speckit-<cmd> instead of Bob 1.x /speckit.<cmd>.
Both now resolve the separator per project mode via invoke_separator_for_mode.

Tests: add regression coverage for the is_skills_mode / invoke_separator_for_mode
hooks and legacy extension command-ref separators; normalize a width-sensitive
workflow assertion to match its siblings. Full suite green.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob,copilot): address review — preserve legacy layout, dual-mode separators, extension-skill token resolution

Addresses PR review 4716036212 (3 comments):

1. Bob legacy-install regression: `use`/`switch`/`upgrade` on an existing
   Bob 1.x project (only `.bob/commands/` on disk, no stored
   `legacy_commands`) called `is_skills_mode(None)` -> True and rewrote
   `ai_skills=True`, silently switching extension/command-reference handling
   to the skills layout. `is_skills_mode` now takes an optional `project_root`;
   Bob preserves an already-installed legacy layout until an explicit upgrade
   creates `.bob/skills/`. A fresh project still defaults to skills.

2. Copilot dual-mode separator: `invoke_separator_for_mode` was inherited
   from the base (mode-independent) and returned Copilot's static `.`, so
   preset/extension command refs in a Copilot skills project rendered
   `/speckit.<name>` instead of `/speckit-<name>`. Override it on Copilot to
   track the persisted `ai_skills` state, consistent with
   `build_command_invocation` and `effective_invoke_separator`.

3. Bob extension-skill command-ref tokens: verified that merging main's
   generic `_resolve_command_ref_tokens` (#3544) resolves Bob's tokens via
   the `CONDITIONAL_SLASH_AGENTS` path (`/speckit-<name>`); added Bob to the
   command-ref regression parametrize plus dedicated Bob use-path tests.

All tests pass (full suite green; merged with current main incl. #3544).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): resolve command-ref separator with project-aware mode before shared-infra refresh (review #3415)

The `use`/`switch` paths refresh shared infrastructure via
`_with_integration_setting()` / `_invoke_separator_for_integration()`,
which previously resolved the invoke separator through
`effective_invoke_separator` / `is_skills_mode` WITHOUT a project_root.
For a pre-PR Bob 1.x project (.bob/commands/ on disk, no stored options),
this defaulted to the skills "-" separator and rewrote rendered
shared-template command refs to /speckit-*, even though ai_skills stayed
false. Thread project_root through effective_invoke_separator, the two
runtime helpers, and every call site so Bob's on-disk legacy detection
governs the separator before shared infra is refreshed.

Add a rendered-shared-template regression test covering `use --force`.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): scope persisted ai_skills flag to active agent when resolving command-ref separator (review #3415)

`register_commands` runs once per detected agent, but the persisted
`ai_skills` flag describes only the active integration (`opts["ai"]`).
When another agent (e.g. Copilot) is active in skills mode while a
legacy `.bob/commands` layout is also present, the previous code passed
that global `True` to Bob's `invoke_separator_for_mode`, rewriting Bob
1.x command refs to `/speckit-*` instead of `/speckit.*`.

Only consult the persisted flag for the agent it describes
(`opts["ai"] == agent_name`); otherwise resolve the separator from the
agent's own project-aware `effective_invoke_separator(None, project_root)`.

Add regression tests covering the mismatched-active-agent case and a
control for Bob-active skills mode.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): detect Spec Kit layout from managed artifacts, not any skills dir (review #3415)

Two related mis-detections from review 4723246468:

1. `BobIntegration.is_skills_mode` treated the mere presence of a
   `.bob/skills/` directory as proof the project is skills-based. A legacy
   Spec Kit install (managed `.bob/commands/speckit.*.md`) that also carried
   unrelated Bob 2 skills would be misclassified as skills, so
   `integration use bob` persisted `ai_skills` and rewrote shared refs.
   Now the layout is inferred from managed Spec Kit artifacts: legacy/command
   mode only when managed `speckit.*.md` command files exist and no managed
   `speckit-*` skill dirs do.

2. The `register_commands` separator for an inactive agent used a disk-based
   `effective_invoke_separator(None, project_root)` fallback that could pick
   the skills separator even though the registrar writes the static command
   layout (`.bob/commands/*.md`). Inactive agents now resolve the separator
   from the registrar's actual output layout (`extension == "/SKILL.md"`),
   so command-layout files keep `/speckit.*` refs regardless of sibling dirs.

Update the affected hook/E2E tests to use managed artifacts and add
regression tests for the mixed-layout and inactive-registrar scenarios.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): apply managed-artifact detection on upgrade + consistent skill post-processing (review #3415)

Two issues from review 4723782860:

1. `BobIntegration.setup()` resolved the layout via `is_skills_mode(parsed_options)`
   WITHOUT `project_root`, so `integration upgrade bob` on a Bob 1.x install
   (managed `.bob/commands/speckit.*.md`, no stored options) ignored the
   existing command files, generated skills, and stale-deleted the legacy
   commands — silently migrating the project. Pass `project_root` so the same
   managed-artifact detection used by `use` also governs upgrades.

2. Only `_BobSkillsHelper` overrode `post_process_skill_content` to suppress
   the shared slash-command hook note. Preset/extension skill generators call
   that hook on the registered `BobIntegration`, which inherited
   `IntegrationBase`'s note-injecting default. Repeat the no-op (delegating to
   the skills helper) on the registered class so every Bob skill-generation
   path is consistent with intent-activated core Bob skills.

Add regression tests for the upgrade-preservation and post-processing paths.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* feat(bob): add --skills migration opt-in; fix separator + manifest loss (review #3415)

Address review #3415 (4724160183):

- Comment 1: Add an explicit `--skills` opt-in to BobIntegration. It forces
  the skills layout over on-disk auto-detection, giving legacy Bob 1.x
  installs a supported migration path
  (`integration upgrade bob --integration-options="--skills"`). `--skills`
  and `--legacy-commands` are mutually exclusive (clean exit-1 error).

- Comment 2: In CommandRegistrar.register_commands, derive the command-ref
  separator from the output layout (agent_config["extension"]) for the
  active agent too, not the persisted ai_skills flag. A command-layout file
  (.bob/commands/*.md, .github/agents/*.agent.md) always renders /speckit.*;
  only a /SKILL.md scaffold uses /speckit-*. Dual-layout agents (Bob,
  Copilot) write skills via their own setup()/skills path, so
  register_commands only ever emits their command-layout files.

- Comment 3: Update docs/reference/integrations.md Bob entry to document the
  skills-based default (.bob/skills/), the deprecated --legacy-commands
  opt-out, and the --skills migration path.

Also fix a latent manifest-loss bug surfaced by the migration path: the
upgrade Phase 2 stale-file cleanup built a throwaway manifest sharing the
integration key and called uninstall(), which always deleted
{key}.manifest.json. Any layout-shrinking upgrade (e.g. legacy->skills)
thus wiped the freshly-saved manifest, leaving the project untracked and
un-upgradeable. uninstall() now takes remove_manifest (default True); the
stale-cleanup pass passes False.

Adds regression tests for the --skills opt-in, mutual exclusion, corrected
active-agent separator, remove_manifest=False, and an end-to-end
legacy->skills migration that verifies the manifest survives and the
project remains upgradeable. Full suite: 4555 passed, 5 skipped.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* docs(agents): align token-resolution comment with output-layout separator rule (review #3415)

Address review #3415 (4725516805). The comment above resolve_command_refs
still described the removed state-based behavior ("resolve it from the
integration using the project's persisted skills state"). Update it to
describe the output-layout rule that register_commands now uses: _sep is
derived from the layout this registrar writes (a /SKILL.md scaffold uses the
skills separator; a command-layout file uses the command separator), not the
persisted ai_skills state. Comment-only change; no behavior change.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): reconcile extension artifacts on layout change (review #3415)

When a dual-mode agent (Bob) flips between the legacy commands layout and
the skills layout during `integration upgrade` (via `--skills` /
`--legacy-commands`), the old layout's extension command/skill files were
left orphaned: Phase 2 stale cleanup only removes files tracked by the
*integration* manifest, while extension artifacts are tracked in the
extension registry. Detect the layout flip by comparing whether the old vs
new manifest tracks a `/SKILL.md` scaffold, and when it changed, unregister
the agent's extension artifacts before the existing re-registration so they
are recreated in the new layout (and the per-agent registry is updated).

Preset artifacts are documented as a known, pre-existing cross-cutting gap:
no agent-scoped preset re-registration exists in use/switch/upgrade for any
agent, so reconciling them is out of scope for this Bob migration.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): reject layout migration when preset overrides are installed (review #3415)

A command↔skills layout change during `integration upgrade` cannot reconcile
preset artifacts: presets track their command/skill files in per-preset
`registered_commands`/`registered_skills` metadata, and there is no
agent-scoped preset re-registration anywhere in the CLI. Migrating would
delete a preset's old-layout files without recreating them in the new layout
and leave the preset registry claiming artifacts that no longer exist.

Detect the intended layout via `is_skills_mode` (so a plain same-layout
upgrade is unaffected) and, when it flips while preset overrides are
installed for the agent, reject the upgrade *before any mutation* with an
actionable error pointing at the remove → upgrade → reinstall workaround.
Extension artifacts are still reconciled for the safe (no-preset) case.

Adds a regression test and documents the migration caveat in the Bob
integration reference entry.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): restrict layout reconciliation to the active integration (review #3415)

`integration_upgrade` supports upgrading a secondary (non-active) integration,
but the layout-change extension reconciliation was unsafe there.
`ExtensionManager.unregister_agent_artifacts()` treats the unscoped
per-extension `registered_skills` list as belonging to the passed agent and,
when that agent's skills directory is absent, falls back to scanning every
agent's skills directory — so reconciling a secondary Bob layout flip could
delete or untrack the *active* agent's extension skills. The subsequent
re-registration cannot repair that because extension skill rendering is
intentionally scoped to the active agent (#2948).

Gate the unregister-before-register reconciliation on `installed_key == key`
so it only runs for the active integration. Secondary agents only ever have
extension command files (skills are active-agent-only), which the existing
re-registration rewrites in place, so skipping the unregister orphans nothing
new. Adds a regression test asserting a secondary Bob layout change leaves the
active agent's extension skill intact on disk and in the registry.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): fail closed when preset registry is unreadable (review #3415)

Address review 4744636079:

- _migrate_commands: the preset guard previously failed *open* — a
  registry read/parse error returned an empty "no presets" list, so a
  --force layout-changing upgrade could delete preset-overridden command
  files while their registry state was unknown. Read the registry file
  directly and raise _PresetRegistryUnreadableError on any read/parse
  failure or malformed structure, rejecting the migration before any
  mutation. A genuinely absent registry still returns [] (safe).

- bob: correct the is_skills_mode docstring — upgrade *does* run setup();
  disk detection is needed because legacy Bob 1.x installs never persisted
  a legacy_commands option, so the stored mode is unavailable.

- tests: add fail-closed E2E (corrupted registry rejected, valid-empty
  allowed) plus a unit test for _installed_presets_affecting_agent covering
  absent / corrupted / malformed / valid / affecting-agent cases.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): fail closed on malformed preset entries too (review #3415)

Address review 4745191015: the preset guard read a parseable registry but
silently skipped malformed per-preset metadata and treated a malformed
registered_commands value as "no matching artifacts". A registry such as
{"presets":{"p1":[]}} therefore allowed a layout migration even though p1's
ownership is unknown, risking deletion of preset-managed files. Now raise
_PresetRegistryUnreadableError for a non-dict preset entry, a non-dict
registered_commands, or a non-list registered_skills. Extend the unit test
to cover these malformed shapes.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-07-21 09:20:20 -05:00
Markus Wondrak d6fa0460ed feat(workflows): WorkflowResolver standalone (PR 1) (#3557) 
* feat(workflows): add standalone WorkflowResolver and overlay subsystem

Implement PR 1 of the workflow-overlays plan: a concrete, standalone
WorkflowResolver for downstream workflow extensibility without touching the
Preset subsystem.

- Add overlay manifest schema (Overlay, OverlayEdit, validate_overlay_yaml)
- Add pure-function merge engine (find_step, apply_edit, merge_steps,
  validate_edits) with recursive anchor search and higher-wins semantics
- Add StepListComposer and tiered layer sources (project, installed, base)
- Add WorkflowResolver facade with inline HIGHER_WINS priority sorting
- Add CLI verbs: workflow overlay add/set-priority/enable/disable/remove/list
  and workflow resolve <id>
- Wire WorkflowEngine.load_workflow through WorkflowResolver
- Extend workflow add to copy optional overlays/ subdirectory from local
  workflow directories
- Add comprehensive unit, integration, and security tests

Refs: discussion #3473 (https://github.com/github/spec-kit/discussions/3473)

Assisted-by: Kimi (model: opencode-go/kimi-k2.7-code, autonomous)

* fix(workflows): reject symlinked overlay directories in layer sources

Address PR #3557 review comments r3594064534 and r3594064563:

- ProjectOverlaySource.collect now rejects symlinked per-workflow overlay
  directories (.specify/workflows/overlays/<id>) before iterating
- InstalledOverlaySource.collect now rejects symlinked installed overlay
  directories (.specify/workflows/<id>/overlays) before iterating
- workflow_overlay_list catches ValueError from resolver and exits with
  code 1 instead of crashing on unhandled exceptions
- Added .specify/workflows/overlays to _reject_unsafe_workflow_storage
  chokepoint for defense-in-depth

These guards prevent symlinked overlay directories from redirecting
auto-loaded overlay YAML to attacker-controlled content outside the
project, which could inject executable shell steps into trusted workflows.

Refs: PR #3557 review comments r3594064534, r3594064563

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* fix(workflows): address Copilot review findings in merge engine

- Apply inserts before winning replace to prevent anchor-not-found errors
  when replace changes step ID (r3594064604)
- Track attribution recursively for nested steps in composite inserts/replaces
  so workflow resolve attributes all child steps correctly (r3594064638)
- Add regression tests for both fixes

Refs: PR #3557 review discussion

Assisted-by: GitHub Copilot (model: qwen3.7-plus, autonomous)

* refactor(workflows): simplify overlay architecture to 2-tier

Remove installed overlays tier to enforce clean separation of concerns:
- workflow add installs workflows only (no overlay copying)
- workflow overlay add installs overlays only (project-local)

Changes:
- Remove InstalledOverlaySource class and all references
- Remove overlay-copying logic from _validate_and_install_local()
- Update WorkflowResolver to 2-tier: project overlays + base workflow
- Fix --priority override timing: apply before validation, not after
- Remove tests for installed overlays (no longer applicable)

Rationale: If upstream controls both base workflow and shipped overlays,
and both get overwritten on bundle update, there's no reason to ship
overlays separately. Overlays only make sense when someone other than
the base author adds them.

Resolves all three review findings from PR #3557:
- r3594064677: workflow add no longer copies overlays from all call sites
- r3594064705: --priority override now applied before validation
- r3594064726: no stale installed overlays (tier removed entirely)

Assisted-by: Claude (model: claude-opus-4-7, autonomous)

* fix(workflows): harden overlay symlink handling

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

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

* docs(workflows): remove stale installed-overlay references from workflows.md

The 2-tier refactor (cc28185) removed the installed-overlay tier entirely,
but docs/reference/workflows.md was not updated. This commit addresses all
four Cluster 2 findings from the PR review:

- workflow add: remove sentence about copying overlays/ subdirectory
- How Overlays Work: drop installed-overlay table row and precedence prose;
  rewrite to 2-tier model (project overlays only, source-order tie-break)
- overlay remove: drop trailing sentence about installed overlays
- Interaction with Bundles: rewrite to say workflow add installs only
  workflow.yml; remove installed-overlay discovery language

Fixes: r3596368791, r3596368831, r3596368873, r3596368919

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): detect ancestor-conflict anchors in merge_steps

When two overlay edits target anchors that share a parent/descendant
relationship (e.g. remove an if-step + insert_after a nested child),
merge_steps processed them independently and in dict-insertion order,
making the outcome non-deterministic.

Add two private helpers to merge.py:
- _descendant_ids(step): returns all step IDs nested inside a step dict
  by delegating to the existing _all_base_step_ids helper on children.
- _check_anchor_conflicts(anchors, base_steps): for each targeted anchor
  finds its descendants and checks whether any other targeted anchor is
  among them; returns human-readable error strings.

Wire _check_anchor_conflicts into merge_steps immediately after
edits_by_anchor is built, before any tree mutation occurs. Raises
ValueError listing the conflicting anchor pair(s) so overlay authors
know exactly what to fix.

Add TestMergeStepsAncestorConflicts (6 cases):
- remove parent + insert_after child raises ValueError
- replace parent + remove child raises ValueError
- conflict across multiple overlays raises ValueError
- sibling anchors (not ancestor/descendant) pass
- single anchor passes
- parent targeted but child not targeted passes

Closes review comment r3596368746 (PR #3557, round 2, cluster 3).

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): fix over-broad conflict detection and non-deterministic ID collision

Finding 1.1 — _check_anchor_conflicts was rejecting any ancestor/descendant
anchor pair, including insert-only edits that are perfectly safe. Only
replace/remove on an ancestor can destroy its subtree and make a descendant
anchor unresolvable. Change the signature to accept a dict[str, str]
(anchor → winning operation) and skip the check for insert_after/insert_before.

Finding 1.2 — merge_steps was calling find_step on the already-mutated tree,
so a replacement step that reused a base step ID could be accidentally targeted
by a later edit group (non-deterministic result depending on dict iteration
order). Replace the anchor-group loop with a single-pass _traverse_and_apply
that walks the original tree structure and applies edits as each step is
encountered. Anchors are never re-looked up in a mutated tree.

Design invariant enforced: overlays always apply to the original base tree and
cannot target steps introduced by other overlays. Non-remove edits on non-base
anchors now raise ValueError early.

Also removes apply_edit (no production callers, only tested in isolation) and
its test class — the new traversal inlines the same mechanics without the
find_step round-trip.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

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

* fix(overlays): reject ID trailing newlines and reuse existing .yaml path

Fix two input validation bugs in the overlay layer (Group 2 of copilot
review PR #3557):

1. _validate_safe_id in schema.py used re.match() which anchors only at
   the start of the string, so IDs like 'overlay\n' passed validation
   and could produce newline-containing file paths. Changed to fullmatch()
   so the entire string must satisfy the pattern.

2. workflow_overlay_add always wrote <id>.yml without checking whether
   <id>.yaml already existed. Since the resolver loads both extensions,
   this created two active layers whose edits applied twice. Now uses
   the existing _find_overlay_file() to detect a pre-existing file and
   reuse its path, falling back to .yml only for new overlays.

Tests added for both fixes.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

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

* fix(overlays): fix display order inversion and wrap file-read errors

Finding group 3 from copilot-review-v2.md:

3.1 — Precedence display inverted (overlays/__init__.py)
collect_all_layers used a single-pass sort by (-priority, source_asc),
which placed the *losing* equal-priority source first in the display
while claiming "highest first". Fix: two-pass stable sort — source
descending then priority descending — so the actual winner (last applied
by the composer) rises to the top of the display.

3.2 — Unwrapped file-read errors (overlays/layer_sources.py)
Only yaml.YAMLError was caught around path.read_text(), so an
unreadable or non-UTF-8 overlay produced a raw traceback. Fix: widen
the except clause to (yaml.YAMLError, OSError, UnicodeDecodeError),
matching the pattern used throughout catalog.py.

Tests:
- test_workflow_resolve_equal_priority_winner_shown_first: verifies
  project:zzz (the winner) appears before project:aaa in workflow resolve
  output when both overlays share the same priority.
- tests/workflows/test_overlay_layer_sources.py (new): OSError and
  non-UTF-8 bytes both produce OverlayLoadError, not raw tracebacks.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

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

* fix: rename misleading overlay test

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* fix: remove EOF blank line in overlay resolver

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: handle overlay read and enumeration errors

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* fix(overlays): validate resolver workflow IDs

Reject unsafe and reserved workflow IDs before overlay or base sources construct paths, preventing traversal through resolver and engine fallback paths.

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

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

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(overlays): drop _remove_sources_recursively from remove branch

In _traverse_and_apply, the remove branch called _remove_sources_recursively
to clean up attribution entries for the deleted step.  This was inherited from
the old apply_edit loop (c70a5d6) where it was needed because the sources dict
was queried exhaustively.

In the current single-pass design, _build_attribution only traverses the result
list, so stale sources entries for removed steps are never read.  The cleanup
call is therefore unnecessary — and actively harmful when another overlay has
replaced a different step with a new step that reuses the same ID: the pop
clobbers the replacement's attribution entry, causing workflow resolve to report
the surviving step as 'unknown'.

Fix: simply remove the _remove_sources_recursively call from the remove branch.
Add an attribution assertion to the existing reused-ID regression test to catch
this case.

Fixes: r3604242050 (Copilot review finding)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

* Fix CLI overlay ID validation anchoring

Use fullmatch for CLI workflow/overlay ID validation so trailing newlines are rejected consistently with manifest validation.

Add regression coverage for newline-suffixed workflow and overlay IDs in overlay set-priority.

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* fix(workflows): validate workflow_id in layer sources before path construction

ProjectOverlaySource.collect() and BaseWorkflowSource.collect() joined
workflow_id directly onto storage paths without validation, enabling
path traversal (e.g. '../../outside') when called outside the
WorkflowResolver.

Add _validate_workflow_id() to layer_sources.py — mirrors the same
_SAFE_ID_PATTERN / _RESERVED_WORKFLOW_IDS check used by WorkflowResolver
in overlays/__init__.py — and call it at the top of both collect()
methods before any path is constructed.

Adds parametrised tests covering unsafe IDs and verifying no filesystem
access occurs for an invalid ID.

Closes review finding r3604772700.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

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

* fix(workflows): align layer source validation with _safe_workflow_id_dir

Plan §4.1 requires that Workflow-ID-Validierung, Symlink-/Containment-
Prüfungen and Fehlerübersetzung must not diverge between workflow
management and the overlay resolver.

My previous fix added ID pattern + reserved-name validation to both
collect() methods but was missing the containment step and the
BaseWorkflowSource directory/file checks that _safe_workflow_id_dir
performs.

Changes:
- Add _ensure_contained_dir(path, root) to layer_sources.py — pure
  domain mirror of overlays/_commands.py::_ensure_contained_dir that
  raises OverlayLoadError instead of typer.Exit
- ProjectOverlaySource.collect(): replace two inline symlink/dir checks
  with _ensure_contained_dir(workflow_overlay_dir, self.overlays_dir),
  adding the missing resolve().relative_to() containment step
- BaseWorkflowSource.collect(): add _ensure_contained_dir on the
  workflow directory, and add workflow.yml symlink check before is_file()

The same logic now lives in three places (workflow CLI, overlay CLI,
layer sources). The DRY extraction to workflows/_validation.py is
deferred to PR 3 per plan §4.1.

Tests: add containment and symlink tests for both sources.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

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

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(overlays): resolve identity from manifest field, not filename

Align overlay identity resolution with the project-wide convention:
presets use preset.id, extensions use extension.id, workflows use
workflow.id, and workflow steps use step.type_key. Overlays must
derive identity from the manifest id field, not the filename.

Rewrite _find_overlay_file() to scan all YAML files in the overlay
directory and match on the manifest id field, fixing the bug where
enable/disable/remove/set-priority failed when filename != manifest id.

Closes: PR #3557 discussion r3605010197

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(workflows): make validation behavior consistent across YAML loading paths

Address PR #3557 review finding r3607632921:

- Wrap yaml.YAMLError → ValueError in from_yaml() and from_string() so
  malformed YAML matches the documented exception contract
- Add except ValueError to workflow_info to handle composition errors
  cleanly instead of crashing with a raw traceback
- Remove validate_workflow() from compose() so the resolver path is
  parse-only like all other YAML loading mechanisms; callers validate
  explicitly via engine.validate()
- Update test to reflect new behavior: resolve() returns composed
  definition, caller validates separately

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* fix(overlays): list disabled overlays in management view

Keep disabled overlays visible in workflow overlay list while leaving resolution behavior unchanged.

- add an include_disabled opt-in to overlay source/resolver collection
- use include_disabled=True for workflow overlay list
- add regression tests for list visibility and default filtering

Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* docs: align overlay extends and resolver contract

Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous)

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

* fix: use atomic write for overlay file updates to prevent hard-link attack

Replace in-place write_text() calls in workflow_overlay_add() and
_update_overlay_field() with the same mkstemp → write → os.replace()
pattern used by the workflow installer (_stage_workflow_file /
_commit_workflow_file / _discard_staged_workflow_file).

The prior code rejected symlinks and validated path containment, but a
hard-linked destination file passes both checks while sharing an inode
with an external file. write_text() would then truncate and overwrite
that external inode. The atomic staging approach never opens the
existing destination for writing, eliminating the hard-link vector.

Fixes findings r3608669512 and r3608669517 on PR #3557.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised)

* fix(composer): preserve invalid base definition instead of coercing steps to []

When 'steps' is not a list, returning early with the unmodified
WorkflowDefinition lets validate_workflow surface the proper error
("'steps' must be a list.") to the caller. The previous silent
coercion to [] masked the validation error entirely.

Fixes: https://github.com/github/spec-kit/pull/3557#discussion_r3608669506

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised)

* fix: align workflow overlay priority semantics

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

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

* fix: validate overlay priority presentation

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

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

* fix: catch OverflowError in normalize_priority for float infinity values

YAML values like `priority: .inf` parse to float('inf'), causing
int() to raise OverflowError. This broke validate_overlay_yaml()'s
'validation never raises' contract. Adding OverflowError to the
except clause makes it fall back to the default priority (10),
consistent with other invalid value handling.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Markus <markus@example.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-07-21 08:35:25 -05:00
Ali jawwad 2df0394cb2 docs(integrations): document the 'integration list --catalog' flag (#3530) 
* docs(integrations): document the 'integration list --catalog' flag

'specify integration list' accepts a --catalog flag (integrations/_query_commands.py:
typer.Option(False, "--catalog", ...)) that browses the full built-in +
community catalog, but the Integrations reference documented no options for the
list command. Add an option table for it, matching the style used by the sibling
'integration search' and 'integration catalog add' sections.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(integrations): clarify that default 'integration list' shows only built-ins

The --catalog row implied the default list already includes the full installed
set; in fact 'integration list' iterates INTEGRATION_REGISTRY (built-ins) and
marks installed status, so a community integration that is not built in only
appears with --catalog. Reword the option and the intro sentence to say the
default shows the built-in integrations and --catalog adds community ones.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-07-17 15:09:28 -05:00
Manfred Riem 396fc2c240 docs: reframe SDD positioning, modernize install, and de-duplicate walkthroughs (#3565) 
* docs: reframe SDD positioning, modernize install, and de-duplicate walkthroughs

Reframe the landing page so Spec Kit reads as a toolkit for Spec-Driven
Development *or your own process* with any AI coding agent, and correct
stale claims: context files and git are now opt-in extensions, and the
install path uses PyPI (specify-cli). Generalize the landing cards to
cover bundles and catalog hosting across all primitives.

Restructure the Quick Start into a lean, guided Taskify walkthrough with
one command per step (install as a prerequisite, Steps 1-9 aligned with
the Full path), and extract the deep per-command detail into two new
reference pages: reference/agentic-sdd.md (the /speckit.* SDD process)
and reference/agentic-bugfix.md (the bug extension). Retitle the
reference overview to "Reference" and group these agentic processes in
their own section, distinct from CLI-managed primitives.

Remove the duplicated "Detailed Process" walkthrough from README.md
(and its TOC entry), repointing readers to the docs-site Quick Start
while keeping the concise "Get Started" section as the front door.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d9232f8-ece4-4aa6-a9bd-ff8d74ca1c89

* docs: address review feedback on accuracy and scope

- quickstart: correct the git/feature note — resolution reads
  .specify/feature.json / SPECIFY_FEATURE, not the checked-out branch,
  so switching branches alone does not switch the active feature.
- quickstart + agentic-sdd: add an invocation-style note ($speckit-* for
  Codex/ZCode, /skill:speckit-* for Kimi) so the agent-neutral commands
  are executable everywhere.
- agentic-sdd: fix the tasks phase structure to match the generator
  (Setup, Foundational, one phase per user story, final Polish; tests
  optional within user-story phases).
- index: soften the catalog claim (catalogs curate discovery, not an
  install allow-list) and relabel the "CLI reference" link to "Reference"
  to match the retitled, broader page.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d9232f8-ece4-4aa6-a9bd-ff8d74ca1c89

* docs: correct feature-resolution override and add bug-command invocation note

- quickstart: the previous fix named the wrong override. The active
  feature *directory* resolves from SPECIFY_FEATURE_DIRECTORY then
  .specify/feature.json; SPECIFY_FEATURE only supplies the identifier
  after a directory is resolved. Rewrite the note to point users at
  .specify/feature.json / SPECIFY_FEATURE_DIRECTORY, and clarify the git
  extension's branches don't by themselves change the active feature.
- agentic-bugfix: add the same invocation-style caveat as the SDD
  reference ($speckit-bug-* for Codex/ZCode, /skill:speckit-bug-* for
  Kimi).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d9232f8-ece4-4aa6-a9bd-ff8d74ca1c89

* docs: tighten bug-command contracts and drop numbered-phase examples

- agentic-bugfix: don't overstate overwrite protection — an interactive
  run can overwrite an existing assessment after confirmation; only
  automated mode refuses and picks a new slug. Correct the verify verdict
  to the schema's verified/partial/failed (not-run is a per-check status);
  an unexercised reproduction downgrades the result to partial.
- agentic-sdd: the implement examples labeled scoping "Phase 1/2", but
  the tasks contract reserves Phase 1 for Setup and Phase 2 for
  Foundational (user stories start at Phase 3). Scope by phase name and
  user-story content instead to avoid mis-scoping execution.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d9232f8-ece4-4aa6-a9bd-ff8d74ca1c89

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-07-16 11:08:25 -05:00
Ehtasham Yasin b08d665837 docs: document extensions.yml hook configuration (#3563) 
* docs: document extesnion.yml hook confriguration

* docs: address hook confriguration review feedback

* clarify auto execute hooks behavior

* docs: clarify hook priority and condition behavior
 2026-07-16 10:43:30 -05:00
Nate Chadwick fd101d531e feat(integrations): add Grok Build skills-based integration (#3535) 
* feat(integrations): add Grok Build skills-based integration

Add first-class support for xAI Grok Build via SkillsIntegration, installing
speckit skills under .grok/skills and wiring init/invocation/catalog surfaces.

Assisted-by: Grok Build (model: grok-4, supervised)

* test+docs: address Copilot review on Grok multi-install and next steps

Assert init next-steps guidance for Grok (.grok/skills, /speckit-*) and
clarify that multi-install safety is path/manifest isolation, not
agent-context defaults such as shared AGENTS.md.

* fix(integrations): Grok headless --always-approve and isolation paths

Document Grok multi-install isolation as .grok/skills and .grok/rules.
Override build_exec_args to pass --always-approve so non-interactive
dispatch is not blocked at tool permission gates.

* docs(integrations): list only managed .grok/skills for Grok isolation

Multi-install isolation documents Spec Kit-managed paths; Grok only
writes .grok/skills, so drop the read-only .grok/rules entry.

* fix(integrations): always-slash Grok hooks and refresh catalog date

Move grok to ALWAYS_SLASH_AGENTS so hooks never emit /speckit.plan when
ai_skills is missing/false. Update slash-format tests, persist ai_skills
on init, and bump catalog updated_at for the Grok entry.

---------

Co-authored-by: Nate Chadwick <1232206+natechadwick@users.noreply.github.com>
Co-authored-by: test <test@example.com>
 2026-07-15 14:55:25 -05:00
Roland Huss 6688b447b7 feat(workflows): expose workflow source directory to steps (#3469) 
* feat(workflows): expose workflow source directory to steps (#3467)

Propagate WorkflowDefinition.source_path to steps via
{{ context.workflow_dir }} in template expressions and
SPECKIT_WORKFLOW_DIR env var for shell steps. The original
source directory is persisted in state.json so resume
restores the correct value instead of the run-directory copy path.

Closes #3467

Assisted-By: 🤖 Claude Code

* fix: apply bot review suggestions (#2)

Applied fixes from bot review comments:
- Comment #3563319058: prevent stale SPECKIT_WORKFLOW_DIR leak from parent env
- Comment #3563319094: use cross-platform Python one-liner instead of printenv
- Comment #3563319103: add monkeypatch.delenv for deterministic env var test
- Comment #3563319116: same env leak fix as #3563319058

Assisted-By: 🤖 Claude Code

* fix: use YAML single-quotes and forward-slash paths for Windows CI (#2)

sys.executable on Windows returns backslash paths (D:\a\...) which YAML
double-quoted strings interpret as escape sequences. Switch to
single-quoted YAML strings and normalize paths with replace("\\", "/").

Assisted-By: 🤖 Claude Code

* fix: resolve workflow_dir to absolute path and add installed-by-ID test (#3469)

Applied fixes from bot review comments:
- Comment #3563382853: resolve source_path before taking parent to ensure absolute paths
- Comment #3563382864: add test for installed-by-ID workflow_dir semantics

Assisted-By: 🤖 Claude Code

* docs: document context.workflow_dir and SPECKIT_WORKFLOW_DIR

Add reference documentation for the new workflow_dir runtime value in
both workflows/README.md and docs/reference/workflows.md so workflow
authors can discover the feature and its semantics.

Assisted-By: 🤖 Claude Code

* fix: clarify installed workflow_dir is an absolute path (#3469)

The documentation for context.workflow_dir described the installed-by-ID
case as ".specify/workflows/<id>/" which appears relative, contradicting
the "resolved absolute path" semantics. Clarified that it is the absolute
path to the installation directory.

Assisted-By: 🤖 Claude Code

* fix: apply bot review suggestions (#3469)

Applied fixes from bot review comments:
- Comment #3580005128: Quote sys.executable in shell step env var test
- Comment #3580005174: Quote sys.executable in no-env-var test

Assisted-By: 🤖 Claude Code

* fix: apply bot review suggestions (#3469)

Applied fixes from bot review comments:
- Comment #3587146944: Quote interpolated workflow_dir path in example

Assisted-By: 🤖 Claude Code
 2026-07-15 08:09:35 -05:00
Marsel Safin d7b6626218 feat(workflows): align workflow CLI with extension command surface (#3419) 
* feat(workflows): align workflow CLI with extension command surface

Adds the missing workflow commands and flags so the workflow CLI
matches the extension/preset pattern: add --dev and --from, search
--author, update, enable and disable. Disabled workflows are blocked
from running and marked in list output.

Fixes #2342

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

* fix(workflows): preserve disabled state on update, guard corrupted registry entries

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

* fix(workflows): guard list against corrupted registry entries, re-raise typer.Exit in catalog install

workflow list now skips non-dict registry entries with a warning instead
of crashing, matching update/enable/disable. The broad except in
_install_workflow_from_catalog no longer swallows typer.Exit, so precise
errors like the non-HTTPS redirect message are not duplicated.

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

* fix(workflows): escape rich markup in id-mismatch errors and validate --from source early

The two id-mismatch error paths interpolated repr() into Rich markup, so
a stray bracket in a user typo could be parsed as markup. Route both
through rich.markup.escape.

`workflow add <source> --from <url>` also validated the source only
after downloading. Validate it up front so a URL/path/typo fails
without a network fetch.

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

* fix(workflows): escape rich markup in list output and catalog install errors, isolate update failures

workflow list now escapes id/name/version/description before printing,
matching how extensions render user-editable fields. The catalog install
helper computes safe_wf_id once and uses it for every early error path
plus the final failure message.

workflow update wraps _safe_workflow_id_dir and the backup read inside
the try/except typer.Exit block so an unsafe id in a corrupted registry
fails that one workflow and the rest continue.

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

* fix(workflows): escape rich markup in --from download exception message

Matches how the catalog install path escapes exception strings.

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

* fix(workflows): catch OSError in per-workflow update loop and make restore best-effort

Transient FS errors (perms, disk full) from backup read or write no
longer abort the whole update run. The restore is wrapped in its own
try/except so a failed write only warns, and the offending workflow
is reported via 'Failed to update' like other per-workflow failures.

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

* fix(workflows): escape rich markup in search output

workflow search now escapes catalog-derived name/id/version/description/
tags before printing, matching extension search and workflow list.

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

* fix: escape workflow validation errors before Rich output

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

* fix(workflows): escape remaining unescaped Rich markup paths

Covers the last few review threads not yet addressed:
- Escape yaml.YAMLError text in the local workflow add install path
  (matches the already-escaped download/catalog paths).
- Escape the non---dev local directory fallback's "No workflow.yml
  found in <path>" message (the --dev branch already escaped it).
- Escape the redirected final_url in the --from non-HTTPS redirect
  error (IPv6 literals like http://[::1]/... are legal and contain
  brackets).
- Escape the "Downloaded workflow is invalid" exception message in
  _install_workflow_from_catalog, matching the sibling catalog-install
  exception handler a few lines above it.

Adds regression tests for each in TestWorkflowCliAlignment, following
the existing escaping-test pattern in this class.

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

* fix(workflows): escape workflow name/id in install success messages

Workflow names and ids come from user-controlled YAML or external catalog
data; printing them unescaped lets bracket characters be interpreted as
Rich tags. Escape them in the add/catalog-install success messages and the
remaining catalog error paths, matching the rest of the output hardening.

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

* fix(workflows): fail cleanly on unparseable catalog install URLs

urlparse raises ValueError on e.g. an unbalanced IPv6 literal before the
invalid-URL branch is reached; on workflow update that also bypassed the
per-workflow handler and aborted the whole command. Convert the parse
failure into a clean error so add fails cleanly and update skips just the
affected workflow.

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

* fix(workflows): reject catalog updates whose downloaded version mismatches

The update path never verified the downloaded workflow carries the catalog
version that triggered the update, so a stale or misconfigured URL could
report success while leaving the old version installed or downgrading it.
Pass the expected version into the install helper and fail the update when
the downloaded definition does not match.

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

* fix(workflows): validate workflow ID in run command and document new CLI flags

Path-equivalent spellings like "align-wf/" previously bypassed the
registry disabled check because the engine normalizes the path while the
registry matches the raw string. workflow run now validates non-file
sources against the workflow ID pattern before lookup.

Also updates docs/reference/workflows.md with --dev/--from install
options, update/enable/disable commands, and the search --author flag.

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

* fix(workflows): enforce disabled state for direct paths to installed workflows

Running the installed copy's YAML directly (specify workflow run
.specify/workflows/align-wf/workflow.yml) skipped the registry check.
File sources resolving inside .specify/workflows/<id>/ now map back to
the workflow ID and refuse to run while disabled.

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

* fix(workflows): reject explicit empty --from URL instead of catalog fallback

'workflow add foo --from ""' fell through 'from_url or ...' to a
catalog install. Distinguish None from empty string so explicit values
stay on the URL-validation path and fail closed.

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

* fix(workflows): registry rollback on save failure, consistent disabled check, honest update summary

- WorkflowRegistry.add now rolls back its in-memory mutation when save()
  raises, so a later successful save cannot persist metadata for a
  failed update alongside the restored YAML backup.
- workflow run uses the same truthiness check for 'enabled' as list and
  disable, so malformed values like 0 or null refuse to run.
- workflow update reports 'No workflows were eligible for update' when
  every target was skipped instead of claiming all are up to date.

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

* fix(workflows): guard non-string catalog URL and keep enable/disable rollback intact

- A truthy non-string catalog url (e.g. 123) reached urlparse and raised
  AttributeError, escaping the clean error path; validate it is a string.
- enable/disable mutated the live registry entry before add(), so add's
  rollback snapshot captured the already-toggled object; pass a fresh
  mapping instead.

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

* fix(workflows): tolerate non-dict registry entries in add and clarify test docstrings

A corrupted-but-parseable registry entry (e.g. a string value) crashed
WorkflowRegistry.add with AttributeError on existing.get. Guard the
non-dict case while still restoring the original raw value on rollback.

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

* fix(workflows): atomic registry save and accurate mixed-target update summary

- save() wrote the registry with open('w'), so a failed dump truncated
  the file and the next load reset every entry. Write to a sibling temp
  file and os.replace into place.
- workflow update no longer claims all workflows are up to date when
  some targets were skipped; it reports checked-only status with a
  skipped count.

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

* fix(workflows): exclusive temp file for registry save and cwd-independent disabled guard

- save() now uses tempfile.mkstemp in the workflows dir (matching the
  engine's atomic writer), so a pre-created symlink at a predictable
  .tmp path cannot redirect the write and concurrent processes cannot
  collide.
- The direct-path disabled guard derives the owning project from the
  resolved file path instead of the caller's cwd, so running an
  installed workflow's YAML from outside the project still refuses when
  disabled.

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

* fix(workflows): symlink guards and shape validation in workflow registry, dev-dir file check

- WorkflowRegistry now mirrors StepRegistry: _load refuses symlinked
  parents/registry file and normalizes a non-dict workflows field;
  save() rejects symlinked paths before writing.
- workflow add --dev requires workflow.yml to be a regular file so a
  directory named workflow.yml gets the documented CLI error instead of
  an uncaught IsADirectoryError.

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

* fix(workflows): validate download redirects before following them

All three workflow download sites (add --from, catalog install, step
install) passed no redirect_validator to open_url, so an HTTPS URL
redirecting to cleartext HTTP issued the insecure request before the
post-hoc geturl() check reported it. Shared validator now rejects
non-HTTPS redirects (loopback HTTP allowed) pre-follow, matching the
preset download path.

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

* test(workflows): accept redirect_validator kwarg in step-add open_url fakes

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

* fix(workflows): guard directory-shaped workflow.yml and unreadable registry

- workflow add's plain local-path fallback (no --dev) checked wf_file.exists()
  before installing, so a directory literally named workflow.yml passed the
  guard and _validate_and_install_local() leaked an uncaught
  IsADirectoryError instead of the documented CLI error. Use is_file(),
  matching the --dev branch's existing guard.
- WorkflowRegistry._load() treated any OSError while reading an existing
  registry the same as corrupted JSON, resetting to an empty in-memory
  registry. A later save() would then silently persist that empty state via
  os.replace, discarding every previously installed workflow entry. Track a
  _load_error flag on OSError-during-read and have save() refuse to write
  when it is set, so a transient I/O failure can no longer overwrite intact
  data on disk.
- docs/reference/workflows.md: document `--from <url>` with its value
  placeholder, matching extensions.md and presets.md.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflow): rollback registry.remove() and guard OSError at CLI boundaries

Critical: WorkflowRegistry.remove() deleted the in-memory entry then
called save() with no rollback, unlike add(). Combined with
workflow_remove deleting the workflow directory before calling
registry.remove(), a save failure permanently destroyed the workflow's
files, left the on-disk registry still claiming it installed, and
surfaced a raw unhandled OSError with no CLI message.

- WorkflowRegistry.remove() now rolls back the in-memory entry on a
  save() OSError, mirroring add()'s existing rollback pattern.
- workflow_remove persists the registry removal (registry.remove(),
  wrapped in try/except OSError -> clean escaped message) before
  deleting any files, so a save failure never touches the workflow
  directory.

Important sibling paths: workflow add (local/--dev/--from and catalog),
enable, and disable all called registry.add() without catching its
deliberate OSError, so a save failure surfaced either an orphaned
install directory (fresh local/catalog installs) or a raw/unhandled
exception with no clean CLI output.

- _validate_and_install_local (backs local/--dev/--from) now removes
  the freshly created directory on a fresh install, or restores the
  prior workflow.yml bytes on a reinstall-over-existing-local install,
  before raising a clean escaped error.
- _install_workflow_from_catalog wraps the final registry.add() using
  the function's own established convention (rmtree the just-downloaded
  workflow_dir, then a clean escaped error) -- workflow_update's
  existing backup/restore around this function is unaffected.
- workflow_enable/workflow_disable catch registry.add()'s OSError and
  print a clean escaped message instead of leaking the exception.

Added failing-first tests proving each behavior (registry-unit rollback
test, CLI-level remove/add/enable/disable save-failure tests
parametrized where they share one root cause), all confirmed red before
the fix and green after.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflow): preserve prior catalog install on reinstall registry-save failure

_install_workflow_from_catalog's final registry.add() failure handler
unconditionally rmtree'd workflow_dir. That's safe for a brand-new
install, but plain `workflow add <catalog-id>` also allows re-adding an
already-installed workflow, downloading the new version over the
existing directory first. If registry.add() then failed to save, the
unconditional rmtree deleted the prior working install while the
registry (after its own rollback) still reported it installed -- data
loss with no way back. workflow_update already avoids this via an outer
backup/restore around this function, but plain add has no such caller.

Fix mirrors _validate_and_install_local's existed-before/backup-aware
handling: capture whether workflow_dir existed and back up its
workflow.yml bytes before any download write, then on a registry.add()
OSError, restore those bytes for a reinstall or rmtree only a
brand-new directory. Only one file (workflow.yml) is ever written by
this path, so no further per-file bookkeeping is needed.

Added a failing-first regression: install a catalog workflow, re-add it
with a simulated registry save OSError, and assert a clean error, the
original workflow.yml restored byte-for-byte, and the registry still
reporting the original version installed. Confirmed red (prior file
deleted) before the fix, green after.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflow): centralize catalog-install cleanup across all failure branches

_install_workflow_from_catalog is new in this PR and has seven failure
branches after the mkdir/download step, each independently rmtree'ing
workflow_dir: redirect-to-non-HTTPS rejection, a generic download
exception, invalid downloaded YAML, a validate_workflow failure, a
workflow-id/catalog-key mismatch, a version mismatch, and (fixed in the
prior commit) a registry.add() OSError. Only the last one had been
special-cased to spare a prior working install on reinstall; the other
six still unconditionally deleted the whole directory, so re-adding an
already-installed catalog workflow and hitting any of those six earlier
failures destroyed the working install even though nothing about it had
actually changed.

Replaced all seven ad hoc rmtree call sites with a single local
_cleanup_failed_install() helper that closes over the existed_before /
prior_workflow_bytes captured once at the top of the function: restore
the prior workflow.yml for a reinstall, or rmtree only a directory that
this attempt itself created. Every failure branch now calls this one
helper, so the fix is structural rather than duplicated, and every
existing error message/exit code is unchanged -- only the cleanup
performed before each message is different.

Added a parametrized regression test covering the four early-failure
trigger points reachable from plain workflow add (redirect rejection,
download exception, invalid YAML, ID mismatch): each installs a catalog
workflow, re-adds it while forcing that specific failure, and asserts a
clean error plus the original workflow.yml surviving byte-for-byte.
Confirmed red against the unfixed code (all four raised FileNotFoundError
reading the deleted file) before applying the helper, green after.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflow): restore registry entry verbatim on post-removal rmtree failure

workflow_remove now persists registry.remove() before deleting any
files (fixed previously), but if the registry write succeeds and the
subsequent shutil.rmtree(workflow_dir) then fails, the registry was
left claiming the workflow uninstalled while its directory remained on
disk -- an orphaned install with no path back to a clean state.
workflow_step_remove already handles this exact sequencing by capturing
the registry entry before removal and restoring it directly into
registry.data plus save() (bypassing add(), which would stamp a new
updated_at) if the directory removal fails afterwards.

Applied the same pattern to workflow_remove: capture registry_metadata
via registry.get() before registry.remove(), and on an rmtree OSError,
write it straight back into registry.data["workflows"][workflow_id] and
save(), matching workflow_step_remove's restore-failure handling (a
yellow warning, not a hard failure, since the primary error is already
about to be reported). Existing error message and exit behavior for the
rmtree failure are unchanged.

Added a failing-first regression: install a workflow, monkeypatch
shutil.rmtree to raise OSError, and assert a clean existing error
message, the directory remaining (rmtree never actually deleted
anything), and the registry entry restored byte-for-byte identical
(including installed_at/updated_at) -- proving the fix bypasses add()
and doesn't re-stamp timestamps. Confirmed red (registry entry stayed
None) before the fix, green after.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix 4 current Copilot review findings on workflow run/registry/install

1. workflow run ownership check followed symlinks via Path.resolve()
   before mapping a direct YAML path back to its installed workflow ID.
   A symlinked .specify/workflows/<id>/workflow.yml resolved outside the
   tree, missed the ownership match entirely, and let the disabled-workflow
   guard be silently skipped while engine.load_workflow still followed the
   symlink. Now maps ownership from a lexically-normalized path (os.path.
   normpath, no symlink following) and explicitly refuses to run if the
   installed <id> directory or workflow.yml leaf is itself a symlink.
   Direct external workflow paths that don't match .specify/workflows/...
   are unaffected.

2. WorkflowRegistry._load() caught a read OSError and silently fell back
   to an empty in-memory registry, only blocking a later save(). Callers
   that only query is_installed()/get()/list() before writing a file
   (e.g. commands/init.py's bundled speckit install, which overwrites
   workflow.yml once is_installed() reports false) could act on that
   false-empty state and destroy real data before ever reaching save().
   _load() now raises OSError immediately so an unreadable registry fails
   closed at construction, before any query or side effect is possible.
   Added _open_workflow_registry() to give every CLI command a consistent
   clean-error boundary around registry construction.

3. _validate_and_install_local's mkdir/copy2 ran before the try/except
   that protected registry.add(); a copy2 failure (e.g. a truncating
   partial write on a reinstall) was not caught at all, so the existing
   backup-restore cleanup never ran and the prior working workflow.yml
   was corrupted with a raw traceback surfaced to the user. mkdir/copy2
   now run inside the same rollback-protected section as registry.add(),
   sharing one _cleanup_failed_install() helper.

4. workflow update's skip message claimed any non-catalog source was
   installed "from a local path or URL", which is wrong for the bundled
   speckit workflow (source: "bundled"). Message is now source-neutral.

Verified all 4 threads are current (not outdated) via GraphQL review
thread query on PR #3419, HEAD 812050a.

Tests: strict TDD per fix (red test proving each bug, minimal production
change, green). tests/test_workflows.py: 474 passed. Full suite: 3976
passed, 110 skipped. ruff check: all checks passed on touched files and
full src tree.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix disabled-workflow bypass via symlinked .specify project root

workflow run's ownership check derived registry_root/registered_id from
the lexical path, then checked the id directory and workflow.yml leaf for
symlinks -- but never checked .specify or .specify/workflows themselves
for that derived root. _reject_unsafe_workflow_storage only guards the
cwd's project_root, which can differ from the path-derived registry_root
(a direct path into an unrelated project, or that project's own .specify
being a symlink to an attacker-controlled tree). WorkflowRegistry's own
symlinked-parent handling silently substitutes an empty registry instead
of raising, so a query against it (is_installed/get returning "not
found") is not a safety signal a caller can rely on: with a symlinked
.specify, the disabled check saw no registry entry and let a disabled
workflow run anyway.

Fix: reject an unsafe .specify/.specify-workflows for the actual derived
registry_root before ever consulting the registry, reusing the existing
_reject_unsafe_dir helper already used by _reject_unsafe_workflow_storage.

Red-first end-to-end repro: victim project's .specify symlinked to an
attacker-controlled tree containing a disabled workflow entry, run
invoked with a direct path from an unrelated cwd -- confirmed the
disabled workflow executed (exit 0) before the fix, now refused cleanly.

Tests: tests/test_workflows.py 475 passed. Full suite: 3977 passed, 110
skipped. ruff check: all checks passed.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix raw exception leak in bundle remove primitive boundary

remove_bundle() had no exception handling around its component
removal loop, unlike install_bundle() which converts any raw
exception into a clean BundlerError. Since WorkflowRegistry now
fails closed (raises OSError) on an unreadable registry file,
and _WorkflowKindManager.__init__ constructs WorkflowRegistry
with no try/except, an unreadable workflow registry surfaced as
a raw OSError through remove_bundle(). The bundle_remove CLI
command only catches BundlerError, so the raw OSError propagated
uncaught, producing exit_code=1 with empty output instead of a
clean, actionable message.

Wrap remove_bundle()'s component loop in the same
try/except BundlerError: raise / except Exception: raise
BundlerError(...) from exc pattern already used by
install_bundle(), converting any raw exception at this shared
boundary. save_records() remains outside the try block, so a
failure still leaves the bundle's record untouched (no removal
side effects recorded).

Tests:
- tests/integration/test_bundler_install_flow.py::test_remove_converts_raw_installer_exception_to_bundler_error
  (function-level regression: a raw OSError from installer.is_installed
  must become a clean BundlerError, and the bundle record must survive)
- tests/contract/test_bundle_cli.py::test_remove_reports_clean_error_when_primitive_raises_raw_exception
  (CLI-level regression: `specify bundle remove` must print a clean
  actionable message and exit non-zero instead of raw/empty output)

Both tests were confirmed red beforehand: the raw OSError propagated
uncaught out of remove_bundle(), and the CLI-level CliRunner result
showed exit_code=1 with empty output.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix 8 current Copilot review findings on registry fail-closed, rollback orphans, backup-read boundaries, and Rich escaping

1. WorkflowRegistry._load(): a symlinked .specify/.specify/workflows
   parent (or a symlinked registry file) silently returned an empty
   registry instead of raising, unlike an unreadable-file read failure.
   A read-only caller (notably the bundler's remove path) querying
   is_installed() before ever writing could conclude an installed
   workflow is absent, skip removing it, then delete the bundle
   record -- leaving the workflow untracked but still on disk. Now
   raises OSError immediately, matching the existing unreadable-file
   fail-closed behavior.

2/8. _validate_and_install_local and _install_workflow_from_catalog:
   when the destination directory already existed but had no prior
   workflow.yml (e.g. a leftover empty dir), existed_before was True
   but there were no backup bytes to restore, so the rollback closure
   did nothing on a later failure -- leaving the newly copied/
   downloaded file behind. Both now unlink the newly created file in
   this case, restoring the pre-existing directory to its prior
   (empty) state.

3/4. Both install paths read the prior workflow.yml bytes (to seed
   the reinstall rollback) *before* any try/except boundary: a read
   failure on the existing file (e.g. a transient permission/FS
   issue) leaked a raw, unescaped OSError instead of the same clean
   CLI error used by every other failure branch in these functions.
   Both reads are now guarded by their own try/except OSError, with
   no writes attempted before the read succeeds (so there is nothing
   to roll back on this specific failure).

5. remove_bundle's exception-conversion message unconditionally
   claimed "No changes were recorded," even though a failure can
   occur after earlier components in the same bundle have already
   been removed from disk (save_records never runs on this path, so
   the record is left claiming the bundle fully installed). The
   message now reports how many components were already removed
   when that happened, instead of asserting no changes occurred.

6/7. workflow_remove's new post-registry-removal directory-failure
   error and its restore-failure warning interpolated workflow_dir
   and the exception values into Rich markup unescaped. A project
   path or OS error message containing Rich-markup-like brackets
   could be parsed as markup and hide/corrupt the displayed text.
   Both now use the existing _escape_markup helper, consistent with
   every other error path in this file.

Tests (tests/test_workflows.py unless noted):
- TestWorkflowRegistry::test_load_symlinked_workflows_dir_fails_closed_not_silently_empty (1)
- TestWorkflowCliAlignment::test_add_dev_fresh_install_into_preexisting_empty_dir_cleans_new_file (2)
- TestWorkflowCliAlignment::test_add_catalog_fresh_install_into_preexisting_empty_dir_cleans_new_file (8)
- TestWorkflowCliAlignment::test_add_dev_reinstall_backup_read_failure_gives_clean_error (3)
- TestWorkflowCliAlignment::test_add_catalog_reinstall_backup_read_failure_gives_clean_error (4)
- tests/integration/test_bundler_install_flow.py::test_remove_partial_failure_message_reflects_partial_state (5)
- TestWorkflowRemoveGuard::test_remove_directory_and_restore_failure_escapes_rich_markup (6/7)

All seven were confirmed red beforehand, matching each thread's
described failure mode exactly (silent empty registry instead of a
raise; orphaned new file left behind; raw unescaped OSError leaking;
a misleading "no changes were recorded" claim; Rich markup consuming
bracketed path/exception text). Also updated
test_registry_save_refuses_symlinked_parent, a pre-existing test that
asserted the symlinked-parent raise at add()/save() time -- it now
raises at construction instead, per fix #1, so the test was adjusted
to match without weakening its guarantee (still asserts no writes
occur under the symlinked target).

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix 3 current Copilot review findings: bookkeeping-aware BundlerError removal, bounded workflow downloads

1. bundle remove: BundlerError raised by the primitive installer itself
   (e.g. from a kind manager) bypassed the partial-removal bookkeeping
   message added previously via a bare `except BundlerError: raise`. Now
   routes through the same detail-construction logic as generic
   exceptions, so a mid-loop BundlerError after an earlier successful
   removal still reports that the project may be partially uninstalled,
   while a zero-removal BundlerError still reports "No components were
   removed." Both preserve the original exception message and chain
   `from exc`.

2/3. workflow add --from and catalog install/update downloads used
   unbounded `response.read()`, buffering the entire server-controlled
   body into memory before any size check, and trusted Content-Length
   alone where checked at all. Added a single shared
   `_read_response_within_limit()` helper reused by both call sites: it
   fails fast on an oversized declared Content-Length, and separately
   enforces the same cap while streaming in 64KiB chunks so a chunked or
   Content-Length-less response cannot bypass the limit by lying about or
   omitting its size. Chose 5 MiB as the cap: workflow YAML definitions
   are small step/metadata text, not binaries, so this is generous
   headroom against a malicious/misbehaving server without affecting any
   legitimate workflow definition. Both call sites already route any
   raised exception through their existing clean-error and rollback
   (`_cleanup_failed_install`) paths, so no additional error-handling
   plumbing was needed.

Tests: extended the shared `_FakeResponse` test helper (and 5 duplicate
per-test FakeResponse classes) to support `.read(amt)` chunked reads with
an internal cursor (backward compatible with existing bare `.read()`
callers) plus header simulation. Added red-first tests for: BundlerError
after partial removal reporting partial state, BundlerError with zero
removals reporting no changes, --from oversized-Content-Length rejection,
--from oversized-streamed-body-without-Content-Length rejection, and the
same two cases for the catalog install path (asserting no orphan
directory/registry mutation on rejection).

tests/integration/test_bundler_install_flow.py: 17 passed
tests/test_workflows.py: 485 passed
tests -q: 3992 passed, 110 skipped
ruff check: clean on all touched files

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix temp-file leak in workflow add --from and strengthen size-limit test assertions

workflow_add's --from download path opened a NamedTemporaryFile(delete=False)
-- which creates the file on disk immediately -- then wrote the size-limited
response body before assigning `tmp_path`. If `_read_response_within_limit`
raised (oversized declared Content-Length, or an over-cap streamed body with
no/understated Content-Length), the exception propagated out of the `with`
block before `tmp_path` was ever set, so the outer except handler had no
path to clean up: a 0-byte `.yml` temp file was left behind permanently on
every rejected/failed --from download. Fixed by assigning `tmp_path`
immediately after the file is opened (before the size-limited read/write),
and unlinking it in the except branch when set. Normal post-download cleanup
in the existing `finally: tmp_path.unlink(missing_ok=True)` is unchanged.

Verified (not assumed) the catalog install path has no equivalent leak: it
writes the response bytes directly to `workflow_file` inside `workflow_dir`
(no separate temp file), and any read/size-limit failure is already caught
by the existing `except Exception: _cleanup_failed_install()` handler, which
correctly restores a reinstalled file or removes a freshly-created directory.

While investigating, found the previous round's 4 size-limit tests were
false positives: `_read_response_within_limit`'s `max_bytes` parameter had
its default bound to `_MAX_WORKFLOW_YAML_BYTES` at function-definition time,
so monkeypatching the module attribute in tests had no effect on the
function's actual behavior -- the tests were passing because the oversized
mock bodies failed downstream YAML/id validation instead of the size check.
Fixed by resolving `max_bytes` from the module attribute at call time
(default `None`, resolved inside the function body) so tests can actually
override the effective limit, and strengthened all 4 tests' assertions to
match the specific size-limit error text (whitespace-collapsed to tolerate
Rich's line-wrapping), so they now prove the real code path fires.

Tests: added 2 red-first regression tests (oversized-streamed-body and
oversized-Content-Length --from downloads leave no leftover temp file,
verified against a scratch tempfile.tempdir), confirmed red (real 0-byte
file found) before the fix and green after. Strengthened the pre-existing
4 --from/catalog size-limit tests to assert on the actual error message
instead of generic exit-code/non-empty-output checks.

tests/test_workflows.py: 487 passed
tests -k bundler: 186 passed
tests -q: 3994 passed, 110 skipped
ruff check: clean on all touched files

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Harden workflow install/remove transactions with atomic staging

Addresses 5 Copilot review findings on HEAD b8269c8, all centered on
transaction integrity around workflow install/remove/registry writes,
following the atomic_write_json pattern already used in _utils.py:

1. WorkflowRegistry.save() now preserves the existing registry file's
   mode (e.g. 0640/0644) across a save instead of silently downgrading
   it to mkstemp's 0600 default; a brand-new registry still gets the
   secure 0600 default.

2. workflow_remove now stages the install directory out of the way via
   an atomic rename *before* the registry write, rather than deleting
   it directly with shutil.rmtree after the registry already claims it
   removed. This closes a real data-integrity gap: a partially-failed
   rmtree could no longer leave a damaged directory re-marked
   "installed" by the old manual restore-after-rmtree-failure code
   (now deleted -- it's structurally impossible to need it). A
   registry-write failure renames the staged directory back
   (guarded, with an explicit warning if the restore-back rename
   itself fails); a registry-write success is durable, so a later
   failure to delete the staged directory is now a warning (exit 0),
   not a contradictory "Error: Failed to remove" (exit 1) that used to
   claim failure while the registry already recorded success.

3. Local (--dev/--from/plain path) and catalog install/reinstall now
   write new content to a same-directory staging file and commit it
   onto the destination workflow.yml via a single atomic swap, instead
   of writing/downloading directly into the destination file. A prior
   file (reinstall) is renamed aside rather than overwritten in place,
   so it can be restored via rename -- never a content rewrite -- if
   registry.add() subsequently fails; a rollback failure is now
   explicitly reported as a warning instead of escaping unguarded and
   masking the original clean error. This also removes the need to
   read the prior file's bytes into memory before installing (that
   read-before-write step and its failure mode are now unreachable),
   and both local and catalog installs share the same four small
   helpers (_stage_workflow_file / _commit_workflow_file /
   _discard_staged_workflow_file / _rollback_committed_workflow_file,
   plus guarded wrappers) rather than duplicating the logic.

4. Updated a stale comment (workflow_run's ownership-guard rationale)
   that still described WorkflowRegistry._load() as silently
   substituting an empty registry; it now fails closed by raising
   OSError, which the comment now states plainly.

Tests: rewrote the two workflow_remove tests whose assertions encoded
the old (incoherent) rmtree-then-restore contract to instead prove the
new stage-then-commit contract (post-registry-success cleanup failure
is a warning+exit 0; pre-registry-success stage-restore failure is
guarded and escapes markup correctly). Rewrote the local/catalog
"backup read failure" tests, which tested a step the new design no
longer performs, into "restore-rename failure" tests proving the new
guarded rollback boundary. Added registry file-mode preservation tests.
All other existing install/remove/reinstall tests (save-failure
cleanup, pre-existing-empty-dir handling, early-failure-during-
reinstall parametrized cases, Rich markup escaping) continue to pass
unmodified against the new implementation.

Verified via GraphQL that all 5 threads are current (not outdated/
resolved) before fixing. Full suite: 3996 passed, 110 skipped. Ruff
clean on all touched files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Discard reinstall backup file after registry.add() succeeds

_commit_workflow_file() renames a prior workflow.yml aside to
workflow.yml.bak so it can be restored if registry.add() subsequently
fails. Neither the local install/reinstall path nor the catalog
install/reinstall path ever cleaned up that backup after a successful
registry.add() -- every successful reinstall permanently left a
workflow.yml.bak sibling, which later reinstalls would silently
overwrite/re-orphan.

Add a shared _discard_committed_backup_file() helper, called from both
success paths right after registry.add() durably succeeds (and before
the final "installed" message, preserving output ordering). A fresh
install (backup_file is None) is a no-op. A cleanup failure is reported
as a warning (exit 0), not a failure, since the install itself already
succeeded -- consistent with workflow_remove's post-commit cleanup
warning semantics.

Add red-first regression tests proving: (1) successful local reinstall
leaves no workflow.yml.bak sibling, (2) successful catalog reinstall
leaves no workflow.yml.bak sibling, (3) a cleanup failure on the backup
file after a successful reinstall reports a warning and still exits 0
with the registry correctly reflecting the new install.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Clean up freshly-created dest_dir when staging mkstemp fails

_stage_workflow_file() does dest_dir.mkdir(parents=True, exist_ok=True)
then tempfile.mkstemp(dir=dest_dir, ...). For a fresh install (no prior
directory), if mkdir succeeds but mkstemp then raises (disk
full/EMFILE/quota), the exception previously propagated straight past
both the local-install and catalog-install call sites without any
cleanup, leaving the newly-created empty workflow directory orphaned
on disk with no error indicating why.

Fix at the shared _stage_workflow_file() boundary instead of duplicating
cleanup at each call site: track whether this call created dest_dir: on
a mkstemp failure, remove that directory via a guarded rmdir (never a
broad rmtree, so any concurrently written content would be left
untouched) before re-raising the original OSError unchanged. A
pre-existing (reinstall) dest_dir is never touched by this cleanup,
and a cleanup failure is reported as its own warning without masking
the original error.

Add red-first regression tests proving: a fresh local install (--dev,
plain local path, --from) and a fresh catalog install both clean up the
orphaned directory on a simulated mkstemp failure, and a reinstall over
a pre-existing directory is left untouched by the same failure.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix installed-workflow ownership/disabled bypass and resume enforcement

Address 3 current Copilot review findings on the disabled-workflow guard
in `workflow run`/`workflow resume`:

- The lexical `.specify/workflows/<id>` ownership scan stopped at the
  first match scanning from the start of the path. A nested project
  living beneath an outer installed workflow's own directory tree (reusing
  the same segment names) was attributed to the wrong (outer) workflow
  and ID, gating the run on an unrelated workflow's disabled state.
  `_scan_for_workflow_owner` now scans from the end so the nearest
  (innermost) owner always wins.

- A path with no `.specify/workflows` segments of its own (e.g.
  `/tmp/alias.yml`) that is itself a symlink resolving *into* installed
  storage bypassed the disabled check entirely, since only the raw
  lexical path was inspected. `_resolve_installed_workflow_ownership` now
  additionally resolves the real path when the lexical scan finds no
  owner and re-runs the same scan against it, so an outward-pointing
  alias into a disabled workflow is caught too. Genuinely standalone
  external files (no symlink anywhere on the path) are unaffected.

- `workflow resume` bypassed the disabled check altogether: engine.resume()
  replays a persisted run directly from disk with no registry awareness.
  RunState now optionally persists `installed_workflow_id` and
  `installed_registry_root` at run start (set by workflow_run when the
  source resolved to an installed ID); `workflow_resume` pre-loads the
  run state and re-checks the registry's *current* disabled state before
  calling engine.resume(), mirroring workflow_run's own guard. Both new
  fields default to None via RunState.load()'s `.get()`, so runs from a
  direct/non-installed source, and any run persisted before this schema
  addition, resume exactly as before.

The ownership-mapping logic (previously inlined in workflow_run) is
extracted into `_resolve_installed_workflow_ownership` /
`_scan_for_workflow_owner` so both the lexical and resolved-path cases
share the same scan and the existing inward-symlink-component refusal.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Guard --from temp cleanup; drop redundant update rollback; mark POSIX-only tests

Two more current Copilot review findings, both in workflow_add/update:

- `workflow add --from`'s `finally: tmp_path.unlink(missing_ok=True)` ran
  unguarded after `_validate_and_install_local` had already committed the
  file and registry entry (success) or already raised its own clean
  `typer.Exit` (failure). An OSError from that cleanup unlink would
  surface as an unhandled failure even though the install itself
  succeeded. It is now wrapped in try/except OSError, printing a neutral
  warning that doesn't claim success or failure (the finally runs on both
  outcomes) instead of propagating.

- `workflow_update`'s per-item loop performed its own outer backup
  (`wf_file.read_bytes()`) and restore (`wf_file.write_bytes(backup)`)
  around `_install_workflow_from_catalog`, which is itself fully
  transactional (staged download, atomic rename-based commit, its own
  rollback on registry failure) and never leaves a raw OSError or a
  partially-written workflow.yml. The outer restore was therefore dead
  weight for its stated purpose, and — being an unguarded byte-level
  write — was itself an unnecessary place a second failure could truncate
  an already-safely-preserved file. Removed; the loop now only records
  success/failure.

Also marks 3 registry-save file-mode tests
(`test_registry_save_preserves_existing_file_mode`,
`test_registry_save_on_new_registry_uses_secure_default_mode`,
`test_registry_save_failure_preserves_file_on_disk`) as POSIX-only via
the repo's existing `skipif(sys.platform == "win32", ...)` pattern, since
they assert exact POSIX permission bits that don't hold on Windows.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Report possible partial changes on zero-removed bundle removal failure

The final Copilot review finding: `remove_bundle`'s zero-removed-components
error message claimed "No components were removed." even when the failing
installer component may have deleted files before raising -- prior review
rounds already established that DefaultPrimitiveInstaller's removal paths
are not atomic and can leave partial filesystem changes despite raising
before `result.uninstalled` is populated. The zero-count message is now a
conservative caution ("...but the failing component may have made partial
changes before raising, so the project may be partially uninstalled.")
instead of an unconditional claim of no side effects. The >0-removed path
(which already reports the confirmed partial list) is unchanged.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix workflow resume disabled-check bypass after project move/rename

RunState.installed_registry_root previously persisted the creation-time
absolute project path unconditionally whenever a run belonged to an
installed workflow. After the whole project directory was renamed or
moved, workflow_resume would open a WorkflowRegistry at that now
nonexistent path, get back an empty/default registry, and silently skip
the disabled-workflow check -- a paused run for a disabled workflow could
be resumed successfully from the new location.

Fix persists installed_registry_root only when the owning root genuinely
differs from the current project_root (true cross-project direct-file-
source invocations). The common same-project case now persists None and
is re-derived from the live project_root at resume time via a new
_resolve_run_owner_root() helper, which also falls back to project_root
if a stored root no longer exists on disk -- covering both the common
case transparently surviving project moves and the cross-project case
degrading safely if its owner project vanishes, rather than silently
skipping the disabled check.

Backward compatible: state files missing the new fields, and states with
a still-existing distinct cross-project root, behave unchanged.

Added regression tests:
- resume blocked after project moved then disabled at new location
- resume still works after project moved while workflow stays enabled
- cross-project registry root is still correctly honored when it exists
- resume falls back to current project's registry when a stored
  cross-project root no longer exists

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix silenced cleanup failures and malformed run-state type validation

Four fixes from Copilot review on HEAD 4f24735:

1. _discard_staged_workflow_file's fresh-install directory removal used
   shutil.rmtree(dest_dir, ignore_errors=True), so a genuine cleanup
   failure there could never reach _safe_discard_staged_workflow_file's
   warning -- an orphaned directory was left behind with zero report.
   Now removes only if dest_dir still exists and lets a real OSError
   propagate to the existing safe wrapper, which warns while the
   already-printed original install error remains primary.

2. _rollback_committed_workflow_file's fresh-install directory removal
   (post registry.add() failure) had the same ignore_errors=True gap;
   fixed identically so _safe_rollback_committed_workflow_file's warning
   can actually fire.

3. In the --from download-failure branch, tmp_path.unlink(missing_ok=
   True) was unguarded: if it raised (e.g. read-only tempdir), it
   replaced the original "Failed to download workflow" error with a raw
   unhandled OSError instead of a clean typer.Exit. Now guarded exactly
   like the later post-install finally cleanup: a cleanup failure prints
   a warning and the original download error is still reported cleanly.

4. RunState.load() trusted installed_workflow_id/installed_registry_root
   straight out of state.json with no type validation. A malformed value
   (int/list/dict/bool instead of str-or-null) would crash deep inside
   _resolve_run_owner_root or the registry lookup (TypeError building a
   Path, unhashable dict/list as a mapping key) instead of failing
   cleanly. Both fields are now validated as str | None during load,
   raising a clear ValueError that workflow_resume's existing ValueError
   boundary already converts into a clean CLI error with no traceback.
   Valid values (including the empty-string fallback already handled by
   _resolve_run_owner_root) continue to load unchanged.

Added red-first regression tests for each: staged-discard cleanup
warning, rollback cleanup warning, download-failure cleanup-vs-original-
error precedence, and parameterized malformed/valid run-state field
coverage.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add ValueError boundary to workflow status single-run lookup

QUALITY re-review flagged that fa410d3's new RunState.load() type
validation (malformed installed_workflow_id/installed_registry_root
raising ValueError) leaked as a raw unhandled traceback through
`workflow status <run_id>`, which only caught FileNotFoundError.
`workflow resume` already had the matching ValueError boundary.

Adds an `except ValueError as exc: console.print(f"[red]Error:[/red]
{exc}"); raise typer.Exit(1)` clause mirroring resume's exact pattern
(unescaped interpolation, consistent with the existing convention at
every other ValueError boundary in this file). FileNotFoundError
behavior and the no-run-id list-all-runs path are unchanged.

Added parametrized regression covering malformed installed_workflow_id/
installed_registry_root (int/list) via `workflow status`, plus
regressions locking in the unaffected FileNotFoundError and no-run-id
list-path behaviors.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: bound workflow step downloads

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

* fix: preserve workflow reinstall state

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

* fix: fail closed on workflow registry state

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

* fix: make workflow installs transactional

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

* fix: close workflow transaction races

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

* fix: clean up failed workflow transactions

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

* fix: clean up workflow removal state

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

* fix: guard workflow update transactions

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

* fix: harden workflow lifecycle edge cases

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

* fix: verify installed workflow ownership

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

* fix: isolate workflow rollback cleanup

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

* fix: preserve unique workflow backups

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

* fix: bind workflow staging to file descriptors

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

* fix: fail closed on corrupt workflow registry

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

* fix: bind workflow ownership and source identity

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

* fix(workflows): harden redirects and Windows tests

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

* fix(workflows): restore staged removals on serialization errors

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

* fix(workflows): preserve state across interrupted writes

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

* fix(workflows): harden resume ownership checks

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

* fix(workflows): validate persisted run state

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

* fix(workflows): validate origin and release metadata

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-07-14 08:03:33 -05:00
Ali jawwad a3bcd67925 docs(bundles): document --integration on 'bundle update' (#3271) 
The 'bundle update' command accepts --integration (verified via
'specify bundle update --help' and the command signature), used as the
integration override when the project's active integration can't be
detected. The Update Bundles options table in reference/bundles.md
omitted it, listing only --all and --offline — unlike the install/init
tables which already document --integration. Add the missing row.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
 2026-07-13 09:33:39 -05:00
Quratulain-bilal a10fd2f355 docs: document copilot skills mode (--skills) and markdown deprecation (#3313) 
* docs: document copilot skills mode (--skills) and markdown deprecation

as of #3256 (v0.12.3) the copilot integration supports a skills mode via
`--integration-options "--skills"`, and installing without it warns that the
legacy markdown default is being phased out. this was undocumented:

- the copilot row in the supported-agents table had an empty notes cell while
  other skills-capable agents describe their behavior there.
- `--skills` was missing from the integration-specific options table (only
  generic and kimi were listed).

fill both. wording matches the code: skills scaffold as speckit-<name>/SKILL.md
under .github/skills/ and are invoked as /speckit-<name>; without the flag the
install emits the deprecation warning from _warn_legacy_markdown_default().

fixes #3300

* docs: describe copilot default as legacy markdown mode (.agent.md + .prompt.md)

the copilot rows said the default installs .agent.md files, but the default
scaffold also writes companion .prompt.md files under .github/prompts/. also
reworded to 'legacy markdown mode' to match the deprecation warning users
actually see and to avoid ambiguity, since skills are markdown too.

* docs: spell out copilot legacy markdown paths and use <command> in copilot rows

address the follow-up copilot review: name where the default scaffold writes
files (.github/agents/*.agent.md plus .github/prompts/*.prompt.md and a
.vscode/settings.json merge), and switch speckit-<name> to speckit-<command>
to match the rest of the table. verified all three paths against the copilot
integration source.

* docs: use --integration-options="..." form in copilot notes cell

match the equals form the rest of the doc uses (generic row, the options
table, and the install example) so readers don't mistake the quotes for part
of the value. addresses copilot review feedback.
 2026-07-13 09:10:14 -05:00
Dyan Galih 13d2cca154 Docs: Document missing CLI flags and integrations (#3182) 
* docs: document missing flags and integrations

* docs: remove invalid --refresh-shared-infra from upgrade command

* docs: address PR feedback for extension and integration flags

* docs: reorder extension add options to match CLI help
 2026-07-08 07:49:42 -05:00
Quratulain-bilal 288bd679f3 docs: drop stale kimi KIMI.md->AGENTS.md migration note (#3291) 
* docs: drop stale kimi KIMI.md->AGENTS.md migration note

#3097 made the agent-context extension a full opt-in and removed the
KIMI.md -> AGENTS.md context migration from the kimi integration
(_migrate_legacy_kimi_context_file and the context_file handling are
gone). kimi's --migrate-legacy now only moves the skills directory. two
lines in the integrations reference still promised the removed context
migration; drop that clause so the docs match the code.

* docs: clarify kimi legacy migration is skill naming, not directory names

address review: the parenthetical said 'dotted->hyphenated directory
names', but the migration is about skill naming (speckit.xxx ->
speckit-xxx), matching the module docstring. reword to match.
 2026-07-02 08:40:30 -05:00
Pascal THUET 490566847c feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver (#3186) 
* feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver

The shell resolver honors SPECIFY_INIT_DIR (#2892), but the Python CLI did
not: it resolved the project as Path.cwd() + a .specify/ check and never read
the override. So setup-plan.sh respected it while `specify integration install`
ignored it, and you still had to cd into the member project.

Route project resolution through a shared _resolve_init_dir_override() that
applies the shell resolver's validation rules (relative to cwd, must exist and
contain .specify/, hard error, no fallback, same error strings). It's wired into
_require_specify_project() — the chokepoint for every project-scoped subcommand
(integration/extension/workflow/preset/...) — and the `workflow run <file>`
standalone path, which re-applies its symlinked-.specify guard on the override
branch too. init is unchanged: it creates .specify/, so the must-pre-exist rule
doesn't apply.

The resolver canonicalizes symlinks via Path.resolve() while the shell keeps the
logical path; they agree for non-symlinked paths (documented in the resolver).

Tests in tests/test_init_dir_cli.py mirror the strict cases from test_init_dir.py
through the CLI; conftest now strips SPECIFY_* for the whole suite so a stray
export can't perturb the now-env-reading resolver. Docs note the CLI applies the
same rules.

Discussion: github/spec-kit#2834

(Disclosure: I used an AI coding agent to audit the call sites and resolver,
draft the change, and run an adversarial code review; reviewed by me.)

* fix(cli): honor SPECIFY_INIT_DIR for bundle commands

Assisted-by: Codex (model: GPT-5, autonomous)

* fix(bundler): refuse symlinked .specify on the SPECIFY_INIT_DIR override path

find_project_root refuses a symlinked .specify (following it could read/write
outside the tree, and a test pins that), but the SPECIFY_INIT_DIR override added
for bundle commands returned early and skipped that guard:
_resolve_init_dir_override validates .specify with is_dir(), which follows
symlinks. So `specify bundle` accepted via the override a layout the cwd path
rejects. Re-check the override result with the same guard, plus a regression test.

(Disclosure: found via an AI code review and fixed with an AI coding agent;
reviewed by me.)

* fix(cli): keep SPECIFY_INIT_DIR strict for bundles

Treat an explicit symlinked SPECIFY_INIT_DIR project as a hard bundle error instead of returning no project, which could initialize the current directory. Align the docs with the actual unset resolver behavior.

Assisted-by: Codex (model: GPT-5, autonomous)

* docs(core): note symlinked .specify handling differs across CLI surfaces

A symlinked .specify is followed by integration/extension/workflow (matching the
shell resolver) but refused by bundle and workflow run <file> (write
confinement). Document the asymmetry so it reads as intentional.

(Disclosure: AI-assisted; reviewed by me.)

* docs(core): reframe symlinked .specify note around the override invariant

Per maintainer feedback on #3186: SPECIFY_INIT_DIR relocates where the project
is, not how a surface treats symlinks. Each surface keeps its cwd-path stance
(write surfaces refuse a symlinked .specify, read/config surfaces follow it),
so the split is one policy relocated, not an inconsistency.

* docs: address Copilot review on resolver docstrings

- _project.py: the error messages "mirror" the shell wording rather than
  "match" it (the CLI renders a Rich `Error:` line, the shell a plain `ERROR:`).
- find_project_root: document that honoring SPECIFY_INIT_DIR when start is None
  can raise typer.Exit / BundlerError, so the Path | None signature isn't
  surprising to direct callers.

* docs(bundler): note require_project_root inherits the override raise behavior

find_project_root can raise typer.Exit / BundlerError under the SPECIFY_INIT_DIR
override (start=None); require_project_root inherits that, so document it
alongside its own BundlerError-on-missing-project.

* docs: clarify symlinked project root behavior

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Address SPECIFY_INIT_DIR review feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Route workflow JSON errors to stderr

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
 2026-07-01 15:55:18 -05:00
Ben Buttigieg 36501d459f chore: retire Roo Code integration — extension shut down (#3167) (#3212) 
* chore: retire roo integration — extension shut down (#3167)

Remove the Roo Code integration after the extension was shut down: subpackage,
registry entry, catalog entry, docs, tests, and issue-template options.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: remove stale Roo Code mention in upgrade guide

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* chore: remove leftover Roo Code references after merge

Drop roo from presets/ARCHITECTURE.md example and the agent-context
defaults map; these came in from main and were flagged by review.

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-06-30 10:24:04 -05:00
Ben Buttigieg c47dd2b812 chore: retire Windsurf integration — absorbed into Cognition Devin (#3168) (#3213) 
* chore: retire windsurf integration — absorbed into Cognition Devin (#3168)

windsurf.com now permanently redirects to devin.ai/desktop following
acquisition. Remove subpackage, registry/catalog entries, docs, and tests;
re-point sample-agent test fixtures to Kilo Code.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: remove stale Windsurf support references

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* docs: fix Kilo Code command path in upgrade guide

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* chore: align integration lists after rebase

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

* docs: align kilocode example with runtime behavior

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-06-30 08:49:49 -05:00
Ben Buttigieg 28a38af6c1 chore: retire iflow integration — product discontinued (#3166) (#3211) 
Remove the iFlow CLI integration whose product was shut down: subpackage,
registry entry, catalog entry, docs, tests, and issue-template options.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-06-30 07:30:52 -05:00
Ben Buttigieg 8215f3308b docs(codebuddy): fix dead install links and CodeBuddy capitalization (#3172) (#3216) 
* fix(codebuddy): repoint install_url to codebuddy.cn (#3172)

The codebuddy.ai domain no longer resolves; CodeBuddy consolidated onto
codebuddy.cn (Tencent). Update install_url and docs links to
https://www.codebuddy.cn/cli (verified live).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: use canonical 'CodeBuddy' capitalization in installation prereqs

Address Copilot review: the link text read 'Codebuddy CLI' while the rest of
the docs and the integration metadata use 'CodeBuddy'.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-06-30 07:29:33 -05:00
Quratulain-bilal 00f6a80201 docs: document integration catalog subcommands (#3206) 
* docs: document integration catalog subcommands

the integration reference omits the 'specify integration catalog'
subcommand group (list/add/remove) that exists in code, while the
extension, preset, and workflow references all document their catalog
equivalents. add a catalog management section matching that structure.

* docs: address review feedback on integration catalog section

- catalogs are consulted by the discovery commands (search/info), not
  install; install resolves from the built-in registry
- 'catalog list' shows project sources as removable only when configured,
  otherwise active sources are non-removable
 2026-06-30 06:13:17 -05:00
Noor ul ain 9dfef8629e docs: document integration search/info/scaffold subcommands (#3174) (#3194) 
* docs: document integration search/info/scaffold subcommands (#3174)

docs/reference/integrations.md omitted three subcommands that exist in
code, breaking parity with the extension/preset/bundle/workflow
references which all document their search/info equivalents.

Added sections for:
- `specify integration search [query]` (--tag, --author)
- `specify integration info <integration_id>`
- `specify integration scaffold <key>` (--type: markdown/skills/toml/yaml)

Content mirrors the command docstrings, arguments, and options in
src/specify_cli/integrations/_query_commands.py and _scaffold_commands.py.

Fixes #3174.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-06-29 16:52:01 -05:00
Ben Buttigieg b1bd9180ca fix(goose): repoint install_url and docs to goose-docs.ai (#3171) (#3215) 
* fix(goose): repoint install_url and docs to goose-docs.ai (#3171)

Goose moved to the Agentic AI Foundation; docs moved from block.github.io/goose
to goose-docs.ai. Update install_url and the docs reference link.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(goose): restore table column alignment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-06-29 16:43:06 -05:00
Ali jawwad 5367f69f6c docs(workflows): add the built-in 'init' step type to the Step Types table (#3234) 
The Step Types table in docs/reference/workflows.md listed command, prompt, shell, gate, if, switch, while, do-while, fan-out, and fan-in, but omitted 'init' -- which IS a registered built-in (workflows/__init__.py _register_builtin_steps registers InitStep) and is documented in steps/init/__init__.py as bootstrapping a project (equivalent to 'specify init'). Add the missing row so the reference matches the registry.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-06-29 15:08:49 -05:00
siC@r10-mw b7e67f55bf Add community bundle submission path (#3162) 
* Add community bundle submission path

* Address bundle submission review feedback

* Align bundle submission triage label

* Clarify bundle submission review scope

* Clarify community bundle catalog listing
 2026-06-26 16:56:34 -05:00
Dyan Galih 465d29910e Docs: add cline and zcode to multi-install-safe table (#3180) 
Fixes #3175
 2026-06-26 12:21:38 -05:00
Dyan Galih 916e29b27b Docs: document missing flags --force and --refresh-shared-infra (#3179) 
* Docs: document missing flags --force and --refresh-shared-infra

Fixes #3177

* Address review: Reorder flags to match CLI help output
 2026-06-26 12:20:51 -05:00
Si Zengyu 1add20341d fix(extensions,presets,workflows): resolve private GHES release assets via /api/v3 (#3157) 
* feat(auth): add github_provider_hosts() to enumerate GHES hosts from auth.json

Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous)

* fix(extensions): resolve GHES release assets via /api/v3

Generalizes resolve_github_release_asset_api_url to GitHub Enterprise
Server hosts (gated by auth.json github hosts), fixing private GHES
extension/preset downloads. github/spec-kit#3147

Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous)

* fix(extensions,presets): pass auth.json github hosts into release resolver

Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous)

* docs(auth): document GHES private catalog + release-asset auth

Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous)

* fix(presets,workflows): pass auth.json github hosts into remaining release resolvers

Wires preset add --from and workflow add through github_provider_hosts()
so private GHES release assets resolve via /api/v3 there too. github/spec-kit#3147

Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous)

* test(presets): use module-level io.BytesIO in GHES preset test

Addresses Copilot review on PR #3157: drop unnecessary __import__("io")
in test_preset_add_from_ghes_release_url_resolves_via_api_v3 since io is
already imported at module level.

* fix(github-http): pass through GHES asset API URLs by path shape

Addresses Copilot review on PR #3157. A direct GHES /api/v3 release asset
URL was only returned as already-resolved when its host was in the
allowlist; otherwise the resolver returned None and the caller downloaded
the same URL without 'Accept: application/octet-stream', fetching JSON
metadata instead of the binary.

Gate the passthrough on path shape alone, mirroring the github.com case.
This is safe: passthrough returns the input URL unchanged and the caller
fetches it either way, so no new request to an arbitrary host is induced;
the token stays independently gated by auth.json in open_url. The
allowlist remains the anti-SSRF gate on the tag-lookup resolving path.

Add test_passthrough_for_unlisted_ghes_api_asset_url.
 2026-06-25 10:44:30 -05:00
WOLIKIMCHENG 7624dd6582 Update preset composition strategy reference (#3143) 
* docs: update preset composition strategy reference

* docs: clarify preset command composition timing

* docs: clarify preset command reconciliation timing

* docs: clarify preset file resolution behavior

* docs: clarify preset command reconciliation wording

---------

Co-authored-by: root <kinsonnee@gmail.com>
 2026-06-25 10:13:14 -05:00
meymchen dc840f07d0 feat(integration): update Kimi integration for Kimi Code CLI (#2979) 
* feat(integration): update Kimi integration for Kimi Code CLI

Update the Kimi integration to target the new Kimi Code CLI
(MoonshotAI/kimi-code) layout:

- Change skills directory from .kimi/skills/ to .kimi-code/skills/
- Change context file from KIMI.md to AGENTS.md
- Extend --migrate-legacy to move old .kimi/skills/ installs and
  migrate KIMI.md user content to AGENTS.md
- Clean up leftover legacy .kimi/skills/ directories on teardown
- Update devcontainer installer to @moonshot-ai/kimi-code
- Update docs and tests

Relates to #1532

* fix(integration): align Kimi dispatch and harden legacy migration

- Override build_command_invocation to emit /skill:speckit-<stem>
  so dispatched commands match Kimi Code CLI's native slash syntax.
- Skip symlinked .kimi/skills directories during legacy migration
  and teardown to avoid operating on files outside the project.
- Remove kimi from the multi-install-safe integrations table.
- Add tests for command invocation and symlink safety.

* fix(integration): resolve custom context markers in Kimi legacy migration

Use IntegrationBase._resolve_context_markers() when migrating legacy
KIMI.md content so that projects with customized context_markers in
.specify/extensions/agent-context/agent-context-config.yml have the
managed section stripped with the correct markers instead of the
hard-coded defaults.

Adds a test verifying custom markers are respected during
--migrate-legacy.

* fix(integration): harden Kimi legacy migration against symlinked paths

* fix(kimi): guard symlinked SKILL.md during migration and teardown

* docs(kimi): mention KIMI.md→AGENTS.md migration in --migrate-legacy help

The --migrate-legacy help text listed only the skills directory move and
dotted→hyphenated renaming, but the flag also migrates KIMI.md user content
into AGENTS.md. Align the help with the actual behavior, docs, and tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(kimi): validate legacy migration destination; clarify docstrings

Address Copilot review feedback on PR #2979:

- setup(): gate skills migration on _is_safe_legacy_dir(new_skills_dir)
  as well as the source. base setup() already rejects a destination that
  escapes the project root, but an in-tree symlinked .kimi-code/skills
  (e.g. -> .) could still misdirect the move; this gives the destination
  the same symlink-component protection as the source.
- _migrate_legacy_kimi_dotted_skills: rewrite docstring as a compatibility
  shim describing same-path delegation to _migrate_legacy_kimi_skills_dir.
- test_presets: clarify that the dotted-skill test exercises legacy naming
  under the current .kimi-code/ base, not the legacy .kimi/ location.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(kimi): harden legacy KIMI.md→AGENTS.md context migration

- Skip context-file migration when the agent-context extension is
  disabled, matching upsert/remove_context_section opt-out behavior so
  an opted-out project's KIMI.md/AGENTS.md are left untouched.
- Safely skip (instead of raising) on filesystem edge cases: unreadable
  or non-UTF-8 KIMI.md, and AGENTS.md existing as a non-file/unwritable.
- Refuse to migrate a corrupted managed section (single marker, or end
  before start) so a partial managed block is never copied into
  AGENTS.md; KIMI.md is preserved for manual repair.

Add regression tests for all three cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Approve fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* chore(kimi): revert CHANGELOG.md edit (auto-generated)

The CHANGELOG is generated from merged PR titles, so a hand-written entry
is redundant; it was also placed under the already-released 0.10.2 section,
which would make those release notes historically inaccurate. Revert to
match main per maintainer feedback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(kimi): skip symlink-safety tests when symlinks are unavailable

The Kimi legacy-migration safety tests create symlinks to assert that
migration/teardown never follow them out of the project. Symlink creation
fails on Windows without the create-symlink privilege and in some restricted
CI sandboxes, so these tests errored during setup instead of skipping.

Wrap every symlink_to() call in a shared _symlink_or_skip() helper that
pytest.skip()s on OSError/NotImplementedError, matching the guard pattern
already used by one of these tests. Verified on Windows: the 6 symlink tests
now skip cleanly (51 passed, 6 skipped) instead of erroring.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(kimi): reject symlinked skills destination before install

Add a destination symlink pre-check in KimiIntegration.setup() before
super().setup() writes any SKILL.md. The base class only rejects a
destination that escapes project_root after resolve(), so an in-tree
symlinked .kimi-code/.kimi-code/skills (e.g. `-> .`) would still
misdirect writes into an unintended in-tree location (./skills/).

Extract the symlink-component walk into a shared _has_symlinked_component()
helper and reuse it from _is_safe_legacy_dir(). Add a regression test.

Also clarify that --migrate-legacy only migrates KIMI.md -> AGENTS.md when
the agent-context extension is enabled, in the CLI help text and the
integration docs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Refactor formatting and simplify logic in Kimi integration

* fix(kimi): reject symlinked target dir during legacy skills migration

When the migration destination already exists, guard against a symlinked
(or non-directory) target_dir before comparing SKILL.md bytes, so the
comparison never follows a link outside the project root. Also skip a
missing/non-file target SKILL.md explicitly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-06-24 15:22:08 -05:00
Zied Jlassi f846d6526c fix(workflows): validate requires keys and reject phantom permissions gate (#3079) 
* fix(workflows): validate requires keys and reject phantom permissions gate

A workflow's `requires` block was parsed but its keys were never
validated, so a typo or an unsupported key was silently ignored. Most
importantly, authors could write `requires.permissions.shell: true`
expecting a runtime capability gate — but no such gate exists: a `shell`
step always runs with the user's privileges. The declaration gave a
false sense of sandboxing.

`validate_workflow` now accepts only the recognised keys
(`speckit_version`, `integrations`, `tools`, `mcp`) and rejects anything
else, with an explicit error for `requires.permissions` pointing authors
to `gate` steps for approval. Docs and the model comment are updated to
state that `requires` is advisory, not a security boundary.

- Reject non-mapping `requires`, unknown keys, and `requires.permissions`
- Clarify workflows reference + PUBLISHING.md shell-step guidance
- Tests for valid keys, non-mapping, unknown key, and permissions

Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>
Assisted-by: AI

* fix(workflows): address review feedback on requires validation

Follow-up to the review on #3079:

- Guard `requires` validation on `is not None` instead of truthiness so a
  falsy non-mapping value (e.g. `requires: []` or `requires: ''`) is
  reported as an error instead of being silently skipped; `requires:`
  (YAML null) is still treated as an omitted block. Add a regression test.
- Reword the workflows security note so `requires.permissions` is shown
  as rejected/unsupported rather than as a valid example of `requires`.
- Standardize on US spelling (`_RECOGNIZED_REQUIRES_KEYS`, "recognized")
  to match the surrounding code and ease searching.
- Tighten the permissions-rejection test to assert on specific message
  markers (`requires.permissions` and the `gate` guidance) so it fails if
  the validation path or wording drifts.

Assisted-by: AI
Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com>

* fix(workflows): scope requires validation to workflow keys (drop tools/mcp)

tools and mcp belong to the bundle manifest requires schema (bundler/models/manifest.py, resolved in bundler/services/resolver.py), not the workflow requires validated here. Drop them from _RECOGNIZED_REQUIRES_KEYS and revert the PUBLISHING.md claim that this PR had introduced, so workflow requires only recognizes speckit_version and integrations.

This keeps the existing docs accurate and resolves the inline doc-consistency review comments.

Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>

* refactor(workflows): type WorkflowDefinition.requires as Any pre-validation

self.requires holds the raw parsed value, which before validate_workflow()
runs may be a non-mapping (None for a bare 'requires:', a list for
'requires: []', etc.). Annotating it dict[str, Any] was misleading for
editors/type-checkers; use Any and document that validate_workflow() enforces
the mapping shape.

Addresses Copilot review feedback on engine.py.

Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>

* fix(workflows): reject YAML-null requires: as a non-mapping

Address Copilot review: validate requires the same way as inputs. A
bare requires: parses as YAML null and was previously treated as an
omitted block, which is inconsistent with inputs and lets a stray
requires: line be silently ignored.

Drop the is-not-None guard and check isinstance(..., dict) directly: an
omitted block still defaults to {} (valid), but a present-but-non-mapping
value -- YAML null, [] or '' -- is now an authoring error that surfaces.

Tests: add YAML-null rejection + an omitted-is-still-valid guard test.
Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>

---------

Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>
Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com>
 2026-06-24 14:49:43 -05:00