Skip to content

fix: build open_terminal from a real fork, remove the dead vendored copy - #67

Merged
dvystrcil merged 1 commit into
mainfrom
build-from-fork-and-cleanup
Aug 1, 2026
Merged

fix: build open_terminal from a real fork, remove the dead vendored copy#67
dvystrcil merged 1 commit into
mainfrom
build-from-fork-and-cleanup

Conversation

@dvystrcil

@dvystrcil dvystrcil commented Aug 1, 2026

Copy link
Copy Markdown
Owner

User description

The core problem (homelab#822)

This repo's Dockerfile has always pulled a pre-built upstream image and only added wrapper tooling on top. It never built or installed this repo's own vendored open_terminal/ package -- every fix this CHANGELOG has described as applied to open_terminal/main.py was real, tested, merged code that the running container never actually ran.

Root-caused while investigating why homelab#720's security fix (process-log retention) wasn't live despite being merged days ago. Auditing the vendored package's full history surfaced three more real, undeployed fixes beyond that one -- all now submitted upstream:

PR What
open-webui/open-terminal#148 Configurable uvicorn keep-alive timeout (intermittent ConnectionResetError)
open-webui/open-terminal#149 Process-log retention security fix
open-webui/open-terminal#150 Two-tier process-result expiry (a slow caller could lose a finished command's result forever)
open-webui/open-terminal#151 insert_after/append_to_section/append endpoints + a defensive replace_file_content check

Plus one homelab-specific fix, ported to the fork but not submitted upstream (it's specific to our own GH App token-file convention): refresh_github_token_env() re-reads the current token from disk before every subprocess spawn, closing a gap BASH_ENV-based shell-profile sourcing doesn't cover.

What changed

  • Dockerfile: stage 1 now builds open_terminal from dvystrcil/open-terminal-app-fork (carries all 5 fixes above) via git clone + pip install ., mirroring upstream's own Dockerfile exactly, instead of pulling the pre-built upstream image. Temporary -- revert to a plain upstream FROM once all four PRs merge and a release picks them up.
  • docker.yml: resolves the fork's current commit SHA via git ls-remote and passes it as a build-arg on every run. Without this, Docker's build cache (keyed on RUN command text, not on what git clone --branch main actually fetches) silently keeps shipping whatever fork commit was cloned the first time -- caught this empirically: a local build without the explicit build-arg produced a stale image missing later fixes despite a fresh fork push.
  • Removed the vendored open_terminal/ package and its tests -- dead weight now that the real fixes live in a fork with a real upstream relationship. Kept tests/test_actor_env.py (tests helpers/bible_bridge.py, which is deployed) and tests/__init__.py. Removed pyproject.toml, dev.sh, .python-version (all specific to developing the now-removed vendored package).
  • README: documents the new build shape and the incident.

Test plan

  • Built the actual image locally (multi-stage, full apt/pip install), ran it, verified all 5 fixes present in the running container via direct imports (sweep function, keep-alive/expiry env values, refresh_github_token_env, the three new endpoint handlers), plus /health and all wrapper tools (kubectl, gh, yq, argocd, act) and the entrypoint's token-refresh profile.
  • Confirmed the FORK_SHA cache-bust actually works: an explicit --build-arg forces a genuine re-clone (visible in build output, not a CACHED layer) rather than silently reusing a stale one -- this is exactly the class of bug this whole PR exists to fix, so I wanted a real receipt that the fix works, not just that it looks right.
  • tests/test_actor_env.py still passes standalone (7/7) against the real helpers/bible_bridge.py.

Ref homelab#822 -- NOT closing it. This fixes the diagnosis and gets the real fixes deployed via the fork, but the issue's underlying "vendored source disconnected from the build" risk only fully resolves once all four upstream PRs merge and this Dockerfile reverts to a plain upstream FROM.


PR Type

Bug fix, CI/CD Update, Documentation, Build Process Refactoring


Description

This PR addresses a critical deployment gap where previously documented fixes for open_terminal were never actually built into the container image. The Dockerfile is refactored to use a multi-stage build that clones and installs from a temporary fork (dvystrcil/open-terminal-app-fork) carrying the necessary upstream PRs. The CI workflow is updated to resolve the fork's current HEAD SHA and pass it as a build argument to prevent Docker layer caching issues with git clone --branch main. Documentation and changelog are updated to reflect the removal of the dead vendored package and the temporary nature of this change.


Diagram Walkthrough

flowchart TD
  A[CI Workflow] -->|Resolves fork HEAD SHA| B(Build Args)
  C[Dockerfile Stage 1] -->|git clone --branch main| D[/build]
  D -->|pip install .| E[fork-build Image]
  F[Dockerfile Stage 2] -->|FROM fork-build| G[Final Image]
  B -->|FORK_SHA busts cache| C
Loading

File Walkthrough

Relevant files
Enhancement
1 files
Dockerfile
Refactor to multi-stage build from temporary fork; add devops tools &
Python deps; set up user & metadata
+96/-3   
Ci-cd
1 files
docker.yml
Add step to resolve fork HEAD SHA and pass as build-arg to bust Docker
cache
+17/-0   
Documentation
2 files
CHANGELOG.md
Document undeployed fixes, new fork-based build process, and removal
of vendored package
+10/-0   
README.md
Update repository layout and clarify that the repo no longer vendors
open_terminal source
+6/-4     
Additional files
25 files
.python-version +0/-1     
dev.sh +0/-2     
__init__.py +0/-1     
__main__.py +0/-3     
cli.py +0/-197 
config.py +0/-91   
env.py +0/-200 
main.py +0/-2078
mcp_server.py +0/-7     
__init__.py [link]   
documents.py +0/-240 
fs.py +0/-271 
github_token.py +0/-41   
log.py +0/-265 
notebooks.py +0/-283 
port.py +0/-201 
runner.py +0/-296 
user_isolation.py +0/-154 
pyproject.toml +0/-44   
conftest.py +0/-9     
test_github_token_refresh.py +0/-73   
test_insert_append.py +0/-367 
test_log_retention_sweep.py +0/-81   
test_process_expiry.py +0/-78   
test_replace_defensive.py +0/-189 

This repo's Dockerfile has always pulled a pre-built upstream image
(FROM .../ghcr-proxy/open-webui/open-terminal:latest) and only added
wrapper tooling on top. It never built or installed this repo's own
vendored open_terminal/ package -- every fix this CHANGELOG has
described as applied to open_terminal/main.py was real, tested,
merged code that the running container never actually ran. See
homelab#822 for the full incident writeup.

Root cause found while investigating why homelab#720's security fix
(process-log retention) wasn't live despite being merged days ago.
Auditing the vendored package's full history surfaced three more real,
undeployed fixes beyond that one, all now submitted upstream:

  - open-webui/open-terminal#148 -- configurable uvicorn keep-alive
    timeout (intermittent ConnectionResetError)
  - open-webui/open-terminal#149 -- process-log retention security fix
  - open-webui/open-terminal#150 -- two-tier process-result expiry
    (a slow caller could lose a finished command's result forever)
  - open-webui/open-terminal#151 -- insert_after/append_to_section/
    append endpoints + a defensive replace_file_content check

Plus one homelab-specific fix NOT appropriate for upstream (ties into
our own GH App token-file convention, not something upstream has any
hook for): refresh_github_token_env(), re-reads the current token
from disk before every subprocess spawn, closing a gap BASH_ENV-based
shell-profile sourcing doesn't cover (plain-shell and PTY spawn paths
never source /etc/profile.d).

## What changed

- Dockerfile: stage 1 now builds open_terminal from
  dvystrcil/open-terminal-app-fork (a real fork carrying all 5 fixes
  above) via git clone + pip install ., mirroring upstream's own
  Dockerfile exactly, instead of pulling the pre-built upstream image.
  TEMPORARY -- revert to a plain upstream FROM once all four PRs merge
  and a release picks them up.
- docker.yml: resolves the fork's current commit SHA via `git
  ls-remote` and passes it as a build-arg on every run. Without this,
  Docker's build cache (keyed on RUN command text, not on what `git
  clone --branch main` actually fetches) would silently keep shipping
  whatever fork commit was cloned the FIRST time this layer built,
  even after new fixes land on the fork -- caught this empirically:
  an initial local build without the explicit build-arg produced a
  stale image missing later fixes despite a fresh fork push.
- Removed the vendored open_terminal/ package and its tests -- dead
  weight now that the real fixes live in a fork with a real upstream
  relationship. Kept tests/test_actor_env.py (tests
  helpers/bible_bridge.py, which *is* deployed) and tests/__init__.py.
  Removed pyproject.toml, dev.sh, .python-version (all specific to
  developing the now-removed vendored package).
- README: documents the new build shape and the incident.

## Testing

Built the actual image locally (multi-stage, full apt/pip install,
~2 min), ran it, and verified all 5 fixes are present in the running
container (direct imports of the sweep function, keep-alive/expiry
env values, refresh_github_token_env, and the three new endpoint
handlers) plus the health endpoint and all wrapper tools (kubectl,
gh, yq, argocd, act) and the entrypoint's token-refresh profile.
Confirmed the FORK_SHA cache-bust actually works: an explicit
--build-arg forces a genuine re-clone (visible in build output, not
a CACHED layer) rather than silently reusing a stale one.

tests/test_actor_env.py still passes standalone (7/7) against the
real helpers/bible_bridge.py, unaffected by the removed package.
@dvystrcil
dvystrcil merged commit 194979b into main Aug 1, 2026
4 checks passed
@dvystrcil
dvystrcil deleted the build-from-fork-and-cleanup branch August 1, 2026 23:54
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Hardcoded fork URL and potential build fragility

The Dockerfile hardcodes the fork URL (https://github.com/dvystrcil/open-terminal-app-fork.git). If this repository becomes private, is deleted, or hits GitHub API rate limits without authentication, the build will fail. Consider using a secret for the URL or SSH key if it's not public.

&& git clone --branch "${FORK_REF}" --depth 1 \
    https://github.com/dvystrcil/open-terminal-app-fork.git /build \
&& cd /build \
&& pip install --no-cache-dir . \
&& cp "$(readlink -f "$(which python3)")" /usr/local/bin/python3-ot \
&& setcap cap_setgid+ep /usr/local/bin/python3-ot \
&& sed -i "1s|.*|#!/usr/local/bin/python3-ot|" "$(which open-terminal)" \
&& rm -rf /build
Network dependency for cache busting step

The 'Resolve open-terminal-app-fork HEAD sha' step runs git ls-remote on every build. If the runner lacks internet access or GitHub rate limits are hit, this will block the pipeline. Ensure the CI environment has reliable external network access or implement a fallback mechanism.

- name: Resolve open-terminal-app-fork HEAD sha
  id: fork
  run: |
    sha=$(git ls-remote https://github.com/dvystrcil/open-terminal-app-fork.git refs/heads/main | cut -f1)
    echo "sha=$sha" >> "$GITHUB_OUTPUT"
    echo "Building against fork main @ $sha"
Large runtime dependency installation in stage 1

Stage 1 installs a massive list of apt packages and Python libraries. While consolidated for layer caching, this significantly increases the base image size and build time. Verify that all listed packages are strictly required at runtime for the open_terminal application, as dev-only tools should be excluded from production images.

RUN apt-get update && apt-get install -y --no-install-recommends \
        coreutils findutils grep sed gawk diffutils patch \
        less file tree bc man-db \
        curl wget net-tools iputils-ping dnsutils netcat-openbsd socat telnet \
        openssh-client rsync \
        vim nano \
        git \
        build-essential cmake make \
        perl ruby-full lua5.4 \
        jq xmlstarlet sqlite3 \
        ffmpeg pandoc imagemagick texlive-latex-base \
        zip unzip tar gzip bzip2 xz-utils zstd p7zip-full \
        procps htop lsof strace sysstat \
        sudo tmux screen tini iptables ipset dnsmasq \
        ca-certificates gnupg apt-transport-https \
        libcap2-bin \
    && rm -rf /var/lib/apt/lists/*

RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
    && apt-get install -y --no-install-recommends nodejs \
    && rm -rf /var/lib/apt/lists/*

RUN curl -fsSL https://get.docker.com | sh

WORKDIR /app

RUN pip install --no-cache-dir \
    numpy pandas scipy scikit-learn \
    matplotlib seaborn plotly \
    jupyter ipython \
    requests beautifulsoup4 lxml \
    sqlalchemy psycopg2-binary \
    pyyaml toml jsonlines \
    tqdm rich \
    openpyxl weasyprint \
    python-docx python-pptx pypdf csvkit

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant