Skip to content

FEP-ef61 portable object vocabulary#928

Open
dahlia wants to merge 6 commits into
fedify-dev:mainfrom
dahlia:feat/fep-ef61/vocab-terms
Open

FEP-ef61 portable object vocabulary#928
dahlia wants to merge 6 commits into
fedify-dev:mainfrom
dahlia:feat/fep-ef61/vocab-terms

Conversation

@dahlia

@dahlia dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member

Why this belongs in the vocabulary layer

FEP-ef61 makes portable ActivityPub objects useful only if their supporting terms can move through the same typed JSON-LD path as the rest of Fedify's vocabulary. Treating gateways and digestMultibase as ad hoc extension properties would leave users with raw JSON handling at exactly the point where portable objects need predictable parsing, ordering, and serialization.

This PR adds those terms to the vocabulary schemas instead. Actor gateway ordering is represented as a JSON-LD list because the FEP gives that order meaning. Media integrity metadata is attached to links and document-like objects through the Data Integrity v2 term, so compacted and expanded JSON-LD use the same identifier that FEP-ef61 expects.

How the contexts are wired

The FEP-ef61 context is vendored into packages/vocab-runtime/src/contexts and registered with the preloaded document loader. Data Integrity v2 is also available through the existing context registry path. That keeps normal compaction and expansion offline for the new terms, which matters because these contexts are part of the library's core vocabulary behavior rather than application data that should require a network fetch.

The vocabulary generator's context heuristics were updated so gateways is handled as an IRI-valued list and digestMultibase resolves to the credentials v2 term. The generated tests and snapshots then exercise the same code paths users get from fromJsonLd() and toJsonLd(), instead of only checking hand written helper behavior.

Documentation shape

The manual now has a dedicated FEP-ef61 portable objects section in docs/manual/vocab.md. The previous portable ID notes were useful, but they were split across general instantiation and security-model text. Keeping portable IDs, gateway ordering, digest metadata, and origin comparison in one place should make the feature easier to find and harder to misunderstand.

Validation

I checked the focused FEP-ef61 vocabulary tests first, then regenerated the affected snapshots for Deno, Node.js, and Bun. After the handler expectation was updated for the new actor context, the full mise test command completed successfully.

Expose FEP-ef61 gateways on actor classes and digestMultibase on
links and document-like media objects.  Preload the FEP-ef61 and Data
Integrity v2 contexts so these terms compact and expand without remote
context fetches.

Update the vocabulary manual with a dedicated portable objects section
and record the change for the planned PR.

fedify-dev#830
fedify-dev#928

Assisted-by: Codex:gpt-5.5
@dahlia dahlia added this to the Fedify 2.4 milestone Jul 9, 2026
@dahlia dahlia self-assigned this Jul 9, 2026
@dahlia dahlia requested review from 2chanhaeng and sij411 as code owners July 9, 2026 07:46
@dahlia dahlia added component/vocab Activity Vocabulary related activitypub/compliance Specification compliance component/vocab-tools Vocabulary code generation (@fedify/vocab-tools) labels Jul 9, 2026
@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy Preview for fedify-json-schema canceled.

Name Link
🔨 Latest commit bb6e70d
🔍 Latest deploy log https://app.netlify.com/projects/fedify-json-schema/deploys/6a4f7090fd18e4000797d988

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for FEP-ef61 portable ActivityPub objects, introducing the gateways property to actor classes and digestMultibase to link and document/media classes, along with updating documentation, fixtures, and tests. The review feedback correctly points out that while the https://w3id.org/security/data-integrity/v2 context is declared as preloaded and used in several vocabulary files, it is missing its actual import and registration in the preloaded contexts registry, which would cause offline expansion and compaction to fail.

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.

Comment thread packages/vocab-runtime/src/contexts.ts
Comment thread packages/vocab-runtime/src/contexts.ts
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds FEP-ef61 support for portable ActivityPub objects by introducing a preloaded JSON-LD context, updating vocab schemas and defaults, expanding fixtures and tests, and documenting portable object handling and related security-model wording.

Changes

FEP-ef61 Vocabulary and Runtime Context Support

Layer / File(s) Summary
Preloaded FEP-ef61 context and gateway URL helpers
packages/vocab-runtime/src/contexts.ts, packages/vocab-runtime/src/contexts/fep-ef61.json, packages/vocab-runtime/src/docloader.ts, packages/vocab-runtime/src/contexts/security-data-integrity-v1.json, packages/vocab-runtime/src/url.ts, packages/vocab-runtime/src/mod.ts, packages/vocab-tools/src/type.ts, packages/vocab-tools/src/class.ts, packages/fixture/src/fixtures/w3id.org/security/data-integrity/v2.json, packages/vocab-runtime/src/url.test.ts
Adds the FEP-ef61 context, registers it as preloaded, adds gateway URL validation/parsing and scalar wiring, extends heuristic context handling, and updates supporting runtime context material.
JSON-LD fixture for FEP-ef61 context
packages/fixture/src/fixtures/w3id.org/fep/ef61.json, packages/fixture/src/fixtures/w3id.org/security/data-integrity/v2.json
Adds the fixture documents defining gateways, digestMultibase, and Data Integrity v2 context mappings.
Vocabulary schema updates: gateways and digestMultibase properties
packages/vocab/src/application.yaml, packages/vocab/src/audio.yaml, packages/vocab/src/document.yaml, packages/vocab/src/group.yaml, packages/vocab/src/image.yaml, packages/vocab/src/link.yaml, packages/vocab/src/organization.yaml, packages/vocab/src/page.yaml, packages/vocab/src/person.yaml, packages/vocab/src/service.yaml, packages/vocab/src/video.yaml
Updates default contexts across vocab entities and adds gateways and digestMultibase properties where applicable.
Link-to-Image digestMultibase preservation
packages/vocab/src/preprocessors.ts
Copies digestMultibase when normalizing a Link into an Image.
Vocab, URL, and handler tests for FEP-ef61
packages/vocab/src/vocab.test.ts, packages/vocab-runtime/src/url.test.ts, packages/fedify/src/federation/handler.test.ts
Adds FEP-ef61 coverage for gateways/digestMultibase round-trips, gateway URL parsing, and expected JSON-LD contexts.
Documentation and changelog updates
CHANGES.md, docs/manual/vocab.md
Documents portable object handling, instantiation guidance, and origin-based security wording changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • fedify-dev/fedify#914: Extends the same JSON-LD context-preload mechanism by registering a new preloadedContexts entry.
  • fedify-dev/fedify#924: Shares the portable-URI comparison helpers referenced by the FEP-ef61 documentation and tests.

Suggested reviewers: sij411, 2chanhaeng

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately highlights the main addition of FEP-ef61 portable object vocabulary support.
Description check ✅ Passed The description is clearly related to the changeset and explains the vocabulary, context, documentation, and validation updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 612ca3d2cb

ℹ️ 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".

Comment thread packages/vocab-runtime/src/contexts/fep-ef61.json
Comment thread packages/vocab/src/link.yaml
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.44444% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/vocab-tools/src/type.ts 70.83% 7 Missing ⚠️
Files with missing lines Coverage Δ
packages/vocab-runtime/src/contexts.ts 100.00% <100.00%> (ø)
packages/vocab-runtime/src/docloader.ts 93.93% <ø> (ø)
packages/vocab-runtime/src/mod.ts 100.00% <100.00%> (ø)
packages/vocab-runtime/src/url.ts 90.43% <100.00%> (+0.31%) ⬆️
packages/vocab-tools/src/class.ts 98.95% <100.00%> (+0.01%) ⬆️
packages/vocab/src/preprocessors.ts 90.32% <100.00%> (+0.32%) ⬆️
packages/vocab-tools/src/type.ts 83.88% <70.83%> (-0.52%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@docs/manual/vocab.md`:
- Around line 212-216: The example in the vocabulary docs uses an unencoded DID
authority that is not a valid copy/paste URI literal. Update the portable
ActivityPub IRI example in the relevant docs section to use the percent-encoded
authority form, keeping the surrounding explanation in sync with the accepted
`ap:` and `ap+ef61:` forms and the canonical serialization behavior described
there.
🪄 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: e00da8bc-eb1f-4a6e-bfd7-5136c2bf1b76

📥 Commits

Reviewing files that changed from the base of the PR and between a974c7d and 612ca3d.

⛔ Files ignored due to path filters (4)
  • packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snap is excluded by !**/*.snap
  • packages/vocab-tools/src/__snapshots__/class.test.ts.node.snap is excluded by !**/*.snap
  • packages/vocab-tools/src/__snapshots__/class.test.ts.snap is excluded by !**/*.snap
  • packages/vocab/src/__snapshots__/vocab.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (21)
  • CHANGES.md
  • docs/manual/vocab.md
  • packages/fedify/src/federation/handler.test.ts
  • packages/fixture/src/fixtures/w3id.org/fep/ef61.json
  • packages/fixture/src/fixtures/w3id.org/security/data-integrity/v2.json
  • packages/vocab-runtime/src/contexts.ts
  • packages/vocab-runtime/src/contexts/fep-ef61.json
  • packages/vocab-runtime/src/docloader.ts
  • packages/vocab-tools/src/type.ts
  • packages/vocab/src/application.yaml
  • packages/vocab/src/audio.yaml
  • packages/vocab/src/document.yaml
  • packages/vocab/src/group.yaml
  • packages/vocab/src/image.yaml
  • packages/vocab/src/link.yaml
  • packages/vocab/src/organization.yaml
  • packages/vocab/src/page.yaml
  • packages/vocab/src/person.yaml
  • packages/vocab/src/service.yaml
  • packages/vocab/src/video.yaml
  • packages/vocab/src/vocab.test.ts

Comment thread docs/manual/vocab.md Outdated
Keep the vendored Data Integrity v1 context from redefining the
digestMultibase term that FEP-ef61 uses for credentials v2 digests.
Also carry digestMultibase through Link-to-Image normalization and fix
the portable URI example to use an encoded DID authority.

fedify-dev#928 (comment)
fedify-dev#928 (comment)
fedify-dev#928 (comment)

Assisted-by: Codex:gpt-5.5
@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for FEP-ef61 portable ActivityPub objects. It introduces the gateways property for actor classes (Application, Group, Organization, Person, Service) and the digestMultibase property for Link and document/media classes (Audio, Document, Image, Video, Page). Additionally, it updates the preloaded JSON-LD contexts, document loader, and property preprocessors to support these terms, and adds comprehensive tests. A review comment correctly points out an incorrect example in the documentation where a percent-encoded DID authority is labeled as decoded.

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.

Comment thread docs/manual/vocab.md Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd8abc8bf6

ℹ️ 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".

Comment thread packages/vocab/src/person.yaml Outdated
Treat actor gateways as their own URL scalar so generated constructors
and JSON-LD decoding both enforce the FEP-ef61 gateway shape.  Gateways
now have to be HTTP(S) base URIs, which keeps portable actor data from
serializing values that downstream dereferencing code should reject.

The manual now avoids calling percent-encoded DID authorities decoded
input, and documents the gateway base URI requirement next to the actor
example.

fedify-dev#928 (comment)
fedify-dev#928 (comment)

Assisted-by: Codex:gpt-5.5
@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for FEP-ef61 portable ActivityPub objects, adding the gateways list to actor classes and digestMultibase to link and media classes, along with preloaded JSON-LD contexts and validation helpers. The review feedback highlights a potential runtime crash in dataCheck when handling null values, advises against modifying standard vendored context files (such as security-data-integrity-v1.json) to resolve term conflicts, and suggests strengthening gateway URL validation to explicitly reject URLs containing user credentials.

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.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/vocab-tools/src/type.ts (340-344)

high

In JavaScript/TypeScript, typeof null evaluates to "object". If ${v} is null, the check typeof ${v} === "object" will pass, and the subsequent "@value" in ${v} expression will throw a TypeError: Cannot use 'in' operator to search for '@value' in null. Adding a null check (${v} !== null) prevents this runtime crash when parsing properties with null values.

    dataCheck(v) {
      return "${v} !== null && typeof ${v} === \"object\" && \"@value\" in ${v}\n        && typeof ${v}[\"@value\"] === \"string\"\n        && ${v}[\"@value\"] !== \"\" && ${v}[\"@value\"] !== \"/\"";
    },

packages/vocab-runtime/src/contexts/security-data-integrity-v1.json (6-9)

high

Modifying standard, third-party vendored context files (like security-data-integrity-v1.json) to resolve term conflicts is highly discouraged. It breaks offline-online parity and interoperability for other documents that use the standard security-data-integrity-v1 context and expect digestMultibase to resolve to https://w3id.org/security#digestMultibase.

Instead, term overriding should be handled by context ordering (e.g., placing the FEP-ef61 context after the Data Integrity v1 context in the document's @context array) or via custom document loader mapping if necessary.

packages/vocab-runtime/src/url.ts (329-332)

medium

A gateway URL should not contain user credentials (username and password) to prevent potential credential leakage or malformed URLs. Checking url.username === "" and url.password === "" ensures that only clean base URIs are accepted.

export function isGatewayUrl(url: URL): boolean {
  return (url.protocol === "http:" || url.protocol === "https:") &&
    url.pathname === "/" && url.search === "" && url.hash === "" &&
    url.username === "" && url.password === "";
}

packages/vocab-runtime/src/url.test.ts (649-656)

medium

Add a test case to verify that gateway URLs with credentials (e.g., https://user:pass@server.example/) are correctly rejected.

  for (
    const url of [
      "ftp://server.example/",
      "https://server.example/path",
      "https://server.example/?x=1",
      "https://server.example/#fragment",
      "https://user:pass@server.example/",
    ]
  ) {

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e1601861c

ℹ️ 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".

Comment thread packages/vocab-tools/src/type.ts
FEP-ef61 gateways are dereferenceable URI references, so generated
JSON-LD should emit them as @id entries instead of value literals.  The
decoder still accepts the existing literal form because the bundled FEP
context can produce it, but @id typed contexts now round-trip through the
same base URI validation.

fedify-dev#928 (comment)

Assisted-by: Codex:gpt-5.5
@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for FEP-ef61 portable ActivityPub objects, introducing gateways lists for actor classes and digestMultibase for links and media/document classes. It also adds preloaded JSON-LD contexts and utility functions (isGatewayUrl, parseGatewayUrl) to validate gateway URLs as HTTP(S) base URIs. Feedback from the review suggests strengthening the gateway URL validation in isGatewayUrl to explicitly reject URLs containing userinfo (username and password) to prevent potential credential leakage, along with adding corresponding test cases.

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.

Comment thread packages/vocab-runtime/src/url.ts
Comment thread packages/vocab-runtime/src/url.test.ts
Comment thread packages/vocab/src/vocab.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-tools/src/type.ts`:
- Around line 340-348: The dataCheck and decoder logic in the type definition
are out of sync: dataCheck only accepts the `@id` branch when `@id` is a non-empty
string, but decoder still chooses `@id` based on presence alone. Update decoder to
use the same type-guarding rules as dataCheck in this type.ts definition, likely
by checking that `@id` is a string before passing it to parseGatewayUrl and
otherwise falling back to `@value`, so malformed objects cannot crash decoding.
🪄 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: efd1917c-9a2a-4d8b-8e81-11669539059a

📥 Commits

Reviewing files that changed from the base of the PR and between 8e16018 and 1840cf4.

⛔ Files ignored due to path filters (3)
  • packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snap is excluded by !**/*.snap
  • packages/vocab-tools/src/__snapshots__/class.test.ts.node.snap is excluded by !**/*.snap
  • packages/vocab-tools/src/__snapshots__/class.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • packages/vocab-tools/src/type.ts
  • packages/vocab/src/vocab.test.ts

Comment thread packages/vocab-tools/src/type.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1840cf4398

ℹ️ 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".

Comment thread packages/vocab-runtime/src/contexts/fep-ef61.json Outdated
Gateway URLs cannot carry credentials, so reject userinfo at the shared
runtime predicate before those values can be accepted by generated vocab
constructors or JSON-LD input.

The generated decoder now mirrors the scalar data guard before choosing
between @id and legacy @value input.  The FEP-ef61 context also types
gateways as IRI references so compact and expanded forms agree.

fedify-dev#928 (comment)
fedify-dev#928 (comment)
fedify-dev#928 (comment)
fedify-dev#928 (comment)
fedify-dev#928 (comment)

Assisted-by: Codex:gpt-5.5
@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for FEP-ef61 portable ActivityPub objects. It adds the gateways property to actor classes and the digestMultibase property to Link and document/media classes, preloads the FEP-ef61 JSON-LD context, and introduces helper functions to parse and validate gateway base URIs. The review feedback highlights a mismatch in the @type definition for gateways in the fixture context compared to the runtime context, suggesting it be updated to @id to ensure consistent JSON-LD compaction and expansion behavior.

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.

Comment thread packages/fixture/src/fixtures/w3id.org/fep/ef61.json Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6adeadbda

ℹ️ 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".

Comment thread packages/vocab/src/document.yaml Outdated
Media documents now define only the credentials v2 digestMultibase
term inline when they also include the Data Integrity v2 context.  This
keeps compact output stable without publishing two contexts that define
the same protected term differently.

The FEP-ef61 fixture context also mirrors the runtime gateway mapping so
fixture-based expansion treats gateways as IRI references.

fedify-dev#928 (comment)
fedify-dev#928 (comment)

Assisted-by: Codex:gpt-5.5
@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@dahlia

dahlia commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for FEP-ef61 portable ActivityPub objects, adding vocabulary terms such as gateways for actor classes and digestMultibase for links and media documents, along with corresponding JSON-LD contexts, validation helpers, and tests. The review feedback highlights a critical issue where modifying the vendored security-data-integrity-v1.json context file to remove digestMultibase will break signature verification. Additionally, it is recommended to make the digestMultibase property non-functional in both document.yaml and link.yaml to support multiple digests as specified by the W3C Verifiable Credential Data Integrity standard.

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.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/vocab-runtime/src/contexts/security-data-integrity-v1.json (6-9)

critical

Modifying the official, vendored https://w3id.org/security/data-integrity/v1 context file to remove digestMultibase is highly problematic and will break signature verification.

During JSON-LD signature verification (e.g., for Data Integrity Proofs), the document is expanded using the preloaded document loader. If the preloaded context differs from the official one by omitting digestMultibase, any incoming document using the v1 context that contains a digestMultibase property will have that property silently dropped during expansion. This will result in a different canonicalized RDF dataset than what the sender signed, causing signature verification to fail.

Instead of modifying the official context, we should keep it intact. If there is a URI conflict in the vocabulary, we should handle it in the vocabulary layer (e.g., by mapping both URIs or treating them as aliases) rather than diverging from the standard context.

packages/vocab/src/document.yaml (35-45)

medium

According to the W3C Verifiable Credential Data Integrity specification (Section 2.3 Resource Integrity), the digestMultibase property can be either a single string value or a list of string values (to support multiple digests/algorithms, such as SHA-256 and SHA-512).

By setting functional: true, we restrict this property to a single string value, which will prevent parsing or serializing multiple digests. Consider making it non-functional by removing functional: true and defining it with a plural name and singular accessor. Note that if you make this change, you should also update normalizeLinkToImage in packages/vocab/src/preprocessors.ts to copy digestMultibases instead of digestMultibase.

- pluralName: digestMultibases
  singularName: digestMultibase
  singularAccessor: true
  compactName: digestMultibase
  uri: "https://www.w3.org/ns/credentials/v2#digestMultibase"
  description: |
    The multibase-encoded integrity digest of an external resource represented
    by this document.

    See [FEP-ef61](https://w3id.org/fep/ef61) for details.
  range:
  - "http://www.w3.org/2001/XMLSchema#string"

packages/vocab/src/link.yaml (53-62)

medium

According to the W3C Verifiable Credential Data Integrity specification (Section 2.3 Resource Integrity), the digestMultibase property can be either a single string value or a list of string values (to support multiple digests/algorithms, such as SHA-256 and SHA-512).

By setting functional: true, we restrict this property to a single string value, which will prevent parsing or serializing multiple digests. Consider making it non-functional by removing functional: true and defining it with a plural name and singular accessor. Note that if you make this change, you should also update normalizeLinkToImage in packages/vocab/src/preprocessors.ts to copy digestMultibases instead of digestMultibase.

- pluralName: digestMultibases
  singularName: digestMultibase
  singularAccessor: true
  compactName: digestMultibase
  uri: "https://www.w3.org/ns/credentials/v2#digestMultibase"
  description: |
    The multibase-encoded integrity digest of the linked resource.

    See [FEP-ef61](https://w3id.org/fep/ef61) for details.
  range:
  - "http://www.w3.org/2001/XMLSchema#string"

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: bb6e70db1e

ℹ️ 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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

activitypub/compliance Specification compliance component/vocab Activity Vocabulary related component/vocab-tools Vocabulary code generation (@fedify/vocab-tools)

Development

Successfully merging this pull request may close these issues.

1 participant