Skip to content

Test that dispatched GitHub build ids pass getployz/build's pattern - #2241

Merged
nick-potts merged 2 commits into
mainfrom
fix/bug-deploys-32
Oct 19, 2026
Merged

nick-potts merged 2 commits into
mainfrom
fix/bug-deploys-32

Conversation

@nick-potts

Copy link
Copy Markdown
Contributor

Every GitHub build failed at once with Invalid build id.. Cloud dispatches build=<deploymentId>.<service>, but getployz/build@v1 only accepted [A-Za-z0-9_-]. Cloud then quietly built on a Server instead. Found in the beta.6 soak (deploys-32).

The fix is in the action: getployz/build#6 accepts . in the id. Cloud keeps the dotted format because Machines parse it with split_once('.') (GithubBuildId in ployz-store).

This PR guards the contract from this side:

  • githubBuildId(deploymentId, service) in github-build-workflow.ts is now the one place Cloud mints the id. planStoreGithubBuilds uses it.
  • github-build.test.ts checks that ids for the shortest, a typical, and the longest Service name (a 63-character DNS label) match the action's pattern, copied from prepare.sh.

Testing

  • pnpm exec vitest run src/modules/github/github-build.test.ts: 6 passed. With the old v1 pattern (^[A-Za-z0-9_-]{1,128}$), the new test fails with expected '36755d9a-dc30-4385-a9bd-86ae1a85e44b.a' to match /^[A-Za-z0-9_-]{1,128}$/.
  • pnpm typecheck and oxlint --type-aware on the changed files are clean.

Release

Users get the fix only after getployz/build#6 merges and its v1 tag moves to the merge commit. This PR does not depend on that.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 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-10-18T23:41:43.600861Z f81b89c PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2f1553a8-8588-477b-8037-c72eb6026b60
📥 Commits

Reviewing files that changed from the base of the PR and between 251add9 and f81b89c.

📒 Files selected for processing: 3
  • dashboard/src/modules/config-store/store-github-builds.server.ts
  • dashboard/src/modules/github/github-build-workflow.ts
  • dashboard/src/modules/github/github-build.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Build identifiers for GitHub deployments now follow the format accepted by the build action.

The change adds githubBuildId to combine a deployment ID and service name. Store GitHub build planning uses the helper, and tests check that generated IDs match the build action’s accepted pattern.

Changes: Store GitHub build IDs

Layer / File(s) Summary
Define and test the build ID format
dashboard/src/modules/github/github-build-workflow.ts, dashboard/src/modules/github/github-build.test.ts
githubBuildId joins the deployment ID and service with a period. Tests check the generated IDs against the build action’s accepted pattern.
Use the helper for Store builds
dashboard/src/modules/config-store/store-github-builds.server.ts
planStoreGithubBuilds uses githubBuildId to generate each target’s build identifier.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to f81b8

The Store build IDs retain their existing format, and the change adds a contract test. No material merge-readiness risk introduced by this PR is evident.

### Architecture Summary

Architecture risk: 🟡 Medium · up to f81b8

The change affects 1 system.

Changed systems: dashboard

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — dashboard (service) was modified; 3 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in dashboard/src/modules/config-store/store-github-builds.server.ts: Imports githubBuildId for generating Store GitHub build identifiers.
  • observed — Modified behavior in dashboard/src/modules/config-store/store-github-builds.server.ts: planStoreGithubBuilds now uses githubBuildId(data.deploymentId, source.service) instead of forming the identifier by joining the deployment ID and service with a period.
  • observed — Modified behavior in dashboard/src/modules/github/github-build-workflow.ts: Added githubBuildId, returning the deployment ID and service joined by a period.
  • observed — Modified behavior in dashboard/src/modules/github/github-build.test.ts: The test file now imports githubBuildId alongside the workflow constant and URL helper.

Reliability and maintainability

  • inferred — Risk-relevant change factors for dashboard: blast_radius_1; direct_dependents_1
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the contract test for dispatched GitHub build IDs and matches the main change.
Description check Passed The description directly explains the invalid build ID failure, the centralized ID helper, the contract tests, and the reported validation results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@nick-potts
nick-potts merged commit 0d76007 into main Oct 19, 2026
17 checks passed
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.

1 participant