Skip to content

Added routes_hash continuity test for the activation bridge#29191

Merged
vershwal merged 1 commit into
mainfrom
route-settings-hash-continuity
Jul 13, 2026
Merged

Added routes_hash continuity test for the activation bridge#29191
vershwal merged 1 commit into
mainfrom
route-settings-hash-continuity

Conversation

@vershwal

@vershwal vershwal commented Jul 9, 2026

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/HKG-1906/build-activation-bridge-in-dynamicroutingservice

Stacked on #29172.

Why

getCurrentHash() is md5(JSON.stringify(...)) over the expanded settings, and route-settings.js hardcodes the hash of the expanded default routes (3d180d52...). That constant is the only hash value external consumers could compare against, so it must survive the switch from validate.js to parser + activation bridge — this test pins both paths to it.

The existing equivalence tests use deepEqual, which is key-order-insensitive; JSON.stringify is not, so hash continuity was untested until now.

Finding worth knowing

Byte-identical hashes for customized configs are not achievable: validate.js mutates the parsed YAML in place, so its output key order (and hash) depends on the order keys were written in the user's routes.yaml, which a normalized domain model cannot preserve. Customized sites will get a harmless one-time routes_hash re-sync when the read path switches — nothing reads routes_hash except the sync comparison itself.

This also keeps the upcoming FileStore PR free of hash concerns: hashing stays in the service, computed over the bridge output.

🤖 Generated with Claude Code

@cursor

cursor Bot commented Jul 9, 2026

Copy link
Copy Markdown

Bugbot is not enabled for this team, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b7af53d6-54e2-4c6a-ab38-a51e7648d769

📥 Commits

Reviewing files that changed from the base of the PR and between f4cba0e and b3cb7bc.

📒 Files selected for processing (1)
  • ghost/core/test/unit/server/services/route-settings/activation-bridge.test.ts

Walkthrough

Added Node imports and YAML parsing support to the route-settings activation bridge tests. Added a hash continuity suite that loads the default routes fixture and verifies legacy validation and bridge expansion produce the same fixed MD5 hash.

Possibly related PRs

  • TryGhost/Ghost#29172: Introduces the activation bridge output verified by the new hash continuity test.

Suggested reviewers: allouis

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the added hash continuity test for the activation bridge.
Description check ✅ Passed The description matches the change and explains why the hash continuity test was added.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch route-settings-hash-continuity

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.

@nx-cloud

nx-cloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit f4cba0e

Command Status Duration Result
nx run ghost-monorepo:lint:boundaries ✅ Succeeded <1s View ↗
nx run-many -t lint -p ghost ✅ Succeeded 33s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 29s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-10 16:55:17 UTC

@vershwal vershwal force-pushed the yamlbridge branch 2 times, most recently from bba334e to dd23a14 Compare July 9, 2026 11:21
Base automatically changed from yamlbridge to main July 9, 2026 11:35
@vershwal vershwal force-pushed the route-settings-hash-continuity branch from 3c64a76 to f4cba0e Compare July 10, 2026 16:49
- getCurrentHash() is md5(JSON.stringify(...)) over the expanded settings, and
  route-settings.js hardcodes the hash of the expanded default routes
  ('3d180d52...'). That constant is the only hash value external consumers
  could compare against, so it must survive the switch from validate.js to
  parser + activation bridge — this test pins both paths to it
- the existing equivalence tests use deepEqual, which is key-order-insensitive;
  JSON.stringify is not, so hash continuity was untested until now
- byte-identical hashes for customized configs are not achievable: validate.js
  mutates the parsed YAML in place, so its output key order (and hash) depends
  on the order keys were written in the user's routes.yaml, which a normalized
  domain model cannot preserve. Customized sites will get a harmless one-time
  routes_hash re-sync when the read path switches — nothing reads routes_hash
  except the sync comparison itself
- keeps the upcoming FileStore PR free of hash concerns: hashing stays in the
  service, computed over the bridge output
@vershwal vershwal force-pushed the route-settings-hash-continuity branch from f4cba0e to b3cb7bc Compare July 10, 2026 16:51
@vershwal vershwal merged commit 33d851f into main Jul 13, 2026
30 checks passed
@vershwal vershwal deleted the route-settings-hash-continuity branch July 13, 2026 05:00
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.

1 participant