Preserve deleted entity types in tombstones#681
Conversation
Teach the vocabulary generator about fedify:vocabEntityType so schema-defined properties can accept generated entity constructors instead of arbitrary IRIs. This adds the generated $EntityType helpers, validates ambiguous schema ranges, and lets scalar decoders skip unknown entity type references. Also add regression tests and refresh the vocab-tools snapshots so the generator output stays pinned across runtimes. fedify-dev#645 Assisted-by: OpenCode:gpt-5.4
Add Tombstone.formerType as a generated vocab entity type reference so deleted actors can advertise what kind of entity was removed. This also patches the bundled and fixture ActivityStreams contexts so compact JSON-LD uses plain type tokens like Person, and adds regression tests for round-tripping and unknown remote values. fedify-dev#645 Assisted-by: OpenCode:gpt-5.4
Update the deleted actor examples to show how applications can include
formerType information in tombstones and inspect it through
getActor(..., { tombstone: "passthrough" }). Also record the new
vocab, vocab-runtime, and vocab-tools behavior, including the
generated entity type helpers, in the changelog.
fedify-dev#645
Assisted-by: OpenCode:gpt-5.4
Restore LogTape's global state after the unknown formerType regression test so later vocab tests do not inherit the temporary buffer logger configuration. This keeps the test isolated and avoids order-dependent side effects during full-suite runs. fedify-dev#645 Assisted-by: OpenCode:gpt-5.4
|
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 Tombstone.formerType support (typed to generated vocabulary entity constructors), emits generated entity-type helpers ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
issues-auto-labeler
bot
added
activitypub/compliance
dahlia
added
type/enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Tombstone.formerType to the ActivityStreams vocabulary, enabling deleted objects to retain their original type during round-trips. It adds a new fedify:vocabEntityType pseudo-scalar to the vocabulary generator, which produces a type-safe $EntityType union and associated helper functions. The ActivityStreams context has been updated to treat formerType as a vocabulary term. Feedback indicates that the vocabulary generator needs to ensure getLogger is imported in the generated code to avoid a potential ReferenceError during decoding.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vocab-tools/src/schema.test.ts`:
- Around line 233-246: Replace the manual try/catch assertion with a direct
throws assertion: call the test helper throws(() => validateTypeSchemas(types),
...) instead of capturing error in a temporary variable; assert that the thrown
error is a TypeError and that its message equals the expected string (the
current message about Tombstone.formerType). Locate the assertion around
validateTypeSchemas in schema.test.ts and update it to use throws with the same
expected error class and message to make the contract explicit.
🪄 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: c7a62774-7d5f-4e95-b63d-fae79a62899e
⛔ Files ignored due to path filters (4)
packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.node.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.snapis excluded by!**/*.snappackages/vocab/src/__snapshots__/vocab.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
CHANGES.mddocs/manual/actor.mddocs/manual/context.mdpackages/fedify/src/federation/handler.test.tspackages/fixture/src/fixtures/www.w3.org/ns/activitystreams.jsonpackages/vocab-runtime/src/contexts/activitystreams.jsonpackages/vocab-tools/src/class.test.tspackages/vocab-tools/src/class.tspackages/vocab-tools/src/codec.tspackages/vocab-tools/src/schema.test.tspackages/vocab-tools/src/schema.tspackages/vocab-tools/src/type.tspackages/vocab/src/tombstone.yamlpackages/vocab/src/type.test.tspackages/vocab/src/vocab.test.ts
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for ActivityStreams Tombstone.formerType in @fedify/vocab, modeling it as a reference to a known Fedify vocabulary entity constructor (not an object/URL reference), and updates JSON-LD context handling to ensure clean compact round-trips (e.g., "Person" rather than "as:Person").
Changes:
- Added
formerType/formerTypestoTombstoneschema and generated API, plus entity-type helper exports ($EntityType,isEntityType(),getEntityTypeById()). - Introduced
fedify:vocabEntityTypepseudo-scalar in the vocab generator with encode/decode support (including warning-and-ignore behavior for unknown incoming values). - Patched bundled + fixture ActivityStreams contexts (
formerTypeuses@vocab) and updated docs/tests/changelog accordingly.
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vocab/src/tombstone.yaml | Adds formerType property definition for Tombstone. |
| packages/vocab/src/vocab.test.ts | Adds serialization/parse regression tests for formerType and warning behavior. |
| packages/vocab/src/type.test.ts | Adds tests for new entity-type helper functions. |
| packages/vocab/src/snapshots/vocab.test.ts.snap | Updates snapshots for Tombstone inspection output. |
| packages/vocab-tools/src/type.ts | Adds fedify:vocabEntityType pseudo-scalar encode/decode/type-guard hooks. |
| packages/vocab-tools/src/codec.ts | Adjusts generated decoders to skip undefined decoded values (supports “ignore unknown” behavior). |
| packages/vocab-tools/src/schema.ts | Rejects mixed ranges involving fedify:vocabEntityType to avoid decoder ambiguity. |
| packages/vocab-tools/src/schema.test.ts | Adds regression test for the new schema validation rule. |
| packages/vocab-tools/src/class.ts | Emits $EntityType helpers and runtime lookup structures into generated vocab output. |
| packages/vocab-tools/src/class.test.ts | Tests codegen output for $EntityType helpers and integration into generated properties. |
| packages/vocab-tools/src/snapshots/class.test.ts.snap | Updates snapshots for new decoder structure and Tombstone formerType codegen. |
| packages/vocab-runtime/src/contexts/activitystreams.json | Changes formerType coercion to @vocab for correct compaction behavior. |
| packages/fixture/src/fixtures/www.w3.org/ns/activitystreams.json | Mirrors the @vocab formerType context change for fixtures. |
| packages/fedify/src/federation/handler.test.ts | Updates actor-handler test expectations to include formerType. |
| docs/manual/actor.md | Updates actor dispatcher example to set formerType. |
| docs/manual/context.md | Updates tombstone-handling example to display formerType. |
| CHANGES.md | Documents the new behavior and context patch. |
Codecov Report❌ Patch coverage is
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Build the generated entity type lookup table from schema URI strings instead of constructor typeId getters so vocab startup does not create URLs eagerly. Also make the unknown-formerType log assertion check the structured log record fields instead of stringifying the whole record. fedify-dev#681 Assisted-by: OpenCode:gpt-5.4
|
@codex review |
|
/gemini review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b497540039
ℹ️ 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.
Code Review
This pull request introduces the formerType property to the Tombstone class, allowing deleted objects to retain a reference to their original ActivityStreams type. Key changes include the addition of the fedify:vocabEntityType pseudo-scalar to the vocabulary generator, which enables the generation of $EntityType helpers, type guards, and lookup functions. The ActivityStreams context has been updated to treat formerType as a vocabulary term, and documentation and tests have been expanded to cover these new features. A review comment noted that the generated code for the new scalar uses a logger that may require an explicit import in the file header to avoid runtime errors.
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
There was a problem hiding this comment.
Code Review
This pull request introduces the formerType property to Tombstone objects, allowing deleted objects to preserve their original ActivityStreams type. To support this, a new fedify:vocabEntityType pseudo-scalar has been added to the vocabulary generator, along with generated helper utilities including the $EntityType union, isEntityType(), and getEntityTypeById(). The implementation includes updated decoding logic that handles unknown types with a warning and validation to ensure entity types are not mixed with other range types in the schema. I have no feedback to provide.
Handle malformed fedify:vocabEntityType values inside the scalar decoder so malformed expanded JSON-LD is ignored instead of throwing. Also add a regression test for malformed Tombstone.formerType input and refresh the generator snapshots. fedify-dev#681 Assisted-by: OpenCode:gpt-5.4
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements the formerType property for Tombstone objects, allowing deleted entities to retain their original ActivityStreams type. It introduces a fedify:vocabEntityType pseudo-scalar to the vocabulary toolchain, which generates $EntityType type definitions and utility functions (isEntityType, getEntityTypeById) for working with entity references. The changes encompass vocabulary schema updates, code generation logic, documentation, and tests. Review feedback highlights critical issues in the dataCheck and decoder implementations for the new scalar, where the use of the in operator on null values could lead to runtime TypeError exceptions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vocab-tools/src/type.ts`:
- Around line 448-457: The dataCheck and decoder string builders (functions
named dataCheck and decoder) must guard against null before using the "in"
operator: update the generated checks so dataCheck uses a null-safe check (e.g.,
`${v} != null && typeof ${v} === "object" && "@id" in ${v} ...`) and decoder
returns early when `${v} == null` before the typeof/in checks; apply the same
null-guard pattern to the other scalar type blocks that emit dataCheck/decoder
(the other two occurrences) so `"@id" in ${v}` never runs on null and avoids
TypeError.
🪄 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: 43bf7d1e-7a4f-4209-a099-5cf9069bec83
⛔ Files ignored due to path filters (3)
packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.node.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/vocab-tools/src/type.tspackages/vocab/src/vocab.test.ts
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ 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". |
Make the @id-based scalar data checks null-safe and guard fedify:vocabEntityType against malformed object shapes before looking up entity constructors. Also add a trailing semicolon in the single-range decoder template and refresh the vocab-tools snapshots. fedify-dev#681 Assisted-by: OpenCode:gpt-5.4
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ 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". |
There was a problem hiding this comment.
Code Review
This pull request introduces Tombstone.formerType and generated entity type helpers to the @fedify/vocab and @fedify/vocab-tools packages, enabling applications to preserve the original ActivityStreams type of deleted objects. The changes include the addition of the fedify:vocabEntityType pseudo-scalar and supporting functions like isEntityType() and getEntityTypeById(). A review comment suggests using optional chaining in getEntityTypeById to prevent a potential TypeError when handling null or undefined identifiers.
Make the generated getEntityTypeById() helper return undefined for unexpected nullish inputs instead of crashing on href access. Also add a regression test for nullish calls and refresh the generator snapshots. fedify-dev#681 Assisted-by: OpenCode:gpt-5.4
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
There was a problem hiding this comment.
Code Review
This pull request introduces the formerType property to the Tombstone vocabulary and adds a new fedify:vocabEntityType pseudo-scalar to the vocabulary generator. This allows vocabulary properties to reference generated Fedify entity constructors, supported by new generated helpers such as $EntityType, isEntityType(), and getEntityTypeById(). The implementation includes updates to documentation, tests, and validation logic to prevent mixing entity type references with other range types, as well as improved null safety in scalar data checks. I have no feedback to provide.
Why this is a little unusual
The surprising part of this PR is that
formerTypeis not implemented as a normal object-valued property. In Fedify, the ordinary object-property path is built aroundObject | URL, async accessors, and optional dereferencing. That would be the wrong model here, becauseformerTypeis really a reference to a vocabulary entity class, not a remote object reference.Another surprising point is what this PR does not do. The branch no longer rewrites the ActivityStreams context semantics for
formerType. I originally tried making compact JSON-LD emit bare values like"Person", but that relies on@vocabsemantics and creates an interoperability risk for consumers that load the standardhttps://www.w3.org/ns/activitystreamscontext, whereformerTypeis still@id-typed. After review, I backed that change out. The current branch keeps the standard semantics, which means compact output stays at values like"as:Person".Another place where it would be easy to make the wrong change is the encoder. I intentionally did not add a handwritten compact encoder for entity types. The generator always emits the expanded
@idform, and JSON-LD compaction is left to the active context. That keeps the context-sensitive logic in the JSON-LD layer instead of hard-coding it in a special serializer branch.What changed
Tombstone.formerTypein packages/vocab/src/tombstone.yaml as a non-functional property with a singular accessor.fedify:vocabEntityTypepseudo-scalar in packages/vocab-tools/src/type.ts and wired it through code generation in packages/vocab-tools/src/class.ts, packages/vocab-tools/src/codec.ts, and packages/vocab-tools/src/schema.ts.$EntityType,isEntityType(), andgetEntityTypeById()so generated properties can accept Fedify entity constructors rather than arbitrary IRIs.formerTypevalues non-fatal: they are ignored with a warning instead of making the wholeTombstonefail to parse.formerTyperegression test, tightening the generated entity type lookup helpers, and fixing malformed changelog references.Notes for reviewers
The write-side API is intentionally constructor-only, for example
new Tombstone({ formerType: Person }). I did not allowPerson.typeIdor arbitraryURLinput because that weakens the type contract and makes it too easy to pass the wrong IRI by accident.I also kept the first implementation intentionally narrow.
formerTypecurrently accepts only Fedify-known entity classes. It does not try to preserve unknown external vocabulary types on the write side yet. That felt like the right trade-off for the first pass, because the escape hatch for external types deserves its own design instead of being smuggled into this change.The generator now rejects mixed ranges that include
fedify:vocabEntityType. I was careful about that because otherwise the decoder would have to disambiguate vocabulary type IRIs from ordinary IRIs at runtime, which is exactly the sort of ambiguity that looks fine in TypeScript but gets messy on the wire.One small but deliberate cleanup is in packages/vocab/src/vocab.test.ts: the warning-path regression test resets LogTape state in a
finallyblock, and the assertion now checks structuredLogRecordfields instead of stringifying the whole record. That looks a little fussy, but without it the temporary global logger configuration can leak into later tests, and the older assertion shape was more brittle than it needed to be.The parts I was most careful about while implementing this were the generated decode path and the distinction between type references and object references. It is very easy to end up with a version that type-checks but either treats
formerTypeas fetchable or accepts arbitrary IRIs at the API boundary. The regression tests are meant to pin down exactly those edges.Testing
mise testThe latest
mise testrun on the current branch passed.Closes #645