BOG-223 Rust Upbit SDK implementation#10
Conversation
Co-authored-by: multica-agent <github@multica.ai>
Add Rust workspace and Upbit API spec
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
…config Add SDK core auth config and error handling
Co-authored-by: multica-agent <github@multica.ai>
…ormance BOG-236 Add spec-driven Upbit mock server
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
…g-performance Add retry fallback and redacted logging controls
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
…dk' into issue/BOG-238-sdk-endpoint-coverage # Conflicts: # Cargo.lock # crates/upbit-sdk/Cargo.toml # crates/upbit-sdk/src/lib.rs
BOG-238 Add typed Upbit REST SDK endpoint coverage
Co-authored-by: multica-agent <github@multica.ai>
…diness BOG-242 Prepare SDK docs and package metadata
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
BOG-244 Add crates.io publish workflow
Co-authored-by: multica-agent <github@multica.ai>
BOG-246 Trigger crates publish from GitHub releases
Co-authored-by: multica-agent <github@multica.ai>
…y-pr-readiness BOG-240 Final spec reconciliation evidence
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
I've completed the review of PR #10 implementing the Rust Upbit SDK. The code is well-structured with comprehensive security practices, proper error handling, and extensive test coverage. However, I found 1 critical logic error that blocks merge.
Critical Issue (Must Fix)
Workflow Logic Error (.github/workflows/publish-crates.yml): The git ancestry validation check is inverted. Line 107 currently checks if tag_commit is an ancestor of main_commit, but it should check if main_commit is an ancestor of tag_commit. This will cause the workflow to reject valid release tags that point to commits already in main, and potentially accept tags pointing to commits not yet in main.
Positive Observations
The implementation demonstrates strong security practices:
- Credentials are properly wrapped in
SecretValuewith redacted debug/display formatters - JWT signing uses secure HMAC-SHA512 with proper query hash validation
- HTTP-only URLs are restricted to loopback addresses for authenticated requests
- Sensitive data is redacted in logs and URLs
- Retry/fallback logic correctly restricts unsafe HTTP methods by default
The code quality is excellent with comprehensive error handling, proper validation, and thorough test coverage across all core modules.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| tag_commit="$(git rev-parse "${GITHUB_REF_NAME}^{commit}")" | ||
| main_commit="$(git rev-parse "origin/main^{commit}")" | ||
|
|
||
| if ! git merge-base --is-ancestor "${tag_commit}" "${main_commit}"; then |
There was a problem hiding this comment.
🛑 Logic Error: The ancestry check validates that main_commit is reachable from tag_commit, but it should verify the opposite. This will incorrectly reject valid release tags that point to commits already merged into main and incorrectly accept tags that point to commits not yet in main.
| if ! git merge-base --is-ancestor "${tag_commit}" "${main_commit}"; then | |
| if ! git merge-base --is-ancestor "${main_commit}" "${tag_commit}"; then |
1 participant
Summary
This PR merges the completed BOG-223 Upbit Rust SDK integration branch into
main.It adds a Rust Cargo workspace for:
upbit-sdk: async Upbit REST SDK with typed request/response coverage, JWT signing, error handling, retry/fallback controls, and redacted logging helpers.upbit-mock: local spec-driven mock server and conformance baseline for credential-free SDK tests.spec/upbit-rest-api.yaml: repo-owned API contract derived from the BOG-228 Upbit official documentation research.Scope And Evidence
Child work merged into
integration/BOG-223-upbit-rust-sdk:katyo/publish-cratesaction, pinned by SHA.Final reconciliation is recorded in
docs/final-reconciliation.md.API Coverage Decision
The integration branch preserves the BOG-228 inventory count of 45 API items, but reconciles the official documentation as:
list_subscriptions, represented asprotocol: websocketand intentionally excluded from REST SDK/mock route coverage.The SDK and mock coverage tests use
spec/upbit-rest-api.yamlas the source of truth and filter byprotocol == rest.Verification
Evidence recorded by BOG-240 QA/review:
ruby -ryaml -e '...'endpoint inventory and SDK coverage checks: pass.cargo fmt --check: pass.cargo clippy --workspace --all-targets -- -D warnings: pass.cargo test --workspace: pass (upbit-mock8 tests,upbit-sdk37 tests, doc-tests 0/0).cargo test --workspace --doc: pass.cargo package -p upbit-sdk --allow-dirty --list: pass, package listing only.SUCCESS.upbit-sdkunpublished at that time.Release And Publishing Preconditions
This PR does not publish to crates.io, create a GitHub Release, create a release tag, or execute any deployment.
Before a real publish, repository operators still need to configure and verify:
CARGO_REGISTRY_TOKENGitHub secret.crates-ioGitHub Environment.mainbranch and release approval process.The publish workflow is designed so PR, integration-branch, main-branch, and dry-run manual paths cannot publish because they run dry-run mode and do not pass the registry token to the third-party action.
Contribution Notes
Repository contribution materials checked before creating this PR:
README.md.github/workflows/publish-crates.ymlCargo.tomlfilesNo
CONTRIBUTING*,LICENSE*,SECURITY*,.github/PULL_REQUEST_TEMPLATE*,.github/ISSUE_TEMPLATE/*, or CODEOWNERS file was present in the integration branch, so this body is written without a repository PR template.