[bug-fix] Fix converge-missing-manifest: track generated skill files by github-actions[bot] · Pull Request #4990 · github/spec-kit

github-actions · 2026-10-15T22:45:20Z

Bug fix — converge-missing-manifest

Proposed fix for issue #4273, applying the remediation from the bug assessment.

Verdict: valid · Severity: medium

Summary

Generated skill files are now written and recorded atomically through IntegrationManifest.record_file, eliminating the write-then-rehash gap. Skills setup also verifies that every generated SKILL.md has a corresponding manifest entry, with regression coverage for fresh setup and re-init over an existing installation.

Changes

File	Change	Notes
`src/specify_cli/integrations/base.py`	modified	Use `record_file` for normalized skill writes and fail loudly if generated skills are missing from the manifest.
`tests/integrations/test_integration_base_skills.py`	modified	Verify all generated skills are tracked and re-init records the newly added converge skill.
`tests/integrations/test_integration_claude.py`	modified	Verify the Claude manifest includes every generated skill, including converge.

Tests Added or Updated

SkillsIntegrationTests.test_all_skill_files_are_tracked_in_manifest — pins the generated-to-manifest inverse relationship.
SkillsIntegrationTests.test_reinit_tracks_new_skill_over_existing_install — covers a pre-existing nine-skill installation and the newly generated converge skill.
TestClaudeIntegration.test_setup_tracks_every_skill_in_manifest — verifies the Claude-specific manifest includes converge.

Local Verification

Commands run: python3 -m compileall -q src/specify_cli/integrations/base.py tests/integrations/test_integration_base_skills.py tests/integrations/test_integration_claude.py → passed.
Commands run: git diff --check → passed.
Targeted pytest command could not run because pytest is not installed in the available Python environment; no dependencies were installed.

Deviations from Assessment

None.

Risks & Review Notes

The new postcondition raises if setup ever produces a skill without recording it, turning a partial manifest into an explicit failure.
Existing manifests remain compatible; the atomic recording path preserves normalized UTF-8 writes and manifest hashes.

Refs #4273 · cc @jabelk

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

Fixes [Bug]: specify init writes speckit.manifest.json without the speckit-converge skill it just installed #4273

Generated by 🛠️ Fix Bug from Labeled Issue for #4273 · copilot · gpt52codex · 3.34 AIC · ⌖ 0.696 AIC · ⊞ 24.6K · ◷

Apply the remediation from the bug assessment on issue #4273. Refs #4273 Assisted-by: GitHub Copilot (model: gpt-5.2-codex, autonomous)

Copilot

🟡 Changes recommended

The symlink-safety regression and failing inherited tests must be resolved before approval.

5 open issues

What changed in this PR

Aims to keep generated skills, including converge, tracked in Specify’s integration manifests for #4273.

Changes:

Route normalized writes through manifest recording.
Check generated skills against manifest entries.
Add fresh-install and re-init tracking assertions.

AI-generated review for the requester by GitHub Copilot (model: unknown; automated review).

File	Description
`tests/integrations/test_integration_claude.py`	Adds Claude skill-tracking assertions.
`tests/integrations/test_integration_base_skills.py`	Adds shared fresh-install and re-init checks.
`src/specify_cli/integrations/base.py`	Changes file recording and adds a manifest consistency guard.

_{Review effort: Max}

Copilot · 2026-10-19T14:18:46Z

+        if created_skill_files != tracked_skill_files:
+            raise RuntimeError(
+                "Skills setup produced a manifest that does not track every "
+                "generated SKILL.md file"
+            )


Added a negative test where the writer creates and returns SKILL.md without recording it; setup now raises and identifies missing paths. Commit: 9555b02.

The missing-recording test was added in commit 9555b02, with additional coverage for generated paths missing from the manifest.

Copilot · 2026-10-19T14:18:46Z

+        tracked_skill_files = {
+            path for path in m.files if path.endswith("/SKILL.md")
+        }
+        assert tracked_skill_files == created_skill_files


Added Hermes-specific overrides using an isolated home directory. They verify that Hermes global skills remain untracked in the project manifest, including on re-init. Commit: 9555b02.

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

        """Override: Hermes writes to global, not project-local."""
        self.test_setup_writes_to_global_skills_dir(tmp_path, monkeypatch)

+    def test_all_skill_files_are_tracked_in_manifest(self, tmp_path, monkeypatch):


Copilot

🟡 Changes recommended

Windows path comparisons can reject valid installations, and legacy migration fixtures become incompatible with the newly recorded ownership metadata.

4 open issues

3 resolved since last review

_{Review effort: Max}

Copilot · 2026-10-19T15:27:27Z

+            rel = dest_absolute.relative_to(project_root_absolute)
+        except ValueError:
+            rel = dest_absolute.relative_to(project_root.resolve())
+        manifest.record_file(rel, normalized.encode("utf-8"))


Updated the Kilo, OpenCode, and Qoder legacy fixtures to remap ownership metadata with their file paths. Commit: 41ef38a.

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The reported upgrade failure remains unreproduced, and the shared writer’s cross-platform behavior still needs validation.

3 open issues

2 previously missed issues

In code that hasn't changed since last review

Test Claude setup rejects project-local skills symlink

src/specify_cli/integrations/base.py:1843

Keeping this destination lexical lets manifest validation reject project-local symlinked skill directories, as required by design/integration.md:306–309. The new helper tests do not protect this setup-level behavior: restoring .resolve() here would hide a linked .claude/skills directory while those tests still passed. Kimi's setup test rejects links in its own pre-check.

Add a Claude setup test linking .claude/skills to an empty directory inside the project. Assert that setup raises ValueError matching symlinked, the target remains empty, and the manifest records no files. This covers the prevented behavior required by CONTRIBUTING.md:229–235.

AI-generated review for the requester by GitHub Copilot (model: unknown; automated mode).

Test fallback with lexical and resolved aliased destinations

tests/integrations/test_base.py:236

This test builds dest from the alias, so it never exercises the new fallback at base.py:689–690. Markdown, TOML, YAML, Forge, and Generic setup resolve their destinations before calling the shared writer; with an aliased project root, they need that fallback to succeed. Parameterize this test with both lexical and resolved destinations to protect both supported paths, as required by CONTRIBUTING.md's “Testing deterministic behavior” section.

AI-generated review for the requester by GitHub Copilot (model: unknown; automated mode).

1 resolved since last review

Resolve paths before comparing manifest skills on Windows

_{Review effort: Max}

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0d9003f-9248-4020-b83d-24897c0406ef

mnriem · 2026-10-19T16:53:51Z

Addressed the latest review round in de2981e.

Added Claude setup-level coverage proving a project-local .claude/skills symlink is rejected without writing through the link or recording manifest ownership.
Parameterized the aliased-project-root writer test to cover both lexical and resolved destination paths.
Confirmed the earlier ownership-metadata fixture remapping, Hermes global-skill overrides, and unrecorded-skill rejection coverage remain present.

Validation: .venv/bin/python -m pytest tests/integrations/test_base.py tests/integrations/test_integration_claude.py tests/integrations/test_integration_hermes.py tests/specify_cli/integrations/test_command_upgrade.py -q passed (234 passed, 1 skipped).

Posted on behalf of @mnriem. Generated with GitHub Copilot using GPT-5.6 Sol in autonomous mode; implementation, test execution, and review-round summary were AI-generated.

Fix converge-missing-manifest: track generated skill files

bb3f143

Apply the remediation from the bug assessment on issue #4273. Refs #4273 Assisted-by: GitHub Copilot (model: gpt-5.2-codex, autonomous)

github-actions Bot added automated bug-fix Trigger the bug-fix agentic workflow labels Oct 15, 2026

github-actions Bot mentioned this pull request Oct 15, 2026

[Bug]: specify init writes speckit.manifest.json without the speckit-converge skill it just installed #4273

Open

mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Oct 16, 2026

mnriem requested a max review from Copilot October 19, 2026 13:39

Copilot started reviewing on behalf of mnriem October 19, 2026 13:40

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Copilot started work on behalf of mnriem October 19, 2026 14:04

View session

Harden skill manifest tracking

9555b02

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

github-code-quality Bot found potential problems Oct 19, 2026

View reviewed changes

Comment thread tests/integrations/test_integration_hermes.py

"""Override: Hermes writes to global, not project-local."""

self.test_setup_writes_to_global_skills_dir(tmp_path, monkeypatch)

def test_all_skill_files_are_tracked_in_manifest(self, tmp_path, monkeypatch):

mnriem requested a max review from Copilot October 19, 2026 14:19

Copilot AI requested a review from mnriem October 19, 2026 14:20

Copilot finished work on behalf of mnriem October 19, 2026 14:20

Copilot started reviewing on behalf of mnriem October 19, 2026 14:20

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Copilot started work on behalf of mnriem October 19, 2026 15:18

View session

Fix integration review findings

41ef38a

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

mnriem requested a max review from Copilot October 19, 2026 15:36

Copilot started reviewing on behalf of mnriem October 19, 2026 15:37

View session

Copilot finished work on behalf of mnriem October 19, 2026 15:40

Copilot AI reviewed Oct 19, 2026

View reviewed changes

mnriem marked this pull request as ready for review October 19, 2026 16:51

test(integrations): cover skill path safeguards

de2981e

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0d9003f-9248-4020-b83d-24897c0406ef

Navigation Menu

Repository navigation

[bug-fix] Fix converge-missing-manifest: track generated skill files - #4990

[bug-fix] Fix converge-missing-manifest: track generated skill files#4990
github-actions[bot] wants to merge 4 commits into
mainfrom
fix/4273-converge-missing-manifest-9a82b7fb4f73186e

github-actions Bot commented Oct 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Oct 19, 2026

Uh oh!

Copilot AI Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

github-actions Bot commented Oct 15, 2026

Bug fix — converge-missing-manifest

Summary

Changes

Tests Added or Updated

Local Verification

Deviations from Assessment

Risks & Review Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

mnriem commented Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants