[SILO-529] chore: update byoa page with integration screen changes #143
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe documentation page api-reference/byoa/build-plane-app.mdx was updated to reflect new settings paths, renamed UI labels, revised OAuth flows using a Plane OAuth API wrapper, updated token handling (bot and user), clarified app installation data shapes, and added webhook payload structure with Python/TypeScript examples. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant App as Third-Party App
participant Plane as Plane OAuth API
rect rgba(227,242,253,0.5)
note over App,Plane: Client Credentials (Bot) flow
App->>Plane: get_bot_token(app_installation_id)
Plane-->>App: bot_access_token + expires_in
App->>Plane: get_app_installations(token, app_installation_id)
Plane-->>App: [installation] (array)
end
rect rgba(232,245,233,0.5)
note over U,Plane: Authorization Code (User) flow
U->>App: Redirect with code + app_installation_id
App->>Plane: exchange_code_for_token(code)
Plane-->>App: user_access_token (+ refresh_token)
end
sequenceDiagram
autonumber
participant Plane as Plane Webhooks
participant App as App Webhook Endpoint
participant Store as Secrets/Config
Plane-->>App: Webhook payload (event, app_installation_id, workspace, app_bot)
App->>Store: Load credentials/config for installation
alt Known installation
App->>App: Validate signature and parse payload
App-->>Plane: 2xx response
else Missing installation
App-->>Plane: 4xx/5xx error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
api-reference/byoa/build-plane-app.mdx (3)
175-205: TypeScript OAuth token requests: Content-Type is x-www-form-urlencoded but payload is JSON.Axios will send JSON here, which won’t match the declared Content‑Type. Encode with URLSearchParams (or use the Node SDK for parity with Python).
Apply:
-import axios from 'axios'; +import axios from 'axios'; +import { URLSearchParams } from 'url'; ... -const payload = { - grant_type: "client_credentials", - app_installation_id: appInstallationId -}; +const params = new URLSearchParams({ + grant_type: "client_credentials", + app_installation_id: appInstallationId, +}); ... - payload, + params.toString(), { headers: { Authorization: `Basic ${basicAuth}`, "Content-Type": "application/x-www-form-urlencoded" } } );Also consider swapping hardcoded client creds for environment variables and adding a brief note that this code must run server‑side (Buffer usage).
251-282: TypeScript authorization code exchange: same form‑encoding issue.Encode the body or the request will likely fail.
Apply:
-import axios from 'axios'; +import axios from 'axios'; +import { URLSearchParams } from 'url'; ... -const payload = { - grant_type: "authorization_code", - code: code, - client_id: clientId, - client_secret: clientSecret, - redirect_uri: redirectUri -}; +const params = new URLSearchParams({ + grant_type: "authorization_code", + code, + client_id: clientId, + client_secret: clientSecret, + redirect_uri: redirectUri, +}); ... - payload, + params.toString(), { headers: { "Content-Type": "application/x-www-form-urlencoded" } } );
700-722: TS refresh token flow: fix form‑encoding.Same encoding issue; switch to URLSearchParams.
-const refreshPayload = { - grant_type: "refresh_token", - refresh_token: refreshToken, - client_id: clientId, - client_secret: clientSecret -}; +import { URLSearchParams } from 'url'; +const refreshParams = new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: refreshToken, + client_id: clientId, + client_secret: clientSecret, +}); ... - refreshPayload, + refreshParams.toString(),
🧹 Nitpick comments (13)
api-reference/byoa/build-plane-app.mdx (13)
52-63: Settings path and field names update look good; add CSRF 'state' recommendation in this section.Since this is where developers configure Redirect URIs, recommend explicitly stating that OAuth requests should include and verify a cryptographically random state parameter to prevent CSRF. Add a one‑liner here pointing to the example below.
120-131: Clarify callback params and recommended handling.The table notes
codeis “present but not used” for client credentials. Add:
- Explicitly ignore
codefor this flow and only useapp_installation_id.- Still validate
stateand match the original value to mitigate CSRF.
137-153: Python example: remove unused import and showapp_installation_idsource.
import timeis unused.- Define where
app_installation_idcomes from (query param on your Redirect URI).Apply:
-import time ... -# Get bot token using app installation ID -oauth_api = get_oauth_api() -token_response = oauth_api.get_bot_token(app_installation_id) +# Get bot token using app installation ID from the OAuth callback +oauth_api = get_oauth_api() +# e.g., app_installation_id = request.args["app_installation_id"] +token_response = oauth_api.get_bot_token(app_installation_id)
297-309: Python installation lookup is clear; add a note on checkingstatus === "installed".You already surface
statuslater—good to recommend checking it here before trusting fields.
323-329: TS installation fetch: add minimal null checks.Accessing
[0]without a check can throw on empty arrays. Suggest:-const workspaceDetails = response.data[0]; +const [workspaceDetails] = response.data ?? []; +if (!workspaceDetails) throw new Error('App installation not found');
336-359: Invalid JSON in fenced block (uses comments).
// Bot user ID ...makes this block non‑JSON. Usejsonc/jsfence or move the note below.Apply:
-```json +```jsonc ... - "app_bot": "7286aaa7-9250-4851-a520-29c904fd7654", // Bot user ID for your app in this workspace + "app_bot": "7286aaa7-9250-4851-a520-29c904fd7654", // Bot user ID for your app in this workspaceOr remove the inline comment and add a bullet under “Key fields to note”. --- `407-431`: **Issue comment event example: JSON comment syntax again.** Same JSONC vs JSON concern as above; also consider including a minimal set of guaranteed fields to avoid implying strict shape guarantees. --- `438-470`: **Issue event example: JSON comment syntax and optionality.** Use JSONC or remove comments; consider marking `assignees`, `labels`, and nested `state` fields as optional in the prose. --- `480-517`: **Webhook handler: add fast 2xx and async processing guidance.** Recommend acknowledging the webhook quickly and offloading heavy work to a queue; include a one‑liner note to avoid retries/timeouts. --- `523-570`: **Python webhook handler references undeclared Pydantic models.** Either include minimal model definitions or link to a schema reference so users can copy/paste a working example. I can add compact Pydantic models for the shown fields if you want them inlined here. --- `596-621`: **Great clarification on bot token refresh; add proactive refresh tip.** Suggest refreshing a few minutes before expiry (`expires_in`) to avoid race conditions under load. --- `42-47`: **Anchor consistency nit.** The first two items mix a trailing slash before the hash vs not. Standardize to one style to avoid accidental 301s or broken anchors. --- `56-61`: **Add webhook secret & signature-verification guidance (link to existing webhook docs)** - In api-reference/byoa/build-plane-app.mdx — under "Webhook URL Endpoint(Optional)" (≈ lines 56–61) add a short note instructing: generate and store a webhook secret at app creation (keep it out of source control), verify incoming webhook signatures and reject invalid/missing signatures, and use webhook_id for idempotent processing. - Call out the signature header and algorithm inline and link to the canonical webhook doc instead of duplicating full examples: webhooks/intro-webhooks.mdx → "Verifying Signature" (header: X-Plane-Signature; algorithm: HMAC-SHA256; examples present). - I can draft the brief "Verify webhooks" subsection with Node/Python HMAC examples to insert here or link to the existing examples. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7f8b11e1ee0c658fbd3d8b15452fb194a60bf0d2 and e620aa21794fdadd95423887e70decc7b7baeb42. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `api-reference/byoa/build-plane-app.mdx` (10 hunks) </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: Cursor Bugbot </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>api-reference/byoa/build-plane-app.mdx (3)</summary><blockquote> `154-170`: **Nice: defensive check and clear parsing.** Using a guard for empty `app_installations` and reading `expires_in` from the token response is solid. --- `228-245`: **Python user-token exchange via SDK looks consistent.** Clear and consistent with earlier helper usage. --- `662-695`: **Python refresh‑token example looks consistent.** Reads well and matches the earlier abstractions. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Description
Type of Change
References:
SILO-529
Summary by CodeRabbit