mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
4871b485f97c7fa452ec58eba325d87536c55c34
38
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
1a44a6aa08 |
fix(presets): skip an unreadable restore source in preset remove (#4020)
* fix(presets): skip an unreadable restore source in `preset remove` `_unregister_skills_in_dir` restores each preset-owned SKILL.md from a core command template or an extension source. Both of those reads were bare `read_text(encoding="utf-8")` calls, so a project-owned override in `.specify/templates/commands/` that exists but cannot be read or decoded raised a raw `UnicodeDecodeError`/`OSError` straight out of `PresetManager.remove()`, which has no handler for it — `specify preset remove` dies with a traceback. Every other failure in this loop degrades with `continue`: an unsafe registry name, a missing skill subdirectory, a foreign owner. Sibling reads of the very same directory are already guarded — `_infer_legacy_skill_ provenance` and `_delete_agent_preset_skills` both wrap their SKILL.md read in `except (OSError, UnicodeDecodeError): continue`, and the read inside `_substitute_core_template` was just given the same boundary in #3961. The two restore reads were the remaining gap. `continue` is the right recovery here rather than falling through: the `else` branch below removes the skill outright, so treating an unreadable source as "no source" would delete a user's skill at exactly the moment its replacement cannot be generated. Skipping leaves the skill in place and keeps it out of the returned `mutated_names`, so callers don't record a restore that never happened. Two regression tests, one per exception arm: a non-UTF-8 core template, and a mocked `PermissionError` so the `OSError` half is also covered under privileged CI where permission bits aren't enforced. Both assert the skill survives untouched and is not reported as mutated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): warn when a skill keeps preset content after a failed restore Review follow-up on two points. Surface the skipped restore. Skipping is still the correct recovery — the alternative branch deletes the skill — but it was silent, and it is a partial removal: `remove()` goes on to delete the preset directory and the registry entry, while this `SKILL.md` keeps the removed preset's content, and leaving the name out of `mutated_names` also keeps it out of reconciliation, so nothing retries it. Both arms now emit a warning naming the skill, the unreadable source, and the exception, and pointing at the re-run that refreshes it once the file is fixed. `warnings.warn` matches how the surrounding code reports non-fatal degradation (the reconciliation failures in `remove()`/`install_from_directory`, the unreadable core template in `_substitute_core_template` from #3961). Cover the extension arm. A skill backed by an installed extension never reaches the core-template read, so the two branches can regress independently and both prior tests exercised only the core one. `test_unregister_skills_in_dir_unreadable_extension_source_skips` installs an extension whose command file is non-UTF-8 and asserts the skill survives byte-for-byte and is absent from `mutated_names`. Verified it raises the raw `UnicodeDecodeError` against unpatched source. The two existing tests now assert the warning via `pytest.warns` so dropping it fails the suite. pytest tests/test_presets.py -> 583 passed, 2 skipped, 7 failed; the 7 are the pre-existing Windows symlink tests that need elevation, unchanged from main. ruff check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
16cfab7724 |
feat(presets): resolve constitution templates at command time (#3984)
* feat(presets): resolve constitutions at command time Gate install-time constitution materialization behind the constitution-sync preset while preserving one-time init seeding and authored-file safeguards. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84 * fix(presets): emit composed template content Add a machine-readable preset resolve mode backed by PresetResolver.resolve_content and require the constitution command to consume it. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84 * fix(presets): unify runtime template composition Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84 * fix(presets): secure runtime template resolution Align runtime resolution across script variants, validate registry path components, and honor canonical extension ordering and convention paths. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): align runtime priority semantics Normalize and tie-break preset priorities consistently across script variants, and preserve template bytes when Python materializes generated files. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): stop at effective template base Avoid parsing irrelevant lower layers once resolution reaches a replace base, and decode raw bytes so Python preserves source line endings. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): align extension template resolution Support root-level extension templates across runtime resolvers, fail safely when Bash cannot parse an extension registry, and validate requested templates in every prerequisite output mode. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): resolve dotted command identifiers Route safe dotted names through command resolution, correct traversal coverage, and make Windows CI text decoding explicit. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Avoid orphan feature directories on template errors Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Align malformed preset manifest handling Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * Complete runtime resolver parity Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * Fail closed on resolver input errors Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * Force UTF-8 and full manifest validation Force UTF-8 decoding for registry and manifest reads in the Bash and PowerShell embedded-Python parsers so resolution no longer depends on the process locale, and validate every manifest template entry's required fields, type, and strategy consistent with the canonical PresetManifest. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * Fail closed on empty manifests and corrupt registries Reject manifests missing the provides/templates sections or declaring an empty template list in all three runtime resolvers, matching the canonical PresetManifest which treats those as invalid instead of silently degrading a composing layer to a convention `replace` lookup. Make a corrupt or unreadable extension registry fail closed in Bash, PowerShell, and Python instead of swallowing the error and treating every on-disk extension directory as unregistered-and-enabled, which could activate a disabled extension. Read the preset and extension registries as explicit UTF-8 in the PowerShell resolver so priority/enabled-state decoding no longer depends on the process code page under Windows PowerShell 5.1. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * fix(presets): fail closed when extension registry is not a regular file The Bash and Python resolvers used is_file()/`-f` to gate reading the extension `.registry`, which returns false for a directory or a broken symlink at that path. In those cases the resolvers treated the registry as absent and scanned every on-disk extension directory as unregistered and enabled — a fail-open path. Detect any filesystem entry at the registry path (including broken symlinks) and reject unless it is a readable regular file. PowerShell now rejects a non-leaf entry explicitly for parity. Adds directory- and broken-symlink parity regressions. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * fix(presets): fail closed on corrupt registry in canonical resolver and PowerShell Two remaining fail-open paths for an invalid extension registry: - The canonical PresetResolver enumerated extensions through ExtensionRegistry, whose _load() normalizes a corrupt or unreadable registry to an empty mapping. The directory scan then admitted every on-disk extension directory as unregistered-and-enabled, so a corrupt registry could still supply constitution content at init and through constitution-sync materialization. Add a non-invasive is_corrupt() probe (recovery behavior for install/enable/disable is unchanged) and raise from _get_all_extensions_by_priority() when the registry exists but is invalid. _load() now also recovers from OSError/UnicodeDecodeError so a directory or unreadable registry no longer crashes construction. - The PowerShell resolver gated the registry read with Test-Path, which returns false for a dangling symlink on Windows, letting a broken .registry symlink bypass the guard and enable every on-disk extension. Detect the entry via directory enumeration (which observes a broken symlink) and reject it unless it is a readable regular file. Adds canonical corrupt/directory-registry regressions and extends the broken-symlink parity test to PowerShell. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c * fix(presets): detect dangling registry symlink in ExtensionRegistry.is_corrupt is_corrupt() gated on Path.exists(), which follows symlinks and returns False for a dangling .registry symlink — so the canonical PresetResolver treated it as an absent registry and fell back to scanning every on-disk extension directory as unregistered-and-enabled, reopening the fail-open path this guard closes. Detect lexical existence with os.path.lexists and require a regular file before parsing, so a broken symlink (or directory) is reported corrupt and resolution fails closed. Adds a canonical broken-symlink regression alongside the directory case. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84 Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c |
||
|
github-actions[bot]GitHubCopilot Appgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>Manfred Riem |
1a60d1b6b8 |
[bug-fix] Fix preset-wrap-drops-argument-hint: inherit argument-hint from core template (#3996)
* Fix preset-wrap-drops-argument-hint: inherit argument-hint from core Apply the remediation from the bug assessment on issue #3991. Extend the inheritance allowlist in _register_skills and _compose_layers to include 'argument-hint', so wrap-strategy presets that omit this key will inherit it from the core template rather than silently dropping it and risking its value being leaked into description. Refs #3991 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(presets): guard wrap argument-hint inheritance for unmapped command The existing regression test for #3991 wraps `speckit.specify`, whose stem is in Claude's ARGUMENT_HINTS map. The string-injection fallback in post_process_skill_content re-adds argument-hint even when wrap composition drops it, so that test passes with or without the inheritance fix and does not actually guard the regression. Add a parallel test that wraps an extension-like command (`speckit.myfeature`) absent from ARGUMENT_HINTS, so the wrap-composition inheritance is the only path that can carry argument-hint into the SKILL.md. This test fails without the fix and passes with it. Refs #3991 Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 970babe2-48cd-4c41-adae-0282d879a9ce --------- 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: 970babe2-48cd-4c41-adae-0282d879a9ce |
||
|
chelsealongandGitHub
|
684b3d8e05 |
feat(extensions): accept provides.templates and provides.scripts in manifest (#4012)
* feat(extensions): accept provides.templates and provides.scripts in manifest Extensions could only formally declare commands under `provides` (plus config/hooks/events); templates and scripts shipped by an extension were picked up purely by filename convention, with no id, description, or metadata. Add optional `provides.templates` and `provides.scripts` sections to the extension manifest schema, mirroring the preset template shape minus an authorable `strategy` (extension artifacts always resolve as replace, so a present `strategy` key is now a validation error rather than a silently accepted no-op). ExtensionManifest gains `templates`/`scripts` properties so tooling can enumerate an extension's declared artifacts directly from the manifest. An extension may now satisfy the "must provide something" rule with only a template or script, not just a command/hook/event. Addresses the manifest-schema portion of #4010; resolver authoritative-vs-convention precedence for these new sections is left for a follow-up. * fix(presets): wire extension-declared templates/scripts into resolver collect_all_layers only consulted ExtensionManifest for command resolution, leaving provides.templates/.scripts purely decorative -- a declared entry whose file didn't sit at the conventional path was validated but never resolved. Extend the existing manifest-fallback branch to cover template_type "template" and "script" the same way it already does "command": convention lookup first, manifest lookup as fallback so undeclared on-disk files keep resolving unchanged. * fix(presets): make extension manifest lookup authoritative over convention Copilot review on #4012 found the manifest-declared template/script lookup was gated on convention lookup missing first, so a stale conventional file could shadow a declared entry at a non-conventional path, and resolve() never consulted the manifest at all (only collect_all_layers() did). Add a shared _extension_manifest_declared_template() helper and check it before convention-based lookup in both resolve() and collect_all_layers(), mirroring the preset manifest precedence. Also update EXTENSION-DEVELOPMENT-GUIDE.md, which still claimed provides only supports commands and required a command or hook. * fix(presets): stop resolving symlinks in extension manifest candidate path _extension_manifest_declared_template() resolved ext_dir/rel_path before returning it, which follows symlinks in ext_dir's ancestors (e.g. macOS's symlinked tmp dir) and diverges from the unresolved paths convention-based lookup returns for the same directory. Resolve only for the traversal containment check; return the unresolved candidate. Fixes the 4 CI test failures across all OS/Python matrix jobs on #4012. |
||
|
|
247abbf5e4 |
fix(presets): treat an unreadable core template as missing (#3961)
* fix(presets): treat an unreadable core template as missing _substitute_core_template() read the resolved core template with a bare read_text(), so one corrupted project-owned override in .specify/templates/commands/ crashed the whole wrap-strategy command registration with a raw UnicodeDecodeError. Both callers (CommandRegistrar.register_pack and _register_commands) are unguarded here, even though register_pack already skips an unreadable preset source with a warning a few lines above the call. Treat an unreadable core template like a missing one — warn and return the body unchanged with empty frontmatter — matching the function's documented no-core contract. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: assert the unreadable-core warning instead of suppressing it Review follow-up: use pytest.warns so removing or changing the promised warning fails the test. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
fe3732e268 |
fix(presets): return None for an unreadable layer in resolve_content (#3959)
* fix(presets): return None for an unreadable layer in resolve_content PresetResolver.resolve_content() reads the winning layer (and each composition layer) with a bare read_text(), so a layer file that cannot be read or decoded crashed command registration with a raw OSError/UnicodeDecodeError. The docstring already promises 'Composed content string, or None if not found', and since #3896 collect_all_layers() deliberately tolerates a non-UTF-8 legacy layer — moving the crash here, where both callers (_register_commands and _reconcile_composed_commands) are unguarded. Return None when the winning or base layer cannot be read, treating an unreadable layer like a missing one per the documented contract. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: cover the base guard and composing-layer read Review follow-up: add an unreadable replace base beneath a valid composing layer, and a mocked-PermissionError composing layer over a valid base, so every new boundary and both exception types are covered. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
6e7818f837 |
fix(presets): start fresh on a non-UTF-8 preset registry (#3955)
PresetRegistry._load() catches json.JSONDecodeError and FileNotFoundError to start fresh on a corrupted or missing registry, but a registry file with invalid UTF-8 bytes raised UnicodeDecodeError before JSON parsing began, crashing every preset command. Catch UnicodeDecodeError in the same clause: undecodable bytes are the same corruption class as unparseable JSON. OSError stays uncaught on purpose — the data may be intact on disk, and starting fresh would let a later _save() wipe it (same fail-closed reasoning as the workflow catalog cache loader). Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
cd996f74eb |
fix(manifests): reject non-string requires.speckit_version (#3980)
`requires.speckit_version` was presence-checked but never type-checked in both the extension and preset manifest validators, so an unquoted YAML `speckit_version: 1.0` (a float) passed validation and reached `SpecifierSet(required)` in `check_compatibility()`. That call is guarded by `except InvalidSpecifier` alone, which a non-string escapes two different ways: - a float/int/bool/None raises `TypeError: 'float' object is not iterable` from the `SpecifierSet` constructor; - a list or dict is an *iterable*, so `SpecifierSet` accepts it and the failure surfaces much later as `AttributeError: 'str' object has no attribute 'filter'` from inside `.contains()`. Neither is a `CompatibilityError`/`PresetCompatibilityError`, so both bypass the CLI's "Compatibility Error" handler in `_commands.py` and exit 1 with a raw traceback that names no field, leaving the author with no hint which manifest key is wrong. Type-check the field in both validators, requiring a non-empty string, and additionally guard `check_compatibility()` in both managers since each is public and reachable with a hand-built or mutated manifest. This mirrors the sibling `IntegrationDescriptor`, which already requires a non-empty string for the same key, and completes the type-checking pass started in #3943 for the neighbouring `extension`/`preset` fields. Adds 33 regression tests across both modules covering every escape path; 26 of them fail without this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Assisted-by: Claude Code (model: Claude Opus 5, supervised) |
||
|
chelsealongandGitHub
|
f4e3110560 |
fix(presets): restore core skills instead of deleting them on preset remove (#3929)
* fix(presets): restore core skills instead of deleting them on preset remove Skill restoration looked for core command templates under .specify/templates/commands, a directory specify init never populates in real projects. Since that lookup always missed, presets overriding a core command (e.g. speckit.plan) had their skill deleted outright on removal instead of restored — the actual core templates live in the bundled core_pack (wheel install) or the repo-root templates/ tree. Restoration now falls back to that bundled location, gated behind a restore_from_bundled_core flag so the existing "retire a stale skill superseded by a command-mode winner" path keeps deleting rather than resurrecting a duplicate skill. Fixes #3928 * fix(tests): use explicit utf-8 encoding reading restored skill content read_text() defaults to the platform locale encoding, which is cp1252 ("charmap") on Windows. The bundled specify.md core template contains a UTF-8 multi-byte emoji whose bytes aren't valid cp1252, so the Windows CI job failed decoding the restored SKILL.md with UnicodeDecodeError. * fix(presets): keep extension restore priority over bundled-core fallback The bundled-core fallback added for #3928 ran before the extension_restore_index lookup, so a skill an installed extension owns could be silently replaced by lower-priority bundled core content on preset removal instead of preserving the extension's winning layer. |
||
|
|
2d8904a21e |
fix(manifests): reject non-string metadata instead of crashing on it (#3943)
* fix(manifests): reject non-string metadata instead of crashing on it `ExtensionManifest` and `PresetManifest` checked only key PRESENCE for `id`/`name`/`version`/`description`, then fed the values straight to `re.match()` and `packaging.Version()`. Both raise a bare `TypeError` on a non-string, which is neither `ValidationError` nor `PresetValidationError`, so it escaped every caller that already handles a malformed manifest. YAML makes this an easy authoring slip rather than a contrived one: an unquoted `version: 1.0` parses as a float and `id: 2` as an int. The user-visible symptom is the one the in-tree comment above the section guards was written to prevent (#3898 for presets, and its extension twin): `list_installed()` degrades a bad manifest to "⚠️ Corrupted extension" but catches only the domain error, so a single bad manifest made `specify extension list` / `specify preset list` exit 1 with a raw traceback and *no output at all* — hiding every healthy extension/preset too, not just the broken one. Also unguarded on the same path: - extension `provides.commands[].name` → `TypeError` from the command-name pattern match. The sibling `file` field was already safe, since `relative_extension_path_violation()` rejects a non-string. - preset `provides.templates[].name`/`.file` → `TypeError` from `re.match` and `os.path.normpath` respectively. The third manifest twin, `IntegrationDescriptor`, is already hardened: it type-checks the same four fields and catches `TypeError` alongside `InvalidVersion`. This brings the other two in line with it. Tests: 68 added across both suites, covering each field against float, int, None, list, dict, and bool, plus an end-to-end guard per manifest type asserting a healthy entry still lists while the bad one degrades. All 68 fail with the source change reverted. Co-Authored-By: Claude Opus 5 (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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
14fab0a1af |
fix(presets): tolerate non-UTF-8 legacy commands (#3896)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
400ad01f12 |
fix(presets): validate required manifest mappings (#3898)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
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 |
||
|
github-actions[bot]GitHubgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>Copilotcopilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
81bf741b92 |
[bug-fix] Fix bundle-update-force-mislead: add refresh() to DefaultPrimitiveInstaller (#3452)
* Fix bundle-update-force-mislead: add refresh() to DefaultPrimitiveInstaller Apply the remediation from the bug assessment on issue #3424. DefaultPrimitiveInstaller lacked a refresh() method, causing _refresh_component() to fall back to install(), which calls ExtensionManager.install_from_directory() with force=False. This raised ExtensionError with a leaked --force hint that bundle update does not support, leaving users with no valid recovery path. Fix: add refresh() to each kind manager (ExtensionKindManager and PresetKindManager delegate to _do_install(force=True); WorkflowKindManager and StepKindManager delegate to install() as their callables are idempotent). DefaultPrimitiveInstaller.refresh() dispatches to the kind manager's refresh(). PresetManager.install_from_directory() and install_from_zip() gain a force parameter that removes the existing preset before reinstalling, mirroring ExtensionManager's force semantics. Refs #3424 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback on primitives.py and test_bundler_primitives.py - Replace ... with pass in _KindManager Protocol method stubs - Conditionally pass force= keyword only when force=True in _PresetKindManager - Fix _StepKindManager.refresh() to remove step before re-installing - Rename test to reflect actual assertion (refresh succeeds + force=True) - Remove duplicate install_bundle import Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * fix: add missing role/effective_integration to InstallPlan in _plan() and remove redundant import - Remove duplicate `DefaultPrimitiveInstaller` import inside test body (already imported at module scope on line 15) - Add required `role` and `effective_integration` fields to `InstallPlan` constructor in `_plan()` helper to prevent TypeError at runtime Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix: address latest PR review comments Assisted-by: GitHub Copilot (model: gpt-5.6-terra, autonomous) --------- 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: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Quratulain-bilalandGitHub
|
db5802b39b | fix: add missing utf-8 encoding to registry file open calls (#3810) | ||
|
|
4bc79fe243 |
fix(presets): guard non-list/non-mapping provides.templates in PresetManifest (#3712)
* fix(presets): guard non-list/non-mapping provides.templates in PresetManifest PresetManifest._validate iterated provides["templates"] with no shape guards, unlike the sibling ExtensionManifest. A malformed third-party preset.yml crashed with a raw TypeError that escapes the install handler's PresetValidationError/PresetError catch and dumps an unhandled traceback: templates: 5 -> "'int' object is not iterable" templates: [null] -> "argument of type 'NoneType' is not iterable" templates: [5] -> "argument of type 'int' is not iterable" (and a string/list entry raised the misleading "Template missing 'type', 'name', or 'file'"). Add a container list-guard and a per-entry mapping-guard that raise a clean PresetValidationError, mirroring ExtensionManifest's provides.commands guards. Valid manifests (list of mappings) are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): check provides.templates type before emptiness Address review feedback: the new shape guard sat behind the existing truthiness check, so a FALSY non-list (templates: 0/false/null/''/{}) still reported the misleading "Preset must provide at least one template" instead of the type error. Only truthy non-lists (5, "oops", {"a": 1}) reached the guard, which is why the original test (templates: 5) passed. Split the checks: presence -> container type -> emptiness. A falsy non-list now reports "expected a list"; an EMPTY LIST keeps the "at least one template" message, since that genuinely is a well-typed container with no templates. Parametrize the non-list test over truthy AND falsy values, and add a regression guard for the empty-list message. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(presets): drop the redundant empty-list templates test Address review feedback: the added test duplicated the pre-existing test_no_templates_provided -- both set provides.templates to [] and assert the same "must provide at least one template" error. That test already guards the empty-list result of the type-before-emptiness ordering, so keeping mine only added maintenance. Left a pointer comment where it was. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
05cb3cba34 |
fix(presets): tolerate non-string and non-list catalog fields in preset search/info (#3769)
* fix(presets): tolerate non-string and non-list catalog fields in preset search/info `preset search` and `preset info` crashed with a raw traceback on catalog payloads that are valid YAML/JSON but not string-typed. Catalog files are user-editable, so these shapes reach the code unvalidated. `PresetCatalog.search` had three unguarded assumptions: - `--author` called `.lower()` on the raw value → `AttributeError: 'int' object has no attribute 'lower'` for `author: 789`. - the query searchable-text join passed raw `name`/`description` through → `TypeError: sequence item 0: expected str instance, int found`. - the `--tag` filter iterated `tags` without a list check, so a scalar `tags: 5` (truthy, not iterable) raised `TypeError: 'int' object is not iterable`. PR #3743 fixed only the non-string *elements* of `tags` here; a non-list `tags` and the `author`/`name`/`description` fields were still unguarded. The sibling catalogs already handle all of these — `extensions/__init__.py` and `integrations/catalog.py` coerce with `str(...)` and gate on `isinstance(raw_tags, list)`. This aligns presets with them. The same scalar-`tags` crash reached the four display sites in `presets/_commands.py`, so those now gate on `isinstance(tags, list)`, matching `integrations/_query_commands.py`. Note `PresetManifest.tags` returns `self.data.get("tags", [])` and manifest validation does not enforce list-ness, so a local `preset.yaml` with `tags: 5` validates successfully and then crashed `preset info` — hence the guard on the local-manifest branch too. While here, `preset search` printed tags unescaped, so a tag containing `[bold]` was silently swallowed as a Rich style tag; it now routes through `_escape_markup` like the `preset list` line directly above it. Regression tests in `TestPresetTagsNonString` drive the full CLI path via CliRunner. All five fail before the fix, each with the exact exception it targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Assisted-by: Claude Opus 5 (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> * chore: regenerate security audit requirements (annotated-doc 0.0.5) The Security Audit workflow's "Check committed audit requirements are current" step regenerates requirements with `uv pip compile --upgrade`, which now resolves annotated-doc==0.0.5. Re-sync the committed snapshot so the check passes. No pyproject dependency changes; upgrade drift only. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> 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> Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481 |
||
|
|
655a3cb8ca |
fix(integrations): preserve native skill invocation prefixes (#3663)
* fix(integrations): use native dollar skill invocations Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): preserve skill post-process idempotence Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): preserve literal skill invocations Resolve generated command references with the active agent prefix instead of rewriting all slash-form text during post-processing. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): preserve shared invocation prefix Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): preserve install invocation prefix Pass dollar-style skill prefixes through bare-project integration installation and cover the shared template output. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): preserve dollar refs everywhere Use agent-native invocation prefixes in extension command registration and dynamic shared-script command hints. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(shared-infra): preserve dollar command hints Escape dollar-prefixed commands embedded in Bash strings and propagate the native prefix into installed Python command helpers. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(shared-infra): render native helper prefixes Rewrite installed Bash and PowerShell formatter return expressions so direct callers receive the selected integration's native prefix. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(skills): use invocation-neutral hook guidance Describe hook-derived references as command invocations so dollar-prefixed skills do not receive contradictory slash-command terminology. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * test(integrations): expect native fallback invocation Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * refactor(integrations): centralize invocation prefix selection Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 * fix(integrations): add Kimi /skill: prefix and fix docstrings - Add SKILL_COLON_AGENTS frozenset and get_invocation_prefix() to _invocation_style.py so Kimi resolves to '/skill:' in skills mode - Switch invoke_prefix_for_integration() to use get_invocation_prefix() instead of the binary dollar/slash check - Update post_process_skill_content docstring (base.py) to cover both slash and dollar native invocation forms - Update _resolve_command_refs_in_skill docstring (presets/__init__.py) to document the dollar-prefixed result alongside slash forms Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix(agents): use get_invocation_prefix for Kimi in register_commands Replace the binary is_dollar_skills_agent ternary with get_invocation_prefix so that __SPECKIT_COMMAND_*__ tokens in Kimi skill files resolve to /skill:speckit-<name> rather than /speckit-<name>. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix(agents): remove unused is_dollar_skills_agent import Leftover from replacing the inline ternary with get_invocation_prefix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix(integrations): use get_invocation_prefix in post_process_skill_content Replaces the binary is_dollar_skills_agent ternary with get_invocation_prefix so that Kimi's hook-command note is injected as /skill:speckit-git-commit from the start. This keeps _inject_hook_command_note idempotent for Kimi: the previous note with its native prefix now matches on repeated passes, preventing duplicate note injection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix(presets): use get_invocation_prefix in _resolve_skill_command_refs Replace the binary is_dollar_skills_agent ternary with get_invocation_prefix so Kimi tokens resolve to /skill:speckit-* directly rather than /speckit-* (which previously relied on the broad post-process body replacement). Also fix test_restore_skill_preserves_dollar_command_refs to write raw_core with the unresolved __SPECKIT_COMMAND_PLAN__ token, exercising the resolver rather than bypassing it with a pre-resolved string. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * docs(presets): document /skill: form in _resolve_skill_command_refs Add /skill:speckit-<cmd> to the docstring so the contract covers all three native prefix forms returned by get_invocation_prefix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * test(integrations): add Kimi /skill: prefix coverage - test_skill_colon_prefix_core_command: resolve_command_refs with /skill: prefix - test_get_invocation_prefix_skill_colon: get_invocation_prefix returns /skill: for kimi (skills), / for kimi (non-skills), $ for codex, / for claude - test_kimi_skill_post_processing_is_idempotent: verifies Kimi's hook-command note is injected with /skill: prefix and does not duplicate on re-runs - test_installed_bash_formatter_uses_skill_colon_prefix: shared-infra bash formatter outputs /skill:speckit-plan when installed with /skill: prefix Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix(kimi): use get_invocation_prefix in process_template, remove broad replacement process_template() was still using a binary is_dollar_skills_agent ternary to select between dollar and slash prefix, so Kimi tokens were emitted as /speckit-* and then corrected by a broad .replace('/speckit-', '/skill:speckit-') in KimiIntegration.post_process_skill_content(). That broad replacement would also rewrite any literal /speckit-* text in generated skill content, contrary to the PR's token-only behavior. - Use get_invocation_prefix(agent_name, invoke_separator == '-') in process_template() so Kimi tokens are emitted as /skill:speckit-* directly. - Remove the broad .replace() from KimiIntegration.post_process_skill_content(); it is now a no-op (tokens are already correctly prefixed at source). - Add test_process_template_kimi_uses_skill_colon_prefix to guard the fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19 Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28 |
||
|
Pascal THUETandGitHub
|
118062eac4 |
harden: secure extension and preset archive downloads (#3141)
* harden: secure extension and preset archive downloads Adopt the shared download-security primitives from #3140 across extension and preset catalog, package, direct-URL, and ZIP-install flows: - bound catalog, package, and inline manifest reads; - verify catalog SHA-256 values when present; - replace path-only extraction with bounded traversal/symlink-safe extraction; - validate malformed hosts and ports before opening download URLs; - handle normalized trailing-backslash directory entries consistently. Redirect enforcement and checksum verification remain owned by the shared helpers already on main; this commit wires them into extension and preset behavior. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close archive and catalog download edge cases Preflight ZIP central directories before ZipFile allocates them, bound both declared and actual payload sizes, and reject ambiguous or non-portable archive paths before extraction. Keep extension update manifest selection consistent with extraction, reject unsafe catalog-derived output filenames and malformed URL types, and escape untrusted values in download errors. Add regression coverage for parser differentials, collisions, platform-specific filenames, bounded call sites, and failure ordering. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: address download security review feedback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close ZIP preflight review gaps Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update preflight and rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
|
6136706ef3 |
fix(presets): coerce non-string catalog tags before joining (#3743)
* fix(presets): coerce non-string catalog tags before joining Preset catalog payloads are user-editable YAML/JSON, so a `tags:` list can legitimately contain non-strings (e.g. numeric tags). The preset list/search/info display paths and the catalog search backend joined tags with a raw `", ".join(...)` / used `t.lower()`, which raised `TypeError: sequence item N: expected str instance, int found` (or `AttributeError` on `.lower()`) and crashed the command. Sibling command surfaces already guard this — extensions, integrations, and workflows coerce with `str(t) for t in ...`. This aligns presets: - `_commands.py`: `preset list`, `preset search`, and both `preset info` branches now join `str(t) for t in ...`. - `__init__.py` `PresetCatalog.search`: tag filter uses `str(t).lower()` and the searchable-text join coerces tags to `str`. Adds regression tests driving `preset search` and `preset info` through CliRunner with numeric tags; both fail before the fix with the TypeError. 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> --------- 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> |
||
|
|
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 |
||
|
|
1f7290c975 |
fix(presets): re-validate catalog URL after redirects (HTTPS parity/security) (#3523)
* fix(presets): re-validate catalog URL after redirects (HTTPS parity) PresetCatalog._fetch_single_catalog opened the catalog URL and trusted the payload without re-validating response.geturl() after redirects. _open_url follows redirects (stripping auth only on an HTTPS->HTTP downgrade), so an https:// catalog entry that 30x-redirects to http://attacker/... was still fetched and trusted. The catalog payload supplies each preset's download_url + sha256, so a redirected payload can drive install of an arbitrary archive that passes verify_archive_sha256. Add the post-redirect geturl() re-validation via _validate_catalog_url, mirroring integrations/catalog.py, workflows/catalog.py, and bundler adapters — and presets/_commands.py, which already does this on its --from download path. This is the lone preset catalog-fetch site missing the guard. Test: an HTTPS URL whose response.geturl() reports http:// is rejected (PresetValidationError). Completed four existing fetch-test mocks that predated this behavior to report geturl() like a real urllib response. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): validate every redirect hop + guard the legacy fetch_catalog path Two follow-ups to the catalog redirect hardening: 1. Validate every redirect hop, not just the terminal URL. A final-geturl-only check passes an https -> http -> attacker-controlled-https chain: the insecure intermediate hop lets a network attacker rewrite the next redirect. _open_url now forwards a redirect_validator to open_url (called before each hop), and _fetch_single_catalog passes _validate_catalog_url through it while retaining the final geturl() check — mirroring bundler/services/adapters.py. 2. The legacy public fetch_catalog() single-catalog path parsed response.read() with no redirect check at all. Give it the same redirect_validator + final geturl() validation. Tests: a stubbed intermediate http hop is rejected (redirect_validator), and the legacy fetch_catalog() rejects an HTTPS->http redirected payload (fail before: no raise). Existing fetch-test mocks updated to accept the redirect_validator kwarg and report geturl() like a real response. Full test_presets.py (365) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(presets): drop duplicate geturl mock; add legacy fetch_catalog per-hop test - Remove the duplicate mock_response.geturl.return_value assignment left by the geturl mock-completion pass (the explanatory comment was stranded between the two identical assignments); keep a single assignment after the comment. - Add test_fetch_catalog_legacy_validates_every_redirect_hop so the legacy fetch_catalog() path is verified to supply the redirect_validator (rejecting an insecure intermediate hop), not just the terminal geturl() — parity with _fetch_single_catalog and the #3524 sibling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
8a5bcc21a5 |
fix(extensions,presets): surface clean error on malformed download URL (#3577)
* fix(extensions,presets): surface clean error on malformed download URL `ExtensionCatalog.download_extension` and `PresetCatalog.download_pack` read `download_url` from catalog payload data and pass it to `urlparse(...).hostname` during the HTTPS validation. A malformed authority (e.g. an unterminated IPv6 bracket like `https://[::1`) makes urlparse/hostname raise a raw `ValueError`, which escapes past the command handlers — they only catch `ExtensionError` / `PresetError` — and surfaces as an uncaught traceback. Guard the parse in a try/except and re-raise as the domain error so the CLI reports a clean "download URL is malformed" message. Mirrors the same fix in catalogs (#3435) and workflows/catalog.py (#3484). Adds regression coverage for both catalogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): escape markup in preset_add error handlers Copilot review on #3577 flagged that the malformed-URL fix stopped short: `download_pack` now raises a clean `PresetError`, but the `preset_add` handler rendered `{e}` unescaped. A catalog `download_url` like `https://[not-an-ip]/x` is embedded verbatim in the message, so Rich interprets `[not-an-ip]` as a markup tag and can raise a style/markup exception while rendering the error — the CLI still crashes instead of exiting cleanly. Escape `str(e)` in the preset command handlers, matching the extension handler at `extensions/_commands.py:657`, and hoist the `rich.markup` import to module scope (dropping the two inline imports). Adds CLI-level regression tests: a bracketed-host `download_url` exits cleanly, and the compatibility/validation/error handlers escape markup-bearing messages. Both tests fail on the pre-fix handler (test-the-test verified). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
eb2252a1cb |
fix(catalogs): 'priority: .inf' yields a clean validation error instead of crashing (#3525)
* fix(catalogs): priority: .inf yields a clean validation error, not OverflowError
_load_catalog_config coerces a catalog entry's priority with int() inside
except (TypeError, ValueError). int(float('inf')) raises OverflowError, which is
not in that tuple, so a YAML 'priority: .inf' escaped as an uncaught traceback
instead of the intended 'expected integer' validation error (the bool-is-int
case is already guarded just above). Add OverflowError to the except tuple.
Test mirrors the existing rejects_boolean_priority test with priority: .inf
(fails before: OverflowError; passes after: ValidationError naming the config).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(presets): priority: .inf in a preset catalog config yields a clean error
The PresetCatalog._load_catalog_config priority parser has its own loader
(separate from CatalogStackBase) that caught only TypeError/ValueError, so a
YAML 'priority: .inf' escaped as an uncaught OverflowError from int(float('inf')).
Add OverflowError to the except tuple (the bool-is-int case is already guarded
just above), matching catalogs.py.
Test mirrors rejects_boolean_priority with priority: .inf (fails before:
OverflowError; passes after: PresetValidationError).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
0a60e53e06 |
fix(presets): raise PresetValidationError, not raw ValueError, on malformed catalog URL (#3576)
`PresetCatalog._validate_catalog_url` called `urlparse(url).hostname` without guarding it. For a malformed authority such as an unterminated IPv6 bracket (`https://[::1`), `urlparse(...).hostname` raises `ValueError: Invalid IPv6 URL`, which escapes the method. Its docstring promises `PresetValidationError`, and its callers (`preset catalog add`, `preset catalog list` reading the `SPECKIT_PRESET_CATALOG_URL` env var / `.specify/preset-catalogs.yml`) only catch `PresetValidationError` -- so a malformed URL crashes the CLI with a traceback instead of a clean error message. The shared `CatalogStackBase` (#3435), `workflows` (#3484), `bundler` (#3433) and `IntegrationCatalog` copies already wrap this in `try/except ValueError`; the preset validator was the remaining un-updated twin. Mirror the shared implementation: wrap `urlparse` + `.hostname`, re-raise as `PresetValidationError("Catalog URL is malformed: ...")`, and read the local `hostname` in the host check. Add a regression test mirroring `IntegrationCatalog`'s `test_malformed_url_rejected_cleanly`; it is red before the fix (raw `ValueError`) and green after. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
353851e966 |
fix(presets): seed constitution from preset constitution-template (#3272) (#3276)
* fix(presets): seed constitution from preset constitution-template (#3272) The constitution is the only template materialized to a live file (.specify/memory/constitution.md) rather than resolved on demand, yet ensure_constitution_from_template hardcoded a copy from the core template and ignored PresetResolver. Combined with init seeding the constitution before preset installation, a preset's constitution-template (e.g. strategy: replace with a ratified constitution) could never go live. Changes: - ensure_constitution_from_template now resolves constitution-template through PresetResolver, so a preset/override/extension wins and core is the fallback. - init seeds the constitution after preset installation so init --preset uses the resolved stack. - install_from_directory re-seeds memory/constitution.md from the resolved preset template, guarded to only act when the memory file is missing or still contains generic placeholder tokens — authored constitutions are never overwritten. Covers preset add and install_from_zip. - Tests for preset seeding, placeholder re-seed, authored-constitution preservation, override resolution, and resolver-aware init seeding. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): compose constitution-template when seeding memory Take on review feedback from Copilot and gglachant: - constitution seeding previously copied the top layer file path verbatim even when the winning layer used a composing strategy (prepend/append/wrap), which could leave {CORE_TEMPLATE} unresolved. - both seeding paths now inspect resolver layers and only copy verbatim for replace; non-replace strategies materialize composed content via PresetResolver.resolve_content(). - add regression tests for wrap strategy composition in both PresetManager seeding and ensure_constitution_from_template. - add a drift-guard test pinning _CONSTITUTION_PLACEHOLDER_TOKENS to the placeholders in templates/constitution-template.md. Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * refactor(presets): unify constitution template materialization Address latest Copilot feedback on the constitution seeding path: - moved resolver/layer I/O behind the existing-memory fast path in init - corrected tracker output for composed materialization - deduplicated materialization logic shared by init and preset install seeding into presets._materialize_constitution_template() Behavior is unchanged for replace strategies (copy verbatim) and remains composed for prepend/append/wrap via resolve_content(). Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(init): restore shutil import The constitution materialization refactor removed the module import, but init still uses shutil.rmtree when cleaning up a failed new-project initialization. Restore the import so the required ruff check passes. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(presets): harden constitution materialization Address the outstanding review batch for preset constitution seeding: - use checked atomic writes and reject symlinked memory paths - replace placeholder heuristics with hash/source provenance - rematerialize unchanged generated constitutions by resolver priority - preserve authored or edited constitutions, including placeholder mentions - warn non-fatally when post-install materialization cannot complete - retain exact core-template comparison for legacy projects without provenance Add focused provenance, priority, symlink, and failure-path coverage, and update integration inventories for the generated provenance sidecar. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 * fix(presets): reconcile constitution after removal When the removed preset supplied constitution-template, rematerialize the winning remaining resolver layer only if provenance proves the live file is still generated and unchanged. Preserve edited constitutions and report post-removal reconciliation failures as non-fatal warnings. Add coverage for restoring the core layer, falling back from a removed higher-priority preset, and preserving edited generated content. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 * fix(presets): tighten legacy constitution provenance Trust only the immutable bundled/source constitution template when migrating legacy projects without provenance. Do not infer core provenance from mutable project templates or preset source labels, including IDs beginning with core. Also detect convention-based constitution-template files before preset removal so unchanged generated constitutions reconcile to the next resolver layer. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 * fix(presets): preserve files with invalid provenance Use immutable-core legacy migration only when the provenance sidecar is absent. If a sidecar is malformed or its hash does not match the live constitution, treat the file as edited and preserve it. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 * fix(presets): reconcile constitution on stack changes Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 * fix(presets): guard constitution reconciliation edges Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
fc1a3fd76c |
fix(presets): resolve() honors manifest-declared file: for installed presets (#3351)
* fix(presets): resolve() honors manifest-declared file: for installed presets PresetResolver.resolve()'s tier-2 (installed presets) loop was convention-only: it looked for templates/<name>.md and <name>.md, ignoring a preset manifest that declares the template with an explicit, non-convention file: path. So resolve() returned the core template (and resolve_with_source() misattributed source='core') while collect_all_layers()/resolve_content() correctly used the preset's declared file — a divergence inside the same class. It could also return a stray convention-path file the manifest deliberately points away from. Mirror collect_all_layers()'s manifest-first logic: use the declared file: when present (skip convention fallback if it's missing, to avoid masking typos), and fall back to the convention walk only when the manifest is absent or doesn't list the template. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(presets): clarify the empty/falsey manifest-file branch comment Per review: 'file' is a required key for every template entry (PresetManifest._validate()), so the manifest-found branch is reached for an empty/falsey/non-usable 'file' value, not a truly absent one. Reword the comment to say so. Comment-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(presets): resolve() returns only real files; test missing-file skip Per review: - Use is_file() (not exists()) when honoring a manifest-declared file: so a manifest pointing at a directory is treated as missing rather than returned to a caller that will read_text() it. Applied in both resolve() and collect_all_layers() so the two stay consistent. - Add a regression test for the skip-convention-fallback-when-declared-file- missing behavior: manifest declares a missing custom/spec.md while the pack has a convention templates/spec-template.md; resolve() must skip the pack and fall through to core, not pick up the stray convention file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(presets): resolve()/collect_all_layers() require a regular file for manifest file: A manifest-declared file: path is honored via exists(), which also accepts a directory. If a preset points file: at a directory, resolve() returned it and downstream read_text() crashes. Use is_file() in both resolve() and collect_all_layers() so a non-file (directory) is treated as missing and the convention fallback is skipped (pack yields to core), matching the existing missing-file behavior. Adds a directory-at-file: test (fails on exists(), passes on is_file()) that also asserts collect_all_layers() never returns the directory as a layer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(presets): extract shared _manifest_declared_template for resolve()/collect_all_layers() Both methods reimplemented the manifest-entry lookup + authoritative-fallback rules independently — the exact duplication that let them diverge and caused the bug this PR fixes. Extract a single _manifest_declared_template(pack_dir, name, type) -> (entry, candidate) helper (candidate is the declared file only when it is_file(); a declared-but-unusable file returns (entry, None) so callers skip the convention fallback). resolve() and collect_all_layers() now both call it, so their manifest-first resolution cannot silently diverge again. Pure refactor, behavior-preserving: full test_presets.py (331) still passes, including the directory-at-file:, missing-file, and manifest-file-wins cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3b7d95a408 |
fix: rewrite extension-relative subdir paths in generated command bodies (#3444)
* fix: rewrite extension-relative subdir paths in generated command bodies Extension command bodies reference bundled files relative to the extension root (agents/, knowledge-base/, templates/, ...). Generated SKILL.md and command files emitted those paths verbatim, so agents resolved them against the workspace root where they do not exist. Add CommandRegistrar.rewrite_extension_paths, which rewrites references to subdirectories that actually exist in the installed extension to .specify/extensions/<id>/..., and call it once in register_commands so every output format and alias gets the fix. commands/, specs/ and dot-directories are never rewritten. Fixes #2101 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: only rewrite relative extension path references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use callable re.sub replacement for extension subdir rewrite subdir and extension_id come from filesystem directory names and were interpolated into a re.sub string replacement template. A directory name containing a backslash (e.g. assets\q) would raise re.error: bad escape, aborting command registration even when the body didn't reference it. Use a callable replacement so these values are treated literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make subdir rewrite regression test cross-platform Renamed the test's subdir fixture from "assets\\q" to "assets[q]": on Windows, backslash is a path separator, so mkdir would create nested "assets/q" dirs instead of one literally-named directory, and iterdir() would only discover "assets", never exercising the rewrite. extension_id keeps a real backslash/"\\1" since it isn't used to create a directory, still verifying the callable replacement handles it literally. Added a sanity assertion for this assumption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite in skills-mode renderer register_commands() rewrote extension-relative subdir references (agents/, knowledge-base/, etc.) via rewrite_extension_paths(), but _register_extension_skills() - the separate renderer used for active non-native skills agents (e.g. Claude with ai_skills: true) - never called it. Generated SKILL.md files left agents/... and knowledge-base/... unresolved, and mapped the extension's own templates/ through the generic project-level rewrite instead of its installed .specify/extensions/<id>/templates/ location. Reuse the existing rewrite_extension_paths() helper in _register_extension_skills() at the same point register_commands() applies it (before resolve_skill_placeholders' generic rewrite), and add a skills-mode regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite on preset restore/reconcile paths _unregister_skills() restored extension-backed SKILL.md content via resolve_skill_placeholders() without first calling rewrite_extension_paths(), so removing a preset override that shadowed an extension command restored the bare, unresolvable agents/... and knowledge-base/... references. Carried extension_id/extension_dir through _build_extension_skill_restore_index() and applied the same rewrite used at initial registration before restoring. Found the identical gap in _reconcile_composed_commands()'s non-skill agent path: when a removed preset's command reverts to an extension winner, register_commands_for_non_skill_agents() was called without extension_id, so the rewrite never ran for plain command-file agents either. Passed extension_id through there too. Added regression tests for both restore paths (skills-mode and non-skill-agent command files) in tests/test_presets.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite when composing over extension base PresetResolver.resolve_content() read the effective base layer's raw content directly via path.read_text() before composing append/prepend/ wrap overlays on top of it, and its outright-replace shortcut did the same. When that base layer was extension-provided, neither read path applied rewrite_extension_paths(), so composing a preset over an extension command (or an extension winning outright through resolve_content) left bare, unresolvable agents/... and knowledge-base/... references in the composed output. All three call sites (PresetManager._register_commands()'s composed path, _reconcile_composed_commands()'s composed path, and skills-mode reading the .composed file written by either) consume resolve_content's return value, so fixing the read at its source covers command output, skill output, and both initial-install and reconcile flows without threading extension identity through each caller. Tagged extension layers in collect_all_layers() with extension_id/ extension_dir, and added a _read_layer_content() helper in resolve_content() that applies rewrite_extension_paths() whenever a layer carries that extension identity — used at both raw-read sites (outright-replace shortcut and composition base). Composing (append/prepend/wrap) layers are never extension-provided (extensions are always inserted with strategy "replace"), so no other read site needs the rewrite. Added regression tests: a parametrized resolve_content() test covering append/prepend/wrap composing over an extension base, a skills-mode test asserting the composed SKILL.md resolves the extension's subdir references, and a non-skill-agent (Gemini) install-time test matching the reported live repro. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Dyan GalihandGitHub
|
6fb7e77b3e |
fix: allow prerelease spec-kit versions in compatibility checks (#2695)
* docs: generate integrations reference from catalog * refactor: integrate table rendering into specify integration search --markdown - Remove standalone scripts/generate_integrations_reference.py - Strip doc injection machinery from catalog_docs.py; keep only table rendering - Wire render_integrations_table() into existing --markdown flag of integration search - Remove old simple markdown table block from integration_search (was Name|ID|Version|Description|Author) - Simplify tests: drop subprocess/doc-path tests, keep table rendering and metadata tests - Clean up docs/reference/integrations.md: remove generated markers, update note * fix: address Copilot review feedback on catalog_docs and integration_search - Warn when --markdown is combined with filters (query/--tag/--author) which are silently ignored; catch ValueError/FileNotFoundError and surface clean error via console instead of raw traceback (r3244821516) - Add coverage enforcement in list_integrations_for_docs(): raises ValueError with actionable message if any registry key is missing from INTEGRATION_DOC_URLS, preventing silently incomplete doc tables (r3244821589) - Rename test to accurately reflect sources: label derives from registry config, URL comes from INTEGRATION_DOC_URLS doc map — not solely from registry (r3244821607) - Simplify test dict construction to idiomatic dict comprehension (r3244821619) * fix: add sync test, INTEGRATIONS_REFERENCE_PATH constant, and fix naming * revert: restore docs/reference/integrations.md to upstream/main; remove sync test (GH Actions job will handle) * fix: remove dead INTEGRATIONS_REFERENCE_PATH, drop URL-length padding, fix docstring, drop FileNotFoundError * fix: send --markdown warnings/errors to stderr, rename test for clarity * fix: detect stale doc-map keys, test _render_cell escaping, strengthen header assertion * refactor: promote _render_cell to public render_cell function * test: mock registry and doc maps to avoid brittle live registry coupling * refactor: flatten patches, remove unused imports, fix trailing whitespace, optimize missing calculation * refactor: make validation non-fatal, fix context manager syntax, add CLI tests * fix: improve docstring clarity, test robustness, and exception handling * fix: improve test assertions, disable warnings by default, enhance exception handling * fix: make CLI tests deterministic and improve config access resilience * fix: remove extra blank line, add stale keys validation, add regression test for docs sync * Fix 5 remaining feedback items: - Rename _get_mocked_cli_runner() to _get_catalog_docs_patches() for clarity - Use ExitStack context manager for guaranteed patch cleanup - Add explicit UTF-8 encoding to file reads - Skip doc sync test gracefully when docs aren't present - Remove exception chaining from typer.Exit to avoid noisy tracebacks * address all outstanding copilot review feedback on PR 2563 * Address Copilot feedback: escape URLs in markdown links, deduplicate cell rendering, fix table parser for escaped pipes * Address 3 new Copilot feedback: add URL escaping test, fix parse_first_markdown_table for escaped pipes, guard community tests with skip * Address 3 new Copilot feedback: escape id field, remove unused alias, escape integration URLs * Address 3 new Copilot feedback: fix comment name, include all integrations in list * Fix architectural issue: escape raw fields before composing Markdown to prevent double-escaping * Deduplicate _escape_url_for_markdown_link and add URL escaping test * Address 4 new Copilot feedback: add trailing newline, fix test helper ExitStack, update warning message * Address 4 new Copilot feedback: make escape function public, fix error message, validate test rows, prevent double newline * Update error message in test_missing_catalog_file for clarity * Remove obsolete integrations sync test * keep integrations docs in sync * fix: allow prerelease spec-kit versions in compatibility checks Allow prerelease/dev builds to satisfy extension and preset compatibility checks when their version number falls within the required specifier range. Also harden the integrations docs rendering helpers and add regression coverage for the markdown table parsing and version gating paths. Tests: pytest -q; python3 -m compileall -q .; black/flake8 unavailable Reference: branch 002-generate-integrations-docs; source patch /tmp/spec-kit-changes.patch * fix: isolate prerelease compatibility gate changes Keep the prerelease/version compatibility fix on its own branch and remove the unrelated integrations docs updates that belong with PR 2563. Tests: full suite passed on the prerelease branch before splitting; docs branch covered by targeted docs tests Reference: upstream/main; source patch /tmp/spec-kit-changes.patch * Address PR 2695 feedback: Centralize prerelease policy and add boundary test * Address remaining Copilot PR feedback: revert docs and add preset prerelease tests * Remove unreachable raise CompatibilityError * Fix PEP8 E302 and E303 formatting issues |
||
|
|
cb7c36c95b |
fix: reject host-less catalog URLs in base and preset validators (#3209) (#3227)
`CatalogStackBase._validate_catalog_url` (inherited by `IntegrationCatalog`) and `PresetCatalog._validate_catalog_url` checked `parsed.netloc`, which is truthy for host-less URLs like `https://:8080` (port only) or `https://user@` (userinfo only). Such URLs slipped past validation despite the error message promising "a valid URL with a host", then failed later with a confusing fetch error. Switch both validators to `parsed.hostname` (None for those inputs), matching the workflow, step, and bundler catalog validators that already do this. Add regression tests covering port-only and userinfo-only URLs for both validators. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Quratulain-bilalandGitHub
|
5bdcb4ad14 |
fix(catalogs): reject host-less catalog URLs in base and preset validators (#3210)
the shared CatalogStackBase validator and PresetCatalog validator checked parsed.netloc to enforce 'a valid URL with a host'. but netloc is truthy for host-less URLs like https://:8080 or https://user@, so those slipped through even though they have no host - contradicting the error message. the workflow, step, and bundler validators already check parsed.hostname (which is None in those cases); this aligns the two stragglers with that. add regression tests covering port-only and userinfo-only URLs. |
||
|
Si ZengyuandGitHub
|
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. |
||
|
Zied JlassiandGitHub
|
b042d2a843 |
feat(extensions): verify catalog archive sha256 before install (#3080)
* feat(extensions): verify catalog archive sha256 before install Extension and preset archives were downloaded over HTTPS and unpacked (with Zip-Slip protection) but their bytes were never checked against a known digest. Trust rested entirely on TLS and the integrity of the release host, so a tampered or swapped archive from a compromised third-party release would be installed silently. Maintainers do not audit extension code, so consumer-side integrity is the only available defence. Catalog entries may now pin an optional `sha256` digest. When present, the downloaded archive is verified before it is written to disk and installed; a mismatch aborts with a clear error. Entries without `sha256` keep working unchanged (a DEBUG line records that the download was unverified), so the change is backwards compatible. The check runs on both download paths (extensions and presets) via a single shared helper so the two stay in parity. - Add `verify_archive_sha256` helper in shared_infra (digest match, `sha256:` prefix, case-insensitive; DEBUG log when no digest declared) - Enforce it in ExtensionCatalog.download_extension and PresetCatalog.download_pack, before the archive is written to disk - Document the optional `sha256` field in the publishing guides - Tests: helper unit tests + matching/mismatch/no-digest on both paths Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Assisted-by: AI * fix(extensions): harden sha256 parsing and tidy download test mocks Follow-up to the review on #3080: - shared_infra.verify_archive_sha256: strip only a literal `sha256:` algorithm prefix (case-insensitive) instead of `split(':', 1)[-1]`, which silently dropped any prefix — so `md5:<64-hex>` was accepted as if it were a valid SHA-256. Validate that the declared value is exactly 64 hex characters and raise a clear error otherwise, and compare with `hmac.compare_digest` for a constant-time check. Add tests covering a malformed digest and a non-`sha256:` prefix (both previously accepted). - Download test helpers: configure the context-manager mock via `__enter__.return_value`/`__exit__.return_value` rather than assigning a `lambda s: s`, which is clearer and independent of the invocation arity. Assisted-by: AI Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> * fix(extensions): reject a declared-but-empty sha256 instead of skipping verification verify_archive_sha256 skipped on any falsy expected value, so a present-but-empty digest (e.g. sha256: "" reached via ...get("sha256")) silently disabled the integrity check instead of surfacing the authoring error. Guard on expected is None so only an absent digest skips; blank/whitespace/bare-prefix values fall through to the 64-hex validation and are rejected. Adds a regression test. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * docs(shared_infra): clarify _SHA256_HEX_RE accepts and normalizes uppercase The comment described the regex as matching '64 lowercase' hex characters, but verify_archive_sha256 lowercases the declared value (raw.lower()) before matching, so an uppercase digest is accepted and normalized rather than rejected. Clarify the comment to avoid misleading future readers. Addresses Copilot review feedback on shared_infra.py. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * test(presets): cover the no-sha256 backwards-compatible path Address Copilot review: download_pack's optional sha256 verification was tested for match/mismatch but not the backwards-compatible path where a catalog entry has no sha256 (pack_info.get("sha256") is None). Add a no-sha256 test mirroring the extensions coverage so the helper never silently becomes mandatory for presets. 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> |
||
|
|
79a34b892d |
fix(presets): use _repo_root() for bundled-core source-checkout fallback (#3086) (#3091)
* fix(presets): use _repo_root() for bundled-core source-checkout fallback The tier-5 fallback in PresetResolver.resolve() and _find_bundled_core() computed the repo root as Path(__file__).parent.parent.parent. After presets.py was moved to presets/__init__.py (#2826) that chain is one level short, resolving to src/ and looking for src/templates/commands/<cmd>.md, which never exists. As a result, wrap-strategy presets found no core base layer in source/editable installs. Use the shared _repo_root() helper so both fallbacks resolve against the actual repo-root templates/ tree. Wheel installs were unaffected (core_pack path), so this only impacts source/editable checkouts. Refs #3086 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(presets): restore dropped def for oserror-manifest test A prior edit accidentally removed the def test_resolve_extension_command_via_manifest_skips_oserror_manifests line, orphaning its body inside the new bundled-core test. Restore the test definition so pytest collects it again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(presets): move bundled-core tests into TestPresetResolver The two tier-5 fallback regression tests exercise collect_all_layers() and resolve(), not resolve_core(), so they belong in TestPresetResolver rather than TestResolveCore. Relocate them for clearer suite navigation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
f9c6cf83e5 |
fix(presets): preserve argument-hint in preset SKILL.md generation (#2978)
* fix(presets): preserve argument-hint in preset SKILL.md generation Preset-provided and extension-override commands that declare `argument-hint:` in their frontmatter had it dropped from the generated Claude SKILL.md, and it was re-dropped when a preset was removed and its overridden skill restored. This is the preset-side analog of the extension fix in #2903 / #2916. Factor the argument-hint carry-over into a shared CommandRegistrar.apply_argument_hint() helper and apply it at the four preset skill-generation sites (register, reconcile override-restore, and the core/extension unregister-restore paths). The extension path from The helper writes argument-hint into the frontmatter dict before serialization (so a folded multi-line description cannot be split into invalid YAML) and only for integrations that support it (those exposing inject_argument_hint -- currently Claude), leaving build_skill_frontmatter's shared shape unchanged for every other agent. Core templates carry no argument-hint, so the core-restore path is a no-op. No behavior change for non-Claude agents or the core path. Add regression tests covering a folding description (Claude) and the non-Claude gate (codex). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): address review - guard skill_frontmatter type and tighten apply_argument_hint annotations Add a symmetric isinstance(skill_frontmatter, dict) guard so the helper stays a safe no-op if a caller passes a non-dict, and annotate the parameters as Dict[str, Any] with an optional integration to match real call-site usage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
811a3aa447 |
fix(skills): preserve non-ASCII characters in skill frontmatter (#2917)
* fix(skills): preserve non-ASCII chars in skill frontmatter Skill SKILL.md frontmatter descriptions containing non-ASCII characters were escaped to \uXXXX / \xXX sequences because yaml.safe_dump() was called without allow_unicode=True. - Add allow_unicode=True to the 7 skill/command frontmatter safe_dump sites (extensions, presets, claude integration) - Add regression tests for the render and extension-install paths Follows the approach of #1936; encoding="utf-8" is already set on the affected write paths, so no encoding change is needed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(_utils): add dump_frontmatter helper Centralize skill/command frontmatter YAML serialization into a single _utils.dump_frontmatter helper so no call site can drop allow_unicode or diverge on formatting. Route the 7 existing sites through it and drop a now-unused local yaml import. Switch the extension test fixtures to yaml.safe_dump for parity with the production safe-dump/safe-load codepaths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
darion-yaphetandGitHub
|
f20e8ee6f7 |
refactor: move preset command handlers to presets/_commands.py (PR-6/8) (#2826)
* refactor(presets): convert presets.py module to presets/ package Pure structural move to mirror integrations/. presets.py becomes presets/__init__.py with relative imports rebased one level deeper. No behavior change; public import surface (from .presets import ...) preserved. Prepares for co-locating preset command handlers in PR-6/8. * refactor: move preset command handlers to presets/_commands.py (PR-6/8) Cut the preset_app / preset_catalog_app Typer groups and all 12 command handlers out of __init__.py into presets/_commands.py, exposing register(app) — mirrors the integration co-location from PR-5. __init__.py now registers via _register_preset_cmds(app), dropping ~620 lines (3282 -> 2663). Handlers lazy-import root helpers (_require_specify_project, get_speckit_version, _locate_bundled_preset, _display_project_path) via 'from .. import' so test monkeypatching of specify_cli.<helper> keeps working. _locate_bundled_preset kept as an explicit re-export in __init__.py for that resolution path. CLI surface and public imports unchanged. Full suite: 3162 passed, 40 skipped. |