Skip to content

feat(micropub): add proposed contacts extension - #368

Merged
davidwkeith merged 4 commits into
mainfrom
codex/micropub-contacts
Jul 22, 2026
Merged

feat(micropub): add proposed contacts extension#368
davidwkeith merged 4 commits into
mainfrom
codex/micropub-contacts

Conversation

@davidwkeith

Copy link
Copy Markdown
Owner

Summary

Packages affected

  • @dwk/micropub

Checklist

  • Read the relevant spec(s) under spec/packages/ and updated them if
    behaviour changed
  • Added/updated colocated tests (src/*.test.ts)
  • Ran the local CI gate: pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test — targeted Micropub typecheck and 124 tests passed; full gate not run
  • Added a changeset (pnpm changeset) if this touches a publishable
    package
  • Updated catalog.json / conformance/status.json if this adds a new
    mountable worker or changes conformance status — not applicable, no new worker

@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.

Automated code review.

CONTRIBUTING.md conformance: PASS. Title/scope, PR template headings, changeset (.changeset/micropub-contacts.md), and spec update are all in order. The unticked "full CI gate" and "catalog.json" boxes carry one-line reasons, which matches the "leave unchecked with a reason" rule — not a violation, but please run the full pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test before taking this out of draft.

Code issues found:

  1. Should-fix — silent data loss on multipart contact create. In handleAction() (packages/micropub/src/handler.ts), the isContactRequest branch returns before the foldUploadedMedia/pendingFiles block runs. A ?q=contact create sent as multipart/form-data with a photo file returns 201 but the file is never uploaded to R2 or folded into properties — silently dropped. Given this repo's "fail loudly, no silent degradation" convention, please either fold uploads for contacts too or explicitly reject multipart contact requests.

  2. Should-fix — missing scope-enforcement test. The new describe("@dwk/micropub proposed contacts") tests never check that a token without the right scope (e.g. create-only attempting action=delete) gets insufficient_scope/403. scopesForAction is reused unmodified from the post path so it's likely correct, but the spec explicitly calls out per-action scoping for contacts and it's currently unverified by any test.

  3. Worth confirming — contacts readable by any authenticated token. handleQuery() calls authorize(..., [], ...), so any valid token (even media-scoped) can read the address book. This mirrors existing q=source behavior, but since the new spec text calls contacts "private owner data," please confirm that's intentional rather than inherited by accident.

  4. NitcontactView() collapses single-value properties to a bare scalar, diverging from the array-always shape sourceView() and the rest of the mf2 JSON surface use. Called out in the added spec text, but flagging since it could surprise clients sharing mf2-parsing code across q=source/q=contact.

  5. Nit — the "URL already in use" conflict message is identical for a genuine URL-identity conflict and a (practically impossible) UUID PK collision; harmless but slightly misleading.

No injection/auth-bypass issues found — parameterized D1 queries throughout, DPoP/scope gating structurally correct, no KV usage.


Generated by Claude Code

@davidwkeith
davidwkeith marked this pull request as ready for review July 22, 2026 14:45
# Conflicts:
#	packages/micropub/README.md
#	packages/micropub/src/config.ts
#	packages/micropub/src/index.ts
#	spec/packages/micropub.md
@davidwkeith
davidwkeith merged commit 2d594d1 into main Jul 22, 2026
5 of 6 checks passed
@davidwkeith
davidwkeith deleted the codex/micropub-contacts branch July 22, 2026 17:09
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