fix(seed): stop go-sdk idempotency-headers fixture from overwriting its nested config - #17318
Conversation
…ts nested config The '.' output folder wiped the sibling auto-generate-idempotency-key output, producing conflicting Update Seed patches. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Straightforward fix: moves the . output config for go-sdk's idempotency-headers fixture into no-custom-config/ and adds a guard preventing a fixture from mixing outputFolder: "." with siblings. Logic is sound; the validator is a bit narrow (only catches the literal "." case) and the hard throw at load time will break unrelated seed commands.
- 🔵 2 suggestion(s)
| const outputFolders = configurations.map((configuration) => configuration.outputFolder); | ||
| if (outputFolders.length > 1 && outputFolders.includes(".")) { |
There was a problem hiding this comment.
🔵 suggestion
The check only catches the literal ".". It misses "./", "", and the more general nesting case (outputFolder: a + outputFolder: a/b), which has exactly the same rm -rf hazard. Consider normalizing with path.normalize and comparing prefixes:
| const outputFolders = configurations.map((configuration) => configuration.outputFolder); | |
| if (outputFolders.length > 1 && outputFolders.includes(".")) { | |
| const normalized = outputFolders.map((f) => path.normalize(f ?? ".").replace(/\/+$/, "")); | |
| const conflicts = normalized.filter((folder, i) => | |
| normalized.some((other, j) => i !== j && (other === folder || other.startsWith(`${folder}/`))) | |
| ); | |
| if (conflicts.length > 0) { |
(and adjust the message body accordingly). At minimum, normalize "./" → "." so the guard can't be trivially bypassed.
| const errors = validateFixtureOutputFolders({ workspaceName: workspace, workspaceConfig }); | ||
| if (errors.length > 0) { | ||
| throw new Error(`Invalid ${SEED_CONFIG_FILENAME}:\n${errors.join("\n")}`); | ||
| } |
There was a problem hiding this comment.
🔵 suggestion
loadGeneratorWorkspaces is called by every seed command, so one malformed seed.yml in an unrelated generator now hard-fails all of them. Consider collecting errors across workspaces and throwing once at the end (so users see every problem), or warning + skipping like the disabled branch above.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
1 participant
Description
The
Update Seedworkflow has failed on every push to main since 7/28 (e.g. run 30761733223, 30769540043):seed/go-sdk/seed.ymlwas the only place in the repo where one fixture had a config writing to the fixture root plus a config nested underneath it:Local generation does
rm -rf <output folder>before copying files (LocalTaskHandler.copyGeneratedFilesNoFernIgnoreDeleteAll), so the.config deletes the sibling's output. That folder has been thrashing in the snapshot PRs since #17009 introduced the nested config — alternately deleted and re-added (#17246 … #17278). It became a hard failure once #17256 changed the go coreinternal/query.go(present in every fixture): the shard runningidempotency-headers:auto-generate-idempotency-keynow uploads a patch modifying those files while the shard runningidempotency-headers:.uploads a patch deleting the folder, andcommit-seed-changes-by-prapplies both in onegit apply. It is self-sustaining, since the snapshot on main never gets updated.Changes Made
.config forgo-sdk'sidempotency-headersfixture intono-custom-config/(seed.ymlplus the generated snapshot, a pure file move — no content changes)validateFixtureOutputFolders, run fromloadGeneratorWorkspaces, which rejects anyseed.ymlfixture that mixesoutputFolder: .with sibling output foldersTesting
packages/seed/src/__test__/validateFixtureOutputFolders.test.ts;pnpm testinpackages/seedpasses (225 tests)loadGeneratorWorkspaces()against the repo'sseed/now resolvesidempotency-headersto["no-custom-config", "auto-generate-idempotency-key"], and no otherseed.ymlfixture trips the new validationLink to Devin session: https://app.devin.ai/sessions/ed6999c08f884f04b6b5a7b29603e123