mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
* feat(brief): consolidate composer into digest cron (retire standalone service)
Merges the Phase 3a standalone Railway composer into the existing
digest cron. End state: one cron (seed-digest-notifications.mjs)
writes brief:{userId}:{issueDate} for every eligible user AND
dispatches the digest to their configured channels with a signed
magazine URL appended. Net -1 Railway service.
User's architectural note: "there is no reason to have 1 digest
preparing all and sending, then another doing a duplicate". This
delivers that — infrastructure consolidation, same send cadence,
single source of truth for brief envelopes.
File moves / deletes:
- scripts/seed-brief-composer.mjs → scripts/lib/brief-compose.mjs
Pure-helpers library: no main(), no env guards, no cron. Exports
composeBriefForRule + groupEligibleRulesByUser + dedupeRulesByUser
(shim) + shouldExitNonZero + date helpers + extractInsights.
- Dockerfile.seed-brief-composer → deleted.
- The seed-brief-composer Railway service is retired (user confirmed
they would delete it manually).
New files:
- scripts/lib/brief-url-sign.mjs — plain .mjs port of the sign path
in server/_shared/brief-url.ts (Web Crypto only, no node:crypto).
- tests/brief-url-sign.test.mjs — parity tests that confirm tokens
minted by the scripts-side signer verify via the edge-side verifier
and produce byte-identical output for identical input.
Digest cron (scripts/seed-digest-notifications.mjs):
- Reads news:insights:v1 once per run, composes per-user brief
envelopes, SETEX brief:{userId}:{issueDate} via body-POST pipeline.
- Signs magazine URL per user (BRIEF_URL_SIGNING_SECRET +
WORLDMONITOR_PUBLIC_BASE_URL new env requirements, see pre-merge).
- Injects magazineUrl into buildChannelBodies for every channel
(email, telegram, slack, discord) as a "📖 Open your WorldMonitor
Brief magazine" footer CTA.
- Email HTML gets a dedicated data-brief-cta-slot near the top of
the body with a styled button.
- Compose failures NEVER block the digest send — the digest cron's
existing behaviour is preserved when the brief pipeline has issues.
- Brief compose extracted to its own functions (composeBriefsForRun
+ composeAndStoreBriefForUser) to keep main's biome complexity at
baseline (64 — was 63 before; inline would have pushed to 117).
Tests: 98/98 across the brief suite. New parity tests confirm cross-
module signer agreement.
PRE-MERGE: add BRIEF_URL_SIGNING_SECRET and WORLDMONITOR_PUBLIC_BASE_URL
to the digest-notifications Railway service env (same values already
set on Vercel for Phase 2). Without them, brief compose is auto-
disabled and the digest falls back to its current behaviour — safe to
deploy before env is set.
* fix(brief): digest Dockerfile + propagate compose failure to exit code
Addresses two seventh-round review findings on PR #3157.
1. Cross-directory imports + current Railway build root (todo 230).
The consolidated digest cron imports from ../api, ../shared, and
(transitively via scripts/lib/brief-compose.mjs) ../server/_shared.
The running digest-notifications Railway service builds from the
scripts/ root — those parent paths are outside the deploy tree
and would 500 on next rebuild with ERR_MODULE_NOT_FOUND.
New Dockerfile.digest-notifications (repo-root build context)
COPYs exactly the modules the cron needs: scripts/ contents,
scripts/lib/, shared/brief-envelope.*, shared/brief-filter.*,
server/_shared/brief-render.*, api/_upstash-json.js,
api/_seed-envelope.js. Tight list to keep the watch surface small.
Pattern matches the retired Dockerfile.seed-brief-composer + the
existing Dockerfile.relay.
2. Silent compose failures (todo 231). composeBriefsForRun logged
counters but never exited non-zero. An Upstash outage or missing
signing secret silently dropped every brief write while Railway
showed the cron green. The retired standalone composer exited 1
on structural failures; that observability was lost in the
consolidation.
Changed the compose fn to return {briefByUser, composeSuccess,
composeFailed}. Main captures the counters, runs the full digest
send loop first (compose-layer breakage must NEVER block user-
visible digest delivery), then calls shouldExitNonZero at the
very end. Exit-on-failure gives ops the Railway-red signal
without touching send behaviour.
Also: a total read failure of news:insights:v1 (catch branch)
now counts as 1 compose failure so the gate trips on insights-
key infra breakage, not just per-user write failures.
Tests unchanged (98/98). Typecheck + node --check clean. Biome
complexity ticks 63→65 — same pre-existing bucket, already tolerated
by CI; no new blocker.
PRE-MERGE Railway work still pending: set BRIEF_URL_SIGNING_SECRET
+ WORLDMONITOR_PUBLIC_BASE_URL on the digest-notifications service,
AND switch its dockerfilePath to /Dockerfile.digest-notifications
before merging. Without the dockerfilePath switch, the next rebuild
fails.
* fix(brief): Dockerfile type:module + explicit missing-secret tripwire
Addresses two eighth-round review findings on PR #3157.
1. ESM .js files parse as CommonJS in the container (todo 232).
Dockerfile.digest-notifications COPYs shared/*.js,
server/_shared/*.js, api/*.js — all ESM because the repo-root
package.json has "type":"module". But the image never copies the
root package.json, so Node's nearest-pjson walk inside /app/
reaches / without finding one and defaults to CommonJS. First
`export` statement throws `SyntaxError: Unexpected token 'export'`
at startup.
Fix: write a minimal /app/package.json with {"type":"module"}
early in the build. Avoids dragging the full root package.json
into the image while still giving Node the ESM hint it needs for
repo-owned .js files.
2. Missing BRIEF_URL_SIGNING_SECRET silently tolerated (todo 233).
The old gate folded "operator-disabled" (BRIEF_COMPOSE_ENABLED=0)
and "required secret missing in rollout" into the same boolean
via AND. A production deploy that forgot the env var would skip
brief compose without any failure signal — Railway green, no
briefs, no CTA in digests, nobody notices.
Split the two states: BRIEF_COMPOSE_DISABLED_BY_OPERATOR (explicit
kill switch, silent) and BRIEF_SIGNING_SECRET_MISSING (the misconfig
we care about). When the secret is missing without the operator
flag, composeBriefsForRun returns composeFailed=1 on first call
so the end-of-run exit gate trips and Railway flags the run red.
Digest send still proceeds — compose-layer issues never block
notifications.
Tests: 98/98. Syntax + node --check clean.
* fix(brief): address 2 remaining P2 review comments on PR #3157
Greptile review (2026-04-18T05:04Z) flagged three P2 items. The
first (shouldExitNonZero never wired into cron) was already fixed in
commit 35a46aa34. This commit addresses the other two.
1. composeBriefForRule: issuedAt used Date.now() instead of the
caller-supplied nowMs. Under the digest cron the delta is
milliseconds and harmless, but it broke the function's
determinism contract — same input must produce same output for
tests + retries. Now uses the passed nowMs.
2. buildChannelBodies: magazineUrl embedded raw inside Telegram HTML
<a href="..."> and Slack <URL|text> syntax. The URL is HMAC-
signed and shape-validated upstream (userId regex + YYYY-MM-DD
date), so injection is practically impossible — but the email
CTA (injectBriefCta) escapes per-target and channel footers
should match that discipline. Added:
- Telegram: escape &, <, >, " to HTML entities
- Slack: strip <, >, | (mrkdwn metacharacters)
Discord and plain-text paths unchanged — Discord links tolerate
raw URLs, plain text has no metacharacters to escape.
Tests: 98/98 still pass (deterministic issuedAt change was
transparent to existing assertions because tests already pass nowMs
explicitly via the issuedAt fixture field).
205 lines
7.9 KiB
JavaScript
205 lines
7.9 KiB
JavaScript
// Regression tests for the Phase 3a composer's rule-selection logic.
|
|
//
|
|
// Two guards:
|
|
// 1. aiDigestEnabled default parity — undefined must be opt-IN, matching
|
|
// seed-digest-notifications.mjs:914 and notifications-settings.ts:228.
|
|
// 2. Per-user dedupe — alertRules are (userId, variant)-scoped but the
|
|
// brief key is user-scoped. Multi-variant users must produce exactly
|
|
// one brief per issue, with a deterministic tie-breaker.
|
|
|
|
import { describe, it } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
import {
|
|
dedupeRulesByUser,
|
|
groupEligibleRulesByUser,
|
|
shouldExitNonZero,
|
|
} from '../scripts/lib/brief-compose.mjs';
|
|
|
|
function rule(overrides = {}) {
|
|
return {
|
|
userId: 'user_abc',
|
|
variant: 'full',
|
|
enabled: true,
|
|
digestMode: 'daily',
|
|
sensitivity: 'high',
|
|
aiDigestEnabled: true,
|
|
digestTimezone: 'UTC',
|
|
updatedAt: 1_700_000_000_000,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe('dedupeRulesByUser', () => {
|
|
it('keeps a single rule unchanged', () => {
|
|
const out = dedupeRulesByUser([rule()]);
|
|
assert.equal(out.length, 1);
|
|
assert.equal(out[0].variant, 'full');
|
|
});
|
|
|
|
it('dedupes multi-variant users to one rule, preferring "full"', () => {
|
|
const out = dedupeRulesByUser([
|
|
rule({ variant: 'finance', sensitivity: 'high' }),
|
|
rule({ variant: 'full', sensitivity: 'critical' }),
|
|
rule({ variant: 'tech', sensitivity: 'all' }),
|
|
]);
|
|
assert.equal(out.length, 1);
|
|
assert.equal(out[0].variant, 'full');
|
|
});
|
|
|
|
it('when no full variant: picks most permissive sensitivity', () => {
|
|
const out = dedupeRulesByUser([
|
|
rule({ variant: 'tech', sensitivity: 'critical' }),
|
|
rule({ variant: 'finance', sensitivity: 'all' }),
|
|
rule({ variant: 'energy', sensitivity: 'high' }),
|
|
]);
|
|
assert.equal(out.length, 1);
|
|
// 'all' is the most permissive.
|
|
assert.equal(out[0].variant, 'finance');
|
|
});
|
|
|
|
it('never cross-contaminates across userIds', () => {
|
|
const out = dedupeRulesByUser([
|
|
rule({ userId: 'user_a', variant: 'full' }),
|
|
rule({ userId: 'user_b', variant: 'tech' }),
|
|
rule({ userId: 'user_a', variant: 'finance' }),
|
|
]);
|
|
assert.equal(out.length, 2);
|
|
const a = out.find((r) => r.userId === 'user_a');
|
|
const b = out.find((r) => r.userId === 'user_b');
|
|
assert.equal(a.variant, 'full');
|
|
assert.equal(b.variant, 'tech');
|
|
});
|
|
|
|
it('drops rules without a string userId', () => {
|
|
const out = dedupeRulesByUser([
|
|
rule({ userId: /** @type {any} */ (null) }),
|
|
rule({ userId: 'user_ok' }),
|
|
]);
|
|
assert.equal(out.length, 1);
|
|
assert.equal(out[0].userId, 'user_ok');
|
|
});
|
|
|
|
it('is deterministic across duplicate full-variant rules via updatedAt tie-breaker', () => {
|
|
const older = rule({ variant: 'full', sensitivity: 'high', updatedAt: 1_000 });
|
|
const newer = rule({ variant: 'full', sensitivity: 'high', updatedAt: 2_000 });
|
|
const out1 = dedupeRulesByUser([older, newer]);
|
|
const out2 = dedupeRulesByUser([newer, older]);
|
|
// Earlier updatedAt wins — stable under input reordering.
|
|
assert.equal(out1[0].updatedAt, 1_000);
|
|
assert.equal(out2[0].updatedAt, 1_000);
|
|
});
|
|
});
|
|
|
|
describe('aiDigestEnabled default parity', () => {
|
|
// The composer's main loop short-circuits on `rule.aiDigestEnabled
|
|
// === false`. Exercising the predicate directly so a refactor that
|
|
// re-inverts it (back to `!rule.aiDigestEnabled`) fails loud.
|
|
|
|
function shouldSkipForAiDigest(rule) {
|
|
return rule.aiDigestEnabled === false;
|
|
}
|
|
|
|
it('includes rules with aiDigestEnabled: true', () => {
|
|
assert.equal(shouldSkipForAiDigest({ aiDigestEnabled: true }), false);
|
|
});
|
|
|
|
it('includes rules with aiDigestEnabled: undefined (legacy rows)', () => {
|
|
assert.equal(shouldSkipForAiDigest({ aiDigestEnabled: undefined }), false);
|
|
});
|
|
|
|
it('includes rules with no aiDigestEnabled field at all (legacy rows)', () => {
|
|
assert.equal(shouldSkipForAiDigest({}), false);
|
|
});
|
|
|
|
it('excludes only when explicitly false', () => {
|
|
assert.equal(shouldSkipForAiDigest({ aiDigestEnabled: false }), true);
|
|
});
|
|
|
|
it('groupEligibleRulesByUser: opted-out preferred variant falls back to opted-in sibling', () => {
|
|
const grouped = groupEligibleRulesByUser([
|
|
rule({ variant: 'full', aiDigestEnabled: false, updatedAt: 100 }),
|
|
rule({ variant: 'finance', aiDigestEnabled: true, updatedAt: 200 }),
|
|
]);
|
|
const candidates = grouped.get('user_abc');
|
|
assert.ok(candidates, 'user is still eligible via the opt-in variant');
|
|
assert.equal(candidates.length, 1);
|
|
assert.equal(candidates[0].variant, 'finance');
|
|
});
|
|
|
|
it('groupEligibleRulesByUser: user with all variants opted-out is dropped entirely', () => {
|
|
const grouped = groupEligibleRulesByUser([
|
|
rule({ variant: 'full', aiDigestEnabled: false }),
|
|
rule({ variant: 'finance', aiDigestEnabled: false }),
|
|
]);
|
|
assert.equal(grouped.size, 0);
|
|
});
|
|
|
|
it('groupEligibleRulesByUser: retains all eligible candidates in preference order', () => {
|
|
const grouped = groupEligibleRulesByUser([
|
|
rule({ variant: 'finance', sensitivity: 'critical', updatedAt: 100 }),
|
|
rule({ variant: 'full', sensitivity: 'critical', updatedAt: 200 }),
|
|
rule({ variant: 'tech', sensitivity: 'all', updatedAt: 300 }),
|
|
]);
|
|
const candidates = grouped.get('user_abc');
|
|
assert.equal(candidates.length, 3);
|
|
// First is full (preferred variant); then tech (most permissive sensitivity);
|
|
// then finance. Fallback loop in the main() script tries them in this order.
|
|
assert.equal(candidates[0].variant, 'full');
|
|
assert.equal(candidates[1].variant, 'tech');
|
|
assert.equal(candidates[2].variant, 'finance');
|
|
});
|
|
|
|
it('shouldExitNonZero: returns false when no failures', () => {
|
|
assert.equal(shouldExitNonZero({ success: 10, failed: 0 }), false);
|
|
});
|
|
|
|
it('shouldExitNonZero: catches 100% failure on small attempted volume', () => {
|
|
// 4 attempted, 4 failed, 96 eligible skipped-empty. The earlier
|
|
// (eligibleUserCount) denominator would read 4/100=4% and pass.
|
|
assert.equal(shouldExitNonZero({ success: 0, failed: 4 }), true);
|
|
});
|
|
|
|
it('shouldExitNonZero: 1/20 failures is exactly at 5% (floor(20*0.05)=1), trips', () => {
|
|
// Exact-threshold boundary: documents intentional behaviour.
|
|
assert.equal(shouldExitNonZero({ success: 19, failed: 1 }), true);
|
|
});
|
|
|
|
it('shouldExitNonZero: 1/50 failures stays under threshold (floor(50*0.05)=2)', () => {
|
|
// Threshold floor is Math.max(1, floor(N*0.05)). For N<40 a
|
|
// single failure always trips. At N=50 the threshold is 2, so
|
|
// 1/50 stays green. Ops intuition: the 5% bar is only a "bar"
|
|
// once you have a meaningful sample.
|
|
assert.equal(shouldExitNonZero({ success: 49, failed: 1 }), false);
|
|
});
|
|
|
|
it('shouldExitNonZero: 2/10 exceeds threshold', () => {
|
|
// floor(10 * 0.05) = 0 → Math.max forces 1. failed=2 >= 1.
|
|
assert.equal(shouldExitNonZero({ success: 8, failed: 2 }), true);
|
|
});
|
|
|
|
it('shouldExitNonZero: single isolated failure still tripwires', () => {
|
|
// floor(1 * 0.05) = 0 → Math.max forces 1. failed=1 >= 1.
|
|
assert.equal(shouldExitNonZero({ success: 0, failed: 1 }), true);
|
|
});
|
|
|
|
it('shouldExitNonZero: zero attempted means no signal, returns false', () => {
|
|
assert.equal(shouldExitNonZero({ success: 0, failed: 0 }), false);
|
|
});
|
|
|
|
it('matches seed-digest-notifications convention', async () => {
|
|
// Cross-reference: the existing digest cron uses the same
|
|
// `!== false` test. If it drifts, the brief and digest will
|
|
// disagree on who is eligible. This assertion lives here to
|
|
// surface the divergence loudly.
|
|
const fs = await import('node:fs/promises');
|
|
const src = await fs.readFile(
|
|
new URL('../scripts/seed-digest-notifications.mjs', import.meta.url),
|
|
'utf8',
|
|
);
|
|
assert.ok(
|
|
src.includes('rule.aiDigestEnabled !== false'),
|
|
'seed-digest-notifications.mjs must keep `rule.aiDigestEnabled !== false`',
|
|
);
|
|
});
|
|
});
|