Skip to content

feat: add SeaRates ocean tracking compatibility gateway - #331

Open
dodeja wants to merge 12 commits into
mainfrom
cursor/searates-compat-proxy-a51c
Open

feat: add SeaRates ocean tracking compatibility gateway#331
dodeja wants to merge 12 commits into
mainfrom
cursor/searates-compat-proxy-a51c

Conversation

@dodeja

@dodejadodeja commented Aug 20, 2026

Copy link
Copy Markdown
Member

Summary

  • add a dedicated apps/migrate Vercel application for vendor compatibility APIs, separate from the MCP deployment
  • expose SeaRates-compatible GET /searates-api/tracking, /container, /reference, and /info/sealines routes
  • translate SeaRates query auth and CT/BL/BK requests onto the public Terminal49 JSON:API using Authorization: Token
  • map ordered container timelines into SeaRates' closed event-code vocabulary, position-aware milestones, route anchors, equipment fields, and shipment statuses
  • return SeaRates-style HTTP 200 error envelopes, including NO_TRACKING_INFO while Terminal49 is unresolved or not found

Mapping behavior

  • event codes are limited to SeaRates' sixteen documented values or null; unmapped and availability rows are retained
  • first/later sea loads and departures, hub/final arrivals and discharges, cargo-state gate pairs, inland LTS, ancillary UNKN, and delay TSD are derived from the ordered timeline
  • duplicate records are folded without collapsing repeated LTS/UNKN movements or distinct calls at the same place
  • prepol, pol, pod, and postpod are derived from timeline anchors with documented fallbacks
  • CT/BK metadata preserves the requested identifier; CT lookups fetch and return only the requested shipment member
  • active T49 container states remain IN_TRANSIT; only delivery or empty return becomes DELIVERED
  • force_update intentionally returns API_KEY_RATE_LIMIT for more than 10 containers and never fires a parallel refresh burst

Deployment architecture

The root vercel.json remains MCP-only. Create a second Vercel project in the Terminal49 team from this repository with:

SettingValue
Root Directoryapps/migrate
Include source files outside the Root DirectoryEnabled
Framework PresetOther
Install Commandcd ../.. && npm ci
Build Commandcd ../.. && npm run build --workspace @terminal49/searates-compat && npm run build --workspace @terminal49/migrate-app
Node.js Version24

Configure migrate authentication variables in that project, not the MCP project. After a successful production deployment and once DNS is ready, add migrate.terminal49.com as its production domain. The custom domain is not currently live; previews use the Vercel preview hostname with the same /searates-api/... paths. CI does not create or configure the Vercel project.

Scope

Ocean tracking and the shipping-lines dictionary only. No rates, schedules, air, parcel, road, route geometry, AIS pins, history, or terminal dictionary. /info/terminals remains omitted because the public API has no terminal-list endpoint.

Verification

  • rebased onto current main at 22259f4; PR is mergeable and conflict-free
  • all workspace builds, lint, and type checks pass locally
  • 373 tests pass across MCP, SDK, CLI, and compatibility packages (1 existing SDK test skipped)
  • CI / migrate ran after the rebase and passed
  • mapper tests cover ordinal loads/arrivals/discharges, gate cargo state, uncoded rows, inland movement, duplicate records, truncated hub timelines, route anchors, status mapping, requested CT selection, sparse includes, timestamps, and equipment enums
  • gateway tests cover Token auth, typed tracking-request reuse, bounded upstream requests, refresh quota/freshness handling, NO_TRACKING_INFO, and /container//reference contracts
  • docs/migrate/searates.mdx now documents the compatibility gateway and native migration paths
  • tests use fixtures and do not call production APIs
Open in WebOpen in Cursor

View with [code]smithAutofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Greptile Summary

The PR adds a dedicated Vercel application and package that expose SeaRates-compatible ocean-tracking and shipping-line endpoints over Terminal49's public JSON:API.

  • Adds request authentication, CT/BL/BK lookup, asynchronous tracking-request polling, refresh handling, and SeaRates error envelopes.
  • Maps shipments, containers, routes, facilities, vessels, and transport events into the compatibility response.
  • Adds workspace configuration, deployment rewrites, CI coverage, fixtures, and tests for the new gateway.

Confidence Score: 2/5

The PR should not merge until tracking responses preserve the requested identifier and correctly represent valid container status and event provenance.

The new mapping layer returns observably incorrect compatibility data for current public API responses: active container states can become unknown, CT/BK metadata can contain a bill-of-lading number, and transport-event provenance is derived from an unavailable attribute.

Files Needing Attention: packages/searates-compat/src/mapping.ts, packages/searates-compat/src/fixtures/t49.ts

Important Files Changed

FilenameOverview
packages/searates-compat/src/mapping.tsImplements the core SeaRates projection but misclassifies valid container states, reports BOL identifiers for CT/BK requests, and relies on an undocumented event-provenance field.
packages/searates-compat/src/service.tsCoordinates authentication, shipment resolution, polling, refreshes, event fetching, and compatibility error translation.
packages/searates-compat/src/client.tsAdds the public JSON:API client and bounded tracking-request polling; request reuse by number and SCAC remains ambiguous but was not established as defective.
packages/searates-compat/src/http.tsAdds GET/OPTIONS handlers, query normalization, environment configuration, and SeaRates-style method/type errors.
apps/migrate/vercel.jsonDefines the isolated compatibility deployment, function durations, and vendor-prefixed rewrites without altering the root MCP routes.
.github/workflows/ci.ymlAdds build, type-check, test, and lint coverage for the compatibility package and migrate application.

Sequence Diagram

sequenceDiagram
participant C as SeaRates Client
participant V as Migrate Vercel Handler
participant G as Compatibility Gateway
participant T as Terminal49 API
C->>V: GET /searates-api/tracking
V->>G: tracking(api_key, query)
G->>T: Find existing shipment
alt Shipment exists
T-->>G: Shipment and containers
else Shipment missing
G->>T: Create or reuse tracking request
loop Bounded polling
G->>T: Read tracking request
T-->>G: Pending, failed, or shipment
end
end
G->>T: Fetch container transport events
T-->>G: JSON:API events
G-->>V: SeaRates-compatible envelope
V-->>C: HTTP 200 JSON
Loading

Fix all with GreploopFix All in CodexFix All in Claude Code

Prompt To Fix All With AI
### Issue 1
packages/searates-compat/src/mapping.ts:201-218
**Active statuses become unknown**
When a container has a valid active Terminal49 status such as `grounded`, `on_rail`, `off_dock`, `dropped`, or `loaded`, this function falls through to `UNKNOWN`, causing both the container and potentially the shipment metadata to report an indeterminate status despite active tracking.
### Issue 2
packages/searates-compat/src/mapping.ts:412-414
**Requested tracking number is replaced**
When a CT or BK request resolves to a shipment with a bill-of-lading number, this expression reports that BOL instead of the requested identifier, causing clients to correlate, display, or cache the response under the wrong tracking number.
```suggestion number: payload.requestedNumber,```### Issue 3
packages/searates-compat/src/mapping.ts:274
**Event provenance uses absent field**
Public transport-event responses do not provide the `data_source` attribute checked here, so real events are always emitted with `is_date_from_sealine: false`, producing incorrect provenance despite the fixture supplying the undocumented field.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "refactor: isolate migration APIs in dedi..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

Context used:

@vercel

vercelBot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
apiReadyReadyPreviewAug 21, 2026 8:07pm

Request Review

Comment on lines +201 to +218
isoCode: typeCode[type] ? `${first}${second}${typeCode[type]}` : null,
sizeType: `${length}'${heightLabel} ${label[type] || type}`,
};
}

function seaRatesStatus(value: unknown): string {
if (typeof value !== 'string') return 'UNKNOWN';
if (['delivered', 'empty_returned', 'picked_up'].includes(value)) {
return 'DELIVERED';
}
if (
[
'available',
'awaiting_inland_transfer',
'in_transit',
'not_available',
'on_ship',
].includes(value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1Active statuses become unknown

When a container has a valid active Terminal49 status such as grounded, on_rail, off_dock, dropped, or loaded, this function falls through to UNKNOWN, causing both the container and potentially the shipment metadata to report an indeterminate status despite active tracking.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/searates-compat/src/mapping.ts
Line: 201-218
Comment:
**Active statuses become unknown**
When a container has a valid active Terminal49 status such as `grounded`, `on_rail`, `off_dock`, `dropped`, or `loaded`, this function falls through to `UNKNOWN`, causing both the container and potentially the shipment metadata to report an indeterminate status despite active tracking.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in CodexFix in Claude Code

Comment on lines +412 to +414
number:
stringValue(shipmentAttributes.bill_of_lading_number) ||
payload.requestedNumber,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1Requested tracking number is replaced

When a CT or BK request resolves to a shipment with a bill-of-lading number, this expression reports that BOL instead of the requested identifier, causing clients to correlate, display, or cache the response under the wrong tracking number.

Suggested change
number:
stringValue(shipmentAttributes.bill_of_lading_number)||
payload.requestedNumber,
number: payload.requestedNumber,
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/searates-compat/src/mapping.ts
Line: 412-414
Comment:
**Requested tracking number is replaced**
When a CT or BK request resolves to a shipment with a bill-of-lading number, this expression reports that BOL instead of the requested identifier, causing clients to correlate, display, or cache the response under the wrong tracking number.
```suggestion number: payload.requestedNumber,```---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in CodexFix in Claude Code

status: mapping.status,
facility: facilityId ? (ids.facilities.get(facilityId) ?? null) : null,
is_additional_event: false,
is_date_from_sealine: attributes.data_source === 'shipping_line',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1Event provenance uses absent field

Public transport-event responses do not provide the data_source attribute checked here, so real events are always emitted with is_date_from_sealine: false, producing incorrect provenance despite the fixture supplying the undocumented field.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/searates-compat/src/mapping.ts
Line: 274
Comment:
**Event provenance uses absent field**
Public transport-event responses do not provide the `data_source` attribute checked here, so real events are always emitted with `is_date_from_sealine: false`, producing incorrect provenance despite the fixture supplying the undocumented field.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in CodexFix in Claude Code

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:ac228f8c01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

...init,
headers: {
Accept: 'application/vnd.api+json',
Authorization: `Bearer ${this.token}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Send API keys with the Token scheme

In both pass-through and service-token modes, the credential is a Terminal49 API key, but every upstream request is sent as Authorization: Bearer .... Terminal49 API keys require the Token scheme (the existing SDK only preserves Bearer for explicitly supplied OAuth tokens), so valid gateway credentials receive 401 responses and tracking is returned as API_KEY_WRONG. Use Token for these API-key-backed requests.

AGENTS.md reference: AGENTS.md:L27-L27

Useful? React with 👍 / 👎.

Comment on lines +212 to +218
[
'available',
'awaiting_inland_transfer',
'in_transit',
'not_available',
'on_ship',
].includes(value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Map active container statuses to IN_TRANSIT

When Terminal49 returns common active statuses such as grounded, on_rail, off_dock, or loaded, this predicate falls through to UNKNOWN; in_transit, meanwhile, is not one of the documented Terminal49 current_status values. Consequently both containers[].status and the aggregate metadata status become UNKNOWN during substantial portions of a shipment's journey instead of IN_TRANSIT.

AGENTS.md reference: AGENTS.md:L27-L27

Useful? React with 👍 / 👎.

Comment on lines +184 to +186
dry: 'G1',
flat_rack: 'P1',
hard_top: 'U1',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Match the API's equipment type values

For open-top and flat-rack containers, the public API returns equipment_type as "open top" and "flat rack", but this lookup only recognizes underscore variants. Those supported containers therefore get iso_code: null and an unnormalized size_type, despite the gateway's stated ISO reconstruction support; normalize the incoming value or key the maps by the schema-defined strings.

AGENTS.md reference: AGENTS.md:L27-L27

Useful? React with 👍 / 👎.

@vorfluxvorfluxBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Reviewed — found 9 issues. This PR adds an isolated Vercel migration app that exposes SeaRates-compatible tracking and shipping-line endpoints backed by Terminal49’s JSON:API. I reviewed workspace and deployment configuration, request authentication, tracking-request resolution and refresh behavior, response mappings, error handling, and unit coverage.

Findings

packages/searates-compat/src/client.ts

  1. API-key requests use the wrong authorization scheme.
  2. Tracking-request reuse can select an incompatible or failed historical request.
  3. Upstream requests have no timeout or abort signal.

packages/searates-compat/src/service.ts

  1. force_update exceeds the refresh rate limit for shipments with more than ten containers.
  2. Post-refresh responses retain stale shipment attributes.

packages/searates-compat/src/mapping.ts

  1. Valid journey states map to incorrect SeaRates statuses.
  2. CT and BK responses can report the bill of lading as the requested number.
  3. Offset timestamps violate the promised SeaRates timestamp shape.
  4. Public equipment values produce missing or invalid ISO codes.

Verdict

⚠️Changes requested. The authentication failure blocks valid production requests, while the request-selection, refresh, and mapping issues can return failed, stale, or incorrectly identified tracking data.


Review with Vorflux

...init,
headers: {
Accept: 'application/vnd.api+json',
Authorization: `Bearer ${this.token}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — use the API-key authorization scheme. This client receives Terminal49 API keys, but sends them as Authorization: Bearer .... Terminal49 API keys require the Token scheme; Bearer is reserved for user-scoped JWTs and requires account context. As written, valid pass-through keys and T49_SEARATES_API_TOKEN credentials will receive 401 responses. Send API keys as Authorization: Token ... and update the header assertion in the tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. Terminal49 API-key requests now send Authorization: Token ...; request-shaping tests assert the scheme.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: requests now use Authorization: Token ..., and the request-shaping coverage asserts it. This finding is resolved.

Comment on lines +194 to +200
const params = new URLSearchParams({
'filter[request_number]': number,
include: 'tracked_object',
'page[size]': '1',
});
if (scac) params.set('filter[scac]', scac);
return this.request(`/tracking_requests?${params.toString()}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — do not reuse an arbitrary historical request. This query returns only one record for the number and optional SCAC, without matching request_type or selecting by status and recency. Multiple requests can exist for the same number, including different request types and failed attempts followed by successful ones, so a BL lookup can reuse a BK/CT request or an old failure and then never create a valid replacement. Retrieve enough candidates to enforce the requested type, prefer a tracked or active request, and create a new request when only failed candidates exist.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. Reuse now fetches up to 30 candidates, matches request_type, excludes failed requests, and prefers a tracked object before recency. Tests cover ignoring failed and mismatched-type history.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: reuse now filters by request_type, excludes failed requests, fetches up to 30 candidates, and prioritizes a tracked object before recency. This finding is resolved.

Comment on lines +203 to +214
private async request(
path: string,
init: RequestInit = {},
): Promise<JsonApiDocument> {
const response = await this.fetchImpl(`${this.baseUrl}${path}`, {
...init,
headers: {
Accept: 'application/vnd.api+json',
Authorization: `Bearer ${this.token}`,
...init.headers,
},
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — bound upstream requests with an abort timeout. The polling loop has a nominal deadline, but each fetch can stall beyond that deadline and eventually be terminated by Vercel. That bypasses the gateway's SeaRates error envelope and can affect shipment, event, shipping-line, and polling calls. Add an AbortSignal timeout bounded by the remaining polling/function budget and translate timeout failures to SEALINE_NO_RESPONSE.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. Every upstream fetch now has an AbortSignal timeout, and polling requests are additionally bounded by the remaining poll deadline. Timeout failures map through the SeaRates error envelope.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The per-call abort handling is an improvement, but this remains partially unresolved. The timeout restarts for each upstream call, so a force_update request can sequentially spend up to 10 seconds on each of ten refreshes before polling and event retrieval, exceeding the Vercel function’s 30-second duration. Please carry one operation-level deadline or abort signal through the full gateway request and cap every call and wait by its remaining budget.

Comment on lines +124 to +127
if (query.forceUpdate && containers.length > 0) {
await Promise.all(
containers.map((container) => client.refreshContainer(container.id)),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — throttle force_update refreshes. Container refreshes are limited to 10 requests per minute, but Promise.all submits every refresh concurrently. Any shipment with more than ten containers can therefore trigger 429 responses, fail the entire otherwise-valid lookup after partially scheduling refreshes, and immediately consume the shared quota in service-token mode. Queue or throttle these calls to the documented limit and define partial-failure behavior.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed with the intentional compatibility bound: requests with more than 10 containers return API_KEY_RATE_LIMIT before scheduling any refresh. Up to 10 refreshes run sequentially, avoiding the previous parallel burst.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The per-invocation guard prevents one request from bursting above ten refreshes, but this remains partially unresolved in service-token mode. The documented quota applies to the shared credential, so concurrent invocations can each pass this guard, partially schedule refreshes, and then receive 429. Please enforce the budget per credential across invocations, or implement explicit best-effort behavior that does not fail the whole tracking response after partial side effects.

Comment on lines +128 to +146
shipmentDocument = await client.shipment(shipment.id);
containers = containerResources(shipmentDocument);
}

const eventsByContainerId = new Map<string, JsonApiDocument>();
await Promise.all(
containers.map(async (container) => {
eventsByContainerId.set(
container.id,
await client.transportEvents(container.id),
);
}),
);
const payload: TrackingPayload = {
eventsByContainerId,
included: shipmentDocument.included || [],
requestedNumber: query.number,
requestedType: type,
shipment,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — replace the shipment resource after refresh. After force_update, this fetches a new shipmentDocument and replaces containers, but the payload still references the pre-refresh shipment. Updated route dates, vessel or carrier metadata, and tracking timestamps from the second response are discarded while newer included containers are used. Re-extract and validate the shipment resource from the refreshed document before constructing TrackingPayload.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: the refreshed document now replaces shipmentDocument, and shipment is re-extracted and validated before payload construction. This finding is resolved.

Comment on lines +206 to +222
function seaRatesStatus(value: unknown): string {
if (typeof value !== 'string') return 'UNKNOWN';
if (['delivered', 'empty_returned', 'picked_up'].includes(value)) {
return 'DELIVERED';
}
if (
[
'available',
'awaiting_inland_transfer',
'in_transit',
'not_available',
'on_ship',
].includes(value)
) {
return 'IN_TRANSIT';
}
return 'UNKNOWN';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — map all public journey states correctly.picked_up is classified as DELIVERED even though it represents transport from the terminal to the warehouse, while active states such as grounded, on_rail, off_dock, loaded, and dropped fall through to UNKNOWN; new also lacks a planned-state mapping. This makes container and aggregate shipment status incorrect for common journeys. Keep pickup and active terminal/movement states in transit, reserve delivery for actual delivery or empty return, and explicitly map the pre-departure state.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. picked_up, grounded, on_rail, off_dock, loaded, dropped, and the other active states map to IN_TRANSIT; new maps to PLANNED; only delivery or empty return maps to DELIVERED.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: active states, including picked_up, map to IN_TRANSIT; new maps to PLANNED; and only delivery/empty return map to DELIVERED. This finding is resolved.

Comment on lines +411 to +414
type: payload.requestedType,
number:
stringValue(shipmentAttributes.bill_of_lading_number) ||
payload.requestedNumber,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — preserve the identifier associated with metadata.type. For type=CT or type=BK, this replaces the submitted identifier with bill_of_lading_number, producing metadata whose type and number refer to different identifiers. Clients can then mis-correlate or mis-cache the response. Preserve payload.requestedNumber, or choose the identifier corresponding to requestedType.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. metadata.number now always echoes payload.requestedNumber, preserving CT/BK correlation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: metadata.number now uses payload.requestedNumber, preserving correlation with metadata.type. This finding is resolved.

Comment on lines +153 to +155
function formatDate(value: unknown): string | null {
if (typeof value !== 'string' || !value) return null;
return value.replace('T', ' ').replace(/(?:\.\d+)?Z$/, '');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — normalize offset timestamps to UTC. This only strips a trailing Z; a valid value such as 2020-04-04T16:13:35-07:00 becomes 2020-04-04 16:13:35-07:00, which does not match the documented YYYY-MM-DD HH:MM:SS response shape and is not UTC. Parse valid ISO-8601 values, normalize to UTC, format consistently, and add coverage for both Z and non-zero offsets.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The output shape is fixed, but this remains partially unresolved against the documented UTC contract. For example, 2026-08-01T10:00:00-07:00 is emitted as 2026-08-01 10:00:00 rather than the equivalent UTC value 2026-08-01 17:00:00; supplying an IANA timezone similarly localizes the instant. Please normalize parsed values through UTC, or explicitly change the documented compatibility contract and tests if local SeaRates wall-clock time is intentional.

Comment on lines +181 to +198
const first = length === 20 ? '2' : length === 40 ? '4' : 'L';
const second = height === 'high_cube' ? '5' : '2';
const typeCode: Record<string, string> = {
dry: 'G1',
flat_rack: 'P1',
hard_top: 'U1',
open_top: 'U1',
reefer: 'R1',
tank: 'T1',
};
const label: Record<string, string> = {
dry: 'Dry',
flat_rack: 'Flat Rack',
hard_top: 'Hard Top',
open_top: 'Open Top',
reefer: 'Reefer',
tank: 'Tank',
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — handle the public equipment values and lengths. The Terminal49 schema emits equipment types such as open top and flat rack, while this table only recognizes underscore variants, yielding iso_code: null and an unnormalized size_type. The fallback length prefix L is also only appropriate for 45-foot equipment, not supported 10-foot containers, whose ISO length character is 1. Map the actual enum values and handle each supported length explicitly.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. Equipment types normalize the public space-separated values (open top, flat rack), and length codes explicitly cover 10/20/40/45 feet.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at the current head: equipment names normalize the public space-separated variants, and length codes explicitly cover 10, 20, 40, and 45 feet. This finding is resolved.

cursoragentand others added 12 commits August 21, 2026 20:04
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>
Co-authored-by: Akshay Dodeja <dodeja@gmail.com>

@vorfluxvorfluxBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Reviewed — found 6 issues. I re-reviewed the complete current SeaRates compatibility gateway diff at head 5d06966507f7766e8a4df79256f06f21c13a4fe7, including the nine prior fixes, gateway lifecycle, refresh handling, response mapping, deployment limits, documentation, and tests. Six prior findings are resolved; timeout, refresh-quota, and timestamp handling remain partially unresolved, and the updated implementation has three additional data/route correctness issues.

Findings

packages/searates-compat/src/client.ts

  1. Per-call timeouts do not bound the complete gateway operation.
  2. Terminal and rail refreshes are not recognized by refresh polling.

packages/searates-compat/src/service.ts

  1. Concurrent requests can still exceed the shared refresh quota.

packages/searates-compat/src/mapping.ts

  1. All event dates are labeled as shipping-line data.
  2. postpod ignores the shipment’s inland destination.
  3. Offset timestamps are not normalized to the documented UTC contract.

Verdict

⚠️Changes requested. The remaining lifecycle and quota issues can still cause platform timeouts or partial refresh failures, while the mapping issues can misrepresent event provenance, destination, and timestamp instants.


Review with Vorflux

Comment on lines +265 to +275
private async request(
path: string,
init: RequestInit = {},
timeoutMs = this.requestTimeoutMs,
): Promise<JsonApiDocument> {
const timeoutSignal = AbortSignal.timeout(Math.max(1, timeoutMs));
const response = await this.fetchImpl(`${this.baseUrl}${path}`, {
...init,
signal: init.signal
? AbortSignal.any([init.signal, timeoutSignal])
: timeoutSignal,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — enforce one deadline for the complete gateway operation. This timeout restarts for every upstream request. A force_update lookup can sequentially await up to ten refresh calls at 10 seconds each, then poll and fetch events, while the Vercel handler has a 30-second maximum duration. Three slow refreshes can therefore exhaust the function before the gateway emits SEALINE_NO_RESPONSE. Carry a shared operation deadline or abort signal through the full request and cap every upstream call and polling wait by its remaining budget.

Comment on lines +71 to +78
function freshnessSignature(document: JsonApiDocument): string {
const shipment = Array.isArray(document.data)
? document.data.find((resource) => resource.type === 'shipment')
: document.data?.type === 'shipment'
? document.data
: undefined;
return String(shipment?.attributes?.line_tracking_last_succeeded_at || '');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — recognize non-carrier refresh results.PATCH /containers/{id}/refresh refreshes shipping-line, terminal, and rail sources, but this signature only watches line_tracking_last_succeeded_at. If terminal or rail data updates while the carrier timestamp is unchanged, the refreshed attributes and events are available but polling times out and the gateway returns NO_TRACKING_INFO. Include relevant container/source freshness markers or compare the refreshed fields used by the response.

Comment on lines +152 to +158
if (query.forceUpdate && containers.length > 0) {
if (containers.length > 10) {
return errorEnvelope('API_KEY_RATE_LIMIT');
}
for (const container of containers) {
await client.refreshContainer(container.id);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — enforce the refresh quota per credential. The ten-container guard is scoped to one invocation, but the 10-per-minute quota applies to the API credential. In service-token mode, concurrent requests share that credential and can each pass this guard, partially schedule refreshes, and then receive 429. Enforce the budget per credential across invocations, or implement explicit best-effort behavior that does not fail the complete tracking response after partial side effects.

Comment on lines +673 to +683
function publicEvent(event: EventDraft, order: number): SeaRatesEvent {
return {
actual: event.actual,
date: event.date,
description: event.description,
event_code: event.code,
event_type: event.eventType,
facility: event.facility,
is_additional_event: false,
is_date_from_sealine: true,
location: event.location,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — preserve event provenance. Every event is emitted with is_date_from_sealine: true, but Terminal49 transport events can have attributes.data_source values such as shipping_line, terminal, or ais. Terminal and AIS milestones are therefore falsely represented as carrier-provided. Preserve data_source in EventDraft and set this field from attributes.data_source === 'shipping_line'.

Comment on lines +822 to +845
const polLocationId = relatedId(payload.shipment, 'port_of_lading');
const podLocationId = relatedId(payload.shipment, 'port_of_discharge');
const fallbackPol = {
actual: Boolean(shipmentAttributes.pol_atd_at),
date: formatDate(
shipmentAttributes.pol_atd_at || shipmentAttributes.pol_etd_at,
stringValue(shipmentAttributes.pol_timezone),
),
location: polLocationId ? (locations.get(polLocationId) ?? null) : null,
};
const fallbackPod = {
actual: Boolean(shipmentAttributes.pod_ata_at),
date: formatDate(
shipmentAttributes.pod_ata_at || shipmentAttributes.pod_eta_at,
stringValue(shipmentAttributes.pod_timezone),
),
location: podLocationId ? (locations.get(podLocationId) ?? null) : null,
};
const pol = polEvent ? routePoint(polEvent) : fallbackPol;
const pod = podEvent ? routePoint(podEvent) : fallbackPod;
const prepol = prepolEvent
? routePoint(prepolEvent)
: { actual: null, date: null, location: pol.location };
const postpod = postpodEvent ? routePoint(postpodEvent) : { ...pod };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — use the inland destination for the postpod fallback. When no post-POD event exists, this clones the discharge port even though the gateway includes shipment.relationships.destination and the migration contract maps postpod to that inland destination. Inland shipments without arrived inland events therefore report POD as their final destination. Build the fallback from the destination relationship and destination_ata_at/destination_eta_at/destination_timezone, falling back to POD only when no destination is present.

Comment on lines +92 to +107
function formatDate(value: unknown, timeZone?: string | null): string | null {
if (typeof value !== 'string' || !value) return null;
const parsed = new Date(value);
if (Number.isNaN(parsed.getTime())) {
const match = value.match(/^(\d{4}-\d{2}-\d{2})[T ](\d{2}:\d{2}:\d{2})/);
return match ? `${match[1]} ${match[2]}` : null;
}
if (timeZone) {
const local = formatParts(parsed, timeZone);
if (local) return local;
}
const offsetMatch = value.match(
/^(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2})(?:\.\d+)?[+-]\d{2}:\d{2}$/,
);
if (offsetMatch) return `${offsetMatch[1]} ${offsetMatch[2]}`;
return parsed.toISOString().slice(0, 19).replace('T', ' ');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue — normalize offsets to the documented UTC contract. The output shape is now correct, but this localizes timestamps or preserves offset wall-clock components. For example, 2026-08-01T10:00:00-07:00 represents 17:00:00Z but is returned as 2026-08-01 10:00:00, contradicting the package’s UTC claim and shifting the instant for consumers. Normalize parsed timestamps through UTC, or explicitly change the documented compatibility contract and tests if local SeaRates wall-clock time is intended.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dodeja@cursoragent