Review: Configure Caddy reverse proxy for landscaping-assistant.app on edge-proxy (re-review)

review-1463-2026-06-14-v2 Review

review approved

Verdict: APPROVED

Re-review of board item #1463. Previous review review-1463-2026-06-14 returned NEEDS_REFINEMENT with 4 [BODY] and 2 [SCOPE] recommendations. All [BODY] issues are resolved. [SCOPE] gaps acknowledged by caller as being addressed separately.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- present, explicitly documents #425 dependency with ordering rationale
  • [x] Repo -- pal-e-platform
  • [x] User Story -- well-formed, platform operator perspective
  • [x] Context -- thorough, references #424 (closed), #425 (open), explains edge-proxy architecture
  • [x] File Targets -- concrete: salt/states/caddy/Caddyfile.j2 with modify/do-not-touch sections
  • [x] Feature Flag -- none (correct, infrastructure work)
  • [x] Acceptance Criteria -- 7 items, all verifiable
  • [x] Test Expectations -- 4 items including run command
  • [x] Constraints -- 5 items, includes blast radius awareness
  • [x] Checklist -- present, 3 items
  • [x] Related -- present

Traceability

  • [x] story:custom-domain label -- present on board item
  • [ ] story note MISSING -- custom-domain not in project-landscaping-assistant user-stories table. [SCOPE] Known gap, being addressed separately per caller. Not blocking.
  • [x] arch:pal-e-platform label -- present on board item
  • [ ] arch note MISSING -- no arch-pal-e-platform note found. [SCOPE] Known gap, being addressed separately per caller. Not blocking.
  • [x] Forgejo issue -- pal-e-platform#434, state: open

File Targets

  • [x] salt/states/caddy/Caddyfile.j2 -- does not exist yet in repo (confirmed via Forgejo tree API). Will be created by #425 (palinks.app Caddy config, state: open). This ticket adds to that file. Correct: ticket states "Depends on #425 (must land first to establish salt/states/caddy/ structure)".
  • [x] Do-not-touch list is explicit: salt/states/caddy/init.sls and palinks.app block. Mirrors #425 pattern correctly.

Repo Placement

OK. Issue filed on pal-e-platform, work is in pal-e-platform salt states. Single repo scope is correct.

Dependencies

  • pal-e-platform#425 (palinks.app Caddy config) -- state: open. Explicitly documented as blocking dependency in Lineage. Must land first to create salt/states/caddy/ structure. Correct.
  • pal-e-platform#435 (DNS A record via godaddy-tofu) -- state: open. Related in Lineage. DNS must resolve before Caddy can obtain TLS cert, but the Caddy config can be deployed before DNS -- cert provisioning just won't complete until DNS is live. Ordering is flexible.
  • pal-e-platform#424 (Hetzner edge provisioning) -- referenced in Context as related. Not a blocking dependency for this ticket.
  • Board items #1461 and #1460 (landscaping-assistant story:custom-domain) -- related items on same board, no blocking relationship.

Acceptance Criteria

7 AC items, all verifiable:
  • curl -I https://landscaping-assistant.app returns 200 -- verifiable post-deploy
  • curl -I https://www.landscaping-assistant.app returns 301 -- verifiable post-deploy (FIXED from previous review)
  • TLS auto-provisioned via Let's Encrypt -- verifiable with curl
  • Caddy config includes server block proxying to landscaping-assistant.tail5b443a.ts.net:443 -- verifiable by reading file (FIXED: explicit target address)
  • Traffic proxied to correct k3s service via Tailscale mesh -- verifiable with curl
  • palinks.app routing unaffected -- regression check, verifiable with curl
  • Salt state applied successfully -- verifiable with salt-call state.apply caddy

Blast Radius

Low but correctly flagged. The edge VPS (178.156.129.142) serves multiple domains. A Caddyfile syntax error could break all routing. The Constraints section explicitly states "Must not break existing palinks.app routing" and AC includes palinks.app regression check. Issue #425 also notes blast radius awareness. The Salt-managed approach ensures reproducibility.

Decomposition Assessment

No decomposition needed. Single repo, 1 file target, 7 AC items (slightly above 5, but all are simple verification curls on the same change). Estimated agent work well under 5 minutes. Straightforward additive config mirroring #425 pattern.

Previous Review Findings -- Resolution Status

  • [x] [BODY] Concrete file targets -- RESOLVED. Now specifies salt/states/caddy/Caddyfile.j2 with modify/do-not-touch lists.
  • [x] [BODY] Proxy target address -- RESOLVED. Now specifies landscaping-assistant.tail5b443a.ts.net:443 in AC and Constraints.
  • [x] [BODY] www redirect AC -- RESOLVED. AC now includes www.landscaping-assistant.app 301 redirect check.
  • [x] [BODY] Ordering dependency on #425 -- RESOLVED. Lineage explicitly states "must land first."
  • [ ] [SCOPE] custom-domain story note -- NOT YET RESOLVED. Being addressed separately per caller instructions. Not blocking.
  • [ ] [SCOPE] arch-pal-e-platform note -- NOT YET RESOLVED. Being addressed separately per caller instructions. Not blocking.

Recommendations

No action needed. All [BODY] recommendations from the previous review have been addressed. The two [SCOPE] gaps (missing custom-domain story note and missing arch-pal-e-platform note) are acknowledged as being addressed separately and do not block this ticket from moving to next_up.