Skip to content

chore: regenerate sdk#4964

Open
jog1t wants to merge 1 commit intographite-base/4964from
05-04-chore_regenerate_sdk
Open

chore: regenerate sdk#4964
jog1t wants to merge 1 commit intographite-base/4964from
05-04-chore_regenerate_sdk

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented May 4, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 4, 2026

🚅 Deployed to the rivet-pr-4964 environment in rivet-frontend

Service Status Web Updated (UTC)
website 😴 Sleeping (View Logs) Web May 7, 2026 at 12:02 am
frontend-cloud ❌ Build Failed (View Logs) Web May 4, 2026 at 9:16 pm
ladle ❌ Build Failed (View Logs) Web May 4, 2026 at 9:15 pm
mcp-hub ✅ Success (View Logs) Web May 4, 2026 at 9:15 pm
frontend-inspector ❌ Build Failed (View Logs) Web May 4, 2026 at 9:15 pm
kitchen-sink ❌ Build Failed (View Logs) Web May 4, 2026 at 9:14 pm

Copy link
Copy Markdown
Contributor Author

jog1t commented May 4, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

Code Review: chore: regenerate sdk

Overview

This PR regenerates the Go, Rust, and TypeScript SDK clients to simplify RunnerConfigsServerlessMetadataError from a 7-variant undiscriminated union (with typed visitor pattern) into a flat { message, details?, metadata? } envelope. The metadata.kind field now discriminates the variant at runtime. Net: -1287 lines, removing per-variant structs, constructors, unmarshaling switch statements, and visitor interfaces.

The generated files are all under engine/sdks/*/api-full/, consistent with the CLAUDE.md rule that these are auto-generated outputs.


Issues

Breaking change not marked

The PR description leaves all checkboxes unchecked. This is a breaking change for any SDK consumer using the old typed variants (RunnerConfigsServerlessMetadataErrorInvalidRequest, visitor pattern, etc.). Those types are deleted. Please check the "Breaking change" checkbox and add a summary of the impact.

Source schema not in this diff

Per CLAUDE.md: engine/sdks/*/api-* are auto-generated. That implies the source (e.g. engine/artifacts/openapi.json) should have changed first. This PR only touches generated outputs. Either link the upstream schema PR, or include engine/artifacts/openapi.json in this diff so reviewers can verify the regeneration is faithful.

Loss of type safety at error dispatch sites

The old design gave callers a compile-time-checked visitor interface; missing a variant was a compile error. The new metadata: interface{} (Go), Option<Option<serde_json::Value>> (Rust), and metadata?: unknown (TS) push all discrimination to runtime. The intent is documented (metadata.kind discriminates), but nothing enforces it. Consider whether this tradeoff is intentional for this type specifically, or whether a discriminated union on kind could be expressed in the schema and generated instead.

Rust double_option adds unnecessary complexity

#[serde(with = "::serde_with::rust::double_option", ...)]
pub details: Option<Option<String>>,
pub metadata: Option<Option<serde_json::Value>>,

double_option is needed only when the wire format distinguishes between field-absent and field-present-with-null. For error response fields that are either present or omitted, plain Option<String> / Option<serde_json::Value> is sufficient and simpler. If the API actually sends explicit JSON null for these fields, the double_option is correct—but that should be confirmed against the server implementation.

TypeScript metadata?: unknown gives no narrowing guidance

export interface RunnerConfigsServerlessMetadataError {
    details?: string;
    message: string;
    metadata?: unknown;
}

Since the variant is determined by metadata.kind, callers need to narrow the type themselves. There is no exported helper type or union for the possible kind values. If the schema supports it, expressing the kind discriminant in the OpenAPI spec and regenerating would give callers at least a tagged-union type for metadata.


Minor / Nits

  • The PR description is entirely a template with no summary. Please add a one-line description of what changed and why.
  • The new Go struct comment block ("Wire-format envelope for serverless metadata errors...") was auto-generated and is fine for auto-generated code, but it's worth checking that the statement metadata.kind discriminates the variant is actually true at the engine level before this lands—otherwise it becomes misleading documentation for callers.

What looks good

  • The mechanical change is consistent across all three SDKs (Go, Rust, TS serialization + types + index exports all updated together).
  • Removing the untagged Rust enum eliminates the ambiguous deserialization ordering that #[serde(untagged)] is notorious for.
  • The flat { message, details, metadata } shape matches the project's universal RivetError pattern (group/code/message/metadata), which is a better fit than per-variant structs for an error type.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Preview packages published to npm

Install with:

npm install rivetkit@pr-4964

All packages published as 0.0.0-pr.4964.bbdda01 with tag pr-4964.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-bbdda01
docker pull rivetdev/engine:full-bbdda01
Individual packages
npm install rivetkit@pr-4964
npm install @rivetkit/react@pr-4964
npm install @rivetkit/rivetkit-napi@pr-4964
npm install @rivetkit/workflow-engine@pr-4964

@jog1t jog1t changed the base branch from 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config to graphite-base/4964 May 5, 2026 18:18
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