Skip to content

Commit 037a969

Browse files
committed
fix: harden mailing list bug fixes and add activity type wiring (CM-1318)
Signed-off-by: Uroš Marolt <uros@marolt.me>
1 parent 1b9e96a commit 037a969

23 files changed

Lines changed: 383 additions & 49 deletions

File tree

backend/package.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@
3333
"script:fix-duplicate-members": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/fix-duplicate-members.ts",
3434
"script:fix-members-activities-after-unaffilation": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/fix-members-activities-after-unaffilation.ts",
3535
"script:process-bot-members": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/process-bot-members.ts",
36-
"script:backfill-email-domain-member-organization-dates": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/backfill-email-domain-member-organization-dates.ts"
36+
"script:backfill-email-domain-member-organization-dates": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/backfill-email-domain-member-organization-dates.ts",
37+
"script:onboard-default-tenant": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/onboard-default-tenant.ts",
38+
"script:onboard-default-tenant:local": "set -a && . ./.env.dist.local && . ./.env.override.local && set +a && pnpm run script:onboard-default-tenant",
39+
"script:create-mailing-list-integration": "SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/create-mailing-list-integration.ts",
40+
"script:create-mailing-list-integration:local": "set -a && . ./.env.dist.local && . ./.env.override.local && set +a && SERVICE=script TS_NODE_TRANSPILE_ONLY=true tsx src/bin/scripts/create-mailing-list-integration.ts"
3741
},
3842
"lint-staged": {
3943
"**/*.ts": [

backend/src/api/integration/helpers/mailingListAuthenticate.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,24 @@
1+
import { z } from 'zod'
2+
13
import Permissions from '../../../security/permissions'
24
import IntegrationService from '../../../services/integrationService'
35
import PermissionChecker from '../../../services/user/permissionChecker'
6+
import { validateOrThrow } from '../../../utils/validation'
7+
8+
const bodySchema = z.object({
9+
lists: z
10+
.array(
11+
z.object({
12+
name: z.string().trim().min(1),
13+
sourceUrl: z.string().trim().min(1),
14+
}),
15+
)
16+
.default([]),
17+
})
418

519
export default async (req, res) => {
620
new PermissionChecker(req).validateHas(Permissions.values.tenantEdit)
7-
const integrationData = {
8-
...req.body,
9-
lists: req.body.lists || [],
10-
}
21+
const integrationData = validateOrThrow(bodySchema, req.body)
1122

1223
const payload = await new IntegrationService(req).mailingListConnectOrUpdate(integrationData)
1324
await req.responseHandler.success(req, res, payload)
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/* eslint-disable no-console */
2+
3+
/* eslint-disable import/no-extraneous-dependencies */
4+
import commandLineArgs from 'command-line-args'
5+
import commandLineUsage from 'command-line-usage'
6+
import * as fs from 'fs'
7+
8+
import { DEFAULT_TENANT_ID, generateUUIDv1 } from '@crowd/common'
9+
10+
import SegmentRepository from '@/database/repositories/segmentRepository'
11+
import SequelizeRepository from '@/database/repositories/sequelizeRepository'
12+
import IntegrationService from '@/services/integrationService'
13+
14+
const options = [
15+
{
16+
name: 'help',
17+
alias: 'h',
18+
type: Boolean,
19+
description: 'Print this usage guide.',
20+
},
21+
{
22+
name: 'file',
23+
alias: 'f',
24+
type: String,
25+
description:
26+
'Path to a JSON file with the lists array, each entry shaped as name + sourceUrl ' +
27+
'(e.g. name "linux-serial", sourceUrl https://lore.kernel.org/linux-serial).',
28+
},
29+
{
30+
name: 'segment',
31+
alias: 's',
32+
type: String,
33+
description: "Segment id. Optional — defaults to the tenant's first subproject.",
34+
},
35+
]
36+
37+
const sections = [
38+
{
39+
header: 'Create mailing list integration',
40+
content:
41+
'Calls IntegrationService.mailingListConnectOrUpdate() directly to create/update the ' +
42+
'mailinglist integration and onboard its lists for processing, until the frontend connect ' +
43+
'UI ships (CM-1318).',
44+
},
45+
{
46+
header: 'Options',
47+
optionList: options,
48+
},
49+
]
50+
51+
const usage = commandLineUsage(sections)
52+
// pnpm forwards a literal `--` separator when args are passed via `pnpm run ... -- <args>`;
53+
// strip it so command-line-args doesn't choke on it.
54+
const argv = process.argv.slice(2).filter((arg) => arg !== '--')
55+
const parameters = commandLineArgs(options, { argv })
56+
57+
if (parameters.help || !parameters.file) {
58+
console.log(usage)
59+
process.exit(parameters.help ? 0 : 1)
60+
} else {
61+
setImmediate(async () => {
62+
let fileContents: string
63+
try {
64+
fileContents = fs.readFileSync(parameters.file, 'utf-8')
65+
} catch (err) {
66+
console.error(`Could not read file at ${parameters.file}: ${(err as Error).message}`)
67+
process.exit(1)
68+
}
69+
70+
let lists
71+
try {
72+
lists = JSON.parse(fileContents)
73+
} catch (err) {
74+
console.error(`File at ${parameters.file} is not valid JSON: ${(err as Error).message}`)
75+
process.exit(1)
76+
}
77+
78+
const repoOptions = await SequelizeRepository.getDefaultIRepositoryOptions()
79+
repoOptions.currentTenant = { id: DEFAULT_TENANT_ID }
80+
;(repoOptions as unknown as { requestId: string }).requestId = generateUUIDv1()
81+
82+
const adminUser = await repoOptions.database.user.findOne({ where: {} })
83+
if (!adminUser) {
84+
console.error('No user found — run script:onboard-default-tenant first.')
85+
process.exit(1)
86+
}
87+
repoOptions.currentUser = adminUser
88+
89+
const segmentRepository = new SegmentRepository(repoOptions)
90+
repoOptions.currentSegments = parameters.segment
91+
? await segmentRepository.findInIds([parameters.segment])
92+
: (await segmentRepository.querySubprojects({ limit: 1, offset: 0 })).rows
93+
94+
if (repoOptions.currentSegments.length === 0) {
95+
console.error('No segment found/resolved — pass --segment explicitly.')
96+
process.exit(1)
97+
}
98+
99+
console.log(`Segment: ${repoOptions.currentSegments[0].id} (${repoOptions.currentSegments[0].name})`)
100+
console.log('Lists:', JSON.stringify(lists, null, 2))
101+
102+
const integration = await new IntegrationService(repoOptions).mailingListConnectOrUpdate(
103+
{ lists },
104+
repoOptions,
105+
)
106+
107+
console.log('Integration:', JSON.stringify(integration, null, 2))
108+
process.exit(0)
109+
})
110+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/* eslint-disable no-console */
2+
3+
/* eslint-disable import/no-extraneous-dependencies */
4+
import commandLineArgs from 'command-line-args'
5+
import commandLineUsage from 'command-line-usage'
6+
7+
import SequelizeRepository from '@/database/repositories/sequelizeRepository'
8+
import AuthService from '@/services/auth/authService'
9+
10+
const options = [
11+
{
12+
name: 'help',
13+
alias: 'h',
14+
type: Boolean,
15+
description: 'Print this usage guide.',
16+
},
17+
{
18+
name: 'email',
19+
alias: 'e',
20+
type: String,
21+
defaultValue: 'local-dev@example.com',
22+
description: 'Email for the dev user driving the onboarding (default: local-dev@example.com)',
23+
},
24+
]
25+
26+
const sections = [
27+
{
28+
header: 'Onboard default tenant',
29+
content:
30+
'Runs the same onboarding logic as a real Auth0 signup (AuthService.signinFromSSO -> ' +
31+
'handleOnboard -> TenantService.createOrJoinDefault), without needing a real Auth0 token. ' +
32+
'Creates the default tenant, default segment, settings, and an admin user/tenantUser row. ' +
33+
'Safe to re-run — createOrJoinDefault joins the existing tenant instead of duplicating it.',
34+
},
35+
{
36+
header: 'Options',
37+
optionList: options,
38+
},
39+
]
40+
41+
const usage = commandLineUsage(sections)
42+
const parameters = commandLineArgs(options)
43+
44+
if (parameters.help) {
45+
console.log(usage)
46+
} else {
47+
setImmediate(async () => {
48+
const repoOptions = await SequelizeRepository.getDefaultIRepositoryOptions()
49+
50+
await AuthService.signinFromSSO(
51+
'auth0',
52+
`dev|${parameters.email}`,
53+
parameters.email,
54+
true,
55+
'Local',
56+
'Dev',
57+
'Local Dev',
58+
null,
59+
null,
60+
null,
61+
repoOptions,
62+
)
63+
64+
const tenant = await repoOptions.database.tenant.findOne({ where: {} })
65+
const segment = await repoOptions.database.segment.findOne({
66+
where: { tenantId: tenant.id },
67+
})
68+
69+
console.log(`Tenant: ${tenant.id} (${tenant.name})`)
70+
console.log(`Segment: ${segment.id} (${segment.name})`)
71+
72+
process.exit(0)
73+
})
74+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-- Mailing list (Collaboration)
2+
INSERT INTO "activityTypes" ("activityType", platform, "isCodeContribution", "isCollaboration", description) VALUES
3+
('message', 'mailinglist', false, true, 'Sent a message to a mailing list');
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# ADR-0009: Skip Mailing List Activities With Unparseable/Implausible Dates
2+
3+
**Date**: 2026-07-19
4+
**Status**: accepted
5+
**Deciders**: Uros Marolt
6+
7+
## Context
8+
9+
The mailing list integration (`services/apps/mailing_list_integration`) parses
10+
RFC2822 `Date` headers from archived mailing list messages
11+
(`crowdmail/services/parse/noteren.py`, `parse_email()`) to build each
12+
activity's `timestamp`. During testing against a real archive
13+
(`linux-serial`, mirrored from lore.kernel.org), a message surfaced whose
14+
`Date` header parsed "successfully" via `email.utils.parsedate_tz()` but
15+
produced an implausible year (e.g. `102` instead of a 4-digit year).
16+
Python's stdlib already RFC2822-normalizes legitimate 2-digit years (`68`
17+
`2068`/`02``2002`), so this is not a parsing bug in our port — the
18+
`Date` header itself is malformed in the source archive. Without a guard,
19+
`mktime_tz()` + `strftime("%Y-...")` emits a non-4-digit year (e.g.
20+
`"102-06-12T11:56:20.000000Z"`), which downstream JS `Date` parsing (in
21+
the Node ingestion pipeline) rejects, causing per-message ingestion errors.
22+
23+
## Decision
24+
25+
Reject any parsed date whose year falls outside `1970–9999` by raising
26+
`ValueError` (caught by the existing `except (ValueError, OverflowError)`
27+
block), leaving `timestamp` empty for that message. `list_worker.py` skips
28+
queuing any activity with an empty timestamp. The message is not ingested
29+
as an activity, and its shard head still advances past it (so it is not
30+
retried on subsequent polls).
31+
32+
## Alternatives Considered
33+
34+
### Alternative 1: Heuristically repair malformed years
35+
- **Pros**: Could recover activities that would otherwise be dropped.
36+
- **Cons**: No reliable signal for what year was intended once the header
37+
is already malformed at the source (e.g. `102` — off by 1900? 2000?
38+
truncated?).
39+
- **Why not**: Any heuristic risks silently mis-dating an activity, which
40+
is worse than dropping it — a wrong timestamp corrupts activity
41+
timelines and trend analytics without any visible error.
42+
43+
### Alternative 2: Ingest with the corrupted timestamp as-is
44+
- **Pros**: No data loss.
45+
- **Cons**: Breaks downstream JS `Date` parsing in the ingestion pipeline,
46+
causing hard failures rather than a clean skip.
47+
- **Why not**: A hard pipeline failure for one bad message is worse than
48+
skipping that one message.
49+
50+
## Consequences
51+
52+
### Positive
53+
- Malformed dates never reach the ingestion pipeline as corrupt
54+
timestamps; no pipeline-level failures from a single bad message.
55+
- Matches the existing skip pattern already used for other
56+
unparseable/missing fields in this parser.
57+
58+
### Negative
59+
- Messages with malformed `Date` headers are silently and permanently
60+
dropped — never retried, since the shard head advances past them
61+
regardless. The only visible trace is a `logging.warning()` line.
62+
63+
### Risks
64+
- If a source archive has a systemic date-formatting issue (not just rare
65+
garbage), this could silently drop a meaningful fraction of activities.
66+
Mitigation: the warning log includes the raw header and commit id, so a
67+
spike can be diagnosed via log volume if ever suspected.

docs/adr/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Use the `/adr` skill in Claude Code to record new ADRs or query past decisions.
1515
| [ADR-0006](./0006-database-schema-types-as-source-of-truth.md) | Database schema types as the source of truth | accepted | 2026-07-09 |
1616
| [ADR-0007](./0007-test-factory-primitives-and-defaults.md) | Test factory primitives and defaults | accepted | 2026-07-10 |
1717
| [ADR-0008](./0008-how-we-write-unit-tests.md) | How we write unit tests | accepted | 2026-07-13 |
18+
| [ADR-0009](./0009-mailinglist-skip-unparseable-dates.md) | Skip mailing list activities with unparseable/implausible dates | accepted | 2026-07-19 |
1819

1920
## Why ADRs?
2021

scripts/services/docker/Dockerfile.mailing_list_integration

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ RUN --mount=type=cache,target=/root/.cache/uv \
4141
FROM base AS runner
4242

4343
# Install runtime dependencies, including public-inbox for mirroring lists
44+
# curl is required by public-inbox-clone itself (not pulled in via --no-install-recommends)
4445
RUN apt-get update && apt-get install -y \
4546
ca-certificates \
47+
curl \
4648
git \
4749
public-inbox \
4850
--no-install-recommends \

services/apps/data_sink_worker/src/service/activity.service.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,7 @@ export default class ActivityService extends LoggerBase {
11621162

11631163
// find members to create
11641164
for (const payload of payloadsWithoutDbMembers) {
1165-
const key = `${payload.platform}:${payload.activity.username}`
1165+
const key = `${payload.platform}:${payload.activity.username?.toLowerCase()}`
11661166
if (!membersToCreateMap.has(key)) {
11671167
const segmentIds = new Set<string>()
11681168
segmentIds.add(payload.segmentId)
@@ -1188,7 +1188,7 @@ export default class ActivityService extends LoggerBase {
11881188

11891189
// find object members to create
11901190
for (const payload of payloadsWithoutDbObjectMembers) {
1191-
const key = `${payload.platform}:${payload.activity.objectMemberUsername}`
1191+
const key = `${payload.platform}:${payload.activity.objectMemberUsername?.toLowerCase()}`
11921192
if (!membersToCreateMap.has(key)) {
11931193
const segmentIds = new Set<string>()
11941194
segmentIds.add(payload.segmentId)
@@ -1756,12 +1756,17 @@ export default class ActivityService extends LoggerBase {
17561756
memberType: 'member' | 'objectMember',
17571757
dbMember?: IDbMember,
17581758
): Promise<string | Record<string, unknown> | undefined> {
1759+
const IDENTITY_CONSTRAINTS = new Set([
1760+
'uix_memberIdentities_platform_value_type_verified',
1761+
'uix_memberIdentities_memberId_platform_value_type',
1762+
])
1763+
17591764
const checkForIdentityConstraint = (error: any): boolean => {
17601765
if (
17611766
error.constructor &&
17621767
error.constructor.name === 'DatabaseError' &&
17631768
error.constraint &&
1764-
error.constraint === 'uix_memberIdentities_platform_value_type_verified'
1769+
IDENTITY_CONSTRAINTS.has(error.constraint)
17651770
) {
17661771
return true
17671772
}
@@ -1814,6 +1819,18 @@ export default class ActivityService extends LoggerBase {
18141819
}
18151820
}
18161821

1822+
// Create path: no dbMember to compare against yet (member row doesn't exist locally).
1823+
// The verified-identity index is globally unique, so a found owner IS the canonical
1824+
// member — attach to it instead of failing (covers whole-batch-retry re-attempting a
1825+
// create whose identity insert already succeeded in an earlier attempt).
1826+
if (ownerId && !dbMember) {
1827+
this.log.warn(
1828+
{ memberId: ownerId, identity: conflictIdentity },
1829+
'Verified identity already belongs to an existing member — attaching to it instead of creating a new one',
1830+
)
1831+
return ownerId
1832+
}
1833+
18171834
// Pass 2: if no external conflict, check whether this member already owns the
18181835
// identity (stale-prefetch race). Re-uses the owners map — no extra DB query.
18191836
if (!ownerId && dbMember) {

0 commit comments

Comments
 (0)