From f33394648f70bd1976cca02377fbbfae76534346 Mon Sep 17 00:00:00 2001 From: Sebastien Melki Date: Wed, 22 Apr 2026 10:21:42 +0300 Subject: [PATCH] fix(shipping/v2): alertThreshold: 0 preserved; drop dead validation branch (#3242 followup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: alert_threshold was a plain int32. proto3 scalar default is 0, so the handler couldn't distinguish "partner explicitly sent 0 (deliver every disruption)" from "partner omitted the field (apply legacy default 50)" — both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop for any partner who wanted every alert. The subsequent `alertThreshold < 0` branch was also unreachable after that coercion. After: - Proto field is `optional int32 alert_threshold` — TS type becomes `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0. - Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number passes through unchanged. - Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0, int32.lte = 100` already enforces the range at the wire layer. Partner wire contract: identical for the omit-field and 1..100 cases. Only behavioural change is explicit 0 — previously impossible to request, now honored per proto3 optional semantics. Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full- regen `@ts-nocheck` drift Seb documented in the #3242 PR comments. Re-applied `@ts-nocheck` on the two regenerated files manually. Tests: - `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`. - New test: `alertThreshold omitted (undefined) applies legacy default 50`. - `rejects > 100` test removed — proto/wire validation handles it; direct handler calls intentionally bypass wire and the handler no longer carries a redundant runtime range check. Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api clean, all 4 custom lints clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/api/ShippingV2Service.openapi.json | 2 +- docs/api/ShippingV2Service.openapi.yaml | 7 +++- .../shipping/v2/register_webhook.proto | 6 +++- .../shipping/v2/register-webhook.ts | 15 ++++---- .../shipping/v2/service_client.ts | 2 +- .../shipping/v2/service_server.ts | 2 +- tests/shipping-v2-handler.test.mjs | 36 ++++++++++++------- 7 files changed, 45 insertions(+), 25 deletions(-) diff --git a/docs/api/ShippingV2Service.openapi.json b/docs/api/ShippingV2Service.openapi.json index ef7238608..45d531e6b 100644 --- a/docs/api/ShippingV2Service.openapi.json +++ b/docs/api/ShippingV2Service.openapi.json @@ -1 +1 @@ -{"components":{"schemas":{"BypassOption":{"description":"Single bypass-corridor option around a disrupted chokepoint.","properties":{"activationThreshold":{"description":"Enum-like string, e.g., \"DISRUPTION_SCORE_60\".","type":"string"},"addedCostMultiplier":{"format":"double","type":"number"},"addedTransitDays":{"format":"int32","type":"integer"},"id":{"type":"string"},"name":{"type":"string"},"type":{"description":"Type of bypass (e.g., \"maritime_detour\", \"land_corridor\").","type":"string"}},"type":"object"},"ChokepointExposure":{"description":"Single chokepoint exposure for a route.","properties":{"chokepointId":{"type":"string"},"chokepointName":{"type":"string"},"exposurePct":{"format":"int32","type":"integer"}},"type":"object"},"Error":{"description":"Error is returned when a handler encounters an error. It contains a simple error message that the developer can customize.","properties":{"message":{"description":"Error message (e.g., 'user not found', 'database connection failed')","type":"string"}},"type":"object"},"FieldViolation":{"description":"FieldViolation describes a single validation error for a specific field.","properties":{"description":{"description":"Human-readable description of the validation violation (e.g., 'must be a valid email address', 'required field missing')","type":"string"},"field":{"description":"The field path that failed validation (e.g., 'user.email' for nested fields). For header validation, this will be the header name (e.g., 'X-API-Key')","type":"string"}},"required":["field","description"],"type":"object"},"ListWebhooksRequest":{"description":"ListWebhooksRequest has no fields — the owner is derived from the caller's\n API-key fingerprint (SHA-256 of X-WorldMonitor-Key).","type":"object"},"ListWebhooksResponse":{"description":"ListWebhooksResponse wire shape preserved exactly: the `webhooks` field\n name and the omission of `secret` are part of the partner contract.","properties":{"webhooks":{"items":{"$ref":"#/components/schemas/WebhookSummary"},"type":"array"}},"type":"object"},"RegisterWebhookRequest":{"description":"RegisterWebhookRequest creates a new chokepoint-disruption webhook\n subscription. Wire shape is byte-compatible with the pre-migration\n legacy POST body.","properties":{"alertThreshold":{"description":"Disruption-score threshold for delivery, 0-100. Default 50.","format":"int32","maximum":100,"minimum":0,"type":"integer"},"callbackUrl":{"description":"HTTPS callback URL. Must not resolve to a private/loopback address at\n registration time (SSRF guard). The delivery worker re-validates the\n resolved IP before each send to mitigate DNS rebinding.","maxLength":2048,"minLength":8,"type":"string"},"chokepointIds":{"items":{"description":"Zero or more chokepoint IDs to subscribe to. Empty list subscribes to\n the entire CHOKEPOINT_REGISTRY. Unknown IDs fail with 400.","type":"string"},"type":"array"}},"required":["callbackUrl"],"type":"object"},"RegisterWebhookResponse":{"description":"RegisterWebhookResponse wire shape preserved exactly — partners persist the\n `secret` because the server never returns it again except via rotate-secret.","properties":{"secret":{"description":"Raw 64-char lowercase hex secret (32 random bytes). No `whsec_` prefix.","type":"string"},"subscriberId":{"description":"`wh_` prefix + 24 lowercase hex chars (12 random bytes).","type":"string"}},"type":"object"},"RouteIntelligenceRequest":{"description":"RouteIntelligenceRequest scopes a route-intelligence query by origin and\n destination country. Query-parameter names are preserved verbatim from the\n legacy partner contract (fromIso2/toIso2/cargoType/hs2 — camelCase).","properties":{"cargoType":{"description":"Cargo type — one of: container (default), tanker, bulk, roro.\n Empty string defers to the server default. Unknown values are coerced to\n \"container\" to preserve legacy behavior.","type":"string"},"fromIso2":{"description":"Origin country, ISO-3166-1 alpha-2 uppercase.","pattern":"^[A-Z]{2}$","type":"string"},"hs2":{"description":"2-digit HS commodity code (default \"27\" — mineral fuels). Non-digit\n characters are stripped server-side to match legacy behavior.","type":"string"},"toIso2":{"description":"Destination country, ISO-3166-1 alpha-2 uppercase.","pattern":"^[A-Z]{2}$","type":"string"}},"required":["fromIso2","toIso2"],"type":"object"},"RouteIntelligenceResponse":{"description":"RouteIntelligenceResponse wire shape preserved byte-for-byte from the\n pre-migration JSON at docs/api-shipping-v2.mdx. `fetched_at` is intentionally\n a string (ISO-8601) rather than int64 epoch ms because partners depend on\n the ISO-8601 shape.","properties":{"bypassOptions":{"items":{"$ref":"#/components/schemas/BypassOption"},"type":"array"},"cargoType":{"type":"string"},"chokepointExposures":{"items":{"$ref":"#/components/schemas/ChokepointExposure"},"type":"array"},"disruptionScore":{"description":"Disruption score of the primary chokepoint, 0-100.","format":"int32","type":"integer"},"fetchedAt":{"description":"ISO-8601 timestamp of when the response was assembled.","type":"string"},"fromIso2":{"type":"string"},"hs2":{"type":"string"},"primaryRouteId":{"type":"string"},"toIso2":{"type":"string"},"warRiskTier":{"description":"War-risk tier enum string, e.g., \"WAR_RISK_TIER_NORMAL\" or \"WAR_RISK_TIER_ELEVATED\".","type":"string"}},"type":"object"},"ValidationError":{"description":"ValidationError is returned when request validation fails. It contains a list of field violations describing what went wrong.","properties":{"violations":{"description":"List of validation violations","items":{"$ref":"#/components/schemas/FieldViolation"},"type":"array"}},"required":["violations"],"type":"object"},"WebhookSummary":{"description":"Single webhook record in the list response. `secret` is intentionally\n omitted; use rotate-secret to obtain a new one.","properties":{"active":{"type":"boolean"},"alertThreshold":{"format":"int32","type":"integer"},"callbackUrl":{"type":"string"},"chokepointIds":{"items":{"type":"string"},"type":"array"},"createdAt":{"description":"ISO-8601 timestamp of registration.","type":"string"},"subscriberId":{"type":"string"}},"type":"object"}}},"info":{"title":"ShippingV2Service API","version":"1.0.0"},"openapi":"3.1.0","paths":{"/api/v2/shipping/route-intelligence":{"get":{"description":"RouteIntelligence scores a country-pair trade route for chokepoint exposure\n and current disruption risk. Partner-facing; wire shape is byte-compatible\n with the pre-migration JSON response documented at docs/api-shipping-v2.mdx.","operationId":"RouteIntelligence","parameters":[{"description":"Origin country, ISO-3166-1 alpha-2 uppercase.","in":"query","name":"fromIso2","required":false,"schema":{"type":"string"}},{"description":"Destination country, ISO-3166-1 alpha-2 uppercase.","in":"query","name":"toIso2","required":false,"schema":{"type":"string"}},{"description":"Cargo type — one of: container (default), tanker, bulk, roro.\n Empty string defers to the server default. Unknown values are coerced to\n \"container\" to preserve legacy behavior.","in":"query","name":"cargoType","required":false,"schema":{"type":"string"}},{"description":"2-digit HS commodity code (default \"27\" — mineral fuels). Non-digit\n characters are stripped server-side to match legacy behavior.","in":"query","name":"hs2","required":false,"schema":{"type":"string"}}],"responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RouteIntelligenceResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"RouteIntelligence","tags":["ShippingV2Service"]}},"/api/v2/shipping/webhooks":{"get":{"description":"ListWebhooks returns the caller's registered webhooks filtered by the\n SHA-256 owner tag of the calling API key. The `secret` is intentionally\n omitted from the response; use rotate-secret to obtain a new one.","operationId":"ListWebhooks","responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ListWebhooksResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"ListWebhooks","tags":["ShippingV2Service"]},"post":{"description":"RegisterWebhook subscribes a callback URL to chokepoint disruption alerts.\n Returns the subscriberId and the raw HMAC secret — the secret is never\n returned again except via rotate-secret.","operationId":"RegisterWebhook","requestBody":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RegisterWebhookRequest"}}},"required":true},"responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RegisterWebhookResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"RegisterWebhook","tags":["ShippingV2Service"]}}}} \ No newline at end of file +{"components":{"schemas":{"BypassOption":{"description":"Single bypass-corridor option around a disrupted chokepoint.","properties":{"activationThreshold":{"description":"Enum-like string, e.g., \"DISRUPTION_SCORE_60\".","type":"string"},"addedCostMultiplier":{"format":"double","type":"number"},"addedTransitDays":{"format":"int32","type":"integer"},"id":{"type":"string"},"name":{"type":"string"},"type":{"description":"Type of bypass (e.g., \"maritime_detour\", \"land_corridor\").","type":"string"}},"type":"object"},"ChokepointExposure":{"description":"Single chokepoint exposure for a route.","properties":{"chokepointId":{"type":"string"},"chokepointName":{"type":"string"},"exposurePct":{"format":"int32","type":"integer"}},"type":"object"},"Error":{"description":"Error is returned when a handler encounters an error. It contains a simple error message that the developer can customize.","properties":{"message":{"description":"Error message (e.g., 'user not found', 'database connection failed')","type":"string"}},"type":"object"},"FieldViolation":{"description":"FieldViolation describes a single validation error for a specific field.","properties":{"description":{"description":"Human-readable description of the validation violation (e.g., 'must be a valid email address', 'required field missing')","type":"string"},"field":{"description":"The field path that failed validation (e.g., 'user.email' for nested fields). For header validation, this will be the header name (e.g., 'X-API-Key')","type":"string"}},"required":["field","description"],"type":"object"},"ListWebhooksRequest":{"description":"ListWebhooksRequest has no fields — the owner is derived from the caller's\n API-key fingerprint (SHA-256 of X-WorldMonitor-Key).","type":"object"},"ListWebhooksResponse":{"description":"ListWebhooksResponse wire shape preserved exactly: the `webhooks` field\n name and the omission of `secret` are part of the partner contract.","properties":{"webhooks":{"items":{"$ref":"#/components/schemas/WebhookSummary"},"type":"array"}},"type":"object"},"RegisterWebhookRequest":{"description":"RegisterWebhookRequest creates a new chokepoint-disruption webhook\n subscription. Wire shape is byte-compatible with the pre-migration\n legacy POST body.","properties":{"alertThreshold":{"description":"Disruption-score threshold for delivery, 0-100. Default 50.\n proto3 `optional` so the handler can distinguish \"partner explicitly sent\n 0 (deliver every alert)\" from \"partner omitted the field (apply default\n 50)\". Without `optional`, both serialise to the proto3 scalar default of\n 0 and the handler can't tell them apart — flagged in #3242 review.","format":"int32","maximum":100,"minimum":0,"type":"integer"},"callbackUrl":{"description":"HTTPS callback URL. Must not resolve to a private/loopback address at\n registration time (SSRF guard). The delivery worker re-validates the\n resolved IP before each send to mitigate DNS rebinding.","maxLength":2048,"minLength":8,"type":"string"},"chokepointIds":{"items":{"description":"Zero or more chokepoint IDs to subscribe to. Empty list subscribes to\n the entire CHOKEPOINT_REGISTRY. Unknown IDs fail with 400.","type":"string"},"type":"array"}},"required":["callbackUrl"],"type":"object"},"RegisterWebhookResponse":{"description":"RegisterWebhookResponse wire shape preserved exactly — partners persist the\n `secret` because the server never returns it again except via rotate-secret.","properties":{"secret":{"description":"Raw 64-char lowercase hex secret (32 random bytes). No `whsec_` prefix.","type":"string"},"subscriberId":{"description":"`wh_` prefix + 24 lowercase hex chars (12 random bytes).","type":"string"}},"type":"object"},"RouteIntelligenceRequest":{"description":"RouteIntelligenceRequest scopes a route-intelligence query by origin and\n destination country. Query-parameter names are preserved verbatim from the\n legacy partner contract (fromIso2/toIso2/cargoType/hs2 — camelCase).","properties":{"cargoType":{"description":"Cargo type — one of: container (default), tanker, bulk, roro.\n Empty string defers to the server default. Unknown values are coerced to\n \"container\" to preserve legacy behavior.","type":"string"},"fromIso2":{"description":"Origin country, ISO-3166-1 alpha-2 uppercase.","pattern":"^[A-Z]{2}$","type":"string"},"hs2":{"description":"2-digit HS commodity code (default \"27\" — mineral fuels). Non-digit\n characters are stripped server-side to match legacy behavior.","type":"string"},"toIso2":{"description":"Destination country, ISO-3166-1 alpha-2 uppercase.","pattern":"^[A-Z]{2}$","type":"string"}},"required":["fromIso2","toIso2"],"type":"object"},"RouteIntelligenceResponse":{"description":"RouteIntelligenceResponse wire shape preserved byte-for-byte from the\n pre-migration JSON at docs/api-shipping-v2.mdx. `fetched_at` is intentionally\n a string (ISO-8601) rather than int64 epoch ms because partners depend on\n the ISO-8601 shape.","properties":{"bypassOptions":{"items":{"$ref":"#/components/schemas/BypassOption"},"type":"array"},"cargoType":{"type":"string"},"chokepointExposures":{"items":{"$ref":"#/components/schemas/ChokepointExposure"},"type":"array"},"disruptionScore":{"description":"Disruption score of the primary chokepoint, 0-100.","format":"int32","type":"integer"},"fetchedAt":{"description":"ISO-8601 timestamp of when the response was assembled.","type":"string"},"fromIso2":{"type":"string"},"hs2":{"type":"string"},"primaryRouteId":{"type":"string"},"toIso2":{"type":"string"},"warRiskTier":{"description":"War-risk tier enum string, e.g., \"WAR_RISK_TIER_NORMAL\" or \"WAR_RISK_TIER_ELEVATED\".","type":"string"}},"type":"object"},"ValidationError":{"description":"ValidationError is returned when request validation fails. It contains a list of field violations describing what went wrong.","properties":{"violations":{"description":"List of validation violations","items":{"$ref":"#/components/schemas/FieldViolation"},"type":"array"}},"required":["violations"],"type":"object"},"WebhookSummary":{"description":"Single webhook record in the list response. `secret` is intentionally\n omitted; use rotate-secret to obtain a new one.","properties":{"active":{"type":"boolean"},"alertThreshold":{"format":"int32","type":"integer"},"callbackUrl":{"type":"string"},"chokepointIds":{"items":{"type":"string"},"type":"array"},"createdAt":{"description":"ISO-8601 timestamp of registration.","type":"string"},"subscriberId":{"type":"string"}},"type":"object"}}},"info":{"title":"ShippingV2Service API","version":"1.0.0"},"openapi":"3.1.0","paths":{"/api/v2/shipping/route-intelligence":{"get":{"description":"RouteIntelligence scores a country-pair trade route for chokepoint exposure\n and current disruption risk. Partner-facing; wire shape is byte-compatible\n with the pre-migration JSON response documented at docs/api-shipping-v2.mdx.","operationId":"RouteIntelligence","parameters":[{"description":"Origin country, ISO-3166-1 alpha-2 uppercase.","in":"query","name":"fromIso2","required":false,"schema":{"type":"string"}},{"description":"Destination country, ISO-3166-1 alpha-2 uppercase.","in":"query","name":"toIso2","required":false,"schema":{"type":"string"}},{"description":"Cargo type — one of: container (default), tanker, bulk, roro.\n Empty string defers to the server default. Unknown values are coerced to\n \"container\" to preserve legacy behavior.","in":"query","name":"cargoType","required":false,"schema":{"type":"string"}},{"description":"2-digit HS commodity code (default \"27\" — mineral fuels). Non-digit\n characters are stripped server-side to match legacy behavior.","in":"query","name":"hs2","required":false,"schema":{"type":"string"}}],"responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RouteIntelligenceResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"RouteIntelligence","tags":["ShippingV2Service"]}},"/api/v2/shipping/webhooks":{"get":{"description":"ListWebhooks returns the caller's registered webhooks filtered by the\n SHA-256 owner tag of the calling API key. The `secret` is intentionally\n omitted from the response; use rotate-secret to obtain a new one.","operationId":"ListWebhooks","responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ListWebhooksResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"ListWebhooks","tags":["ShippingV2Service"]},"post":{"description":"RegisterWebhook subscribes a callback URL to chokepoint disruption alerts.\n Returns the subscriberId and the raw HMAC secret — the secret is never\n returned again except via rotate-secret.","operationId":"RegisterWebhook","requestBody":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RegisterWebhookRequest"}}},"required":true},"responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/RegisterWebhookResponse"}}},"description":"Successful response"},"400":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/ValidationError"}}},"description":"Validation error"},"default":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Error"}}},"description":"Error response"}},"summary":"RegisterWebhook","tags":["ShippingV2Service"]}}}} \ No newline at end of file diff --git a/docs/api/ShippingV2Service.openapi.yaml b/docs/api/ShippingV2Service.openapi.yaml index 79fa11136..f42cefe27 100644 --- a/docs/api/ShippingV2Service.openapi.yaml +++ b/docs/api/ShippingV2Service.openapi.yaml @@ -277,7 +277,12 @@ components: maximum: 100 minimum: 0 format: int32 - description: Disruption-score threshold for delivery, 0-100. Default 50. + description: |- + Disruption-score threshold for delivery, 0-100. Default 50. + proto3 `optional` so the handler can distinguish "partner explicitly sent + 0 (deliver every alert)" from "partner omitted the field (apply default + 50)". Without `optional`, both serialise to the proto3 scalar default of + 0 and the handler can't tell them apart — flagged in #3242 review. required: - callbackUrl description: |- diff --git a/proto/worldmonitor/shipping/v2/register_webhook.proto b/proto/worldmonitor/shipping/v2/register_webhook.proto index 0e86bd6f8..6fb678ff4 100644 --- a/proto/worldmonitor/shipping/v2/register_webhook.proto +++ b/proto/worldmonitor/shipping/v2/register_webhook.proto @@ -20,7 +20,11 @@ message RegisterWebhookRequest { // the entire CHOKEPOINT_REGISTRY. Unknown IDs fail with 400. repeated string chokepoint_ids = 2; // Disruption-score threshold for delivery, 0-100. Default 50. - int32 alert_threshold = 3 [ + // proto3 `optional` so the handler can distinguish "partner explicitly sent + // 0 (deliver every alert)" from "partner omitted the field (apply default + // 50)". Without `optional`, both serialise to the proto3 scalar default of + // 0 and the handler can't tell them apart — flagged in #3242 review. + optional int32 alert_threshold = 3 [ (buf.validate.field).int32.gte = 0, (buf.validate.field).int32.lte = 100 ]; diff --git a/server/worldmonitor/shipping/v2/register-webhook.ts b/server/worldmonitor/shipping/v2/register-webhook.ts index 0373cec40..0fcea5643 100644 --- a/server/worldmonitor/shipping/v2/register-webhook.ts +++ b/server/worldmonitor/shipping/v2/register-webhook.ts @@ -66,14 +66,13 @@ export async function registerWebhook( ]); } - // Proto default int32 is 0 — treat 0 as "unset" to preserve the legacy - // default of 50 when the caller omits alertThreshold. - const alertThreshold = req.alertThreshold > 0 ? req.alertThreshold : 50; - if (alertThreshold < 0 || alertThreshold > 100) { - throw new ValidationError([ - { field: 'alertThreshold', description: 'alertThreshold must be a number between 0 and 100' }, - ]); - } + // alert_threshold is `optional int32` (#3242 followup #4) — undefined means + // the partner omitted the field, so apply the legacy default of 50. An + // explicit 0 is preserved (deliver every alert). The 0..100 range is + // enforced by buf.validate at the proto layer; the handler doesn't need + // a runtime branch (the previous `< 0` check was dead code after the + // `> 0 ? : 50` coercion). + const alertThreshold = req.alertThreshold ?? 50; const ownerTag = await callerFingerprint(ctx.request); const newSubscriberId = generateSubscriberId(); diff --git a/src/generated/client/worldmonitor/shipping/v2/service_client.ts b/src/generated/client/worldmonitor/shipping/v2/service_client.ts index 8039b582d..f7a5be043 100644 --- a/src/generated/client/worldmonitor/shipping/v2/service_client.ts +++ b/src/generated/client/worldmonitor/shipping/v2/service_client.ts @@ -39,7 +39,7 @@ export interface BypassOption { export interface RegisterWebhookRequest { callbackUrl: string; chokepointIds: string[]; - alertThreshold: number; + alertThreshold?: number; } export interface RegisterWebhookResponse { diff --git a/src/generated/server/worldmonitor/shipping/v2/service_server.ts b/src/generated/server/worldmonitor/shipping/v2/service_server.ts index 8483af986..2124ed242 100644 --- a/src/generated/server/worldmonitor/shipping/v2/service_server.ts +++ b/src/generated/server/worldmonitor/shipping/v2/service_server.ts @@ -39,7 +39,7 @@ export interface BypassOption { export interface RegisterWebhookRequest { callbackUrl: string; chokepointIds: string[]; - alertThreshold: number; + alertThreshold?: number; } export interface RegisterWebhookResponse { diff --git a/tests/shipping-v2-handler.test.mjs b/tests/shipping-v2-handler.test.mjs index b0ac3ee29..eafb67efc 100644 --- a/tests/shipping-v2-handler.test.mjs +++ b/tests/shipping-v2-handler.test.mjs @@ -207,16 +207,13 @@ describe('ShippingV2Service handlers', () => { ); }); - it('rejects alertThreshold > 100 with ValidationError', async () => { - await assert.rejects( - () => registerWebhook(proCtx(), { - callbackUrl: 'https://hooks.example.com/wm', - chokepointIds: [], - alertThreshold: 150, - }), - (err) => err instanceof ValidationError && err.violations[0].field === 'alertThreshold', - ); - }); + // alert_threshold 0..100 range is now enforced at the proto/wire layer + // by buf.validate (gte/lte on `optional int32 alert_threshold`). + // Direct handler invocation bypasses wire validation; the handler no + // longer carries a redundant runtime range check (was dead code after + // the previous `> 0 ? : 50` coercion). The wire path is exercised by + // the sebuf gateway integration; this unit test would only assert + // behaviour the handler intentionally delegates upstream. it('happy path returns wh_-prefixed subscriberId and 64-char hex secret; issues SET + SADD + EXPIRE pipeline with 30-day TTL', async () => { const calls = stubRedisOk(); @@ -246,7 +243,22 @@ describe('ShippingV2Service handlers', () => { assert.equal(pipeline[2][2], String(86400 * 30)); }); - it('alertThreshold 0 (proto default) coerces to legacy default 50', async () => { + it('alertThreshold omitted (undefined) applies the legacy default of 50', async () => { + const calls = stubRedisOk(); + await registerWebhook(proCtx(), { + callbackUrl: 'https://hooks.example.com/wm', + chokepointIds: [], + // alertThreshold omitted — proto3 `optional int32` arrives as undefined + }); + const record = JSON.parse(calls[0][0][2]); + assert.equal(record.alertThreshold, 50); + }); + + it('alertThreshold explicit 0 is preserved (deliver every alert)', async () => { + // #3242 followup #4 — proto3 `optional` lets the handler distinguish + // "partner explicitly sent 0" from "partner omitted the field". The + // pre-fix handler coerced both to 50, silently dropping the partner's + // intent to receive every disruption. const calls = stubRedisOk(); await registerWebhook(proCtx(), { callbackUrl: 'https://hooks.example.com/wm', @@ -254,7 +266,7 @@ describe('ShippingV2Service handlers', () => { alertThreshold: 0, }); const record = JSON.parse(calls[0][0][2]); - assert.equal(record.alertThreshold, 50); + assert.equal(record.alertThreshold, 0); }); it('empty chokepointIds subscribes to the full CHOKEPOINT_REGISTRY', async () => {