Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions server/api/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,22 @@ GITHUB_APP_ID=
GITHUB_APP_PRIVATE_KEY=
GITHUB_APP_INSTALLATION_ID=
GITHUB_DISPATCH_REPOSITORY=

# Monitoring alert email + admin URL for Epic #616 Phase 3 (issue #809).
# - MONITORING_NOTIFY_EMAIL: recipient address for `notifyApiErrorAlert`. When
# unset the notifier is a no-op so the AI callback path works in unconfigured
# staging environments.
# - ADMIN_BASE_URL: public origin of the admin panel (e.g. `https://admin.zedi-note.app`).
# Used to build a `<base>/errors/<api_error_id>` link inside alert emails.
# Internal/private URLs must NOT be set here. When unset the alert email
# omits the link line and falls back to sentry_issue_id only.
#
# Epic #616 Phase 3(issue #809)用の通知設定。
# - MONITORING_NOTIFY_EMAIL: severity が `high` / `medium` の API エラー通知の
# 宛先メール。未設定時は notifier が no-op となり、AI コールバックは正常に
# 動作する(ステージング段階で通知のみ無効化できる)。
# - ADMIN_BASE_URL: 管理画面の公開オリジン(例: `https://admin.zedi-note.app`)。
# 通知メール本文に `<base>/errors/<api_error_id>` のリンクを差し込む際に使う。
# 内部 URL を設定してはいけない。未設定時はリンク行を省略する。
MONITORING_NOTIFY_EMAIL=
ADMIN_BASE_URL=
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => {
beforeEach(() => {
vi.spyOn(console, "error").mockImplementation(() => undefined);
vi.spyOn(console, "log").mockImplementation(() => undefined);
vi.spyOn(console, "warn").mockImplementation(() => undefined);
process.env.GITHUB_APP_ID = "123";
process.env.GITHUB_APP_INSTALLATION_ID = "456";
process.env.GITHUB_APP_PRIVATE_KEY = "stub";
Expand Down Expand Up @@ -317,4 +318,51 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => {
});
expect(res.status).toBe(404);
});

it("invokes notifyApiErrorAlert exactly once with the post-update row (Phase 3 / #809)", async () => {
// 通知は AI コールバックの 1 箇所からのみ起動する(二重通知防止)。
// notifyApiErrorAlert が成功 200 のたびに 1 回だけ呼ばれることを確認する。
//
// The notifier is called from this single site to prevent duplicate
// alerts. Verify that a successful 200 response invokes it exactly once
// with the post-update row's fields.
vi.resetModules();
const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true, id: "e1" } });
await vi.doMock("../../../services/notifier.js", () => ({
notifyApiErrorAlert: notifySpy,
}));
await stubVerifyInstallationToken(true);
const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js");
const updated = {
id: VALID_UUID,
sentryIssueId: "sentry-xyz",
severity: "high",
title: "TypeError",
};
const { db } = createMockDb([[updated]]);
const app = new Hono<AppEnv>();
app.onError(errorHandler);
app.use("*", async (c, next) => {
c.set("db", db as unknown as AppEnv["Variables"]["db"]);
await next();
});
app.route("/api/webhooks/github/ai-result", routes);

const res = await app.request(`/api/webhooks/github/ai-result/${VALID_UUID}`, {
method: "PUT",
headers: { "Content-Type": "application/json", Authorization: "Bearer ghs_ok" },
body: JSON.stringify({ severity: "high" }),
});
expect(res.status).toBe(200);
// fire-and-forget の microtask が消化されるのを待つ。
// Drain any pending microtasks queued by the fire-and-forget call.
await new Promise((resolve) => setImmediate(resolve));
expect(notifySpy).toHaveBeenCalledTimes(1);
expect(notifySpy).toHaveBeenCalledWith({
apiErrorId: VALID_UUID,
sentryIssueId: "sentry-xyz",
severity: "high",
title: "TypeError",
});
});
});
16 changes: 16 additions & 0 deletions server/api/src/routes/webhooks/githubAiCallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
type UpdateAiAnalysisInput,
} from "../../services/apiErrorService.js";
import { publishApiErrorUpdate } from "../../services/apiErrorBroadcaster.js";
import { notifyApiErrorAlert } from "../../services/notifier.js";
import { verifyInstallationToken } from "../../lib/githubAppAuth.js";
import type { AppEnv } from "../../types/index.js";

Expand Down Expand Up @@ -161,6 +162,21 @@ app.put("/:id", async (c) => {
// SSE 購読者へ AI 解析結果を配信 (Phase 2 / issue #807)。
// Notify SSE subscribers so the admin UI updates without a page reload.
publishApiErrorUpdate(updated);
// 重要エラーのメール通知 (Phase 3 / issue #809)。severity が `high` /
// `medium` 以外、もしくは `MONITORING_NOTIFY_EMAIL` 未設定の場合は no-op。
// ここを唯一の呼び出し元にして二重通知を防ぐ。fire-and-forget で
// webhook 応答を遅らせない(notifier 側でエラーは swallow 済み)。
//
// Email alert for high-impact errors (Phase 3 / #809). The notifier is a
// no-op when severity is `low`/`unknown` or `MONITORING_NOTIFY_EMAIL` is
// unset. This is the single call site to prevent duplicate alerts; we
// fire-and-forget so the webhook response doesn't await Resend.
void notifyApiErrorAlert({

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 Gate alert sends on severity changes, not every callback

notifyApiErrorAlert is invoked unconditionally after updateAiAnalysis, but updateAiAnalysis intentionally supports partial/no-op callbacks (it returns the existing row when no fields are updated). That means a retry or follow-up callback like {} (or one that only updates another AI field) will resend the same high/medium alert for an already-escalated issue, creating duplicate operational emails. Please only trigger notifications when the callback actually transitions severity into high/medium (or otherwise prove this is the first alert for that row).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 96da9b6.

The route now reads the row via getApiErrorById before updateAiAnalysis and gates the notifier on a new helper severityBecameNotifiable(prev, next) which returns true only when prev was NOT in {high, medium} AND next IS. This covers the cases you raised:

  • Empty body / partial callback (e.g. only ai_summary): post severity equals pre severity → no notify.
  • Idempotent retry (highhigh): no transition → no notify.
  • Lateral move (mediumhigh): both notifiable → no notify (chosen so each row produces at most one email; medium→high escalation is intentionally collapsed into the original alert).
  • Downgrade (highlow): post is not notifiable → no notify.
  • First-sight escalation (unknown/lowhigh/medium): exactly one notify.

Tests added in githubAiCallback.test.ts cover all four no-notify paths plus the positive transition.


Generated by Claude Code

apiErrorId: updated.id,
sentryIssueId: updated.sentryIssueId,
severity: updated.severity,
title: updated.title,
});
// 外部 (GitHub Actions) 向けの webhook なので、`error` キーを成功時に流用する
// 内部 admin API の慣習ではなく、`data` キーで返して "error 有無で失敗判定"
// できる素直な形にする(admin/src/api/admin.ts と異なり消費者がまだ存在しない)。
Expand Down
193 changes: 193 additions & 0 deletions server/api/src/services/notifier.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
/**
* `notifier.ts` の単体テスト (Epic #616 Phase 3 / sub-issue #809)。
*
* 本プロジェクトでは外部通知をメール経由のみに限定する(Slack は使わない)。
* テストは `emailService` を mock して、severity と環境変数の組み合わせで
* 1 通だけ送られる / 送られないことを担保する。PII (Authorization / Cookie /
* raw email) が本文に混入しないことも検証する。
*
* Unit tests for `notifier.ts` (Epic #616 Phase 3 / sub-issue #809). External
* alerting is intentionally email-only (no Slack). Tests mock `emailService`
* and assert send count for each (severity, env) combination, plus PII-safe
* body content (no Authorization / Cookie / raw email leakage).
*/
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";

const mockSendEmail = vi.fn();

vi.mock("./emailService.js", () => ({
sendEmail: mockSendEmail,
}));

// モック設定後にインポート / Import after mock setup
const { notifyApiErrorAlert } = await import("./notifier.js");

describe("notifyApiErrorAlert", () => {
const originalEnv = process.env;

beforeEach(() => {
process.env = { ...originalEnv };
vi.clearAllMocks();
mockSendEmail.mockResolvedValue({ success: true, id: "email_id_1" });
vi.spyOn(console, "warn").mockImplementation(() => undefined);
vi.spyOn(console, "error").mockImplementation(() => undefined);
vi.spyOn(console, "log").mockImplementation(() => undefined);
});

afterEach(() => {
process.env = originalEnv;
vi.restoreAllMocks();
});

it("MONITORING_NOTIFY_EMAIL 未設定なら送信しない (no-op)", async () => {
delete process.env.MONITORING_NOTIFY_EMAIL;

const result = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError: cannot read property",
});

expect(result.email.sent).toBe(false);
expect(mockSendEmail).not.toHaveBeenCalled();
});

it("severity が low / unknown のときは送信しない", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";

const lowResult = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-low",
severity: "low",
title: "minor",
});
const unknownResult = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000002",
sentryIssueId: "sentry-unknown",
severity: "unknown",
title: "unclassified",
});

expect(lowResult.email.sent).toBe(false);
expect(unknownResult.email.sent).toBe(false);
expect(mockSendEmail).not.toHaveBeenCalled();
});

it("severity=high かつ MONITORING_NOTIFY_EMAIL 設定済みで 1 通だけ送る", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";

const result = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError: cannot read property",
});

expect(result.email.sent).toBe(true);
expect(mockSendEmail).toHaveBeenCalledTimes(1);
const args = mockSendEmail.mock.calls[0]?.[0];
expect(args.to).toBe("ops@example.com");
expect(args.subject).toContain("high");
expect(args.subject).toContain("sentry-abc");
});

it("severity=medium でも送る", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";

const result = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000003",
sentryIssueId: "sentry-med",
severity: "medium",
title: "Latency spike",
});

expect(result.email.sent).toBe(true);
expect(mockSendEmail).toHaveBeenCalledTimes(1);
});

it("ADMIN_BASE_URL 設定時は管理画面 URL を本文に含める", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";
process.env.ADMIN_BASE_URL = "https://admin.zedi-note.app";

await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError",
});

const args = mockSendEmail.mock.calls[0]?.[0];
expect(args.html).toContain(
"https://admin.zedi-note.app/errors/00000000-0000-0000-0000-000000000001",
);
});

it("ADMIN_BASE_URL 未設定時は URL を本文に含めない(no-op フォールバック)", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";
delete process.env.ADMIN_BASE_URL;

await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError",
});

const args = mockSendEmail.mock.calls[0]?.[0];
// 管理画面 URL は載せないが、最低限 sentry_issue_id と severity は本文にある。
// No admin URL is rendered, but sentry_issue_id and severity are still
// present so an on-call can pivot to the Sentry issue directly.
expect(args.html).not.toContain("http");
expect(args.html).toContain("sentry-abc");
expect(args.html).toContain("high");
});

it("PII (Authorization / Cookie / raw email) を本文に含めない", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";
process.env.ADMIN_BASE_URL = "https://admin.zedi-note.app";

await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError",
});

const args = mockSendEmail.mock.calls[0]?.[0];
const haystack = `${args.subject}\n${args.html}`.toLowerCase();
expect(haystack).not.toContain("authorization");
expect(haystack).not.toContain("cookie");
expect(haystack).not.toContain("bearer ");
});

it("emailService が失敗しても throw せず success=false を返す", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";
mockSendEmail.mockResolvedValueOnce({ success: false, error: "Rate limit" });

const result = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError",
});

expect(result.email.sent).toBe(false);
expect(result.email.error).toBe("Rate limit");
});

it("emailService が例外を投げても throw せず success=false を返す", async () => {
process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com";
mockSendEmail.mockRejectedValueOnce(new Error("Network down"));

const result = await notifyApiErrorAlert({
apiErrorId: "00000000-0000-0000-0000-000000000001",
sentryIssueId: "sentry-abc",
severity: "high",
title: "TypeError",
});

expect(result.email.sent).toBe(false);
expect(result.email.error).toContain("Network down");
});
});
Loading
Loading