Skip to content

Fix legacy sharing article redirect chain - #1180

Merged
backnotprop merged 1 commit into
mainfrom
codex-fix-sharing-plan-redirect
Aug 2, 2026
Merged

Fix legacy sharing article redirect chain#1180
backnotprop merged 1 commit into
mainfrom
codex-fix-sharing-plan-redirect

Conversation

@backnotprop

Copy link
Copy Markdown
Owner

Summary

  • redirect the legacy sharing article directly to the current public OSS sharing guide
  • keep the migrated RSS item aligned
  • cover slash and slashless redirect variants

Validation

  • bun test apps/marketing/cloudfront/url-rewrite.test.ts
  • bun run build:marketing
  • verified the destination returns 200 without another redirect, self-canonicalizes, appears in the Docs sitemap, and is crawlable

@backnotprop

Copy link
Copy Markdown
Owner Author

Review (at 47e4f1c3)

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:

  1. Add a CloudFront publish-function step to deploy.yml so this file stops being decorative (or document the manual publish).
  2. 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.
  3. 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.
  4. 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.

@backnotprop
backnotprop merged commit 0935cf0 into main Aug 2, 2026
14 checks passed
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