Skip to content

Added ADR for frontend versioning strategy #1161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

allouis
Copy link
Collaborator

@allouis allouis commented Aug 6, 2025

TL;DR Instead of the frontend version being determined by Ghost version, causing mismatches, the ActivityPub backend will have an endpoint that tells the frontend loader which CDN bundle URL to use. This guarantees the frontend and backend versions are always compatible - each backend version declares its matching frontend version. Simple fix, minimal code changes, solves all the deployment scenarios.

TL;DR Instead of the frontend version being determined by Ghost version,
causing mismatches, the ActivityPub backend will have an endpoint that
tells the frontend loader which CDN bundle URL to use. This guarantees
the frontend and backend versions are always compatible - each backend
version declares its matching frontend version. Simple fix, minimal code
changes, solves all the deployment scenarios.
Copy link

coderabbitai bot commented Aug 6, 2025

Walkthrough

A new design proposal document has been added, outlining a frontend versioning strategy for the ActivityPub service. The document describes deployment scenarios, identifies compatibility challenges between frontend and backend versions, and proposes a solution involving a backend endpoint that returns the appropriate frontend bundle URL. The frontend loader will use this endpoint to dynamically import the compatible bundle. The proposal details release coordination, version pinning, and the implications for different hosting environments, without introducing any changes to code or exported entities.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch versioning-adr

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
adr/frontend-versioning.md (3)

1-6: Add standard ADR front-matter (ID, Date, Status) for traceability
The project’s existing ADRs carry a short YAML/markdown header (id, date, status, deciders, etc.) so they can be indexed automatically. Consider adding it here to keep the file machine-discoverable and sortable.

+# ---
+# id: 00XX-frontend-versioning
+# title: ActivityPub Service Frontend Versioning via Backend Configuration
+# date: 2025-08-??            # fill in PR merge date
+# status: Proposed
+# deciders: @allouis
+# ---

35-45: Specify method, auth & cache semantics of /.ghost/activitypub/client-config
The ADR explains WHAT the endpoint returns but omits operational details that tend to bite later:

  1. HTTP method – presumably GET only.
  2. Caching – Our previous learning shows a preference for broad surrogate caching (exclude v1 APIs, cache everything else). Clarify whether this endpoint should inherit that rule or set an explicit Surrogate-Control: max-age=… header; otherwise stale bundle URLs could linger in CDN/proxy layers.
  3. Authentication – Should self-hosted proxies hitting our infra call an unauthenticated endpoint, or must it be session-bound? Stating this avoids later ambivalence.

A concise paragraph covering the above would complete the “Implementation” section.


68-70: Mitigate the extra RTT by recommending aggressive caching of config response
The “Negative” list mentions the extra HTTP request but not that, with even a short-lived public cache (e.g., 5 min in CDN + private browser cache), the cost is effectively amortised. Given our earlier caching preference, add a note that the endpoint SHOULD send Surrogate-Control and Cache-Control headers so loaders usually hit the local cache, not origin.

This aligns the ADR with established caching conventions noted in previous PR #1114.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5da04b5 and da2c41f.

📒 Files selected for processing (1)
  • adr/frontend-versioning.md (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: allouis
PR: TryGhost/ActivityPub#1010
File: src/app.ts:744-744
Timestamp: 2025-07-09T06:47:50.064Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately.
Learnt from: allouis
PR: TryGhost/ActivityPub#1114
File: src/app.ts:597-608
Timestamp: 2025-07-28T03:28:29.993Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers simple, broad caching logic over explicit endpoint whitelisting for the Surrogate-Control header. The approach of "exclude v1 API endpoints, cache everything else ActivityPub" combined with GET/HEAD method restrictions is preferred for its simplicity and maintainability, even when it includes dynamic federation endpoints like followers/following/outbox that are still reasonable to cache for read operations.
Learnt from: allouis
PR: TryGhost/ActivityPub#1021
File: src/post/post.entity.ts:327-329
Timestamp: 2025-07-10T08:39:14.900Z
Learning: In the TryGhost/ActivityPub repository, when parsing data from Ghost CMS (such as post excerpts) using branded types with validation (like PostSummary.parse()), allouis prefers to let the parsing throw errors if Ghost sends incompatible data rather than adding defensive handling like truncation. This approach helps detect incompatibilities between the ActivityPub system and Ghost CMS early, ensuring issues are addressed at the source rather than silently ignored.
Learnt from: allouis
PR: TryGhost/ActivityPub#593
File: src/http/api/reply.ts:48-59
Timestamp: 2025-04-30T11:39:20.993Z
Learning: In the TryGhost/ActivityPub repository, when working with URL IDs in API handlers, the codebase uses a pattern where the raw ID parameter is passed to `lookupObject` even when a decoded version has been created with `decodeURIComponent`. This pattern is consistent with existing functionality in `handlers.ts`.
Learnt from: allouis
PR: TryGhost/ActivityPub#858
File: features/support/content.js:15-22
Timestamp: 2025-06-16T15:43:23.967Z
Learning: allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. Functional improvements like error handling should be suggested for separate PRs.
📚 Learning: in the tryghost/activitypub repository, allouis prefers simple, broad caching logic over explicit en...
Learnt from: allouis
PR: TryGhost/ActivityPub#1114
File: src/app.ts:597-608
Timestamp: 2025-07-28T03:28:29.993Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers simple, broad caching logic over explicit endpoint whitelisting for the Surrogate-Control header. The approach of "exclude v1 API endpoints, cache everything else ActivityPub" combined with GET/HEAD method restrictions is preferred for its simplicity and maintainability, even when it includes dynamic federation endpoints like followers/following/outbox that are still reasonable to cache for read operations.

Applied to files:

  • adr/frontend-versioning.md
📚 Learning: in the tryghost/activitypub repository, allouis prefers to keep refactoring prs focused on code rest...
Learnt from: allouis
PR: TryGhost/ActivityPub#1010
File: src/app.ts:744-744
Timestamp: 2025-07-09T06:47:50.064Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately.

Applied to files:

  • adr/frontend-versioning.md
📚 Learning: in the tryghost/activitypub repository, the exponential backoff for delivery failures in src/account...
Learnt from: mike182uk
PR: TryGhost/ActivityPub#1043
File: src/account/account.service.ts:759-794
Timestamp: 2025-07-15T13:07:04.697Z
Learning: In the TryGhost/ActivityPub repository, the exponential backoff for delivery failures in src/account/account.service.ts is intentionally designed without a maximum cap. This unbounded growth is by design - longer delivery failure periods give more confidence that an account is truly broken rather than temporarily unavailable, while still giving receivers a fair chance to recover from temporary issues.

Applied to files:

  • adr/frontend-versioning.md
📚 Learning: in the tryghost/activitypub repository, mike182uk designed the delivery failure backoff mechanism in...
Learnt from: mike182uk
PR: TryGhost/ActivityPub#1043
File: src/account/account.service.ts:54-55
Timestamp: 2025-07-15T13:05:42.512Z
Learning: In the TryGhost/ActivityPub repository, mike182uk designed the delivery failure backoff mechanism in src/account/account.service.ts to intentionally use unbounded exponential backoff without a maximum cap. This design serves as a confidence mechanism where the longer delivery attempts fail, the more confident the system becomes that the account is truly broken rather than experiencing temporary issues. The backoff gives receivers a "fair chance" of receiving future activities if they're having temporary issues that later get resolved.

Applied to files:

  • adr/frontend-versioning.md

@kevinansfield
Copy link
Member

kevinansfield commented Aug 7, 2025

At first read-through this sounds like a really nice approach! A couple of questions...

  1. How are upgrades to the backend with breaking changes handled for already-running clients?
    • E.g. do all backend responses include the compatible client version so clients can react accordingly with an auto-reload or notification when they get a version-incompatible response/error?
  2. Are there any dependencies between the client and the client's parent app? If so, how do those get handled?
    • E.g. for Koenig there's a fair amount of data and functions passed through from the host app (ghost) to the client app (editor) so there's a coupled dependency across both apps for certain types of changes. Does ActivityPub have anything similar?

@allouis
Copy link
Collaborator Author

allouis commented Aug 7, 2025

Great Q's!

  1. Yeah, I think we need to include a header in the response that the client can do something with, either a forced reload, or a notification. I think we can just respond with the required semver for the client 🤔
  2. The client app depends on the admin-x-framework code, which handles things like allowing us to read/update route params, I don't think there is much we can do here, other than not making breaking changes in the framework. We also have a dependency in Admin on the notifications count endpoint, which will need to be treated as a special case and not have breaking changes made to it.

@kevinansfield
Copy link
Member

kevinansfield commented Aug 7, 2025

The client app depends on the admin-x-framework code

Does that get bundled with the AP client build or does it depend on an already-loaded version provided by the host app?

@allouis
Copy link
Collaborator Author

allouis commented Aug 7, 2025

Good point, that is bundled with the AP client!

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.

2 participants