Skip to content

Gate the seed on an SDK_LANGUAGES profile and check that its generate targets do work - #21

Open
jeremy wants to merge 7 commits into
mainfrom
iss7/generate-coverage-and-profile
Open

jeremy wants to merge 7 commits into
mainfrom
iss7/generate-coverage-and-profile

Conversation

@jeremy

@jeremy jeremy commented Sep 13, 2026 •

Copy link
Copy Markdown
Member

Closes #7.

Two things the seed's Makefile could not say: which languages an SDK actually ships, and whether a <prefix>-generate-services target does anything when called. Both items from the issue, one change.

Profile: SDK_LANGUAGES

One variable at the top of the rendered Makefile, by prefix, defaulting to the full set:

SDK_LANGUAGES := go ts rb swift kt rs

check, check-full, generate-services, generate-services-check, clean and conformance derive from it; check-mvp's go-check and the Go-embedded provenance-check are included only when go is listed. Conformance runners are named by language, so a CONFORMANCE_RUNNER_<prefix> map turns ts into conformance-typescript; Swift has no mapping because the aggregate did not run the Swift runner before this change either (the seeded test.yml runs it on a macOS leg), and conformance-build is split into conformance-build-{go,typescript,kotlin} so it too only builds the profile's runners. Per-language targets are untouched: make ts-check on a go rs profile still exists and still fails on the missing tree, which is the point.

On the full profile the aggregates expand to the same commands in the same order as before. I diffed make -n of check, check-full, check-mvp, generate-services, conformance, conformance-build and clean between a render of main and a render of this branch: the only differences are the new generate-services-check step in check and the per-runner ==> banners under conformance-build. (generate-services now lists swift-generate-services instead of swift-generate; both expand to $(MAKE) -C swift generate.)

Rejected alternative: infer the profile from the directory tree. sync-api-version-check and sync-version.sh already skip a language whose file is absent, so ranging the aggregates over $(wildcard go typescript ruby ...) would have needed no instantiation step at all. It was rejected because it makes the gate narrow itself: a scaffold step that silently failed to create go/ would remove Go from check instead of failing it, which is the same class of silent no-op item 1 exists to catch. The file-presence skips in the version sync stay, because a constant that does not exist has nothing to be out of sync with; the gate's scope is declared, not observed.

Not covered here, and named in MAKEFILE-CONVENTION.md: make release's version-constant guards still grep all six languages' files, so a narrower profile cannot yet release. That is a separate change to the release target.

Coverage: scripts/check-generate-targets.sh

Ships in seed/scripts/, so it lands in every seeded repo. For each prefix in the profile it asserts three things:

  1. the generator artifact the root recipe invokes exists (go/cmd/generate-services, typescript/scripts/generate-services.ts, ruby/scripts/generate-services.rb, swift/Makefile, kotlin/generator/build.gradle.kts, rust/generator/Cargo.toml);
  2. make -n <prefix>-generate-services succeeds and its plan, after dropping echoes and make's own recursion and "Nothing to be done" lines, contains the invocation of that generator (go run ./cmd/generate-services, scripts/generate-services.ts, ruby scripts/generate-services.rb, gradlew :generator:run, cargo run .*-generator, and for Swift a swift run line). A prerequisite's commands do not satisfy it: ts-install still plans npm ci after the ts recipe is gone, and that must fail. make -n follows $(MAKE) -C swift generate into the sub-Makefile, so Swift's delegation is covered by the same rule: a generate target with no recipe there, or a root alias that lost its swift-generate prerequisite, both come back as "never invokes the generator";
  3. unless --dry-run, make <prefix>-generate-services runs and exits 0.

make generate-services-check runs the dry form and is part of make check (it is non-mutating; the full form rewrites generated trees, which check must not do, so the full form is the bootstrap checkpoint in prompts/seed-sdk.md). The check does not assert what the generator wrote: that is what <prefix>-check-drift is for, and some generators legitimately skip unchanged files.

This repo cannot run the full form: the seed ships no generator for any language, so on a pristine render all six targets fail, exactly as the issue describes. What CI pins instead is the detector, in hack/test/check-generate-targets.bats (new bats step in the "Seed templates" job): the pristine render fails for go ts rb swift kt rs; stubbed artifacts pass, Swift through its sub-Makefile; a sub-Makefile generate with no recipe fails; a root alias with no prerequisite fails; a recipe removed while its prerequisites still plan work fails; a generator named only inside an echo fails; a go rs profile is checked for go rs only; run mode (a fake swift on PATH) reports the target's exit status; an unknown prefix is rejected. The "temporarily break one target" proof from the issue is tests 4 through 7.

Running it locally

bats hack/test                                   # the detector's own tests
hack/render-seed.sh /tmp/r && cd /tmp/r
scripts/check-generate-targets.sh --dry-run      # fails: no generator ships in the seed
make generate-services-check                     # same, via the Makefile
make -n check SDK_LANGUAGES="go rs"              # a narrowed profile's expansion

In a seeded repo: make generate-services-check (dry), scripts/check-generate-targets.sh (runs every generate target in the profile), scripts/check-generate-targets.sh swift (one language).

Gates run locally: hack/render-seed.sh, actionlint on the rendered workflows, zizmor 1.30.0 --config hack/zizmor-seed.yml --min-severity high on them, shellcheck on the rendered scripts (the new script included), actionlint + zizmor on .github/workflows/ci.yml, and bats hack/test. seed/rust/ is untouched, so the rendered Rust scaffold job is unaffected.

Docs

MAKEFILE-CONVENTION.md gains a Language Profile section and a generate-services-check row; AGENTS.md a section on what adding a language to the seed touches and what the bats file pins; seed/AGENTS.md.tmpl and seed/CONTRIBUTING.md.tmpl replace the interim "all six unconditionally, trim at instantiation" notes from #5 with the contract; prompts/seed-sdk.md sets SDK_LANGUAGES in Phase 3, runs scripts/check-generate-targets.sh <prefix> as each language's step 4, trims the conformance matrix in Phase 5, and adds the full run to post-bootstrap verification.

Copilot AI balanced review requested due to automatic review settings September 13, 2026 00:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-13T02:37:53.626684Z 15e1410 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The detector can mistake prerequisite setup commands for generator work and approve a vacuous target.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Introduces an SDK_LANGUAGES profile for generated SDKs and validates generation targets.

Changes:

  • Makes aggregate Make targets profile-aware.
  • Adds generator wiring validation.
  • Adds documentation and Bats CI coverage.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File summaries
File Description
seed/scripts/check-generate-targets.sh Validates generator artifacts and target plans.
seed/Makefile.tmpl Adds profile-aware aggregates and validation target.
seed/CONTRIBUTING.md.tmpl Documents profile-aware contributor workflow.
seed/AGENTS.md.tmpl Updates generated repository guidance.
prompts/seed-sdk.md Integrates profiles and validation into bootstrapping.
MAKEFILE-CONVENTION.md Defines the language-profile contract.
hack/test/check-generate-targets.bats Tests generator validation behavior.
AGENTS.md Documents repository maintenance requirements.
.github/workflows/ci.yml Runs the new Bats coverage.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Balanced

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

Comment thread seed/scripts/check-generate-targets.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c937a52ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread seed/scripts/check-generate-targets.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b69881bdc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread seed/Makefile.tmpl Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unanchored invocation matching can still approve an echo-only generator target, and several documents overstate Swift conformance coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

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

prompts/seed-sdk.md:28

  • This says make conformance follows every profile entry, but swift is intentionally unmapped in the Makefile, so a full profile still omits Swift from this aggregate. Mention the macOS CI exception here; otherwise the bootstrap instructions overstate what the local gate covers.
    seed/AGENTS.md.tmpl:38
  • This overstates the profile contract because Swift is listed in SDK_LANGUAGES but intentionally omitted from CONFORMANCE_RUNNERS. Call out that exception here so generated agent guidance does not claim make check runs Swift conformance.
    seed/CONTRIBUTING.md.tmpl:35
  • The “nothing else” claim is inaccurate for Swift: SDK_LANGUAGES can include swift, but the aggregate deliberately has no Swift conformance mapping. Document that Swift conformance remains CI-only so contributors do not mistake make check for covering it.
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread seed/scripts/check-generate-targets.sh
@jeremy

jeremy commented Sep 13, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a72ae91a9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread seed/scripts/check-generate-targets.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 017fbd6fce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread seed/scripts/check-generate-targets.sh
Comment thread seed/scripts/check-generate-targets.sh
Comment thread seed/Makefile.tmpl
@jeremy

jeremy commented Sep 13, 2026

Copy link
Copy Markdown
Member Author

Review threads: 7 resolved (4 fixed, 3 declined with the reasoning in each thread).

Declined:

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.

Executable coverage for bootstrapped generate targets, and a profile contract for make check

2 participants