fix: bind Ollama to 0.0.0.0 so api-proxy Docker container can reach it#40888
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #40888 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold 100). The 24 changed files are shell/python scripts under .github/skills/, which are outside the default business logic directories. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. PR #40888 only modifies .lock.yml workflow compilation artifacts and the daily-byok-ollama-test.md workflow source file. Test Quality Sentinel skipped. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Pull request overview
This pull request aims to fix the daily BYOK Ollama connectivity test by ensuring the Ollama server listens on an address reachable from the api-proxy sidecar container (instead of defaulting to 127.0.0.1:11434).
Changes:
- Configure the BYOK Ollama test workflow to start Ollama with
OLLAMA_HOST=0.0.0.0:11434sohost.docker.internal:11434can reach it. - Recompile the
daily-byok-ollama-testworkflow lockfile to reflect the workflow change. - Includes regeneration of many other workflow
.lock.ymlfiles (appears unrelated to the Ollama change).
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/daily-byok-ollama-test.md | Sets OLLAMA_HOST in the “Start Ollama service” step so Ollama binds to all interfaces. |
| .github/workflows/daily-byok-ollama-test.lock.yml | Recompiled lockfile reflecting the new env var on the Ollama start step. |
| .github/workflows/weekly-issue-summary.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/stale-repo-identifier.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/safe-output-health.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/prompt-clustering-analysis.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/portfolio-analyst.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/org-health-report.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/issue-arborist.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/github-mcp-structural-analysis.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/deep-report.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/daily-safe-output-optimizer.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/daily-repo-chronicle.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/daily-news.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/daily-issues-report.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/daily-code-metrics.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/copilot-opt.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/copilot-agent-analysis.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/cloclo.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/cli-version-checker.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/changeset.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/audit-workflows.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
| .github/workflows/api-consumption-report.lock.yml | Regenerated lockfile metadata (unrelated to Ollama change). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 24/24 changed files
- Comments generated: 1
| @@ -1,4 +1,4 @@ | |||
| # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"c5c8773029352bf15831dd840010e86368daaef97ef69e64a1712461b9f24684","body_hash":"0abfa3700ec7bd8d1430bed719110769a17b763365f890b84b2e260b8f7b93d0","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.63"}} | |||
| # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"c5c8773029352bf15831dd840010e86368daaef97ef69e64a1712461b9f24684","body_hash":"6004843c013d8242ed667f20ebe51b0638c65353bc1ff061286d9980c6c9292b","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.63"}} | |||
![github-actions[bot]](https://avatars-githubusercontent-com.shimmerl.top/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose — approving with one minor observation.
📋 Key Themes & Highlights
Root Cause & Fix
The diagnosis is clear and well-documented: Ollama defaults to 127.0.0.1:11434, which is unreachable from the api-proxy sidecar container. Setting OLLAMA_HOST: 0.0.0.0:11434 makes Ollama bind on all interfaces, resolving the 503/400 failures.
Positive Highlights
- ✅ Minimal, surgical change — only the affected step is touched
- ✅ PR description clearly states root cause, symptom (503/400), and fix
- ✅ Lock file correctly recompiled via
make recompile - ✅ The existing readiness loop (
seq 1 30withsleep 1) still works correctly —0.0.0.0binding includeslocalhost
Observations
- Readiness check (non-blocking): The loop validates
localhost:11434but not the Docker bridge path. Since binding to0.0.0.0implieslocalhostis also bound, this is functionally equivalent — but a bridge-IP probe could surface future topology regressions earlier (see inline comment). - 22 other lock files: The body_hash-only changes in 22 other lock files are expected output of
make recompile(shared runtime component drift correction) — no content changes to those workflows.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 61.7 AIC · ⌖ 7.78 AIC · ⊞ 6.5K
Comments that could not be inline-anchored
.github/workflows/daily-byok-ollama-test.md:36
[/diagnose] The readiness loop confirms Ollama is up on localhost, but never verifies the Docker-bridge path (host.docker.internal:11434) that the agent will actually use. If the 0.0.0.0 binding is correct, localhost will also be bound, so this is almost always fine — but a bridge-IP probe would provide a stronger signal and turn a silent runtime 503 into an explicit CI failure.
<details>
<summary>💡 Optional extra probe</summary>
After the existing loop, add a one-liner to verify…
There was a problem hiding this comment.
Non-blocking observations on this fix
The core change — adding OLLAMA_HOST: "0.0.0.0:11434" so Ollama binds beyond localhost — correctly addresses the stated failure (api-proxy sidecar container could not reach Ollama at host.docker.internal:11434). No correctness blockers.
Review themes
Networking scope (low): 0.0.0.0 exposes Ollama on all runner interfaces; 172.17.0.1 (Docker bridge) would be more targeted but less portable. See inline comment.
Mass body_hash rotation (medium): 22 unrelated strict:true lock files have their body_hash updated with no corresponding source change. This is likely a side effect of make recompile picking up a hash normalization change, but the PR description doesn't explain it. Reviewers should confirm this is intentional before merging. See inline comment on api-consumption-report.lock.yml.
🔎 Code quality review by PR Code Quality Reviewer · 122.8 AIC · ⌖ 7.6 AIC · ⊞ 5.1K
| echo "OLLAMA_API_KEY=$OLLAMA_API_KEY" >> "$GITHUB_ENV" | ||
| - name: Start Ollama service | ||
| env: | ||
| OLLAMA_HOST: "0.0.0.0:11434" |
There was a problem hiding this comment.
OLLAMA_HOST: 0.0.0.0 unnecessarily exposes Ollama on every network interface: only the Docker bridge interface needs to reach Ollama from the api-proxy container.
💡 More targeted alternative and tradeoff analysis
Binding to 0.0.0.0 means Ollama is reachable from every interface on the runner — not just the Docker bridge — for the full duration of the workflow run. While OLLAMA_API_KEY provides authentication protection, least-privilege networking would restrict this to the bridge IP:
env:
OLLAMA_HOST: "172.17.0.1:11434"Note that 172.17.0.1 is the default Docker bridge IP and may differ in non-default Docker network configurations. If runner portability matters more than strict interface isolation, 0.0.0.0 is a pragmatic trade-off — but that should be acknowledged explicitly.
| @@ -1,4 +1,4 @@ | |||
| # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"57a9cda8f18159d4874da1fc22fc9a636affde17b0fdd42f5ceafadbdbe25509","body_hash":"2b888cea7f6a7fb9885cf3f668b084be06816f71c9687802bb1d531125761f50","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.185"}} | |||
| # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"57a9cda8f18159d4874da1fc22fc9a636affde17b0fdd42f5ceafadbdbe25509","body_hash":"ca6b120bb96d5e1784db3642a07a7d7f43f36c232d910a014579305452cbdd51","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.185"}} | |||
There was a problem hiding this comment.
Mass body_hash rotation on 22 unrelated strict: true workflows without any body content change is unexplained: the PR description only mentions the Ollama fix, but every other lock file in the repo had its body_hash silently updated.
💡 Why this matters and what to verify
All 22 unrelated lock files changed from:
"body_hash":"2b888cea7f6a7fb9885cf3f668b084be06816f71c9687802bb1d531125761f50"
to:
"body_hash":"ca6b120bb96d5e1784db3642a07a7d7f43f36c232d910a014579305452cbdd51"
with no corresponding body content change visible in the diff. These files carry strict:true, which means the hash is used to enforce runtime integrity.
Possible causes:
- A hash normalization or algorithm change in the gh-aw compiler (e.g., newline handling, encoding)
- A change in a shared runtime partial (e.g.,
shared/noop-reminder.md) that is inlined at compile time but whose source file is not in this PR's diff - Stale hashes that were corrected by a global
make recompile
None of these are inherently wrong, but a mass hash rotation across 22 strict:true workflows should be explicitly documented in the PR description so reviewers can confirm the rotation is intentional and the new hashes are trustworthy.
| OLLAMA_API_KEY="$(openssl rand -hex 16)" | ||
| echo "OLLAMA_API_KEY=$OLLAMA_API_KEY" >> "$GITHUB_ENV" | ||
| - name: Start Ollama service | ||
| env: |
There was a problem hiding this comment.
OLLAMA_HOST is step-scoped but controls a job-lifetime server: the server binding address is a job-level network topology concern, not a single-step concern.
💡 Why this matters
It works today because ollama serve & inherits the step's env vars before backgrounding. But if this is ever refactored — serve call extracted to its own step, a restart handler added, or a wrapper script that forks after env teardown — the binding silently reverts to 127.0.0.1 with no error at startup, breaking the Docker container's connectivity non-obviously.
Moving OLLAMA_HOST to the job-level env block makes the intent explicit:
engine:
id: copilot
bare: true
env:
COPILOT_PROVIDER_BASE_URL: "(host.docker.internal/redacted)
COPILOT_PROVIDER_API_KEY: "${{ env.OLLAMA_API_KEY }}"
COPILOT_MODEL: "qwen2.5:0.5b"
OLLAMA_HOST: "0.0.0.0:11434"| echo "OLLAMA_API_KEY=$OLLAMA_API_KEY" >> "$GITHUB_ENV" | ||
| - name: Start Ollama service | ||
| env: | ||
| OLLAMA_HOST: "0.0.0.0:11434" |
There was a problem hiding this comment.
OLLAMA_API_KEY is not co-declared in the same step that binds Ollama to 0.0.0.0: the auth key comes from $GITHUB_ENV set in a prior step, creating an implicit ordering dependency.
💡 Risk and suggested fix
In GitHub Actions, $GITHUB_ENV exports are available in subsequent steps, so this works today. However, if the Generate Ollama API key step is reordered, skipped, or fails silently after writing a partial value, ollama serve starts on 0.0.0.0:11434 with an empty or missing API key — meaning an unauthenticated Ollama endpoint is exposed on all runner interfaces.
Before this PR, localhost-only binding made a missing key a minor issue; now that Ollama is on 0.0.0.0, the impact is higher.
Making the dependency explicit with a guard is safer:
- name: Start Ollama service
env:
OLLAMA_HOST: "0.0.0.0:11434"
run: |
: "${OLLAMA_API_KEY:?OLLAMA_API_KEY must be set before starting Ollama}"
ollama serve &
...This makes the step fail fast with a clear message if the key is absent rather than silently serving unauthenticated.
The daily BYOK Ollama test fails because Ollama defaults to binding on
127.0.0.1:11434, making it unreachable from the api-proxy sidecar container. The api-proxy forwards requests tohost.docker.internal:11434(which resolves to the Docker bridge IP172.17.0.1), but nothing is listening there — producing 503 on/v1/modelsand 400 on/v1/chat/completions.Changes
daily-byok-ollama-test.md— addOLLAMA_HOST: "0.0.0.0:11434"env var to theStart Ollama servicestep so Ollama binds on all interfacesdaily-byok-ollama-test.lock.yml— recompiled