Skip to content

fix(integrations): atomically record generated skill files - #4405

Open
Gambit-Checkmate wants to merge 1 commit into
github:mainfrom
Gambit-Checkmate:fix/record-generated-skills
Open

fix(integrations): atomically record generated skill files#4405
Gambit-Checkmate wants to merge 1 commit into
github:mainfrom
Gambit-Checkmate:fix/record-generated-skills

Conversation

@Gambit-Checkmate

Copy link
Copy Markdown

Summary

  • route generated integration files through IntegrationManifest.record_file so writing and manifest tracking happen together
  • add Claude integration coverage ensuring every generated SKILL.md, including speckit-converge, persists in claude.manifest.json

Fixes #4273

Validation

  • python -m compileall -q completed for both modified Python files

The full repository test suite is not available in the local environment, so CI should run the integration test.

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The two modified files need to be normalized back to LF before this can be reviewed. The repository enforces * text=auto eol=lf, but this commit introduces mixed CRLF/LF endings across almost every line: the displayed diff is 2,778 additions and 2,772 deletions, while git diff -w reduces it to roughly 22 changed lines. This makes the substantive change effectively unreadable and creates unnecessary merge risk.

Please also demonstrate that the regression test fails on main before the implementation change. As written, it appears to assert behavior the existing write_file_and_record() path already provides, so it does not yet establish that switching from write-then-record_existing() to record_file() fixes the reported upgrade scenario.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants