Align public error contracts

[tolgahan-arikan]

Summary

• Align Swift SDK public error contracts with the native Kotlin/TypeScript SDK shape: nullable retryable, normalized upstreamError, stable operation IDs, and transaction recovery fields.
• Preserve transaction write boundaries with OMS_TRANSACTION_EXECUTION_UNCONFIRMED for failed execute calls and OMS_TRANSACTION_STATUS_LOOKUP_FAILED for failed post-submit polling.
• Add centralized public error contract tests and document the contract matrix in docs/error-contracts.md, API.md, README.md, and TESTING.md.

Impact

Apps can branch on stable OmsSdkErrorCode raw values while using operation, status, txnId, retryable, and upstreamError for recovery, logging, and diagnostics. Transaction errors now distinguish retrying status lookup from resending a write.

Validation

• swift build
• swift test

[github-actions]

Claude finished @tolgahan-arikan's task —— View job

---

PR Review: Align public error contracts

I reviewed the error-contract changes against the focus areas. Overall this is a well-structured, conservative change with strong test coverage and documentation that stays in sync with the code. Approving in spirit — only minor/optional notes below.

✅ Security (private key material / P-256 non-extractable)

No regressions. The diff never persists key material into session storage. Session expiry/clear paths (WalletSessions.swift:95-108, 220-239) continue to call credentialSession.clearSignerKeepingCredentials() / clear() and rebuild the signed client; they only touch wallet ID/address/metadata. The Keychain-backed signer remains the single owner of credential state. OIDC redirect state stays separate from completed session state, and restorePendingOidcRedirectAuth (WalletAuth.swift:307-317) still validates signerCredentialId + key type before resuming — good.

✅ No floating-point for token amounts

formatTokenAmount (WalletOperations.swift:494-498) delegates to formatUnits; zero-balance fallbacks use the string "0". No float math introduced.

✅ Platform availability

New public surfaces (OmsSdkError, OmsSdkErrorCode, OmsSdkOperation, OmsUpstreamService, OmsUpstreamError) are plain value types with no availability-gated APIs, consistent with how other pure model types in the SDK are declared. The internal runOmsOperation helpers correctly carry @available(macOS 12.0, iOS 15.0, *) (OmsSdkError.swift:153-177). No missing annotation found.

✅ Test coverage

PublicErrorContractsTests.swift is thorough — it exercises real public calls across WaaS transport/domain/HTTP/bad-response families, local session/selection/OIDC errors, indexer backend/transport/malformed paths, and the two transaction-boundary codes. The non-JSON sanitization assertions (:114-115, :581-582) are a nice touch verifying raw HTML bodies don't leak into message/upstreamError.

✅ Correctness — transaction write boundaries

The execute vs. polling distinction is implemented correctly:

• Execute failure → transactionExecutionUnconfirmed, retryable: false, preserves txnId (WalletOperations.swift:238-250).
• Post-submit polling failure → transactionStatusLookupFailed, retryable: true, preserves txnId (:401-413).
• toOmsSdkError preserves these via isNestedTransactionBoundary so the outer walletSendTransaction/walletCallContract wrapper doesn't overwrite them (OmsSdkError.swift:181, 411-415). Verified by the polling test which asserts operation == .walletTransactionStatus survives the outer wrapper.

---

Minor / optional observations (non-blocking)

1. Unused enum cases. OmsSdkErrorCode.walletSelectionInFlight (OmsSdkError.swift:12) and OmsSdkOperation.pendingWalletSelection (:19) are declared but never thrown/used in SDK code (walletTransactionStatus and the others are used). If these exist purely for Kotlin/TS parity, that's reasonable — but a one-line comment or a note in docs/error-contracts.md would prevent a future reader from assuming they're dead code. Worth confirming intent.

2. normalizedCode is computed but its companion isn't symmetric. In WebRPCError.toOmsSdkError (OmsSdkError.swift:266), the .invalidResponse branch condition kind == .webrpcBadResponse || kind == .unknown && code == ... relies on Swift precedence (&& binds tighter than ||). It's correct, but adding parentheses kind == .webrpcBadResponse || (kind == .unknown && code == WebRPCErrorKind.unknown.code) would make intent unambiguous.

3. Indexer transport upstream name. String(describing: type(of: error)) yields e.g. NSURLError for a URLError (asserted in test :598). This is fine and tested; just flagging that the name is Swift-runtime-derived rather than a stable service code, which the docs already acknowledge ("service-specific").

4. Docs/paths. No new shell examples with unquoted spacey paths were introduced in the README/API diffs. docs/error-contracts.md correctly references Tests/Swift SDKTests/PublicErrorContractsTests.swift as a path string (not a shell command), so quoting isn't required there.

---

⚠️ Could not run build/tests

swift build / swift test are not permitted in this review environment, so I could not independently verify compilation. The PR reports both pass, and CI (ci.yml) gates this. Please ensure the green CI run reflects the latest commit.

---

Summary: No security, floating-point, availability, or contract-correctness issues found. The four notes above are optional polish (mainly: clarify the two unused parity enum cases). Nice work keeping API.md, README.md, TESTING.md, and docs/error-contracts.md aligned with the implementation.
• align-error-contracts

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 000d45e01e

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[tolgahan-arikan]

@claude review

[github-actions]

Claude finished @tolgahan-arikan's task —— View job

---

PR Review: Align public error contracts

• Read OmsSdkError.swift (WebRPC/WaaS mapping)
• Read IndexerClient.swift (error mapping)
• Validate Codex P2 findings
• Review transaction boundary + demo error handling
• Post review

I reviewed the latest commit (c8bc8ba, "address error contract review feedback") against the focus areas. This is a clean, well-tested change and the two Codex P2 findings appear to have already been resolved in c8bc8ba. Details below.

---

✅ Codex finding #1 — WaaS 5xx mapped as .httpError (resolved)

Codex reviewed 000d45e, where WebrpcServerPanic/WebrpcInternalError fell through to .requestFailed. In the current isHttpWebRPCError (Sources/Swift SDK/Models/OmsSdkError.swift:321-337) those kinds are now explicitly in the switch:

case .webrpcBadRoute, .webrpcBadMethod, .webrpcBadRequest,
     .webrpcBadResponse, .webrpcServerPanic, .webrpcInternalError:
    return true

So a WaaS WebRPC failure with a 4xx/5xx status now maps to .httpError with status set and retryable = status >= 500. Contract matches the docs. ✔️

✅ Codex finding #2 — local Indexer .invalidUrl misclassified (resolved)

In IndexerClient.postJson (Sources/Swift SDK/Clients/IndexerClient.swift:151-154) the catch now narrows to transport only:

} catch let error as HttpError {
    guard case .transport = error else { throw error }
    ...
}

Non-transport HttpError cases (.invalidUrl, .encodingFailed) re-throw and flow through runOmsOperation → toOmsSdkError → HttpError.toOmsSdkError, which maps them to .validationError (OmsSdkError.swift:393-399) — non-retryable and not attributed to the Indexer service. A malformed custom indexerGatewayUrl is now correctly a local validation failure. ✔️

---

✅ Security (P-256 / non-extractable credential)

No key material persisted into session storage anywhere in the diff. Session/expiry clearing continues to delegate to the Keychain-backed signer, and OIDC redirect state stays separate from completed session state. No regressions.

✅ No floating-point for token amounts

enrichFeeOptionsWithBalances zero-balance fallback uses the string "0" (WalletOperations.swift:357) and amounts go through formatTokenAmount/formatUnits. No float math.

✅ Platform availability

New public types (OmsSdkError, OmsSdkErrorCode, OmsSdkOperation, OmsUpstreamService, OmsUpstreamError) are pure value types; the internal runOmsOperation helpers carry @available(macOS 12.0, iOS 15.0, *) (OmsSdkError.swift:153,167). Consistent.

✅ Transaction write boundaries

Execute failure → .transactionExecutionUnconfirmed, retryable: false, preserves txnId (WalletOperations.swift:240-249). Post-submit polling failure → .transactionStatusLookupFailed, retryable: true, preserves txnId (:403-412). isNestedTransactionBoundary (OmsSdkError.swift:417-419) prevents the outer wrapper from overwriting these. Correct.

✅ Demo error handling

AppError.swift surfaces the new contract fields (operation, code, status, retryable, upstream) and tri-states retryable via retryableDescription (:108-117), matching the nullable contract. Good alignment with the demo-app guidance in AGENTS.md.

---

Minor / optional (non-blocking)

1. Parity-only enum cases. OmsSdkErrorCode.walletSelectionInFlight (OmsSdkError.swift:12) and OmsSdkOperation.pendingWalletSelection (:19) are declared but not thrown in SDK code. If they exist for Kotlin/TS parity, a one-line note in docs/error-contracts.md would stop a future reader from treating them as dead code. (Carried over from the prior review — still worth a quick confirmation of intent.)

2. Indexer retryable symmetry. validateSuccessResponse uses retryable: response.statusCode >= 500 (IndexerClient.swift:244), matching the WaaS HTTP path. Indexer transport errors are hard-coded retryable: true (:160). Consistent and reasonable; just noting the two intentionally differ for the non-status transport case.

3. String(describing: type(of: error)) for upstream name (IndexerClient.swift:256,263) yields a Swift-runtime-derived name (e.g. NSURLError) rather than a stable code. Docs already acknowledge this as "service-specific"; fine as-is.

---

⚠️ Could not run build/tests

swift build / swift test aren't available in this review environment, so I couldn't independently verify compilation. CI (ci.yml) gates this PR — please ensure the green run reflects c8bc8ba.

---

Summary: Both Codex P2 findings are already addressed in the latest commit. No security, floating-point, availability, or contract-correctness issues found — only optional polish (clarify the parity-only enum cases). Docs (API.md, README.md, TESTING.md, docs/error-contracts.md) stay in sync with the implementation. Nice work. ✅
• align-error-contracts