Skip to content

Fix #4345: deliver bundled extension updates - catalog-synced bumps, CI guard, staleness detection, local-package installs - #4351

Open
CrazyBaran wants to merge 10 commits into
github:mainfrom
CrazyBaran:fix/4345-bundled-extension-version-bumps
Open

Fix #4345: deliver bundled extension updates - catalog-synced bumps, CI guard, staleness detection, local-package installs#4351
CrazyBaran wants to merge 10 commits into
github:mainfrom
CrazyBaran:fix/4345-bundled-extension-version-bumps

Conversation

@CrazyBaran

@CrazyBaran CrazyBaran commented Aug 27, 2026

Copy link
Copy Markdown

Fixes #4345

Summary

specify extension update decides whether an installed extension needs updating by comparing the semver in extensions/catalog.json against the installed copy's registered version. Both bundled extensions (agent-context, git) have carried version: "1.0.0" since they were created while absorbing months of content changes — including platform-breaking fixes — so every installed copy has been told "Up to date (v1.0.0)" forever (drift quantified in #4345).

This PR implements the issue's proposed fixes 1–3, plus one additional piece that turned out to be required to make the update path work end-to-end. One commit per phase:

1. Version bumps + catalog sync (60dc320)

  • agent-context and git1.1.0; assess1.0.1 (it also drifted: one shipped command file changed since its version was set); bug verified clean and stays 1.0.0.
  • extensions/catalog.json bumped in lockstep — the catalog is what the update check compares against, so bumping the manifests alone has no effect on installed copies.
  • Updated the existing test that asserts the git extension's manifest version.

2. CI guard so this cannot regress (dfa1fa6)

  • New workflow extension-version-guard.yml runs .github/scripts/check_extension_version_bump.py on any PR touching extensions/** and enforces two invariants for cataloged extensions:
    1. any change under extensions/<id>/ must increase that extension's extension.yml version;
    2. the extensions/catalog.json version must equal the manifest version.
  • selftest and template are exempt (not in the catalog; no update flow reads their versions).
  • The sync invariant is additionally enforced from a plain working tree by tests/contract/test_bundled_extension_versions.py, so it also holds on forks that don't run the workflow.
  • Exercised against real scenarios: unbumped content change → fail; version downgrade → fail; manifest bump without catalog sync → fail; bumped change → pass; selftest change → pass.

3. Content-hash staleness detection (1683f27)

  • New compute_extension_content_hash(): SHA256 over an extension's shipped files (sorted POSIX relative paths + bytes), excluding exactly what installs treat as user-owned (*-config.yml, *-config.local.yml) or skip (.extensionignore and its patterns). Symlinks are never followed.
  • install_from_directory() — the single sink all install routes funnel through — records content_hash in the registry.
  • When the catalog version equals the installed version, extension update compares the recorded hash (falling back to hashing the installed directory for registry entries that predate content_hash, which covers every existing install) against the copy bundled with the running spec-kit release. A mismatch is reported as stale content with the exact refresh command (specify extension add <id> --force) instead of "Up to date". Advisory only — exit codes unchanged, user config edits never flagged.

4. Bundled updates install from the local package (7204848)

  • Without this, the bumps make extension update offer 1.0.0 → 1.1.0 and then fail: the pipeline is download-based and download_extension refuses bundled extensions (no download URL), with an error hinting at reinstalling the CLI — which never refreshes the project-installed extension. This path was previously unreachable precisely because the versions never moved.
  • Bundled updates now resolve the copy shipped with the running spec-kit release at offer time, get packaged as a ZIP, and go through the identical hardened pipeline as downloads (bounded extraction, manifest preflight, ID/version checks, backup/rollback) — no second install code path. When the running release ships an older copy than the catalog advertises, the user is told to upgrade spec-kit first instead of receiving a doomed offer.

5–6. Constitution compliance pass (cbe4b31, 08246e5)

Audited the series against .specify/memory/constitution.md: the hash walk never follows symlinks (Principle IV), new code uses modern typing and from __future__ import annotations (Principle I), the new symlink test skips where privileges are missing instead of failing (Principle II), and user-facing docs moved with the behavior change per the workflow gate (docs/reference/extensions.md, extensions/EXTENSION-DEVELOPMENT-GUIDE.md).

Relationship to #4350

Appreciation to @ranka23 for picking the issue up — however #4350 (bumping the two extension.yml versions, nothing else) cannot fix the reported behavior as written:

  • It bumps the wrong side of the comparison. extension_update compares installed versions against extensions/catalog.json, which Fix #4345: Extension version staleness #4350 leaves at 1.0.0 across the board. Installed copies are still told "Up to date (v1.0.0)"; the net effect is a manifest↔catalog desync (new installs register 1.1.0 while the catalog advertises 1.0.0).
  • It fails the existing test suite. tests/extensions/git/test_git_extension.py asserts m.version == "1.0.0" and is not updated in that PR.
  • Even with the catalog synced, the offered update would fail to install. Bundled extensions have no download URL, so the confirmed update dies at the download step with a misleading "reinstall the CLI" hint (fixed by phase 4 here).
  • No recurrence protection (issue fix Fix release workflow to work with repository rules #2), no detection for already-stale installs (issue fix Update README.md #3), no assess bump, no test/docs updates.

This PR supersedes #4350; the two overlapping bump lines are identical. Notably, the new CI guard's sync invariant would have flagged #4350's catalog desync automatically.

Testing

  • Full extension battery locally (Windows, Python 3.14): tests/test_extensions.py, tests/test_extension_update_hardening.py, tests/test_extension_content_staleness.py (14 new tests), tests/contract/test_bundled_extension_versions.py (9 new), tests/extensions/660 passed; the only 2 failures are pre-existing environmental ones (os.symlink needs Windows developer mode) in tests this PR does not touch.
  • ruff check src tests and git diff --check clean; guard script exercised against the positive and negative scenarios listed above.
  • No slash-command templates are changed by this PR.

AI disclosure

This PR was authored by Claude Code (model: claude-fable-5) operating autonomously on behalf of @CrazyBaran. Every commit carries an Assisted-by: trailer per AGENTS.md, and follow-up review-round commits and comments will disclose the same.


— Claude Code (model: claude-fable-5, autonomous), posted on behalf of @CrazyBaran

🤖 Generated with Claude Code

Jakub Baranowski and others added 6 commits August 27, 2026 08:33
…xt, git 1.1.0; assess 1.0.1)

The bundled agent-context and git extensions have carried version 1.0.0
since they were created while their content kept changing - including
fixes for failures that made them unusable on some platforms
(agent-context: 15 commits, +1,120/-169 across 7 of its 8 files;
git: 23 commits, +2,191/-567 across all 21 files). Because
`specify extension update` compares semver only, every installed copy
is reported "Up to date (v1.0.0)" forever and never receives those
fixes (github#4345).

Bump both manifests to 1.1.0 and sync extensions/catalog.json so
existing installs finally see an available update. assess also drifted
(one docs-only change to a shipped command file since its version was
set), so it gets a patch bump to 1.0.1; bug has no drift and stays at
1.0.0.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The version staleness fixed in the previous commit regressed silently
because nothing enforced bumping: extensions/agent-context and
extensions/git each absorbed months of content changes (including
platform-breaking bug fixes) while extension.yml stayed at 1.0.0, so
`specify extension update` kept telling every installed copy it was up
to date (github#4345).

Add a PR-gating workflow (extension-version-guard.yml) that runs
.github/scripts/check_extension_version_bump.py whenever a PR touches
extensions/**. The script enforces two invariants for extensions listed
in extensions/catalog.json:

1. Any change under extensions/<id>/ must increase
   extension.yml's version (base vs head of the PR).
2. The catalog.json version must equal the manifest version - the
   catalog drives the update check, and the update preflight rejects a
   manifest whose version differs from it.

Extensions not in the catalog (the selftest fixture and the template
scaffold) are exempt: no update flow is driven by their versions.

The sync invariant (2) is additionally enforced from a plain working
tree by tests/contract/test_bundled_extension_versions.py, so it also
holds on platforms and forks that do not run the workflow.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on update`

Belt-and-braces for github#4345: even with the CI guard from the previous
commit, any bundled-extension content change that ever ships without a
version bump is invisible to `specify extension update` - the registry's
manifest_hash covers only extension.yml, and the update check compares
semver alone, so installed copies report "Up to date" while silently
missing shipped fixes (in our case a hook that failed on every run for
two months).

Add compute_extension_content_hash(), a SHA256 over an extension
directory's shipped files (sorted relative paths + bytes), excluding
exactly what installs treat as user-owned or skip: *-config.yml /
*-config.local.yml and .extensionignore plus whatever it ignores. The
same function therefore yields comparable hashes for a bundled source
directory and an installation made from it.

- install_from_directory() now records the source's content_hash in the
  registry (all install routes funnel through it); a hash failure never
  fails an install.
- When `extension update` finds a bundled extension (no download URL)
  whose catalog version equals the installed version, it compares the
  recorded hash - falling back to hashing the installed directory for
  registry entries that predate content_hash, which covers every
  existing install - against the copy bundled with the running spec-kit
  version. A mismatch is reported as stale content with the exact
  refresh command (`specify extension add <id> --force`) instead of
  "Up to date", and suppresses the green "All extensions are up to
  date!" all-clear. Exit code stays 0: the report is advisory, and the
  version comparison's verdict is unchanged.

User config edits are not flagged (config files are excluded from the
hash), and non-bundled/downloadable extensions are untouched - their
updates are served by the normal version flow.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kage

With the version bumps earlier in this series, `specify extension
update` now offers agent-context/git/assess updates for the first time -
and then failed to deliver them: the update pipeline is download-based,
and ExtensionCatalog.download_extension refuses bundled extensions that
have no download URL. The user confirmed an offered update only to get
"Failed: ... Try reinstalling: uv tool install specify-cli ..." - a hint
that upgrades the CLI but never refreshes the project-installed
extension. This path was previously unreachable precisely because the
bundled versions never moved (github#4345).

Route bundled updates through the local package instead:

- At offer time, a bundled catalog entry (no download URL) resolves the
  copy shipped with the running spec-kit release via
  _locate_bundled_extension. When that local copy is newer than the
  installed version, it becomes the offered update; when it lags the
  catalog (older CLI) or is absent, the update is reported as requiring
  a spec-kit upgrade, with the exact next step, instead of being
  offered and then failing at the download step.
- At install time, the resolved bundled directory is packaged as a ZIP
  and fed through the unchanged update pipeline, so bundled updates get
  the identical bounded extraction, manifest preflight, ID/version
  checks, and backup/rollback as downloaded ones - no second install
  code path.
- When every checked extension is blocked on a newer spec-kit release,
  the summary says so instead of "All extensions are up to date!".

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng and symlink rules

Compliance pass of this branch against .specify/memory/constitution.md
(v1.0.0) surfaced three code-level deviations in the new github#4345 code:

- Principle IV ("never follow symlinks out of the project root"):
  compute_extension_content_hash() followed directory symlinks (with a
  cycle guard) and file symlinks, so a symlink inside an installed
  extension directory could pull bytes from outside the project into
  the hash. Skip symlinks entirely instead - installs dereference file
  symlinks during copytree and bundled extensions ship none, so both
  sides of a comparison stay symmetric. The cycle-guard visited set is
  no longer needed. Covered by a new test that skips (not fails) where
  symlink creation needs privileges, per Principle II's guarding rule.
- Principle I ("legacy Dict/List/Optional forms are rejected"): the
  visited set used typing.Set; gone with the visited set. The new
  _commands.py helpers (_bundled_content_is_stale,
  _bundled_update_source) were missing parameter/return annotations -
  added with modern syntax via TYPE_CHECKING-only imports.
- Principle I ("every new module begins with from __future__ import
  annotations"): added to tests/test_extension_content_staleness.py.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mp rule

The constitution's Development Workflow gate requires user-facing docs
to move with behavior changes, and Principle III requires command groups
to stay documented under docs/reference/. The github#4345 series changed
`specify extension update` behavior without touching either:

- docs/reference/extensions.md (Update Extensions): bundled extensions
  now update from the copy shipped with the running spec-kit release;
  a catalog version newer than the release ships is reported as
  requiring a spec-kit upgrade; matching-version installs whose files
  differ from the shipped copy are flagged as stale content with the
  `specify extension add <name> --force` refresh command; config files
  are preserved and never counted as stale.
- extensions/EXTENSION-DEVELOPMENT-GUIDE.md (Versioning): record that a
  content change without a version bump never reaches installed copies,
  and that CI (extension-version-guard.yml) enforces the bump plus
  catalog.json sync for the bundled extensions in this repository.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@CrazyBaran
CrazyBaran requested a review from mnriem as a code owner August 27, 2026 07:35
Copilot AI balanced review requested due to automatic review settings August 27, 2026 07:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds reliable bundled-extension updates through synchronized versions, content-staleness detection, and local-package installation.

Changes:

  • Bumps bundled extension versions and synchronizes the catalog.
  • Adds content hashing and local bundled-update support.
  • Adds CI guards, contract tests, and documentation.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/specify_cli/extensions/__init__.py Adds extension content hashing and registry persistence.
src/specify_cli/extensions/_commands.py Detects stale content and installs bundled updates locally.
.github/scripts/check_extension_version_bump.py Validates version bumps and catalog synchronization.
.github/workflows/extension-version-guard.yml Runs the version guard for extension changes.
extensions/catalog.json Synchronizes bumped extension versions.
extensions/agent-context/extension.yml Bumps agent-context to 1.1.0.
extensions/assess/extension.yml Bumps assess to 1.0.1.
extensions/git/extension.yml Bumps git to 1.1.0.
extensions/EXTENSION-DEVELOPMENT-GUIDE.md Documents version-bump requirements.
docs/reference/extensions.md Documents bundled updates and stale-content recovery.
tests/test_extensions.py Tests local bundled-update behavior.
tests/test_extension_content_staleness.py Tests hashing and stale-content detection.
tests/contract/test_bundled_extension_versions.py Enforces catalog/manifest version parity.
tests/extensions/git/test_git_extension.py Updates the expected git version.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/extensions/__init__.py Outdated
Comment thread src/specify_cli/extensions/_commands.py Outdated
Comment thread src/specify_cli/extensions/_commands.py
Comment thread .github/scripts/check_extension_version_bump.py Outdated
All four review findings were valid; each is applied with regression
tests:

1. compute_extension_content_hash() excluded *-config.yml /
   *-config.local.yml at every depth, but the remove/backup/restore
   machinery only preserves top-level config files
   (_target_follows_preserved_convention), so a changed nested shipped
   file like templates/foo-config.yml was overwritten by installation
   yet invisible to staleness detection. The exclusion now applies only
   to direct children of the extension directory.

2. The stale-content check ran on every catalog_version <=
   installed_version outcome. When the installed copy is newer than the
   catalog or the running release's bundled copy (e.g. written by a
   newer CLI), a hash difference is version skew, not unbumped drift -
   and the suggested `extension add --force` would downgrade the
   installation. The check is now gated on catalog_version ==
   installed_version at the call site AND on the bundled copy declaring
   the same version as the installed one inside the helper (the second
   guard also covers an older CLI run against an up-to-date project,
   which the call-site gate alone would miss).

3. _archive_extension_directory() followed file symlinks (is_file() +
   ZipFile.write() dereference), so a symlink in a source directory
   could turn out-of-tree bytes into a regular archive member before
   the hardened extractor sees it. Symlinks are now skipped, matching
   the rule in compute_extension_content_hash().

4. The CI guard's fallback comparison only required inequality for
   non-dotted-numeric versions, so a PEP 440 prerelease downgrade like
   2.0.0 -> 1.0.0rc1 passed. The script now compares with
   packaging.version.Version - the same semantics extension
   update/install use - and fails closed on unparseable versions; the
   workflow installs packaging alongside pyyaml. Verified locally that
   1.1.0 -> 1.0.0rc1 with content changes is now rejected.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 27, 2026 08:02
@CrazyBaran

CrazyBaran commented Aug 27, 2026

Copy link
Copy Markdown
Author

Review round 1 addressed in 18a12b9. All four findings were valid; each is applied with regression tests:

  1. Content-hash config exclusion narrowed to top-level files — matching the _target_follows_preserved_convention preservation semantics, so nested *-config.yml files (which installs overwrite) now count toward staleness.
  2. Stale-content check gated to matching versionscatalog_version == installed_version at the call site, plus the bundled copy must declare the installed version inside the helper; both skew directions are now inert instead of steering users into a downgrading --force refresh.
  3. Update archiving no longer follows symlinks — consistent with compute_extension_content_hash(); out-of-tree bytes can't become regular archive members ahead of the hardened extractor.
  4. CI guard compares versions per PEP 440 (packaging.version) and fails closed on unparseable versions, so prerelease downgrades such as 2.0.0 -> 1.0.0rc1 can no longer bypass it; the workflow installs packaging.

Local verification: 74 tests across the affected suites pass, ruff clean, and the guard script was re-exercised against its positive/negative scenarios including the new prerelease-downgrade case.

— Claude Code (model: claude-fable-5, autonomous), posted on behalf of @CrazyBaran

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/scripts/check_extension_version_bump.py:71

  • The new CI guard is not exercised by an automated test, so regressions in its diff parsing or version comparisons could silently disable the recurrence protection. Add tests that run main against temporary git histories for an unbumped content change, downgrade, catalog mismatch, valid bump, new extension, removal, and uncataloged exemption; the repository already tests the analogous .github/scripts/check_security_requirements.py in tests/test_security_workflow.py.
def main(argv: list[str]) -> int:

Comment thread src/specify_cli/extensions/_commands.py Outdated
…he catalog

Copilot review round 2 on github#4351: the offer gate only blocked a bundled
copy that was no newer than the installation. With installed v1, locally
bundled v2, and catalog v3, the command offered and installed v2 and
reported success - leaving the project lagging the catalog with no
mention of it, contrary to the documented "the update is reported as
requiring a spec-kit upgrade first" behavior (and re-nagging about the
upgrade on every subsequent run).

Compare the bundled version against the catalog version instead: any
older local copy is blocked with the upgrade-spec-kit guidance. This
subsumes the previous gate (inside the catalog > installed branch,
bundled <= installed implies bundled < catalog). A local copy at or
above the catalog version (dev/source checkouts) is still offered and
installed. Tests added for the intermediate-version block and the
newer-than-catalog install.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 27, 2026 08:12
@CrazyBaran

CrazyBaran commented Aug 27, 2026

Copy link
Copy Markdown
Author

Review round 2 addressed in 002443a. The finding was valid: with installed v1, locally bundled v2, and catalog v3, the offer gate installed the intermediate v2 and reported success, contrary to the documented "upgrade spec-kit first" behavior. The gate now blocks whenever the local bundled copy lags the catalog version (subsuming the previous installation-lagging check), while a local copy at or above the catalog version — a dev/source checkout — is still offered and installed. Regression tests added for both sides; 67 tests across the affected suites pass and ruff is clean.

— Claude Code (model: claude-fable-5, autonomous), posted on behalf of @CrazyBaran

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

.github/workflows/extension-version-guard.yml:15

  • This filter does not run the guard when its Python implementation or workflow is modified. Because the regular lint job only checks src and tests, a future syntax/runtime regression in .github/scripts/check_extension_version_bump.py could merge without this job ever executing. Include the guard script and workflow paths so changes to the protection validate themselves.
    paths:
      - "extensions/**"

.github/scripts/check_extension_version_bump.py:90

  • The new recurrence guard has no automated tests for its rejection paths; this workflow only exercises it against the current passing diff. Add tests for an unbumped content change, downgrade/prerelease, manifest/catalog desync, new extension, removal, and uncataloged exemption so changes cannot silently weaken the merge guard.
    # -- Invariant 1: content change requires a version bump ---------------
    changed = _git(
        "diff", "--name-only", "--no-renames", base_ref, head_ref, "--", EXTENSIONS_ROOT
    ).splitlines()

extensions/EXTENSION-DEVELOPMENT-GUIDE.md:629

  • This newly added rationale says the update command compares versions only, but this PR also makes it compare content hashes and report stale bundled copies. Clarify that automatic update installation remains version-driven while hash detection is advisory and requires a forced refresh; otherwise this guide contradicts the new reference documentation.
- **Bump on every content change**: `specify extension update` compares
  versions only, so a content change shipped without a version bump never
  reaches already-installed copies. For the bundled extensions in this

Comment thread extensions/agent-context/extension.yml
…em in fixtures

Copilot review round 3 on github#4351: the agent-context bump left every
checked-in bundle pinned to 1.0.0. BundleExtensionPrimitive enforces
exact pins against the bundled manifest, so the offline installs in
tests/integration/test_bundler_local_install.py and
test_bundler_init_install.py failed, and all four
examples/bundles/*/bundle.yml examples stopped being installable.

- examples/bundles/{business-analyst,developer,product-manager,
  security-researcher}/bundle.yml: agent-context pin 1.0.0 -> 1.1.0
  (exact pins are the point of the example format, so they stay
  literal).
- The two integration-test fixtures now resolve the pin through a new
  tests/bundler_helpers.bundled_extension_version() helper, which reads
  the version via the same _locate_bundled_extension lookup the
  primitive enforces against - so the fixtures test the bundler's pin
  mechanics rather than a frozen version literal, and the next
  legitimate extension bump cannot silently break them again.

The git and assess extensions are not pinned by any checked-in bundle;
tests/contract/test_bundle_cli.py's 1.0.0 pin feeds `bundle validate`,
which checks existence only, and keeps passing unchanged.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 27, 2026 08:42
@CrazyBaran

CrazyBaran commented Aug 27, 2026

Copy link
Copy Markdown
Author

Review round 3 addressed in edb30a4. The finding was valid — and it caught a real hole in my earlier verification, which had run the extension suites but not tests/integration/. Both offline bundle-install tests failed exactly as described against the exact-pin enforcement, and all four examples/bundles/*/bundle.yml examples had become uninstallable.

  • The four example bundles now pin agent-context 1.1.0 (pins stay literal there — exact pins are the point of the format).
  • The two integration-test fixtures resolve the pin through a new tests/bundler_helpers.bundled_extension_version() helper, which reads the version via the same _locate_bundled_extension lookup the primitive enforces against — so the fixtures keep exercising the bundler's pin mechanics and a future legitimate bump cannot silently break them again.
  • git/assess are not pinned by any checked-in bundle; test_bundle_cli.py's pin feeds bundle validate (existence check only) and passes unchanged.

This round the full local suite was run, not just the affected subsets: 6751 passed; the 68 failures on this Windows workstation are all pre-existing environmental issues (symlink tests requiring developer-mode privileges, plus four setup-tasks tests tripping over an unquoted space-containing user path), none in code this PR touches.

— Claude Code (model: claude-fable-5, autonomous), posted on behalf of @CrazyBaran

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/extension-version-guard.yml:15

  • The workflow does not run when its checker or workflow definition is changed, because the path filter only includes extensions/**. A checker-only PR can therefore introduce a syntax/runtime regression without exercising this guard. Include both guard files in the trigger paths.
    paths:
      - "extensions/**"

.github/scripts/check_extension_version_bump.py:90

  • This critical recurrence guard has no automated coverage for its diff/version branches; the scenarios in the PR description were only exercised manually. The repository tests the analogous GitHub workflow checker in tests/test_security_workflow.py, so add tests using temporary Git repositories for unbumped changes, downgrades, catalog mismatches, additions/removals, and exemptions to prevent the guard itself from silently regressing.
    # -- Invariant 1: content change requires a version bump ---------------
    changed = _git(
        "diff", "--name-only", "--no-renames", base_ref, head_ref, "--", EXTENSIONS_ROOT
    ).splitlines()

Comment thread extensions/EXTENSION-DEVELOPMENT-GUIDE.md Outdated
Comment thread .github/scripts/check_extension_version_bump.py Outdated
…leness

Copilot review round 4 on github#4351: two documentation spots still described
`specify extension update` as comparing "versions only" / "purely" by
semver, with unbumped content reporting "Up to date" forever - wording
this PR itself made stale when it added the content-hash check.

Clarify in extensions/EXTENSION-DEVELOPMENT-GUIDE.md (Versioning) and
the .github/scripts/check_extension_version_bump.py module docstring
that update offers remain version-driven - a bump is still required for
automatic delivery - while the content-hash check on bundled extensions
is only an advisory stale-content warning pointing at a manual --force
reinstall. Wording only; no behavior change.

Refs github#4345

Assisted-by: Claude Code (model: claude-fable-5, autonomous)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 27, 2026 08:54
@CrazyBaran

Copy link
Copy Markdown
Author

Review round 4 addressed in 16c5642. Both findings were valid documentation inconsistencies this PR introduced against itself: the development guide's Versioning bullet and the guard script's module docstring still described update discovery as comparing "versions only" / "purely" by semver, which stopped being the whole story once the content-hash check landed. Both spots now distinguish the two mechanisms: update offers remain version-driven (a bump is required for automatic delivery), while the content-hash check on bundled extensions is an advisory stale-content warning pointing at a manual --force reinstall. Wording only — no behavior change; ruff clean and the guard script re-verified against the branch.

— Claude Code (model: claude-fable-5, autonomous), posted on behalf of @CrazyBaran

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

docs/reference/extensions.md:80

  • This describes a live comparison of the installed files, but _bundled_content_is_stale() normally compares the registry's install-time content_hash and only hashes the installed directory for legacy entries without that field. Consequently, editing or deleting a shipped non-config file after a current install still reports “Up to date,” while the same edit on a legacy registry entry is flagged. Either hash the installed directory consistently or document that this checks the content snapshot recorded at installation; the CLI message at _commands.py:1816-1817 needs the same clarification.
When an installed bundled extension's files differ from the copy shipped with your spec-kit release even though the versions match (content that shipped without a version bump), the check flags it as stale content and points to the refresh command:

.github/scripts/check_extension_version_bump.py:75

  • The new guard's decision logic has no automated tests; the contract test only checks manifest/catalog synchronization and never exercises the PR-diff version-bump rule. Add tests for an unbumped content change, downgrade/prerelease comparison, new/removed or uncataloged extensions, and catalog desynchronization. Similar GitHub support scripts are loaded and tested directly in tests/test_security_workflow.py, so relying on manual scenarios leaves this recurrence-prevention path itself unprotected.
def main(argv: list[str]) -> int:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants