Phase 1e — container image, entrypoint, and podman quadlets #17
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No assignees
3 participants
Notifications
Due date
No due date set.
Reference
marvin8/xique!17
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-6-phase-1e-ops-containers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #6
Summary
XIQUE_HOST/XIQUE_PORTsettings with a settings-driven uvicorn bind;database_pathdefault moves to/data/db/xique.dbContainerfileimplementing the CI-encoded contract (uv-pinned python:3.14-slim, editable install at/app/src, non-root uid 1000, tag-baked release metadata) pluscontainer-entrypoint.shand.containerignorecontainerandpublish-containerCI jobs enabled; smoke amended to real first-boot order (worker migrates, then web serves) with a CI-only throwaway secretty-pre-commitrevdeploy/: pod + web + worker + tailscaleservesidecar, host bind mount for the database, named volume for transient Trakt uploads; README "Running it" sectionDocs site and operator guide: #16.
Verification
uv run noxgreen on the branchcontainerjob on this PR: image builds on zhuiri, metadata version check,/app/srcresolution, real entrypoint boots worker→web and/loginrendersWuMing
Found 14 issue(s). See inline comments below.
@ -0,0 +1,42 @@FROM python:3.14-slimshell [HIGH]
Python 3.14 is not yet released and does not exist on official Docker registries. Using 'python:3.14-slim' will cause the image build to fail immediately. Pin to a stable, available release (e.g., 3.12 or 3.13) until 3.14 is officially published.
@ -0,0 +6,4 @@ENV PYTHONUNBUFFERED=1 \PIP_DISABLE_PIP_VERSION_CHECK=1RUN pip install --no-cache-dir "uv==${UV_VERSION}"shell [MEDIUM]
The build argument UV_VERSION lacks a default value. If passed as empty or unset, it expands to 'uv==', which triggers a pip syntax error or installs an unintended version. Add a default value in the ARG declaration (e.g., 'ARG UV_VERSION=0.5.26') or validate the variable before building.
@ -0,0 +36,4 @@&& chown -R appuser:appuser /dataUSER appuserCOPY --chown=root:root container-entrypoint.sh /usr/local/bin/container-entrypoint.shshell [LOW]
The COPY command specifies '--chown=root:root' but executes after 'USER appuser' (line 37). Docker/Podman evaluation order means post-USER COPY operations inherit the active user context, overriding the --chown flag. The file will be created as appuser:appuser. Move this COPY before the USER instruction, or add an explicit 'RUN chown root:root /usr/local/bin/container-entrypoint.sh' afterward for predictable ownership.
@ -115,0 +117,4 @@## Running itThe container image is published to `registry.marvin8.zone/marvin8/xique` onrelease tags, and the `deploy/` directory holds the rootless-podman quadlets:docs [MEDIUM]
Missing code formatting for the directory name. Add backticks around 'deploy/' for consistency with other technical paths in the document.
@ -115,0 +128,4 @@- Host directories: `~/docker/xique/data/db` (the sqlite database — the onlybacked-up content), `~/docker/xique/ts-state` (Tailscale node state), and`~/docker/xique/ts-serve` containing `xique-serve.json` (copied from`deploy/`; adjust the `${TS_CERT_DOMAIN}` proxy target if you change ports).docs [MEDIUM]
Variable '${TS_CERT_DOMAIN}' should be wrapped in backticks. For consistency, also wrap 'xique-serve.json' (referenced on the previous line) in backticks.
@ -115,0 +132,4 @@- `~/docker/xique/xique.env` with at least `XIQUE_SECRET_KEY` (required),optionally `XIQUE_TMDB_TOKEN`, and `XIQUE_IMPORT_DIR=/data/imports-temp`so uploads stream into the shared volume web and worker both mount.- The shared `~/docker/tailscale-systemd.env` pinning `TAILSCALE_VERSION`docs [MEDIUM]
File path '~/docker/tailscale-systemd.env' and environment variable 'TAILSCALE_VERSION' lack code formatting. Wrap both in backticks for clarity and scanability.
@ -115,0 +135,4 @@- The shared `~/docker/tailscale-systemd.env` pinning `TAILSCALE_VERSION`for the sidecar image.Bring-up:docs [LOW]
'Bring-up:' sits loosely between the prerequisite list and the numbered steps. Converting it to a subheading (e.g., '### Bring-up:') will improve document hierarchy and prevent rendering quirks in some markdown parsers.
@ -115,0 +148,4 @@tailnet. On a first-ever boot the worker migrates the database while the webprocess restarts until the schema is ready — this settles in seconds.Upgrades pin `:latest`: `podman pull registry.marvin8.zone/marvin8/xique:latestdocs [LOW]
The container tag ':latest' is missing backticks. Wrap in backticks to clearly distinguish it from surrounding prose.
@ -50,6 +52,8 @@ class Settings(BaseSettings):trusted_proxies: str = ""login_attempt_limit: int = 5login_window_seconds: int = 900host: str = "127.0.0.1"code [MEDIUM]
The new host setting has no validation. An empty XIQUE_HOST value is accepted and passed to uvicorn, which can cause socket.getaddrinfo('') to bind all interfaces, silently overriding the secure loopback default. Add a field validator that rejects empty host strings or explicitly documents/permits only intended values.
@ -56,3 +56,3 @@settings = Settings()expect(settings.database_path).to_equal(Path("/data/xique.db"))expect(settings.database_path).to_equal(Path("/data/db/xique.db"))tests [MEDIUM]
Assertion checks the default of
database_pathwas updated to/data/db/xique.db. This is a literal constant expected to match the new source default. While not a tautology, the test is brittle: it duplicates the default value from the source and would need updating if the default changes again. Consider deriving the expected value from a shared constant or verifying the path is non-None and of type Path.@ -57,2 +57,3 @@expect(settings.database_path).to_equal(Path("/data/xique.db"))expect(settings.database_path).to_equal(Path("/data/db/xique.db"))expect(settings.host).to_equal("127.0.0.1")tests [LOW]
The default host assertion duplicates the
host = "127.0.0.1"literal from the source. It is a valid test but brittle; a change to the source default would require a test change. Not critical.@ -58,1 +58,3 @@expect(settings.database_path).to_equal(Path("/data/xique.db"))expect(settings.database_path).to_equal(Path("/data/db/xique.db"))expect(settings.host).to_equal("127.0.0.1")expect(settings.port).to_equal(8000)tests [LOW]
The default port assertion duplicates the
port = 8000literal. Same brittleness concern.@ -0,0 +79,4 @@original_uvicorn = install_run_recorder(calls)try:server.main()code [LOW]
server.main() calls the real configure_logging(settings), which mutates process-wide logging configuration and can leak into later tests. Patch or mock configure_logging in these entrypoint-binding tests to keep them isolated.
@ -0,0 +62,4 @@calls.append({"args": args, "kwargs": kwargs})original_uvicorn = server.uvicornserver.uvicorn = cast(Any, SimpleNamespace(run=recording_run))tests [HIGH]
Module-level global
server.uvicornis mutated byinstall_run_recorderand restored in afinallyblock only inside the two tests. If either test fails before thefinallyblock or if tests run in an order where cleanup is missed, the global remains poisoned for subsequent tests in the same session. Use a fixture with teardown (yield-based cleanup) to guarantee restoration regardless of test outcome.WuMing review wave — resolutions
src/xique/config.py— code [MEDIUM]✅ Fixed in
185206c— a_valid_hostvalidator now rejects empty/whitespace-only values (RED→GREEN:web host must be a non-empty address).tests/test_web_server.py— code [LOW]✅ Fixed in
a00cb9c— therecorded_uvicornfixture now stubsconfigure_loggingalongsideuvicorn.run, so process-wide logging is never touched by these tests.tests/test_web_server.py— tests [HIGH]✅ Adopted in
a00cb9c— both module bindings are now owned by a yield-based fixture whose teardown restores them and clears the settings cache regardless of test outcome. For the record the original code was already safe (thefinallyruns even when soft assertions fail afterwards), but the fixture is the better shape.README.md— docs [MEDIUM]🔴 Already formatted —
deploy/appears backticked at first mention: "thedeploy/directory holds the rootless-podman quadlets".README.md— docs [MEDIUM]🔴 Already formatted — both
xique-serve.jsonand${TS_CERT_DOMAIN}are wrapped in backticks in the prerequisites list.README.md— docs [MEDIUM]🔴 Already formatted —
~/docker/tailscale-systemd.envandTAILSCALE_VERSIONare both backticked.README.md— docs [LOW]✅ Fixed in
13e1f2d— now a proper### Bring-upheading.README.md— docs [LOW]🔴 Already formatted —
:latestsits inside a code span ("Upgrades pin:latest").Containerfile— shell [HIGH]🔴 Incorrect — Python 3.14 was released in October 2025; this repository requires
python >=3.14and its CI runs 3.14 nox sessions. Empirically, the failing CI build pulledpython:3.14-slimand reached STEP 11/20 — the tag exists and works. The real failure was a missing LICENSE file in the build context (fixed in185206c).Containerfile— shell [MEDIUM]🔴 Declined by design — CI derives
UV_VERSIONfrom[build-system] requiresand exits 1 when it cannot parse it, so an empty value cannot reach the build silently. A baked-in default would mask a contract break between pyproject.toml and the image instead of failing loudly.Containerfile— shell [LOW]✅ Simplified in
185206c— dropped the redundant--chown=root:root(plain COPY already yields root:root; the flag was a no-op, though not for the stated reason —--chownoverrides the active USER regardless of instruction order). The exec-bitchmodremains.tests/test_config.py— tests [LOW]🔴 Declined — default-pinning tests exist to pin the documented default verbatim. README documents
127.0.0.1as theXIQUE_HOSTdefault; the literal is the contract, mirroring the existingtest_settings_uses_defaultspattern.tests/test_config.py— tests [LOW]🔴 Declined — same rationale:
8000is the documented default in README and must stay pinned.tests/test_config.py— tests [MEDIUM]🔴 Declined — same rationale:
/data/db/xique.dbis the documentedXIQUE_DATABASE_PATHdefault; the test pins the documentation.WuMing
Found 8 issue(s). See inline comments below.
@ -0,0 +1,42 @@FROM python:3.14-slimshell [LOW]
Python 3.14-slim may not be fully stable or widely published yet. Ensure this tag is validated in your registry before merging to avoid CI pipeline breakage during image pulls. Consider pinning to a known digest or waiting for a stabilized tag.
@ -0,0 +6,4 @@ENV PYTHONUNBUFFERED=1 \PIP_DISABLE_PIP_VERSION_CHECK=1RUN pip install --no-cache-dir "uv==${UV_VERSION}"shell [LOW]
Installing
uvviapipintroduces unnecessary overhead and dependency resolution steps compared to native installation methods. Useuv's official installer script or download the static binary directly for faster, more reliable setups.@ -115,0 +131,4 @@`deploy/`; adjust the `${TS_CERT_DOMAIN}` proxy target if you change ports).- `~/docker/xique/xique.env` with at least `XIQUE_SECRET_KEY` (required),optionally `XIQUE_TMDB_TOKEN`, and `XIQUE_IMPORT_DIR=/data/imports-temp`so uploads stream into the shared volume web and worker both mount.docs [LOW]
Sentence structure is slightly awkward. Consider adding 'that' or restructuring for better readability.
@ -115,0 +132,4 @@- `~/docker/xique/xique.env` with at least `XIQUE_SECRET_KEY` (required),optionally `XIQUE_TMDB_TOKEN`, and `XIQUE_IMPORT_DIR=/data/imports-temp`so uploads stream into the shared volume web and worker both mount.- The shared `~/docker/tailscale-systemd.env` pinning `TAILSCALE_VERSION`docs [LOW]
Configuration file path is missing backticks for consistent code formatting.
@ -115,0 +148,4 @@tailnet. On a first-ever boot the worker migrates the database while the webprocess restarts until the schema is ready — this settles in seconds.Upgrades pin `:latest`: `podman pull registry.marvin8.zone/marvin8/xique:latestdocs [LOW]
Inline shell command and the
:latest:tag blend with prose. Wrap the command and tag in backticks for clarity.@ -115,0 +150,4 @@Upgrades pin `:latest`: `podman pull registry.marvin8.zone/marvin8/xique:latest&& systemctl --user restart xique-pod`. Release tags are available forrollbacks. Every container carries an explicit memory cap in its unitdocs [LOW]
Pronoun agreement mismatch: 'Every container' is singular but implies multiple instances. Consider 'their' or rephrasing.
@ -0,0 +3,4 @@# Container entrypoint: maps PORT onto the app's XIQUE_PORT and execs the# requested console script. The web default host is 0.0.0.0 because the# tailscale sidecar proxies to the pod's shared loopback.set -eushell [LOW]
set -euenables strict mode but omitsset -o pipefail. While no pipes are currently used, addingset -euo pipefailis a standard hardening practice that ensures sub-command failures in pipelines are correctly propagated as script failures.@ -0,0 +5,4 @@}},"Web": {"${TS_CERT_DOMAIN}:443": {config [MEDIUM]
Contains shell variable placeholder ${TS_CERT_DOMAIN}. Tailscale's serve configuration does not natively expand shell variables, so this literal string will be treated as the hostname unless processed by an external templating or script step before application. Consider using a proper template engine or documenting the mandatory substitution step.
WuMing
Found 6 issue(s). See inline comments below.
@ -119,0 +121,4 @@# assertion below is web rendering the login page. The throwaway# key lives only in this step; production supplies the real one.buildah run --isolation chroot "$ctr" \env XIQUE_SECRET_KEY=ci-smoke-only \security [LOW]
A02: Hard-coded test secret XIQUE_SECRET_KEY in CI. Use CI secret management instead of embedding credentials.
@ -141,3 +149,3 @@# server.setsid buildah run --isolation chroot "$ctr" \env PORT=18099 /usr/local/bin/container-entrypoint.sh web >/tmp/xique-smoke.log 2>&1 &env PORT=18099 XIQUE_SECRET_KEY=ci-smoke-only /usr/local/bin/container-entrypoint.sh web >/tmp/xique-smoke.log 2>&1 &security [LOW]
A02: Hard-coded test secret XIQUE_SECRET_KEY in CI. Use CI secret management instead of embedding credentials.
@ -0,0 +31,4 @@# Pre-create the mount-point directories: /data/db exists for bare chroot# smoke runs, and a freshly mounted named volume inherits this ownership.# No USER directive: under rootless podman, container root maps to the hostsecurity [MEDIUM]
A05: Container runs as root (no USER directive). Add a non-root USER to reduce privilege if runtime is not rootless.
@ -115,0 +137,4 @@### Bring-up1. `cp deploy/* ~/.config/containers/systemd/`docs [LOW]
Copying deploy/* to the systemd directory may include non-quadlet files (e.g., scripts or ignore files), which can clutter the directory or trigger warnings on daemon-reload. Consider explicitly naming only the required unit files.
@ -115,0 +140,4 @@1. `cp deploy/* ~/.config/containers/systemd/`2. `systemctl --user daemon-reload`3. `systemctl --user start xique-pod`4. First boot only: approve the Tailscale login URL printed by the sidecar —docs [MEDIUM]
Step 4 conflates reading logs with approving a URL. Clarify that users should extract the login URL from the logs and open it in a browser to complete authentication.
@ -115,0 +145,4 @@The app is then reachable at `https://xique.<your-tailnet>.ts.net`; TLS comesfrom Tailscale and funnel stays disabled, so nothing is exposed beyond thetailnet. On a first-ever boot the worker migrates the database while the webdocs [MEDIUM]
This conflicts with the PR summary which states a serial boot order ('worker migrates, then web serves'). Either update the summary to reflect the concurrent/retry behavior, or clarify that the web process handles connection errors gracefully during migration rather than actively restarting.
WuMing review wave 2 — resolutions
deploy/xique-serve.json— config [MEDIUM]🔴 Declined — tailscale's container boot performs
${TS_CERT_DOMAIN}substitution when readingTS_SERVE_CONFIG; this exact config shape runs in production today on the operator's other stack, and the design deliberately preserved the proven shape.README.md— docs [LOW]🔴 Already formatted — every path in that list (
~/docker/xique/xique.env,~/docker/tailscale-systemd.env, etc.) is backticked.README.md— docs [LOW]🔴 Declined — pure style preference, no actionable ambiguity.
README.md— docs [LOW]🔴 Already formatted —
:latestand the wholepodman pull … && systemctl …command sit inside a (multi-line) code span.README.md— docs [LOW]🔴 Correct as written — "Every container … in its unit" is valid singular agreement.
Containerfile— shell [LOW]🔴 Empirically false — the
containerjob on this very head pulledpython:3.14-slim, built the image, booted the real entrypoint, and passed the login-page smoke. Python 3.14 has been stable since October 2025.Containerfile— shell [LOW]🔴 Deliberate —
pip install uv==${UV_VERSION}pins exactly the version CI derives from[build-system] requires, keeping the image and the lock check honest; the official installer script conflicts with this environment's no-curl | shpolicy.container-entrypoint.sh— shell [LOW]🔴 Would break at boot — the shebang is
#!/bin/sh(dash), which rejectsset -o pipefail(set: Illegal option -o); the script contains no pipelines, so strict mode is already sufficient..forgejo/workflows/ci.yml— security [LOW]🔴 Not a credential — the value only ever signs sessions for an ephemeral smoke server inside a throwaway chroot and grants nothing; the design documents it as CI-only, and the gitleaks job passes. Secret management would imply it means something.
.forgejo/workflows/ci.yml— security [LOW]🔴 Same as the sibling comment — throwaway smoke value, not a secret.
Containerfile— security [MEDIUM]🔴 Intentional — the deployment target is rootless podman: container root maps to the unprivileged host user, which is exactly why the
USERdirective was removed (see the comment block in the Containerfile). The reviewer's own hedge — "if runtime is not rootless" — does not apply.README.md— docs [MEDIUM]✅ Fixed in
4c77296— step 5 now says to extract the URL withjournalctland open it in a browser to approve the node.README.md— docs [LOW]✅ Fixed in
4c77296— the walkthrough now names the five unit files explicitly and copiesxique-serve.jsonto~/docker/xique/ts-serve/, where the sidecar actually mounts it (a plaincp deploy/*would indeed have put it in the wrong place).README.md— docs [MEDIUM]✅ Clarified in
4c77296— the README now states the pod starts all containers simultaneously and that web restarts until the worker migrates; the serial worker-then-web order is called out as the CI smoke's proof of the same first-boot, resolving the apparent conflict with the PR summary.WuMing
Found 7 issue(s). See inline comments below.
@ -0,0 +1,42 @@FROM python:3.14-slimshell [CRITICAL]
Python 3.14 does not exist in official distribution repositories. The Docker build will fail immediately. Use a released stable version (e.g., python:3.12-slim or python:3.13-slim).
@ -0,0 +20,4 @@# the pyproject version in place (CI smoke compares against pyproject).# --no-sync keeps uv from relocking; if `uv sync --frozen` then rejects the# version mismatch, drop --no-sync here so `uv version` relocks instead.RUN if [ -n "${VERSION}" ]; then uv version --no-sync "${VERSION}"; fi \shell [HIGH]
The
uvCLI does not have aversionsubcommand that accepts--no-syncor dynamically modifies installed package metadata. This will cause a non-zero exit during the build. To bake release metadata, updatepyproject.tomlvia a templating tool or useuv syncdirectly after adjusting the version field.@ -99,3 +99,3 @@| Variable | Default | Purpose ||---|---|---|| `XIQUE_DATABASE_PATH` | `/data/xique.db` | sqlite database file || `XIQUE_DATABASE_PATH` | `/data/db/xique.db` | sqlite database file |docs [MEDIUM]
Inconsistent indentation introduces a leading space before the first pipe, which may break markdown table parsing or alignment. Remove the leading space to match the surrounding table rows.
@ -102,2 +101,4 @@| `XIQUE_DATABASE_PATH` | `/data/db/xique.db` | sqlite database file || `XIQUE_SECRET_KEY` | *required* | signs sessions and CSRF tokens || `XIQUE_TMDB_TOKEN` | unset | TMDB read access token || `XIQUE_HOST` | `127.0.0.1` | web listen address |docs [MEDIUM]
Inconsistent indentation introduces a leading space before the first pipe. Remove the leading space to maintain table alignment consistency.
@ -102,2 +102,4 @@| `XIQUE_SECRET_KEY` | *required* | signs sessions and CSRF tokens || `XIQUE_TMDB_TOKEN` | unset | TMDB read access token || `XIQUE_HOST` | `127.0.0.1` | web listen address || `XIQUE_PORT` | `8000` | web listen port |docs [MEDIUM]
Inconsistent indentation introduces a leading space before the first pipe. Remove the leading space to maintain table alignment consistency.
@ -115,0 +131,4 @@`deploy/`; adjust the `${TS_CERT_DOMAIN}` proxy target if you change ports).- `~/docker/xique/xique.env` with at least `XIQUE_SECRET_KEY` (required),optionally `XIQUE_TMDB_TOKEN`, and `XIQUE_IMPORT_DIR=/data/imports-temp`so uploads stream into the shared volume web and worker both mount.docs [LOW]
Grammatically awkward phrasing in the prerequisites list. Insert a preposition/conjunction for clarity: change 'shared volume web and worker both mount' to 'shared volume that both the web and worker mount' or 'shared volume mounted by both the web and worker'.
@ -0,0 +93,4 @@environment["XIQUE_PORT"] = "18099"get_settings.cache_clear()server.main()code [MEDIUM]
The test calls server.main() which invokes create_app(settings) with default database path /data/db/xique.db. This can cause filesystem side effects or fail if the path is not writable in the test environment (e.g., CI). To improve test isolation and reliability, mock create_app or set XIQUE_DATABASE_PATH to a temporary directory.
WuMing review wave 3 — resolutions
tests/test_web_server.py— code [MEDIUM]🔴 Declined — no filesystem side effects are possible:
uvicorn.runis stubbed, so the ASGI lifespan (the only code that opens the database) never runs;create_appjust builds routes, andSettingsmerely holds aPathobject. The suite is green locally and in the CIchecksjob.README.md— docs [MEDIUM]🔴 Not present — no table row in README begins with whitespace; verified by
grep -n '^ \+|' README.md, which returns nothing.README.md— docs [MEDIUM]🔴 Not present — same verification as the sibling comment.
README.md— docs [MEDIUM]🔴 Not present — same verification as the sibling comment.
README.md— docs [LOW]✅ Fixed — reworded to "the shared volume that both web and worker mount".
Containerfile— shell [CRITICAL]🔴 Empirically false (third raising of this claim) — the
containerjob on this PR pulledpython:3.14-slim, built the image, booted the real entrypoint, and passed the login-page smoke. Python 3.14 has been stable since October 2025 and this repository requires>=3.14.Containerfile— shell [HIGH]🔴 Empirically false — verified against the exact pinned version:
uv==0.12.0 --helpfor the version subcommand lists--no-sync Avoid syncing the virtual environment after re-locking. The release-metadata path will not exit non-zero on that account.WuMing
Found 2 issue(s). See inline comments below.
@ -115,0 +127,4 @@`~/.config/containers/systemd/`.- Host directories: `~/docker/xique/data/db` (the sqlite database — the onlybacked-up content), `~/docker/xique/ts-state` (Tailscale node state), and`~/docker/xique/ts-serve` containing `xique-serve.json` (copied fromdocs [HIGH]
The
xique-serve.jsonfile is a JSON configuration, which does not natively support shell variable expansion (e.g.,${TS_CERT_DOMAIN}). Copying it directly as instructed will result in an invalid or unresolved config. Please clarify whether the file requires manual key replacement before mounting, or if it passes through a preprocessing step.@ -115,0 +155,4 @@in seconds. (The CI smoke proves the same first-boot in serial order: workermigrates first, then web serves.)Upgrades pin `:latest`: `podman pull registry.marvin8.zone/marvin8/xique:latestdocs [MEDIUM]
"pin
:latest" is functionally contradictory, as:latestis a mutable tag and cannot be pinned. Consider rewriting to "Upgrades pull:latest:" or "Images use the:latesttag; pull it to update:".WuMing review wave 4 — resolutions
README.md— docs [HIGH]🔴 Declined for the third time — tailscale's container boot performs
${TS_CERT_DOMAIN}substitution when readingTS_SERVE_CONFIG; this exact config shape runs in production today on the operator's other stack, and the design preserved it deliberately. No preprocessing step exists or is needed.README.md— docs [MEDIUM]✅ Fair — reworded in HEAD: "Upgrades use the
:latesttag" (a mutable tag is tracked, not pinned).WuMing
Found 4 issue(s). See inline comments below.
@ -0,0 +6,4 @@ENV PYTHONUNBUFFERED=1 \PIP_DISABLE_PIP_VERSION_CHECK=1RUN pip install --no-cache-dir "uv==${UV_VERSION}"shell [LOW]
Installing uv via pip bypasses the official cross-platform static binary installer. This can lead to inconsistencies or conflicts when uv's native resolver is used later in the build step. Prefer downloading the official installer script or static binary directly from Astral's distribution channel for guaranteed reproducibility.
@ -115,0 +155,4 @@in seconds. (The CI smoke proves the same first-boot in serial order: workermigrates first, then web serves.)Upgrades use the `:latest` tag: `podman pull registry.marvin8.zone/marvin8/xique:latestdocs [MEDIUM]
Inline code spans two lines, which can break Markdown rendering or disrupt the paragraph flow. Consolidate onto a single line or move to a fenced code block.
@ -0,0 +8,4 @@web)export XIQUE_HOST="${XIQUE_HOST:-0.0.0.0}"export XIQUE_PORT="${PORT:-8000}"exec /app/.venv/bin/xique-webshell [MEDIUM]
Hardcodes the virtual environment path (/app/.venv/bin/), tightly coupling the entrypoint to uv's internal layout. If UV_PROJECT_ENVIRONMENT or the installation strategy changes, this command will fail. Since PATH is already configured to include the venv bin directory (Containerfile line 27), using "exec xique-web" improves portability and simplifies maintenance.
@ -0,0 +11,4 @@exec /app/.venv/bin/xique-web;;worker)exec /app/.venv/bin/xique-workershell [MEDIUM]
Same hardcoded path issue as line 11. Relies on a fixed virtual environment layout that may break if uv configuration changes. Prefer relying on the prepended PATH with "exec xique-worker" for better resilience.
WuMing review wave 5 — resolutions
README.md— docs [MEDIUM]✅ Fixed in HEAD — the upgrade commands now live in a fenced
shblock on separate lines.Containerfile— shell [LOW]🔴 Declined (repeat of the earlier installer comment) —
pip install uv==${UV_VERSION}pins exactly the version CI derives from[build-system] requires; the official installer script conflicts with this environment's no-curl | shpolicy, and uv 0.12.0 resolved the lock fine in the green CI build.container-entrypoint.sh— shell [MEDIUM]🔴 Declined — the venv location is not uv's incidental layout: the image pins
UV_PROJECT_ENVIRONMENT=/app/.venvitself, so the absolute path is an internal contract, explicit and smoke-proven. Switching working, CI-verified config to PATH resolution for a hypothetical relocation is churn without benefit.container-entrypoint.sh— shell [MEDIUM]🔴 Declined — same rationale as the sibling comment.
WuMing
Found 4 issue(s). See inline comments below.
@ -115,0 +128,4 @@- Host directories: `~/docker/xique/data/db` (the sqlite database — the onlybacked-up content), `~/docker/xique/ts-state` (Tailscale node state), and`~/docker/xique/ts-serve` containing `xique-serve.json` (copied from`deploy/`; adjust the `${TS_CERT_DOMAIN}` proxy target if you change ports).docs [MEDIUM]
Escaping the dollar sign with a backslash in prose is unnecessary and may render visibly as '$'. Use backticks to denote the variable name clearly:
`${TS_CERT_DOMAIN}`.@ -115,0 +138,4 @@### Bring-up1. Copy the five quadlet units into the systemd directory:`cp deploy/xique.pod deploy/xique-web.container deploy/xique-worker.container deploy/xique-tailscale.container deploy/xique-imports.volume ~/.config/containers/systemd/`docs [LOW]
The shell command exceeds ~180 characters and is difficult to scan in a README. Consider breaking it across multiple lines with trailing backslashes for improved readability.
@ -115,0 +150,4 @@The app is then reachable at `https://xique.<your-tailnet>.ts.net`; TLS comesfrom Tailscale and funnel stays disabled, so nothing is exposed beyond thetailnet. All containers in the pod start simultaneously; on a first-ever bootthe web process finds no schema, refuses to serve, and keeps restarting everydocs [LOW]
Repeats the word 'seconds' twice within the same sentence ('every couple of seconds ... settles in seconds'). Consider rephrasing to improve flow, e.g., 'keeps restarting repeatedly... this resolves quickly.'
@ -115,0 +169,4 @@Backups: the database is a plain file at`~/docker/xique/data/db/xique.db`; uploaded Trakt archives live on a separatenamed volume and are transient. Full backup/restore guidance (including WALcheckpoint behaviour) is coming in the operator documentation (#16).docs [MEDIUM]
Avoid future tense and forward references for documentation. Instead of stating guidance 'is coming', either link to the completed document when ready or simply state 'Backup/restore guidance (including WAL checkpoint behaviour) is documented in the operator guide (#16).'
WuMing review wave 6 — resolutions
README.md— docs [MEDIUM]🔴 Not present — no backslash-escaped dollar sign exists anywhere in README (
grep '\\\$'returns nothing);${TS_CERT_DOMAIN}appears only inside backticks.README.md— docs [LOW]✅ Fixed in HEAD — the copy command is now a fenced, line-continued block.
README.md— docs [LOW]✅ Fixed in HEAD — the sentence no longer repeats "seconds".
README.md— docs [MEDIUM]🔴 Declined — the suggested present tense would be untrue today: the operator guide does not exist yet (#16 is open). "Is coming" is the honest state; it will be updated to a link when #16 lands.
WuMing
Found 1 issue(s). See inline comments below.
@ -0,0 +14,4 @@from tryke import fixturefrom tryke import testfrom xique.config import Settingscode [LOW]
Unused import of
Settings. It is not referenced in this test module. Remove to keep imports lean.WuMing review wave 7 — resolutions
tests/test_web_server.py— code [LOW]🔴 Not unused —
Settingsannotates thestub_configure_logging(settings: Settings)parameter at tests/test_web_server.py:65.WuMing
Found 2 issue(s). See inline comments below.
@ -0,0 +5,4 @@ARG VERSIONENV PYTHONUNBUFFERED=1 \PIP_DISABLE_PIP_VERSION_CHECK=1shell [LOW]
PIP_DISABLE_PIP_VERSION_CHECK=1 is declared in ENV but is entirely unused since the Dockerfile switches to uv for all dependency operations immediately after. Removing it avoids polluting the final image's environment variables.
@ -0,0 +105,4 @@environment: Annotated[MutableMapping[str, str], Depends(environment)],calls: Annotated[list[dict[str, Any]], Depends(recorded_uvicorn)],) -> None:environment["XIQUE_SECRET_KEY"] = "test-key"code [MEDIUM]
The test
test_server_defaults_to_loopbackuses the default database path/data/db/xique.dbwithout mocking or overriding it. Ifcreate_app(called insideserver.main()) initializes a database connection or opens the file, the test may fail on systems without that directory or write unwanted files. Consider patching the setting or using a temporary directory to isolate the test from the file system.WuMing review wave 8 — resolutions
Containerfile— shell [LOW]🔴 Declined — the variable is not unused: it is consumed by the very next instruction,
pip install --no-cache-dir "uv==${UV_VERSION}", which is the only pip invocation in the build. It does persist into the runtime env, where it is inert (nothing runs pip at runtime).tests/test_web_server.py— code [MEDIUM]🔴 Declined again (same finding as two waves ago) — no filesystem access is possible:
uvicorn.runis stubbed, so the ASGI lifespan (the only code that opens the database) never executes;create_apponly builds the route table andSettingsmerely holds aPathobject. The suite is green locally and in the CIchecksjob.