chore: merge upstream 0.14.4, retire _hooks_fork.py, adopt events - #105
Merged
Conversation
…ions (github#3529) Forge installs its slash-commands with hyphenated names (speckit-foo-bar, via format_forge_command_name and the injected frontmatter name), but ForgeIntegration inherited MarkdownIntegration.build_command_invocation, which builds the dotted /speckit.<cmd>. So 'workflow'/command dispatch invoked /speckit.plan while the registered command is /speckit-plan — a name Forge never registered. Override build_command_invocation to reuse format_forge_command_name, producing /speckit-<name> (with '.'-to-'-' for extension commands), mirroring the skills agents' hyphenated invocation. Tests assert Forge core + extension invocations are hyphenated, incl. args (fail before: dotted /speckit.plan / /speckit.git.commit). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ub#3520) * fix(workflows): route 'workflow status --json' errors to stderr The workflow_status run_id error paths (FileNotFoundError -> 'Run not found', ValueError -> invalid run) used the stdout console and fired before the json_output branch, so 'specify workflow status <bad-id> --json' wrote a Rich-rendered error to stdout and corrupted the JSON stream a consumer would json.loads(). Route both through _error_console(json_output) so they go to stderr under --json, matching the sibling 'workflow run'/'workflow resume' commands (which use the identical RunState.load try/except) and the documented stdout-purity contract. Test asserts the not-found error appears on stderr and stdout stays empty under --json (fails before: the error was on stdout). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(workflows): cover the ValueError handler in workflow status --json purity The stderr-routing fix reroutes both the FileNotFoundError and ValueError run_id handlers, but the test only exercised FileNotFoundError — a regression of the ValueError path back to stdout would have gone uncaught. Add a ValueError case (RunState.load raising) asserting the same stderr-only / empty-stdout behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ithub#3582) `PromptStep.execute` str()-coerces `config['prompt']` and dispatches the result to the integration CLI as the model's instructions. But its `validate` only checked that `prompt` was *present*, not that it was a string — the exact parity gap the sibling `ShellStep` closes for `run`. So a YAML authoring slip like `prompt: [review, this]` or `prompt:` (null) passed validation, then `execute` sent the Python repr (`"['review', 'this']"`, `"None"`) to the LLM verbatim — silently wrong instructions with no error and a COMPLETED status. The engine does not auto-validate step config (`load_workflow` explicitly defers validation), so validation is the only place this surfaces before dispatch. Extend `validate` to reject any non-string `prompt` with the shell-step's phrasing ("'prompt' must be a string, got <type>"), mirroring the shell `run` and command `input`/`options` type checks. A `{{ ... }}` expression is still a str, so it stays valid. Adds regression coverage for non-string prompts (null/list/int/dict) and confirms an expression prompt still validates. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…late (github#3537) * fix(workflows): fail fan-out loudly on a truthy non-mapping step template A fan-out step whose `step:` is a truthy scalar or list (an authoring mistake) passed execute and reached the engine, which calls template.get("id", ...) in _run_fan_out — raising AttributeError and taking down the whole run. validate already rejects a non-mapping step, but the engine does not auto-validate, so an unvalidated run crashed. Guard execute to FAIL the step (with a clear error and normalized empty output) instead, mirroring the existing non-list items guard and the switch non-dict cases guard. Add the matching test_execute_non_dict_step_fails_loudly covering the execute-path guard (validate was already covered). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(workflows): reject explicit fan-out `step: null` in validate() The runtime guard in execute() rejects a truthy non-mapping step, but `config.get("step", {})` only substitutes the `{}` default for an *absent* key — an explicit `step: null` reaches the guard as None and FAILS the step. validate() previously exempted None (`step is not None and ...`), so such a workflow passed validation and then failed during execution. Align validate() with the runtime guard: a present-but-non-mapping `step` (including `None`) is an authoring mistake and is now rejected up front. Extend the validate and execute regression cases to cover None. Addresses Copilot review feedback on github#3537. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…thub#3579) `FanInStep.execute` already guards a non-list `wait_for` (github#3482), and the engine's load-time validation rejects non-string entries. But the engine does not auto-validate step config, so on an unvalidated run `execute` iterated the list's *elements* raw: - An unhashable entry (a list/dict from a YAML indentation slip like `wait_for: [[a, b]]`) crashed the whole run at `context.steps.get(entry, ...)` with a raw `TypeError: cannot use 'list' as a dict key`. - A hashable-but-non-string entry (`wait_for: [123]`) silently joined an empty `{}` and still reported COMPLETED — the exact "silent empty result + COMPLETED" wiring bug the whole-list guard and the engine's fan-in validation both exist to prevent. Extend the execute() guard to reject any non-string entry with the engine's "entries must be step-id strings" phrasing, mirroring the sibling non-list guard right above it. Adds regression coverage for unhashable and hashable-non-string entries. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ithub#3530) * docs(integrations): document the 'integration list --catalog' flag 'specify integration list' accepts a --catalog flag (integrations/_query_commands.py: typer.Option(False, "--catalog", ...)) that browses the full built-in + community catalog, but the Integrations reference documented no options for the list command. Add an option table for it, matching the style used by the sibling 'integration search' and 'integration catalog add' sections. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(integrations): clarify that default 'integration list' shows only built-ins The --catalog row implied the default list already includes the full installed set; in fact 'integration list' iterates INTEGRATION_REGISTRY (built-ins) and marks installed status, so a community integration that is not built in only appears with --catalog. Reword the option and the intro sentence to say the default shows the built-in integrations and --catalog adds community ones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ad of crashing (github#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>
…p catalog loaders (github#3526) * fix(workflows): reject bool/.inf catalog priority in workflow & step catalog loaders The WorkflowRegistry and StepRegistry catalog-config loaders coerced priority with int() inside except (TypeError, ValueError), missing two guards the base CatalogStackBase loader already has: - bool is an int subclass, so 'priority: true' was silently coerced to 1; - int(float('inf')) raises OverflowError (not caught), so 'priority: .inf' crashed with an uncaught traceback. Add the explicit bool check and OverflowError to both loaders, and add OverflowError to the two _coerce_priority helpers used by 'catalog add' (they return 0 on an uncoercible existing priority instead of crashing). Parametrized tests on both TestWorkflowCatalog and TestStepCatalog reject priority true/false/.inf (fail before: bool coerced to 1 / inf OverflowError). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(workflows): cover add_catalog() OverflowError fallback for existing priority: .inf The workflow/step catalog priority guards added OverflowError to _coerce_priority (the 'catalog add' fallback), but the tests only exercised get_active_catalogs(). Add tests that prewrite an existing 'priority: .inf' entry and call add_catalog() for both WorkflowCatalog and StepCatalog, asserting the command succeeds and the new entry gets a valid priority (inf coerced to 0, +1). Fails before: int(inf) OverflowError crashed add_catalog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…emove (github#3589) IntegrationCatalog.add_catalog and remove_catalog re-validate the existing catalog entries' priorities inline, separately from the base loader. Both did `int(raw_priority)` under `except (TypeError, ValueError)`, so a `priority: .inf` (float('inf')) raised OverflowError: add_catalog leaked a raw traceback instead of IntegrationValidationError, and remove_catalog crashed while building the display order. Add OverflowError to both handlers, matching the base loader (github#3525) and the workflow/step loaders (github#3526). add_catalog now raises IntegrationValidationError; remove_catalog falls back to positional order like the other non-integer priorities. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* chore: bump version to 0.13.1 * chore: begin 0.13.2.dev0 development --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github#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 (github#3435) and workflows/catalog.py (github#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 github#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>
* feat(workflows): add standalone WorkflowResolver and overlay subsystem Implement PR 1 of the workflow-overlays plan: a concrete, standalone WorkflowResolver for downstream workflow extensibility without touching the Preset subsystem. - Add overlay manifest schema (Overlay, OverlayEdit, validate_overlay_yaml) - Add pure-function merge engine (find_step, apply_edit, merge_steps, validate_edits) with recursive anchor search and higher-wins semantics - Add StepListComposer and tiered layer sources (project, installed, base) - Add WorkflowResolver facade with inline HIGHER_WINS priority sorting - Add CLI verbs: workflow overlay add/set-priority/enable/disable/remove/list and workflow resolve <id> - Wire WorkflowEngine.load_workflow through WorkflowResolver - Extend workflow add to copy optional overlays/ subdirectory from local workflow directories - Add comprehensive unit, integration, and security tests Refs: discussion github#3473 (github#3473) Assisted-by: Kimi (model: opencode-go/kimi-k2.7-code, autonomous) * fix(workflows): reject symlinked overlay directories in layer sources Address PR github#3557 review comments r3594064534 and r3594064563: - ProjectOverlaySource.collect now rejects symlinked per-workflow overlay directories (.specify/workflows/overlays/<id>) before iterating - InstalledOverlaySource.collect now rejects symlinked installed overlay directories (.specify/workflows/<id>/overlays) before iterating - workflow_overlay_list catches ValueError from resolver and exits with code 1 instead of crashing on unhandled exceptions - Added .specify/workflows/overlays to _reject_unsafe_workflow_storage chokepoint for defense-in-depth These guards prevent symlinked overlay directories from redirecting auto-loaded overlay YAML to attacker-controlled content outside the project, which could inject executable shell steps into trusted workflows. Refs: PR github#3557 review comments r3594064534, r3594064563 Assisted-by: opencode-go/qwen3.7-max (autonomous) * fix(workflows): address Copilot review findings in merge engine - Apply inserts before winning replace to prevent anchor-not-found errors when replace changes step ID (r3594064604) - Track attribution recursively for nested steps in composite inserts/replaces so workflow resolve attributes all child steps correctly (r3594064638) - Add regression tests for both fixes Refs: PR github#3557 review discussion Assisted-by: GitHub Copilot (model: qwen3.7-plus, autonomous) * refactor(workflows): simplify overlay architecture to 2-tier Remove installed overlays tier to enforce clean separation of concerns: - workflow add installs workflows only (no overlay copying) - workflow overlay add installs overlays only (project-local) Changes: - Remove InstalledOverlaySource class and all references - Remove overlay-copying logic from _validate_and_install_local() - Update WorkflowResolver to 2-tier: project overlays + base workflow - Fix --priority override timing: apply before validation, not after - Remove tests for installed overlays (no longer applicable) Rationale: If upstream controls both base workflow and shipped overlays, and both get overwritten on bundle update, there's no reason to ship overlays separately. Overlays only make sense when someone other than the base author adds them. Resolves all three review findings from PR github#3557: - r3594064677: workflow add no longer copies overlays from all call sites - r3594064705: --priority override now applied before validation - r3594064726: no stale installed overlays (tier removed entirely) Assisted-by: Claude (model: claude-opus-4-7, autonomous) * fix(workflows): harden overlay symlink handling Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(workflows): remove stale installed-overlay references from workflows.md The 2-tier refactor (cc28185) removed the installed-overlay tier entirely, but docs/reference/workflows.md was not updated. This commit addresses all four Cluster 2 findings from the PR review: - workflow add: remove sentence about copying overlays/ subdirectory - How Overlays Work: drop installed-overlay table row and precedence prose; rewrite to 2-tier model (project overlays only, source-order tie-break) - overlay remove: drop trailing sentence about installed overlays - Interaction with Bundles: rewrite to say workflow add installs only workflow.yml; remove installed-overlay discovery language Fixes: r3596368791, r3596368831, r3596368873, r3596368919 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(overlays): detect ancestor-conflict anchors in merge_steps When two overlay edits target anchors that share a parent/descendant relationship (e.g. remove an if-step + insert_after a nested child), merge_steps processed them independently and in dict-insertion order, making the outcome non-deterministic. Add two private helpers to merge.py: - _descendant_ids(step): returns all step IDs nested inside a step dict by delegating to the existing _all_base_step_ids helper on children. - _check_anchor_conflicts(anchors, base_steps): for each targeted anchor finds its descendants and checks whether any other targeted anchor is among them; returns human-readable error strings. Wire _check_anchor_conflicts into merge_steps immediately after edits_by_anchor is built, before any tree mutation occurs. Raises ValueError listing the conflicting anchor pair(s) so overlay authors know exactly what to fix. Add TestMergeStepsAncestorConflicts (6 cases): - remove parent + insert_after child raises ValueError - replace parent + remove child raises ValueError - conflict across multiple overlays raises ValueError - sibling anchors (not ancestor/descendant) pass - single anchor passes - parent targeted but child not targeted passes Closes review comment r3596368746 (PR github#3557, round 2, cluster 3). Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(overlays): fix over-broad conflict detection and non-deterministic ID collision Finding 1.1 — _check_anchor_conflicts was rejecting any ancestor/descendant anchor pair, including insert-only edits that are perfectly safe. Only replace/remove on an ancestor can destroy its subtree and make a descendant anchor unresolvable. Change the signature to accept a dict[str, str] (anchor → winning operation) and skip the check for insert_after/insert_before. Finding 1.2 — merge_steps was calling find_step on the already-mutated tree, so a replacement step that reused a base step ID could be accidentally targeted by a later edit group (non-deterministic result depending on dict iteration order). Replace the anchor-group loop with a single-pass _traverse_and_apply that walks the original tree structure and applies edits as each step is encountered. Anchors are never re-looked up in a mutated tree. Design invariant enforced: overlays always apply to the original base tree and cannot target steps introduced by other overlays. Non-remove edits on non-base anchors now raise ValueError early. Also removes apply_edit (no production callers, only tested in isolation) and its test class — the new traversal inlines the same mechanics without the find_step round-trip. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(overlays): reject ID trailing newlines and reuse existing .yaml path Fix two input validation bugs in the overlay layer (Group 2 of copilot review PR github#3557): 1. _validate_safe_id in schema.py used re.match() which anchors only at the start of the string, so IDs like 'overlay\n' passed validation and could produce newline-containing file paths. Changed to fullmatch() so the entire string must satisfy the pattern. 2. workflow_overlay_add always wrote <id>.yml without checking whether <id>.yaml already existed. Since the resolver loads both extensions, this created two active layers whose edits applied twice. Now uses the existing _find_overlay_file() to detect a pre-existing file and reuse its path, falling back to .yml only for new overlays. Tests added for both fixes. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(overlays): fix display order inversion and wrap file-read errors Finding group 3 from copilot-review-v2.md: 3.1 — Precedence display inverted (overlays/__init__.py) collect_all_layers used a single-pass sort by (-priority, source_asc), which placed the *losing* equal-priority source first in the display while claiming "highest first". Fix: two-pass stable sort — source descending then priority descending — so the actual winner (last applied by the composer) rises to the top of the display. 3.2 — Unwrapped file-read errors (overlays/layer_sources.py) Only yaml.YAMLError was caught around path.read_text(), so an unreadable or non-UTF-8 overlay produced a raw traceback. Fix: widen the except clause to (yaml.YAMLError, OSError, UnicodeDecodeError), matching the pattern used throughout catalog.py. Tests: - test_workflow_resolve_equal_priority_winner_shown_first: verifies project:zzz (the winner) appears before project:aaa in workflow resolve output when both overlays share the same priority. - tests/workflows/test_overlay_layer_sources.py (new): OSError and non-UTF-8 bytes both produce OverlayLoadError, not raw tracebacks. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: rename misleading overlay test Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove EOF blank line in overlay resolver Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: handle overlay read and enumeration errors Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(overlays): validate resolver workflow IDs Reject unsafe and reserved workflow IDs before overlay or base sources construct paths, preventing traversal through resolver and engine fallback paths. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(overlays): drop _remove_sources_recursively from remove branch In _traverse_and_apply, the remove branch called _remove_sources_recursively to clean up attribution entries for the deleted step. This was inherited from the old apply_edit loop (c70a5d6) where it was needed because the sources dict was queried exhaustively. In the current single-pass design, _build_attribution only traverses the result list, so stale sources entries for removed steps are never read. The cleanup call is therefore unnecessary — and actively harmful when another overlay has replaced a different step with a new step that reuses the same ID: the pop clobbers the replacement's attribution entry, causing workflow resolve to report the surviving step as 'unknown'. Fix: simply remove the _remove_sources_recursively call from the remove branch. Add an attribution assertion to the existing reused-ID regression test to catch this case. Fixes: r3604242050 (Copilot review finding) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * Fix CLI overlay ID validation anchoring Use fullmatch for CLI workflow/overlay ID validation so trailing newlines are rejected consistently with manifest validation. Add regression coverage for newline-suffixed workflow and overlay IDs in overlay set-priority. Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate workflow_id in layer sources before path construction ProjectOverlaySource.collect() and BaseWorkflowSource.collect() joined workflow_id directly onto storage paths without validation, enabling path traversal (e.g. '../../outside') when called outside the WorkflowResolver. Add _validate_workflow_id() to layer_sources.py — mirrors the same _SAFE_ID_PATTERN / _RESERVED_WORKFLOW_IDS check used by WorkflowResolver in overlays/__init__.py — and call it at the top of both collect() methods before any path is constructed. Adds parametrised tests covering unsafe IDs and verifying no filesystem access occurs for an invalid ID. Closes review finding r3604772700. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): align layer source validation with _safe_workflow_id_dir Plan §4.1 requires that Workflow-ID-Validierung, Symlink-/Containment- Prüfungen and Fehlerübersetzung must not diverge between workflow management and the overlay resolver. My previous fix added ID pattern + reserved-name validation to both collect() methods but was missing the containment step and the BaseWorkflowSource directory/file checks that _safe_workflow_id_dir performs. Changes: - Add _ensure_contained_dir(path, root) to layer_sources.py — pure domain mirror of overlays/_commands.py::_ensure_contained_dir that raises OverlayLoadError instead of typer.Exit - ProjectOverlaySource.collect(): replace two inline symlink/dir checks with _ensure_contained_dir(workflow_overlay_dir, self.overlays_dir), adding the missing resolve().relative_to() containment step - BaseWorkflowSource.collect(): add _ensure_contained_dir on the workflow directory, and add workflow.yml symlink check before is_file() The same logic now lives in three places (workflow CLI, overlay CLI, layer sources). The DRY extraction to workflows/_validation.py is deferred to PR 3 per plan §4.1. Tests: add containment and symlink tests for both sources. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(overlays): resolve identity from manifest field, not filename Align overlay identity resolution with the project-wide convention: presets use preset.id, extensions use extension.id, workflows use workflow.id, and workflow steps use step.type_key. Overlays must derive identity from the manifest id field, not the filename. Rewrite _find_overlay_file() to scan all YAML files in the overlay directory and match on the manifest id field, fixing the bug where enable/disable/remove/set-priority failed when filename != manifest id. Closes: PR github#3557 discussion r3605010197 Assisted-by: opencode-go/qwen3.7-max (autonomous) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(workflows): make validation behavior consistent across YAML loading paths Address PR github#3557 review finding r3607632921: - Wrap yaml.YAMLError → ValueError in from_yaml() and from_string() so malformed YAML matches the documented exception contract - Add except ValueError to workflow_info to handle composition errors cleanly instead of crashing with a raw traceback - Remove validate_workflow() from compose() so the resolver path is parse-only like all other YAML loading mechanisms; callers validate explicitly via engine.validate() - Update test to reflect new behavior: resolve() returns composed definition, caller validates separately Assisted-by: opencode-go/qwen3.7-max (autonomous) * fix(overlays): list disabled overlays in management view Keep disabled overlays visible in workflow overlay list while leaving resolution behavior unchanged. - add an include_disabled opt-in to overlay source/resolver collection - use include_disabled=True for workflow overlay list - add regression tests for list visibility and default filtering Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * docs: align overlay extends and resolver contract Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use atomic write for overlay file updates to prevent hard-link attack Replace in-place write_text() calls in workflow_overlay_add() and _update_overlay_field() with the same mkstemp → write → os.replace() pattern used by the workflow installer (_stage_workflow_file / _commit_workflow_file / _discard_staged_workflow_file). The prior code rejected symlinks and validated path containment, but a hard-linked destination file passes both checks while sharing an inode with an external file. write_text() would then truncate and overwrite that external inode. The atomic staging approach never opens the existing destination for writing, eliminating the hard-link vector. Fixes findings r3608669512 and r3608669517 on PR github#3557. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised) * fix(composer): preserve invalid base definition instead of coercing steps to [] When 'steps' is not a list, returning early with the unmodified WorkflowDefinition lets validate_workflow surface the proper error ("'steps' must be a list.") to the caller. The previous silent coercion to [] masked the validation error entirely. Fixes: github#3557 (comment) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised) * fix: align workflow overlay priority semantics Assisted-by: GitHub Copilot (model: GPT-5.6 Terra, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: validate overlay priority presentation Assisted-by: GitHub Copilot (model: GPT-5.6 Terra, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: catch OverflowError in normalize_priority for float infinity values YAML values like `priority: .inf` parse to float('inf'), causing int() to raise OverflowError. This broke validate_overlay_yaml()'s 'validation never raises' contract. Adding OverflowError to the except clause makes it fall back to the default priority (10), consistent with other invalid value handling. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Markus <markus@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Assisted-by: Codex (model: GPT-5, autonomous)
…b#3607) Add test-coverage-drift-control extension submitted by @benizzio to: - extensions/catalog.community.json (alphabetical order) - docs/community/extensions.md community extensions table Closes github#3600 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
Update okf extension submitted by @alexcpn: - extensions/catalog.community.json (version, download_url, description, provides.commands, updated_at) - docs/community/extensions.md community extensions table Closes github#3602 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
…ub#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` (github#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. github#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 github#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 github#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 github#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 github#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 github#3415) Address review github#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 github#3415) Address review github#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 github#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 github#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 github#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 (github#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 github#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 github#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>
…ain reinstall after --keep-config (github#3449) * Fix reinstall-overwrites-kept-config: preserve config on plain reinstall after --keep-config Apply the remediation from the bug assessment on issue github#3427. Before the unconditional shutil.rmtree(dest_dir), scan dest_dir for any *-config.yml and *-config.local.yml files and hold their contents in memory. After shutil.copytree succeeds, write them back so user-customized values always win over the packaged defaults. This mirrors the existing backup/restore logic for the --force reinstall path but handles the case where remove --keep-config left config files behind in an unregistered extension directory. Refs github#3427 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: restore method decl, move config restore before registration, preserve file mode - Restore missing `test_install_force_without_existing` method declaration in tests/test_extensions.py so pytest collects it as a separate test. - Move stranded-config restoration to immediately after `copytree`, before command/skill/hook registration, so a failed registration step can't leave preserved configs permanently lost. - Store `(bytes, mode)` tuples instead of bare bytes when rescuing stranded configs, and reapply the original file mode after writing so permission bits (e.g. 0600 for credential files) are faithfully restored. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * fix: mask setuid/setgid bits when restoring stranded config file mode Only preserve user/group read-write bits (mode & 0o660) to avoid restoring setuid, setgid, or world-writable permissions from a user-modified config file. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * fix: add copytree rollback path and strengthen regression test with packaged default config - Wrap shutil.copytree in a try/except BaseException so stranded configs rescued before rmtree are written back even if copytree fails mid-way (addresses review comment: configs were permanently lost on copy failure) - Add a packaged default config to extension_dir in the regression test so a naive 'restore only when absent' implementation would fail; assert the user's customized values beat the packaged defaults after reinstall Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix: restore configs with secure atomic writes Assisted-by: GitHub Copilot (model: gpt-5, autonomous) * fix: write secure temp file then chmod to preserved_mode; add copytree-failure test - _restore_stranded_config_file: write content while temp file is at its secure OS-default mode (typically 0600 on POSIX), then apply the original preserved_mode after the file is fully written and before the atomic os.replace. Removes the & 0o660 mask that was silently stripping world-read and executable bits (e.g. 0644 → 0640). - Add test_copytree_failure_restores_stranded_config: patches shutil.copytree to create a partial destination then raise OSError, then asserts that the preserved config bytes and file mode are restored by the rollback path and that the extension remains unregistered. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * Potential fix for pull request finding 'Unused local variable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Module is imported with 'import' and 'import from'' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: durable staging for stranded configs and import style fix - Stage stranded config files to a durable rescue_staging_dir (extensions_dir/.rescue-staging-<id>) before rmtree so original bytes survive partial rmtree, copytree failure, or partial restore on retry. On retry the staging dir is detected and its content reused instead of whatever mix of packaged defaults and partial restores remains on disk. The staging dir is cleaned up only after every restore succeeds. - Fix CodeQL: change `import specify_cli.extensions as _ext_module` to `from specify_cli import extensions as _ext_module` in test file. Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix: harden rescue staging dir - symlink checks, secure writes, cleanup errors - Thread 14: Change except BaseException to except Exception in the staging fallback block so KeyboardInterrupt/SystemExit propagate correctly - Thread 15: Add explanatory comment to the bare pass in the chmod except block to satisfy static analysis - Thread 16: Reject a symlinked staging directory and only reload non-symlinked files whose names match the two recognised config suffixes - Thread 17: Create each staging file via os.open with mode 0600 and O_CREAT|O_EXCL before writing so preserved bytes are never transiently exposed to other local users - Thread 18: Remove ignore_errors=True from the final staging-dir cleanup so a failed rmtree propagates rather than silently leaving a stale backup that could be misread on the next retry Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix: mask file-type bits from chmod, harden staging dir symlink check - Add `import stat` to imports - Use `stat.S_IMODE(mode)` before chmod in staging write (thread 20, line 1464) - Use `stat.S_IMODE(preserved_mode)` and make chmod best-effort in `_restore_stranded_config_file` (thread 18, line 1492) - Add `not rescue_staging_dir.is_symlink()` guard to cleanup (thread 19, line 1522) Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * fix: use completion marker for rescue staging, abort on staging failure, full os.write Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) * fix(extensions): make rescue staging durable Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(extensions): fix flaky copytree regression test Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(extensions): fix module import alias for review feedback Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(workflows): keep cleanup warnings single-line and remove dead helper Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding 'Module is imported with 'import' and 'import from'' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Preserve rescued extension config across retry Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify ignored directory fsync cleanup errors Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix reinstall durability and workflow cleanup warnings Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Open rescue staging file in binary mode to fix Windows CRLF corruption On Windows os.open() defaults to text mode, so os.write() of preserved config bytes containing \r\n was translated to \r\r\n, corrupting the staged backup and failing the retry-restore regression test. Add O_BINARY (0 on POSIX) to the staging file open flags. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Load .extensionignore before deleting dest_dir on reinstall The .extensionignore loader can raise ValidationError (invalid UTF-8) or OSError. Previously it ran after dest_dir was removed, so such a failure left the kept config only in the hidden staging directory rather than its documented location. Load/validate it before the rmtree so every post-deletion failure path restores the config. Adds a regression test. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Validate .extensionignore before publishing rescue staging Loading .extensionignore after the rescue staging directory was published meant a validation failure left a complete staging copy behind. A later retry (after the user fixed the ignore file and edited the kept config) would reload the stale staged bytes and silently overwrite the newer config. Move the loader ahead of reading/creating rescue staging so a failure aborts while the kept config is still authoritative on disk, and extend the regression test to prove no staging is published and a retry adopts the newer bytes. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden preserved-config rescue against divergence and long names Address three review findings on the reinstall config-rescue path: - A complete .rescue-complete marker proves only that staging finished, not that dest_dir was modified. A crash after staging sync but before the rmtree leaves the live kept config intact; if the user edits it before retrying, preferring the staged bytes silently overwrote the newer config. The two copies are indistinguishable in provenance from disk, so detect divergence between a complete staging copy and the live config and abort (preserving both) instead of unconditionally choosing staging. - The staging directory embedded the full extension ID in one path component. Extension IDs are length-unbounded, so a valid long ID could install at dest_dir yet fail every reinstall-after-keep-config with ENAMETOOLONG. Derive the staging component from a fixed-length hash via a new _rescue_staging_dir() helper. - The stranded-config restore used the full config filename as a NamedTemporaryFile prefix; a name already near the component limit plus the random suffix raised ENAMETOOLONG. Use a short fixed prefix. Updates the retry regression test to the new divergence semantics and adds conflict-abort, long-ID, and fixed-prefix coverage. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden preserved-config rescue divergence check and fix test path Assisted-by: GitHub Copilot (model: GPT-5.6, autonomous) * fix: reject/flag symlinked preserved configs on reinstall Assisted-by: GitHub Copilot (model: GPT-5.6-Sol, autonomous) * fix: include symlinks in live-dir config enumeration and address review feedback - _recognized_config_names() now accepts follow_symlinks=False for live dir so symlinked *-config.yml entries are detected and treated as conflicts rather than being silently deleted by rmtree. - Add explanatory comment to bare 'except OSError: pass' in _restore_stranded_config_file's finally block. - Resolve CodeQL dual-import style: use 'from specify_cli import extensions as _ext_module' instead of 'import specify_cli.extensions as _ext_module'. Assisted-by: GitHub Copilot (model: claude-sonnet-4, autonomous) * test: add staging-failure fault-injection test for rescue staging block Add test_staging_failure_aborts_before_dest_dir_removal covering three failure modes (mkdir, os.open/O_CREAT, fsync with EIO) in the rescue staging block. Each parametrized case verifies: - the install aborts before dest_dir is removed - the preserved config bytes remain authoritative - any partial staging is cleaned up and not left as complete - the extension stays unregistered Addresses review feedback on PRRT_kwDOPiFCnc6R351t. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * test: add test_retry_restores_config_from_staging_when_live_absent Exercises the retry-from-staging branch (if staging_is_complete at line 1505 of extensions/__init__.py) in a scenario where the live config is absent — simulating a power loss that interrupted the rollback before it could write the config back. When the live copy is gone, the live-dir fallback (elif dest_dir.exists()) finds no stranded configs and the packaged default would be kept. Only the staging-complete branch can restore the original bytes and mode. This proves staging (not the fallback) is used on retry. Addresses review feedback on PRRT_kwDOPiFCnc6SAL3L. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * fix: keep staging files writable; record modes in .rescue-modes.json; fix live-only conflict message Thread 64: Remove os.fchmod/chmod from staged files to avoid Windows read-only attribute that prevents shutil.rmtree from cleaning up. Original permission bits are now written to a .rescue-modes.json sidecar in the staging dir and reloaded during retry, with a fall-back to the staged file's own mode for backwards-compat with pre-sidecar staging dirs. Thread 65: Split the ValidationError message for staging-vs-live conflicts into two accurate cases: files that diverged between both locations ("Both copies have been preserved") and live-only files that have no backup counterpart, which previously incorrectly claimed "Both copies have been preserved" and offered a restore instruction that was impossible. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: add .keep-config provenance marker to guard rescue path against partially-failed installs When `remove --keep-config` strands config files, write a `.keep-config` marker into the extension directory. `install_from_directory` now only enters the rescue path when that marker is present, preventing a partially- failed install (which also leaves dest_dir with no registry entry but no marker) from having its packaged default configs treated as user-preserved data on a retry from an updated package. Refs: github#3449 (comment) Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * refactor: extract _has_keep_config_marker helper and document empty-content choice Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous) * fix: defer rescue-backup cleanup until registry commit; validate modes sidecar shape Assisted-by: GitHub Copilot (model: GPT-5.6, autonomous) * fix legacy keep-config rescue and retry baseline handling --------- 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> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
… substring (github#3590) CommandRegistrar.parse_frontmatter located the closing delimiter with content.find("---", 3), a raw substring search. It stopped at the first "---" anywhere after the opening — including one embedded in a frontmatter value (e.g. a description "Separate sections with --- markers") or inside an indented literal block — which truncated the frontmatter and spilled the remainder into the body, silently corrupting both the parsed metadata and the rendered command body. Match the closing "---" on line boundaries, mirroring the line-anchored scan already used by VibeIntegration._inject_frontmatter_flag.
… Python (github#3386) * feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python Ports the three core workflow scripts to Python as part of github#3280, following the check-prerequisites PoC pattern from github#3302. Adds resolve_template() to the shared common.py module and parity tests that run bash and Python side by side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(tests): treat only None env as unset in parity run helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): fall back to directory scan on any registry error, skip hidden preset dirs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(templates): add py: lines for setup_plan and setup_tasks Ships with the scripts they reference; the remaining templates got their py: lines in github#3403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: support py variant in skills placeholder resolver resolve_skill_placeholders only accepted sh/ps, so a py init option fell into the fallback path and {SCRIPT} rendered without an interpreter prefix. Accept py and prefix the resolved interpreter, matching process_template. Also guard ps_cmd against a missing PowerShell with a clear assert. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: pin clean-error behavior for invalid --number Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(scripts): reword unused-arg comment to match implementation The loop accepts and silently ignores extra positional args (it doesn't build a collected list); match the wording to what the code and setup-plan.sh actually do. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fall back when configured script variant is missing from frontmatter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): reject signed/whitespace --number values to match bash 10# parity The bash twin uses $((10#$BRANCH_NUMBER)), which rejects signed and whitespace-padded values. Python's int() accepted them (e.g. -1), producing a malformed -01-... prefix that sequential scans ignore. Restrict --number to unsigned decimal digits before conversion, and pin the parity with a bash-comparison test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete Python port installation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(integrations): fall back for missing script variants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: make Python script checks platform-aware Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix Windows Python command invocation parity Use PowerShell's call operator for spaced Python interpreter paths and align setup-tasks missing-template errors across script variants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): preserve cross-platform Python parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject signed PowerShell feature numbers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align feature number range Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): reject exhausted feature numbers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete create feature parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align create feature outputs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): harden cross-platform parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): keep truncation JSON clean Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align setup failure parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): close parity edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): propagate PowerShell setup errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): harden fallback resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): stabilize PowerShell fallbacks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete setup-plan parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): require runnable script fallbacks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): preserve shell fallback without preference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): restore help and symlink parity - setup-tasks.ps1: check -Help before unknown-argument validation so '-Help --bogus' exits 0 like the Bash/Python variants - common.py: strip the repo root prefix lexically in persist_feature_json instead of resolve(), so a symlinked specs/ still persists the relative 'specs/NNN-name' path the Bash/PowerShell helpers store Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align persist-hint quoting with shlex.quote - create-new-feature.sh: replace printf %q with a shell_quote helper that emits shlex.quote-identical output, so the persistence hints stay byte-identical between the Bash and Python variants (printf %q output also varies between bash versions) - promote the negative --number test to an all-variants parity test now that Bash and PowerShell reject signed values consistently - add a spaced-repo-path parity test for the persistence hints Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ecurity) (github#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 github#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>
* Add community bundle submission automation Add the discovery-only community bundle catalog, online and offline catalog loading, and a restricted agentic workflow for validating bundle submissions and opening draft catalog PRs. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fbe794bc-667a-4c9e-b48b-825067debc6d * Address community bundle review feedback Ensure explicit install-allowed catalogs take precedence over built-in discovery, tighten component installability validation, and use issue-linked community branches. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fbe794bc-667a-4c9e-b48b-825067debc6d * Address follow-up bundle review feedback Make offline catalog coverage content-agnostic and require autonomous catalog commits to include the assisted-by trailer. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fbe794bc-667a-4c9e-b48b-825067debc6d * Harden bundle catalog table rendering Require single-line escaped Markdown table values for untrusted submission metadata. The needs-info label used by validation is now present in the repository. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fbe794bc-667a-4c9e-b48b-825067debc6d * Clear stale bundle validation labels Allow the submission workflow to remove prior outcome labels before applying the current validation state. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fbe794bc-667a-4c9e-b48b-825067debc6d
…y/security) (github#3524) * fix(extensions): re-validate catalog URL after redirects (HTTPS parity) ExtensionCatalog._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 payload supplies each extension's download_url + sha256, so a redirected payload can drive install of an arbitrary archive that passes sha256 verification. Add the post-redirect geturl() re-validation via _validate_catalog_url, mirroring integrations/catalog.py, presets, workflows/catalog.py, and bundler adapters. Sibling of the same fix in the presets catalog fetcher. Test: an HTTPS URL whose response.geturl() reports http:// is rejected (ExtensionError). Completed 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(extensions): correct auth-strip comment; validate every redirect hop + guard legacy fetch_catalog - Correct the comment: _StripAuthOnRedirect strips auth not only on an HTTPS->HTTP downgrade but also whenever the redirect leaves the configured trusted hosts. The comment now describes both cases. - Parity with the presets fix: validate EVERY redirect hop (not just the terminal URL) so an https -> http -> attacker-https chain can't slip a redirected payload past the final-URL check. _open_url forwards a redirect_validator to open_url; _fetch_single_catalog passes _validate_catalog_url through it while keeping the final geturl() check. - Give the legacy public fetch_catalog() single-catalog path the same redirect_validator + final geturl() validation (it previously parsed the body with no redirect check). Tests: an intermediate http hop is rejected, and the legacy fetch_catalog() rejects an HTTPS->http redirected payload (both fail before). Full test_extensions.py (356) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(extensions): cover legacy fetch_catalog() per-hop redirect validation The legacy fetch_catalog() regression test only exercised the terminal geturl() check, so it would still pass if the per-hop redirect_validator were dropped from that duplicated path. Add test_fetch_catalog_legacy_validates_every_redirect_hop, which asserts fetch_catalog() supplies a redirect_validator that rejects an insecure intermediate hop (fails before: the legacy path passed no validator -> NoneType not callable). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…b#3595) `GateStep.validate` rejects a non-list (or empty) `options` and requires every option to be a string, but the engine does not auto-validate before `execute`. On an unvalidated run a scalar/dict/None `options` reached `_prompt` and crashed the whole workflow with a raw `TypeError` (`enumerate`/`len` on a non-iterable) or `KeyError` (indexing a dict); an empty list spun `_prompt`'s input loop forever; a non-string option crashed the reject check at `choice.lower()` with `AttributeError`. Guard `execute` to FAIL the step cleanly instead, before the non-TTY PAUSE short-circuit so the error surfaces in CI too rather than pausing and only crashing later on interactive resume. Mirrors the switch 'cases' and command 'input' unvalidated-execute guards. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#3596) `CommandStep.validate` only checked that a `command` field is *present*, never its type. On an unvalidated run (the engine does not auto-validate before `execute`) a non-string `command` — null, a list, an int — was passed straight through `_try_dispatch` to the integration's `build_command_invocation`, which does `command_name.startswith("speckit.")` and crashes the whole workflow with a raw `AttributeError` once a resolvable integration with an installed CLI is found. Guard both paths, mirroring the sibling steps: - `validate()` rejects a non-string `command` (like prompt-step `prompt` github#3582 and shell-step `run`). - `execute()` fails the step cleanly with the same contract error before dispatch (like the existing `input`/`options` guards in this file), so an unvalidated run FAILs the step instead of crashing the run. An expression like `{{ inputs.cmd }}` is still a string, so it stays valid. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: bump version to 0.13.2 * chore: begin 0.13.3.dev0 development --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…hub#3601) A workflow input whose `enum` is a scalar or string (e.g. `enum: 5`, `enum: "abc"`) previously slipped past `validate_workflow` and crashed at run time. The `value not in enum_values` membership test in `_coerce_input` raises a raw `TypeError` ("argument of type 'int' is not iterable") for a scalar, and a bare string turns enum membership into a silent substring test. The `TypeError` also escapes `validate_workflow`'s `except ValueError`, breaking its documented "return a list of errors, never raise" contract. This is the same unvalidated-`execute()` crash class as the fan-in `wait_for` (github#3482) and fan-out step-template (github#3537) fixes: `validate()` should reject the value, but the value can still reach the engine via `execute()`, which accepts unvalidated definitions. Fix: - `_coerce_input` requires a list `enum` (or `None`), raising a clean ValueError for any other shape — so both `validate_workflow` and runtime `_resolve_inputs` fail fast with a clear message. - `validate_workflow` checks `enum` shape directly (not only via the default-coercion path, which is reached only when a `default` exists), and strips a malformed `enum` before coercing the default so the wrong-typed-default error is not duplicated as an enum-shape error. - The `integration: auto` sentinel only strips a *list* `enum`; a non-list `enum` stays in the definition so it is rejected rather than silently exempted by the `auto` membership skip. Tests cover all three layers: `_coerce_input` directly, authoring-time `validate_workflow` (with no default present), and runtime `_resolve_inputs`, plus the `integration: auto` interaction. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add intake-review-governance preset submitted by @hindermath to: - presets/catalog.community.json (alphabetical order) - docs/community/presets.md community presets table Closes github#3604 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
* ci: add dependency audit workflow Add a Security Audit workflow with a dependency-audit job. Push/PR/manual runs pip-audit against a committed --generate-hashes requirements snapshot (.github/security-audit-requirements.txt) for deterministic CI, while the weekly scheduled run resolves the runtime + test dependency set live across the supported Python/OS matrix to surface newly published advisories. A sync gate (.github/scripts/check_security_requirements.py) fails PRs whose dependency inputs changed without refreshing the committed snapshot, so the committed file can't silently drift from pyproject.toml. Assisted-by: Codex (model: GPT-5, autonomous) * ci: split dependency audit schedule matrix Assisted-by: Codex (model: GPT-5, autonomous) * ci: harden dependency audit sync checks Assisted-by: Codex (model: GPT-5, autonomous) * ci: align security workflow python pin Assisted-by: Codex (model: GPT-5, autonomous) * ci: refresh dependency audit baseline Assisted-by: Codex (model: GPT-5, autonomous) * docs: clarify security snapshot audit Assisted-by: Codex (model: GPT-5, autonomous)
…& prompt steps (github#3597) * fix(workflows): reject a non-string 'integration'/'model' in command & prompt steps A non-string `integration` on a command or prompt step is passed to `get_integration()`, which uses it as a dict key: an unhashable list/dict raises a raw `TypeError` there — and because neither `validate()` nor `validate_workflow` checked the type, this crashes even a *validated* run, not just an unvalidated one. A non-string `model` likewise reaches `build_exec_args()` and is fed into the CLI argv. Guard both fields in `validate()` (reject a literal non-string, mirroring the existing 'command'/'prompt'/'input'/'options' checks) and in `execute()` (fail the step cleanly rather than take down the whole run, mirroring the 'input'/'options' guards). An explicit YAML-null (inherit the workflow default) and a "{{ ... }}" expression both stay valid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(workflows): route falsey non-string integration/model to the type guard Address Copilot review: `config.get("integration") or context.default_integration` (and the model equivalent) coerced a *falsey* non-string ([], {}, 0, False) into the workflow default before the type guard ran. On an unvalidated execute() such a step was silently accepted and — with a configured default — could dispatch using the wrong integration/model instead of failing with the contract error. Fall back to the workflow default only for genuinely-unset values (missing / YAML-null / empty string) so every non-string reaches the guard. Add parametrized falsey execute() cases ([], {}, 0, False) to both TestCommandStep and TestPromptStep; with the fix stashed all 8 fail (swallowed into the default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: clarify hook priority validation semantics * docs: clarify stored hook priority normalization --------- Co-authored-by: root <kinsonnee@gmail.com>
) * fix: bound response reads in extension catalog and download Replace unbounded esponse.read() calls with ead_response_limited() from _download_security in extensions/__init__.py to prevent denial- of-service via oversized catalog or extension archive responses. Three call sites fixed: - _fetch_single_catalog JSON read (catalog metadata) - _fetch_catalog JSON read (legacy path) - download_extension ZIP read (binary download) All existing mock tests updated to use side_effect with BytesIO.read instead of eturn_value, ensuring compatibility with the chunked read loop in ead_response_limited. Two regression tests added: - test_oversized_catalog_response_rejected - test_oversized_extension_download_rejected * fix: remove .decode utf-8 to preserve bytes for json.loads json.loads accepts bytes directly. Removing .decode maintains compatibility with BOM-bearing or UTF-16/32 catalogs.
…t init time (github#3914) * Add --extension flag to specify init for installing extensions at init time Adds a repeatable --extension flag to `specify init` so users can opt into extensions (bundled name, local path, or HTTPS URL) during initialization, without a separate `specify extension add` step. - New `_install_extension_during_init` helper in commands/init.py that auto-detects source type (URL / local path / bundled name / catalog) and installs via ExtensionManager. Failures are non-fatal and recorded in the tracker without aborting init. - Extension tracker steps are pre-registered before the Live context and run after preset install, before finalize. - Five new tests in TestExtensionFlag covering bundled name, multiple extensions, local absolute path, unknown extension (graceful error), and combination with --preset. Rebased onto upstream/main and adapted to the refactored init command (moved to src/specify_cli/commands/init.py) from stale PR github#2396. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address review: reuse hardened downloader, refresh events, escape labels, fix bundler call Responds to review feedback on github#3914 and fixes CI (pytest bundler failure). - Extract shared `install_extension_from_url` helper in extensions/_commands.py that reuses the authenticated, redirect-guarded, bounded (50 MiB) download and TOCTOU-safe transient archive used by `extension add --from`. Both `extension add --from` and `specify init --extension <url>` now go through this single downloader instead of a second raw urlopen path. - Refresh native event configuration once after successful extension installs during init (mirrors `_refresh_events_and_warn` in the add path) so an extension declaring `events:` has its hooks activated. - Escape user-controlled extension specs and error text before interpolating them into StepTracker labels (Rich markup injection). - Pass `extensions=None` from bundler's `_run_init` so the init callback no longer receives the typer OptionInfo sentinel ('OptionInfo' object is not iterable), which broke `test_install_initializes_uninitialized_project`. - Add init URL coverage in TestExtensionFlag: non-HTTPS rejection and a successful HTTPS ZIP install with download-cache cleanup assertion. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Add default-deny trust confirmation for URL extension installs at init URL-based --extension installs now require explicit trust, matching the `extension add --from` posture. Interactive sessions show an "Untrusted Source" panel and prompt (default no); non-interactive sessions deny by default unless --trust-extension-urls is passed. Trust is resolved before the Live display since the prompt can't be answered under the spinner. - Add --trust-extension-urls option and _ext_spec_is_url / _confirm_extension_url_trust helpers - Skip (not abort) unconfirmed URL extensions, consistent with other non-fatal extension failures - Pass trust_extension_urls=False from the bundler init callback - Add tests for deny-by-default, interactive confirm, and trusted install Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hub#3887) execute() did: output_config = config.get("output") or {} if not isinstance(output_config, dict): output_config = {} so every non-mapping `output` was silently discarded and the step still returned COMPLETED — every declared aggregation key vanished, and downstream `{{ steps.<id>.output.<key> }}` resolved to None and interpolated as an empty string: output=[] -> completed, error=None output=False -> completed, error=None output=0 -> completed, error=None output='' -> completed, error=None output=['a'] -> completed, error=None output='oops' -> completed, error=None output=5 -> completed, error=None `validate` already rejects this and its comment names the flaw exactly: "execute() silently coerces a non-mapping output to {}, so the author's declared aggregation keys would vanish with no error." The engine does not auto-validate before execute(), so on an unvalidated run that is what happened — and `x or {}` masked the falsy shapes before the isinstance check even ran. Fail loudly with validate()'s own message, mirroring the `wait_for` guard in the same method. An explicit `output:` (YAML null) stays valid. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: eliminate TOCTOU race in zip packaging Open file once and derive both stat info and content from the same file descriptor to prevent race conditions where the file is modified between stat() and read_bytes() calls. * test: add regression test for TOCTOU stat/read consistency in packager The old implementation called file_path.stat() then file_path.read_bytes() as separate syscalls. The fix opens the file once and uses os.fstat() + fh.read() on the same handle. This test verifies the archived bytes and mode are consistent with the opened file descriptor.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ithub#3888) execute() reads `on_reject = config.get("on_reject", "abort")` and, in the reject branch, handles only "abort" and "retry" before falling through to its `# on_reject == "skip"` case. So any other value makes a REJECTED gate report COMPLETED and the run walks straight past the review the gate exists to enforce: on_reject='abort' -> failed "Gate rejected by user at step 'g'" on_reject='retry' -> paused on_reject='skip' -> completed (by design) on_reject='Abort' -> completed <-- rejection silently discarded on_reject='fail' -> completed <-- same on_reject='stop' -> completed <-- same on_reject=None -> completed <-- same on_reject=5 -> completed <-- same Reachable by a capitalisation slip, a guessed verb, a non-string, or a bare `on_reject:` — note `config.get(k, default)` does NOT substitute the default for an explicit YAML null. `validate` already rejects anything outside abort/skip/retry, but the engine does not auto-validate before execute(). Fail loudly instead, mirroring the `options` and `verdict_input` guards in the same method, and placed before the non-TTY short-circuit so it surfaces in CI too. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Remove unused imports (pytest, Path, run_and_tee, etc.) - Remove unused local variables in evals extension tests - Replace run_and_tee import with importlib.util.find_spec - Regenerate .github/security-audit-requirements.txt with uv pip compile (updated dependency hashes) Assisted-by: opencode (model: litellm/glm-5.2, supervised)
The CI check script uses 'uv pip compile --universal' (not --python-version). Previous commit used wrong flags. Assisted-by: opencode (model: litellm/glm-5.2, supervised)
…s tests - Sync tests/test_authentication.py to upstream 0.14.4 version, adjusting provider 'gitlab' -> 'bitbucket' for test_unknown_provider_raises due to fork gitlab provider. - Add EXTENSION_ALIAS_PATTERN_ENABLED skip guards to alias group toggle tests in test_extension_skills.py. - Fix _FORK_HAS_TEE mocking in test_workflows.py prompt step test. - Port discovery functions & bash-compatible quoting to Python create_new_feature twin and add last_refresh normalization to parity tests. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…version - Update mock HTTP responses in test_extensions.py and test_presets.py to use BytesIO.read for read_response_limited chunked reading compatibility. - Update agent-context version in test_bundler_local_install.py from 1.2.0 to 1.3.0. - Update test_registrar_path_traversal.py assertion for fork alias-only registration mode. - Set bundled preset priority to 20 in _init_fork.py so user-installed presets take precedence. - Add dict type check guard to preset priority sorting in scripts/bash/common.sh. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…nventions - Use _cmd_prefix() in test_cli.py and test_integration_bob.py for fork command-prefix compatibility (spec- vs speckit-). - Support spec.*.md / spec-* patterns in Bob integration managed-artifact detection. - Commit explicit --number conflict handling in create-new-feature.sh and create-new-feature.ps1. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…mand-ref assertions in test_integration_bob - Patch open_url instead of urllib.request.urlopen in test_integration_catalog.py for 0.14.4 http security compatibility. - Allow fork command-ref prefixes (/spec-plan, /spec.plan) in test_integration_bob.py assertions. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…fork command prefix - Use _cmd_prefix() in test_integration_copilot.py for specify command template file path assertions. - Use _cmd_prefix() in test_integration_droid.py for build_command_invocation assertions. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…/grok tests - Update kilocode command path from .kilocode/workflows to .kilo/commands (upstream 0.14.4 dir change) in test_integration_forge.py. - Use _cmd_prefix()/_skill_prefix() for command invocation assertions in test_integration_grok.py. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…, and upgrade backfill tests - Expect empty local skills directory for Hermes in test_integration_hermes.py (global-skills integration). - Use _cmd_prefix() in test_failed_switch_keeps_fallback_metadata_consistent (-plan for codex). - Remove default presets before command<->skills layout change in test_upgrade_default_refreshes_shared_script_refs_for_option_separator_change (satisfies fork preset-guard). - Align test_upgrade_backfills_extension_commands_for_agent with upstream github#2948 behavior (non-active upgrade skips extension backfill). Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…nstall disjoint manifest check Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…install disjoint check Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…n-sync preset - Merge 31 upstream commits from 0.15.0 and 0.15.1 (commits 400ad01..7f40c82). - Adopt upstream tar archive install support, URL download cache hardening, TOCTOU safety improvements, and constitution-sync preset. - Retain fork identity (agentic-sdlc-specify-cli v0.15.1+adlc1), theming, bundled presets, and _*_fork modules. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…tting, and preset test assertions - Fix extension_update syntax and indentation in extensions/_commands.py. - Remove duplicate try: and fix syntax in presets/_commands.py. - Fix trailing bracket in presets/catalog.json. - Use _cmd_prefix() and alias-only skip guards in test_presets.py assertions. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…lidation - Update pyproject.toml wheel force-include path for bundled presets from specify_cli/bundled_presets/ to specify_cli/core_pack/presets/ (fixes test_wheel_bundled_presets.py). - Update Number parameter in create-new-feature.ps1 and create-new-feature-branch.ps1 to string type with explicit parsing to avoid PowerShell parameter binder crashes on non-numeric or oversized values. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…rnings with fork conventions - Write branch truncation warnings directly to stderr in create-new-feature.ps1 and create-new-feature-branch.ps1 so PowerShell matches Bash/Python twins. - Update test_extensions.py assertions to support fork command-prefix, accent theming, and alias-only mode skip guards. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…umber handling, and JSON discovery nesting - Convert DISCOVERED_DIRECTIVES and DISCOVERED_SKILLS JSON strings back to objects before outer ConvertTo-Json in create-new-feature.ps1 so JSON mode outputs nested objects. - Treat empty -Number parameter as omitted. - Truncate requested branch name before Test-Path check to avoid macOS path length errors. - Print persist hints to stderr in non-dry text mode. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…rflow & empty string handling - Detect Int64 overflow when computing highest number from existing spec directories and output matching error. - Support empty string -Number argument as omitted value. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…umber handling - Fix double-increment where Get-NextBranchNumber result was incremented a second time (+1). - Strip empty/whitespace -Number parameter before delegating to git extension script so empty number is treated as omitted. - Ignore out-of-range directory prefixes during highest-number scanning. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
…across twins - Bash: treat empty --number '' as omitted (skip explicit-number conflict path), matching Python's falsy check and PowerShell's bound-param removal. - PowerShell: emit SPECIFY_FEATURE persist hints to BOTH stdout and stderr in non-dry text mode, mirroring the bash/python twins. Assisted-by: opencode (model: gemini-3.6-flash, supervised)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upstream merge of 232 commits across 13 releases (0.12.16 → 0.14.4). The upstream PR github#3704 (authored by @kanfil) reworked the fork's
_hooks_fork.pyinto a first-classevents.pysystem — the fork module is retired in favor of the upstream implementation.Key changes
Hooks → Events migration (core)
_hooks_fork.py+test_hooks_fork.py; adopted upstreamevents.py(2096 lines) +test_events.py(2226 lines)CANONICAL_TO_NATIVE/events_config_file/events_format) instead of fork--hooksoptionruntime_hooks: SessionStart→events: session_start+scripts:frontmatterFork identity preserved
resolve_command_refs: merged fork'sproject_root(preset alias) + upstream'sprefix(invocation-style//$//skill:)build_command_invocation: keeps fork convention (always/prefix,COMMAND_PREFIX)_cleanup_replaced_commands(replaces: feature) +inject_model_invocation_flagpreservedaccent()/make_typer()re-applied on all merged modules alongside upstream's_escape_markupescapingUpstream security adopted
_download_security.py), zip-bomb protection, TOCTOU fixesNew upstream features
assessextension, conventional commits,build_python_invocation/select_script_variant,is_skills_mode(),register_enabled_presets_for_agent(),unregister_agent_artifacts()Test fixes
read_response_limited(BytesIO side_effect)registered_skillsassertions (flat list → per-agent dict)only_agentkwarg)Bug fix
--team-ai-directivesrelative path now resolves early ininit(); improved error message showing resolved pathCommits
6ca8c3a9— merge upstream 0.14.4, retire _hooks_fork.py, adopt events (feat: first-class agent-native runtime hooks for integrations github/spec-kit#3704)f7b9c089— fix: update test mocks for upstream download-security + events migration8637b623— fix: ruff lint + team-ai-directives path resolution + error messageAssisted-by: opencode (model: litellm/glm-5.2, supervised)