feat(js): add @celestoai/sdk npm package#7
Merged
Conversation
Move the Gatekeeper-only JavaScript/TypeScript SDK out of the backend repo and into this sdk repo under js/, so the Python and JS clients live side by side and can be versioned/released from one place. - Copy src/, package.json, tsconfig.json, tsup.config.ts, README, LICENSE, and test.mjs under js/ (no code changes to the sources). - Drop the self-referential "@celestoai/sdk" entry from dependencies in package.json — it was a leftover from when test.mjs was wired up and causes npm install to loop on the package itself. - Add .github/workflows/js-test.yml: Node 20 build + tsc --noEmit, path-scoped to js/** so Python-only changes don't spin up Node. - Add a matching path filter to the existing Python test.yml so JS-only changes don't re-run the Python suite. - Ignore node_modules/ and *.tsbuildinfo in .gitignore (dist/ was already covered by the Python section). - Document the JS SDK layout, build commands, and publish process in CLAUDE.md so future agents know about both packages. The old copy at celesto-backend/packages/sdk/ is intentionally left in place for now; it will be deleted in a follow-up once this lands and CI is green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRight then, listen here. You've got yourself a fresh JavaScript SDK wrapped up and ready for distribution. New entry under Changes
Sequence DiagramsequenceDiagram
participant Client as Client Code
participant SDK as GatekeeperClient
participant HTTP as request() Handler
participant API as Gatekeeper API
participant Error as Error Handler
Client->>SDK: connect(payload, overrides?)
SDK->>HTTP: request(ctx, {POST, /connect, body})
HTTP->>HTTP: Build URL + query string
HTTP->>HTTP: Merge headers (auth, org, agent)
HTTP->>HTTP: Create timeout AbortController
HTTP->>API: fetch(url, {method, headers, body, signal})
alt Success (200-299)
API-->>HTTP: Response {status, body}
HTTP->>HTTP: Parse JSON/text by content-type
HTTP-->>SDK: Typed response<br/>(T)
else Not Modified (204)
API-->>HTTP: {status: 204}
HTTP-->>SDK: undefined
else Client/Server Error (4xx/5xx)
API-->>HTTP: Response {status, body}
HTTP->>HTTP: Extract error from<br/>detail/message/error
HTTP->>Error: throw CelestoApiError
Error-->>SDK: ❌ Error
end
HTTP->>HTTP: Clear timeout
SDK-->>Client: GatekeeperConnectResponse
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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 |
aniketmaurya
changed the title
![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
🧹 Nitpick comments (5)
CLAUDE.md (1)
58-60: Tighten the JS command docs for reproducibility.Use
npm cion Line 58 to mirror CI and avoid lockfile drift surprises; then keep smoke-test instructions explicitly tied to a built package.📝 Suggested doc tweak
-- Build: `cd js && npm install && npm run build` (tsup → ESM + CJS + DTS under `js/dist/`) +- Build: `cd js && npm ci && npm run build` (tsup → ESM + CJS + DTS under `js/dist/`) -- Smoke test: `cd js && node test.mjs` (requires `CELESTO_API_KEY` and hits the live API) +- Smoke test (after build): `cd js && node test.mjs` (requires `CELESTO_API_KEY` and hits the live API)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 58 - 60, Replace the install step "cd js && npm install && npm run build" with a CI-safe install using "npm ci" to avoid lockfile drift, and change the smoke-test instruction that currently runs the unbuilt test (e.g., "cd js && node test.mjs") to explicitly run the built artifact under js/dist (i.e., run the smoke test against the built package in js/dist) while keeping the CELESTO_API_KEY requirement noted.js/src/gatekeeper/client.ts (1)
182-190: The path interpolation's worth a word.The
connectionIdgoes straight into the URL path. Now, the server's meant to validate this proper, but if you're feeling cautious, you could add a simple client-side check to reject values containing path separators. Defense in depth, as they say in the business.This is optional—server-side validation should handle malicious input, but client-side validation provides an extra layer:
🛡️ Optional validation helper
const validatePathParam = (value: string, name: string): void => { if (value.includes('/') || value.includes('\\')) { throw new Error(`Invalid ${name}: must not contain path separators`); } };Based on learnings: "validate all file paths before operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/gatekeeper/client.ts` around lines 182 - 190, getConnection currently interpolates connectionId directly into the URL path; add a small client-side validation to reject path separators before calling request (e.g. add and call a helper like validatePathParam(connectionId, "connectionId") at the top of getConnection) so that values containing '/' or '\' throw early; keep the rest of getConnection unchanged and ensure validatePathParam is reusable for other path params.js/README.md (2)
49-51: Same matter here, mate.Another code block missing a language identifier.
📝 Proposed fix
-``` +```text https://celesto.ai/legal/terms</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@js/README.mdaround lines 49 - 51, The fenced code block containing the URL
"https://celesto.ai/legal/terms" is missing a language identifier; update the
block fromtotext so it becomes a markdown code block with the language
tag (i.e., replace the openingwithtext) to ensure correct formatting
and consistent linting in README.md.</details> --- `36-38`: **Right, let's tidy up these code blocks, eh.** The static analysis tool's flagged these fenced code blocks without a language specified. For URLs like this, use `text` or `plaintext` to keep the linters quiet. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```text https://docs.celesto.ai/celesto-sdk/gatekeeper ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@js/README.mdaround lines 36 - 38, The fenced code block containing the URL
"https://docs.celesto.ai/celesto-sdk/gatekeeper" is missing a language
specifier; update that code fence (the triple-backtick block that wraps the URL)
to use a language liketextorplaintext(e.g., changetotext) so
static analysis/linters accept the block and rendering remains unchanged.</details> </blockquote></details> <details> <summary>js/src/gatekeeper/types.ts (1)</summary><blockquote> `9-9`: **Type the `status` field properly — that union's collapsing to plain `string`.** Look here. When you write `"redirect" | "connected" | string`, TypeScript folds the whole thing down to `string`. You lose the literal types, the autocomplete, all of it. That's not good business. Use `(string & {})` to keep the known values sharp while still accepting whatever else the wire might throw at you. <details> <summary>The fix</summary> ```diff +export type GatekeeperConnectStatus = "redirect" | "connected" | (string & {}); + export interface GatekeeperConnectResponse { - status: "redirect" | "connected" | string; + status: GatekeeperConnectStatus; oauthUrl?: string; connectionId?: string; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@js/src/gatekeeper/types.ts` at line 9, The status field is currently declared as status: "redirect" | "connected" | string which collapses to plain string and loses literal types; update the type of the status property in the Gatekeeper type (the status field in js/src/gatekeeper/types.ts) to preserve the literal types while still allowing arbitrary strings by replacing the final plain string with an intersection like (string & {}), i.e. make status be "redirect" | "connected" | (string & {}). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@js/test.mjs:
- Around line 4-10: Check for presence of CELESTO_API_KEY before constructing
GatekeeperClient and fail fast (throw or process.exit) if missing; specifically
validate process.env.CELESTO_API_KEY used in the GatekeeperClient constructor.
Do not console.log the full response from client.listConnections (the res
variable); instead log a safe summary (e.g., connection count, non-sensitive
fields, or masked IDs) or stringify only an allowlist of fields to avoid
exposing sensitive identifiers. Ensure these changes touch the GatekeeperClient
instantiation and the listConnections result handling.
Nitpick comments:
In@CLAUDE.md:
- Around line 58-60: Replace the install step "cd js && npm install && npm run
build" with a CI-safe install using "npm ci" to avoid lockfile drift, and change
the smoke-test instruction that currently runs the unbuilt test (e.g., "cd js &&
node test.mjs") to explicitly run the built artifact under js/dist (i.e., run
the smoke test against the built package in js/dist) while keeping the
CELESTO_API_KEY requirement noted.In
@js/README.md:
- Around line 49-51: The fenced code block containing the URL
"https://celesto.ai/legal/terms" is missing a language identifier; update the
block fromtotext so it becomes a markdown code block with the language
tag (i.e., replace the openingwithtext) to ensure correct formatting
and consistent linting in README.md.- Around line 36-38: The fenced code block containing the URL
"https://docs.celesto.ai/celesto-sdk/gatekeeper" is missing a language
specifier; update that code fence (the triple-backtick block that wraps the URL)
to use a language liketextorplaintext(e.g., changetotext) so
static analysis/linters accept the block and rendering remains unchanged.In
@js/src/gatekeeper/client.ts:
- Around line 182-190: getConnection currently interpolates connectionId
directly into the URL path; add a small client-side validation to reject path
separators before calling request (e.g. add and call a helper like
validatePathParam(connectionId, "connectionId") at the top of getConnection) so
that values containing '/' or '' throw early; keep the rest of getConnection
unchanged and ensure validatePathParam is reusable for other path params.In
@js/src/gatekeeper/types.ts:
- Line 9: The status field is currently declared as status: "redirect" |
"connected" | string which collapses to plain string and loses literal types;
update the type of the status property in the Gatekeeper type (the status field
in js/src/gatekeeper/types.ts) to preserve the literal types while still
allowing arbitrary strings by replacing the final plain string with an
intersection like (string & {}), i.e. make status be "redirect" | "connected" |
(string & {}).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `1bfe5677-77d8-47c4-95bb-f3dff9feac2f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between af7fd8b2c427657e98acd0a1de4ed9d515b22fa8 and 348f6db1bf07d89b9ab1b8796abed067d765c02e. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `js/package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (17)</summary> * `.github/workflows/js-test.yml` * `.github/workflows/test.yml` * `.gitignore` * `CLAUDE.md` * `js/LICENSE` * `js/README.md` * `js/package.json` * `js/src/core/config.ts` * `js/src/core/errors.ts` * `js/src/core/http.ts` * `js/src/gatekeeper/client.ts` * `js/src/gatekeeper/index.ts` * `js/src/gatekeeper/types.ts` * `js/src/index.ts` * `js/test.mjs` * `js/tsconfig.json` * `js/tsup.config.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Drop the stray indentation from the original snippet and throw a clear error when the env var is missing, so a missing token surfaces as a one-line message instead of a deeper 401 from the API call. Keep the `console.log(res)` as-is: this script is a manual developer smoke test, and the whole point is to eyeball the real response shape after SDK changes — reducing it to a summary defeats the purpose. The script is not run in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Add npm package
Summary by CodeRabbit
New Features
Chores