Skip to content

feat: add Amara plugin - #648

Merged
devjain32 merged 6 commits into
corsairdev:mainfrom
GarvChopra:feat/amara-plugin
Aug 12, 2026
Merged

feat: add Amara plugin#648
devjain32 merged 6 commits into
corsairdev:mainfrom
GarvChopra:feat/amara-plugin

Conversation

@GarvChopra

@GarvChopra GarvChopra commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the Amara plugin (@corsair-dev/amara) with 30 endpoints against the live Amara API (https://amara.org/api), authenticated via X-api-key.

Covers videos, subtitle languages/subtitles/notes/actions, video URLs, users, teams, activity, languages, and messages. Zod schemas match official Amara response shapes. Webhook scaffold removed (Amara has no triggers here). Includes unit + live contract tests.

Checklist

Before submitting your PR, please verify the following:

  • I have run pnpm lint and all checks pass
  • I have run pnpm typecheck and there are no TypeScript errors
  • I have run pnpm build and all packages build successfully
  • I have run pnpm test and all tests pass
  • I have added or updated tests where applicable
  • I have added or updated necessary documentation

Screenshots / Demos (if applicable)

Screenshot 2026-08-12 at 1 51 45 PM

Additional Notes

  • Auth: API key only (X-api-key)
  • Docs: https://apidocs.amara.org/
  • Live tests: AMARA_API_KEY=... pnpm --filter @corsair-dev/amara test

Summary by CodeRabbit

  • New Features

    • Added Amara integration with API authentication, endpoint requests, webhook handling, tenant matching, and OAuth support.
    • Added an example endpoint and webhook with payload validation and event matching.
    • Added Amara provider visibility across supported provider selections.
    • Added schema metadata and typed request, response, and webhook definitions.
  • Bug Fixes

    • Added retry handling for rate limits and disabled retries for authentication and other errors.
  • Tests

    • Added validation for Amara schema metadata and package test configuration.

Summary by CodeRabbit

  • New Features
    • Added Amara as a supported provider.
    • Added comprehensive Amara integration for videos, subtitles, users, teams, languages, activity, and messaging.
    • Added video, URL, subtitle, team, and user management operations.
    • Added response validation and structured handling for authentication, rate limits, validation, not-found, and server errors.
  • Tests
    • Added extensive coverage for API requests, endpoint behavior, schemas, error handling, and live API contracts.

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@GarvChopra is attempting to deploy a commit to the corsair Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the core Changes in packages/corsair label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c37660e-a811-4579-8fd2-f00075bc686e

📥 Commits

Reviewing files that changed from the base of the PR and between c126b15 and 1193dd8.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • packages/amara/api.test.ts
  • packages/amara/endpoints/handlers.test.ts
  • packages/amara/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/amara/api.test.ts
  • packages/amara/package.json

📝 Walkthrough

Walkthrough

The PR adds a complete Amara provider package. It includes typed API contracts, authenticated endpoint handlers, schema validation, retry policies, Corsair plugin wiring, provider registration, package configuration, and API contract tests.

Changes

Amara provider integration

Layer / File(s) Summary
Amara contracts and schemas
packages/amara/endpoints/types.ts, packages/amara/schema/*, packages/amara/endpoints/types.test.ts, packages/amara/schema.test.ts
Defines Zod input/output contracts, endpoint type maps, schema registries, and database entities for Amara resources.
API client and error policies
packages/amara/client.ts, packages/amara/error-handlers.ts, packages/amara/*client.test.ts, packages/amara/*error-handlers.test.ts
Adds authenticated requests, query/path helpers, normalized empty responses, AmaraAPIError, and status-based retry handlers.
Endpoint implementations and validation
packages/amara/endpoints/*, packages/amara/endpoints/handlers.test.ts, packages/amara/api.test.ts
Adds video, subtitle, user, team, activity, language, and message operations. Handlers validate responses and log completion events.
Plugin wiring and provider registration
packages/amara/index.ts, packages/corsair/core/constants.ts
Registers endpoint bindings, schemas, metadata, API-key authentication, error handlers, factory behavior, and the amara provider.
Package build and test support
packages/amara/package.json, packages/amara/tsconfig.json, packages/amara/tsup.config.ts, packages/amara/jest.config.cjs
Adds package exports, build scripts, TypeScript settings, ESM bundling, and Jest configuration.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: plugin

Suggested reviewers: devjain32

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding the Amara plugin and its API integrations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces the Amara scaffold with a real API-key-authenticated integration.

  • Adds 30 endpoints across videos, subtitles, users, teams, activity, languages, and messages.
  • Adds Amara response schemas, error handling, provider registration, and live and mocked tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/amara/client.ts Implements the real Amara HTTP client with X-api-key authentication and normalized provider errors.
packages/amara/index.ts Registers 30 typed endpoints, API-key auth, endpoint metadata, schemas, and error handlers without the previous webhook/OAuth scaffold.
packages/amara/endpoints/videos.ts Implements the primary video, URL, subtitle, action, note, and activity operations.
packages/amara/endpoints/handlers.test.ts Directly exercises all 30 handlers and verifies their request construction.
packages/amara/endpoints/types.ts Defines the input and output schemas for the complete Amara endpoint surface.
packages/corsair/core/constants.ts Registers Amara as a supported provider and adds its display name.

Sequence Diagram

sequenceDiagram
  participant App as Corsair consumer
  participant Plugin as Amara endpoint
  participant Client as makeAmaraRequest
  participant API as amara.org/api
  App->>Plugin: Invoke typed endpoint
  Plugin->>Client: Path, API key, query/body
  Client->>API: HTTP request with X-api-key
  API-->>Client: Amara response
  Client-->>Plugin: Raw response
  Plugin->>Plugin: Validate output with Zod
  Plugin-->>App: Typed result
Loading

Reviews (4): Last reviewed commit: "fix(amara): cover all handlers and tight..." | Re-trigger Greptile

Comment thread packages/amara/webhooks/types.ts Outdated
Comment thread packages/amara/client.ts Outdated
Comment thread packages/amara/schema.test.ts Outdated
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Plugin PR scorecard — packages/amara

Check Status Notes
R1 — Scope: plugin files only
R2 — Tests with assertions
R3 — Description complete
R3 — Linked issue / claim ⚠️ No "Fixes #…" or claim link — add one if this PR has a claim or issue
R4 — Demo video / recording

Rules: PLUGIN_PR_RULES.md · re-runs on every push

@github-actions github-actions Bot added the gate:failed Plugin PR gate checks failing label Aug 10, 2026
@github-actions

Copy link
Copy Markdown

Hey @GarvChopra, thanks for the contribution! 🏴‍☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push.

Must fix

  • P1 packages/amara/webhooks/types.ts:58Webhook signatures always pass
    When an unauthenticated sender submits an example payload with any x-amara-signature header, this verifier returns valid without checking the request or secret, causing the forged event to be accepted and logged as completed. How this was verified: The request path reaches this unconditional verifier and the completed-event logging call without any upstream signature check.

Knowledge Base Used: The provider-plugin package pattern

  • P1 packages/amara/client.ts:16Provider integration remains scaffolded
    When a consumer invokes amara().example.get, the client sends the request to the placeholder api.example.com host instead of Amara, causing the endpoint to fail; the related webhook and OAuth paths likewise retain placeholder fields that prevent real payloads from being reliably routed or linked.

Rule Used: Flag boilerplate residue from the plugin generator... (source)

Knowledge Base Used: The provider-plugin package pattern

  • P1 packages/amara/schema.test.ts:18Endpoint has no behavioral test
    This is the package's only test, but it imports only AmaraSchema and never invokes example.get, violating the requirement that each implemented endpoint have corresponding assertions and leaving its URL, authentication, response shape, and event behavior unverified.

Rule Used: Plugin packages must include at least one *.test.t... (source)

Knowledge Base Used: The provider-plugin package pattern

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

PR requirements (rules)

  • R3 — Checklist has unchecked boxes
  • R4 — Required in "Screenshots / Demos" before a maintainer reviews

If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge.

@github-actions github-actions Bot added the bot:round-1 Review bot posted consolidated findings label Aug 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 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/amara/client.ts`:
- Around line 55-60: Update the catch logic in makeAmaraRequest to detect
ApiError before the generic Error handling and rethrow it unchanged, preserving
retryAfter and other API metadata for RATE_LIMIT_ERROR.handler. Keep wrapping
non-ApiError instances in AmaraAPIError with their existing message or the
Unknown error fallback.
- Around line 15-16: Update packages/amara/client.ts lines 15-16 to set
AMARA_API_BASE to https://amara.org/api/ and use Amara’s documented
authentication headers; update the callback matcher in packages/amara/index.ts
lines 152-155 to use a supported callback identifier instead of
x-amara-signature, and add outbound and inbound integration tests covering these
behaviors.

In `@packages/amara/jest.config.cjs`:
- Line 2: Remove the preset: 'ts-jest' entry from the Jest configuration while
retaining the existing custom transform map, so the configuration uses only one
transform strategy.
- Around line 35-44: Update the JavaScript transformer rule in the Jest
configuration so ts-jest receives allowJs: true in its tsconfig, or remove the
.*\\.js$ transform and corresponding uuid exception if JavaScript transformation
is unnecessary. Keep the existing ESM and interop settings intact.

In `@packages/amara/package.json`:
- Line 5: Update the package test script in package.json to invoke Jest with
Node’s --experimental-vm-modules flag, or reuse a workspace-level NODE_OPTIONS
configuration that sets it before Jest runs. Ensure the existing test command
remains otherwise unchanged and ESM tests execute under Jest 29.
- Around line 9-15: Update the package exports configuration so the published
package’s "." entry does not reference the absent ./index.ts: either remove the
workspace-only "dev-source" export or include index.ts and all required source
files in the package files list. Ensure npm pack --dry-run confirms the chosen
published-interface behavior.
- Around line 25-32: Declare `@types/node` in the devDependencies of the package
manifest using the repository’s version catalog, since
packages/amara/tsconfig.json requests Node typings. Keep the plugin
self-contained and do not rely on workspace-hoisted dependencies; only remove
"node" from compilerOptions.types if Node typings are not required.

In `@packages/amara/webhooks/tenant-matcher.ts`:
- Around line 17-24: Update tenant matching in tenant-matcher.ts around the
externalId extraction to use Amara’s documented team or primary_team identifier
and team slug fields instead of tenant_external_id. In oauth-tenant-link.ts,
remove the unsupported access_token or /me fallback and use only the enterprise
OAuth integration contract; apply the corresponding change to the sibling site
as needed.

In `@packages/amara/webhooks/types.ts`:
- Around line 52-58: Implement verifyAmaraWebhookSignature to read the
documented Amara signing header, validate the expected signature and payload
format, compute the corresponding HMAC, and compare signatures using a
timing-safe comparison. Return valid: false with an error whenever the header is
missing, malformed, or does not match; only return valid: true for a verified
request.
🪄 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: 2c9709fd-8da0-43b2-9dd6-72ece5d92c6b

📥 Commits

Reviewing files that changed from the base of the PR and between 99ade55 and 2923b12.

📒 Files selected for processing (19)
  • packages/amara/client.ts
  • packages/amara/endpoints/example.ts
  • packages/amara/endpoints/index.ts
  • packages/amara/endpoints/types.ts
  • packages/amara/error-handlers.ts
  • packages/amara/index.ts
  • packages/amara/jest.config.cjs
  • packages/amara/package.json
  • packages/amara/schema.test.ts
  • packages/amara/schema/database.ts
  • packages/amara/schema/index.ts
  • packages/amara/tsconfig.json
  • packages/amara/tsup.config.ts
  • packages/amara/webhooks/example.ts
  • packages/amara/webhooks/index.ts
  • packages/amara/webhooks/oauth-tenant-link.ts
  • packages/amara/webhooks/tenant-matcher.ts
  • packages/amara/webhooks/types.ts
  • packages/corsair/core/constants.ts

Comment thread packages/amara/client.ts Outdated
Comment thread packages/amara/client.ts
@@ -0,0 +1,55 @@
module.exports = {
preset: 'ts-jest',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the ts-jest preset when supplying a custom transform.

This configuration sets preset: 'ts-jest' and then replaces the transform map. The ts-jest documentation warns that combining a preset with a custom transform can cause Jest to transform files incorrectly. Remove the preset or build the transform map from the selected preset. (kulshekhar.github.io)

Verification
#!/usr/bin/env bash
set -euo pipefail

rg -n 'preset|transform' packages/amara/jest.config.cjs

Use one transform configuration strategy.

Also applies to: 20-34

🤖 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/amara/jest.config.cjs` at line 2, Remove the preset: 'ts-jest' entry
from the Jest configuration while retaining the existing custom transform map,
so the configuration uses only one transform strategy.

Comment on lines +35 to +44
'.*\\.js$': [
'ts-jest',
{
useESM: true,
tsconfig: {
esModuleInterop: true,
allowSyntheticDefaultImports: true,
},
},
],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not send JavaScript files through ts-jest without enabling allowJs.

The .*\\.js$ rule sends JavaScript files to ts-jest, and the uuid exception allows matching dependency files to reach that rule. The TypeScript configuration does not set allowJs: true. ts-jest requires allowJs for its JavaScript-with-TypeScript/ESM transforms. (kulshekhar.github.io)

Enable allowJs for this transformer, or remove the JavaScript transform and its dependency exception if they are not required.

Verification
#!/usr/bin/env bash
set -euo pipefail

rg -n 'allowJs|\\.js\\$|transformIgnorePatterns' \
  packages/amara/jest.config.cjs \
  packages/amara/tsconfig.json

Also applies to: 51-51

🤖 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/amara/jest.config.cjs` around lines 35 - 44, Update the JavaScript
transformer rule in the Jest configuration so ts-jest receives allowJs: true in
its tsconfig, or remove the .*\\.js$ transform and corresponding uuid exception
if JavaScript transformation is unnecessary. Keep the existing ESM and interop
settings intact.

"name": "@corsair-dev/amara",
"version": "0.1.0",
"description": "Amara plugin for Corsair",
"type": "module",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Enable the Node ESM runtime for the package test command.

The package uses ESM and the Jest configuration emits ESM. The script still runs plain jest. Jest 29 requires Node's --experimental-vm-modules flag for ESM execution. Add the flag or verify that the workspace sets NODE_OPTIONS before invoking this script. (jestjs.io)

Possible fix
-    "test": "jest"
+    "test": "node --experimental-vm-modules ./node_modules/jest/bin/jest.js"
Verification
#!/usr/bin/env bash
set -euo pipefail

rg -n 'experimental-vm-modules|NODE_OPTIONS|"test"[[:space:]]*:' \
  --glob '**/package.json' \
  --glob '**/*.yml' \
  --glob '**/*.yaml' \
  .

Also applies to: 19-19

🤖 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/amara/package.json` at line 5, Update the package test script in
package.json to invoke Jest with Node’s --experimental-vm-modules flag, or reuse
a workspace-level NODE_OPTIONS configuration that sets it before Jest runs.
Ensure the existing test command remains otherwise unchanged and ESM tests
execute under Jest 29.

Comment on lines +9 to +15
"exports": {
".": {
"dev-source": "./index.ts",
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep the dev-source export valid in the packed package.

exports["."]."dev-source" targets ./index.ts, but files includes only dist. npm includes only the selected files when packing, so the published package does not contain that target. (docs.npmjs.com)

If dev-source is workspace-only, remove it from the published export map or verify that release tooling strips it. Otherwise include the source entry and its required files.

Verification
#!/usr/bin/env bash
set -euo pipefail

npm pack --dry-run --ignore-scripts --json packages/amara |
  jq -e '.[0].files[] | select(.path == "index.ts")'

The command should find index.ts if dev-source is part of the published interface.

Also applies to: 41-43

🤖 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/amara/package.json` around lines 9 - 15, Update the package exports
configuration so the published package’s "." entry does not reference the absent
./index.ts: either remove the workspace-only "dev-source" export or include
index.ts and all required source files in the package files list. Ensure npm
pack --dry-run confirms the chosen published-interface behavior.

Comment thread packages/amara/package.json
Comment thread packages/amara/webhooks/tenant-matcher.ts Outdated
Comment thread packages/amara/webhooks/types.ts Outdated
Comment on lines +52 to +58
export function verifyAmaraWebhookSignature(
request: WebhookRequest<AmaraWebhookPayload>,
secret: string,
): { valid: boolean; error?: string } {
// TODO: Implement webhook signature verification
return { valid: true };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Implement signature verification before registering this webhook.

This function returns valid: true for every request. An attacker can submit a forged webhook if the caller uses this result as the authentication gate.

Verify the documented Amara signing header and payload format. Compute and compare the signature with a timing-safe comparison. Fail closed when the signature is missing, malformed, or invalid.

#!/bin/bash
set -euo pipefail

rg -n -C 5 'verifyAmaraWebhookSignature|signature|WebhookRequest' packages/amara packages/corsair
🤖 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/amara/webhooks/types.ts` around lines 52 - 58, Implement
verifyAmaraWebhookSignature to read the documented Amara signing header,
validate the expected signature and payload format, compute the corresponding
HMAC, and compare signatures using a timing-safe comparison. Return valid: false
with an error whenever the header is missing, malformed, or does not match; only
return valid: true for a verified request.

@Dhirenderchoudhary

Copy link
Copy Markdown
Collaborator

@greptile review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (5)
packages/amara/endpoints/videos.ts (1)

71-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the no-op destructure.

const { ...body } = input; copies every property and creates no new binding semantics. Use input directly.

♻️ Proposed change
 export const create: AmaraEndpoints['videosCreate'] = async (ctx, input) => {
-	const { ...body } = input;
 	const raw = await makeAmaraRequest('videos/', ctx.key, {
 		method: 'POST',
-		body,
+		body: input,
 	});
🤖 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/amara/endpoints/videos.ts` around lines 71 - 76, Remove the no-op
rest destructuring in create and pass input directly as the request body to
makeAmaraRequest, preserving the existing POST behavior.
packages/amara/client.test.ts (1)

95-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Guarantee that the catch block runs.

If makeAmaraRequest resolves, the assertions in the catch block never run and the test still passes. The second test in this file uses an explicit guard. Use the same pattern here, or add expect.assertions(4).

♻️ Proposed change
 		try {
 			await makeAmaraRequest('videos/', 'k');
+			throw new Error('expected makeAmaraRequest to throw');
 		} catch (error) {
 			const amaraError = error as AmaraAPIError;
🤖 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/amara/client.test.ts` around lines 95 - 112, Add an assertion guard
to the test for makeAmaraRequest, using expect.assertions(4) or the explicit
failure pattern already used by the neighboring test, so the catch block's four
property assertions must execute when verifying the wrapped AmaraAPIError.
packages/amara/api.test.ts (2)

178-196: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Delete the created video, or drop the live write test.

This test performs a non-idempotent write against the real Amara account. When creation succeeds, the video stays in the account. Repeated runs accumulate records. Add a cleanup step that deletes the created video in a finally block, or restrict the write path to the mocked client tests.

🤖 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/amara/api.test.ts` around lines 178 - 196, The live videos.create
test leaves successful creations in the Amara account; either remove this live
write test or add cleanup in a finally block that deletes the created video
using its returned id, while preserving the existing error assertion.

15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Set an explicit timeout for the live suite.

Every test in this suite performs at least one network call, and some perform three. The Jest default timeout is 5000 ms. Pass a timeout to each it, or call jest.setTimeout for the file, so a slow Amara response does not produce a false failure.

🤖 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/amara/api.test.ts` around lines 15 - 16, Set an explicit Jest
timeout for the entire test file, preferably near the test setup, using
jest.setTimeout with a duration appropriate for the network-backed tests. Ensure
all tests in the suite, including those making multiple Amara requests, use this
extended timeout without changing their assertions.
packages/amara/error-handlers.ts (1)

21-94: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Apply the message heuristics only when the status is unknown.

Each matcher first checks the status, then falls back to substring checks on the message. The fallback also runs when the status is known and does not match. A message can contain a foreign number, for example a video id 4045 or a body fragment "404", so a 500 error can route to NOT_FOUND_ERROR and lose its retries. Gate the message checks on status === undefined.

♻️ Proposed change for one matcher; apply the same pattern to the others
 	NOT_FOUND_ERROR: {
 		match: (error: Error) => {
-			if (getStatus(error) === 404) return true;
+			const status = getStatus(error);
+			if (status !== undefined) return status === 404;
 			const msg = error.message.toLowerCase();
 			return msg.includes('404') || msg.includes('not found');
 		},
🤖 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/amara/error-handlers.ts` around lines 21 - 94, Update the match
functions in errorHandlers—RATE_LIMIT_ERROR, AUTH_ERROR, NOT_FOUND_ERROR,
VALIDATION_ERROR, and SERVER_ERROR—so message substring heuristics run only when
getStatus(error) returns undefined. Preserve direct status matching, and return
false immediately for known non-matching statuses to prevent message contents
from overriding the classified error.
🤖 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/amara/endpoints/types.test.ts`:
- Around line 14-26: Extend the test named “requires video_url and title when
creating a video” with a case containing video_url but omitting title, and
assert that AmaraEndpointInputSchemas.videosCreate.safeParse(...).success is
false. Keep the existing valid case and missing-video_url rejection unchanged.

In `@packages/amara/endpoints/users.ts`:
- Around line 16-21: Remove the raw input.identifier from the
logEventFromContext payloads in both the amara.users.getData and
amara.users.getActivity handlers, or replace it with the project’s established
hashing mechanism so persisted events never retain human-readable identifiers.

In `@packages/amara/endpoints/videos.ts`:
- Around line 265-281: Update VideosCreateSubtitleLanguageInputSchema and
createSubtitleLanguage so the request input uses and sends the required
language_code field to Amara’s languages endpoint instead of language. Add a
write-path test covering the POST body and retain the existing response parsing
and event logging behavior.

---

Nitpick comments:
In `@packages/amara/api.test.ts`:
- Around line 178-196: The live videos.create test leaves successful creations
in the Amara account; either remove this live write test or add cleanup in a
finally block that deletes the created video using its returned id, while
preserving the existing error assertion.
- Around line 15-16: Set an explicit Jest timeout for the entire test file,
preferably near the test setup, using jest.setTimeout with a duration
appropriate for the network-backed tests. Ensure all tests in the suite,
including those making multiple Amara requests, use this extended timeout
without changing their assertions.

In `@packages/amara/client.test.ts`:
- Around line 95-112: Add an assertion guard to the test for makeAmaraRequest,
using expect.assertions(4) or the explicit failure pattern already used by the
neighboring test, so the catch block's four property assertions must execute
when verifying the wrapped AmaraAPIError.

In `@packages/amara/endpoints/videos.ts`:
- Around line 71-76: Remove the no-op rest destructuring in create and pass
input directly as the request body to makeAmaraRequest, preserving the existing
POST behavior.

In `@packages/amara/error-handlers.ts`:
- Around line 21-94: Update the match functions in
errorHandlers—RATE_LIMIT_ERROR, AUTH_ERROR, NOT_FOUND_ERROR, VALIDATION_ERROR,
and SERVER_ERROR—so message substring heuristics run only when getStatus(error)
returns undefined. Preserve direct status matching, and return false immediately
for known non-matching statuses to prevent message contents from overriding the
classified error.
🪄 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: 1a468e98-d466-4b77-a98a-d3c792d69ce0

📥 Commits

Reviewing files that changed from the base of the PR and between 2923b12 and a4ef7f8.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (20)
  • packages/amara/api.test.ts
  • packages/amara/client.test.ts
  • packages/amara/client.ts
  • packages/amara/endpoints/activity.ts
  • packages/amara/endpoints/index.ts
  • packages/amara/endpoints/languages.ts
  • packages/amara/endpoints/messages.ts
  • packages/amara/endpoints/teams.ts
  • packages/amara/endpoints/types.test.ts
  • packages/amara/endpoints/types.ts
  • packages/amara/endpoints/users.ts
  • packages/amara/endpoints/videos.ts
  • packages/amara/error-handlers.test.ts
  • packages/amara/error-handlers.ts
  • packages/amara/index.ts
  • packages/amara/package.json
  • packages/amara/schema.test.ts
  • packages/amara/schema/database.ts
  • packages/amara/schema/index.ts
  • packages/corsair/core/constants.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/corsair/core/constants.ts
  • packages/amara/package.json

Comment thread packages/amara/endpoints/types.test.ts
Comment thread packages/amara/endpoints/users.ts Outdated
Comment on lines +265 to +281
export const createSubtitleLanguage: AmaraEndpoints['videosCreateSubtitleLanguage'] =
async (ctx, input) => {
const { video_id, ...body } = input;
const raw = await makeAmaraRequest(
`${videoPath(video_id)}languages/`,
ctx.key,
{ method: 'POST', body },
);
const response = SubtitleLanguageSchema.parse(raw);
await logEventFromContext(
ctx,
'amara.videos.createSubtitleLanguage',
{ video_id },
'completed',
);
return response;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Amara API create subtitle language POST videos languages request body field

💡 Result:

To create a new subtitle language track for a video in the Amara API, you perform a POST request to the languages endpoint associated with the specific video [1][2]. Endpoint URL: POST https://amara.org/api/videos/(video-id)/languages/ [1][2] Request Body Fields: When creating a subtitle language, the request body should be a JSON object containing the following primary field: 1. language_code (string, required): The ISO 639-1 two-letter language code (e.g., "en", "es", "fr") for the subtitle language you wish to create [2][3][4]. Additionally, you can include the following optional field: 2. is_primary_audio_language (boolean, optional): Set to true if this language is the primary spoken language of the video; otherwise, set to false (default is false) [1][2]. Example Request Body: { "language_code": "en", "is_primary_audio_language": false } Important Considerations: - Each video can have multiple subtitle languages, but you cannot create the same language twice for the same video [3][5]. - It is recommended to use list_subtitle_languages to check for existing languages before attempting to create a new one [3][5]. - While ISO 639-1 codes are standard, other codes may be accepted by the API, though they may display as the code itself if not explicitly supported by the platform [6].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- relevant files ---'
fd -i '.*' packages/amara | sed -n '1,120p'

printf '%s\n' '--- schema and endpoint references ---'
rg -n -C 8 'VideosCreateSubtitleLanguageInputSchema|videosCreateSubtitleLanguage|createSubtitleLanguage|updateSubtitleLanguage|language_code|is_primary_audio_language' packages/amara

Repository: corsairdev/corsair

Length of output: 43422


Send language_code when creating a subtitle language.

VideosCreateSubtitleLanguageInputSchema defines language, but Amara requires language_code for POST /videos/{video_id}/languages/. Update the schema and add a write-path test.

🤖 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/amara/endpoints/videos.ts` around lines 265 - 281, Update
VideosCreateSubtitleLanguageInputSchema and createSubtitleLanguage so the
request input uses and sends the required language_code field to Amara’s
languages endpoint instead of language. Add a write-path test covering the POST
body and retain the existing response parsing and event logging behavior.

Comment thread packages/amara/api.test.ts Outdated
@github-actions github-actions Bot removed the gate:failed Plugin PR gate checks failing label Aug 12, 2026
@Dhirenderchoudhary

Copy link
Copy Markdown
Collaborator

@greptile review

Comment thread packages/amara/endpoints/videos.test.ts Outdated
@github-actions github-actions Bot added the bot:round-2 Review bot pushed an automated fix label Aug 12, 2026
@github-actions

Copy link
Copy Markdown

Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/amara/api.test.ts`:
- Around line 196-198: Update the catch block in the team-creation test to
assert the structured HTTP 400 status and the documented “Team is required”
error explicitly, rather than matching any string containing 400 or “Bad
Request.” Preserve the finally cleanup behavior.
🪄 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: 44be75d6-df15-444b-b3e1-7dba5b3ba7e0

📥 Commits

Reviewing files that changed from the base of the PR and between 2d2c522 and c126b15.

📒 Files selected for processing (9)
  • packages/amara/api.test.ts
  • packages/amara/client.test.ts
  • packages/amara/endpoints/types.test.ts
  • packages/amara/endpoints/types.ts
  • packages/amara/endpoints/users.ts
  • packages/amara/endpoints/videos.test.ts
  • packages/amara/endpoints/videos.ts
  • packages/amara/error-handlers.test.ts
  • packages/amara/error-handlers.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/amara/endpoints/types.test.ts
  • packages/amara/client.test.ts
  • packages/amara/endpoints/users.ts
  • packages/amara/endpoints/videos.ts
  • packages/amara/error-handlers.ts
  • packages/amara/endpoints/types.ts

Comment thread packages/amara/api.test.ts
@Dhirenderchoudhary

Copy link
Copy Markdown
Collaborator

@greptile review

@github-actions

Copy link
Copy Markdown

Maintainer review needed

Automated rounds are exhausted. Remaining findings:

  • P1 packages/amara/endpoints/videos.test.tsEndpoint handlers remain uncovered
    When a regression changes the method, path, request body, output parsing, or event logging of an untested handler such as videos.update, videos.deleteUrl, videos.performSubtitleAction, or messages.send, the tests still pass because only three handlers are invoked directly and the live suite calls makeAmaraRequest instead. This leaves at least 11 exposed endpoints without the required behavioral coverage.

Rule Used: Flag any types on exported or public surfaces as... (source)

Knowledge Base Used: The provider-plugin package pattern

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@github-actions github-actions Bot added the needs-maintainer Automated rounds exhausted - human review needed label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:round-1 Review bot posted consolidated findings bot:round-2 Review bot pushed an automated fix core Changes in packages/corsair needs-maintainer Automated rounds exhausted - human review needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants