Update custom-domain doc to reflect Hetzner edge decision #47

Merged

ldraney merged 1 commit from update-custom-domain-doc into main

2026-06-13 13:16:19 +00:00

ldraney commented

2026-06-13 13:05:39 +00:00

Owner

Summary

Replace stale Cloudflare Tunnel spike recommendation with the actual Hetzner edge node implementation. Also includes minor view/config cleanup that was staged.

Changes

docs/custom-domain.md — rewrote to reflect Hetzner edge decision: architecture diagram, DNS table with live IP, remaining work items. Removed spike analysis (decision is made).
app/views/links/_link.html.erb — link card title navigates to actual URL instead of palinks show page
app/views/links/show.html.erb — same: title links directly to URL
config/environments/development.rb — removed stale palinks-dev host entry

Test Plan

Doc accurately reflects live infrastructure (edge node at 178.156.129.142)
Verify link cards navigate to external URLs in browser

Review Checklist

No secrets or credentials in changes
Doc matches pal-e-platform/docs/hetzner-edge.md architecture

Closes #25 (custom domain spike — superseded by implementation)

## Summary Replace stale Cloudflare Tunnel spike recommendation with the actual Hetzner edge node implementation. Also includes minor view/config cleanup that was staged. ## Changes - `docs/custom-domain.md` — rewrote to reflect Hetzner edge decision: architecture diagram, DNS table with live IP, remaining work items. Removed spike analysis (decision is made). - `app/views/links/_link.html.erb` — link card title navigates to actual URL instead of palinks show page - `app/views/links/show.html.erb` — same: title links directly to URL - `config/environments/development.rb` — removed stale `palinks-dev` host entry ## Test Plan - [x] Doc accurately reflects live infrastructure (edge node at 178.156.129.142) - [ ] Verify link cards navigate to external URLs in browser ## Review Checklist - [x] No secrets or credentials in changes - [x] Doc matches pal-e-platform/docs/hetzner-edge.md architecture ## Related Notes - Closes #25 (custom domain spike — superseded by implementation)

ldraney added 1 commit

2026-06-13 13:05:39 +00:00

Update custom-domain doc and fix stale view/config changes b18234915f

- Replace Cloudflare Tunnel spike recommendation with actual Hetzner
  edge node implementation (178.156.129.142)
- Link cards navigate to actual URL instead of show page
- Remove stale palinks-dev host entry from development config

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-06-13 13:07:54 +00:00

Author

Owner

PR #47 Review

DOMAIN REVIEW

Tech stack identified: Ruby on Rails (ERB views, Rails config), Markdown documentation, Mermaid diagrams.

docs/custom-domain.md -- The rewrite is clean. The 163-line spike analysis is correctly replaced with a concise 45-line implementation doc. The architecture diagram accurately reflects the Hetzner edge node pattern (Caddy + Tailscale mesh). The DNS table includes the live IP 178.156.129.142. The "Remaining Work" section lists actionable next steps. Cross-reference to pal-e-platform/docs/hetzner-edge.md is a good practice.

app/views/links/_link.html.erb -- Changed link_to link.title, link_path(link) to link_to link.title, link.url. The URL field is validated at the model level (URI::DEFAULT_PARSER.make_regexp(%w[http https])) so javascript: protocol XSS is not a risk. Rails link_to also escapes HTML attributes. No security concern here.

app/views/links/show.html.erb -- The "Visit" button (link_to "Visit", @link.url, target: "_blank", rel: "noopener noreferrer") was removed and replaced with the title linking directly to the URL. Two observations below in NITS.

config/environments/development.rb -- Removed palinks-dev.tail5b443a.ts.net host entry. This was added by PR #46 and is now determined to be stale. Clean removal.

BLOCKERS

None.

The changes are documentation, view-layer navigation, and config cleanup. There is no new functionality that would require new tests -- the link card navigation change is a one-line link_to target swap, and the model already validates URLs. The test gap (no view/system tests for link rendering) predates this PR.

NITS

Missing target="_blank" on external links -- Both _link.html.erb (line 13) and show.html.erb (line 4) now link to external URLs without target: "_blank". Clicking a link title navigates the user away from palinks entirely. The previous show page had a "Visit" button with target: "_blank", rel: "noopener noreferrer" for exactly this reason. Consider adding target: "_blank", rel: "noopener noreferrer" to preserve the "stay on palinks" behavior:
```
<%= link_to link.title, link.url, target: "_blank", rel: "noopener noreferrer" %>
```
This is non-blocking since the previous behavior (linking to the show page) also required a click-through, but it is a UX regression from the "Visit" button pattern.
Show page lost its dedicated "Visit" action -- The show page previously had both the title (plain text) and a prominent "Visit" button. Now the title IS the link and the "Visit" button is gone. The URL is still displayed as text below (link-detail-url), but the explicit call-to-action button is lost. This may be intentional simplification. Non-blocking.
IP address in docs -- 178.156.129.142 is hardcoded in the DNS table. This is fine for operational documentation, but item 5 in "Remaining Work" (delegate to Hetzner DNS for IaC) would address this long-term. No action needed now.

SOP COMPLIANCE

PR body has Summary, Changes, Test Plan, Related sections
No secrets or credentials committed (IP address is public infrastructure, not a secret)
No unnecessary file changes -- all four files relate to the custom domain decision or the link navigation fix
Commit scope is reasonable (doc update + staged view/config cleanup)

One note: the Test Plan has an unchecked item ("Verify link cards navigate to external URLs in browser"). This is worth confirming before merge but is not a blocker for code review.

PROCESS OBSERVATIONS

This PR closes a spike issue (#25) by replacing its output with the actual implementation doc. Good spike lifecycle practice.
The link navigation change (fixing cards to link to actual URLs) was previously addressed in PR #44 but appears to have been staged separately here. The PR body acknowledges this ("Also includes minor view/config cleanup that was staged"). Slightly mixed scope but acceptable given the small size.
The palinks-dev host removal undoes work from PR #46 (merged one commit ago). This suggests the host entry was short-lived. Consider whether the host was needed at all, or if there is a naming convention issue to resolve upstream.

VERDICT: APPROVED

## PR #47 Review ### DOMAIN REVIEW **Tech stack identified:** Ruby on Rails (ERB views, Rails config), Markdown documentation, Mermaid diagrams. **docs/custom-domain.md** -- The rewrite is clean. The 163-line spike analysis is correctly replaced with a concise 45-line implementation doc. The architecture diagram accurately reflects the Hetzner edge node pattern (Caddy + Tailscale mesh). The DNS table includes the live IP `178.156.129.142`. The "Remaining Work" section lists actionable next steps. Cross-reference to `pal-e-platform/docs/hetzner-edge.md` is a good practice. **app/views/links/_link.html.erb** -- Changed `link_to link.title, link_path(link)` to `link_to link.title, link.url`. The URL field is validated at the model level (`URI::DEFAULT_PARSER.make_regexp(%w[http https])`) so `javascript:` protocol XSS is not a risk. Rails `link_to` also escapes HTML attributes. No security concern here. **app/views/links/show.html.erb** -- The "Visit" button (`link_to "Visit", @link.url, target: "_blank", rel: "noopener noreferrer"`) was removed and replaced with the title linking directly to the URL. Two observations below in NITS. **config/environments/development.rb** -- Removed `palinks-dev.tail5b443a.ts.net` host entry. This was added by PR #46 and is now determined to be stale. Clean removal. ### BLOCKERS None. The changes are documentation, view-layer navigation, and config cleanup. There is no new functionality that would require new tests -- the link card navigation change is a one-line `link_to` target swap, and the model already validates URLs. The test gap (no view/system tests for link rendering) predates this PR. ### NITS 1. **Missing `target="_blank"` on external links** -- Both `_link.html.erb` (line 13) and `show.html.erb` (line 4) now link to external URLs without `target: "_blank"`. Clicking a link title navigates the user away from palinks entirely. The previous show page had a "Visit" button with `target: "_blank", rel: "noopener noreferrer"` for exactly this reason. Consider adding `target: "_blank", rel: "noopener noreferrer"` to preserve the "stay on palinks" behavior: ```erb <%= link_to link.title, link.url, target: "_blank", rel: "noopener noreferrer" %> ``` This is non-blocking since the previous behavior (linking to the show page) also required a click-through, but it is a UX regression from the "Visit" button pattern. 2. **Show page lost its dedicated "Visit" action** -- The show page previously had both the title (plain text) and a prominent "Visit" button. Now the title IS the link and the "Visit" button is gone. The URL is still displayed as text below (`link-detail-url`), but the explicit call-to-action button is lost. This may be intentional simplification. Non-blocking. 3. **IP address in docs** -- `178.156.129.142` is hardcoded in the DNS table. This is fine for operational documentation, but item 5 in "Remaining Work" (delegate to Hetzner DNS for IaC) would address this long-term. No action needed now. ### SOP COMPLIANCE - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] No secrets or credentials committed (IP address is public infrastructure, not a secret) - [x] No unnecessary file changes -- all four files relate to the custom domain decision or the link navigation fix - [x] Commit scope is reasonable (doc update + staged view/config cleanup) One note: the Test Plan has an unchecked item ("Verify link cards navigate to external URLs in browser"). This is worth confirming before merge but is not a blocker for code review. ### PROCESS OBSERVATIONS - This PR closes a spike issue (#25) by replacing its output with the actual implementation doc. Good spike lifecycle practice. - The link navigation change (fixing cards to link to actual URLs) was previously addressed in PR #44 but appears to have been staged separately here. The PR body acknowledges this ("Also includes minor view/config cleanup that was staged"). Slightly mixed scope but acceptable given the small size. - The `palinks-dev` host removal undoes work from PR #46 (merged one commit ago). This suggests the host entry was short-lived. Consider whether the host was needed at all, or if there is a naming convention issue to resolve upstream. ### VERDICT: APPROVED