feat(adapters): add tier 1 batch 4#46
Open
khaliqgant wants to merge 2 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughAdds four new adapters (Airtable, Calendly, Mixpanel, Segment) with path mappers, webhook normalization, read/writeback resolvers, tests, package barrels/configs, and updates the publish workflow options and .gitignore. ChangesAdapters and Tooling
CI and Repo Hygiene
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant Normalizer
participant ProviderAPI
Client->>Adapter: ingestWebhook(rawBody, headers)
Adapter->>Normalizer: validate + normalize
Normalizer-->>Adapter: NormalizedWebhook
Adapter->>ProviderAPI: optional read/writeback (queries)
Adapter-->>Client: writeFile/deleteFile via VFS
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/airtable/src/writeback.ts`:
- Around line 87-101: The early-return branch that returns payload.fields when
isRecord(payload.fields) is true can return an empty object (e.g.,
{"fields":{}}) and bypass the later non-empty check; update the
isRecord(payload.fields) branch in writeback.ts to validate that payload.fields
is a non-empty object (e.g., check Object.keys(payload.fields).length > 0) and
throw the same Error('Airtable writeback requires at least one field') if empty,
so only non-empty field objects are returned; keep the existing
ENVELOPE_KEYS-based build path unchanged for the other payload shapes.
In `@packages/calendly/src/webhook-normalizer.ts`:
- Around line 149-159: The freshness check in webhook-normalizer.ts currently
only rejects timestamps older than the tolerance window; update the logic around
nowMs, toleranceMs and parsed.timestampMs to reject timestamps whose absolute
difference from nowMs exceeds toleranceMs (i.e., use Math.abs(nowMs -
parsed.timestampMs) > toleranceMs) so that both past and future clock skew
outside the tolerance are rejected and return the same expired-timestamp
response shape (including receivedSignature, timestamp, timestampMs).
In `@packages/mixpanel/src/path-mapper.ts`:
- Around line 53-66: stableIdSuffix currently strips separators so distinct IDs
like "abc-123" and "abc_123" collide; update stableIdSuffix (and usages in
labelSegmentWithId / mixpanelEventPath) to produce a lossless, filesystem-safe
suffix — e.g., URL-encode or percent-encode the id (encodeURIComponent) or allow
and preserve '-' and '_' by changing the regex to /[^a-zA-Z0-9-_]+/g and avoid
collapsing meaningful characters; then use that encoded/preserved suffix in
labelSegmentWithId instead of the current lossy suffix so event paths remain
unique.
In `@packages/mixpanel/src/queries.ts`:
- Around line 56-62: The cohort members branch uses the deprecated GET
/api/2.0/cohorts/members; change it to call the Mixpanel Query API by setting
method to 'POST', endpoint to '/api/query/engage' (or the full Mixpanel query
path your client expects), and replace the query param shape with a body param
named filter_by_cohort whose value is JSON.stringify({ id: cohortId }) where
cohortId is the decoded cohortMembersMatch[1]; update any consumer of the
returned object (e.g., request builder) to send that body in the POST.
In `@packages/mixpanel/src/webhook-normalizer.ts`:
- Around line 224-236: The current logic treats record.event (Mixpanel event
name) as a candidate for the normalized eventType which causes labels containing
dots to be returned as normalized types; update the selection so explicit does
NOT include record.event and only considers record.eventType / record.event_type
and metadata/webhook equivalents (use readOptionalString on record.eventType,
record.event_type, metadata?.eventType, metadata?.event_type,
webhook?.eventType, webhook?.event_type), then keep the existing dot-check on
explicit and fallback to `${resolvedObjectType}.${resolvedAction}` if no
explicit or no dot; locate the variable explicit and the surrounding logic in
webhook-normalizer.ts (where readOptionalString, resolvedObjectType, and
resolvedAction are used) and remove record.event from that chain.
- Around line 501-518: readWebhookTimestamp currently falls back to event
timestamps (record.timestamp and metadata?.timestamp) which are event times and
should not be used for webhook freshness; remove those two fallbacks so the
function only considers the HTTP header (MIXPANEL_TIMESTAMP_HEADER) and
webhook-specific timestamps (record.webhookTimestamp, record.webhook_timestamp,
metadata.webhookTimestamp, metadata.webhook_timestamp, webhook.timestamp,
webhook.webhookTimestamp, webhook.webhook_timestamp) when computing the webhook
delivery timestamp in the readWebhookTimestamp function.
In `@packages/segment/src/types.ts`:
- Around line 139-140: The method union on the Segment request type is too
broad: change the method property in the type that declares "method: 'POST' |
'PUT' | 'PATCH';" to only allow 'POST' for the endpoints listed (the type that
also declares endpoint: '/v1/identify' | '/v1/track' | '/v1/page' | '/v1/group'
| '/v1/batch'), so update that method union to 'POST' only to match Segment's
supported HTTP verb and prevent 405s at runtime.
In `@packages/segment/src/webhook-normalizer.ts`:
- Around line 91-95: The code currently calls rawBodyString(rawPayload) and uses
that for signature logic; instead, when signature validation is required
(options.requireSignature) or a secret is present, use the original rawPayload
(the original string | Buffer) for HMAC checks rather than a re-serialized
string. Change the flow so resolveWebhookSecret and
assertValidSegmentWebhookSignature receive the original rawPayload when
performing signature validation (use rawPayload directly instead of
rawBodyString(rawPayload)), and add a guard that throws or returns a clear error
if rawPayload is not a string or Buffer when options.requireSignature is true.
Keep references to rawBodyString, resolveWebhookSecret,
assertValidSegmentWebhookSignature, rawPayload, and options.requireSignature
when making the changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dc53ade1-9a30-42fe-a9b0-28e37ee873aa
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (46)
.github/workflows/publish.yml.gitignorepackages/airtable/package.jsonpackages/airtable/src/__tests__/airtable-adapter.test.tspackages/airtable/src/__tests__/webhook-normalizer.test.tspackages/airtable/src/airtable-adapter.tspackages/airtable/src/index.tspackages/airtable/src/path-mapper.tspackages/airtable/src/queries.tspackages/airtable/src/types.tspackages/airtable/src/webhook-normalizer.tspackages/airtable/src/writeback.tspackages/airtable/tsconfig.jsonpackages/calendly/package.jsonpackages/calendly/src/__tests__/calendly-adapter.test.tspackages/calendly/src/__tests__/webhook-normalizer.test.tspackages/calendly/src/calendly-adapter.tspackages/calendly/src/index.tspackages/calendly/src/path-mapper.tspackages/calendly/src/queries.tspackages/calendly/src/types.tspackages/calendly/src/webhook-normalizer.tspackages/calendly/src/writeback.tspackages/calendly/tsconfig.jsonpackages/mixpanel/package.jsonpackages/mixpanel/src/__tests__/mixpanel-adapter.test.tspackages/mixpanel/src/__tests__/webhook-normalizer.test.tspackages/mixpanel/src/index.tspackages/mixpanel/src/mixpanel-adapter.tspackages/mixpanel/src/path-mapper.tspackages/mixpanel/src/queries.tspackages/mixpanel/src/types.tspackages/mixpanel/src/webhook-normalizer.tspackages/mixpanel/src/writeback.tspackages/mixpanel/tsconfig.jsonpackages/segment/package.jsonpackages/segment/src/__tests__/segment-adapter.test.tspackages/segment/src/__tests__/webhook-normalizer.test.tspackages/segment/src/index.tspackages/segment/src/path-mapper.tspackages/segment/src/queries.tspackages/segment/src/segment-adapter.tspackages/segment/src/types.tspackages/segment/src/webhook-normalizer.tspackages/segment/src/writeback.tspackages/segment/tsconfig.json
Comment on lines
+82
to
+84
| if (options.webhookSecret !== undefined) { | ||
| assertValidAirtableWebhookSignature(rawPayload, normalizedHeaders, options.webhookSecret); | ||
| } |
There was a problem hiding this comment.
Don't verify Airtable MACs against re-serialized objects.
assertValidAirtableWebhookSignature() accepts unknown, but for object inputs toRawBodyBuffer() hashes a fresh JSON.stringify(...) output rather than the exact request bytes Airtable signed. That makes signature verification fail for otherwise valid requests once the body has been parsed upstream.
Suggested fix
const normalizedHeaders = normalizeHeaders(headers);
if (options.webhookSecret !== undefined) {
+ if (
+ typeof rawPayload !== 'string' &&
+ !Buffer.isBuffer(rawPayload) &&
+ !(rawPayload instanceof Uint8Array) &&
+ !(rawPayload instanceof ArrayBuffer)
+ ) {
+ throw new Error('Airtable webhook signature validation requires the original raw request body.');
+ }
assertValidAirtableWebhookSignature(rawPayload, normalizedHeaders, options.webhookSecret);
}📝 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
| if (options.webhookSecret !== undefined) { | |
| assertValidAirtableWebhookSignature(rawPayload, normalizedHeaders, options.webhookSecret); | |
| } | |
| if (options.webhookSecret !== undefined) { | |
| if ( | |
| typeof rawPayload !== 'string' && | |
| !Buffer.isBuffer(rawPayload) && | |
| !(rawPayload instanceof Uint8Array) && | |
| !(rawPayload instanceof ArrayBuffer) | |
| ) { | |
| throw new Error('Airtable webhook signature validation requires the original raw request body.'); | |
| } | |
| assertValidAirtableWebhookSignature(rawPayload, normalizedHeaders, options.webhookSecret); | |
| } |
Comment on lines
+87
to
+101
| if (isRecord(payload.fields)) { | ||
| return { ...payload.fields }; | ||
| } | ||
|
|
||
| const fields: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(payload)) { | ||
| if (ENVELOPE_KEYS.has(key)) { | ||
| continue; | ||
| } | ||
| fields[key] = value; | ||
| } | ||
|
|
||
| if (Object.keys(fields).length === 0) { | ||
| throw new Error('Airtable writeback requires at least one field'); | ||
| } |
There was a problem hiding this comment.
Reject empty fields objects before building writeback requests.
At Line 87, payload.fields is returned directly, so {"fields":{}} bypasses the non-empty check at Line 99 and sends invalid writeback payloads downstream.
💡 Suggested fix
function unwrapFields(payload: Record<string, unknown>): Record<string, unknown> {
if (isRecord(payload.payload) && looksLikeSyncedEnvelope(payload)) {
return unwrapFields(payload.payload);
}
if (isRecord(payload.fields)) {
- return { ...payload.fields };
+ if (Object.keys(payload.fields).length === 0) {
+ throw new Error('Airtable writeback requires at least one field');
+ }
+ return { ...payload.fields };
}
const fields: Record<string, unknown> = {};
for (const [key, value] of Object.entries(payload)) {
if (ENVELOPE_KEYS.has(key)) {🤖 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/airtable/src/writeback.ts` around lines 87 - 101, The early-return
branch that returns payload.fields when isRecord(payload.fields) is true can
return an empty object (e.g., {"fields":{}}) and bypass the later non-empty
check; update the isRecord(payload.fields) branch in writeback.ts to validate
that payload.fields is a non-empty object (e.g., check
Object.keys(payload.fields).length > 0) and throw the same Error('Airtable
writeback requires at least one field') if empty, so only non-empty field
objects are returned; keep the existing ENVELOPE_KEYS-based build path unchanged
for the other payload shapes.
Comment on lines
+149
to
+159
| const nowMs = options.nowMs ?? Date.now(); | ||
| const toleranceMs = options.toleranceMs ?? DEFAULT_CALENDLY_WEBHOOK_TOLERANCE_MS; | ||
| if (nowMs - parsed.timestampMs > toleranceMs) { | ||
| return { | ||
| ok: false, | ||
| reason: 'expired-timestamp', | ||
| receivedSignature: parsed.signature, | ||
| timestamp: parsed.timestamp, | ||
| timestampMs: parsed.timestampMs, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Reject future timestamps outside the tolerance window too.
The current freshness check is one-sided, so a valid signature with a timestamp far in the future is still accepted. That weakens the tolerance guard; it should reject absolute clock skew in either direction.
Suggested fix
- if (nowMs - parsed.timestampMs > toleranceMs) {
+ if (Math.abs(nowMs - parsed.timestampMs) > toleranceMs) {
return {
ok: false,
reason: 'expired-timestamp',
receivedSignature: parsed.signature,
timestamp: parsed.timestamp,📝 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
| const nowMs = options.nowMs ?? Date.now(); | |
| const toleranceMs = options.toleranceMs ?? DEFAULT_CALENDLY_WEBHOOK_TOLERANCE_MS; | |
| if (nowMs - parsed.timestampMs > toleranceMs) { | |
| return { | |
| ok: false, | |
| reason: 'expired-timestamp', | |
| receivedSignature: parsed.signature, | |
| timestamp: parsed.timestamp, | |
| timestampMs: parsed.timestampMs, | |
| }; | |
| } | |
| const nowMs = options.nowMs ?? Date.now(); | |
| const toleranceMs = options.toleranceMs ?? DEFAULT_CALENDLY_WEBHOOK_TOLERANCE_MS; | |
| if (Math.abs(nowMs - parsed.timestampMs) > toleranceMs) { | |
| return { | |
| ok: false, | |
| reason: 'expired-timestamp', | |
| receivedSignature: parsed.signature, | |
| timestamp: parsed.timestamp, | |
| timestampMs: parsed.timestampMs, | |
| }; | |
| } |
🤖 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/calendly/src/webhook-normalizer.ts` around lines 149 - 159, The
freshness check in webhook-normalizer.ts currently only rejects timestamps older
than the tolerance window; update the logic around nowMs, toleranceMs and
parsed.timestampMs to reject timestamps whose absolute difference from nowMs
exceeds toleranceMs (i.e., use Math.abs(nowMs - parsed.timestampMs) >
toleranceMs) so that both past and future clock skew outside the tolerance are
rejected and return the same expired-timestamp response shape (including
receivedSignature, timestamp, timestampMs).
Comment on lines
+53
to
+66
| function stableIdSuffix(id: string): string { | ||
| return id | ||
| .trim() | ||
| .replace(/[^a-zA-Z0-9]+/g, '') | ||
| .toLowerCase(); | ||
| } | ||
|
|
||
| function labelSegmentWithId(label: string | undefined, id: string): string { | ||
| const normalizedId = assertNonEmptySegment(id, 'object id'); | ||
| const slug = label ? slugify(label) : ''; | ||
| const suffix = stableIdSuffix(normalizedId); | ||
| if (slug && suffix) { | ||
| return `${slug}--${suffix}`; | ||
| } |
There was a problem hiding this comment.
Preserve the exact event ID in the path suffix.
stableIdSuffix() is currently lossy: IDs like abc-123 and abc_123 both collapse to abc123. If the label slug also matches, mixpanelEventPath() can generate the same file path for two different events.
Suggested fix
function stableIdSuffix(id: string): string {
- return id
- .trim()
- .replace(/[^a-zA-Z0-9]+/g, '')
- .toLowerCase();
+ return encodeMixpanelPathSegment(id);
}🤖 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/mixpanel/src/path-mapper.ts` around lines 53 - 66, stableIdSuffix
currently strips separators so distinct IDs like "abc-123" and "abc_123"
collide; update stableIdSuffix (and usages in labelSegmentWithId /
mixpanelEventPath) to produce a lossless, filesystem-safe suffix — e.g.,
URL-encode or percent-encode the id (encodeURIComponent) or allow and preserve
'-' and '_' by changing the regex to /[^a-zA-Z0-9-_]+/g and avoid collapsing
meaningful characters; then use that encoded/preserved suffix in
labelSegmentWithId instead of the current lossy suffix so event paths remain
unique.
Comment on lines
+56
to
+62
| const cohortMembersMatch = path.match(/^\/mixpanel\/cohorts\/([^/]+)\/members\.json$/); | ||
| if (cohortMembersMatch?.[1]) { | ||
| return { | ||
| method: 'GET', | ||
| endpoint: '/api/2.0/cohorts/members', | ||
| query: { id: decodeURIComponent(cohortMembersMatch[1]) }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Using official Mixpanel developer docs, what endpoint + HTTP method should be used to fetch members of a cohort by cohort ID, and what request parameter shape is required (e.g., filter_by_cohort)?
💡 Result:
Use the Mixpanel Query API Engage endpoint: POST https://mixpanel.com/api/query/engage (or the regional variant) [1]. The required request parameter shape is that you send a form field named filter_by_cohort whose value is a JSON string/object containing the cohort id, e.g. filter_by_cohort: {"id": <cohort_id>} [1].
Citations:
Replace cohort endpoint with correct Mixpanel Query API route.
Lines 60–62 use the deprecated /api/2.0/cohorts/members endpoint with GET method. The current Mixpanel Query API requires POST to https://mixpanel.com/api/query/engage with a filter_by_cohort parameter structured as a JSON string containing the cohort ID (e.g., filter_by_cohort: JSON.stringify({ id: cohortId })). Update the endpoint, method, and parameter shape to match the documented API.
🤖 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/mixpanel/src/queries.ts` around lines 56 - 62, The cohort members
branch uses the deprecated GET /api/2.0/cohorts/members; change it to call the
Mixpanel Query API by setting method to 'POST', endpoint to '/api/query/engage'
(or the full Mixpanel query path your client expects), and replace the query
param shape with a body param named filter_by_cohort whose value is
JSON.stringify({ id: cohortId }) where cohortId is the decoded
cohortMembersMatch[1]; update any consumer of the returned object (e.g., request
builder) to send that body in the POST.
Comment on lines
+224
to
+236
| const explicit = | ||
| readOptionalString(record.eventType) ?? | ||
| readOptionalString(record.event_type) ?? | ||
| readOptionalString(record.event) ?? | ||
| readOptionalString(metadata?.eventType) ?? | ||
| readOptionalString(metadata?.event_type) ?? | ||
| readOptionalString(webhook?.eventType) ?? | ||
| readOptionalString(webhook?.event_type); | ||
| if (explicit?.includes('.')) { | ||
| return explicit.trim().toLowerCase(); | ||
| } | ||
|
|
||
| return `${resolvedObjectType}.${resolvedAction}`; |
There was a problem hiding this comment.
Treat event as a label, not as the normalized webhook event type.
record.event is the Mixpanel event name. If that name contains a dot, this path returns the label itself as eventType instead of ${objectType}.${action}, which breaks downstream routing on normalized event types.
Suggested fix
const explicit =
readOptionalString(record.eventType) ??
readOptionalString(record.event_type) ??
- readOptionalString(record.event) ??
readOptionalString(metadata?.eventType) ??
readOptionalString(metadata?.event_type) ??
readOptionalString(webhook?.eventType) ??
readOptionalString(webhook?.event_type);🤖 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/mixpanel/src/webhook-normalizer.ts` around lines 224 - 236, The
current logic treats record.event (Mixpanel event name) as a candidate for the
normalized eventType which causes labels containing dots to be returned as
normalized types; update the selection so explicit does NOT include record.event
and only considers record.eventType / record.event_type and metadata/webhook
equivalents (use readOptionalString on record.eventType, record.event_type,
metadata?.eventType, metadata?.event_type, webhook?.eventType,
webhook?.event_type), then keep the existing dot-check on explicit and fallback
to `${resolvedObjectType}.${resolvedAction}` if no explicit or no dot; locate
the variable explicit and the surrounding logic in webhook-normalizer.ts (where
readOptionalString, resolvedObjectType, and resolvedAction are used) and remove
record.event from that chain.
Comment on lines
+501
to
+518
| function readWebhookTimestamp( | ||
| record: MixpanelRecord, | ||
| headers: Record<string, string>, | ||
| ): number | undefined { | ||
| const metadata = getRecord(record.metadata); | ||
| const webhook = getRecord(record._webhook); | ||
| return ( | ||
| readOptionalTimestamp(headers[MIXPANEL_TIMESTAMP_HEADER]) ?? | ||
| readOptionalTimestamp(record.timestamp) ?? | ||
| readOptionalTimestamp(record.webhookTimestamp) ?? | ||
| readOptionalTimestamp(record.webhook_timestamp) ?? | ||
| readOptionalTimestamp(metadata?.timestamp) ?? | ||
| readOptionalTimestamp(metadata?.webhookTimestamp) ?? | ||
| readOptionalTimestamp(metadata?.webhook_timestamp) ?? | ||
| readOptionalTimestamp(webhook?.timestamp) ?? | ||
| readOptionalTimestamp(webhook?.webhookTimestamp) ?? | ||
| readOptionalTimestamp(webhook?.webhook_timestamp) | ||
| ); |
There was a problem hiding this comment.
Don't use event timestamps for webhook freshness validation.
readWebhookTimestamp() falls back to record.timestamp and metadata.timestamp. Those fields are typically the event time, not the delivery time, so delayed or replayed-but-valid events can be rejected as stale even when the webhook was just delivered.
Suggested fix
return (
readOptionalTimestamp(headers[MIXPANEL_TIMESTAMP_HEADER]) ??
- readOptionalTimestamp(record.timestamp) ??
readOptionalTimestamp(record.webhookTimestamp) ??
readOptionalTimestamp(record.webhook_timestamp) ??
- readOptionalTimestamp(metadata?.timestamp) ??
readOptionalTimestamp(metadata?.webhookTimestamp) ??
readOptionalTimestamp(metadata?.webhook_timestamp) ??
readOptionalTimestamp(webhook?.timestamp) ??🤖 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/mixpanel/src/webhook-normalizer.ts` around lines 501 - 518,
readWebhookTimestamp currently falls back to event timestamps (record.timestamp
and metadata?.timestamp) which are event times and should not be used for
webhook freshness; remove those two fallbacks so the function only considers the
HTTP header (MIXPANEL_TIMESTAMP_HEADER) and webhook-specific timestamps
(record.webhookTimestamp, record.webhook_timestamp, metadata.webhookTimestamp,
metadata.webhook_timestamp, webhook.timestamp, webhook.webhookTimestamp,
webhook.webhook_timestamp) when computing the webhook delivery timestamp in the
readWebhookTimestamp function.
Comment on lines
+139
to
+140
| method: 'POST' | 'PUT' | 'PATCH'; | ||
| endpoint: '/v1/identify' | '/v1/track' | '/v1/page' | '/v1/group' | '/v1/batch'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Segment Tracking API official docs: what HTTP methods are supported for /v1/identify, /v1/track, /v1/page, /v1/group, and /v1/batch?
💡 Result:
Supported HTTP methods for Segment’s HTTP Tracking API endpoints (api.segment.io/v1/): /v1/identify — POST [1] /v1/track — POST [1] /v1/page — POST [1] /v1/group — POST [1] /v1/batch — POST [1] All of these examples show POST requests to the respective /v1/ paths in Segment’s HTTP API source documentation. [1]
Citations:
Narrow method type to POST for these Segment API endpoints.
The /v1/identify, /v1/track, /v1/page, /v1/group, and /v1/batch endpoints only support POST requests according to Segment's official documentation. Including PUT and PATCH allows the type to accept requests that these endpoints reject at runtime with 405 errors.
Suggested change
export interface SegmentWritebackRequest {
- method: 'POST' | 'PUT' | 'PATCH';
+ method: 'POST';
endpoint: '/v1/identify' | '/v1/track' | '/v1/page' | '/v1/group' | '/v1/batch';
body: Record<string, unknown>;
}📝 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
| method: 'POST' | 'PUT' | 'PATCH'; | |
| endpoint: '/v1/identify' | '/v1/track' | '/v1/page' | '/v1/group' | '/v1/batch'; | |
| export interface SegmentWritebackRequest { | |
| method: 'POST'; | |
| endpoint: '/v1/identify' | '/v1/track' | '/v1/page' | '/v1/group' | '/v1/batch'; | |
| body: Record<string, unknown>; | |
| } |
🤖 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/segment/src/types.ts` around lines 139 - 140, The method union on
the Segment request type is too broad: change the method property in the type
that declares "method: 'POST' | 'PUT' | 'PATCH';" to only allow 'POST' for the
endpoints listed (the type that also declares endpoint: '/v1/identify' |
'/v1/track' | '/v1/page' | '/v1/group' | '/v1/batch'), so update that method
union to 'POST' only to match Segment's supported HTTP verb and prevent 405s at
runtime.
Comment on lines
+91
to
+95
| const rawBody = rawBodyString(rawPayload); | ||
| const secret = resolveWebhookSecret(rawBody, normalizedHeaders, options); | ||
| if (options.requireSignature || secret) { | ||
| assertValidSegmentWebhookSignature(rawBody, normalizedHeaders, secret, options); | ||
| } |
There was a problem hiding this comment.
Use the original request body for Segment signature checks.
If rawPayload is already a parsed object, rawBodyString() re-serializes it before HMAC validation. That byte stream can differ from the provider-signed body, so valid webhooks get rejected. Require the original string | Buffer whenever signature validation is enabled instead of verifying a reconstructed payload.
Suggested fix
const rawBody = rawBodyString(rawPayload);
const secret = resolveWebhookSecret(rawBody, normalizedHeaders, options);
if (options.requireSignature || secret) {
+ if (typeof rawPayload !== 'string' && !Buffer.isBuffer(rawPayload)) {
+ throw new Error('Segment webhook signature validation requires the original raw request body.');
+ }
assertValidSegmentWebhookSignature(rawBody, normalizedHeaders, secret, options);
}📝 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
| const rawBody = rawBodyString(rawPayload); | |
| const secret = resolveWebhookSecret(rawBody, normalizedHeaders, options); | |
| if (options.requireSignature || secret) { | |
| assertValidSegmentWebhookSignature(rawBody, normalizedHeaders, secret, options); | |
| } | |
| const rawBody = rawBodyString(rawPayload); | |
| const secret = resolveWebhookSecret(rawBody, normalizedHeaders, options); | |
| if (options.requireSignature || secret) { | |
| if (typeof rawPayload !== 'string' && !Buffer.isBuffer(rawPayload)) { | |
| throw new Error('Segment webhook signature validation requires the original raw request body.'); | |
| } | |
| assertValidSegmentWebhookSignature(rawBody, normalizedHeaders, secret, options); | |
| } |
🤖 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/segment/src/webhook-normalizer.ts` around lines 91 - 95, The code
currently calls rawBodyString(rawPayload) and uses that for signature logic;
instead, when signature validation is required (options.requireSignature) or a
secret is present, use the original rawPayload (the original string | Buffer)
for HMAC checks rather than a re-serialized string. Change the flow so
resolveWebhookSecret and assertValidSegmentWebhookSignature receive the original
rawPayload when performing signature validation (use rawPayload directly instead
of rawBodyString(rawPayload)), and add a guard that throws or returns a clear
error if rawPayload is not a string or Buffer when options.requireSignature is
true. Keep references to rawBodyString, resolveWebhookSecret,
assertValidSegmentWebhookSignature, rawPayload, and options.requireSignature
when making the changes.
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.
Summary
Adds Tier 1 batch 4 adapter packages for Airtable, Segment, Mixpanel, and Calendly. Each package includes adapter runtime, path mapping, read/query helpers, writeback helpers, webhook normalization, package metadata, tsconfig, and node:test coverage.
Also registers the new package slugs in
.github/workflows/publish.ymlfor one-off publishing andpackage=all, updates the npm lockfile with the new workspaces, and ignores generated local relay/verifier artifacts.Verifier follow-up
The verifier produced endpoint drift findings after the initial implementation. This PR includes fixes for the actionable issues before review:
GET /v1/*calls against write-only Tracking API routes. Segment writeback still uses Tracking API/v1/*POST routes, while read helpers now target Public API Delivery Overview metrics.POST /inviteesfor invitee creation,POST /scheduled_events/{uuid}/cancellation,POST /event_types, andPATCH /event_types/{uuid}. Unsupported scheduled-event updates and invitee update/cancel paths now throw instead of emitting invalid API calls.References: https://docs.segmentapis.com/, https://docs.segmentapis.com/tag/Delivery-Overview/, and https://developer.calendly.com/api-docs
Trajectories
traj_mbzdbmgf9ppt—tier1-adapters-batch-4-workflow, completed, sourceworkflow-runner:0078a2c0ccfd387cbb127151, retrospective: all 25 steps completed, confidence 79%.traj_6ia16qeu2ido—tier1-verify-batch-4-workflow, completed/abandoned after aggregate drift, sourceworkflow-runner:d79bd56baa165709b31fcb73; drift artifacts identified Segment and Calendly endpoint fixes included in this PR.traj_7c6u9akui41f—Open PR for tier1 adapters batch 4 changes, completed, confidence 82%.Validation
Workflow evidence from run
0078a2c0ccfd387cbb127151:tier1-adapters-batch-4-workflowcompleted successfully.BATCH_4_OK adapters=airtable,segment,mixpanel,calendly.Local checks before PR/update:
npm run typecheck --workspace @relayfile/adapter-airtablenpm test --workspace @relayfile/adapter-airtable— 12 passednpm run typecheck --workspace @relayfile/adapter-segmentnpm test --workspace @relayfile/adapter-segment— 11 passed after the read-route fixnpm run typecheck --workspace @relayfile/adapter-mixpanelnpm test --workspace @relayfile/adapter-mixpanel— 12 passednpm run typecheck --workspace @relayfile/adapter-calendlynpm test --workspace @relayfile/adapter-calendly— 14 passed after the endpoint fixAGENTS.md— no outputgit diff --check— no outputRelease note
No existing package
package.jsonversions were changed; new adapter packages start at0.1.0and publishing remains handled by the release workflow.