mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
a9c990540099087e215b5d4ce8f1d3448169dcac
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Pascal THUET
|
118062eac4 |
harden: secure extension and preset archive downloads (#3141)
* harden: secure extension and preset archive downloads Adopt the shared download-security primitives from #3140 across extension and preset catalog, package, direct-URL, and ZIP-install flows: - bound catalog, package, and inline manifest reads; - verify catalog SHA-256 values when present; - replace path-only extraction with bounded traversal/symlink-safe extraction; - validate malformed hosts and ports before opening download URLs; - handle normalized trailing-backslash directory entries consistently. Redirect enforcement and checksum verification remain owned by the shared helpers already on main; this commit wires them into extension and preset behavior. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close archive and catalog download edge cases Preflight ZIP central directories before ZipFile allocates them, bound both declared and actual payload sizes, and reject ambiguous or non-portable archive paths before extraction. Keep extension update manifest selection consistent with extraction, reject unsafe catalog-derived output filenames and malformed URL types, and escape untrusted values in download errors. Add regression coverage for parser differentials, collisions, platform-specific filenames, bounded call sites, and failure ordering. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: address download security review feedback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close ZIP preflight review gaps Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update preflight and rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
Ali jawwad
|
52b20f1a82 |
fix(bundler): InstallResult.changed counts uninstalled as a change (#3692)
* fix(bundler): InstallResult.changed counts uninstalled as a change The `changed` property only considered `installed` and `refreshed`, omitting `uninstalled`. A `bundle update` whose new manifest drops components (removing them via the refresh path) with no new install/refresh produces installed=[], refreshed=[], uninstalled=[dropped set] — yet `changed` returned False, misreporting a mutating update as a no-op. Include `uninstalled` in the disjunction (it is the third mutating outcome list on the same dataclass, also the sole output of the remove_bundle path). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: pin ruff to 0.15.0 to avoid 0.16.0 default-ruleset breakage ruff 0.16.0 expanded its default rule set from ~59 to ~413 rules, causing the unpinned `uvx ruff check` step to report ~1475 pre-existing violations unrelated to this change. Pin to 0.15.0 to restore green lint. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> |
||
|
Ben Buttigieg
|
5760061316 |
Add community bundle submission automation (#3553)
* 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 |
||
|
Marsel Safin
|
d7b6626218 |
feat(workflows): align workflow CLI with extension command surface (#3419)
* feat(workflows): align workflow CLI with extension command surface Adds the missing workflow commands and flags so the workflow CLI matches the extension/preset pattern: add --dev and --from, search --author, update, enable and disable. Disabled workflows are blocked from running and marked in list output. Fixes #2342 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): preserve disabled state on update, guard corrupted registry entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard list against corrupted registry entries, re-raise typer.Exit in catalog install workflow list now skips non-dict registry entries with a warning instead of crashing, matching update/enable/disable. The broad except in _install_workflow_from_catalog no longer swallows typer.Exit, so precise errors like the non-HTTPS redirect message are not duplicated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in id-mismatch errors and validate --from source early The two id-mismatch error paths interpolated repr() into Rich markup, so a stray bracket in a user typo could be parsed as markup. Route both through rich.markup.escape. `workflow add <source> --from <url>` also validated the source only after downloading. Validate it up front so a URL/path/typo fails without a network fetch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in list output and catalog install errors, isolate update failures workflow list now escapes id/name/version/description before printing, matching how extensions render user-editable fields. The catalog install helper computes safe_wf_id once and uses it for every early error path plus the final failure message. workflow update wraps _safe_workflow_id_dir and the backup read inside the try/except typer.Exit block so an unsafe id in a corrupted registry fails that one workflow and the rest continue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in --from download exception message Matches how the catalog install path escapes exception strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): catch OSError in per-workflow update loop and make restore best-effort Transient FS errors (perms, disk full) from backup read or write no longer abort the whole update run. The restore is wrapped in its own try/except so a failed write only warns, and the offending workflow is reported via 'Failed to update' like other per-workflow failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in search output workflow search now escapes catalog-derived name/id/version/description/ tags before printing, matching extension search and workflow list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: escape workflow validation errors before Rich output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape remaining unescaped Rich markup paths Covers the last few review threads not yet addressed: - Escape yaml.YAMLError text in the local workflow add install path (matches the already-escaped download/catalog paths). - Escape the non---dev local directory fallback's "No workflow.yml found in <path>" message (the --dev branch already escaped it). - Escape the redirected final_url in the --from non-HTTPS redirect error (IPv6 literals like http://[::1]/... are legal and contain brackets). - Escape the "Downloaded workflow is invalid" exception message in _install_workflow_from_catalog, matching the sibling catalog-install exception handler a few lines above it. Adds regression tests for each in TestWorkflowCliAlignment, following the existing escaping-test pattern in this class. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape workflow name/id in install success messages Workflow names and ids come from user-controlled YAML or external catalog data; printing them unescaped lets bracket characters be interpreted as Rich tags. Escape them in the add/catalog-install success messages and the remaining catalog error paths, matching the rest of the output hardening. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): fail cleanly on unparseable catalog install URLs urlparse raises ValueError on e.g. an unbalanced IPv6 literal before the invalid-URL branch is reached; on workflow update that also bypassed the per-workflow handler and aborted the whole command. Convert the parse failure into a clean error so add fails cleanly and update skips just the affected workflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): reject catalog updates whose downloaded version mismatches The update path never verified the downloaded workflow carries the catalog version that triggered the update, so a stale or misconfigured URL could report success while leaving the old version installed or downgrading it. Pass the expected version into the install helper and fail the update when the downloaded definition does not match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate workflow ID in run command and document new CLI flags Path-equivalent spellings like "align-wf/" previously bypassed the registry disabled check because the engine normalizes the path while the registry matches the raw string. workflow run now validates non-file sources against the workflow ID pattern before lookup. Also updates docs/reference/workflows.md with --dev/--from install options, update/enable/disable commands, and the search --author flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): enforce disabled state for direct paths to installed workflows Running the installed copy's YAML directly (specify workflow run .specify/workflows/align-wf/workflow.yml) skipped the registry check. File sources resolving inside .specify/workflows/<id>/ now map back to the workflow ID and refuse to run while disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): reject explicit empty --from URL instead of catalog fallback 'workflow add foo --from ""' fell through 'from_url or ...' to a catalog install. Distinguish None from empty string so explicit values stay on the URL-validation path and fail closed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): registry rollback on save failure, consistent disabled check, honest update summary - WorkflowRegistry.add now rolls back its in-memory mutation when save() raises, so a later successful save cannot persist metadata for a failed update alongside the restored YAML backup. - workflow run uses the same truthiness check for 'enabled' as list and disable, so malformed values like 0 or null refuse to run. - workflow update reports 'No workflows were eligible for update' when every target was skipped instead of claiming all are up to date. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard non-string catalog URL and keep enable/disable rollback intact - A truthy non-string catalog url (e.g. 123) reached urlparse and raised AttributeError, escaping the clean error path; validate it is a string. - enable/disable mutated the live registry entry before add(), so add's rollback snapshot captured the already-toggled object; pass a fresh mapping instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): tolerate non-dict registry entries in add and clarify test docstrings A corrupted-but-parseable registry entry (e.g. a string value) crashed WorkflowRegistry.add with AttributeError on existing.get. Guard the non-dict case while still restoring the original raw value on rollback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): atomic registry save and accurate mixed-target update summary - save() wrote the registry with open('w'), so a failed dump truncated the file and the next load reset every entry. Write to a sibling temp file and os.replace into place. - workflow update no longer claims all workflows are up to date when some targets were skipped; it reports checked-only status with a skipped count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): exclusive temp file for registry save and cwd-independent disabled guard - save() now uses tempfile.mkstemp in the workflows dir (matching the engine's atomic writer), so a pre-created symlink at a predictable .tmp path cannot redirect the write and concurrent processes cannot collide. - The direct-path disabled guard derives the owning project from the resolved file path instead of the caller's cwd, so running an installed workflow's YAML from outside the project still refuses when disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): symlink guards and shape validation in workflow registry, dev-dir file check - WorkflowRegistry now mirrors StepRegistry: _load refuses symlinked parents/registry file and normalizes a non-dict workflows field; save() rejects symlinked paths before writing. - workflow add --dev requires workflow.yml to be a regular file so a directory named workflow.yml gets the documented CLI error instead of an uncaught IsADirectoryError. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate download redirects before following them All three workflow download sites (add --from, catalog install, step install) passed no redirect_validator to open_url, so an HTTPS URL redirecting to cleartext HTTP issued the insecure request before the post-hoc geturl() check reported it. Shared validator now rejects non-HTTPS redirects (loopback HTTP allowed) pre-follow, matching the preset download path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(workflows): accept redirect_validator kwarg in step-add open_url fakes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard directory-shaped workflow.yml and unreadable registry - workflow add's plain local-path fallback (no --dev) checked wf_file.exists() before installing, so a directory literally named workflow.yml passed the guard and _validate_and_install_local() leaked an uncaught IsADirectoryError instead of the documented CLI error. Use is_file(), matching the --dev branch's existing guard. - WorkflowRegistry._load() treated any OSError while reading an existing registry the same as corrupted JSON, resetting to an empty in-memory registry. A later save() would then silently persist that empty state via os.replace, discarding every previously installed workflow entry. Track a _load_error flag on OSError-during-read and have save() refuse to write when it is set, so a transient I/O failure can no longer overwrite intact data on disk. - docs/reference/workflows.md: document `--from <url>` with its value placeholder, matching extensions.md and presets.md. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): rollback registry.remove() and guard OSError at CLI boundaries Critical: WorkflowRegistry.remove() deleted the in-memory entry then called save() with no rollback, unlike add(). Combined with workflow_remove deleting the workflow directory before calling registry.remove(), a save failure permanently destroyed the workflow's files, left the on-disk registry still claiming it installed, and surfaced a raw unhandled OSError with no CLI message. - WorkflowRegistry.remove() now rolls back the in-memory entry on a save() OSError, mirroring add()'s existing rollback pattern. - workflow_remove persists the registry removal (registry.remove(), wrapped in try/except OSError -> clean escaped message) before deleting any files, so a save failure never touches the workflow directory. Important sibling paths: workflow add (local/--dev/--from and catalog), enable, and disable all called registry.add() without catching its deliberate OSError, so a save failure surfaced either an orphaned install directory (fresh local/catalog installs) or a raw/unhandled exception with no clean CLI output. - _validate_and_install_local (backs local/--dev/--from) now removes the freshly created directory on a fresh install, or restores the prior workflow.yml bytes on a reinstall-over-existing-local install, before raising a clean escaped error. - _install_workflow_from_catalog wraps the final registry.add() using the function's own established convention (rmtree the just-downloaded workflow_dir, then a clean escaped error) -- workflow_update's existing backup/restore around this function is unaffected. - workflow_enable/workflow_disable catch registry.add()'s OSError and print a clean escaped message instead of leaking the exception. Added failing-first tests proving each behavior (registry-unit rollback test, CLI-level remove/add/enable/disable save-failure tests parametrized where they share one root cause), all confirmed red before the fix and green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): preserve prior catalog install on reinstall registry-save failure _install_workflow_from_catalog's final registry.add() failure handler unconditionally rmtree'd workflow_dir. That's safe for a brand-new install, but plain `workflow add <catalog-id>` also allows re-adding an already-installed workflow, downloading the new version over the existing directory first. If registry.add() then failed to save, the unconditional rmtree deleted the prior working install while the registry (after its own rollback) still reported it installed -- data loss with no way back. workflow_update already avoids this via an outer backup/restore around this function, but plain add has no such caller. Fix mirrors _validate_and_install_local's existed-before/backup-aware handling: capture whether workflow_dir existed and back up its workflow.yml bytes before any download write, then on a registry.add() OSError, restore those bytes for a reinstall or rmtree only a brand-new directory. Only one file (workflow.yml) is ever written by this path, so no further per-file bookkeeping is needed. Added a failing-first regression: install a catalog workflow, re-add it with a simulated registry save OSError, and assert a clean error, the original workflow.yml restored byte-for-byte, and the registry still reporting the original version installed. Confirmed red (prior file deleted) before the fix, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): centralize catalog-install cleanup across all failure branches _install_workflow_from_catalog is new in this PR and has seven failure branches after the mkdir/download step, each independently rmtree'ing workflow_dir: redirect-to-non-HTTPS rejection, a generic download exception, invalid downloaded YAML, a validate_workflow failure, a workflow-id/catalog-key mismatch, a version mismatch, and (fixed in the prior commit) a registry.add() OSError. Only the last one had been special-cased to spare a prior working install on reinstall; the other six still unconditionally deleted the whole directory, so re-adding an already-installed catalog workflow and hitting any of those six earlier failures destroyed the working install even though nothing about it had actually changed. Replaced all seven ad hoc rmtree call sites with a single local _cleanup_failed_install() helper that closes over the existed_before / prior_workflow_bytes captured once at the top of the function: restore the prior workflow.yml for a reinstall, or rmtree only a directory that this attempt itself created. Every failure branch now calls this one helper, so the fix is structural rather than duplicated, and every existing error message/exit code is unchanged -- only the cleanup performed before each message is different. Added a parametrized regression test covering the four early-failure trigger points reachable from plain workflow add (redirect rejection, download exception, invalid YAML, ID mismatch): each installs a catalog workflow, re-adds it while forcing that specific failure, and asserts a clean error plus the original workflow.yml surviving byte-for-byte. Confirmed red against the unfixed code (all four raised FileNotFoundError reading the deleted file) before applying the helper, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): restore registry entry verbatim on post-removal rmtree failure workflow_remove now persists registry.remove() before deleting any files (fixed previously), but if the registry write succeeds and the subsequent shutil.rmtree(workflow_dir) then fails, the registry was left claiming the workflow uninstalled while its directory remained on disk -- an orphaned install with no path back to a clean state. workflow_step_remove already handles this exact sequencing by capturing the registry entry before removal and restoring it directly into registry.data plus save() (bypassing add(), which would stamp a new updated_at) if the directory removal fails afterwards. Applied the same pattern to workflow_remove: capture registry_metadata via registry.get() before registry.remove(), and on an rmtree OSError, write it straight back into registry.data["workflows"][workflow_id] and save(), matching workflow_step_remove's restore-failure handling (a yellow warning, not a hard failure, since the primary error is already about to be reported). Existing error message and exit behavior for the rmtree failure are unchanged. Added a failing-first regression: install a workflow, monkeypatch shutil.rmtree to raise OSError, and assert a clean existing error message, the directory remaining (rmtree never actually deleted anything), and the registry entry restored byte-for-byte identical (including installed_at/updated_at) -- proving the fix bypasses add() and doesn't re-stamp timestamps. Confirmed red (registry entry stayed None) before the fix, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix 4 current Copilot review findings on workflow run/registry/install 1. workflow run ownership check followed symlinks via Path.resolve() before mapping a direct YAML path back to its installed workflow ID. A symlinked .specify/workflows/<id>/workflow.yml resolved outside the tree, missed the ownership match entirely, and let the disabled-workflow guard be silently skipped while engine.load_workflow still followed the symlink. Now maps ownership from a lexically-normalized path (os.path. normpath, no symlink following) and explicitly refuses to run if the installed <id> directory or workflow.yml leaf is itself a symlink. Direct external workflow paths that don't match .specify/workflows/... are unaffected. 2. WorkflowRegistry._load() caught a read OSError and silently fell back to an empty in-memory registry, only blocking a later save(). Callers that only query is_installed()/get()/list() before writing a file (e.g. commands/init.py's bundled speckit install, which overwrites workflow.yml once is_installed() reports false) could act on that false-empty state and destroy real data before ever reaching save(). _load() now raises OSError immediately so an unreadable registry fails closed at construction, before any query or side effect is possible. Added _open_workflow_registry() to give every CLI command a consistent clean-error boundary around registry construction. 3. _validate_and_install_local's mkdir/copy2 ran before the try/except that protected registry.add(); a copy2 failure (e.g. a truncating partial write on a reinstall) was not caught at all, so the existing backup-restore cleanup never ran and the prior working workflow.yml was corrupted with a raw traceback surfaced to the user. mkdir/copy2 now run inside the same rollback-protected section as registry.add(), sharing one _cleanup_failed_install() helper. 4. workflow update's skip message claimed any non-catalog source was installed "from a local path or URL", which is wrong for the bundled speckit workflow (source: "bundled"). Message is now source-neutral. Verified all 4 threads are current (not outdated) via GraphQL review thread query on PR #3419, HEAD |
||
|
Ali jawwad
|
ba1f13a8b1 |
fix(bundle): reject file:// / local download_url — catalog URLs are HTTPS-only (#3344)
* fix(bundle): resolve file:// download_url via the file-URL helper _download_manifest built the local path from raw parsed.path, which keeps the leading slash of file:///C:/x (yielding a \C:\x path that never exists on Windows) and skips percent-decoding (my%20bundles stays encoded on every OS) — so a catalog entry whose download_url is the canonical URI Python itself produces via Path.as_uri() always fails with 'Bundle manifest not found'. Route the file scheme through the existing bundler.services.adapters._file_url_to_path helper, which already handles drive letters, UNC hosts, and percent-decoding for catalog file:// URLs (make_catalog_fetcher). The bare-path branch is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bundle): reject file:// / local download_url; catalog URLs are HTTPS-only Per maintainer review (route B): file:// in a catalog download_url was never intended — catalog URLs are HTTPS-only (http for localhost) across the extensions/presets/workflows catalog systems, and disk installs go through the positional path (specify bundle install <path>), handled by _local_manifest_source before catalog resolution. Remove the file:///bare-path branch from _download_manifest and route everything through _download_remote_manifest (HTTPS-only via _require_https), with an actionable error pointing at the positional install. Invert the file:// tests to assert rejection (+ a positional-path resolution test), and migrate the three bundle-info contract tests off local download_urls onto an HTTPS-only entry with a mocked manifest fetch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bundle): validate HTTPS before the offline gate in _download_manifest Per review: for a non-local download_url the offline check ran before any URL validation, so an invalid/non-HTTPS scheme surfaced a misleading 'Network access disabled' error under --offline when the real problem is the URL would be rejected even online. Call _require_https before the offline gate so the correct error is reported in every mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bundle): reword non-HTTPS download_url error to not mislabel scheme-less URLs A scheme-less download_url (urlparse scheme == "") can be a bare filesystem path OR a missing-scheme value like 'example.com/foo.zip', not necessarily file://. Reword the reject error to state the real HTTPS-only constraint and enumerate what is rejected (file://, local path, scheme-less), instead of labeling every case 'local/file://'. Behavior unchanged; the 'bundle install' actionable hint is preserved, so the existing reject-path tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ali jawwad
|
d5ba062eab |
fix(bundler): bundle update uninstalls components dropped by new version (#3353)
On refresh (bundle update), install_bundle iterated only the new plan's components, so a component the previous version owned but the new one no longer ships was left installed on disk while being dropped from the rewritten record (contributed only holds plan.components). With no record referencing it, remove_bundle could never clean it up — permanently orphaned, violating the provenance invariant (FR-022). After the component loop, when refresh and a prior record exists, uninstall each previously-owned component absent from the new plan — unless another bundle still needs it (components_still_needed refcount, mirroring remove_bundle), in which case it stays installed and is simply de-attributed. Runs inside the existing try so a failed removal takes the same no-record-written rollback path. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Quratulain-bilal
|
b5f1194168 |
fix(bundler): resolve catalog search at highest-precedence source before filtering (#3331)
CatalogStack.search() claimed a bundle id in `seen` only when the entry matched
the query. so when the highest-precedence entry for an id did NOT match, a
lower-precedence entry with the same id could match and be returned instead --
even though resolve()/install always use the highest-precedence entry. search
advertised a bundle (name, version, source) the user could never actually get,
contradicting the method's own docstring ("resolved at its highest-precedence
source").
resolve every id to its highest-precedence entry first, then filter the
resolved set by the query. search now agrees with resolve(): a query that only
a shadowed lower-precedence copy matches returns nothing.
add a regression test covering the shadowed-entry case.
|
||
|
Pascal THUET
|
490566847c |
feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver (#3186)
* feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver The shell resolver honors SPECIFY_INIT_DIR (#2892), but the Python CLI did not: it resolved the project as Path.cwd() + a .specify/ check and never read the override. So setup-plan.sh respected it while `specify integration install` ignored it, and you still had to cd into the member project. Route project resolution through a shared _resolve_init_dir_override() that applies the shell resolver's validation rules (relative to cwd, must exist and contain .specify/, hard error, no fallback, same error strings). It's wired into _require_specify_project() — the chokepoint for every project-scoped subcommand (integration/extension/workflow/preset/...) — and the `workflow run <file>` standalone path, which re-applies its symlinked-.specify guard on the override branch too. init is unchanged: it creates .specify/, so the must-pre-exist rule doesn't apply. The resolver canonicalizes symlinks via Path.resolve() while the shell keeps the logical path; they agree for non-symlinked paths (documented in the resolver). Tests in tests/test_init_dir_cli.py mirror the strict cases from test_init_dir.py through the CLI; conftest now strips SPECIFY_* for the whole suite so a stray export can't perturb the now-env-reading resolver. Docs note the CLI applies the same rules. Discussion: github/spec-kit#2834 (Disclosure: I used an AI coding agent to audit the call sites and resolver, draft the change, and run an adversarial code review; reviewed by me.) * fix(cli): honor SPECIFY_INIT_DIR for bundle commands Assisted-by: Codex (model: GPT-5, autonomous) * fix(bundler): refuse symlinked .specify on the SPECIFY_INIT_DIR override path find_project_root refuses a symlinked .specify (following it could read/write outside the tree, and a test pins that), but the SPECIFY_INIT_DIR override added for bundle commands returned early and skipped that guard: _resolve_init_dir_override validates .specify with is_dir(), which follows symlinks. So `specify bundle` accepted via the override a layout the cwd path rejects. Re-check the override result with the same guard, plus a regression test. (Disclosure: found via an AI code review and fixed with an AI coding agent; reviewed by me.) * fix(cli): keep SPECIFY_INIT_DIR strict for bundles Treat an explicit symlinked SPECIFY_INIT_DIR project as a hard bundle error instead of returning no project, which could initialize the current directory. Align the docs with the actual unset resolver behavior. Assisted-by: Codex (model: GPT-5, autonomous) * docs(core): note symlinked .specify handling differs across CLI surfaces A symlinked .specify is followed by integration/extension/workflow (matching the shell resolver) but refused by bundle and workflow run <file> (write confinement). Document the asymmetry so it reads as intentional. (Disclosure: AI-assisted; reviewed by me.) * docs(core): reframe symlinked .specify note around the override invariant Per maintainer feedback on #3186: SPECIFY_INIT_DIR relocates where the project is, not how a surface treats symlinks. Each surface keeps its cwd-path stance (write surfaces refuse a symlinked .specify, read/config surfaces follow it), so the split is one policy relocated, not an inconsistency. * docs: address Copilot review on resolver docstrings - _project.py: the error messages "mirror" the shell wording rather than "match" it (the CLI renders a Rich `Error:` line, the shell a plain `ERROR:`). - find_project_root: document that honoring SPECIFY_INIT_DIR when start is None can raise typer.Exit / BundlerError, so the Path | None signature isn't surprising to direct callers. * docs(bundler): note require_project_root inherits the override raise behavior find_project_root can raise typer.Exit / BundlerError under the SPECIFY_INIT_DIR override (start=None); require_project_root inherits that, so document it alongside its own BundlerError-on-missing-project. * docs: clarify symlinked project root behavior Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * Address SPECIFY_INIT_DIR review feedback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * Route workflow JSON errors to stderr Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
Manfred Riem
|
487af97864 |
feat: add specify bundle command (#3070)
* docs: dogfood Spec Kit — bundler SDD artifacts + constitution Scaffold Spec Kit (--integration copilot) and run the full SDD workflow against the `specify bundle` subcommand feature: - spec.md (4 user stories, 31 FRs, 8 success criteria) + clarifications - plan.md, research.md, data-model.md, contracts/, quickstart.md - tasks.md (43 dependency-ordered tasks, organized by user story) - Spec Kit Constitution v1.0.0 (code quality, testing, UX, performance, dependency/security principles) derived from deep codebase analysis - plan Constitution Check + tasks grounded against the ratified principles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(bundler): add `specify bundle` subcommand for role-based setups Implements the Spec Kit Bundler as a `specify bundle ...` subcommand group that calls existing primitive machinery in-process with zero new dependencies, per the v1.0.0 constitution (Principles I-V). Adds the `specify_cli.bundler` package (models, services, lib helpers) and the `commands/bundle` Typer group wiring search, info, list, install, update, remove, validate, build, init, and catalog list/add/remove (with --json and --offline). Includes manifest/catalog schemas, version + integration-clash gating, discovery-only refusal, idempotent install with atomic rollback, non-collateral removal, and offline-first catalog resolution. Ships an 82-test suite (contract/unit/integration), four sample role bundles (product-manager, business-analyst, security-researcher, developer), README "Bundles" docs, and an AGENTS.md pitfall on the test-venv gotcha. Marks tasks T001-T043 complete and records follow-ups T044 (live in-process primitive dispatch) and T045 (install from a local artifact path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(contributing): document running the full test suite via project .venv Add a "Running the full test suite" subsection under Automated checks covering `uv pip install -e ".[test]"` + `.venv/bin/python -m pytest`, with the shared/global editable-install contamination caveat that mirrors the AGENTS.md pitfall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(bundler): wire real in-process primitive install + local-artifact install Closes the two follow-ups left after the initial bundler landing. T044 — DefaultPrimitiveInstaller now performs real installs through existing machinery instead of raising "use the primitive command" errors: - presets/extensions install via their reusable managers (install_from_directory / install_from_zip); bundled assets install fully offline, catalog assets are fetched only when the network is allowed. - workflows/steps delegate to the existing `workflow add` / `workflow step add` command callables in-process (project root as cwd), avoiding any duplicated download/validation logic (Principle I). - `--offline` is threaded through DefaultPrimitiveInstaller(allow_network=…) so network-only kinds refuse with an actionable message rather than silently reaching out. T045 — `specify bundle install` now accepts a local path (a built .zip artifact, a bundle directory, or a bundle.yml) and installs directly without consulting the catalog stack; bundle-ids still resolve via the stack. Adds 13 tests (routing, offline gating, local-source resolution, and an end-to-end offline build → install → list → remove of the bundled agent-context extension). Bundler suite: 95 passing; ruff clean. Marks T044 and T045 complete in tasks.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(bundler): append Phase 8 convergence tasks from converge assessment Ran the converge command: assessed the codebase against spec.md, plan.md, tasks.md, and the v1.0.0 constitution. Appended 7 traceable gap-closure tasks (T046–T052) as a new "Phase 8: Convergence" section. Append-only — no existing tasks were modified and no application code was changed. Findings: 1 CRITICAL (Constitution III — bundle group undocumented under docs/reference/), 3 HIGH (FR-005/SC-007 validate references; FR-009/SC-002 info expansion; FR-012 install-time init), 3 MEDIUM (FR-013 integration precedence; FR-020 surface overlaps; FR-028 update refresh). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Implement Phase 8 convergence tasks (T046–T052) Close the gaps the converge command found between the bundler spec/plan/ constitution and the code: - T046: add docs/reference/bundles.md documenting the full `specify bundle` command group; link it from docs/reference/overview.md (Constitution III). - T047: wire a reference checker into `bundle validate` (services/references.py); online runs fail and name unresolved component references, offline runs warn. - T048: expand `bundle info` to enumerate the full component set (versions, preset priority/strategy) plus the bundle integration — info == install. - T049/T050: `bundle install`/`bundle init` now scaffold an uninitialized project via the existing `specify init` machinery, choosing the integration by precedence (override → bundle-declared → Copilot + OS default script type). - T051: surface foreseeable component overlaps during info and install. - T052: `bundle update` refreshes already-installed components via a new refresh path in install_bundle, preserving primitive-level overrides. Adds unit/contract/integration coverage (107 tests pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * converge: append Phase 9 (T053) — surface bundle trust indicator Re-run of converge after Phase 8. The seven Phase 8 tasks are verified closed. One residual partial gap remains: the `verified`/trust indicator (FR-010, FR-027) is exposed only in `bundle info --json`, absent from `bundle search` (the primary discovery surface) and `bundle info` text. Appended as a single new task for implement to complete. Append-only; no code changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Implement T053 — surface bundle trust indicator in discovery `bundle search` (text + JSON) and `bundle info` (text + JSON) now expose each catalog entry's verification/trust level (verified vs community), so users can judge a bundle's trust before installing, per FR-010 / FR-027. Previously `verified` was only present in `bundle info --json`. Adds contract coverage; 108 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: dogfood Spec Kit — bundler SDD artifacts + constitution Scaffold Spec Kit (--integration copilot) and run the full SDD workflow against the `specify bundle` subcommand feature: - spec.md (4 user stories, 31 FRs, 8 success criteria) + clarifications - plan.md, research.md, data-model.md, contracts/, quickstart.md - tasks.md (43 dependency-ordered tasks, organized by user story) - Spec Kit Constitution v1.0.0 (code quality, testing, UX, performance, dependency/security principles) derived from deep codebase analysis - plan Constitution Check + tasks grounded against the ratified principles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(bundler): add `specify bundle` subcommand for role-based setups Implements the Spec Kit Bundler as a `specify bundle ...` subcommand group that calls existing primitive machinery in-process with zero new dependencies, per the v1.0.0 constitution (Principles I-V). Adds the `specify_cli.bundler` package (models, services, lib helpers) and the `commands/bundle` Typer group wiring search, info, list, install, update, remove, validate, build, init, and catalog list/add/remove (with --json and --offline). Includes manifest/catalog schemas, version + integration-clash gating, discovery-only refusal, idempotent install with atomic rollback, non-collateral removal, and offline-first catalog resolution. Ships an 82-test suite (contract/unit/integration), four sample role bundles (product-manager, business-analyst, security-researcher, developer), README "Bundles" docs, and an AGENTS.md pitfall on the test-venv gotcha. Marks tasks T001-T043 complete and records follow-ups T044 (live in-process primitive dispatch) and T045 (install from a local artifact path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(contributing): document running the full test suite via project .venv Add a "Running the full test suite" subsection under Automated checks covering `uv pip install -e ".[test]"` + `.venv/bin/python -m pytest`, with the shared/global editable-install contamination caveat that mirrors the AGENTS.md pitfall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(bundler): wire real in-process primitive install + local-artifact install Closes the two follow-ups left after the initial bundler landing. T044 — DefaultPrimitiveInstaller now performs real installs through existing machinery instead of raising "use the primitive command" errors: - presets/extensions install via their reusable managers (install_from_directory / install_from_zip); bundled assets install fully offline, catalog assets are fetched only when the network is allowed. - workflows/steps delegate to the existing `workflow add` / `workflow step add` command callables in-process (project root as cwd), avoiding any duplicated download/validation logic (Principle I). - `--offline` is threaded through DefaultPrimitiveInstaller(allow_network=…) so network-only kinds refuse with an actionable message rather than silently reaching out. T045 — `specify bundle install` now accepts a local path (a built .zip artifact, a bundle directory, or a bundle.yml) and installs directly without consulting the catalog stack; bundle-ids still resolve via the stack. Adds 13 tests (routing, offline gating, local-source resolution, and an end-to-end offline build → install → list → remove of the bundled agent-context extension). Bundler suite: 95 passing; ruff clean. Marks T044 and T045 complete in tasks.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(bundler): append Phase 8 convergence tasks from converge assessment Ran the converge command: assessed the codebase against spec.md, plan.md, tasks.md, and the v1.0.0 constitution. Appended 7 traceable gap-closure tasks (T046–T052) as a new "Phase 8: Convergence" section. Append-only — no existing tasks were modified and no application code was changed. Findings: 1 CRITICAL (Constitution III — bundle group undocumented under docs/reference/), 3 HIGH (FR-005/SC-007 validate references; FR-009/SC-002 info expansion; FR-012 install-time init), 3 MEDIUM (FR-013 integration precedence; FR-020 surface overlaps; FR-028 update refresh). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Implement Phase 8 convergence tasks (T046–T052) Close the gaps the converge command found between the bundler spec/plan/ constitution and the code: - T046: add docs/reference/bundles.md documenting the full `specify bundle` command group; link it from docs/reference/overview.md (Constitution III). - T047: wire a reference checker into `bundle validate` (services/references.py); online runs fail and name unresolved component references, offline runs warn. - T048: expand `bundle info` to enumerate the full component set (versions, preset priority/strategy) plus the bundle integration — info == install. - T049/T050: `bundle install`/`bundle init` now scaffold an uninitialized project via the existing `specify init` machinery, choosing the integration by precedence (override → bundle-declared → Copilot + OS default script type). - T051: surface foreseeable component overlaps during info and install. - T052: `bundle update` refreshes already-installed components via a new refresh path in install_bundle, preserving primitive-level overrides. Adds unit/contract/integration coverage (107 tests pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * converge: append Phase 9 (T053) — surface bundle trust indicator Re-run of converge after Phase 8. The seven Phase 8 tasks are verified closed. One residual partial gap remains: the `verified`/trust indicator (FR-010, FR-027) is exposed only in `bundle info --json`, absent from `bundle search` (the primary discovery surface) and `bundle info` text. Appended as a single new task for implement to complete. Append-only; no code changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Implement T053 — surface bundle trust indicator in discovery `bundle search` (text + JSON) and `bundle info` (text + JSON) now expose each catalog entry's verification/trust level (verified vs community), so users can judge a bundle's trust before installing, per FR-010 / FR-027. Previously `verified` was only present in `bundle info --json`. Adds contract coverage; 108 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address PR review — annotations, Windows paths, HTTPS, errors, reproducible builds Resolves automated review feedback on github/spec-kit#3070: - validator: drop redundant string-quoting on ReferenceChecker's `str | None` return so the annotation evaluates as a real union under `from __future__ import annotations`. - adapters: normalize Windows drive-letter paths (e.g. C:\...) to the local-file branch so offline file catalogs resolve on Windows. - adapters: enforce HTTPS (HTTP only for localhost) and require a host on remote catalog URLs before any network call, mirroring specify_cli.catalogs URL validation (MITM/downgrade protection). - adapters: pass `origin` to loads_json for local files and HTTP payloads so JSON parse errors name the real source instead of <string>. - manifest: parse component `priority` defensively, raising an actionable BundlerError on non-integer values instead of a raw ValueError. - packager: write zip members with a fixed timestamp + permissions so identical inputs yield byte-for-byte identical artifacts (genuinely reproducible builds), and strengthen the determinism test accordingly. Adds regression tests for priority validation, plain-HTTP/host rejection, and byte-level artifact reproducibility (111 bundler tests pass; ruff clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address PR review round 2 — nested output dir + file:// URLs - packager: when --output points inside the bundle directory, exclude the whole output subtree from collection so previously-built artifacts are never re-packaged (prevents broken reproducibility and unbounded growth). - adapters: resolve file:// catalog URLs via url2pathname and preserve netloc, so Windows file URLs (file:///C:/...) and UNC shares (file://server/share) resolve correctly instead of dropping the host or producing /C:/x. Adds regression tests for nested-output exclusion and file:// resolution (113 bundler tests pass; ruff clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address PR review round 3 — discovery UX + hardening - bundle search/info: fall back to the built-in/user catalog stack instead of requiring a Spec Kit project, so discovery works in a fresh directory (and the README/quickstart examples now match actual behavior). install still auto-initializes a project as before. - packager: traverse with os.walk(followlinks=False) and prune symlinked directories before descending, so a symlink-to-dir can no longer pull in out-of-tree files (which previously turned "skip symlinks" into a hard ensure_within() failure and did extra filesystem work). - records: parse contributed-component priority defensively, raising an actionable BundlerError on a corrupt records file instead of leaking a raw ValueError/traceback. - installer: give install_bundle's manifest parameter an explicit BundleManifest | None type for a clearer, safer service API. Adds regression tests for project-less search/info, symlinked-dir pruning, and corrupt-priority records (117 bundler tests pass; ruff clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address PR review round 4 + markdownlint exclusions Review fixes: - bundle info: expand the manifest regardless of install policy so discovery-only bundles remain inspectable (only install is refused). - _download_manifest: handle local .zip download_url by extracting bundle.yml (via _local_manifest_source), and add a real remote HTTPS fetch path using the shared authenticated, redirect-validated open_url client (HTTPS enforced on the initial URL and every redirect; offline still refuses). - _run_init: thread the --offline flag through to the init callback so `bundle install/init --offline` never performs network init. - conflict.ConflictReport: use field(default_factory=list) and drop the None + __post_init__ workaround. - CatalogSource.from_dict: parse priority defensively, raising an actionable BundlerError naming the source + offending value instead of a raw ValueError. markdownlint: - Exclude .specify/, .github/, and specs/ (and their subdirectories) from markdownlint so the in-flight dogfooding scaffolding doesn't trip the linter. Adds regression tests for discovery-only info, local-zip download_url, and non-integer catalog priority (120 bundler tests pass; ruff clean; the PR's own markdown lints clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address PR review round 5 + ignore generated files in whitespace check Review fixes: - packager: exclude any prior build artifact for this bundle (matching <id>-*.zip), not just the current output path, so older artifacts next to bundle.yml are never re-packaged. - docs(bundles): correct the note — `search` and `info` work without a project (they fall back to the built-in/user catalog stack); only list/update/remove/ catalog require an initialized project. CI / generated files: - .gitattributes: mark the generated dogfooding scaffolding (.specify/**, the speckit .github agent/prompt files, copilot-instructions.md, specs/**) with -whitespace so `git diff --check` (the Lint workflow's whitespace gate) stops flagging emitted trailing whitespace. These files are produced by `specify init` and are scrubbed before merge. Adds a regression test for prior-artifact exclusion (121 bundler tests pass; ruff clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): collision-resistant catalog ids, canonical local paths, explicit uninstalled result Addresses review round 6 (PR #3070): - catalog_config._derive_id now combines host label with the URL path stem so multiple catalogs from the same host get distinct, stable default ids. - add_source canonicalizes local file paths to absolute before persisting, so project config no longer depends on the caller's cwd. - InstallResult gains a dedicated `uninstalled` list; remove_bundle no longer overloads `installed` for removals, and the CLI prints from `uninstalled`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): confine config writes, guard indeterminate integration, fix validate docs Addresses review round 7 (PR #3070): - save_records and catalog_config._write now pass within=project_root to dump_json/dump_yaml, refusing symlinked .specify paths that escape the project (defense-in-depth, matching the rest of the codebase). - resolve_install_plan now fails when a bundle pins an integration but the project's active integration cannot be determined and no explicit --integration override was given, instead of silently adopting the bundle's required integration (FR-019 guard). CLI passes integration_explicit. - docs/reference/bundles.md: corrected the validate semantics to describe the actual best-effort online behavior (unreachable catalogs warn, not fail). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): Windows path handling + review round 8 hardening Fix Windows CI failures: - is_safe_relpath now rejects POSIX-absolute (/abs) and Windows drive-absolute (C:\x, UNC) paths on every OS, instead of passing them through on Windows where os.path.isabs('/abs') is False and Path('/abs').parts yields '\\'. - _download_manifest treats a Windows drive-letter download_url (C:\bundle.yml, which urlparse reads as scheme 'c') as a local file, fixing the empty component set in `bundle info` on Windows. Address review round 8 (PR #3070): - Bundled workflows now install under --offline (locate via _locate_bundled_workflow) instead of being refused unconditionally. - bundle update preserves the original installed_at timestamp on refresh (import find_record; reuse the existing record's timestamp). - _derive_id lowercases the host label so 'Example.com' and 'example.com' produce the same deterministic id. - CatalogEntry.from_dict validates 'tags' is a list and 'verified' is a real boolean, raising BundlerError on invalid untrusted shapes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): normalize SemVer prerelease spellings before version parsing Addresses review round 9 (PR #3070): parse_version and is_semver now apply the same prerelease normalization (mirroring specify_cli._version._normalize_tag) so SemVer spellings like 1.2.3-rc1 / 1.2.3-alpha1 validate and compare consistently across is_semver, parse_version, and satisfies. Leading 'v' is also stripped. Keeps the manifest validator and constraint checks in agreement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): no collateral removal + enforce manifest-pinned versions Addresses review round 10 (PR #3070): - install_bundle records only the components this bundle actually contributed: freshly-installed components, plus pre-existing ones already owned by this bundle (refresh) or a sibling bundle (shared/refcounted). A component that is installed on disk but tracked by no bundle was installed independently and is no longer attributed, so `bundle remove` won't uninstall it (FR-022). - preset/extension/workflow install paths now verify the active catalog's advertised version matches the manifest-pinned component.version before downloading/installing, raising BundlerError on mismatch so bundles stay reproducible. When a catalog advertises no version the pin can't be enforced and installation proceeds. Added regression tests: independent pre-existing component survives removal; version-mismatch refusal (helper + workflow path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(scripts): add SPECIFY_INIT_DIR to target a member project from the repo root (#2892) * feat(scripts): add SPECIFY_INIT_DIR to target a member project from the repo root Resolve an explicit SPECIFY_INIT_DIR project override once in the core get_repo_root / Get-RepoRoot, so a non-interactive / CI caller can target a member project (the directory containing .specify/) from a monorepo root without cd. Strict by design: the path must exist and contain .specify/, otherwise it hard-errors with no silent fallback. - Single resolver in core; the git feature-branch script inherits it by sourcing core, with no per-extension copies. - PS resolver verifies the resolved path is a directory (Resolve-Path also succeeds for files) so a file value errors as "not an existing directory". - get_feature_paths splits decl/assignment so a SPECIFY_INIT_DIR failure propagates instead of being masked by `local`. - create-new-feature-branch: when core is absent (only git-common loaded) and SPECIFY_INIT_DIR is set, hard-error rather than silently using the git root. - Document SPECIFY_INIT_DIR and SPECIFY_FEATURE_DIRECTORY in the core reference. - Tests for valid/relative/trailing-slash/file/missing/no-.specify targets, feature-axis composition, the no-core guard, and a PowerShell mirror. * fix: guard SPECIFY_INIT_DIR with stale core scripts * docs: clarify SPECIFY_FEATURE_DIRECTORY precedence wording * fix: normalize trailing slash in PowerShell SPECIFY_INIT_DIR resolver Resolve-Path preserves a trailing separator from its input, so a SPECIFY_INIT_DIR ending in a slash returned a root that didn't match the bash resolver (whose `cd && pwd` strips it). That broke test_ps_trailing_slash_tolerated on the CI runners, which do have pwsh. Trim it with TrimEndingDirectorySeparator (no-op on a bare root or a path with no trailing separator). Also fix the misleading test comment: the PowerShell mirror runs on the CI ubuntu/windows runners (they ship pwsh), it is not skipped there. * test: normalize bash path expectations on Windows * docs: clarify SPECIFY_INIT_DIR root helpers * chore: sync dogfooded .specify core scripts with SPECIFY_INIT_DIR Mirror the SPECIFY_INIT_DIR resolver (resolve_specify_init_dir in common.sh) into the committed dogfooding .specify/scripts/bash copies so the git extension's create-new-feature-branch.sh finds an up-to-date common.sh instead of failing with "requires updated Spec Kit core scripts". Fixes the test_init_dir.py CI failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): harden remote catalog fetch and config parsing - adapters: route catalog HTTP fetches through the shared authenticated client (authentication.http.open_url) so auth.json tokens apply and the Authorization header is stripped on cross-host/downgrade redirects. Reject any redirect that leaves HTTPS via a redirect_validator and re-validate the final URL after redirects, closing the urlopen auto-redirect MITM/downgrade gap. - catalog_config._read: raise an actionable BundlerError when the config top level is not a mapping, 'catalogs' is not a list, or an entry is not a mapping, instead of letting list(<str>) produce a downstream AttributeError. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): tighten record read confinement, policy gate, and precedence Addresses review 4534504799: - records.load_records: confine the read via ensure_within(project_root, ...) so a symlinked/traversal-escaping .specify cannot read arbitrary files outside the project (matches the write path's within= guard). - catalog_config._slug: lowercase so derived catalog ids are deterministic across platforms and case-variant duplicates can't slip past the case-sensitive dup check. - installer.install_bundle: reword the docstring's misleading "atomic on failure" claim to describe the real scoped guarantee (record written only on full success; rollback limited to newly-installed components). - bundle update: enforce the source install_policy like install, refusing to update from a discovery-only source (FR-025). - catalog source precedence: the CLI now passes ~/.specify as the user config dir so project > user > built-in precedence is actually reachable (previously the user scope was silently ignored). - .gitattributes: scope the specs whitespace exemption to the generated dogfooding feature dir (specs/001-spec-kit-bundler/**) instead of all of specs/**. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): no collateral refresh, catalog id integrity, loud info Addresses review 4534571362: - installer: in refresh mode (bundle update) only re-apply already- installed components that this bundle (or a sibling) owns. Components installed independently and tracked by no bundle are now skipped, never refreshed, so update cannot make collateral changes (FR-022). - catalog.load_catalog_payload: validate each entry's own id is present and matches its enclosing bundles key, rejecting catalogs that would otherwise list a spoofed or unresolvable id. - bundle info: stop swallowing manifest download failures. If the manifest can't be resolved (e.g. --offline against an https download_url or a download failure), surface the error and exit non-zero instead of silently degrading to catalog `provides` counts, preserving the "info == what install applies" guarantee. Added regressions: refresh leaves independently-installed components untouched, catalog id key/field mismatch + missing id rejection, and info exits non-zero when the manifest is unresolvable offline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): confine catalog-config and integration-marker reads Addresses review 4534716790: two more state reads bypassed the symlink/path-escape confinement that records and the write paths already enforce. - catalog_config._read: validate the config path with ensure_within(project_root, ...) before exists()/read, so a symlinked .specify resolving outside project_root is rejected instead of read. - lib.project.active_integration: confine the .specify/integration.json read the same way; an out-of-tree escape is treated as "not determinable" (returns None) rather than followed. Added regressions covering both via a symlinked .specify pointing outside the project root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): validate manifest tags, disambiguate derived ids by full host Addresses review 4534768419: - manifest.from_dict: reject a non-list `tags` (e.g. a bare string) instead of splitting it character-by-character, matching the catalog parser and the schema contract (tags = list of strings). - catalog_config._derive_id: derive ids from the full host (TLD included) so example.com and example.net no longer collide on the same id. Updated the affected id assertions. - CHANGELOG: call out the new `specify bundle` command group in the unreleased section (the PR's headline user-facing feature). - .gitattributes: clarify the specs whitespace exemption — the dogfooding feature dir is scrubbed before merge (not retained), so it doesn't weaken checks for kept docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(gitattributes): retain whitespace exemption for constitution.md The project constitution (.specify/memory/constitution.md) is the one dogfooding artifact carried forward past the pre-merge scrub. Give it its own standalone whitespace exemption so it survives removal of the broader .specify/** generated-scaffolding exemption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): accurate uninstall count, confine catalog read, safe bundle id Addresses review 4534812056: - installer.remove_bundle: only count a component as uninstalled when installer.remove() actually ran; components already absent on disk are reported as skipped, keeping the uninstalled count accurate. - catalog.load_source_stack: confine the project-scoped .specify config read with ensure_within, so a symlinked .specify/ resolving outside the project root is refused (consistent with the bundler's other guarded reads). - manifest: enforce a filesystem-safe slug for bundle.id in structural validation; packager.build_bundle adds an ensure_within defense-in-depth check so a crafted id can never push the artifact outside the output dir. Also reverts the CHANGELOG entry (the changelog is updated separately). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): validate requires/provides shapes in manifest and catalog Addresses review 4534855443: - manifest: validate requires.tools and requires.mcp as list-of-strings via a shared _parse_str_list helper (also reused for tags), so a bare string like `tools: docker` is rejected with an actionable BundlerError instead of being split character-by-character. - catalog.CatalogEntry.from_dict: validate that `requires` and `provides` are mappings before accessing them, so an untrusted catalog payload with `requires: "..."` raises a named BundlerError rather than escaping as a raw AttributeError traceback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): require README.md when building a bundle artifact Addresses review 4534938014: build_bundle now fails early with an actionable error when README.md is missing, matching the documented artifact contract (manifest + README) instead of silently producing a bundle with no human-facing description. Also reverts CHANGELOG.md to the upstream/main copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): validate record shapes; drop stale install --refresh claim Addresses review 4534969692: - records.InstalledBundleRecord.from_dict: hard-error when contributed_components is not a list, instead of iterating a corrupt bare string character-by-character. - records.load_records: validate the top-level 'bundles' field is a list and fail with a clear BundlerError when a corrupt file makes it a mapping/string. - PR description: remove the inaccurate "supports --refresh" note from `bundle install` (refresh is the `bundle update` path); docs already omit it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): refuse symlinked .specify, reject bad url schemes, IPv6 ids Addresses review 4534997724: - lib.project.find_project_root: a symlinked .specify is no longer accepted as a project root (is_dir() follows symlinks), matching the confinement the rest of the CLI applies and avoiding confusing downstream failures. - catalog_config.add_source: reject unsupported url schemes (ssh://, ftp://, ...) up front instead of silently treating them as local paths; local paths containing ':' but not '://' are still allowed. - catalog_config._derive_id: derive the host via urlparse().hostname so IPv6 literals, credentials, and ports no longer corrupt the derived id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): strict semver, narrow artifact skip, preserve priority 0 Addresses review 4535084048: - versioning.is_semver: enforce a full MAJOR.MINOR.PATCH SemVer (with optional pre-release/build) via a dedicated regex, instead of accepting any packaging.version.Version-parseable string (e.g. "1", "1.0"). This makes BundleManifest.structural_errors() reject non-semver versions. - packager: narrow the prior-artifact skip pattern to semver-named zips (<id>-<x.y.z>.zip) so legitimate assets like <id>-assets.zip are still packaged. - primitives (preset + extension install): use an explicit `is None` check so an intentional priority of 0 is preserved instead of being replaced by the default. Adds regressions: non-semver rejection ("1"/"1.0"/"1.2.3.4"), asset-not- excluded vs semver-artifact-excluded, and priority-0 pass-through. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): artifact regex for prerelease+build; clarify integration/priority docs Addresses review 4535132279: - packager: the prior-artifact skip regex now matches semver names carrying both a prerelease and build-metadata segment (e.g. 1.0.0-rc1+build5), so such an existing artifact is excluded rather than re-packaged — keeping builds bounded/deterministic, consistent with is_semver(). - docs/reference/bundles.md: correct the install integration wording. --integration selects the integration when initializing a new project and confirms the target when a pinned bundle's active integration can't be determined; it does NOT override a bundle that targets a specific integration (a mismatch aborts with no changes). - examples/security-researcher README: reword the preset priority note in terms of the numeric comparison (ascending priority order) to avoid inverting the meaning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): --integration can't bypass clash guard; honest rollback docs Addresses review 4535159341: - bundle install: for an already-initialized project, the project's recorded active integration is now authoritative. --integration no longer overrides it (which let a copilot project install a claude-pinned bundle via `--integration claude`, bypassing the FR-019 clash guard). The override still selects the integration at init time and confirms the target only when the active integration cannot be determined. - docs/reference/bundles.md: reword the install guarantee to match the implementation — no provenance record is written unless the install fully succeeds, and rollback of this run's components is best-effort (removal errors are swallowed, so partial on-disk state may remain). Dropped the inaccurate "atomic / rolls back everything" claim. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): validate component kind/id when loading records Addresses review 4535194606: _component_from_dict now rejects a contributed component whose 'kind' is not a supported component kind or whose 'id' is empty, raising a BundlerError that explicitly flags the records file as corrupt. Previously such a record loaded successfully and only failed later (e.g. in primitive_manager() during bundle remove/update) with a less actionable error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): address review 4535234003 (7 findings) - versioning: tolerate an uppercase `V` prefix in `_normalize_semver` and `is_semver`, mirroring specify_cli._version tag normalization (V -> v) so `V1.2.3` parses and validates consistently. - validator: import BundlerError and narrow the speckit_version constraint except clause to `BundlerError` only, so programming errors are no longer masked behind an "invalid constraint" message. - bundle update: accept `--integration` and thread it through resolve_install_plan the same way `bundle install` does (override used only when the active integration can't be auto-detected), so integration-pinned bundles can be updated where `.specify/integration.json` is missing/unreadable. - bundle validate: fold reference warnings into `report.warnings` so the ValidationReport is the single warning channel at the CLI layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(bundler): make update --integration help assertion ANSI-safe Rich can split the "--integration" option label with ANSI escape codes between the two leading dashes, so the literal substring check failed under CI's terminal settings. Match the un-split option word instead, mirroring how test_bundle_help_lists_all_commands checks bare command names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): preserve exec bits in artifacts; document install-time pins Addresses review 4535280786: - packager.build_bundle: no longer forces every ZIP member to 0644, which stripped the executable bit from bundled scripts (e.g. extension hook scripts) and could break them after extraction. Permissions are now normalized reproducibly to 0755 when the source file has any execute bit set, otherwise 0644 — identical inputs still yield byte-for-byte identical artifacts. - installer.install_bundle + docs/reference/bundles.md: document that version pins are enforced install-time only. Because primitive is_installed checks are id-based (not version-aware), an already-present component is skipped during install without comparing its on-disk version to the manifest pin; pins are guaranteed applied only on a real install or `bundle update` refresh. Added a regression asserting executable sources map to 0755 and plain files to 0644 in the built artifact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(bundler): skip exec-bit packager test on Windows Windows filesystems do not carry Unix execute bits, so chmod(0o755) is a no-op and the source file reports no execute bit — the packager then correctly stores the member as 0644. The assertion that an executable source maps to 0755 is only meaningful on POSIX, so skip it on nt rather than asserting platform-specific behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): normalize prerelease spellings inside version constraints Addresses review 4535327154: parse_version() normalized SemVer prerelease spellings (e.g. 1.2.3-rc1 -> 1.2.3rc1) but parse_constraint() passed the constraint to packaging.SpecifierSet unmodified, so ">=1.2.3-rc1" raised InvalidSpecifier even though the same spelling is accepted for installed versions. parse_constraint() now normalizes the version portion of each comma-separated clause via the shared _normalize_semver helper, so prerelease handling is consistent across versions and constraints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(bundler): validate schema versions and required record identity fields Addresses review 4535351596: - records.load_records: validate the on-disk 'schema_version' (required; forward-compatible across same-major minor bumps) and fail fast with an actionable error on a missing/unknown version, rather than silently parsing a possibly-incompatible format and risking incorrect bundle attribution/removal. - records.InstalledBundleRecord.from_dict: treat missing 'bundle_id' or 'version' as corruption and raise BundlerError, instead of coercing them to empty strings that let later list/remove/update operations behave unpredictably. - catalog_config._read: validate 'schema_version' when present (same-major compatibility) and fail fast on an unsupported version so an incompatible future config shape can't be mis-parsed into a wrong effective catalog stack. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(bundler): scrub generated dogfooding scaffold before merge The bundler feature was developed by dogfooding Spec Kit on itself. Now that the work is complete, remove all generated scaffolding so it does not land in the repository on merge: - specs/001-spec-kit-bundler/** (spec, plan, research, data-model, contracts, quickstart, tasks, checklists) - .specify/** (extensions, integrations, scripts, templates, workflows, feature/init/integration metadata) - .github/agents/speckit.*.agent.md, .github/prompts/speckit.*.prompt.md, and .github/copilot-instructions.md (Copilot integration scaffold) Retained: .specify/memory/constitution.md — the single dogfooding artifact carried forward — with its whitespace exemption in .gitattributes. .gitattributes and .markdownlint-cli2.jsonc are reverted to the upstream baseline (plus the constitution whitespace exemption), dropping the now-moot exemptions for the removed scaffold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Pascal THUET <pascal.thuet@arte.tv> |