Skip to content

fix(activitypub): give follow-request delivery tests a signing key - #516

Merged
davidwkeith merged 1 commit into
mainfrom
fix/activitypub-follow-request-test-signer
Sep 3, 2026
Merged

davidwkeith merged 1 commit into
mainfrom
fix/activitypub-follow-request-test-signer

Conversation

@davidwkeith

Copy link
Copy Markdown
Owner

Summary

The release dry run for 1.0.0-beta.5 failed on mastodon-api.test.tsrespondToFollowRequest(actor, 'authorize') … again, after #514's alarm-order hardening. Root cause, finally: the two follow-request tests build their config with freshConfig()no privateKeyPem — so when the real DO alarm wins the race against the test's /__resolve drive, #processDeliveries finds no signer and drops the queued Accept as undeliverable: no fetch (nothing for the stub to record), no row (nothing to count). The test's own drive only runs the pending-accept pass, never delivery, which is why it passes whenever it gets there first.

Fix: pass the file's generated privateKeyPem to both tests, exactly as the outbox-delivery test at the top of the file already does. The alarm's delivery pass then POSTs through the fetch stub (authorize) or fails retryably against the unresolvable remote.example and keeps the row (reject). Verified 6/6 local runs; test-only, no changeset.

Packages affected

@dwk/activitypub (tests only)

Checklist

  • Read the relevant spec(s) under spec/packages/ and updated them if
    behaviour changed — not applicable: no behaviour change
  • Added/updated colocated tests (src/*.test.ts)
  • Ran the local CI gate: pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test — lint/format on the changed file, mastodon-api project run 6×; CI runs the full gate here
  • Added a changeset (pnpm changeset) if this touches a publishable
    package — not applicable: test-only, not user-visible
  • Updated catalog.json / conformance/status.json if this adds a new
    mountable worker or changes conformance status — not applicable

🤖 Generated with Claude Code

The 'authorize'/'reject' `respondToFollowRequest` tests built their config
without `privateKeyPem`. When the real DO alarm won the race against the
test's explicit `/__resolve` drive, `#processDeliveries` found no signer
and dropped the queued Accept/Reject as undeliverable — no fetch, no row,
nothing for the assertions to count (release dry run: "expected [] to have
a length of 1"). With the file's generated key the alarm's delivery pass
POSTs through the fetch stub (authorize) or fails retryably and keeps the
row (reject), so the outcome no longer depends on alarm scheduling.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@davidwkeith davidwkeith left a comment

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.

Reviewed against CONTRIBUTING.md and the actual #processDeliveries code path.

Correctness: confirmed. object.ts #processDeliveries (line ~3134-3139) does exactly what the PR description says: if (!signer) { ...DELETE FROM delivery...; continue; }. Since freshConfig() builds a ResolvedConfig with no privateKeyPem, any real alarm that wins the race against the test's own /__resolve drive silently drops the queued Accept/Reject row before the assertions ever see it. Passing privateKeyPem (already generated in beforeAll and already used by the outbox-delivery test earlier in the same file) closes that gap by making the alarm's delivery attempt retryable instead of a silent drop. Both tests already account for either interleaving (real alarm winning vs. the test's own /__resolve drive winning) by checking delivery + in-flight fetch stub / pending_accept in combination, so the fix doesn't just paper over the flake — it makes the outcome deterministic either way.

CONTRIBUTING.md conformance:

  • PR title fix(activitypub): give follow-request delivery tests a signing key — correct Conventional Commits form (lowercase type, scope = package name minus @dwk/ prefix, no trailing period).
  • Body uses the PR template's Summary / Packages affected / Checklist headings verbatim, with the unchecked boxes (spec update, changeset, catalog/status.json) each given a one-line applicability reason rather than deleted — matches §7's requirement.
  • Test-only change with colocated test updates, no behaviour change → no changeset and no spec update needed, correctly reasoned in the checklist.
  • Comments added to the two tests explain the why (race with the real alarm) rather than restating the diff — consistent with this repo's comment style.

No changes requested — nice, minimal fix with good reasoning captured in-line for the next person who hits this.


Generated by Claude Code

@davidwkeith
davidwkeith merged commit bd0ad3f into main Sep 3, 2026
8 checks passed
@davidwkeith
davidwkeith deleted the fix/activitypub-follow-request-test-signer branch September 3, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant