feat(plugin): Ambient Weather (ambient_weather) - #650
Merged
devjain32 merged 10 commits intoAug 12, 2026
Conversation
Contributor
|
@meetbatra is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the Ambient Weather Corsair plugin with API-key authentication, typed device and historical-data endpoints, response and database schemas, rate-limit handling, package configuration, tests, and provider registration. ChangesAmbient Weather plugin
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Corsair
participant AmbientWeatherEndpoint
participant AmbientWeatherClient
participant AmbientWeatherAPI
participant Database
Corsair->>AmbientWeatherEndpoint: Invoke devices list or getData
AmbientWeatherEndpoint->>AmbientWeatherClient: Pass credentials and query
AmbientWeatherClient->>AmbientWeatherAPI: Send authenticated GET request
AmbientWeatherAPI-->>AmbientWeatherClient: Return device or reading data
AmbientWeatherClient-->>AmbientWeatherEndpoint: Return validated response
AmbientWeatherEndpoint->>Database: Optionally upsert devices or readings
AmbientWeatherEndpoint-->>Corsair: Return endpoint response
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Contributor
Greptile SummaryThe PR replaces the Ambient Weather scaffold with a functional API-key integration.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Corsair
participant Plugin as Ambient Weather Plugin
participant API as AmbientWeather.net
participant DB as Corsair Database
Caller->>Corsair: devices.list / devices.getData
Corsair->>Plugin: Bound endpoint with packed credentials
Plugin->>API: GET with apiKey and applicationKey
API-->>Plugin: Devices or historical readings
Plugin->>Plugin: Validate response with Zod
Plugin->>DB: Upsert devices or readings
Plugin-->>Caller: Validated response
Reviews (3): Last reviewed commit: "fix(ambientweather): parse Retry-After H..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+3
to
+20
| describe('AmbientWeather schema', () => { | ||
| it('declares a semver version', () => { | ||
| expect(AmbientWeatherSchema.version).toBeDefined(); | ||
| expect(AmbientWeatherSchema.version).toMatch(/^\d+\.\d+\.\d+$/); | ||
| }); | ||
|
|
||
| it('declares an entities map', () => { | ||
| expect(typeof AmbientWeatherSchema.entities).toBe('object'); | ||
| expect(AmbientWeatherSchema.entities).not.toBeNull(); | ||
| expect(Array.isArray(Object.keys(AmbientWeatherSchema.entities))).toBe( | ||
| true, | ||
| ); | ||
| for (const entity of Object.values(AmbientWeatherSchema.entities)) { | ||
| expect(entity).toBeDefined(); | ||
| } | ||
| }); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
When CI runs this package, these assertions inspect only schema metadata and never invoke example.get, allowing broken endpoint wiring, request authentication, paths, and response handling to pass the test suite.
Rule Used: Plugin packages must include at least one *.test.t... (source)
Knowledge Base Used: The provider-plugin package pattern
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @meetbatra, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
Rule Used: Flag boilerplate residue from the plugin generator... (source) Knowledge Base Used: The provider-plugin package pattern
Rule Used: Plugin packages must include at least one *.test.t... (source) Knowledge Base Used: The provider-plugin package pattern PR requirements (rules)
If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/ambientweather/client.ts`:
- Around line 54-58: Update makeAmbientWeatherRequest’s catch handling to
preserve ApiError instances and their status and retryAfter metadata instead of
replacing them with metadata-free AmbientWeatherAPIError values. Re-throw
ApiError unchanged, or ensure AmbientWeatherAPIError retains those fields and
update the related handlers to use it for 401/429 classification and
headersRetryAfterMs.
- Around line 14-39: Implement the Ambient Weather authentication contract
across packages/ambientweather/client.ts:14-39 and
packages/ambientweather/index.ts:38-46, 117-135, and 195-207. Use
https://rt.ambientweather.net/v1, add application_key to the api_key account
configuration, resolve both credentials, and append them as apiKey and
applicationKey query parameters. Remove OpenAPI TOKEN usage, bearer/OAuth
authentication, and generator placeholder comments; update each listed index.ts
site as needed to use the new credential flow.
In `@packages/ambientweather/endpoints/example.ts`:
- Around line 6-12: Extend packages/ambientweather/endpoints/example.ts:6-12
with device-list and latest-data endpoint implementations, using GET /v1/devices
for device listings and consuming each device’s lastData for latest readings;
reserve /v1/devices/{macAddress} for historical data only. Update the endpoint
schemas and types in packages/ambientweather/endpoints/types.ts:3-29 to describe
these operations and responses, then register all required operations in
packages/ambientweather/index.ts:86-105 alongside example.get.
In `@packages/ambientweather/webhooks/tenant-matcher.ts`:
- Around line 14-24: Replace the unsupported tenant_external_id webhook matching
in packages/ambientweather/webhooks/tenant-matcher.ts (lines 14-24) with
matching on the documented Ambient Weather delivery identifier, such as
macAddress, and keep the matcher linkType identical to the configured account
field. Remove or update the placeholder OAuth resolver in
packages/ambientweather/webhooks/oauth-tenant-link.ts (lines 9-30) so routing
uses the same persisted identifier and succeeds for actual Ambient Weather
credential payloads.
In `@packages/ambientweather/webhooks/types.ts`:
- Around line 60-65: Update verifyAmbientWeatherWebhookSignature to stop
accepting requests unconditionally: inspect the provider’s supported webhook
authentication behavior, and if no signed protocol exists, return invalid rather
than { valid: true }. Ensure the example.handler registration is removed or
gated so unsigned Ambient Weather webhook requests cannot be accepted or logged.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0cd883b0-6cdd-40f7-8e71-916eb8ba2890
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
packages/ambientweather/client.tspackages/ambientweather/endpoints/example.tspackages/ambientweather/endpoints/index.tspackages/ambientweather/endpoints/types.tspackages/ambientweather/error-handlers.tspackages/ambientweather/index.tspackages/ambientweather/jest.config.cjspackages/ambientweather/package.jsonpackages/ambientweather/schema.test.tspackages/ambientweather/schema/database.tspackages/ambientweather/schema/index.tspackages/ambientweather/tsconfig.jsonpackages/ambientweather/tsup.config.tspackages/ambientweather/webhooks/example.tspackages/ambientweather/webhooks/index.tspackages/ambientweather/webhooks/oauth-tenant-link.tspackages/ambientweather/webhooks/tenant-matcher.tspackages/ambientweather/webhooks/types.tspackages/corsair/core/constants.ts
Comment on lines
+60
to
+65
| export function verifyAmbientWeatherWebhookSignature( | ||
| request: WebhookRequest<AmbientWeatherWebhookPayload>, | ||
| secret: string, | ||
| ): { valid: boolean; error?: string } { | ||
| // TODO: Implement webhook signature verification | ||
| return { valid: true }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Reject unsigned webhook requests.
verifyAmbientWeatherWebhookSignature always returns valid. It does not inspect request or secret. example.handler therefore accepts and logs a spoofed request with "type": "example".
Implement the provider-supported authentication check before registering this handler. If the provider has no signed webhook protocol, return invalid and do not expose this handler.
Safe interim change
export function verifyAmbientWeatherWebhookSignature(
request: WebhookRequest<AmbientWeatherWebhookPayload>,
secret: string,
): { valid: boolean; error?: string } {
- // TODO: Implement webhook signature verification
- return { valid: true };
+ return {
+ valid: false,
+ error: 'Webhook signature verification is not implemented',
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function verifyAmbientWeatherWebhookSignature( | |
| request: WebhookRequest<AmbientWeatherWebhookPayload>, | |
| secret: string, | |
| ): { valid: boolean; error?: string } { | |
| // TODO: Implement webhook signature verification | |
| return { valid: true }; | |
| export function verifyAmbientWeatherWebhookSignature( | |
| request: WebhookRequest<AmbientWeatherWebhookPayload>, | |
| secret: string, | |
| ): { valid: boolean; error?: string } { | |
| return { | |
| valid: false, | |
| error: 'Webhook signature verification is not implemented', | |
| }; | |
| } |
🤖 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 `@packages/ambientweather/webhooks/types.ts` around lines 60 - 65, Update
verifyAmbientWeatherWebhookSignature to stop accepting requests unconditionally:
inspect the provider’s supported webhook authentication behavior, and if no
signed protocol exists, return invalid rather than { valid: true }. Ensure the
example.handler registration is removed or gated so unsigned Ambient Weather
webhook requests cannot be accepted or logged.
This was referenced Aug 10, 2026
github-actions
Bot
added
app
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 `@docs/plugins/ambientweather/get-credentials.mdx`:
- Around line 30-35: Update docs/plugins/ambientweather/overview.mdx lines 36-39
and scripts/generate-plugin-docs.ts to document a non-argument credential flow,
using the accepted applicationKey field and hidden prompt, stdin, or safe
API-based input instead of exposing keys through process arguments. Update
docs/plugins/ambientweather/get-credentials.mdx lines 30-35 separately with the
same safe setup guidance; this page is not generated automatically.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88b475f3-10ed-40b8-a19f-ae482f929c75
📒 Files selected for processing (15)
demo/mcp/corsair.tsdocs/plugins/ambientweather/api.mdxdocs/plugins/ambientweather/get-credentials.mdxdocs/plugins/ambientweather/overview.mdxpackages/ambientweather/api.test.tspackages/ambientweather/client.tspackages/ambientweather/endpoints/devices.tspackages/ambientweather/endpoints/index.tspackages/ambientweather/endpoints/types.tspackages/ambientweather/error-handlers.tspackages/ambientweather/index.tspackages/ambientweather/schema/database.tspackages/ambientweather/tsconfig.jsonwww/src/app/oss/u/[username]/page.tsxwww/src/components/landing/menu/site-menu.tsx
💤 Files with no reviewable changes (1)
- demo/mcp/corsair.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ambientweather/tsconfig.json
Collaborator
|
@greptile review |
Dhirenderchoudhary
removed
docs
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 `@packages/ambientweather/schema/database.ts`:
- Around line 67-75: Update pickAmbientWeatherReadingFields to validate the
filtered out object with z.object(AmbientWeatherReadingFields).partial().parse
and return the schema-inferred partial type instead of using a type assertion.
Ensure complete historical records are parsed with AmbientWeatherReading so
dateutc remains required and numeric/raw field types are validated.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 46d1d71b-e4ef-450d-9b58-298ad9349eb9
📒 Files selected for processing (9)
packages/ambientweather/api.test.tspackages/ambientweather/client.tspackages/ambientweather/endpoints/devices.tspackages/ambientweather/endpoints/types.tspackages/ambientweather/error-handlers.tspackages/ambientweather/schema.test.tspackages/ambientweather/schema/database.tspackages/ambientweather/schema/index.tspackages/corsair/core/constants.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/ambientweather/schema.test.ts
- packages/corsair/core/constants.ts
- packages/ambientweather/error-handlers.ts
- packages/ambientweather/api.test.ts
- packages/ambientweather/client.ts
Comment on lines
+67
to
+75
| export function pickAmbientWeatherReadingFields( | ||
| data: Record<string, unknown>, | ||
| ): Omit<AmbientWeatherReading, 'macAddress' | 'checkedAt'> { | ||
| const out: Record<string, unknown> = {}; | ||
| for (const key of Object.keys(AmbientWeatherReadingFields)) { | ||
| if (key in data) out[key] = data[key]; | ||
| } | ||
| return out as Omit<AmbientWeatherReading, 'macAddress' | 'checkedAt'>; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return a validated partial field object.
data can omit dateutc, but Line 69 promises that dateutc exists and is a number. Raw values can also have invalid types. This assertion can let an invalid historical record pass TypeScript checks before later persistence fails.
Validate out with z.object(AmbientWeatherReadingFields).partial() and return its inferred type. Parse the complete historical record with AmbientWeatherReading where dateutc must be required.
#!/bin/bash
set -euo pipefail
# Map the package before inspecting selector consumers.
ast-grep outline packages/ambientweather --items all --type function
# Confirm that each consumer validates the complete historical record.
rg -n --type ts -C 4 \
'pickAmbientWeatherReadingFields\s*\(|AmbientWeatherReading\.(parse|safeParse)\s*\(' \
packages/ambientweather🤖 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 `@packages/ambientweather/schema/database.ts` around lines 67 - 75, Update
pickAmbientWeatherReadingFields to validate the filtered out object with
z.object(AmbientWeatherReadingFields).partial().parse and return the
schema-inferred partial type instead of using a type assertion. Ensure complete
historical records are parsed with AmbientWeatherReading so dateutc remains
required and numeric/raw field types are validated.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/ambientweather/client.ts`:
- Around line 75-92: Ensure Ambient Weather request URLs contain only one API
version segment: update endpoint values passed by every
makeAmbientWeatherRequest caller to be relative paths such as /devices, or
remove /v1 from AMBIENTWEATHER_API_BASE, then keep buildAmbientWeatherUrl
consistent with that single convention.
- Around line 132-139: Update the Retry-After parsing near retryAfterHeader to
accept non-negative numeric delta-seconds and, when numeric parsing is invalid
or negative, parse the header as an HTTP date and convert it to a non-negative
millisecond delay relative to the current time. Preserve undefined for invalid
dates or otherwise unusable values, and remove the current Number-only handling.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ec2007e-58e2-48b0-bcc9-eb87df32a1f9
📒 Files selected for processing (4)
packages/ambientweather/api.test.tspackages/ambientweather/client.tspackages/ambientweather/endpoints/devices.tspackages/ambientweather/schema/database.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ambientweather/api.test.ts
- packages/ambientweather/endpoints/devices.ts
Collaborator
|
@greptile review |
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
devjain32
approved these changes
Aug 12, 2026
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
3 participants
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.
Description
Adds the Ambient Weather plugin (
ambient_weather) for accessing real-time andhistorical data from personal Ambient Weather stations via the AmbientWeather.net API.
Auth: API key + application key (no OAuth, query param based).
Key endpoints:
Fixes #643
Checklist
Before submitting your PR, please verify the following:
pnpm lintand all checks pass (4 pre-existing failures in unrelated files not touched by this PR)pnpm typecheckand there are no TypeScript errorspnpm buildand all packages build successfullypnpm testand all tests passScreenshots / Demos
Demo (tests passing + code walkthrough): https://drive.google.com/file/d/1ug9Oebe1wboud1o52eeMDU5HOYk7Wklb/view?usp=sharing
Additional Notes
Implementation complete. The 4 lint failures reported by
pnpm lintare pre-existingin unrelated files (explorer/data/plugins.json, demo/mcp/corsair.ts,
www/src/app/oss/u/[username]/page.tsx, www/src/components/landing/menu/site-menu.tsx)
that are not touched by this PR.
Summary by CodeRabbit
New Features
Tests