GeezerVison

🟠 major — Fallback link uses url.origin, which drops the base path in sub-path deployments, so the link 404s

Build the fallback from config.FRONTEND_URL (for example ${config.FRONTEND_URL}/alerts, the same way mcp/tools/alerts/getAlert.ts:81 and providers/default.ts:523 do), or add a buildAlertsLink() method to the provider. Today all alert links are built from FRONTEND_URL, and the app supports a base path (NEXT_PUBLIC_HYPERDX_BASE_PATH; ClickStack uses /clickstack in packages/app/next.config.mjs). With FRONTEND_URL=https://host/clickstack, this code turns https://host/clickstack/chart?... into https://host/alerts.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

This is a valid concern. @Tyagiquamar Could you address it?

🟠 major — The deep link is swapped for generic /alerts even when the body is what's too long

Only fall back when the link itself is the problem: if header(hdxLink).length + 3 fits within maxLength, keep hdxLink and truncate the body; use /alerts only when the header alone is too big. Example: a saved-search alert has a ~120-char /search/:id?from=..&to=.. link and a ~3100-char body (sample results can be up to 2503 chars plus the template text). The code swaps in /alerts and then truncates the body anyway, so the user loses the link to the exact search for no reason.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

🔵 minor — Body truncation cuts off the closing ``` of the saved-search results block

Saved-search bodies end with a -fenced `__hdx_query_results__` block (`template.ts:928-930`). Cutting the body at a fixed index drops the closing fence, so Slack shows the raw fence and the code formatting breaks. Fix: if the cut lands inside an open fence, close it (e.g. append '\n' and reserve room for it), or truncate inside the fence. Add a test for this case.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Hand-written slice-plus-ellipsis duplicates the existing truncateString helper

Use truncateString(body, availableBodyLen) from packages/api/src/utils/common.ts:38, which template.ts already uses for alert bodies. This follows the REQUIRED DRY/'Before you add a helper' convention.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Last-resort branch cuts through the mrkdwn link syntax

Slicing *<link | title>* at maxLength cuts through the <...> link markup, so Slack gets broken mrkdwn. Truncate title instead (keep the *<link | ...>* wrapper intact), or drop the link and send a plain truncated title.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — The webhook integration test only covers the short-message path, which already worked

Add a handleSendSlackWebhook case with a >3000-char hdxLink (like the real inline-alert /chart?config= link) and assert the posted block text is ≤3000 chars and uses the fallback link. Also add a case for a FRONTEND_URL that has a path prefix. Without these, removing the formatSlackSectionText call from the send path would still pass.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — The test uses as any casts, against the TypeScript conventions

Build the fixtures with real types (WebhookService.Slack, AlertState.ALERT, a typed webhook object), and use mockResolvedValue(undefined) or a typed response instead of {} as any. The repo conventions say to avoid any and as casts.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Fallback links lose deployment paths

getFallbackHdxLink keeps only the origin. With FRONTEND_URL=https://host.example/hyperdx, an oversized chart link becomes https://host.example/alerts instead of https://host.example/hyperdx/alerts. The app supports this prefix through NEXT_PUBLIC_HYPERDX_BASE_PATH, so the shortened link points outside the app. Build the fallback from the configured frontend URL and add a prefixed-URL test.

Long titles break link formatting

When the title makes the header exceed 3000 characters, this branch cuts through the Slack link text and removes its closing >*. Titles can come from a rendered alert.name template. The message fits the length limit but contains an unfinished link. Shorten the title before building the header, keep the link markers intact, and test this branch.

Channel fixture bypasses type checks

Casting the entire channel fixture to any stops TypeScript from checking whether it matches WebhookChannel. The repository guide requires proper types instead of any; this requirement must be satisfied before merging. Build a complete typed fixture instead. The same pattern appears in the mocked response and message state.

Context Used: AGENTS.md (source)

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

Required changeset is missing

This changes notification behavior in @hyperdx/api, but the PR adds no changeset. AGENTS.md requires a changeset for behavior changes and explicitly includes @hyperdx/api. Add a patch changeset before merging so the fix is included in the release notes and version plan.

Context Used: AGENTS.md (source)

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

🟠 major — Fallback link drops the deployment path prefix and points to a page that doesn't exist

The links come from ${config.FRONTEND_URL}/chart (packages/api/src/tasks/checkAlerts/providers/default.ts:523). The app also supports serving under a path prefix (NEXT_PUBLIC_HYPERDX_BASE_PATH, packages/app/src/config.ts:62-66). url.origin throws that prefix away, so a FRONTEND_URL like https://host/clickstack falls back to https://host/alerts, which is a 404. Build the fallback from config.FRONTEND_URL instead of parsing it back out of the link.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

🔵 minor — Transport guesses app routes by string-parsing hdxLink; the fallback belongs in the provider and should target the specific alert

The Slack transport has no knowledge of frontend routes or the alert id, so all it can do is link to the generic list. Have AlertProvider (providers/default.ts) build a short link alongside hdxLink, e.g. ${FRONTEND_URL}/alerts/${alert.id} (that detail route is used in packages/app/src/components/alerts/AlertDetails.tsx:150). Pass it on Message so the recipient lands on the alert that fired, and other length-limited transports can reuse it.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Re-implements the existing truncateString helper

truncateString in packages/api/src/utils/common.ts:38 already appends '...' after slicing, and template.ts uses it. Reuse it (e.g. header + truncateString(body, maxLength - header.length - 3)) rather than adding a second inline version. The repo conventions mark this DRY check as REQUIRED.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Last-resort slice cuts through the mrkdwn link markup

When the title alone pushes the header past the limit, *<link | title>*.slice(0, maxLength) drops the closing >*. Slack then renders the raw <https://... | ... text instead of a link. Truncate title so the *<link | ...>* wrapper stays intact.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Webhook test never exercises the over-limit path the PR fixes

handleSendSlackWebhook is only tested with a short hdxLink, so it would still pass if formatSlackSectionText were removed from the payload. Add a case with a >3000-char chart link and assert the posted block text is the fallback/truncated value and is ≤3000 chars. Add a fallback test where the link has a path prefix (e.g. https://host/prefix/chart?...). While there, replace the as any casts with properly typed fixtures, per the repo conventions.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}