fix: render hapi relay url on demand

[axisrow]

Summary

• Render the dashboard "HAPI Server" link on demand from live hapi runtime files: server.log relay URL + settings.json cliApiToken.
• Keep /home/hapi/url as a compatibility fallback only while the hapi binary is available, so headless INSTALL_HAPI=false deployments do not resurrect stale links from a persisted volume.
• Document the new behavior in README/CLAUDE/.env example comments.

Approach

Chose variant A from the brief: runtime render in the menu request path. This removes the race with the entrypoint's one-shot 60s URL writer because the dashboard no longer depends on that file being created at startup.

Environment

No new or changed environment variables.

Validation

• Added failing tests first for runtime URL construction, missing URL/token cases, missing files, and stale headless data.
• python -m pytest tests/ (356 passed)
• docker build -t clihost:hapi-url-race-64 .
• Docker smoke: container /health returned ok, sshd stayed running, and the runtime URL builder worked inside the built image.

Closes #64

[axisrow]

@claude @codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 3m 2s —— View job

---

Code Review — PR #87

• Gather context / read changed files
• Review views.py — core URL rendering logic
• Review app.py — integration point
• Review test file — coverage correctness
• Post findings

---

Summary

The approach is sound and solves the race condition cleanly. Two real issues worth fixing before merge, one low-severity note.

---

1. Unbounded server.log read on every dashboard request — potential production latency

load_runtime_hapi_url reads the entire server.log on every GET / request:

# views.py:147
server_log_text = Path(server_log_file).read_text(encoding="utf-8", errors="replace")

The log is written by stdbuf -oL hapi server --relay 2>&1 | tee "${HAPI_SERVER_LOG}" (entrypoint.sh:436) with no rotation, truncation, or size cap. A container running for days/weeks accumulates unbounded log output. re.findall then scans the entire buffer looking for the last relay URL.

This also means settings.json is read unnecessarily on every dashboard load even when the relay URL hasn't changed.

Mitigations to consider:

• Read only the tail of server.log (e.g., last 8 KB via seek) — the most recent URL is what matters and will always be near the end.
• Or, cache the resolved URL at module level and invalidate it on file mtime change (cheap os.stat before the full read).

Fix this →

---

2. which("hapi") called on every dashboard request — unnecessary per-request work

In app.py:438:

hapi_url = load_dashboard_hapi_url(
    HAPI_HOME,
    HAPI_URL_FILE,
    hapi_available=which("hapi") is not None,
)

shutil.which walks every directory in PATH doing os.access() calls on each iteration. Whether hapi is installed doesn't change at runtime — it's a build-time decision. This should be computed once at module startup:

# module level, evaluated once at import
_HAPI_AVAILABLE = which("hapi") is not None

and reused in the handler. Not catastrophic (no subprocess), but it's avoidable work on every page load.

Fix this →

---

3. Dead guard if not token: — unreachable code (informational)

# views.py:136-138
token = token_match.group(1)
if not token:
    return None

_HAPI_CLI_TOKEN_RE's capture group is [^"]+ — requires one or more non-quote characters — so token_match.group(1) can never be an empty string if the match succeeded. The if not token: branch is dead. This is harmless but worth noting.

---

What looks correct

• The _HAPI_RELAY_URL_RE pattern matches the entrypoint's grep -oE pattern exactly; using relay_urls[-1] (last match) correctly handles log files from containers that have restarted and gotten a new relay URL.
• Paths are consistent: HAPI_HOME/server.log and HAPI_HOME/settings.json match what the entrypoint writes as HAPI_SERVER_LOG and HAPI_SETTINGS_FILE.
• The hapi_available=False guard correctly short-circuits the entire lookup (including the legacy fallback) for headless INSTALL_HAPI=false builds, preventing stale volume data from reappearing.
• URL construction is safe: quote(relay_url, safe="") + quote(token, safe="") then html.escape(..., quote=True) in render_menu_page gives the correct double-encoding (URL-encoded in the href value, HTML-escaped in the attribute). No injection path.
• Test coverage is solid for the new functions.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92760af095

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Bound relay log reads on dashboard loads

In long-lived containers where /home/hapi/.hapi/server.log grows because the entrypoint continuously tees hapi server --relay output there, every authenticated dashboard request now reads the entire log and then scans all matches before rendering the menu. A large log can stall or OOM the proxy, making the dashboard/terminals unavailable; read only a bounded tail or cache the discovered relay URL instead of loading the full file on each request.

Useful? React with 👍 / 👎.

[axisrow]

Addressed the confirmed review findings in e807c46:

• load_runtime_hapi_url now reads a bounded 8192-byte tail of server.log instead of loading the whole file on each dashboard request.
• which("hapi") is now evaluated once at module import and reused by handle_menu.
• Added unit coverage for bounded tail selection.

Claude note #3 about the dead if not token: guard is informational: the branch is harmless defensive code and does not affect functionality or production behavior.

Validation after the fixes:

• ruff check app/ttydproxy/app.py app/ttydproxy/views.py tests/unit/test_load_hapi_url.py
• python -m pytest tests/ (357 passed)
• docker build -t clihost:hapi-url-race-64 .
• Docker smoke: /health ok, sshd running, runtime URL builder works inside the image.

Note: full ruff check app tests still reports a pre-existing unrelated E741 in tests/unit/test_shell_scripts.py:1082, outside this PR diff.

[axisrow]

@claude @codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 2m 43s —— View job

---

Code Review — PR #87

• Gather context / understand PR
• Read app/ttydproxy/views.py — core URL rendering logic (post-fix)
• Read app/ttydproxy/app.py — integration point (post-fix)
• Read tests/unit/test_load_hapi_url.py — test coverage
• Check diff against origin/main
• Post findings

---

Both issues from the prior round have been correctly addressed. Reviewing the current state of the branch for any remaining critical problems.

---

1. settings.json read is unbounded — low-severity, noted for completeness

load_runtime_hapi_url tails server.log to 8 KB (good), but reads settings.json without a size bound:

# views.py:149
settings_text = Path(settings_file).read_text(encoding="utf-8", errors="replace")

settings.json is an internal file written by hapi itself and lives at /home/hapi/.hapi/settings.json — not user-controlled input. In normal operation it's a handful of bytes. The risk is negligible in the current threat model; mentioning it for completeness.

---

2. Token is not URL-encoded in the legacy fallback file — pre-existing, does not affect the new runtime path

The entrypoint's legacy URL-writer (line 454 of entrypoint.sh) only encodes : and / in the relay URL and writes the token raw:

ENCODED_URL=$(echo "$RELAY_URL" | sed 's/:/%3A/g; s/\//%2F/g')
FULL_URL="https://app.hapi.run/?hub=${ENCODED_URL}&token=${TOKEN}"

The new runtime path in build_hapi_url_from_runtime correctly calls quote(token, safe=""), so the primary path is fine. The legacy file is only hit when both server.log and settings.json are missing (i.e. very early startup or no-relay deploy). If the cliApiToken contains characters like +, =, or & the legacy file URL would be malformed — but the new runtime path, which is now primary, is correct. Pre-existing issue; not introduced by this PR.

---

What looks correct after the fixes

• _read_text_tail: The seek(0, 2) → tell() → seek(max(0, size - 8192)) pattern is the standard bounded-tail idiom. errors="replace" prevents any UnicodeDecodeError from a split UTF-8 boundary. ✓
• _HAPI_AVAILABLE at module level: which("hapi") is now evaluated once at import, not per-request. ✓
• Regex consistency: _HAPI_RELAY_URL_RE (https://[A-Za-z0-9-]+\.relay\.hapi\.run) is identical to the entrypoint's grep -oE pattern. relay_urls[-1] correctly picks the most recent URL across restarts. ✓
• Headless guard: hapi_available=False short-circuits the entire lookup including the legacy fallback, so stale volume data never resurfaces in INSTALL_HAPI=false builds. ✓
• No injection path: relay URL comes from a strict regex match, token from a capture group — both are quote()'d into the query string, then html.escape(..., quote=True)'d in the template. ✓
• Path alignment: HAPI_HOME / "server.log" matches entrypoint.sh's HAPI_SERVER_LOG="${HAPI_HOME}/server.log". ✓
• Test for tail bounding: test_reads_only_bounded_server_log_tail writes old_url + padding(8192 bytes) + current_url and asserts only current_url is found. The math checks out. ✓
• OSError catch: Handles missing files, permission errors, and fresh containers before hapi has written either file — falls back gracefully to None (no menu link) until the files exist. ✓

---

Summary

No critical issues remain. The two confirmed findings from the first review (unbounded log read, per-request which) are properly fixed. The dead if not token: guard is harmless defensive code. Ready to merge.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: e807c462a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".