Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 41 additions & 17 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1859,23 +1859,6 @@ jobs:
# with the GIL quietly re-enabled.
DISABLE_SQLALCHEMY_CEXT_RUNTIME: "1"

services:
# For the image smoke test at the end of the job. The pytest run above uses the
# embedded pg0, not this.
postgres:
image: pgvector/pgvector:pg17
env:
POSTGRES_USER: hindsight
POSTGRES_PASSWORD: hindsight
POSTGRES_DB: hindsight
ports:
- 5432:5432
options: >-
--health-cmd "pg_isready -U hindsight"
--health-interval 5s
--health-timeout 5s
--health-retries 10

steps:
- uses: actions/checkout@v6
with:
Expand Down Expand Up @@ -1942,6 +1925,47 @@ jobs:
./.venv-ft/bin/python -m pytest tests -v \
-m "not hs_llm_mat and not hs_llm_core"

build-freethreaded-image:
needs: [detect-changes]
# Gated on `docker` as well as `core`: this is the only job that builds
# Dockerfile.freethreaded, and it used to hang off the free-threaded pytest job,
# which does not run for a docker-only change. Two Dockerfile.freethreaded PRs
# merged without CI ever building the image before this was split out.
#
# Deliberately NOT gated on `has_secrets`, for the same reason as the pytest job
# above: the smoke test runs a mock LLM and a stub embedder, so it needs no
# provider credentials and therefore also covers fork PRs, which skip
# build-docker-images entirely.
if: >-
github.event_name == 'workflow_dispatch' ||
needs.detect-changes.outputs.core == 'true' ||
needs.detect-changes.outputs.docker == 'true' ||
needs.detect-changes.outputs.ci == 'true'
runs-on: ubuntu-latest
timeout-minutes: 30
name: build-freethreaded-image

services:
# For the smoke test; the image runs against this rather than embedded pg0.
postgres:
image: pgvector/pgvector:pg17
env:
POSTGRES_USER: hindsight
POSTGRES_PASSWORD: hindsight
POSTGRES_DB: hindsight
ports:
- 5432:5432
options: >-
--health-cmd "pg_isready -U hindsight"
--health-interval 5s
--health-timeout 5s
--health-retries 10

steps:
- uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.head.sha || '' }}

- name: Build the free-threaded image
# Its own Dockerfile, not a target of the standalone one: different interpreter,
# different resolution, no control plane, no local ML. The build itself asserts
Expand Down
4 changes: 1 addition & 3 deletions docker/standalone/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ WORKDIR /app
# commit can now resolve different package versions.
RUN apt-get update && apt-get upgrade -y \
&& apt-get install -y \
curl \
procps \
libssl3 \
libgssapi-krb5-2 \
Expand Down Expand Up @@ -310,7 +309,7 @@ FROM python:3.11-slim AS standalone

WORKDIR /app

# Install curl, uv, and system dependencies. Node arrives separately, below.
# Install uv and system dependencies. Node arrives separately, below.
# Note: libicu version varies by Debian version - try common versions in order
# Runtime images use uv directly; remove pip build tooling after installation so
# vulnerable setuptools-vendored packages and wheel are not shipped in production.
Expand All @@ -321,7 +320,6 @@ WORKDIR /app
# commit can now resolve different package versions.
RUN apt-get update && apt-get upgrade -y \
&& apt-get install -y \
curl \
procps \
libssl3 \
libgssapi-krb5-2 \
Expand Down
2 changes: 1 addition & 1 deletion docker/standalone/Dockerfile.freethreaded
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ FROM python:3.14-slim AS api-only-freethreaded
WORKDIR /app

RUN apt-get update && apt-get upgrade -y \
&& apt-get install -y --no-install-recommends curl procps libssl3 libpq5 \
&& apt-get install -y --no-install-recommends procps libssl3 libpq5 \
&& rm -rf /var/lib/apt/lists/* \
&& useradd -m -u 1000 hindsight

Expand Down
34 changes: 32 additions & 2 deletions docker/standalone/start-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,34 @@ resolve_api_startup_wait_seconds() {
echo "$DEFAULT_API_STARTUP_WAIT_SECONDS"
}

# =============================================================================
# HTTP readiness probe
#
# The implementation is hindsight_api.http_probe, not Python embedded here: it
# needs to be linted, type-checked and unit-tested, and the parity rules it
# encodes (notably that `curl -sf` does NOT follow redirects) are too easy to
# get subtly wrong to leave in a shell string. See that module's docstring.
#
# Every image that probes anything ships the API package, so `python3 -m` finds
# it. It is held to stdlib-only imports by a test - see its docstring. cp-only
# has no Python at all and probes nothing.
# =============================================================================
http_probe() {
local url="$1"
local timeout_seconds="${2:-5}"

python3 -m hindsight_api.http_probe "$url" "$timeout_seconds"
}

# A probe that cannot run at all would silently degrade into "never ready", so
# check once, up front, where it can still say why.
require_http_probe_runtime() {
if ! python3 -c "import hindsight_api.http_probe" >/dev/null 2>&1; then
echo "❌ HTTP readiness probes need python3 with hindsight_api.http_probe importable."
exit 1
fi
}

if [ "${HINDSIGHT_START_ALL_SOURCE_ONLY:-false}" = "true" ]; then
return 0 2>/dev/null || exit 0
fi
Expand All @@ -143,6 +171,7 @@ ENABLE_CP="${HINDSIGHT_ENABLE_CP:-true}"
# This wait loop ensures dependencies are ready before starting.
# =============================================================================
if [ "${HINDSIGHT_WAIT_FOR_DEPS:-false}" = "true" ]; then
require_http_probe_runtime
LLM_BASE_URL="${HINDSIGHT_API_LLM_BASE_URL:-http://host.docker.internal:1234/v1}"
MAX_RETRIES="${HINDSIGHT_RETRY_MAX:-0}" # 0 = infinite
RETRY_INTERVAL="${HINDSIGHT_RETRY_INTERVAL:-10}"
Expand All @@ -167,7 +196,7 @@ if [ "${HINDSIGHT_WAIT_FOR_DEPS:-false}" = "true" ]; then
}

check_llm() {
curl -sf "${LLM_BASE_URL}/models" --connect-timeout 5 &>/dev/null
http_probe "${LLM_BASE_URL}/models" 5 &>/dev/null
}

echo "⏳ Waiting for dependencies to be ready..."
Expand Down Expand Up @@ -263,6 +292,7 @@ PIDS=()

# Start API if enabled
if [ "$ENABLE_API" = "true" ]; then
require_http_probe_runtime
cd /app/api
API_HEALTH_URL="${HINDSIGHT_API_HEALTH_URL:-http://localhost:${HINDSIGHT_API_PORT:-8888}/health}"
API_STARTUP_WAIT_SECONDS="$(resolve_api_startup_wait_seconds)"
Expand All @@ -279,7 +309,7 @@ if [ "$ENABLE_API" = "true" ]; then
wait "$API_PID"
exit $?
fi
if curl -sf "$API_HEALTH_URL" &>/dev/null; then
if http_probe "$API_HEALTH_URL" 5 &>/dev/null; then
api_ready=true
break
fi
Expand Down
78 changes: 77 additions & 1 deletion docker/standalone/test-start-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ source "$SCRIPT_DIR/start-all.sh"
unset HINDSIGHT_START_ALL_SOURCE_ONLY

TMP_DIR="$(mktemp -d)"
trap 'chmod -R u+rwx "$TMP_DIR" 2>/dev/null || true; rm -rf "$TMP_DIR"' EXIT
HTTP_SERVER_PID=""

cleanup() {
if [ -n "$HTTP_SERVER_PID" ]; then
kill "$HTTP_SERVER_PID" 2>/dev/null || true
wait "$HTTP_SERVER_PID" 2>/dev/null || true
fi
chmod -R u+rwx "$TMP_DIR" 2>/dev/null || true
rm -rf "$TMP_DIR"
}
trap cleanup EXIT

assert_contains() {
local output="$1"
Expand Down Expand Up @@ -44,6 +54,72 @@ assert_empty() {
fi
}

# =============================================================================
# http_probe wiring
#
# The probe's semantics are covered by pytest, against the module that
# implements them: hindsight-api-slim/tests/test_http_probe.py. All that is
# left to check here is that this script delegates to it correctly.
#
# Skipped when hindsight_api.http_probe is not importable - this file also runs in CI
# from a bare checkout with no virtualenv, where only the pg0 helpers below
# are exercisable.
# =============================================================================
if python3 -c "import hindsight_api.http_probe" >/dev/null 2>&1; then
HTTP_PORT_FILE="$TMP_DIR/http-port"

python3 - "$HTTP_PORT_FILE" <<'PY' &
import http.server
import pathlib
import sys


class Handler(http.server.BaseHTTPRequestHandler):
def do_GET(self):
self.send_response(204 if self.path == "/ok" else 404)
self.end_headers()

def log_message(self, *_args):
pass


server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler)
pathlib.Path(sys.argv[1]).write_text(str(server.server_port), encoding="ascii")
server.serve_forever()
PY
HTTP_SERVER_PID=$!

for _ in $(seq 1 50); do
[ -s "$HTTP_PORT_FILE" ] && break
sleep 0.1
done
if [ ! -s "$HTTP_PORT_FILE" ]; then
echo "HTTP probe test server did not start"
exit 1
fi
HTTP_TEST_URL="http://127.0.0.1:$(cat "$HTTP_PORT_FILE")"

if ! http_probe "$HTTP_TEST_URL/ok" 5 >/dev/null 2>&1; then
echo "http_probe should succeed against a healthy endpoint"
exit 1
fi
if http_probe "$HTTP_TEST_URL/missing" 5 >/dev/null 2>&1; then
echo "http_probe should fail against a 404"
exit 1
fi
if ! require_http_probe_runtime; then
echo "require_http_probe_runtime should pass when the module imports"
exit 1
fi

kill "$HTTP_SERVER_PID" 2>/dev/null || true
wait "$HTTP_SERVER_PID" 2>/dev/null || true
HTTP_SERVER_PID=""
echo "start-all HTTP probe wiring checks passed"
else
echo "start-all HTTP probe wiring checks skipped (hindsight_api.http_probe not importable)"
fi

mkdir -p "$TMP_DIR/empty"
assert_empty "$(check_pg0_data_integrity "$TMP_DIR/empty")"

Expand Down
109 changes: 109 additions & 0 deletions hindsight-api-slim/hindsight_api/http_probe.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
"""Container readiness probe. Standard library only, by rule.

``docker/standalone/start-all.sh`` polls the API's health endpoint while it
starts. That used to be ``curl -sf``, which meant shipping curl - and with it
libcurl and libssh2 - in every runtime image to make one GET request. Nothing
else in those images used it, and the three packages carried nine HIGH CVEs
with no Debian fix available.

**This module must never import the engine, the config, or any third-party
package.** It answers "is an API process up?", and pulling the application in
to ask that would put the application's startup cost - and its side effects -
on a loop that runs once per second. ``hindsight-admin`` is the cautionary
number: it takes ~5s to start in the built image because it loads the CLI and
everything behind it, against ~0.03s for this.

That rule is a test, not a convention:
``tests/test_http_probe.py::test_imports_nothing_heavy`` imports this module in
a clean subprocess and asserts that it pulled in no third-party module and none
of the engine. Living next to the code it must not touch is exactly why the
check is automated.

The contract is ``curl -sf`` *without* ``-L``, which is what this replaced:

* 2xx and 3xx succeed. curl does not follow redirects unless asked, so a 302 is
a completed transfer, not a failure. This matters more than it looks:
``urllib.request.urlopen`` *does* follow redirects, so the obvious
implementation reports a healthy service that redirects as "not ready".
* 4xx and 5xx fail, as ``-f`` does.
* Connection, DNS and timeout errors fail.
* Credentials in the URL are sent as Basic auth, as curl does.

Exit codes are not reproduced - curl's 22 and 7 both become 1. Every call site
tests zero/non-zero only.

One deliberate difference from what it replaced: curl was invoked with
``--connect-timeout``, which caps only the connection phase, and the API health
loop passed no timeout at all - so a server that accepted a connection and then
never answered hung the probe forever. The timeout here covers the whole
request.
"""

from __future__ import annotations

import base64
import http.client
import sys
from urllib.parse import unquote, urlsplit

__all__ = ["DEFAULT_TIMEOUT_SECONDS", "main", "probe"]

DEFAULT_TIMEOUT_SECONDS = 5.0


def probe(url: str, timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS) -> bool:
"""Return True if ``url`` answers the way ``curl -sf`` would call success.

Never raises: a probe that blew up on an unexpected socket error would be
indistinguishable from a crash in the readiness loop that calls it.
"""
parts = urlsplit(url)

if parts.scheme == "https":
connection: http.client.HTTPConnection = http.client.HTTPSConnection(
parts.hostname or "", parts.port, timeout=timeout_seconds
)
elif parts.scheme == "http":
connection = http.client.HTTPConnection(parts.hostname or "", parts.port, timeout=timeout_seconds)
else:
return False

path = parts.path or "/"
if parts.query:
path = f"{path}?{parts.query}"

headers: dict[str, str] = {}
if parts.username is not None:
raw = f"{unquote(parts.username)}:{unquote(parts.password or '')}"
headers["Authorization"] = "Basic " + base64.b64encode(raw.encode()).decode()

try:
connection.request("GET", path, headers=headers)
status = connection.getresponse().status
except Exception:
return False
finally:
connection.close()

return status < 400


def main(argv: list[str] | None = None) -> int:
args = sys.argv[1:] if argv is None else argv
if not args or len(args) > 2:
print("usage: python -m hindsight_api.http_probe URL [TIMEOUT_SECONDS]", file=sys.stderr)
return 2

timeout_seconds = DEFAULT_TIMEOUT_SECONDS
if len(args) == 2:
try:
timeout_seconds = float(args[1])
except ValueError:
print(f"invalid timeout: {args[1]}", file=sys.stderr)
return 2

return 0 if probe(args[0], timeout_seconds) else 1


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading