You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Verdict: merge as-is, with one operational caveat that matters more than anything in the diff.
Operational caveat: merging does not change production. Nothing in CI deploys url-rewrite.js. deploy.yml syncs apps/marketing/dist/ to S3 and invalidates the distribution, but there is no update-function/publish-function step and no IaC covering the CloudFront Function. The function must be published manually in the AWS console after merge, or the fix silently sits unshipped while green tests assert a state production does not have. Pre-existing gap, not a defect of this PR, but it should be either done at merge time or added as a deploy step.
Everything claimed was independently re-verified. The destination returns a direct 200 with a matching canonical, no noindex meta or header, is present in the docs sitemap (102 locs), and is allowed by robots.txt. The current production before-state was reproduced exactly as described: 301 to the workspaces path, 308 to help.plannotator.ai, 200 noindex. The change collapses it to one hop for both slash variants, which normalize before the map lookup and hit the identical branch.
Function correctness: no new syntax beyond the file's existing conservative style, 4.4KB against the 10KB limit (size asserted by a test on every run), exact-key map lookup so ordering and shadowing are non-issues, 301 with the one-hour cache-control consistent with all 36 other entries. The docs-path entry already mapped to the same destination, so this aligns the blog article with the existing docs redirect rather than inventing a new target. Query strings are dropped on redirect, pre-existing behavior across all 37 entries, acceptable for a cross-host migration.
Tests: 5 pass; mutation-checked by reverting the target, which fails the new test with the exact expected/received pair. The tests eval the real function bytes, not a reimplementation. Both slash variants asserted.
RSS: the change was required, not cosmetic; migratedLinks is a separate hand-maintained copy of the mapping and would have kept feed subscribers in the old chain. Production build succeeds and the built feed emits the new target; grep of built output for every old-chain hostname and path returns zero hits.
Destination content match is strong: the article's three promises (share link via Export, teammate annotations, returned-review import) are all covered on the destination in more detail than the original, and the deprecated-OSS framing matches the status note the article itself already carries. No framing contradiction.
Non-blocking follow-ups:
Add a CloudFront publish-function step to deploy.yml so this file stops being decorative (or document the manual publish).
The new test replaced the old blog-redirect test rather than joining it, so /blog/plan-diff-see-what-changed is now mapped but unasserted; restore both.
The mapping is duplicated between url-rewrite.js and rss.xml.ts with nothing keeping them in sync; a cross-check test would remove that drift class.
Out of scope but noted: the destination's own Workspaces link points at the noindex help.plannotator.ai host, so the noindex problem is only removed from this path, not resolved. That lives in the docs repo.
Hygiene clean: single commit, three files, no em dashes, CI fully green, mergeable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
bun test apps/marketing/cloudfront/url-rewrite.test.tsbun run build:marketing