Skip to content

Handle URL string icons#774

Open
puneetdixit200 wants to merge 1 commit into
fedify-dev:mainfrom
puneetdixit200:fix/420-icon-url-fallback
Open

Handle URL string icons#774
puneetdixit200 wants to merge 1 commit into
fedify-dev:mainfrom
puneetdixit200:fix/420-icon-url-fallback

Conversation

@puneetdixit200
Copy link
Copy Markdown

Summary

  • Allow Activity Vocabulary icon URL strings to decode as Link values instead of remote Image document references.
  • Keep WebFinger avatar generation working for both Image.url and Link.href icons.
  • Refresh generated vocab-tools snapshots for Deno, Node, and Bun.

Fixes: #420

Verification

  • docker run --rm -v "$PWD":/workspace -w /workspace denoland/deno:2.7.13 deno task -f @fedify/vocab compile
  • docker run --rm -v "$PWD":/workspace -w /workspace/packages/vocab denoland/deno:2.7.13 deno test --allow-read --allow-write --allow-env --unstable-kv --trace-leaks --filter "URL string icon" src/vocab.test.ts
  • docker run --rm -v "$PWD":/workspace -w /workspace/packages/vocab denoland/deno:2.7.13 deno test --allow-read --allow-write --allow-env --unstable-kv --trace-leaks --filter "relative URLs" src/vocab.test.ts
  • docker run --rm -v "$PWD":/workspace -w /workspace/packages/fedify denoland/deno:2.7.13 deno test --check --doc --allow-read --allow-write --allow-env --unstable-kv --trace-leaks src/federation/webfinger.test.ts
  • docker run --rm -v "$PWD":/workspace -w /workspace/packages/vocab-tools denoland/deno:2.7.13 deno test -A
  • node --experimental-transform-types --test src/class.test.ts from packages/vocab-tools
  • bun test --timeout 15000 src/class.test.ts from packages/vocab-tools
  • docker run --rm -v "$PWD":/workspace -w /workspace denoland/deno:2.7.13 deno fmt --check CHANGES.md packages/fedify/src/federation/webfinger.ts packages/vocab-tools/src/codec.ts packages/vocab-tools/src/property.ts packages/vocab/src/vocab.test.ts
  • docker run --rm -v "$PWD":/workspace -w /workspace denoland/deno:2.7.13 deno lint packages/fedify/src/federation/webfinger.ts packages/vocab-tools/src/codec.ts packages/vocab-tools/src/property.ts packages/vocab/src/vocab.test.ts
  • docker run --rm -v "$PWD":/workspace -w /workspace denoland/deno:2.7.13 deno check packages/fedify/src/federation/webfinger.ts packages/vocab-tools/src/codec.ts packages/vocab-tools/src/property.ts packages/vocab/src/vocab.test.ts
  • git diff --check

AI usage

This change and PR description were assisted by OpenAI GPT-5. The commit includes the required Assisted-by: OpenAI GPT-5:gpt-5 trailer.

Parse URL string icon values as ActivityStreams Link values so callers do not dereference image files as JSON-LD documents.

Fixes: fedify-dev#420

Assisted-by: OpenAI GPT-5:gpt-5
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6cf8bb17-45ef-4648-b2eb-91e0097195b5

📥 Commits

Reviewing files that changed from the base of the PR and between 8db5848 and 3659106.

⛔ 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 (6)
  • CHANGES.md
  • packages/fedify/src/federation/webfinger.ts
  • packages/vocab-tools/src/codec.ts
  • packages/vocab-tools/src/property.ts
  • packages/vocab/src/object.yaml
  • packages/vocab/src/vocab.test.ts

📝 Walkthrough

Walkthrough

This PR extends Fedify's icon property to accept both Link and Image types per ActivityStreams 2.0 specification, fixing a bug where URL strings for icon properties caused JSON parsing errors. The JSON-LD decoder is updated to conditionally wrap bare @id values into Link objects when appropriate, WebFinger is integrated to handle the new type, and test coverage validates the behavior.

Changes

Icon property Link and Image support

Layer / File(s) Summary
Vocabulary schema update
packages/vocab/src/object.yaml
The icon property's range is extended to include both https://www.w3.org/ns/activitystreams#Link and https://www.w3.org/ns/activitystreams#Image.
JSON-LD decoder code generation
packages/vocab-tools/src/codec.ts, packages/vocab-tools/src/property.ts
IRI constants for ActivityStreams Link, Object, and xsd:anyURI are added. A per-property decodeBareIdAsLink flag is computed when the range includes Link but excludes Object and anyURI. When true, bare @id-only JSON objects are wrapped into new Link({ href }) instead of plain URL instances. Template punctuation is adjusted for multi-range decoder generation.
WebFinger avatar extraction
packages/fedify/src/federation/webfinger.ts
Icon handling for actor WebFinger links now supports both LinkObject instances (extracting .href) and URL-like objects (extracting .url), then stringifies consistently as href.href.toString() with optional mediaType.
Test regression and assertions
packages/vocab/src/vocab.test.ts
A new test verifies that Person.fromJsonLd() treats URL-string icons as Link instances without dereferencing. Existing icon tests replace optional-chain URL checks with strict assertInstanceOf(..., Image) and direct icon.url comparisons.
Release notes
CHANGES.md
Version 2.3.0 changelog documents the @fedify/vocab fix: icon URL strings are now exposed as Link instances rather than dereferenced Image documents.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

component/federation, activitypub/interop

Suggested reviewers

  • sij411
  • 2chanhaeng
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Handle URL string icons' is concise and directly describes the main change: supporting URL string icons instead of only Image objects.
Description check ✅ Passed The description clearly explains the changes: allowing icon URL strings to decode as Link values, keeping WebFinger working for both Image and Link icons, and references issue #420.
Linked Issues check ✅ Passed The PR addresses issue #420 by updating the icon property range to include Link, modifying codec generation to decode bare @id values as Link objects, and updating WebFinger to handle both Image.url and Link.href icons.
Out of Scope Changes check ✅ Passed All changes are directly related to handling URL string icons: vocabulary schema updates, codec generation logic, WebFinger handling, and corresponding tests. No unrelated changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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 and usage tips.

Copy link
Copy Markdown
Contributor

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

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 fixes an issue where URL strings in Activity Vocabulary icon values were incorrectly dereferenced as remote Image documents. The icon property range in object.yaml now includes Link, and the vocab-tools decoder has been updated to handle bare IDs as Link instances when appropriate. Additionally, the WebFinger implementation was updated to support these changes, and new tests were added to verify that string-based icons are correctly parsed as links without being dereferenced. I have no feedback to provide as there were no review comments.

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.

Fedify fails to handle URL strings for icon property

1 participant