Skip to content

Fail CI when generated tool wrappers are out of sync with their .json specs#512

Merged
dennisdoomen merged 1 commit into
Fallout-build:mainfrom
dennisdoomen:dennisdoomen-verify-generated-tools-ci
Jul 19, 2026
Merged

Fail CI when generated tool wrappers are out of sync with their .json specs#512
dennisdoomen merged 1 commit into
Fallout-build:mainfrom
dennisdoomen:dennisdoomen-verify-generated-tools-ci

Conversation

@dennisdoomen

@dennisdoomen dennisdoomen commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Problem

GenerateTools regenerates the .Generated.cs tool wrappers from their .json specs, but it's a manual target with no CI enforcement. A contributor can edit a tool's .json spec without regenerating its wrapper, merge silently, and ship stale wrapper code in the next release.

Outcome

Adds a VerifyGeneratedTools target that regenerates the tool wrappers from a known-clean checkout and asserts the working copy stays clean afterward, failing the build if regeneration produces a diff. Wires it into the ubuntu-latest required PR gate ahead of Test and Pack, so drift between a .json spec and its generated wrapper is caught before merge instead of shipping in the next release.

Includes the regenerated .fallout/build.schema.json and .github/workflows/ubuntu-latest.yml (both generated from build/Build.CI.GitHubActions.cs / the target list — not hand-edited).

@dennisdoomen dennisdoomen added enhancement New feature or request target/2026 labels Jul 19, 2026
@dennisdoomen dennisdoomen mentioned this pull request Jul 19, 2026
Closed
@dennisdoomen dennisdoomen changed the title Add VerifyGeneratedTools CI gate for tool wrapper drift Fail CI when generated tool wrappers are out of sync with their .json specs Jul 19, 2026
@dennisdoomen
dennisdoomen force-pushed the dennisdoomen-verify-generated-tools-ci branch from 0427b12 to 3dc81b5 Compare July 19, 2026 05:44
@IT-VBFK

IT-VBFK commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

So: you want the contributors to commit the generated files? Then we have to fix two things:

  • The Contributing guidelines
  • The comment generated in every wrapper's header, which contains the remote repo of the current executing user. This is the crucial one, because otherwise it will fail everytime

@dennisdoomen

dennisdoomen commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

So: you want the contributors to commit the generated files? Then we have to fix two things:

The docs are wrong. Those generated files were never regenerated unless you ran the GenerateTools target. I'll update the docs as well, but wasn't ready yet, hence a draft PR ;-)

@dennisdoomen
dennisdoomen requested a review from a team as a code owner July 19, 2026 06:59
@IT-VBFK

IT-VBFK commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Would be quite fancy, if the CI's generated tool wrappers get committed automagically. Like: CI re-generates tools -> CI commits (the new ones) -> pushes to the PR.

So, it would always fit and nothing to forget about.

@dennisdoomen
dennisdoomen marked this pull request as draft July 19, 2026 07:10
@dennisdoomen

dennisdoomen commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

Would be quite fancy, if the CI's generated tool wrappers get committed automagically. Like: CI re-generates tools -> CI commits (the new ones) -> pushes to the PR.

So, it would always fit and nothing to forget about.

I'd prefer a manual check that the generated file makes sense.

@dennisdoomen
dennisdoomen marked this pull request as ready for review July 19, 2026 07:16
@dennisdoomen dennisdoomen added skip-changelog and removed enhancement New feature or request labels Jul 19, 2026
@dennisdoomen dennisdoomen mentioned this pull request Jul 19, 2026
Merged
@IT-VBFK

IT-VBFK commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The docs are wrong. Those generated files were never regenerated unless you ran the GenerateTools target.

Maybe this was a manual step done by Matthias before a release.. But running by contributors and check in CI/CD is more convenient :)

@IT-VBFK

IT-VBFK commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Missing the update here:

Fallout/CONTRIBUTING.md

Line 56 in c99b238

- Don't commit code generated by `./build.ps1 GenerateTools` — generated `.cs` files are regenerated manually once per release.

meaning: align to the agents/conventions.md

- Commit the regenerated .csoutput alongside the.jsonspec —VerifyGeneratedTools fails CI if they drift.

@ChrisonSimtian ChrisonSimtian added target/vCurrent Targets the current version Can Merge These PRs are just waiting to be merged, nothing more to do and maintainer has approved and removed target/2026 labels Jul 19, 2026
@dennisdoomen
Fail CI when generated tool wrappers are out of sync with their .json…
0294cce 
… specs

Add VerifyGeneratedTools to CI so that editing a tool wrapper's .json spec
without regenerating and committing its .Generated.cs output fails the
build. Make the generated header's source URL independent of the local
git remote so the check doesn't fail depending on who runs it, and
regenerate all tool wrappers with the canonical repo URL.

Update AGENTS.md, docs/agents/conventions.md, and CONTRIBUTING.md to
reflect that generated tool wrapper output must now be committed
alongside its .json spec.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@dennisdoomen
dennisdoomen force-pushed the dennisdoomen-verify-generated-tools-ci branch from e9ff4b8 to 0294cce Compare July 19, 2026 15:09
@dennisdoomen
dennisdoomen enabled auto-merge (rebase) July 19, 2026 15:11
@dennisdoomen
dennisdoomen merged commit a41f6dd into Fallout-build:main Jul 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisonSimtian ChrisonSimtian ChrisonSimtian approved these changes

Assignees

No one assigned

Labels

Can Merge These PRs are just waiting to be merged, nothing more to do and maintainer has approved skip-changelog target/vCurrent Targets the current version

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dennisdoomen @IT-VBFK @ChrisonSimtian