Repository navigation
Conversation
Repo Avatar
…goLabs#246) * feat(db): add resource_usage_logs table with repository functions * fix(db): add type assertions for snapshotRow and currentDayRow in repositories.ts * fix(core): replace empty RentWindowProjection interface with type alias * fix: resolve schema corruption and linting bypasses --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
…goLabs#246) * feat(db): add resource_usage_logs table with repository functions * fix(db): add type assertions for snapshotRow and currentDayRow in repositories.ts * fix(core): replace empty RentWindowProjection interface with type alias * fix: resolve schema corruption and linting bypasses --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
Cover the 'alerts test' subcommand with unit tests: - dispatcher is invoked with correct channel type, target, and event - test message delivered to webhook and Slack channels - webhook secret passed through to dispatcher - exits with code 1 on delivery failure - exits with code 1 when config ID is not found or not numeric
Cover the 'alerts test' subcommand with unit tests: - dispatcher is invoked with correct channel type, target, and event - test message delivered to webhook and Slack channels - webhook secret passed through to dispatcher - exits with code 1 on delivery failure - exits with code 1 when config ID is not found or not numeric
…Labs#289) Fix the default timeout from 10s to the specified 5 seconds and add comprehensive timeout tests that were missing: - Change TIMEOUT_MS from 10_000 to 5_000 in sendWebhookAlert - Add 'Timeout handling' test group with 3 tests: - AbortError from fetch propagates as a delivery failure - Request is aborted after exactly 5 seconds (threshold verified with fake timers: not aborted at 4999ms, aborted at 5001ms) - Request that completes within 5 seconds is never aborted Existing 17 tests continue to pass; no unhandled rejection warnings. Co-authored-by: dave <jesugbohunmi23@mail.com>
…Labs#289) Fix the default timeout from 10s to the specified 5 seconds and add comprehensive timeout tests that were missing: - Change TIMEOUT_MS from 10_000 to 5_000 in sendWebhookAlert - Add 'Timeout handling' test group with 3 tests: - AbortError from fetch propagates as a delivery failure - Request is aborted after exactly 5 seconds (threshold verified with fake timers: not aborted at 4999ms, aborted at 5001ms) - Request that completes within 5 seconds is never aborted Existing 17 tests continue to pass; no unhandled rejection warnings. Co-authored-by: dave <jesugbohunmi23@mail.com>
…rsing (TegoLabs#115) (TegoLabs#288) Complete the ExtendFootprintTTLOp transaction building pipeline: - Add `feeCharged` to `SubmitTransactionResult`: capture the actual fee paid by parsing `feeCharged` from the Stellar RPC getTransaction response inside `pollTransaction` - Add `feeCharged` to `ExtensionResult` and thread it through `extendEntries` so callers receive both the tx hash and actual fee charged Tests added in tests/rpc/client.test.ts: - simulateExtension returns minResourceFee for footprint estimation - simulateExtension propagates RPC error on failure - submitExtension calls simulateTransaction before sendTransaction (order verified) - submitExtension parses feeCharged from successful transaction result - submitExtension returns feeCharged as undefined when absent from result Tests added in tests/core/extension.test.ts: - extendEntries propagates feeCharged from submitted transaction result - simulateExtension delegates to RPC client and returns estimated fee Co-authored-by: dave <jesugbohunmi23@mail.com>
…rsing (TegoLabs#115) (TegoLabs#288) Complete the ExtendFootprintTTLOp transaction building pipeline: - Add `feeCharged` to `SubmitTransactionResult`: capture the actual fee paid by parsing `feeCharged` from the Stellar RPC getTransaction response inside `pollTransaction` - Add `feeCharged` to `ExtensionResult` and thread it through `extendEntries` so callers receive both the tx hash and actual fee charged Tests added in tests/rpc/client.test.ts: - simulateExtension returns minResourceFee for footprint estimation - simulateExtension propagates RPC error on failure - submitExtension calls simulateTransaction before sendTransaction (order verified) - submitExtension parses feeCharged from successful transaction result - submitExtension returns feeCharged as undefined when absent from result Tests added in tests/core/extension.test.ts: - extendEntries propagates feeCharged from submitted transaction result - simulateExtension delegates to RPC client and returns estimated fee Co-authored-by: dave <jesugbohunmi23@mail.com>
…egoLabs#287) Add 'sorokeep restore <contractId>' command that builds and submits a RestoreFootprintOp transaction for archived contract entries. Supports --entry <keyXdr> for specific entries or --all for all tracked entries. Resolves keypair from --keypair or --keypair-env flag. Outputs tx hash and fee spent in XLM on success. Also threads feeCharged through SubmitTransactionResult and RestoreResult so callers can surface the actual fee charged by the network. Co-authored-by: dave <jesugbohunmi23@mail.com> Co-authored-by: CL999 <114596864+AbdulmalikAlayande@users.noreply.github.com>
…egoLabs#287) Add 'sorokeep restore <contractId>' command that builds and submits a RestoreFootprintOp transaction for archived contract entries. Supports --entry <keyXdr> for specific entries or --all for all tracked entries. Resolves keypair from --keypair or --keypair-env flag. Outputs tx hash and fee spent in XLM on success. Also threads feeCharged through SubmitTransactionResult and RestoreResult so callers can surface the actual fee charged by the network. Co-authored-by: dave <jesugbohunmi23@mail.com> Co-authored-by: CL999 <114596864+AbdulmalikAlayande@users.noreply.github.com>
Replace all 46 `as any` casts with properly typed local interfaces (SimCost, SimSuccessWithCost, SendResultWithDetails, TxResponseWithMeta, ServerWithExtensions, HealthResponse, FeeStatsRpcResponse, FeePercentiles). Change `catch (error: any)` to `error: unknown` with type narrowing. Remove unused FeeBumpTransaction import. Closes TegoLabs#360
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe RPC client replaces broad ChangesRPC client typing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)src/rpc/client.tsFile contains syntax errors that prevent linting: Line 390: Expected a catch clause but instead found 'async'.; Line 390: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 390: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 425: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 425: expected ... [truncated 8361 characters] ... e of reserved keyword Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…r PR CI Every open PR's CI Pipeline was failing at the "Audit Dependencies" step on the exact same findings, since actions/checkout defaults to the merge-with-main ref for pull_request events - meaning every PR inherited main's vulnerable dependency tree regardless of its own diff. - Bump @stellar/stellar-sdk ^16.0.1 -> ^16.2.0, which resolves to a patched axios (>=1.18.0), fixing the high-severity DoS/prototype- pollution advisories in the previously-resolved 1.16.1. - npm audit fix for @hono/node-server, fast-uri, and postcss. - Scope the CI audit step to --omit=dev: the one remaining high-severity finding (brace-expansion, via @vitest/coverage-v8's glob chain) only exists in devDependencies, is never shipped to npm install'ers, and fixing it requires a vitest 3->4 major bump that needs its own deliberate verification, not a blind force-fix to unblock CI. Verified locally: lint, tsc --noEmit, full test suite (995/995), build, and npm audit --omit=dev all clean before pushing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/rpc/client.ts`:
- Around line 63-70: The ServerWithExtensions type is redundantly redeclaring
SDK RPC methods and uses incompatible response assumptions. Remove the extension
type and use rpc.Server directly with its non-optional getLatestLedger and
getFeeStats methods and SDK response types; update related calls to remove
defensive optional-method branches. In SendResultWithDetails and every status
=== "ERROR" diagnostic path, replace diagnosticEventsXdr with diagnosticEvents?:
xdr.DiagnosticEvent[] (or unknown[] only if required), consuming the parsed
sendTransaction response field so diagnostics are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50d601c5-68ea-41d5-a51c-9bac8b98fb33
📒 Files selected for processing (1)
src/rpc/client.ts
📜 Review details
🔇 Additional comments (13)
src/rpc/client.ts (13)
314-340: 🎯 Functional CorrectnessSame
getLatestLedger/getFeeStatsoptionality concern as the type definitions.This is the usage site for the
ServerWithExtensionstyping issue flagged on lines 63-70/104-119 — thetypeof serverExt.getLatestLedger === "function"/getFeeStats !== "function"guards are defensive against methods that are actually always present and typed onrpc.Server.
687-714: 🎯 Functional CorrectnessDiagnostics fallback issue inherited from
SendResultWithDetails.
errDetails.diagnosticEventsXdrat Line 689/696 is covered by the type-mismatch flagged on Lines 104-119; the rest of this typed-cast refactor (cpuInsns/memBytes extraction,addResourcesToSuccess) looks correct.
775-798: 🎯 Functional CorrectnessSame diagnostics fallback issue as
submitExtension.Lines 778/785 use
feeBumpErrDetails.diagnosticEventsXdr, subject to the same field mismatch flagged on Lines 104-119.
849-872: 🎯 Functional CorrectnessSame diagnostics fallback issue as
submitExtension/submitExtensionWithFeeBump.Lines 852/859 use
restErrDetails.diagnosticEventsXdr, subject to the same field mismatch flagged on Lines 104-119.
10-12: LGTM on theFeeBumpTransactionimport removal.No remaining reference to the
FeeBumpTransactionsymbol is visible in the reviewed code;TransactionBuilder.buildFeeBumpTransaction(...)is used instead, so the type isn't needed directly.
43-47: LGTM!
91-102: 🎯 Functional CorrectnessVerify whether
costis already a typed field onSimulateTransactionSuccessResponse.
SimSuccessWithCostre-declarescost(cpuInsns/memBytes/readBytes/writeBytes) as an extension on top ofrpc.Api.SimulateTransactionSuccessResponse. If the installed SDK version already types thiscostfield, this duplicates existing typing rather than reusing it, which runs counter to the PR's goal of preferring "existing SDK types" over local shapes. Worth confirming against the pinned@stellar/stellar-sdk@16.0.1.d.tsbefore deciding whether to keep this extension type.
262-264: LGTM!
589-598: LGTM!
750-750: LGTM!
909-918: LGTM!
928-940: LGTM!
970-1018: 🎯 Functional CorrectnessNo change needed:
ledgeris present on parsed SUCCESS/FAILED responses.
@stellar/stellar-sdk16.0.1 definesGetSuccessfulTransactionResponse.ledgerandGetFailedTransactionResponse.ledger, so these branches use the transaction’s inclusion ledger rather than falling back to the current tip.
| // ── Extended RPC response shapes (fields not yet in @stellar/stellar-sdk types) ─ | ||
|
|
||
| /** Server with optional extension methods not yet typed by the SDK. */ | ||
| type ServerWithExtensions = rpc.Server & { | ||
| getLatestLedger?: () => Promise<{ sequence: number }>; | ||
| getFeeStats?: () => Promise<FeeStatsRpcResponse>; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
@stellar/stellar-sdk 16 rpc.Server sendTransaction SendTransactionResponse diagnosticEvents type definition
💡 Result:
In the @stellar/stellar-sdk (version 16), the diagnosticEvents property within the SendTransactionResponse interface is defined as an array of xdr.DiagnosticEvent objects [1][2]. Specifically, the response structure for the sendTransaction method is as follows: 1. Raw Response (RawSendTransactionResponse): When interacting with the RPC server, the raw response includes a diagnosticEventsXdr field, which is an optional array of base64-encoded strings (string[]) representing the serialized XDR diagnostic events [1][2]. 2. Parsed Response (SendTransactionResponse): The SDK automatically parses these raw XDR strings into structured objects. In the parsed SendTransactionResponse interface, the diagnosticEvents property is defined as: diagnosticEvents?: xdr.DiagnosticEvent[] [1][2] This property is optional and is typically only populated when the transaction status is "ERROR" and diagnostic events have been enabled on the RPC server [1][2][3]. The SDK's internal parser (parseRawSendTransaction) performs this conversion using xdr.DiagnosticEvent.fromXDR(evt, "base64") [4]. Diagnostic events themselves provide debugging information, such as logs from contract execution (e.g., explaining why a contract trapped or panicked), and are unmetered [5].
Citations:
- 1: https://github.com/stellar/js-stellar-sdk/blob/main/src/rpc/api.ts
- 2: https://cdn.jsdelivr.net/npm/@stellar/stellar-sdk@16.1.0/lib/esm/rpc/api.d.ts
- 3: https://deepwiki.com/stellar/js-stellar-sdk/4.2-rpc-methods-reference
- 4: https://github.com/stellar/js-stellar-sdk/blob/main/src/rpc/parsers.ts
- 5: https://developers.stellar.org/docs/tools/quickstart/debugging/diagnostic-events
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- repo files around client.ts ---\n'
git ls-files | rg '(^|/)client\.ts$|package\.json$|package-lock\.json$|pnpm-lock\.yaml$|yarn\.lock$|bun\.lockb?$' || true
printf '\n--- client.ts relevant sections ---\n'
sed -n '1,140p' src/rpc/client.ts
printf '\n--- lines 280-350 ---\n'
sed -n '280,350p' src/rpc/client.ts
printf '\n--- lines 660-705 ---\n'
sed -n '660,705p' src/rpc/client.ts
printf '\n--- lines 755-800 ---\n'
sed -n '755,800p' src/rpc/client.ts
printf '\n--- lines 835-870 ---\n'
sed -n '835,870p' src/rpc/client.ts
printf '\n--- package versions ---\n'
if [ -f package.json ]; then cat package.json; fi
if [ -f pnpm-lock.yaml ]; then rg -n "`@stellar/stellar-sdk`" pnpm-lock.yaml -A2 -B2 | sed -n '1,80p'; fi
if [ -f package-lock.json ]; then node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package-lock.json','utf8'));
for (const k of ['dependencies.@stellar/stellar-sdk','devDependencies.@stellar/stellar-sdk', ...Object.keys(p.packages || {}).filter(k => k.includes('node_modules/@stellar/stellar-sdk'))]) {
console.log(k, p.dependencies?.[k]?.version || p.devDependencies?.[k]?.version || p.packages?.[k]?.version);
}
JS
fiRepository: AbdulmalikAlayande/sorokeep
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repo files around client.ts ---'
git ls-files | grep -E '(^|/)client\.ts$|(^|/)package\.json$|(^|/)package-lock\.json$|(^|/)pnpm-lock\.yaml$|(^|/)yarn\.lock$|(^|/)bun\.lockb?$' || true
printf '%s\n' '--- client.ts relevant sections ---'
sed -n '1,140p' src/rpc/client.ts
printf '%s\n' '--- lines 280-350 ---'
sed -n '280,350p' src/rpc/client.ts
printf '%s\n' '--- lines 660-705 ---'
sed -n '660,705p' src/rpc/client.ts
printf '%s\n' '--- lines 755-800 ---'
sed -n '755,800p' src/rpc/client.ts
printf '%s\n' '--- lines 835-870 ---'
sed -n '835,870p' src/rpc/client.ts
printf '%s\n' '--- package versions ---'
if [ -f package.json ]; then cat package.json; fi
if [ -f pnpm-lock.yaml ]; then grep -E '`@stellar/stellar-sdk`' pnpm-lock.yaml -A2 -B2 | sed -n '1,120p'; fi
if [ -f package-lock.json ]; then node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package-lock.json','utf8'));
const keys = Object.keys(p.packages || {})
.filter(k => k.includes('node_modules/@stellar/stellar-sdk')) || [];
try {
console.log(p.dependencies?.['`@stellar/stellar-sdk`']?.version || p.devDependencies?.['`@stellar/stellar-sdk`']?.version);
} catch {}
for (const k of keys) {
console.log(k, p.packages[k]?.version);
}
JS
fiRepository: AbdulmalikAlayande/sorokeep
Length of output: 17782
🌐 Web query:
@stellar/stellar-sdk 16.0.1 rpc.Server getLatestLedger getFeeStats type definition
💡 Result:
In @stellar/stellar-sdk version 16.0.1, the rpc.Server class provides methods to interact with the Soroban RPC API, with type definitions managed through the Api namespace [1][2]. getLatestLedger This method fetches metadata about the latest ledger from the network to which the RPC server is connected [1][2]. Definition: getLatestLedger: Promise<Api.GetLatestLedgerResponse> [1][2] Response Type (Api.GetLatestLedgerResponse) [3]: - id: string (The ledger hash) - sequence: number (The ledger sequence number) - protocolVersion: string (The protocol version associated with the ledger) getFeeStats This method provides an analysis of recent fee statistics for regular and smart contract operations [1][2]. Definition: getFeeStats: Promise<Api.GetFeeStatsResponse> [1][2] The specific fields within Api.GetFeeStatsResponse are typically defined in the SDK's api.d.ts file, which maps to the Soroban RPC getFeeStats response format (which includes fields such as min/max/p10/p50/p90/p95 fee distributions for different operation types) [4][1]. You can reference the official Stellar documentation for the underlying RPC method schemas, which these SDK methods implement: - getLatestLedger: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getLatestLedger [5] - getFeeStats: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getFeeStats [4]
Citations:
- 1: https://cdn.jsdelivr.net/npm/@stellar/stellar-sdk@16.1.0/lib/axios/esm/rpc/server.d.ts
- 2: https://cdn.jsdelivr.net/npm/@stellar/stellar-sdk@16.1.0/lib/esm/rpc/server.d.ts
- 3: https://cdn.jsdelivr.net/npm/stellar-sdk@13.1.0/lib/rpc/api.d.ts
- 4: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getFeeStats
- 5: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getLatestLedger
Don’t redeclare existing SDK RPC shapes as optional extensions.
rpc.Server in @stellar/stellar-sdk already exposes non-optional getLatestLedger(): Promise<rpc.Api.GetLatestLedgerResponse> and getFeeStats(): Promise<rpc.Api.GetFeeStatsResponse>, so ServerWithExtensions creates dead defensive branches and sidesteps the SDK’s return types. Also, sendTransaction() parses diagnostic events into SendTransactionResponse.diagnosticEvents: xdr.DiagnosticEvent[]; SendResultWithDetails.diagnosticEventsXdr will be undefined because the parsed public response does not expose raw base64, so the fallback can miss diagnostics at all status === "ERROR" call sites. Use the SDK types directly and rename/retype the diagnostic field to diagnosticEvents?: xdr.DiagnosticEvent[] or unknown[] if the type still needs to be less specific.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/rpc/client.ts` around lines 63 - 70, The ServerWithExtensions type is
redundantly redeclaring SDK RPC methods and uses incompatible response
assumptions. Remove the extension type and use rpc.Server directly with its
non-optional getLatestLedger and getFeeStats methods and SDK response types;
update related calls to remove defensive optional-method branches. In
SendResultWithDetails and every status === "ERROR" diagnostic path, replace
diagnosticEventsXdr with diagnosticEvents?: xdr.DiagnosticEvent[] (or unknown[]
only if required), consuming the parsed sendTransaction response field so
diagnostics are preserved.
Closes TegoLabs#314 - Add src/alerts/opsgenie.ts with OpsgenieChannel class and sendOpsgenieAlert function following the AlertChannel interface - POST to /v2/alerts with GenieKey authorization header for threshold_crossed, resource_alert, and state_changed events - Map alert_resolved to POST /v2/alerts/{alias}/close endpoint - buildAlias() mirrors pagerduty.ts buildDedupKey() exactly so repeated threshold crossings for the same entry deduplicate - Validate API key before any network call with a clear error message - Use AbortController/timeout pattern consistent with other channels - Register in builtins.ts with targetOption: routingKey - Add 32 tests in tests/alerts/opsgenie.test.ts (TDD, written first) - Update tests/alerts/builtins.test.ts for the new 6-channel count
- Add ?identifierType=alias to the close URL so Opsgenie looks up the alert by alias rather than ID (critical: without this, close silently finds nothing since the default identifierType is 'id') - Surface Opsgenie response body in error messages for faster debugging - Test that identifierType=alias is present in the close URL - Test that error body message is included in the thrown error - Test AbortController timeout fires after 10 seconds - Assert opsgenie missingTargetError wording in builtins test
Ensures vi.useRealTimers() is always called even if the test assertion fails, preventing timer pollution affecting subsequent tests.
Distinguish AbortError (our own 10-second timeout) from other fetch failures by catching it explicitly and rethrowing with a message that names the configured timeout duration: 'Opsgenie API request timed out after 10 seconds' Other network errors (ECONNREFUSED etc.) are rethrown unchanged. Update the timeout test assertion to match the new message.
- Store trimmed API key in constructor so leading/trailing whitespace is not forwarded in the GenieKey Authorization header - Simplify 'sets priority P3 for info severity' test: remove the redundant alert_resolved call and mock reset; test directly with a resource event at info severity
…egoLabs#550) Small maintainer fixup on top of TegoLabs#550's Opsgenie channel: the timeout error was thrown without a cause chain, tripping preserve-caught-error.
… rebuild (fixup for TegoLabs#528) migrateAlertConfigsChannelTypeCheck's and relaxChannelTypeChecks's table-rebuild INSERT/SELECT lists didn't include quiet_hours_start/end/timezone, so any existing database still carrying the legacy channel_type CHECK enum would silently drop configured quiet-hours windows the first time it opened after upgrading. Caught by CodeRabbit on TegoLabs#528; fixing directly since it touches the same migration functions from earlier this session.
…e.test.ts (TegoLabs#537) Verified locally: lint, typecheck, full suite (1039/1039), build, and audit all clean. Closes TegoLabs#367.
….test.ts (TegoLabs#575) Verified locally: lint, typecheck, full suite (1039/1039), build, and audit all clean. Closes TegoLabs#368.
… tests (TegoLabs#553) Type-only changes, no behavior change. Verified locally: lint, typecheck, full suite (1039/1039), build, and audit all clean. Closes TegoLabs#362.
…imulation_cache.test (TegoLabs#551) Relocates src/alerts/keys.test.ts (misplaced outside the vitest.config.ts include glob) to tests/alerts/keys.test.ts, and deletes the orphaned src/alerts/simulation_cache.test.ts (a self-contained duplicate reimplementation of a SimulationCacheManager, never wired to the real module). Verified locally: lint, typecheck, full suite (1042/1042), build, and audit all clean. Closes TegoLabs#357, TegoLabs#359.
Every `any` cast replaced with a precisely-named interface (ErrorLike, SorobanMetaLike, SimulateWithCost, SendTransactionErrorResult, GetTransactionRawFields); reviewed the full diff line-by-line, every runtime property access/behavior is preserved exactly. Verified locally: lint, typecheck, full suite (1042/1042, including all 66 rpc tests), build, and audit all clean. Closes TegoLabs#360.
…med to scope) Adds README.pt.md and a language-switcher link in README.md, per TegoLabs#476. Trimmed from the original PR before merging: - src/core/costs.ts / tests/core/costs.test.ts (an unrelated fee-trend cost-forecasting feature — that's issue TegoLabs#498's scope, not TegoLabs#476's) - README/README.pt.md badge blocks and tests/docs/readme-badges.test.ts (badges are issue TegoLabs#488's scope; the URLs also pointed at the contributor's own fork - github.com/OlaBakare/sorokeep - not this repo, which would have shown broken/wrong CI and license badges on the real README) The translation itself (README.pt.md, tests/docs/readme-portuguese.test.ts) is unchanged and verified: 56/56 tests pass.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rpc/client.ts (1)
757-785: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the partially retained pre-refactor branches.
The file contains both old and replacement statements, causing duplicate declarations/object keys and malformed method scope; Biome cannot parse the file, so lint and tests cannot run.
src/rpc/client.ts#L757-L785: keep one diagnostics/cost implementation and one set of returned resource properties.src/rpc/client.ts#L849-L878: remove the obsolete fee-bump retry branch; send once and poll the surviving retry result.src/rpc/client.ts#L920-L959: apply the same cleanup to the restore retry branch.src/rpc/client.ts#L1017-L1026: retain oneisBadSeqErrorsignature and one ternary fallback.src/rpc/client.ts#L1070-L1110: retain one metadata extraction path and oneledgerproperty per return object.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/rpc/client.ts` around lines 757 - 785, Remove the partially retained pre-refactor code in src/rpc/client.ts: at lines 757-785 keep one diagnostics/cost implementation and one set of resource properties; at lines 849-878 remove the obsolete fee-bump retry branch and send once before polling the surviving retry result; at lines 920-959 apply the same cleanup to the restore retry flow; at lines 1017-1026 retain one isBadSeqError signature and ternary fallback; and at lines 1070-1110 retain one metadata extraction path and one ledger property per return object. Ensure the surrounding addResourcesToSuccess method remains syntactically valid.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/rpc/client.ts`:
- Around line 757-785: Remove the partially retained pre-refactor code in
src/rpc/client.ts: at lines 757-785 keep one diagnostics/cost implementation and
one set of resource properties; at lines 849-878 remove the obsolete fee-bump
retry branch and send once before polling the surviving retry result; at lines
920-959 apply the same cleanup to the restore retry flow; at lines 1017-1026
retain one isBadSeqError signature and ternary fallback; and at lines 1070-1110
retain one metadata extraction path and one ledger property per return object.
Ensure the surrounding addResourcesToSuccess method remains syntactically valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d23f8eb-7235-44ef-b3fb-99900b9080dc
📒 Files selected for processing (1)
src/rpc/client.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: chore(lint): eliminate no-explicit-any warnings in src/rpc/client.ts
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
GitHub Actions: CI Pipeline / build-and-test (22.x): chore(lint): eliminate no-explicit-any warnings in src/rpc/client.ts
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
🧰 Additional context used
🪛 Biome (2.5.5)
src/rpc/client.ts
[error] 770-770: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 771-771: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 785-785: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 785-785: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 785-785: expected , but instead found :
(parse)
[error] 785-785: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 854-854: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 859-859: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 869-869: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 870-870: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 935-935: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 940-940: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 950-950: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 951-951: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 1018-1018: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 1018-1018: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 1018-1018: expected , but instead found :
(parse)
[error] 1018-1018: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 1026-1026: Expected a statement but instead found ': (errorResult as xdr.TransactionResult)'.
(parse)
[error] 1096-1096: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt
src/rpc/client.ts
[error] 390-390: ESLint parsing error: 'catch' or 'finally' expected.
🪛 GitHub Actions: CI Pipeline / build-and-test (22.x)
src/rpc/client.ts
[error] 390-390: ESLint Parsing error: 'catch' or 'finally' expected
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
43ad363 to
8692701
Compare
|
Thanks for taking this on, but closing without merging: two problems. First, src/rpc/client.ts already has zero no-explicit-any warnings on current main ( |
18 participants
Closes #360
Replace all 46
as anycasts with properly typed local interfaces (SimCost, SimSuccessWithCost, SendResultWithDetails, TxResponseWithMeta, ServerWithExtensions, HealthResponse, FeeStatsRpcResponse, FeePercentiles).Change
catch (error: any)toerror: unknownwith type narrowing. Remove unused FeeBumpTransaction import.What does this PR do?
Why?
Does this touch secret-key handling or transaction submission?
Checklist
npm test)npx tsc --noEmit)npm run lint)console.login core logic