Extend update-sdk-examples skill to cover the RPC OpenRPC spec#2670
Open
ElliotFriend wants to merge 4 commits into
Open
Extend update-sdk-examples skill to cover the RPC OpenRPC spec#2670ElliotFriend wants to merge 4 commits into
ElliotFriend wants to merge 4 commits into
Conversation
The skill already tracks stellar/stellar-rpc for the Go-client relocation; a new release should also trigger a check of the Stellar RPC OpenRPC spec we ship in openrpc/. Adds a dedicated section describing that workflow: - bump info.version in openrpc/scripts/build.mjs to the current release - verify each method's request/response types against the canonical Go structs in go-stellar-sdk/protocols/rpc at the commit stellar-rpc pins (incl. the ,string wire-encoding quirks and per-method type divergence) - refresh examples from the live testnet RPC, truncating oversized xdr blobs - regenerate with pnpm rpcspec:build and validate with pnpm rpcspec:validate, committing sources + regenerated static spec together Also wires the spec outcome into the report and cross-links from the existing stellar-rpc gotcha. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Extends the SDK-update skill to audit and regenerate the Stellar RPC OpenRPC specification.
Changes:
- Adds release-triggered OpenRPC auditing and reporting.
- Documents type verification, example refresh, build, and validation steps.
- Reuses the existing
stellar-rpcrelease state.
Recommendation: NEEDS-CHANGES — fix state sequencing, version normalization, and JSON wire-shape inference.
Comment on lines
+178
to
+182
| URL. For each method, compare our `methods/<m>.json` params and result against | ||
| the Go struct `json:"..."` tags — missing/extra/renamed fields, type | ||
| mismatches, and required-vs-optional (`,omitempty` or a pointer ⇒ optional). | ||
| Resolve our `$ref`s (into `schemas/`, `contentDescriptors/`) before concluding | ||
| a field is missing. **Wire-encoding quirks matter:** a `json:",string"` tag |
|
Preview is available here: |
Contributor
|
Reviewed with full source verification — the encoded procedure is factually accurate, but I'd address two of Copilot's comments before merge. Verified against primary sources (all true):
On Copilot's five comments — two are confirmed real and worth a small patch:
The other three (future tagged go.mod pin, Since this skill runs unattended on a schedule, the state-ordering gap is the one I'd consider blocking; everything else is polish. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Preview is available here: |
…alers Addresses Copilot review on the OpenRPC spec-check step. Corrects the required-vs-optional heuristic (a pointer without `,omitempty` is a required nullable field, not optional) and adds a caveat that types with custom Marshal/UnmarshalJSON serialize independently of their struct tags (EventTypeSet, SegmentFilter, LedgerEntryChangeType). Keeps the existing `json:",string"` wire-encoding guidance. Examples verified against go-stellar-sdk protocols/rpc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Kaan's review flagged that the state-ordering fix lived only in the RPC spec section while step 3.6 still gave a blanket "update the state file entry" instruction — a latent contradiction for the stellar/stellar-rpc entry. Carve out the exception where the agent actually acts, so an interrupted spec audit reliably stays in scope for the next run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Preview is available here: |
1 similar comment
|
Preview is available here: |
Contributor
Author
|
Thanks for the thorough review, @kaankacar — the full source verification is much appreciated. All points are now resolved:
Should be good to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
update-sdk-examplesskill already tracksstellar/stellar-rpc(for the Go-client relocation). This extends it so a newstellar-rpcrelease also triggers a check of the Stellar RPC OpenRPC spec we ship inopenrpc/— encoding the workflow used in #2669.Changes to
.claude/skills/update-sdk-examples/SKILL.mdinfo.versioninbuild.mjs; verify each method's request/response types against the canonical Go structs ingo-stellar-sdk/protocols/rpcat the commitstellar-rpcpins (including the,stringwire-encoding quirks and the per-method string-vs-number divergence); refresh examples from the live testnet RPC (truncating oversized xdr blobs); regenerate withpnpm rpcspec:buildand validate withpnpm rpcspec:validate, committing sources + regeneratedstatic/stellar-rpc.openrpc.jsontogether.stellar/stellar-rpcstate-file entry to gate the check (no new state key).stellar-rpcgotcha and the intro/Context.No behavior change to the SDK-example workflow itself.
🤖 Generated with Claude Code