docs(hutch): note homepage A/B noindex + canonical=/ is intentional defence-in-depth#928
docs(hutch): note homepage A/B noindex + canonical=/ is intentional defence-in-depth#928FagnerMartinsBrack wants to merge 2 commits into
Conversation
612e329 to
dfa2d2f
Compare
|
@claude Resolve conflicts. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 18m 1s —— View job ✅ Conflicts resolved — branch rebased onto
|
0701ed7 to
623ae28
Compare
|
@claude Review this PR. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 5m 40s —— View job Review —
|
High Priority Issues (Must Fix)None found Medium Priority Issues1. "deliberately redundant" mischaracterises the mechanism — and could invite the exact removal it warns againstThe new comment says the
Because the comment exists to stop a maintainer removing a half, calling the pairing "redundant" invites the inference "redundant ⇒ safe to drop" — the opposite of intent. Rewording to "complementary" (isbot gate = redirect-discovery path; Low Priority Suggestions1. Tighten the three-line block and wrap to ~100 columnsThe added ~180-char line sits below a two-line block that already repeats the canonical-stays-on- 2. Comment policy — the comment itself is justifiedPer the code-comments skill, the why-comment is allowed: #908's squash commit is a bare one-liner and |
|
@claude Fix the review issues. Use your Read tool to read the local file Context
Review to AddressSee the review comment above this one for the issues to fix. |
|
Claude finished @FagnerMartinsBrack's task in 15m 47s —— View job ✅ Review issues fixed — reworded the homepage A/B noindex comment
What was fixedMEDIUM — "deliberately redundant" mischaracterised the mechanism. I traced the full A/B path and confirmed every claim in the review against source:
So the isbot gate (redirect-discovery path) and the arm - // The A/B arms are noindex so only the canonical `/` competes for SEO;
- // the canonical above stays on `/` for all three renders.
- // Defence-in-depth: the isbot gate already keeps crawlers off the arms, so pairing noindex with canonical=`/` here is deliberately redundant — don't remove either half.
+ // The A/B arms render `noindex` with canonical=`/`, so only the canonical `/` competes for
+ // SEO across all three renders. This is defence-in-depth, not redundancy: the isbot gate on
+ // `/` only keeps crawlers off the arms they'd reach via the client redirect. A direct hit to
+ // a `/landing-*` URL is not bot-gated and not disallowed in robots.txt, so on that path the
+ // arm's own noindex + canonical=`/` are the only guard. Don't remove either half.Verification
Commit noteCommitted as (Summary posted here in my existing comment rather than a separate |
|
@claude Review this PR. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 3m 32s —— View job Review —
|
| Claim | Source | Verdict |
|---|---|---|
Arms render noindex with canonical=/ |
home.component.ts:67 (canonical hardcoded /, not variant-conditional) + :74 (variant → "noindex, follow") |
✔ |
Only canonical / competes for SEO across all three renders |
control is index, both arms noindex, all three canonical / |
✔ |
isbot gate on / only keeps crawlers off arms reached via the client redirect |
server.ts:679 abSplit = !isbot(ua) gates only the split script (home.component.ts:260-264); gate lives on the / handler alone |
✔ |
Direct hit to /landing-* is not bot-gated |
renderLanding (landing.routes.ts:30-47) has only a req.userId auth bounce — no isbot check |
✔ |
| Arms not disallowed in robots.txt | server.ts:477-481 disallows only /queue, /export, /oauth, /forgot-password |
✔ |
| (supporting) arms not in sitemap | server.ts:522-532 lists /, /install, /login, /signup, /privacy, /terms, /llms*.txt, /auth.md — arms absent |
✔ |
Prior MEDIUM resolved. The new wording explicitly says "defence-in-depth, not redundancy" and correctly frames the two mechanisms as covering different paths — the isbot gate for the redirect-discovery path, the arm noindex for direct hits — which is a stronger, accurate argument for keeping both halves. The old prior-review LOW (fold the repetitive 3-line block, wrap to ~100 cols) is also absorbed: the block is now one wrapped ~90-col unit with the duplicate "canonical stays on /" line gone.
High Priority Issues (Must Fix)
None found
Medium Priority Issues
None found
Low Priority Suggestions
1. (Optional) The comment closes the "don't remove either half" loop but not the "don't replace with Disallow" one. The other move a future maintainer might reach for is adding /landing-* to robots.txt Disallow (server.ts:477-481) as the "proper" way to hide the arms. That would be wrong for exactly the reason the comment implies but doesn't state: a Disallowed URL can't be crawled, so the crawler never reads the noindex/canonical meta — the URL can still be indexed link-only, worse than the current allow-crawl-but-noindex design. If you want the comment to be fully future-proof you could add a half-sentence to that effect, but it's genuinely optional — the comment is already correct and complete as written, and the prior round worked to tighten it, so adding prose cuts the other way.
2. Comment policy — the comment itself remains justified. Per the code-comments skill, a why-comment is allowed only when the rationale isn't recoverable from history. It holds: #908's squash commit is a bare one-liner, and server.ts:677-678 carries only half the rationale (the redirect-discovery path), so the direct-hit half of the defence-in-depth pairing isn't discoverable elsewhere — and the human opened this PR specifically to record it. No action; noting it so the comment isn't later flagged as an unapproved addition. The docs(hutch): commit type is also correct for a comment-only change per the git-commit skill.
cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· claude/homepage-robots-noindex-comment-58et30
High Priority Issues (Must Fix)None found Medium Priority IssuesNone found Low Priority Suggestions1. (Optional) Comment closes the "don't remove either half" loop but not the "don't replace with
|
… defence-in-depth The server-side isbot gate already keeps crawlers on the control `/` and off the A/B arms, so the arms' `noindex` paired with `canonical=/` reads as redundant. Note that the redundancy is deliberate belt-and-suspenders so a future reader does not strip one half. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sfj512E2zhVqMoHpwTewdg
… not redundant The prior comment called the arm noindex + canonical=/ pairing "deliberately redundant" with the server-side isbot gate. They cover different paths: the isbot gate only keeps crawlers off the arms reached via the client redirect from `/`, while a direct hit to a `/landing-*` URL is not bot-gated and not disallowed in robots.txt, so there the arm noindex is the sole guard. Call them complementary so a future reader does not drop the noindex assuming the isbot gate already covers it. Co-authored-by: Fayner Brack <FagnerMartinsBrack@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a68af81 to
a839934
Compare
|
@claude Review this PR. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 5m 6s —— View job Review —
|
| Claim | Source | Verdict |
|---|---|---|
Arms render noindex with canonical=/ |
canonical hardcoded https://readplace.com at home.component.ts:74 (not variant-conditional) + :81 variant ? "noindex, follow" : "index, follow" |
✔ |
Only canonical / competes for SEO across all three renders |
control index, both arms noindex, all three share the hardcoded canonical / |
✔ |
isbot gate on / only keeps crawlers off arms reached via the client redirect |
server.ts:714 abSplit = !isbot(ua) toggles only the split script (home.component.ts:267-271); gate lives on the / handler alone |
✔ |
Direct hit to /landing-* is not bot-gated |
renderLanding (landing.routes.ts:30-47) has only a req.userId auth bounce — no isbot check |
✔ |
| Arms not disallowed in robots.txt | server.ts:501-505 disallows only /queue, /export, /oauth, /forgot-password; Allow: / |
✔ |
| (supporting) arms not in sitemap | server.ts:546-558 lists /, /install, /import, /login, /signup, /privacy, /terms, /support, /llms*.txt, /auth.md — arms absent |
✔ |
I also checked the naive-bot middleware as a possible hidden gate: it 403s only narrow scripted-client UAs (curl, python-requests, Go-http-client…) and deliberately lets unknown/search crawlers through (naive-bot.ts:14-15). So it doesn't serve noindex and doesn't contradict "not bot-gated" — for a search crawler's direct hit, the arm's own noindex really is the operative guard.
The prior MEDIUM ("deliberately redundant" mischaracterised the mechanism) stays resolved: the text now says "defence-in-depth, not redundancy" and correctly frames the isbot gate (redirect-discovery path) and the arm noindex (direct-hit path) as covering different paths — the stronger, accurate argument for keeping both halves.
High Priority Issues (Must Fix)
None found
Medium Priority Issues
None found
Low Priority Suggestions
1. (Optional, already raised twice) The comment closes "don't remove either half" but leaves "don't replace with Disallow" implied. A maintainer might still "fix" arm exposure by adding /landing-* to robots.txt Disallow (server.ts:501-505) — which is wrong: a Disallowed URL is never crawled, so the crawler never reads the noindex/canonical meta and the URL can still be indexed link-only, worse than the current allow-crawl-but-noindex design. The comment was deliberately tightened over two rounds and the human has left this optional line untaken both times, so I'm not re-pressing it — flagging only that it's a conscious open choice, not an oversight.
2. (Optional, wording) "noindex + canonical=/ are the only guard" slightly over-credits the canonical. On the direct-hit path the operative guard is noindex; canonical / is a consolidation hint (advisory, and technically a mixed signal on a noindex page), not a second enforcement. Immaterial — behaviour is unchanged and intent is clear — no action.
3. Comment policy — the comment remains justified. Per the code-comments skill, a why-comment is allowed only when the rationale isn't recoverable from history: #908's squash commit is a bare one-liner and server.ts:712-713 documents only the redirect-discovery half, so the direct-hit half of the pairing isn't discoverable elsewhere, and the human opened this PR specifically to record it. No action.
cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· claude/homepage-robots-noindex-comment-58et30
Review —
|
| Claim | Source | Verdict |
|---|---|---|
Arms render noindex with canonical=/ |
canonical hardcoded https://readplace.com at home.component.ts:74 (not variant-conditional) + :81 variant ? "noindex, follow" : "index, follow" |
✔ |
Only canonical / competes for SEO across all three renders |
control index, both arms noindex, all three share the hardcoded canonical / |
✔ |
isbot gate on / only keeps crawlers off arms reached via the client redirect |
server.ts:714 abSplit = !isbot(ua) toggles only the split script (home.component.ts:267-271); gate lives on the / handler alone |
✔ |
Direct hit to /landing-* is not bot-gated |
renderLanding (landing.routes.ts:30-47) has only a req.userId auth bounce — no isbot check |
✔ |
| Arms not disallowed in robots.txt | server.ts:501-505 disallows only /queue, /export, /oauth, /forgot-password; Allow: / |
✔ |
| (supporting) arms not in sitemap | server.ts:546-558 lists /, /install, /import, /login, /signup, /privacy, /terms, /support, /llms*.txt, /auth.md — arms absent |
✔ |
I also checked the naive-bot middleware as a possible hidden gate: it 403s only narrow scripted-client UAs (curl, python-requests, Go-http-client…) and deliberately lets unknown/search crawlers through (naive-bot.ts:14-15). So it doesn't serve noindex and doesn't contradict "not bot-gated" — for a search crawler's direct hit, the arm's own noindex really is the operative guard.
The prior MEDIUM ("deliberately redundant" mischaracterised the mechanism) stays resolved: the text now says "defence-in-depth, not redundancy" and correctly frames the isbot gate (redirect-discovery path) and the arm noindex (direct-hit path) as covering different paths — the stronger, accurate argument for keeping both halves.
High Priority Issues (Must Fix)
None found
Medium Priority Issues
None found
Low Priority Suggestions
1. (Optional, already raised twice) The comment closes "don't remove either half" but leaves "don't replace with Disallow" implied. A maintainer might still "fix" arm exposure by adding /landing-* to robots.txt Disallow (server.ts:501-505) — which is wrong: a Disallowed URL is never crawled, so the crawler never reads the noindex/canonical meta and the URL can still be indexed link-only, worse than the current allow-crawl-but-noindex design. The comment was deliberately tightened over two rounds and the human has left this optional line untaken both times, so I'm not re-pressing it — flagging only that it's a conscious open choice, not an oversight.
2. (Optional, wording) "noindex + canonical=/ are the only guard" slightly over-credits the canonical. On the direct-hit path the operative guard is noindex; canonical / is a consolidation hint (advisory, and technically a mixed signal on a noindex page), not a second enforcement. Immaterial — behaviour is unchanged and intent is clear — no action.
3. Comment policy — the comment remains justified. Per the code-comments skill, a why-comment is allowed only when the rationale isn't recoverable from history: #908's squash commit is a bare one-liner and server.ts:712-713 documents only the redirect-discovery half, so the direct-hit half of the pairing isn't discoverable elsewhere, and the human opened this PR specifically to record it. No action.
What & why
Stacked on #908. Adds a single explanatory comment beside the homepage A/B
robotsline inhome.component.ts— no behaviour change.The A/B arms render
robots: noindex, followwhile keepingcanonical=/. Because the server-sideisbotgate already assigns bots the control/(so a crawler never follows the client redirect into an arm), thatnoindex+canonical=/pairing can read as redundant to a future maintainer. The comment records that the redundancy is deliberate defence-in-depth, so nobody strips one half on the assumption the other already covers it.Base
Targets
claude/homepage-ab-redirect-g6vdrs(the #908 branch) because therobotsline only exists there; the diff is a single comment line. GitHub will retarget this PR tomainautomatically once #908 merges.🤖 Generated with Claude Code
https://claude.ai/code/session_01Sfj512E2zhVqMoHpwTewdg
Generated by Claude Code