fix(seed): unnest go-sdk idempotency-headers output and make orphan cleanup push resilient - #17357
fix(seed): unnest go-sdk idempotency-headers output and make orphan cleanup push resilient#17357devin-ai-integration[bot] wants to merge 2 commits into
Conversation
… and make orphan cleanup push resilient 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
Fixes two seed-workflow failures: an overlapping . output folder in the go-sdk seed config (moved to no-custom-config/), and a non-fast-forward push race in the orphan-cleanup job (now retries with rebase). The fixture move itself is a clean 100%-similarity rename. Two things worth tightening: unchecked rebase failure in the retry loop, and an apparent divergence between the two idempotency-headers outputs' internal/query.go.
- 🟡 2 warning(s)
- 🔵 1 suggestion(s)
AI Review
🟡 warning — seed/go-sdk/idempotency-headers/auto-generate-idempotency-key/internal/query.go (line 89)
This applyQueryDefaultsOnNilRequest block lands only in the auto-generate-idempotency-key output; no-custom-config/internal/query.go is a 100%-similarity rename, i.e. it still has the old body. Both outputs come from the same generator and this looks like generator-core code, so one shard appears not to have been regenerated. Worth re-running seed for both configs to confirm the divergence is intentional rather than a stale fixture that the next main seed run will churn.
| # main moves constantly (release commits), so rebase and retry | ||
| # instead of losing the race to a non-fast-forward rejection. | ||
| for attempt in 1 2 3 4 5; do | ||
| if git push; then | ||
| exit 0 | ||
| fi | ||
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/main..." | ||
| git pull --rebase --autostash origin main | ||
| sleep 5 | ||
| done | ||
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | ||
| exit 1 |
There was a problem hiding this comment.
🟡 warning
The retry loop swallows a couple of failure modes:
git pull --rebase --autostashisn't checked. GitHub's default shell isbash -e, so a conflicting rebase kills the step immediately with a cryptic git error and leaves the working tree mid-rebase — the "Failed to push..." message never prints.exit 0terminates the whole step, not just the loop. Works today because nothing follows theif/fi, but it silently breaks the moment someone appends a command.
| # main moves constantly (release commits), so rebase and retry | |
| # instead of losing the race to a non-fast-forward rejection. | |
| for attempt in 1 2 3 4 5; do | |
| if git push; then | |
| exit 0 | |
| fi | |
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/main..." | |
| git pull --rebase --autostash origin main | |
| sleep 5 | |
| done | |
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | |
| exit 1 | |
| # main moves constantly (release commits), so rebase and retry | |
| # instead of losing the race to a non-fast-forward rejection. | |
| pushed=false | |
| for attempt in 1 2 3 4 5; do | |
| if git push; then | |
| pushed=true | |
| break | |
| fi | |
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/main..." | |
| if ! git pull --rebase --autostash origin main; then | |
| git rebase --abort || true | |
| echo "Rebase onto origin/main failed; aborting cleanup." | |
| exit 1 | |
| fi | |
| sleep 5 | |
| done | |
| if [ "$pushed" != "true" ]; then | |
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | |
| exit 1 | |
| fi |
| if applyQueryDefaultsOnNilRequest { | ||
| // A nil request carries no explicit values, so every default applies. | ||
| for wireName, defaultVal := range defaults { | ||
| values.Set(wireName, valueString(reflect.ValueOf(defaultVal), tagOptions{}, reflect.StructField{})) |
There was a problem hiding this comment.
🔵 suggestion
reflect.ValueOf(defaultVal) is an invalid reflect.Value when defaultVal is nil (map[string]interface{}{"x": nil}), which will likely panic inside valueString. The non-nil-request path presumably goes through the same helper, so it may already be guarded — but if this is generator-emitted code, worth confirming upstream.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # main moves constantly (release commits), so rebase and retry | ||
| # instead of losing the race to a non-fast-forward rejection. | ||
| for attempt in 1 2 3 4 5; do | ||
| if git push; then | ||
| exit 0 | ||
| fi | ||
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/main..." | ||
| git pull --rebase --autostash origin main | ||
| sleep 5 | ||
| done | ||
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | ||
| exit 1 |
There was a problem hiding this comment.
🟡 Automated cleanup can rewrite a non-main branch onto main's history
The retry path always re-bases the cleanup commit onto the main branch (git pull --rebase --autostash origin main at .github/workflows/seed.yml:92) even when the job is running on a different branch, so a manually triggered run on that branch can push main's commits into it.
Impact: A manual run on a non-main branch can silently merge main's history into that branch and push it.
Why the branch can differ from main
The step guard is only github.event_name != 'pull_request' (.github/workflows/seed.yml:74), and the workflow is also triggered by workflow_dispatch and workflow_call (.github/workflows/seed.yml:14-15), which can run on any ref. On such a ref the checked-out branch is not main; if the first git push is rejected, the hardcoded origin main rebase replays the cleanup commit on top of origin/main and the following push writes that history to the current branch.
| # main moves constantly (release commits), so rebase and retry | |
| # instead of losing the race to a non-fast-forward rejection. | |
| for attempt in 1 2 3 4 5; do | |
| if git push; then | |
| exit 0 | |
| fi | |
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/main..." | |
| git pull --rebase --autostash origin main | |
| sleep 5 | |
| done | |
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | |
| exit 1 | |
| # main moves constantly (release commits), so rebase and retry | |
| # instead of losing the race to a non-fast-forward rejection. | |
| for attempt in 1 2 3 4 5; do | |
| if git push; then | |
| exit 0 | |
| fi | |
| echo "Push rejected (attempt $attempt/5), rebasing onto origin/${{ github.ref_name }}..." | |
| git pull --rebase --autostash origin "${{ github.ref_name }}" | |
| sleep 5 | |
| done | |
| echo "Failed to push orphaned seed folder cleanup after 5 attempts." | |
| exit 1 |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — the step also runs on workflow_dispatch/workflow_call refs. Applied the suggestion: rebase onto origin/${{ github.ref_name }} instead of hardcoding main.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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 |
1 participant
Description
Fixes the two recurring
mainfailures in the seed workflows.1.
Update Seed→commit-seed-changes-by-prfails atgit apply artifacts/seed-*.patchseed/go-sdk/seed.ymldeclaredidempotency-headerswithoutputFolder: .plus a second config inauto-generate-idempotency-key, i.e. one output nested inside the other's directory. Those two configs land in different CI shards; the.shard wipes the fixture root before regenerating, so its patch deletes all 44 files underauto-generate-idempotency-key/, while the other shard's patch modifiesinternal/query.goin that same folder. Both patches are applied by onegit apply, the delete lands first, and the modify then fails. It was the only fixture in the repo with a.output and a nested sibling output.The
.output now writes tono-custom-config/(matching the ruby/csharp seed configs for this fixture), so no output folder contains another.2.
Seed Snapshot Tests→check-orphaned-seed-foldersfails atgit pushThe auto-clean step commits directly to
mainwith a baregit push, andmainadvances during the ~2 min job (release commits land constantly), so it always loses the race. It now rebases and retries.The folders it was trying to clean are also removed here, along with the loose root files the detector misses (it only walks directories), so
pnpm seed:local clean --dry-runis now clean.Changes Made
seed/go-sdk/seed.yml:idempotency-headersoutputFolder: .→no-custom-config; regenerated seed output (pure move of the fixture-root output intono-custom-config/, plus the pendingapplyQueryDefaultsOnNilRequestupdate underauto-generate-idempotency-key/)..github/workflows/seed.yml: retry loop withgit pull --rebase --autostash origin mainaround the orphan-cleanup push.seed/go-sdk/x-fern-default/andseed/python-sdk/simple-api/from earlier.→ named-folder migrations.Testing
pnpm seed test --generator go-sdk --fixture idempotency-headers --skip-scripts→ 2/2 passed; the regeneratedno-custom-config/output is byte-identical to the previous fixture-root output.pnpm seed:local clean --dry-run→ "No orphaned seed folders found.".output folder overlaps a sibling output folder in anyseed/*/seed.yml.Link to Devin session: https://app.devin.ai/sessions/92daa6febdf344f2a7b56b3900d31309