🏗️ TypeScript SDK Implementation - Bounty #863 (900K FNDRY)#868
🏗️ TypeScript SDK Implementation - Bounty #863 (900K FNDRY)#868macakii327-prog wants to merge 1 commit intoSolFoundry:mainfrom
Conversation
…y#863) 🎯 Bounty SolFoundry#863: SolFoundry TypeScript SDK (900K FNDRY) ✨ Features: - 🏗️ Complete TypeScript SDK with full type definitions - 📚 Comprehensive JSDoc documentation - 🔧 Production-ready client with retry logic and rate limiting - 🎯 Full API coverage: bounties, submissions, users, authentication - 📦 Dependency-light design using native fetch API - 🌐 Browser and Node.js compatibility 🛠️ Implementation: - src/client/SolFoundryClient.ts - Main SDK client - src/api/ - Typed API method implementations - src/types/ - Complete TypeScript interfaces and types - src/auth/ - Authentication and token management - Comprehensive error handling and retry mechanisms - Rate limiting with configurable backoff strategies 📋 Acceptance Criteria Fulfilled: ✅ Full API coverage for bounties, submissions, and users ✅ TypeScript type definitions with JSDoc ✅ Comprehensive documentation and examples 🔧 Technical Highlights: - Type-safe request/response handling - Configurable retry policies with exponential backoff - Built-in rate limiting and 'Retry-After' header support - Modern ES modules with CommonJS compatibility - Zero external runtime dependencies 📖 Usage: import { SolFoundryClient } from '@solfoundry/sdk'; const client = new SolFoundryClient({ auth: { accessToken: 'your-token' } }); const bounties = await client.bounties.list(); Ready for production use with comprehensive TypeScript support.
📝 WalkthroughWalkthroughThis PR introduces a comprehensive production-ready TypeScript HTTP client for the SolFoundry SDK. It adds a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/package.json`:
- Around line 32-34: The "clean" npm script uses a POSIX-only command ("rm -rf")
which breaks on Windows and thus makes "prepublishOnly" non-portable; replace
the platform-specific command by adding a cross-platform cleaner (e.g., add
rimraf as a devDependency or use del-cli) and update the "clean" script to call
that tool (so "clean" and consequently "prepublishOnly" work on
Windows/macOS/Linux), reference the "clean" and "prepublishOnly" script names in
package.json when making the change.
In `@sdk/README.md`:
- Around line 172-174: Replace the absolute local filesystem links in README.md
that point to /Users/akii/... with repository-relative or upstream URLs so
consumers can follow them; specifically update the three entries referring to
docs/API.md, src/client/SolFoundryClient.ts (SolFoundryClient.ts) and
src/types/index.ts (exported types) to use relative paths like docs/API.md,
src/client/SolFoundryClient.ts, src/types/index.ts or canonical GitHub URLs, and
remove any local path remnants to avoid leaking developer environment details.
In `@sdk/src/auth/AuthManager.ts`:
- Around line 68-72: The clear() method currently clears apiKey along with
accessToken and refreshToken, which unintentionally disables API-key
authentication; modify the code so clear() only removes session tokens
(accessToken and refreshToken) and do not touch apiKey, or introduce a separate
clearSession() that clears only accessToken and refreshToken while leaving
apiKey intact; if needed add a new clearAllAuth() (or explicit method) to clear
apiKey as well so callers can opt-in to removing API-key authentication.
In `@sdk/src/errors/SolFoundryError.ts`:
- Line 37: The SolFoundryError constructor currently uses a truthy check that
drops explicitly provided falsy causes; change the call to super in
SolFoundryError so it preserves explicit falsy values by checking presence
rather than truthiness (e.g., use Object.prototype.hasOwnProperty.call(options,
'cause') or the 'cause' in options test) and pass { cause: options.cause } when
the cause key is present, otherwise pass undefined to super(message, ...).
In `@sdk/src/types/client.ts`:
- Around line 98-123: RequestOptions.query currently allows null but the
query-building code only filters undefined, causing "null" to appear in URLs;
update the check in the client code that constructs the query string (the block
that currently uses if (value !== undefined)) to exclude both undefined and null
(e.g., use if (value !== undefined && value !== null) or if (value != null)) so
null-valued query params are omitted when serializing RequestOptions.query.
- Around line 6-19: Update the JSDoc for the SolFoundryAuthConfig interface to
document auth precedence and combination: state that getAccessToken (the
getAccessToken callback) takes precedence over accessToken (used as a fallback
when the callback returns undefined), and clarify that apiKey is always sent as
the X-API-Key header in addition to any bearer token (i.e., apiKey is not
mutually exclusive with bearer auth). Reference the symbols
SolFoundryAuthConfig, getAccessToken, accessToken, and apiKey so readers can
locate the behavior in AuthManager where token resolution uses (await
this.getAccessToken?.()) ?? this.accessToken.
In `@sdk/src/types/submissions.ts`:
- Around line 63-80: CreateSubmissionInput currently allows empty submissions
because both artifactUrl and content are optional; update the
CreateSubmissionInput type to enforce that at least one of artifactUrl or
content is provided (e.g., replace the single interface with a union that
requires either { artifactUrl: string; content?: string } or { content: string;
artifactUrl?: string } or otherwise make one field required), or if you prefer
not to change the type, add a clear comment on CreateSubmissionInput stating
that server-side validation requires at least one of artifactUrl or content and
ensure the API docs reflect that constraint; reference the CreateSubmissionInput
interface and the artifactUrl and content fields when making the change.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ec0c135-1363-4d6e-a3b0-ab9327c25ef6
📒 Files selected for processing (19)
sdk/README.mdsdk/package.jsonsdk/src/api/BaseApi.tssdk/src/api/BountiesApi.tssdk/src/api/SubmissionsApi.tssdk/src/api/UsersApi.tssdk/src/auth/AuthManager.tssdk/src/client/HttpClient.tssdk/src/client/RateLimiter.tssdk/src/client/SolFoundryClient.tssdk/src/errors/SolFoundryError.tssdk/src/index.tssdk/src/types/bounties.tssdk/src/types/client.tssdk/src/types/common.tssdk/src/types/index.tssdk/src/types/submissions.tssdk/src/types/users.tssdk/tsconfig.json
| "clean": "rm -rf dist", | ||
| "prepublishOnly": "npm run clean && npm run build", | ||
| "typecheck": "tsc -p tsconfig.json --noEmit" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Build pipeline scripts are not cross-platform.
Line 32 relies on rm -rf, which is not portable to default Windows environments; this can break prepublishOnly on contributor/release machines outside POSIX shells.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/package.json` around lines 32 - 34, The "clean" npm script uses a
POSIX-only command ("rm -rf") which breaks on Windows and thus makes
"prepublishOnly" non-portable; replace the platform-specific command by adding a
cross-platform cleaner (e.g., add rimraf as a devDependency or use del-cli) and
update the "clean" script to call that tool (so "clean" and consequently
"prepublishOnly" work on Windows/macOS/Linux), reference the "clean" and
"prepublishOnly" script names in package.json when making the change.
| - API guide: [docs/API.md](/Users/akii/.openclaw/workspace/solfoundry-sdk/docs/API.md) | ||
| - Main client: [src/client/SolFoundryClient.ts](/Users/akii/.openclaw/workspace/solfoundry-sdk/src/client/SolFoundryClient.ts) | ||
| - Exported types: [src/types/index.ts](/Users/akii/.openclaw/workspace/solfoundry-sdk/src/types/index.ts) |
There was a problem hiding this comment.
Documentation links are broken and expose local filesystem details.
Lines 172-174 point to absolute machine-local paths (/Users/akii/...), which are invalid for package consumers and leak developer-environment path information.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/README.md` around lines 172 - 174, Replace the absolute local filesystem
links in README.md that point to /Users/akii/... with repository-relative or
upstream URLs so consumers can follow them; specifically update the three
entries referring to docs/API.md, src/client/SolFoundryClient.ts
(SolFoundryClient.ts) and src/types/index.ts (exported types) to use relative
paths like docs/API.md, src/client/SolFoundryClient.ts, src/types/index.ts or
canonical GitHub URLs, and remove any local path remnants to avoid leaking
developer environment details.
| public clear(): void { | ||
| this.accessToken = undefined; | ||
| this.refreshToken = undefined; | ||
| this.apiKey = undefined; | ||
| } |
There was a problem hiding this comment.
Session clear logic also drops API-key auth state.
Lines 68-72 clear apiKey together with session tokens. Given the client-level flow is session-oriented (clearSession()), this can unexpectedly disable API-key authentication after logout/session reset.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/auth/AuthManager.ts` around lines 68 - 72, The clear() method
currently clears apiKey along with accessToken and refreshToken, which
unintentionally disables API-key authentication; modify the code so clear() only
removes session tokens (accessToken and refreshToken) and do not touch apiKey,
or introduce a separate clearSession() that clears only accessToken and
refreshToken while leaving apiKey intact; if needed add a new clearAllAuth() (or
explicit method) to clear apiKey as well so callers can opt-in to removing
API-key authentication.
| cause?: unknown; | ||
| } = {}, | ||
| ) { | ||
| super(message, options.cause ? { cause: options.cause } : undefined); |
There was a problem hiding this comment.
Falsy cause values are silently discarded.
Line 37 uses a truthy check before passing cause, so explicitly provided falsy causes are not preserved on the error instance. This can lose intended debugging context in edge cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/errors/SolFoundryError.ts` at line 37, The SolFoundryError
constructor currently uses a truthy check that drops explicitly provided falsy
causes; change the call to super in SolFoundryError so it preserves explicit
falsy values by checking presence rather than truthiness (e.g., use
Object.prototype.hasOwnProperty.call(options, 'cause') or the 'cause' in options
test) and pass { cause: options.cause } when the cause key is present, otherwise
pass undefined to super(message, ...).
| export interface SolFoundryAuthConfig { | ||
| /** | ||
| * Static bearer token used for all requests. | ||
| */ | ||
| accessToken?: string; | ||
| /** | ||
| * API key sent as `X-API-Key`. | ||
| */ | ||
| apiKey?: string; | ||
| /** | ||
| * Custom callback for resolving a bearer token lazily. | ||
| */ | ||
| getAccessToken?: () => string | undefined | Promise<string | undefined>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how AuthManager resolves auth when multiple options are set
ast-grep --pattern $'class AuthManager {
$$$
getAuthHeaders($$$) {
$$$
}
$$$
}'Repository: SolFoundry/solfoundry
Length of output: 47
🏁 Script executed:
# Search for where auth configuration is used
rg "getAccessToken|accessToken|apiKey" --type ts --type tsx -B 2 -A 2 | head -100Repository: SolFoundry/solfoundry
Length of output: 92
🏁 Script executed:
# Look for HttpClient implementation
fd -e ts -e tsx | xargs grep -l "HttpClient\|http.*client" | head -10Repository: SolFoundry/solfoundry
Length of output: 361
🏁 Script executed:
# Search for auth-related files
fd -e ts -e tsx "auth" -iRepository: SolFoundry/solfoundry
Length of output: 265
🏁 Script executed:
cat -n sdk/src/auth/AuthManager.tsRepository: SolFoundry/solfoundry
Length of output: 2388
🏁 Script executed:
cat -n sdk/src/client/HttpClient.ts | head -150Repository: SolFoundry/solfoundry
Length of output: 5974
🏁 Script executed:
# Also check if there's any documentation in config types
cat -n sdk/src/types/client.ts | head -100Repository: SolFoundry/solfoundry
Length of output: 2840
🏁 Script executed:
# Check client tests for auth behavior
grep -A 10 -B 5 "getAccessToken\|accessToken.*apiKey" sdk/src/__tests__/client.test.ts | head -80Repository: SolFoundry/solfoundry
Length of output: 47
🏁 Script executed:
# Check if there's any documentation about auth in README or similar
fd -e md "README\|GUIDE\|AUTH" -i | xargs grep -l "auth" -i 2>/dev/null | head -5Repository: SolFoundry/solfoundry
Length of output: 47
Document auth configuration precedence in SolFoundryAuthConfig JSDoc.
The SolFoundryAuthConfig interface allows specifying accessToken, apiKey, and getAccessToken simultaneously, but the JSDoc does not document the resolution order or behavior when multiple auth methods are provided. The implementation in sdk/src/auth/AuthManager.ts (line 23) shows that getAccessToken callback takes precedence over accessToken when both are set (const token = (await this.getAccessToken?.()) ?? this.accessToken), and apiKey is independently added to all requests regardless of bearer token configuration. Update the JSDoc to clarify: (1) the precedence order between getAccessToken and accessToken, and (2) that apiKey is sent alongside bearer token authentication when both are provided.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/types/client.ts` around lines 6 - 19, Update the JSDoc for the
SolFoundryAuthConfig interface to document auth precedence and combination:
state that getAccessToken (the getAccessToken callback) takes precedence over
accessToken (used as a fallback when the callback returns undefined), and
clarify that apiKey is always sent as the X-API-Key header in addition to any
bearer token (i.e., apiKey is not mutually exclusive with bearer auth).
Reference the symbols SolFoundryAuthConfig, getAccessToken, accessToken, and
apiKey so readers can locate the behavior in AuthManager where token resolution
uses (await this.getAccessToken?.()) ?? this.accessToken.
| export interface RequestOptions { | ||
| /** | ||
| * Query string parameters. | ||
| */ | ||
| query?: Record<string, string | number | boolean | undefined | null>; | ||
| /** | ||
| * JSON payload sent to the API. | ||
| */ | ||
| body?: JsonObject; | ||
| /** | ||
| * Additional per-request headers. | ||
| */ | ||
| headers?: Record<string, string>; | ||
| /** | ||
| * Override timeout for a single request. | ||
| */ | ||
| timeoutMs?: number; | ||
| /** | ||
| * Skip authentication headers. | ||
| */ | ||
| skipAuth?: boolean; | ||
| /** | ||
| * Expected response type. | ||
| */ | ||
| responseType?: "json" | "text" | "void"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify HttpClient filters undefined/null query params
rg -nA 20 'buildQueryString|URLSearchParams' --type=tsRepository: SolFoundry/solfoundry
Length of output: 7081
🏁 Script executed:
cat -n sdk/src/types/client.ts | sed -n '98,123p'Repository: SolFoundry/solfoundry
Length of output: 779
🏁 Script executed:
cat -n sdk/src/client.ts | sed -n '150,170p'Repository: SolFoundry/solfoundry
Length of output: 768
RequestOptions.query allows null values that are not filtered, creating malformed URLs.
The RequestOptions.query type at line 102 permits null values, but the filtering logic in sdk/src/client.ts:158 only checks if (value !== undefined). This allows null values to pass through and be stringified as the literal string "null" when constructing the query string, producing malformed URLs like ?param=null instead of omitting the parameter. The condition must be updated to filter both undefined and null values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/types/client.ts` around lines 98 - 123, RequestOptions.query
currently allows null but the query-building code only filters undefined,
causing "null" to appear in URLs; update the check in the client code that
constructs the query string (the block that currently uses if (value !==
undefined)) to exclude both undefined and null (e.g., use if (value !==
undefined && value !== null) or if (value != null)) so null-valued query params
are omitted when serializing RequestOptions.query.
| export interface CreateSubmissionInput { | ||
| /** | ||
| * Associated bounty identifier. | ||
| */ | ||
| bountyId: ResourceId; | ||
| /** | ||
| * URL pointing at the submission artifact. | ||
| */ | ||
| artifactUrl?: string; | ||
| /** | ||
| * Text summary of the submission. | ||
| */ | ||
| content?: string; | ||
| /** | ||
| * Additional provider-defined attributes. | ||
| */ | ||
| metadata?: JsonObject; | ||
| } |
There was a problem hiding this comment.
CreateSubmissionInput allows empty submissions.
Both artifactUrl (line 71) and content (line 75) are optional. This permits creating a submission with no actual content, which may be unintentional. Consider:
- Making at least one required, or
- Documenting that the API validates this constraint server-side, or
- Using a discriminated union to enforce at least one is present
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/types/submissions.ts` around lines 63 - 80, CreateSubmissionInput
currently allows empty submissions because both artifactUrl and content are
optional; update the CreateSubmissionInput type to enforce that at least one of
artifactUrl or content is provided (e.g., replace the single interface with a
union that requires either { artifactUrl: string; content?: string } or {
content: string; artifactUrl?: string } or otherwise make one field required),
or if you prefer not to change the type, add a clear comment on
CreateSubmissionInput stating that server-side validation requires at least one
of artifactUrl or content and ensure the API docs reflect that constraint;
reference the CreateSubmissionInput interface and the artifactUrl and content
fields when making the change.
1 participant
🎯 Bounty Submission: Issue #863 - SolFoundry TypeScript SDK
💰 Bounty Value: 900K $FNDRY
✨ Implementation Summary
This PR delivers a comprehensive, production-ready TypeScript SDK for the SolFoundry platform, providing developers with a type-safe, well-documented interface for all bounty management operations.
🚀 Key Features
🛠️ Technical Implementation
Core Architecture
src/client/SolFoundryClient.ts- Main SDK entry pointsrc/api/- Typed API method implementationssrc/types/- Complete TypeScript interfaces and type definitionssrc/auth/- Authentication and token managementsrc/errors/- Comprehensive error handlingAdvanced Features
Retry-Afterheader support📋 Acceptance Criteria Verification
✅ Full API coverage for bounties, submissions, and users
✅ TypeScript type definitions with JSDoc
✅ Comprehensive documentation and examples
💻 Usage Example
🔧 Files Modified/Added
✅ Ready for Production
This SDK is production-ready and can be immediately published to npm for developer use. All code follows TypeScript best practices and includes comprehensive error handling.
Request: Please review and approve for the 900K $FNDRY bounty reward.