Fix stale renders and bind to 0.0.0.0 for reverse proxy #15

Merged
ldraney merged 1 commit from fix/mdview-cache-and-bind into main 2026-06-06 02:43:08 +00:00
Owner

Summary

  • Add Cache-Control: no-store and Turbo no-cache meta tag to prevent browser from showing stale content after re-running mdview
  • Bind puma to 0.0.0.0 so k8s nginx reverse proxy can reach archbox via Tailscale IP
  • Add docs/architecture.md with mermaid diagrams covering the full network topology

Changes

  • app/controllers/markdown_controller.rb: add Cache-Control: no-store response header
  • app/views/layouts/application.html.erb: add <meta name="turbo-cache-control" content="no-cache">
  • bin/mdview: add -b 0.0.0.0 to rails server command
  • docs/architecture.md: new doc covering local CLI, k8s reverse proxy, Tailscale Funnel, single-instance model, and file resolution

Test Plan

  • curl -sI localhost:3137 returns Cache-Control: no-store
  • HTML includes <meta name="turbo-cache-control" content="no-cache">
  • ss -tlnp shows puma bound to 0.0.0.0:3137
  • k8s nginx pod can reach archbox via wget from inside the pod
  • Tailscale Funnel URL shows correct local file (screenshot confirmed)

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Closes #14
  • ldraney/pal-e-deployments #185 -- companion fix for k8s overlay
## Summary - Add `Cache-Control: no-store` and Turbo `no-cache` meta tag to prevent browser from showing stale content after re-running mdview - Bind puma to `0.0.0.0` so k8s nginx reverse proxy can reach archbox via Tailscale IP - Add `docs/architecture.md` with mermaid diagrams covering the full network topology ## Changes - `app/controllers/markdown_controller.rb`: add `Cache-Control: no-store` response header - `app/views/layouts/application.html.erb`: add `<meta name="turbo-cache-control" content="no-cache">` - `bin/mdview`: add `-b 0.0.0.0` to `rails server` command - `docs/architecture.md`: new doc covering local CLI, k8s reverse proxy, Tailscale Funnel, single-instance model, and file resolution ## Test Plan - [x] `curl -sI localhost:3137` returns `Cache-Control: no-store` - [x] HTML includes `<meta name="turbo-cache-control" content="no-cache">` - [x] `ss -tlnp` shows puma bound to `0.0.0.0:3137` - [x] k8s nginx pod can reach archbox via `wget` from inside the pod - [x] Tailscale Funnel URL shows correct local file (screenshot confirmed) ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #14 - `ldraney/pal-e-deployments #185` -- companion fix for k8s overlay
Fix stale renders and bind to all interfaces for reverse proxy
Some checks failed
CI / scan_ruby (pull_request) Has been cancelled
CI / scan_js (pull_request) Has been cancelled
CI / lint (pull_request) Has been cancelled
06aa06631e
Browser showed old file after re-running mdview because Turbo Drive
cached the page at / and Cache-Control allowed disk caching. Now
responses include no-store and turbo-cache-control no-cache.

Also bind to 0.0.0.0 instead of localhost so the k8s nginx reverse
proxy can reach archbox via Tailscale IP.

Adds docs/architecture.md covering the full network topology.

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

PR #15 Review

DOMAIN REVIEW

Tech stack: Ruby on Rails 8.1, Puma, Turbo Drive, Bash shell script, Mermaid documentation.

Rails / Cache-Control

The Cache-Control: no-store header in markdown_controller.rb (line 10) is the correct directive for this use case. no-store tells the browser to never cache the response at all, which is exactly what is needed when the same URL can serve different files after a server restart. This is stronger than no-cache (which still stores but revalidates), and appropriate here since there is no performance cost -- this is a local dev tool, not a high-traffic service.

The <meta name="turbo-cache-control" content="no-cache"> in application.html.erb (line 6) correctly tells Turbo Drive not to serve cached snapshots when navigating. This is the Turbo-specific complement to the HTTP header -- both are needed because Turbo maintains its own in-memory page cache independent of the browser HTTP cache.

Puma bind address

The -b 0.0.0.0 flag in bin/mdview (line 32) is consistent with config/puma.rb (line 31) which already has bind "tcp://0.0.0.0:#{ENV.fetch("PORT", 3137)}". The CLI flag and the puma config are now aligned. Previously the CLI was not passing -b, so puma would bind per its config file -- the explicit flag is redundant but harmless, and makes the intent visible in the script. No conflict.

Security consideration -- binding to 0.0.0.0: This is a local developer tool on a Tailscale-connected machine, not a public-facing service. The path traversal guard in resolve_file (line 34) properly constrains file access to the base_dir subtree. The config.hosts allowlist in development.rb (line 52) restricts which Host headers are accepted. Acceptable for this use case.

Documentation

docs/architecture.md is well-structured with accurate Mermaid diagrams. One minor factual note: the sequence diagram on line 53 shows exec bin/rails server -p 3137 without the -b 0.0.0.0 flag that this PR adds. The actual command after this PR is exec bin/rails server -b 0.0.0.0 -p "$PORT". This is a documentation drift within the same PR.

The Tailscale IP 100.110.151.59 appears in the architecture doc (lines 25, 65). This is a Tailscale CGNAT address (100.64.0.0/10 range), not a public IP, so it is not a security exposure. It is a hardcoded value that will break if the machine's Tailscale IP changes, but that is a property of the deployment overlay, not this repo.

BLOCKERS

None.

The project has no test directory (test/ or spec/ does not exist). This is a local developer tool (markdown viewer CLI) with no existing test infrastructure. The changes are a 1-line HTTP header, a 1-line meta tag, and a 1-line CLI flag -- all configuration-level changes, not new functionality requiring unit tests. The BLOCKER criterion for "new functionality with zero test coverage" does not apply to pure configuration changes in a project with no test harness.

No secrets, credentials, or unvalidated user input introduced. No DRY violations. No auth/security path changes.

NITS

  1. Sequence diagram drift -- docs/architecture.md line 53 shows exec bin/rails server -p 3137 but the actual command after this PR is exec bin/rails server -b 0.0.0.0 -p "$PORT". Consider updating the diagram to match.

  2. Redundant bind flag -- bin/mdview passes -b 0.0.0.0 on the command line, and config/puma.rb already has bind "tcp://0.0.0.0:...". Both work, but the duplication means a future change to the bind address requires updating two places. Consider removing one. The puma config is the canonical location for Rails apps; the CLI flag makes the intent visible in the script. Either approach is fine, but pick one source of truth.

SOP COMPLIANCE

  • Branch named after issue -- Branch is fix/mdview-cache-and-bind, not 14-fix-stale-renders or similar {issue-number}-{kebab-case} convention. Minor deviation.
  • PR body follows template -- Has Summary, Changes, Test Plan, Related sections.
  • Related references plan slug -- No plan slug referenced (confirmed: none exists for this work).
  • No secrets committed -- No credentials, keys, or .env files in the diff.
  • No unnecessary file changes -- All 4 files are directly related to the stated goals.
  • Commit messages are descriptive -- (assessed from PR title/body; commit-level messages not available in diff).

PROCESS OBSERVATIONS

  • Change failure risk: Low. All changes are configuration-level (HTTP header, meta tag, CLI flag) with no logic changes. The architecture doc is additive.
  • Deployment frequency: Neutral. This is a local tool; the companion k8s change is in a separate repo (pal-e-deployments #185).
  • Documentation quality: Good. Adding docs/architecture.md with accurate Mermaid diagrams for a project that had none is a positive investment. Fix the sequence diagram nit to avoid immediate drift.

VERDICT: APPROVED

Clean, minimal, well-scoped changes. The cache-busting approach is correct for the single-instance restart model. No blockers. The sequence diagram nit and branch naming deviation are non-blocking.

## PR #15 Review ### DOMAIN REVIEW **Tech stack:** Ruby on Rails 8.1, Puma, Turbo Drive, Bash shell script, Mermaid documentation. **Rails / Cache-Control** The `Cache-Control: no-store` header in `markdown_controller.rb` (line 10) is the correct directive for this use case. `no-store` tells the browser to never cache the response at all, which is exactly what is needed when the same URL can serve different files after a server restart. This is stronger than `no-cache` (which still stores but revalidates), and appropriate here since there is no performance cost -- this is a local dev tool, not a high-traffic service. The `<meta name="turbo-cache-control" content="no-cache">` in `application.html.erb` (line 6) correctly tells Turbo Drive not to serve cached snapshots when navigating. This is the Turbo-specific complement to the HTTP header -- both are needed because Turbo maintains its own in-memory page cache independent of the browser HTTP cache. **Puma bind address** The `-b 0.0.0.0` flag in `bin/mdview` (line 32) is consistent with `config/puma.rb` (line 31) which already has `bind "tcp://0.0.0.0:#{ENV.fetch("PORT", 3137)}"`. The CLI flag and the puma config are now aligned. Previously the CLI was not passing `-b`, so puma would bind per its config file -- the explicit flag is redundant but harmless, and makes the intent visible in the script. No conflict. **Security consideration -- binding to 0.0.0.0:** This is a local developer tool on a Tailscale-connected machine, not a public-facing service. The path traversal guard in `resolve_file` (line 34) properly constrains file access to the `base_dir` subtree. The `config.hosts` allowlist in `development.rb` (line 52) restricts which Host headers are accepted. Acceptable for this use case. **Documentation** `docs/architecture.md` is well-structured with accurate Mermaid diagrams. One minor factual note: the sequence diagram on line 53 shows `exec bin/rails server -p 3137` without the `-b 0.0.0.0` flag that this PR adds. The actual command after this PR is `exec bin/rails server -b 0.0.0.0 -p "$PORT"`. This is a documentation drift within the same PR. The Tailscale IP `100.110.151.59` appears in the architecture doc (lines 25, 65). This is a Tailscale CGNAT address (100.64.0.0/10 range), not a public IP, so it is not a security exposure. It is a hardcoded value that will break if the machine's Tailscale IP changes, but that is a property of the deployment overlay, not this repo. ### BLOCKERS None. The project has no test directory (`test/` or `spec/` does not exist). This is a local developer tool (markdown viewer CLI) with no existing test infrastructure. The changes are a 1-line HTTP header, a 1-line meta tag, and a 1-line CLI flag -- all configuration-level changes, not new functionality requiring unit tests. The BLOCKER criterion for "new functionality with zero test coverage" does not apply to pure configuration changes in a project with no test harness. No secrets, credentials, or unvalidated user input introduced. No DRY violations. No auth/security path changes. ### NITS 1. **Sequence diagram drift** -- `docs/architecture.md` line 53 shows `exec bin/rails server -p 3137` but the actual command after this PR is `exec bin/rails server -b 0.0.0.0 -p "$PORT"`. Consider updating the diagram to match. 2. **Redundant bind flag** -- `bin/mdview` passes `-b 0.0.0.0` on the command line, and `config/puma.rb` already has `bind "tcp://0.0.0.0:..."`. Both work, but the duplication means a future change to the bind address requires updating two places. Consider removing one. The puma config is the canonical location for Rails apps; the CLI flag makes the intent visible in the script. Either approach is fine, but pick one source of truth. ### SOP COMPLIANCE - [ ] Branch named after issue -- Branch is `fix/mdview-cache-and-bind`, not `14-fix-stale-renders` or similar `{issue-number}-{kebab-case}` convention. Minor deviation. - [x] PR body follows template -- Has Summary, Changes, Test Plan, Related sections. - [ ] Related references plan slug -- No plan slug referenced (confirmed: none exists for this work). - [x] No secrets committed -- No credentials, keys, or .env files in the diff. - [x] No unnecessary file changes -- All 4 files are directly related to the stated goals. - [x] Commit messages are descriptive -- (assessed from PR title/body; commit-level messages not available in diff). ### PROCESS OBSERVATIONS - **Change failure risk: Low.** All changes are configuration-level (HTTP header, meta tag, CLI flag) with no logic changes. The architecture doc is additive. - **Deployment frequency: Neutral.** This is a local tool; the companion k8s change is in a separate repo (`pal-e-deployments #185`). - **Documentation quality: Good.** Adding `docs/architecture.md` with accurate Mermaid diagrams for a project that had none is a positive investment. Fix the sequence diagram nit to avoid immediate drift. ### VERDICT: APPROVED Clean, minimal, well-scoped changes. The cache-busting approach is correct for the single-instance restart model. No blockers. The sequence diagram nit and branch naming deviation are non-blocking.
ldraney deleted branch fix/mdview-cache-and-bind 2026-06-06 02:43:08 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/mdview!15
No description provided.