fix(activitypub,safe-fetch,webfinger): cap outbox scan and dedupe timeout signal - #370
Conversation
…eout signal Address two non-blocking follow-ups from #364's review: bound the owner-outbox merge loop in the Mastodon timeline so a like/announce-dominated outbox can't force a near-full-table scan per request, and consolidate the three near-identical createTimeoutSignal implementations into one shared export from @dwk/safe-fetch.
davidwkeith
left a comment
There was a problem hiding this comment.
Automated review pass.
CONTRIBUTING.md conformance: looks good — PR title follows Conventional Commits (fix(activitypub,safe-fetch,webfinger): cap outbox scan and dedupe timeout signal), the template's Summary/Packages affected/Checklist headings are intact with unchecked items given a one-line reason, a changeset is included for all three touched publishable packages, and colocated tests were added (mastodon-api.test.ts).
Correctness of the fix: the MAX_OUTBOX_SCAN_BATCHES cap and the new createTimeoutSignal(callerSignal, timeoutMs) signature both look correct. I traced the boundary case in the new test (1000 buried-behind-likes row, BATCH=40, cap=25 ⇒ exactly 1000 rows scanned) and the off-by-one lines up with the assertion.
One thing worth fixing or at least tracking, since it's exactly the class of issue this PR is hardening against:
packages/activitypub/src/object.ts— the sibling scan loop just above the one this PR caps (#serveClientList's inbox batch loop,while (matches.length < limit && !exhausted)starting around line 1384) has the identical unbounded-scan shape the outbox loop had before this PR: forkind: "notifications", an inbox dominated by plainCreate/Updaterows (no favourites/reblogs/mentions) will scan the whole table the same way a like-dominated outbox did for the timeline merge. Since the PR's own rationale is "an owner outbox dominated by non-post activities... forces a near-full-table scan," the same argument applies here and it'd be inconsistent to cap one and not the other. Worth either capping it in this PR with the sameMAX_..._BATCHESpattern, or filing a fast follow-up issue so it isn't lost.
Everything else — the @dwk/safe-fetch de-duplication, the @dwk/webfinger → @dwk/safe-fetch dependency addition (both are pure/Node-testable per the composition contract, so no Cloudflare-specifics leak), and the timeout.ts deletion (confirmed no remaining references) — looks correct and matches repo conventions.
Generated by Claude Code
…ore-build and cap inbox scan Add the missing @dwk/safe-fetch path mapping to every tsconfig.json that transitively pulls in @dwk/webfinger's source (host-meta, remotestorage, webfinger itself) — CI runs typecheck before build, so these packages could not resolve webfinger's new @dwk/safe-fetch import from source. Also cap the inbox notifications scan loop with the same MAX_SCAN_BATCHES bound already applied to the outbox merge, per review: a notifications page over an inbox dominated by plain Create/Update rows had the identical unbounded-scan shape.
|
Addressed in a6436f7: the inbox notifications batch loop in Generated by Claude Code |
Summary
Follow-up on two non-blocking suggestions from the automated review on #364 (already merged), which the PR author asked to have addressed:
#serveClientListmerges owner posts into a Mastodon timeline page. Without a cap, an owner outbox dominated by non-post activities (Like/Announce/etc.) forces a near-full-table scan per timeline request. PastMAX_OUTBOX_SCAN_BATCHES(25 batches), the page now simply returns fewer thanlimitowner posts instead of exhausting the table.@dwk/activitypub'stimeout.ts, inline in@dwk/safe-fetch, inline in@dwk/webfinger).@dwk/safe-fetchnow exportscreateTimeoutSignal;@dwk/activitypuband@dwk/webfingerboth import it instead of carrying their own copy.@dwk/webfingergains a new@dwk/safe-fetchdependency (both are pure, Node-testable, protocol-agnostic libs — no new runtime coupling).Packages affected
@dwk/activitypub, @dwk/safe-fetch, @dwk/webfinger
Checklist
spec/packages/and updated them if behaviour changed — not applicable; internal hardening/refactor, no observable behaviour or contract changesrc/*.test.ts)pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test— all passedpnpm changeset) if this touches a publishable packagecatalog.json/conformance/status.jsonif this adds a new mountable worker or changes conformance status — not applicable; no worker or status changeGenerated by Claude Code