Canonicalize and compare portable ActivityPub URIs#924
Conversation
Add vocab-runtime helpers for canonicalizing and comparing FEP-ef61 portable URI identifiers. The comparison form accepts both ap: and ap+ef61:, decodes DID authorities, strips query hints, and preserves path and fragment identity. Document the helper behavior and cover decoded, encoded, query, fragment, casing, and rejection cases in the URL runtime tests. Closes fedify-dev#828 Assisted-by: Codex:gpt-5.5
✅ Deploy Preview for fedify-json-schema canceled.
|
|
@codex review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesPortable URI Comparison Feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URIs. The review feedback highlights a potential issue where comparisons could fail if percent-encoded characters in the DID authority differ in casing (e.g., %2f vs %2F), and suggests normalizing these percent-encodings to uppercase to ensure robust comparisons.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b3123164f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vocab-runtime/src/url.ts`:
- Around line 66-85: Add a brief inline comment in canonicalizePortableUri near
the decodePortableAuthority(parsed.host) call explaining that parsed.host comes
from parsePortableIri as an encodeURIComponent-escaped authority, so this path
relies on the generic percent-decoding branch rather than the DID_SCHEME_PATTERN
branch. Mention that this is the inverse of the earlier encoding step and is
shared behavior with decodePortableAuthority.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75db782a-828b-44b2-b517-245b6624fff6
📒 Files selected for processing (5)
CHANGES.mddocs/manual/vocab.mdpackages/vocab-runtime/src/mod.tspackages/vocab-runtime/src/url.test.tspackages/vocab-runtime/src/url.ts
Keep the comparison canonicalizer from using URL-normalized path components, because FEP-ef61 treats portable URI paths as opaque object identity strings. Also normalize percent-encoding case in DID authorities so equivalent authority spellings compare consistently. fedify-dev#924 (comment) fedify-dev#924 (comment) fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces two new helper functions, canonicalizePortableUri() and arePortableUrisEqual(), to the @fedify/vocab-runtime package to facilitate the comparison of FEP-ef61 portable ActivityPub URIs. The changes include comprehensive unit tests and documentation updates. Feedback on the code suggests removing a redundant runtime null check in canonicalizePortableUri() since the URI format is already validated by a preceding helper function.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8b53a3a9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vocab-runtime/src/url.test.ts`:
- Around line 258-279: Add a regression test in canonicalizePortableUri()
coverage for URL-object inputs with dot segments, since string-based cases
already cover opaque-path preservation but parseIri() / URL instances normalize
dot segments before canonicalizePortableUri() runs. Use the existing url.test.ts
suite and the canonicalizePortableUri() / arePortableUrisEqual() helpers to
assert the current behavior for a URL object with a path like a/../b, so the
behavior is explicit even if it differs from string inputs.
In `@packages/vocab-runtime/src/url.ts`:
- Around line 76-95: The canonicalization logic in canonicalizePortableUri is
inconsistent for string versus URL inputs because reading input.href on a URL
object can already normalize dot segments and change the preserved portable IRI
identity. Update canonicalizePortableUri to accept only raw string input or
explicitly document this limitation in the helper’s API/docs, and make sure the
behavior around parsePortableIri, PORTABLE_IRI_PATTERN, and the raw
path/fragment handling stays consistent with that choice.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6203fb9f-8d07-461a-8b41-9163e4e6ef01
📒 Files selected for processing (2)
packages/vocab-runtime/src/url.test.tspackages/vocab-runtime/src/url.ts
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Make the portable URI comparison helpers accept raw strings only, since URL objects may already have normalized opaque path segments before the helpers can compare identity. Keep the redundant pattern check out of the validated path and document the string-only comparison contract. fedify-dev#924 (comment) fedify-dev#924 (comment) fedify-dev#924 (comment) fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URI strings, along with associated documentation and tests. The review feedback highlights two key improvement opportunities: first, arePortableUrisEqual should handle non-portable URIs gracefully by falling back to direct string comparison instead of throwing a TypeError to prevent runtime crashes; second, canonicalizePortableUri should normalize percent-encoding casing in the path and fragment components as well as the authority component to ensure compliance with RFC 3986.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a6b24ac8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vocab-runtime/src/url.ts (1)
92-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePercent-encoding case normalization is authority-only; path/fragment are byte-compared.
normalizePercentEncodingis applied to the decoded authority but not tomatch[3](path) ormatch[5](fragment), so two portable URIs differing only in pct-encoding hex casing within the path/fragment (e.g.%2fvs%2F) will not be treated as equal byarePortableUrisEqual, unlike equivalent authority differences which are normalized. This appears intentional given the opaque-path-preservation design and existing tests, but a short comment noting the asymmetry would help future maintainers avoid assuming full pct-encoding normalization.Also applies to: 104-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vocab-runtime/src/url.ts` around lines 92 - 95, Add a short explanatory comment in arePortableUrisEqual/normalizePortableUriUri handling near the authority reconstruction in url.ts to note that normalizePercentEncoding is intentionally applied only to the authority, while match[3] and match[5] (path and fragment) are compared byte-for-byte and keep pct-encoding case differences significant; use the existing decodePortableAuthority and normalizePercentEncoding logic as the reference point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/vocab-runtime/src/url.ts`:
- Around line 92-95: Add a short explanatory comment in
arePortableUrisEqual/normalizePortableUriUri handling near the authority
reconstruction in url.ts to note that normalizePercentEncoding is intentionally
applied only to the authority, while match[3] and match[5] (path and fragment)
are compared byte-for-byte and keep pct-encoding case differences significant;
use the existing decodePortableAuthority and normalizePercentEncoding logic as
the reference point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 065bf493-88bd-4127-8a69-eebfe92b8bbb
📒 Files selected for processing (4)
CHANGES.mddocs/manual/vocab.mdpackages/vocab-runtime/src/url.test.tspackages/vocab-runtime/src/url.ts
Clarify that portable URI comparison normalizes percent-encoding case only for the DID authority. Path and fragment components remain byte-for-byte from the raw match because FEP-ef61 treats portable object paths as opaque identity strings. fedify-dev#924 (review) Assisted-by: Codex:gpt-5.5
Normalize percent-escape hex casing in preserved path and fragment components, while still reading those components from the raw URI string so URL dot-segment normalization cannot collapse portable object identity. Also make arePortableUrisEqual() fall back to direct string comparison for non-portable strings. Portable-looking inputs still use the strict portable URI parser, so malformed portable authorities continue to fail instead of being silently accepted. fedify-dev#924 (comment) fedify-dev#924 (comment) fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the canonicalizePortableUri() and arePortableUrisEqual() helper functions to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URI strings. These helpers normalize ap: and ap+ef61: schemes, decode DID authorities, handle percent-encoding normalization, and strip query hints such as gateways. The changes also include comprehensive unit tests and updated documentation. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c6cc4c25c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Validate path and fragment percent escapes before canonicalizing portable URI components. This keeps malformed inputs from being normalized into the same canonical form as a different valid URI string during equality checks. fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces two new helper functions, canonicalizePortableUri() and arePortableUrisEqual(), to the @fedify/vocab-runtime package. These helpers facilitate the comparison of FEP-ef61 portable ActivityPub URI strings by normalizing schemes, decoding DID authorities, and ignoring query hints such as gateways. The changes include implementation, extensive unit tests, and updated documentation. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8478f3b7ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Normalize over-encoded unreserved characters in portable DID authorities so URL-safe spellings compare equal to their decoded form. Reserved DID-internal escapes such as encoded slashes and colons remain percent-encoded to preserve existing authority identity semantics. fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URI strings, along with corresponding documentation and comprehensive unit tests. The review feedback highlights an issue in normalizePortableComponent where percent-encoded unreserved characters in the path and fragment components are not decoded, potentially causing false negatives during URI comparison. A code suggestion is provided to decode these unreserved characters to ensure RFC 3986 compliance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca7b4eec40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Normalize percent-encoded unreserved characters in portable URI paths and fragments so escaped and decoded spellings compare equal. Reserved escapes remain percent-encoded, and raw dot segments are still preserved instead of being collapsed through URL pathname normalization. fedify-dev#924 (comment) fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() helper functions to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URI strings. It also includes documentation updates and comprehensive unit tests. The reviewer suggested simplifying the regular expression and conditional checks in normalizePortableComponent() since invalid percent-encodings are already validated and rejected at the start of the function.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Remove the redundant bare-percent branch from portable component normalization. Invalid percent escapes are already rejected before matching, so percent matches are always complete escape triplets. fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() to @fedify/vocab-runtime for comparing FEP-ef61 portable ActivityPub URI strings, along with corresponding documentation and comprehensive unit tests. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faeab95d5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Raw portable URI paths and fragments can contain unpaired UTF-16 surrogates when they arrive from decoded JSON strings. Convert the URIError from encodeURI() into the same TypeError used for other invalid portable components so canonicalization and comparison keep their public contracts. fedify-dev#924 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces canonicalizePortableUri() and arePortableUrisEqual() to the @fedify/vocab-runtime package to facilitate the comparison of FEP-ef61 portable ActivityPub URI strings. These helpers normalize URIs to the ap+ef61: scheme, decode DID authorities, and ignore query hints such as gateways. The changes include comprehensive unit tests and documentation updates. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #828.
What changed
This adds
canonicalizePortableUri()andarePortableUrisEqual()to@fedify/vocab-runtimefor comparing FEP-ef61 portable ActivityPub URI identifiers.It also documents the new helpers in docs/manual/vocab.md, exports them from packages/vocab-runtime/src/mod.ts, adds regression coverage in packages/vocab-runtime/src/url.test.ts, and records the change in CHANGES.md.
How it works
The new canonicalization helper reuses the existing portable IRI parser in packages/vocab-runtime/src/url.ts instead of introducing a second parser. That keeps validation behavior aligned with the URI handling added for #826: both
ap:andap+ef61:inputs are accepted, decoded and percent-encoded DID authorities are normalized through the same code path, malformed portable authorities are rejected consistently, and JavaScriptURLinstances using URL-safe authorities work the same way as string inputs.For comparison, the helper emits a string form with the
ap+ef61:scheme, a decoded DID authority, no query component, and the original path and fragment. Removing the query component follows FEP-ef61’s comparison rule, which treats query parameters such asgatewaysas location hints rather than object identity. Preserving the path and fragment keeps the identity boundary strict where the URI syntax still carries object-specific meaning.The canonicalization also normalizes only the leading DID scheme from
DID:todid:. It deliberately does not lowercase the whole DID authority, because DID method-specific identifiers may have their own case-sensitive rules. This matches the existing portable-origin comparison behavior while avoiding a broader transformation than the runtime already relies on.arePortableUrisEqual()is intentionally a thin wrapper overcanonicalizePortableUri(). That keeps the public API simple and makes equality semantics auditable from a single canonical form rather than duplicating comparison logic.Why this approach
FEP-ef61 distinguishes serialization from comparison. Fedify already serializes portable IRIs with
formatIri(), and serialization needs to preserve query hints because they can help consumers find a portable actor through gateways. Object identity comparison has a different rule: query hints must be ignored. Adding a separate comparison helper avoids changing serialization behavior for generated vocabulary classes while still giving callers a spec-aware way to compare portable identifiers.The implementation stays scoped to issue #828. It does not convert compatible HTTP gateway identifiers, dereference gateways, resolve DID documents, verify FEP-8b32 proofs, or extend FEP-fe34 cryptographic origin checks. Those concerns depend on gateway and trust-model behavior outside this issue, while the comparison helper is a small runtime primitive that later work can build on.
The tests cover decoded and encoded authorities, both supported schemes, query stripping, path and fragment preservation, DID-internal percent escapes, DID scheme casing, non-equivalent authorities, non-equivalent paths, non-equivalent fragments, and unsupported or malformed inputs. These cases are intended to pin down the boundary between accepted portable URI spellings and the narrower canonical form used for comparison.
Verification
mise run test:deno packages/vocab-runtime/src/url.test.tsmise run check-each vocab-runtimemise run check:mdmise run test-each vocab-runtimemise run test-each vocab-runtimecompleted successfully. The selected package reported that it has notest:bunscript.