feat(mosaico): make docker/mosaico a self-contained deployment bundle

[ofir-frd]

Why

The MOSAICO consortium asked how a "forked pr-agent" on GitLab would be maintained. It is not a fork: the MOSAICO A2A agent is merged upstream in pr_agent/mosaico/, ships in the release wheel, and is published as pragent/pr-agent:<version>-mosaico_agent.

This makes docker/mosaico/ a self-contained deployment bundle that consumes that published image, so the GitLab side can hold deployment assets only and no Python. Upgrading becomes a one-line tag bump.

What changed

• Compose overlay now references pragent/pr-agent:0.40.0-mosaico_agent instead of building :local, so it works with no checkout.
• Three defects fixed that would have broken registration on the demonstrator, unrelated to the image pin:
  • host port 22000 collided with DOCSTRING_AGENT_PORT; now 23000
  • AGENT_NAME was a display string; register-agent.py looks up the kebab-case slug
  • AGENT_CARD_URL used a compose service name, but it is stored in the repository and fetched by clients outside Docker, so it must be host-reachable
• Smoke test tracked, and its A2A call corrected. It still spoke pre-1.0 and could not have passed: 1.0 requires the A2A-Version header (a missing header is treated as 0.3 and rejected), renames the method to SendMessage, drops the kind discriminator, and returns the reply as a task artifact. The old assertion read status.message, which is only populated on failure, so it failed on success and would have passed on a failed task. The README documented the same stale protocol.
• README rewritten for an integrator who has never cloned pr-agent, plus a sanitized env template and a bundled MIT LICENSE.
• Registration template version 0.35.0 to 1.0.0, matching all four peer registration JSONs; it describes the contract, not the running build, so it no longer needs a bump per release.
• .dockerignore: docker/Dockerfile ADDs docker/mosaico into the test stage, so a local docker build --target test baked any local .env into an image layer. That stage is not in the publish matrix, so nothing was ever pushed.

The cloud dry-run probe section is dropped from the README: probe.py is untracked and confirmed absent from the published image, so those instructions cannot work outside a checkout.

Verification

• pytest tests/unittest: 1518 passed, 1 skipped, 1 xfailed
• docker compose config against a copy of the mosaico-demonstrator resolves cleanly; the healthcheck matches ip-solution-agent and docstring-agent byte for byte
• ./smoke_test.sh against the pinned image: FULL ROUND-TRIP PASSED (card validated, /health 200, live review returned)

No pr-agent source is removed or modified; pr_agent/mosaico/ stays upstream.

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)

1. Smoke test deletes containers ✓ Resolved 🐞 Bug ☼ Reliability

Description

docker/mosaico/smoke_test.sh hard-codes the container name and installs an EXIT trap that always
runs docker rm -f on that name, so a failure (including docker run failing due to name
collision) can delete a pre-existing, unrelated container.

Code

docker/mosaico/smoke_test.sh[R19-60]

+IMAGE="${IMAGE:-pragent/pr-agent:0.40.0-mosaico_agent}"
+PORT="${PORT:-9000}"
+CONTAINER="pr-agent-mosaico-test"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ENV_FILE="${ENV_FILE:-$SCRIPT_DIR/.env}"
+BASE="http://localhost:${PORT}"
+
+# 0700 by construction, so the /health body (which embeds the raw provider exception
+# when unhealthy) is neither world-readable nor writable at a predictable path.
+TMPDIR_RUN="$(mktemp -d)"
+cleanup() {
+  docker rm -f "$CONTAINER" >/dev/null 2>&1 || true
+  rm -rf "$TMPDIR_RUN"
+}
+trap cleanup EXIT
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+echo "==> Pulling $IMAGE"
+docker pull "$IMAGE" || fail "docker pull failed"
+
+# AGENT_CARD_* must match the published host port, otherwise the card advertises
+# http://localhost:9000/ regardless of -p and the URL assertion below would be
+# meaningless. This is the misconfiguration the README calls out as the easiest to get
+# wrong, so the smoke test exercises it rather than sidestepping it.
+run_args=(-d --rm --name "$CONTAINER" -p "${PORT}:9000"
+          -e AGENT_CARD_HOST=localhost -e "AGENT_CARD_PORT=${PORT}")
+have_creds=0
+if [[ -f "$ENV_FILE" ]]; then
+  run_args+=(--env-file "$ENV_FILE")
+  # Full round-trip requires all three LLM keys to be present and non-empty.
+  if grep -qE '^API_BASE=.+' "$ENV_FILE" \
+     && grep -qE '^API_KEY=.+' "$ENV_FILE" \
+     && grep -qE '^MODEL_NAME=.+' "$ENV_FILE"; then
+    have_creds=1
+  fi
+fi
+
+mode=$([[ $have_creds == 1 ]] && echo "full round-trip" || echo "smoke only")
+echo "==> Starting container ($mode)"
+docker run "${run_args[@]}" "$IMAGE" || fail "docker run failed"
+

Evidence

The script always force-removes the container name on exit, and the name is fixed; therefore any
early-exit path (including docker run failing because the name is already taken) will still remove
an existing container with that name.

docker/mosaico/smoke_test.sh[19-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`smoke_test.sh` uses a fixed container name and unconditionally force-removes it in an EXIT trap. If a container with that name already exists, or if the script exits early after a failure, the trap can delete a container that was not created by this run.
## Issue Context
The script sets `CONTAINER="pr-agent-mosaico-test"`, then `docker run --name "$CONTAINER" ...` and `trap cleanup EXIT`, where cleanup does `docker rm -f "$CONTAINER"`.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[19-60]
## Suggested fix
- Generate a unique container name per run (e.g., include PID + random suffix).
- Track whether this invocation successfully started a container (e.g., `started=1` after `docker run` succeeds) and only `docker rm -f` when `started==1`.
- Alternatively, label the container (e.g., `--label pr-agent.smoke=1`) and in cleanup remove by ID captured from `docker run`/`docker ps` filtering for that label, avoiding name-based deletion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unchecked mktemp failure 🐞 Bug ☼ Reliability ⭐ New

Description

docker/mosaico/smoke_test.sh assigns TMPDIR_RUN from mktemp -d without checking for failure;
if mktemp fails, TMPDIR_RUN can become empty and later curl -o "$TMPDIR_RUN/health.json" /
resp.json can write to absolute paths like /health.json and /resp.json (or fail
unpredictably). This can break the full round-trip path and can clobber files when run with elevated
permissions.

Code

docker/mosaico/smoke_test.sh[R26-38]

+# 0700 by construction, so the /health body (which embeds the raw provider exception
+# when unhealthy) is neither world-readable nor writable at a predictable path.
+TMPDIR_RUN="$(mktemp -d)"
+# Only tear down a container this run actually started: the name is fixed, so an early
+# exit (a failed pull, or a `docker run` that lost a name race) must not reap someone
+# else's container - including the one a concurrent run is still testing against.
+started=0
+cleanup() {
+  if [[ $started == 1 ]]; then
+    docker rm -f "$CONTAINER" >/dev/null 2>&1 || true
+  fi
+  rm -rf "$TMPDIR_RUN"
+}

Evidence

The script creates TMPDIR_RUN with mktemp -d but never checks whether it succeeded, and later
uses TMPDIR_RUN to build output file paths for curl responses; an empty TMPDIR_RUN makes those
paths absolute (/health.json, /resp.json).

docker/mosaico/smoke_test.sh[26-38]
docker/mosaico/smoke_test.sh[106-126]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`TMPDIR_RUN` is created via `mktemp -d` but the script doesn’t validate that it succeeded. If `mktemp` fails (platform differences, permissions, disk full, etc.), `TMPDIR_RUN` may be empty and later output paths like `"$TMPDIR_RUN/health.json"` resolve to `/health.json`.

### Issue Context
This impacts the script’s **full round-trip** path that writes `/health` and `SendMessage` responses to `$TMPDIR_RUN`.

### Fix Focus Areas
- docker/mosaico/smoke_test.sh[26-38]
- docker/mosaico/smoke_test.sh[106-126]

### What to change
- After `mktemp -d`, validate `TMPDIR_RUN` is non-empty and that the directory exists; otherwise `fail` immediately.
- Optionally add a portable fallback for BSD/macOS (e.g., try `mktemp -d -t pr-agent-mosaico` if plain `mktemp -d` fails).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. urlopen() healthcheck missing timeout 📘 Rule violation ☼ Reliability

Description

The compose healthcheck probes /health via urllib.request.urlopen() without an explicit
timeout, which can hang and delay (or block) service_healthy and downstream depends_on startup.
This weakens failure handling for an availability-critical readiness check.

Code

docker/mosaico/docker-compose.pr-agent.yml[25]

+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT:-9000}/health')\""]

Evidence

PR Compliance ID 3 requires graceful handling of error scenarios; a readiness probe that can hang
indefinitely is a missing edge-case handling path. The healthcheck command at
docker-compose.pr-agent.yml:25 calls urllib.request.urlopen() without a timeout parameter.

Rule 3: Robust Error Handling
docker/mosaico/docker-compose.pr-agent.yml[25-25]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The docker-compose healthcheck uses `urllib.request.urlopen()` without a timeout, which can hang for a long time on network stalls.
## Issue Context
This healthcheck gates the registration container via `depends_on: condition: service_healthy`, so a hung probe can prevent registration and make deployments appear stuck.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[25-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Smoke-test curl lacks timeouts ✓ Resolved 📘 Rule violation ☼ Reliability

Description

smoke_test.sh uses curl for /health and SendMessage without any connect/overall timeouts, so
the script can hang indefinitely on a stalled network or unresponsive container. This is missing
edge-case handling for a test script intended to provide fast feedback.

Code

docker/mosaico/smoke_test.sh[R94-112]

+# --- FULL: /health (live LLM ping -> 200/503) ---
+echo "==> [full] GET /health (live LLM probe)"
+code=$(curl -s -o "$TMPDIR_RUN/health.json" -w '%{http_code}' "$BASE/health")
+# On 503 the body is a raw provider exception (it can name the endpoint), which is
+# exactly the diagnostic you want here - just don't paste it into a public issue.
+cat "$TMPDIR_RUN/health.json"; echo
+[[ "$code" == "200" ]] || fail "/health returned $code (expected 200) — check LLM creds"
+
+# --- FULL: SendMessage review on an inline diff (no PR URL / GitHub token needed) ---
+# A2A 1.0 wire contract: the `A2A-Version: 1.0` header is REQUIRED (the server treats a
+# missing header as 0.3 and rejects the call), the method is the gRPC-style `SendMessage`,
+# and Message/Part carry no `kind` discriminator.
+echo "==> [full] A2A SendMessage review (inline diff)"
+read -r -d '' BODY <<'JSON'
+{"id":"smoke-1","jsonrpc":"2.0","method":"SendMessage","params":{"message":{"messageId":"smoke-msg-1","role":"ROLE_USER","parts":[{"text":"review the following\n```diff\ndiff --git a/foo.py b/foo.py\nindex 1111111..2222222 100644\n--- a/foo.py\n+++ b/foo.py\n@@ -1,2 +1,2 @@\n-x = 1\n+x = 2\n y = 3\n```"}]}}}
+JSON
+
+curl -fsS -X POST "$BASE/" -H 'Content-Type: application/json' -H 'A2A-Version: 1.0' \
+  -d "$BODY" -o "$TMPDIR_RUN/resp.json" || fail "SendMessage request failed"

Evidence

PR Compliance ID 3 calls for robust handling of error scenarios; network calls should bound wait
time to avoid indefinite hangs. The /health probe and the A2A POST request use curl without
timeout controls in the cited lines.

Rule 3: Robust Error Handling
docker/mosaico/smoke_test.sh[94-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The smoke test's `curl` invocations have no explicit timeouts, which can cause the script to hang indefinitely.
## Issue Context
This script is intended to be a quick validation (smoke/full). Adding `--connect-timeout` and `--max-time` makes failures deterministic and easier to diagnose.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[94-112]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (4)

5. Healthcheck calls live LLM 🐞 Bug ➹ Performance

Description

The compose overlay’s healthcheck hits /health, and /health performs a real
litellm.acompletion call, so Docker healthchecks will continuously generate external LLM traffic
while the container runs and any LLM outage/misconfig will keep the service unhealthy (blocking the
registration container via depends_on: condition: service_healthy).

Code

docker/mosaico/docker-compose.pr-agent.yml[R20-39]

+    # Mirrors the peer agents' probe. /health is a live LLM check, so no LLM means no
+    # registration - intended: a registered card with a dead LLM is worse.
+    # PORT comes from base-definitions.yml's `agent` service; the :-9000 fallback keeps
+    # the probe correct even if this overlay is composed without it.
healthcheck:
-      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9000/.well-known/agent-card.json')\""]
+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT:-9000}/health')\""]
pr-agent-solution-agent-registration:
extends:
 file: base-definitions.yml
 service: agent-registration
environment:
-      AGENT_NAME: PR-Agent Solution Agent
+      AGENT_NAME: pr-agent-solution-agent
 AGENT_JSON: /app/pr-agent-solution-agent.json
-      # Fetched container-to-container: use the service name and in-container port 9000.
-      AGENT_CARD_URL: http://pr-agent-solution-agent:9000/.well-known/agent-card.json
+      # Stored in the repository and fetched by out-of-Docker clients, so it must be
+      # host-reachable, not a compose service name.
+      AGENT_CARD_URL: http://${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}:${PR_AGENT_PORT:-23000}/.well-known/agent-card.json
depends_on:
 pr-agent-solution-agent:
   condition: service_healthy

Evidence

The overlay healthcheck probes /health, and the server’s health_check() implementation issues a
real LLM request via litellm.acompletion, so periodic healthchecks will repeatedly invoke the LLM
and LLM failures will keep the service unhealthy (preventing the dependent registration service from
starting).

docker/mosaico/docker-compose.pr-agent.yml[20-39]
pr_agent/mosaico/executor.py[88-116]
pr_agent/mosaico/server.py[40-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new container healthcheck probes `/health`, which is implemented as a live LLM connectivity probe (`litellm.acompletion`). Docker healthchecks run periodically for the lifetime of the container, so this introduces continuous external LLM requests (cost/rate-limit/latency exposure) and makes registration permanently dependent on the LLM being reachable.
## Issue Context
- Compose healthcheck now calls `/health`.
- `/health` delegates to `health_check()`, which performs a real LLM request.
- Registration container is gated by `depends_on: condition: service_healthy`.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[20-39]
- pr_agent/mosaico/executor.py[88-116]
## Suggested fix
Keep the intended semantics (only register when LLM is healthy) but reduce ongoing overhead:
- Explicitly set healthcheck `interval` to a longer period (e.g., minutes) and `timeout`/`retries`/`start_period` to match expected LLM behavior.
- If you want Docker health to reflect HTTP readiness rather than LLM readiness, switch the healthcheck to a cheap endpoint (e.g., agent-card), and make `/health` a manual diagnostic (while adjusting registration gating accordingly).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Temp-file clobber in smoke ✓ Resolved 🐞 Bug ⛨ Security

Description

docker/mosaico/smoke_test.sh writes /health and SendMessage responses to fixed filenames under /tmp
and never removes them, so concurrent runs can overwrite each other and stale data can be read by
later runs. Because /health includes exception text in its JSON body when unhealthy, the leftover
file can also retain internal diagnostic details on disk (permissions depend on umask).

Code

docker/mosaico/smoke_test.sh[R88-107]

+# --- FULL: /health (live LLM ping -> 200/503) ---
+echo "==> [full] GET /health (live LLM probe)"
+code=$(curl -s -o /tmp/mosaico_health.json -w '%{http_code}' "$BASE/health")
+# On 503 the body is a raw provider exception (it can name the endpoint), which is
+# exactly the diagnostic you want here - just don't paste it into a public issue.
+cat /tmp/mosaico_health.json; echo
+[[ "$code" == "200" ]] || fail "/health returned $code (expected 200) — check LLM creds"
+
+# --- FULL: SendMessage review on an inline diff (no PR URL / GitHub token needed) ---
+# A2A 1.0 wire contract: the `A2A-Version: 1.0` header is REQUIRED (the server treats a
+# missing header as 0.3 and rejects the call), the method is the gRPC-style `SendMessage`,
+# and Message/Part carry no `kind` discriminator.
+echo "==> [full] A2A SendMessage review (inline diff)"
+read -r -d '' BODY <<'JSON'
+{"id":"smoke-1","jsonrpc":"2.0","method":"SendMessage","params":{"message":{"messageId":"smoke-msg-1","role":"ROLE_USER","parts":[{"text":"review the following\n```diff\ndiff --git a/foo.py b/foo.py\nindex 1111111..2222222 100644\n--- a/foo.py\n+++ b/foo.py\n@@ -1,2 +1,2 @@\n-x = 1\n+x = 2\n y = 3\n```"}]}}}
+JSON
+
+curl -fsS -X POST "$BASE/" -H 'Content-Type: application/json' -H 'A2A-Version: 1.0' \
+  -d "$BODY" -o /tmp/mosaico_resp.json || fail "SendMessage request failed"
+python3 - <<'PY' || fail "SendMessage response invalid"

Evidence

The smoke test currently writes to fixed /tmp paths with no cleanup, and /health explicitly
includes the health_check exception text in the JSON response body when unhealthy; together, this
demonstrates both clobber risk and the potential for leaving diagnostic data on disk.

docker/mosaico/smoke_test.sh[88-117]
pr_agent/mosaico/server.py[40-47]
pr_agent/mosaico/executor.py[88-119]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`docker/mosaico/smoke_test.sh` writes responses to hard-coded `/tmp/mosaico_health.json` and `/tmp/mosaico_resp.json` and never deletes them. This can cause (1) clobbering between concurrent/overlapping runs and (2) leaving behind diagnostic response bodies (including exception strings from `/health`).
### Issue Context
- The script already has an `EXIT` trap for container cleanup; reuse that trap to also remove any temp files/dirs.
- `/health` responses can embed exception text when unhealthy.
### Fix Focus Areas
- docker/mosaico/smoke_test.sh[26-28]
- docker/mosaico/smoke_test.sh[88-107]
### Suggested change
- Create a per-run temp directory (e.g., `tmpdir=$(mktemp -d)`), store `health.json` and `resp.json` inside it, and extend the `EXIT` trap to `rm -rf "$tmpdir"`.
- Optionally set restrictive permissions (e.g., `umask 077`) before writing files, if you want to minimize local disclosure on shared hosts/CI runners.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Healthcheck needs PORT set ✓ Resolved 🐞 Bug ☼ Reliability

Description

The compose healthcheck probes http://localhost:$PORT/health but this overlay does not set PORT
and the mosaico_agent image also does not define it, so $PORT can expand empty and the service
never becomes healthy (blocking the registration container via `depends_on: condition:
service_healthy). Fixing this only addresses the port-expansion bug; /health` can still
intentionally return 503 when LLM creds/model aren’t configured.

Code

docker/mosaico/docker-compose.pr-agent.yml[R15-36]

environment:
AGENT_CARD_HOST: ${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}
-      AGENT_CARD_PORT: ${PR_AGENT_PORT:-22000}
+      AGENT_CARD_PORT: ${PR_AGENT_PORT:-23000}
MODEL_NAME: ${PR_AGENT_MODEL}
-    # Gate registration on card/HTTP readiness, not /health (a live LLM probe).
+    # Matches the peer agents' probe verbatim. /health is a live LLM check, so no LLM
+    # means no registration - intended: a registered card with a dead LLM is worse.
healthcheck:
-      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9000/.well-known/agent-card.json')\""]
+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT}/health')\""]
pr-agent-solution-agent-registration:
extends:
file: base-definitions.yml
service: agent-registration
environment:
-      AGENT_NAME: PR-Agent Solution Agent
+      AGENT_NAME: pr-agent-solution-agent
AGENT_JSON: /app/pr-agent-solution-agent.json
-      # Fetched container-to-container: use the service name and in-container port 9000.
-      AGENT_CARD_URL: http://pr-agent-solution-agent:9000/.well-known/agent-card.json
+      # Stored in the repository and fetched by out-of-Docker clients, so it must be
+      # host-reachable, not a compose service name.
+      AGENT_CARD_URL: http://${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}:${PR_AGENT_PORT:-23000}/.well-known/agent-card.json
depends_on:
pr-agent-solution-agent:
condition: service_healthy

Evidence

The overlay healthcheck expands $PORT but does not define it, while the image simply runs the
MOSAICO server without setting any PORT env; the server code defaults to 9000 when PORT is
absent, so the application can start but the shell-expanded healthcheck can still fail due to an
empty variable.

docker/mosaico/docker-compose.pr-agent.yml[12-37]
docker/Dockerfile[40-43]
pr_agent/mosaico/server.py[95-103]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The healthcheck command uses `$PORT` (`$${PORT}` in compose-escaped form), but neither the compose overlay nor the Docker image defines `PORT`. When `PORT` is unset, the healthcheck URL becomes invalid (`http://localhost:/health`), keeping the agent `unhealthy` and preventing the registration service from starting.
## Issue Context
- The server binds to `PORT` if set, otherwise defaults to `9000`.
- The healthcheck should not depend on `PORT` being injected by an external compose base.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[15-23]
### Suggested change
Either:
1) Add `PORT: 9000` to the service environment so `$PORT` is always present, **or**
2) Change the healthcheck URL to use a fixed `9000` or a shell fallback `${PORT:-9000}` (escaped appropriately for compose).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Smoke test ignores advertised URL ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new smoke_test.sh maps host ${PORT} to container 9000 but does not ensure the container
advertises that mapped port in supportedInterfaces (and it never asserts the advertised URL), so
PORT=19000 ./smoke_test.sh can pass while the agent card still advertises
http://localhost:9000/.

Code

docker/mosaico/smoke_test.sh[R19-70]

+IMAGE="${IMAGE:-pragent/pr-agent:0.40.0-mosaico_agent}"
+PORT="${PORT:-9000}"
+CONTAINER="pr-agent-mosaico-test"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ENV_FILE="${ENV_FILE:-$SCRIPT_DIR/.env}"
+BASE="http://localhost:${PORT}"
+
+cleanup() { docker rm -f "$CONTAINER" >/dev/null 2>&1 || true; }
+trap cleanup EXIT
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+echo "==> Pulling $IMAGE"
+docker pull "$IMAGE" || fail "docker pull failed"
+
+run_args=(-d --rm --name "$CONTAINER" -p "${PORT}:9000")
+have_creds=0
+if [[ -f "$ENV_FILE" ]]; then
+  run_args+=(--env-file "$ENV_FILE")
+  # Full round-trip requires all three LLM keys to be present and non-empty.
+  if grep -qE '^API_BASE=.+' "$ENV_FILE" \
+     && grep -qE '^API_KEY=.+' "$ENV_FILE" \
+     && grep -qE '^MODEL_NAME=.+' "$ENV_FILE"; then
+    have_creds=1
+  fi
+fi
+
+mode=$([[ $have_creds == 1 ]] && echo "full round-trip" || echo "smoke only")
+echo "==> Starting container ($mode)"
+docker run "${run_args[@]}" "$IMAGE" || fail "docker run failed"
+
+# --- SMOKE: fetch the card (with retry; needs no LLM) and validate it ---
+echo "==> [smoke] fetching + validating agent card"
+CARD=""
+for _ in $(seq 1 30); do
+  CARD=$(curl -fsSL "$BASE/.well-known/agent-card.json" 2>/dev/null)
+  [[ -n "$CARD" ]] && break
+  sleep 2
+done
+[[ -n "$CARD" ]] || { docker logs "$CONTAINER" 2>&1 | tail -40; fail "card endpoint never served a body"; }
+
+CARD="$CARD" python3 - <<'PY' || fail "agent card invalid"
+import json, os
+c = json.loads(os.environ["CARD"])
+assert c["name"] == "PR-Agent Solution Agent", c.get("name")
+assert c["capabilities"]["streaming"] is False, "streaming must be False"
+exts = c["capabilities"]["extensions"]
+assert any(e.get("required") and "observability" in e["uri"] for e in exts), "observability ext missing/required"
+ids = sorted(s["id"] for s in c["skills"])
+assert ids == ["ask", "describe", "improve", "review"], ids
+print("    card OK: name, streaming=False, observability required, skills", ids)
+PY

Evidence

The script’s PORT is used only for host mapping and the curl base URL, but it is not injected into
the container as AGENT_CARD_PORT or PORT. The agent card generation defaults to localhost and
port AGENT_CARD_PORT || PORT || 9000, so with no injected env it will advertise :9000 even when
the host mapping uses a different port.

docker/mosaico/smoke_test.sh[19-70]
pr_agent/mosaico/card.py[19-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`smoke_test.sh` allows overriding the host port via `PORT=...`, but it does not set `AGENT_CARD_HOST`/`AGENT_CARD_PORT` (unless the user happens to put them in `.env`) and does not validate `supportedInterfaces[0].url`. This can report a successful smoke test even though the card advertises an endpoint that doesn’t match the actual exposed host port.
## Issue Context
The agent card URL is computed from `AGENT_CARD_HOST` and `AGENT_CARD_PORT` (or `PORT`, or default 9000). If the container is started without those env vars, it will default to `localhost:9000` regardless of the host port mapping.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[19-70]
- pr_agent/mosaico/card.py[19-26]
### Suggested change
- When launching the container, set `AGENT_CARD_HOST=localhost` and `AGENT_CARD_PORT=$PORT` unless those keys are already provided in the `.env` file.
- Extend the Python card validation to assert `supportedInterfaces[0]['url'] == f'http://localhost:{PORT}/'` (or derived from `$BASE/`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Previous review results

Review updated until commit 1b9540c ⚖️ Balanced

Results up to commit 4203da6

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

1. Smoke test deletes containers ✓ Resolved 🐞 Bug ☼ Reliability

Description

docker/mosaico/smoke_test.sh hard-codes the container name and installs an EXIT trap that always
runs docker rm -f on that name, so a failure (including docker run failing due to name
collision) can delete a pre-existing, unrelated container.

Code

docker/mosaico/smoke_test.sh[R19-60]

+IMAGE="${IMAGE:-pragent/pr-agent:0.40.0-mosaico_agent}"
+PORT="${PORT:-9000}"
+CONTAINER="pr-agent-mosaico-test"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ENV_FILE="${ENV_FILE:-$SCRIPT_DIR/.env}"
+BASE="http://localhost:${PORT}"
+
+# 0700 by construction, so the /health body (which embeds the raw provider exception
+# when unhealthy) is neither world-readable nor writable at a predictable path.
+TMPDIR_RUN="$(mktemp -d)"
+cleanup() {
+  docker rm -f "$CONTAINER" >/dev/null 2>&1 || true
+  rm -rf "$TMPDIR_RUN"
+}
+trap cleanup EXIT
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+echo "==> Pulling $IMAGE"
+docker pull "$IMAGE" || fail "docker pull failed"
+
+# AGENT_CARD_* must match the published host port, otherwise the card advertises
+# http://localhost:9000/ regardless of -p and the URL assertion below would be
+# meaningless. This is the misconfiguration the README calls out as the easiest to get
+# wrong, so the smoke test exercises it rather than sidestepping it.
+run_args=(-d --rm --name "$CONTAINER" -p "${PORT}:9000"
+          -e AGENT_CARD_HOST=localhost -e "AGENT_CARD_PORT=${PORT}")
+have_creds=0
+if [[ -f "$ENV_FILE" ]]; then
+  run_args+=(--env-file "$ENV_FILE")
+  # Full round-trip requires all three LLM keys to be present and non-empty.
+  if grep -qE '^API_BASE=.+' "$ENV_FILE" \
+     && grep -qE '^API_KEY=.+' "$ENV_FILE" \
+     && grep -qE '^MODEL_NAME=.+' "$ENV_FILE"; then
+    have_creds=1
+  fi
+fi
+
+mode=$([[ $have_creds == 1 ]] && echo "full round-trip" || echo "smoke only")
+echo "==> Starting container ($mode)"
+docker run "${run_args[@]}" "$IMAGE" || fail "docker run failed"
+

Evidence

The script always force-removes the container name on exit, and the name is fixed; therefore any
early-exit path (including docker run failing because the name is already taken) will still remove
an existing container with that name.

docker/mosaico/smoke_test.sh[19-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`smoke_test.sh` uses a fixed container name and unconditionally force-removes it in an EXIT trap. If a container with that name already exists, or if the script exits early after a failure, the trap can delete a container that was not created by this run.
## Issue Context
The script sets `CONTAINER="pr-agent-mosaico-test"`, then `docker run --name "$CONTAINER" ...` and `trap cleanup EXIT`, where cleanup does `docker rm -f "$CONTAINER"`.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[19-60]
## Suggested fix
- Generate a unique container name per run (e.g., include PID + random suffix).
- Track whether this invocation successfully started a container (e.g., `started=1` after `docker run` succeeds) and only `docker rm -f` when `started==1`.
- Alternatively, label the container (e.g., `--label pr-agent.smoke=1`) and in cleanup remove by ID captured from `docker run`/`docker ps` filtering for that label, avoiding name-based deletion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. urlopen() healthcheck missing timeout 📘 Rule violation ☼ Reliability

Description

The compose healthcheck probes /health via urllib.request.urlopen() without an explicit
timeout, which can hang and delay (or block) service_healthy and downstream depends_on startup.
This weakens failure handling for an availability-critical readiness check.

Code

docker/mosaico/docker-compose.pr-agent.yml[25]

+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT:-9000}/health')\""]

Evidence

PR Compliance ID 3 requires graceful handling of error scenarios; a readiness probe that can hang
indefinitely is a missing edge-case handling path. The healthcheck command at
docker-compose.pr-agent.yml:25 calls urllib.request.urlopen() without a timeout parameter.

Rule 3: Robust Error Handling
docker/mosaico/docker-compose.pr-agent.yml[25-25]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The docker-compose healthcheck uses `urllib.request.urlopen()` without a timeout, which can hang for a long time on network stalls.
## Issue Context
This healthcheck gates the registration container via `depends_on: condition: service_healthy`, so a hung probe can prevent registration and make deployments appear stuck.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[25-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Smoke-test curl lacks timeouts ✓ Resolved 📘 Rule violation ☼ Reliability

Description

smoke_test.sh uses curl for /health and SendMessage without any connect/overall timeouts, so
the script can hang indefinitely on a stalled network or unresponsive container. This is missing
edge-case handling for a test script intended to provide fast feedback.

Code

docker/mosaico/smoke_test.sh[R94-112]

+# --- FULL: /health (live LLM ping -> 200/503) ---
+echo "==> [full] GET /health (live LLM probe)"
+code=$(curl -s -o "$TMPDIR_RUN/health.json" -w '%{http_code}' "$BASE/health")
+# On 503 the body is a raw provider exception (it can name the endpoint), which is
+# exactly the diagnostic you want here - just don't paste it into a public issue.
+cat "$TMPDIR_RUN/health.json"; echo
+[[ "$code" == "200" ]] || fail "/health returned $code (expected 200) — check LLM creds"
+
+# --- FULL: SendMessage review on an inline diff (no PR URL / GitHub token needed) ---
+# A2A 1.0 wire contract: the `A2A-Version: 1.0` header is REQUIRED (the server treats a
+# missing header as 0.3 and rejects the call), the method is the gRPC-style `SendMessage`,
+# and Message/Part carry no `kind` discriminator.
+echo "==> [full] A2A SendMessage review (inline diff)"
+read -r -d '' BODY <<'JSON'
+{"id":"smoke-1","jsonrpc":"2.0","method":"SendMessage","params":{"message":{"messageId":"smoke-msg-1","role":"ROLE_USER","parts":[{"text":"review the following\n```diff\ndiff --git a/foo.py b/foo.py\nindex 1111111..2222222 100644\n--- a/foo.py\n+++ b/foo.py\n@@ -1,2 +1,2 @@\n-x = 1\n+x = 2\n y = 3\n```"}]}}}
+JSON
+
+curl -fsS -X POST "$BASE/" -H 'Content-Type: application/json' -H 'A2A-Version: 1.0' \
+  -d "$BODY" -o "$TMPDIR_RUN/resp.json" || fail "SendMessage request failed"

Evidence

PR Compliance ID 3 calls for robust handling of error scenarios; network calls should bound wait
time to avoid indefinite hangs. The /health probe and the A2A POST request use curl without
timeout controls in the cited lines.

Rule 3: Robust Error Handling
docker/mosaico/smoke_test.sh[94-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The smoke test's `curl` invocations have no explicit timeouts, which can cause the script to hang indefinitely.
## Issue Context
This script is intended to be a quick validation (smoke/full). Adding `--connect-timeout` and `--max-time` makes failures deterministic and easier to diagnose.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[94-112]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Healthcheck calls live LLM 🐞 Bug ➹ Performance

Description

The compose overlay’s healthcheck hits /health, and /health performs a real
litellm.acompletion call, so Docker healthchecks will continuously generate external LLM traffic
while the container runs and any LLM outage/misconfig will keep the service unhealthy (blocking the
registration container via depends_on: condition: service_healthy).

Code

docker/mosaico/docker-compose.pr-agent.yml[R20-39]

+    # Mirrors the peer agents' probe. /health is a live LLM check, so no LLM means no
+    # registration - intended: a registered card with a dead LLM is worse.
+    # PORT comes from base-definitions.yml's `agent` service; the :-9000 fallback keeps
+    # the probe correct even if this overlay is composed without it.
  healthcheck:
-      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9000/.well-known/agent-card.json')\""]
+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT:-9000}/health')\""]
pr-agent-solution-agent-registration:
  extends:
    file: base-definitions.yml
    service: agent-registration
  environment:
-      AGENT_NAME: PR-Agent Solution Agent
+      AGENT_NAME: pr-agent-solution-agent
    AGENT_JSON: /app/pr-agent-solution-agent.json
-      # Fetched container-to-container: use the service name and in-container port 9000.
-      AGENT_CARD_URL: http://pr-agent-solution-agent:9000/.well-known/agent-card.json
+      # Stored in the repository and fetched by out-of-Docker clients, so it must be
+      # host-reachable, not a compose service name.
+      AGENT_CARD_URL: http://${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}:${PR_AGENT_PORT:-23000}/.well-known/agent-card.json
  depends_on:
    pr-agent-solution-agent:
      condition: service_healthy

Evidence

The overlay healthcheck probes /health, and the server’s health_check() implementation issues a
real LLM request via litellm.acompletion, so periodic healthchecks will repeatedly invoke the LLM
and LLM failures will keep the service unhealthy (preventing the dependent registration service from
starting).

docker/mosaico/docker-compose.pr-agent.yml[20-39]
pr_agent/mosaico/executor.py[88-116]
pr_agent/mosaico/server.py[40-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new container healthcheck probes `/health`, which is implemented as a live LLM connectivity probe (`litellm.acompletion`). Docker healthchecks run periodically for the lifetime of the container, so this introduces continuous external LLM requests (cost/rate-limit/latency exposure) and makes registration permanently dependent on the LLM being reachable.
## Issue Context
- Compose healthcheck now calls `/health`.
- `/health` delegates to `health_check()`, which performs a real LLM request.
- Registration container is gated by `depends_on: condition: service_healthy`.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[20-39]
- pr_agent/mosaico/executor.py[88-116]
## Suggested fix
Keep the intended semantics (only register when LLM is healthy) but reduce ongoing overhead:
- Explicitly set healthcheck `interval` to a longer period (e.g., minutes) and `timeout`/`retries`/`start_period` to match expected LLM behavior.
- If you want Docker health to reflect HTTP readiness rather than LLM readiness, switch the healthcheck to a cheap endpoint (e.g., agent-card), and make `/health` a manual diagnostic (while adjusting registration gating accordingly).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (3)

5. Temp-file clobber in smoke ✓ Resolved 🐞 Bug ⛨ Security

Description

docker/mosaico/smoke_test.sh writes /health and SendMessage responses to fixed filenames under /tmp
and never removes them, so concurrent runs can overwrite each other and stale data can be read by
later runs. Because /health includes exception text in its JSON body when unhealthy, the leftover
file can also retain internal diagnostic details on disk (permissions depend on umask).

Code

docker/mosaico/smoke_test.sh[R88-107]

+# --- FULL: /health (live LLM ping -> 200/503) ---
+echo "==> [full] GET /health (live LLM probe)"
+code=$(curl -s -o /tmp/mosaico_health.json -w '%{http_code}' "$BASE/health")
+# On 503 the body is a raw provider exception (it can name the endpoint), which is
+# exactly the diagnostic you want here - just don't paste it into a public issue.
+cat /tmp/mosaico_health.json; echo
+[[ "$code" == "200" ]] || fail "/health returned $code (expected 200) — check LLM creds"
+
+# --- FULL: SendMessage review on an inline diff (no PR URL / GitHub token needed) ---
+# A2A 1.0 wire contract: the `A2A-Version: 1.0` header is REQUIRED (the server treats a
+# missing header as 0.3 and rejects the call), the method is the gRPC-style `SendMessage`,
+# and Message/Part carry no `kind` discriminator.
+echo "==> [full] A2A SendMessage review (inline diff)"
+read -r -d '' BODY <<'JSON'
+{"id":"smoke-1","jsonrpc":"2.0","method":"SendMessage","params":{"message":{"messageId":"smoke-msg-1","role":"ROLE_USER","parts":[{"text":"review the following\n```diff\ndiff --git a/foo.py b/foo.py\nindex 1111111..2222222 100644\n--- a/foo.py\n+++ b/foo.py\n@@ -1,2 +1,2 @@\n-x = 1\n+x = 2\n y = 3\n```"}]}}}
+JSON
+
+curl -fsS -X POST "$BASE/" -H 'Content-Type: application/json' -H 'A2A-Version: 1.0' \
+  -d "$BODY" -o /tmp/mosaico_resp.json || fail "SendMessage request failed"
+python3 - <<'PY' || fail "SendMessage response invalid"

Evidence

The smoke test currently writes to fixed /tmp paths with no cleanup, and /health explicitly
includes the health_check exception text in the JSON response body when unhealthy; together, this
demonstrates both clobber risk and the potential for leaving diagnostic data on disk.

docker/mosaico/smoke_test.sh[88-117]
pr_agent/mosaico/server.py[40-47]
pr_agent/mosaico/executor.py[88-119]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`docker/mosaico/smoke_test.sh` writes responses to hard-coded `/tmp/mosaico_health.json` and `/tmp/mosaico_resp.json` and never deletes them. This can cause (1) clobbering between concurrent/overlapping runs and (2) leaving behind diagnostic response bodies (including exception strings from `/health`).
### Issue Context
- The script already has an `EXIT` trap for container cleanup; reuse that trap to also remove any temp files/dirs.
- `/health` responses can embed exception text when unhealthy.
### Fix Focus Areas
- docker/mosaico/smoke_test.sh[26-28]
- docker/mosaico/smoke_test.sh[88-107]
### Suggested change
- Create a per-run temp directory (e.g., `tmpdir=$(mktemp -d)`), store `health.json` and `resp.json` inside it, and extend the `EXIT` trap to `rm -rf "$tmpdir"`.
- Optionally set restrictive permissions (e.g., `umask 077`) before writing files, if you want to minimize local disclosure on shared hosts/CI runners.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Healthcheck needs PORT set ✓ Resolved 🐞 Bug ☼ Reliability

Description

The compose healthcheck probes http://localhost:$PORT/health but this overlay does not set PORT
and the mosaico_agent image also does not define it, so $PORT can expand empty and the service
never becomes healthy (blocking the registration container via `depends_on: condition:
service_healthy). Fixing this only addresses the port-expansion bug; /health` can still
intentionally return 503 when LLM creds/model aren’t configured.

Code

docker/mosaico/docker-compose.pr-agent.yml[R15-36]

environment:
AGENT_CARD_HOST: ${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}
-      AGENT_CARD_PORT: ${PR_AGENT_PORT:-22000}
+      AGENT_CARD_PORT: ${PR_AGENT_PORT:-23000}
MODEL_NAME: ${PR_AGENT_MODEL}
-    # Gate registration on card/HTTP readiness, not /health (a live LLM probe).
+    # Matches the peer agents' probe verbatim. /health is a live LLM check, so no LLM
+    # means no registration - intended: a registered card with a dead LLM is worse.
healthcheck:
-      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9000/.well-known/agent-card.json')\""]
+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT}/health')\""]
pr-agent-solution-agent-registration:
extends:
file: base-definitions.yml
service: agent-registration
environment:
-      AGENT_NAME: PR-Agent Solution Agent
+      AGENT_NAME: pr-agent-solution-agent
AGENT_JSON: /app/pr-agent-solution-agent.json
-      # Fetched container-to-container: use the service name and in-container port 9000.
-      AGENT_CARD_URL: http://pr-agent-solution-agent:9000/.well-known/agent-card.json
+      # Stored in the repository and fetched by out-of-Docker clients, so it must be
+      # host-reachable, not a compose service name.
+      AGENT_CARD_URL: http://${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}:${PR_AGENT_PORT:-23000}/.well-known/agent-card.json
depends_on:
pr-agent-solution-agent:
 condition: service_healthy

Evidence

The overlay healthcheck expands $PORT but does not define it, while the image simply runs the
MOSAICO server without setting any PORT env; the server code defaults to 9000 when PORT is
absent, so the application can start but the shell-expanded healthcheck can still fail due to an
empty variable.

docker/mosaico/docker-compose.pr-agent.yml[12-37]
docker/Dockerfile[40-43]
pr_agent/mosaico/server.py[95-103]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The healthcheck command uses `$PORT` (`$${PORT}` in compose-escaped form), but neither the compose overlay nor the Docker image defines `PORT`. When `PORT` is unset, the healthcheck URL becomes invalid (`http://localhost:/health`), keeping the agent `unhealthy` and preventing the registration service from starting.
## Issue Context
- The server binds to `PORT` if set, otherwise defaults to `9000`.
- The healthcheck should not depend on `PORT` being injected by an external compose base.
## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[15-23]
### Suggested change
Either:
1) Add `PORT: 9000` to the service environment so `$PORT` is always present, **or**
2) Change the healthcheck URL to use a fixed `9000` or a shell fallback `${PORT:-9000}` (escaped appropriately for compose).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Smoke test ignores advertised URL ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new smoke_test.sh maps host ${PORT} to container 9000 but does not ensure the container
advertises that mapped port in supportedInterfaces (and it never asserts the advertised URL), so
PORT=19000 ./smoke_test.sh can pass while the agent card still advertises
http://localhost:9000/.

Code

docker/mosaico/smoke_test.sh[R19-70]

+IMAGE="${IMAGE:-pragent/pr-agent:0.40.0-mosaico_agent}"
+PORT="${PORT:-9000}"
+CONTAINER="pr-agent-mosaico-test"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ENV_FILE="${ENV_FILE:-$SCRIPT_DIR/.env}"
+BASE="http://localhost:${PORT}"
+
+cleanup() { docker rm -f "$CONTAINER" >/dev/null 2>&1 || true; }
+trap cleanup EXIT
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+echo "==> Pulling $IMAGE"
+docker pull "$IMAGE" || fail "docker pull failed"
+
+run_args=(-d --rm --name "$CONTAINER" -p "${PORT}:9000")
+have_creds=0
+if [[ -f "$ENV_FILE" ]]; then
+  run_args+=(--env-file "$ENV_FILE")
+  # Full round-trip requires all three LLM keys to be present and non-empty.
+  if grep -qE '^API_BASE=.+' "$ENV_FILE" \
+     && grep -qE '^API_KEY=.+' "$ENV_FILE" \
+     && grep -qE '^MODEL_NAME=.+' "$ENV_FILE"; then
+    have_creds=1
+  fi
+fi
+
+mode=$([[ $have_creds == 1 ]] && echo "full round-trip" || echo "smoke only")
+echo "==> Starting container ($mode)"
+docker run "${run_args[@]}" "$IMAGE" || fail "docker run failed"
+
+# --- SMOKE: fetch the card (with retry; needs no LLM) and validate it ---
+echo "==> [smoke] fetching + validating agent card"
+CARD=""
+for _ in $(seq 1 30); do
+  CARD=$(curl -fsSL "$BASE/.well-known/agent-card.json" 2>/dev/null)
+  [[ -n "$CARD" ]] && break
+  sleep 2
+done
+[[ -n "$CARD" ]] || { docker logs "$CONTAINER" 2>&1 | tail -40; fail "card endpoint never served a body"; }
+
+CARD="$CARD" python3 - <<'PY' || fail "agent card invalid"
+import json, os
+c = json.loads(os.environ["CARD"])
+assert c["name"] == "PR-Agent Solution Agent", c.get("name")
+assert c["capabilities"]["streaming"] is False, "streaming must be False"
+exts = c["capabilities"]["extensions"]
+assert any(e.get("required") and "observability" in e["uri"] for e in exts), "observability ext missing/required"
+ids = sorted(s["id"] for s in c["skills"])
+assert ids == ["ask", "describe", "improve", "review"], ids
+print("    card OK: name, streaming=False, observability required, skills", ids)
+PY

Evidence

The script’s PORT is used only for host mapping and the curl base URL, but it is not injected into
the container as AGENT_CARD_PORT or PORT. The agent card generation defaults to localhost and
port AGENT_CARD_PORT || PORT || 9000, so with no injected env it will advertise :9000 even when
the host mapping uses a different port.

docker/mosaico/smoke_test.sh[19-70]
pr_agent/mosaico/card.py[19-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`smoke_test.sh` allows overriding the host port via `PORT=...`, but it does not set `AGENT_CARD_HOST`/`AGENT_CARD_PORT` (unless the user happens to put them in `.env`) and does not validate `supportedInterfaces[0].url`. This can report a successful smoke test even though the card advertises an endpoint that doesn’t match the actual exposed host port.
## Issue Context
The agent card URL is computed from `AGENT_CARD_HOST` and `AGENT_CARD_PORT` (or `PORT`, or default 9000). If the container is started without those env vars, it will default to `localhost:9000` regardless of the host port mapping.
## Fix Focus Areas
- docker/mosaico/smoke_test.sh[19-70]
- pr_agent/mosaico/card.py[19-26]
### Suggested change
- When launching the container, set `AGENT_CARD_HOST=localhost` and `AGENT_CARD_PORT=$PORT` unless those keys are already provided in the `.env` file.
- Extend the Python card validation to assert `supportedInterfaces[0]['url'] == f'http://localhost:{PORT}/'` (or derived from `$BASE/`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Make docker/mosaico a self-contained deployment bundle (pinned published image)

✨ Enhancement ⚙️ Configuration changes 📝 Documentation 🧪 Tests 🕐 20-40 Minutes

AI Description

• Pin the MOSAICO solution-agent overlay to the published pragent/pr-agent mosaico_agent image.
• Fix demonstrator registration blockers (port collision, agent slug, host-reachable card URL).
• Refresh docs and add env template + smoke test aligned with A2A 1.0 wire contract.

Diagram

graph TD
  op(["Integrator / Operator"]) --> env[/".env / pr-agent.env.example"/] --> compose["Compose overlay"] --> sa["Solution-agent container"] --> endpoints["/card / /health / JSON-RPC"]
  template[/"pr-agent-solution-agent.json"/] --> reg["Registration container"] --> repo[("MOSAICO repository")]
  repo --> ref{{"Reference agent / clients"}} --> sa

  subgraph Legend
    direction LR
    _actor(["Actor"]) ~~~ _file[/"File"/] ~~~ _svc["Container/Service"] ~~~ _db[("Repository")] ~~~ _ext{{"External"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep building a local image in the demonstrator

• ➕ No dependency on DockerHub availability or tag publishing cadence
• ➕ Allows local patches without waiting for upstream releases
• ➖ Requires a pr-agent checkout in the deployment environment
• ➖ More complex upgrade and reproducibility story (build toolchain, cache, context leaks)

2. Mirror the pinned image into the GitLab registry

• ➕ Reduces reliance on DockerHub and improves supply-chain control for GitLab deployments
• ➕ Supports air-gapped or restricted egress environments
• ➖ Adds mirroring automation and governance (retention, provenance, scanning)
• ➖ Still requires tag bumps and validation, just against a different registry

Recommendation: Pinning a published mosaico_agent image is the right default for a self-contained deployment bundle: it removes the need for a source checkout/build and makes upgrades a one-line tag bump. If the target GitLab environment has registry/egress constraints, consider additionally mirroring the pinned image into the GitLab registry while keeping the same overlay semantics.

Files changed (7) +316 / -66

Tests (1) +115 / -0

smoke_test.shAdd smoke test for card validation and A2A 1.0 SendMessage round-trip +115/-0

Add smoke test for card validation and A2A 1.0 SendMessage round-trip

• Adds a script that pulls the pinned mosaico_agent image, boots it, and validates the agent card. When creds are present via a local .env, it also checks /health and performs an A2A 1.0 SendMessage call, asserting the completed task artifact contains a reply.

docker/mosaico/smoke_test.sh

Documentation (2) +152 / -56

LICENSEBundle MIT license for the deployment assets +21/-0

Bundle MIT license for the deployment assets

• Adds an MIT LICENSE file under docker/mosaico to make the deployment bundle self-contained and explicitly licensed for redistribution.

docker/mosaico/LICENSE

README.mdRewrite README for image-pinned, no-checkout deployment and A2A 1.0 +131/-56

Rewrite README for image-pinned, no-checkout deployment and A2A 1.0

• Reframes docker/mosaico as a self-contained deployment bundle that consumes a published pragent/pr-agent:<version>-mosaico_agent image. Documents the required env-var contract, correct card URL reachability, demonstrator integration steps, and troubleshooting; removes nonfunctional probe instructions.

docker/mosaico/README.md

Other (4) +49 / -10

.dockerignoreIgnore .env files in all Docker build contexts +1/-0

Ignore .env files in all Docker build contexts

• Adds a global **/.env ignore pattern to prevent local environment files from being copied into image layers during local builds (notably the docker/Dockerfile test stage). This reduces accidental credential leakage via build cache or pushed layers.

.dockerignore

docker-compose.pr-agent.ymlPin published mosaico_agent image and fix demonstrator registration wiring +10/-9

Pin published mosaico_agent image and fix demonstrator registration wiring

• Switches the solution-agent service to use pragent/pr-agent:0.40.0-mosaico_agent instead of a locally built image. Fixes a host-port collision by moving the default to 23000, sets AGENT_NAME to the required kebab-case slug, makes AGENT_CARD_URL host-reachable, and aligns the healthcheck with peer agents (/health).

docker/mosaico/docker-compose.pr-agent.yml

pr-agent-solution-agent.jsonUpdate registration template version to 1.0.0 +1/-1

Update registration template version to 1.0.0

• Bumps the template version field from 0.35.0 to 1.0.0 to match the peer registration JSONs and represent the contract rather than a specific build version.

docker/mosaico/pr-agent-solution-agent.json

pr-agent.env.exampleAdd sanitized env template for standalone and demonstrator overlay +37/-0

Add sanitized env template for standalone and demonstrator overlay

• Introduces a placeholder-only environment template covering LLM credentials, optional token budget/observability, and demonstrator overlay knobs. Calls out AGENT_CARD_HOST/PORT as critical to avoiding silent registration of an unreachable localhost card URL.

docker/mosaico/pr-agent.env.example

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Healthcheck needs PORT set 🐞 Bug ☼ Reliability

Description

The compose healthcheck probes http://localhost:$PORT/health but this overlay does not set PORT
and the mosaico_agent image also does not define it, so $PORT can expand empty and the service
never becomes healthy (blocking the registration container via `depends_on: condition:
service_healthy). Fixing this only addresses the port-expansion bug; /health` can still
intentionally return 503 when LLM creds/model aren’t configured.

Code

docker/mosaico/docker-compose.pr-agent.yml[R15-36]

    environment:
      AGENT_CARD_HOST: ${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}
-      AGENT_CARD_PORT: ${PR_AGENT_PORT:-22000}
+      AGENT_CARD_PORT: ${PR_AGENT_PORT:-23000}
      MODEL_NAME: ${PR_AGENT_MODEL}
-    # Gate registration on card/HTTP readiness, not /health (a live LLM probe).
+    # Matches the peer agents' probe verbatim. /health is a live LLM check, so no LLM
+    # means no registration - intended: a registered card with a dead LLM is worse.
    healthcheck:
-      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9000/.well-known/agent-card.json')\""]
+      test: ["CMD-SHELL", "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:$${PORT}/health')\""]

  pr-agent-solution-agent-registration:
    extends:
      file: base-definitions.yml
      service: agent-registration
    environment:
-      AGENT_NAME: PR-Agent Solution Agent
+      AGENT_NAME: pr-agent-solution-agent
      AGENT_JSON: /app/pr-agent-solution-agent.json
-      # Fetched container-to-container: use the service name and in-container port 9000.
-      AGENT_CARD_URL: http://pr-agent-solution-agent:9000/.well-known/agent-card.json
+      # Stored in the repository and fetched by out-of-Docker clients, so it must be
+      # host-reachable, not a compose service name.
+      AGENT_CARD_URL: http://${PR_AGENT_HOST:-${DEFAULT_TASK_AGENT_HOST}}:${PR_AGENT_PORT:-23000}/.well-known/agent-card.json
    depends_on:
      pr-agent-solution-agent:
        condition: service_healthy

Evidence

The overlay healthcheck expands $PORT but does not define it, while the image simply runs the
MOSAICO server without setting any PORT env; the server code defaults to 9000 when PORT is
absent, so the application can start but the shell-expanded healthcheck can still fail due to an
empty variable.

docker/mosaico/docker-compose.pr-agent.yml[12-37]
docker/Dockerfile[40-43]
pr_agent/mosaico/server.py[95-103]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The healthcheck command uses `$PORT` (`$${PORT}` in compose-escaped form), but neither the compose overlay nor the Docker image defines `PORT`. When `PORT` is unset, the healthcheck URL becomes invalid (`http://localhost:/health`), keeping the agent `unhealthy` and preventing the registration service from starting.

## Issue Context
- The server binds to `PORT` if set, otherwise defaults to `9000`.
- The healthcheck should not depend on `PORT` being injected by an external compose base.

## Fix Focus Areas
- docker/mosaico/docker-compose.pr-agent.yml[15-23]

### Suggested change
Either:
1) Add `PORT: 9000` to the service environment so `$PORT` is always present, **or**
2) Change the healthcheck URL to use a fixed `9000` or a shell fallback `${PORT:-9000}` (escaped appropriately for compose).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Smoke test ignores advertised URL 🐞 Bug ☼ Reliability

Description

The new smoke_test.sh maps host ${PORT} to container 9000 but does not ensure the container
advertises that mapped port in supportedInterfaces (and it never asserts the advertised URL), so
PORT=19000 ./smoke_test.sh can pass while the agent card still advertises
http://localhost:9000/.

Code

docker/mosaico/smoke_test.sh[R19-70]

+IMAGE="${IMAGE:-pragent/pr-agent:0.40.0-mosaico_agent}"
+PORT="${PORT:-9000}"
+CONTAINER="pr-agent-mosaico-test"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ENV_FILE="${ENV_FILE:-$SCRIPT_DIR/.env}"
+BASE="http://localhost:${PORT}"
+
+cleanup() { docker rm -f "$CONTAINER" >/dev/null 2>&1 || true; }
+trap cleanup EXIT
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+echo "==> Pulling $IMAGE"
+docker pull "$IMAGE" || fail "docker pull failed"
+
+run_args=(-d --rm --name "$CONTAINER" -p "${PORT}:9000")
+have_creds=0
+if [[ -f "$ENV_FILE" ]]; then
+  run_args+=(--env-file "$ENV_FILE")
+  # Full round-trip requires all three LLM keys to be present and non-empty.
+  if grep -qE '^API_BASE=.+' "$ENV_FILE" \
+     && grep -qE '^API_KEY=.+' "$ENV_FILE" \
+     && grep -qE '^MODEL_NAME=.+' "$ENV_FILE"; then
+    have_creds=1
+  fi
+fi
+
+mode=$([[ $have_creds == 1 ]] && echo "full round-trip" || echo "smoke only")
+echo "==> Starting container ($mode)"
+docker run "${run_args[@]}" "$IMAGE" || fail "docker run failed"
+
+# --- SMOKE: fetch the card (with retry; needs no LLM) and validate it ---
+echo "==> [smoke] fetching + validating agent card"
+CARD=""
+for _ in $(seq 1 30); do
+  CARD=$(curl -fsSL "$BASE/.well-known/agent-card.json" 2>/dev/null)
+  [[ -n "$CARD" ]] && break
+  sleep 2
+done
+[[ -n "$CARD" ]] || { docker logs "$CONTAINER" 2>&1 | tail -40; fail "card endpoint never served a body"; }
+
+CARD="$CARD" python3 - <<'PY' || fail "agent card invalid"
+import json, os
+c = json.loads(os.environ["CARD"])
+assert c["name"] == "PR-Agent Solution Agent", c.get("name")
+assert c["capabilities"]["streaming"] is False, "streaming must be False"
+exts = c["capabilities"]["extensions"]
+assert any(e.get("required") and "observability" in e["uri"] for e in exts), "observability ext missing/required"
+ids = sorted(s["id"] for s in c["skills"])
+assert ids == ["ask", "describe", "improve", "review"], ids
+print("    card OK: name, streaming=False, observability required, skills", ids)
+PY

Evidence

The script’s PORT is used only for host mapping and the curl base URL, but it is not injected into
the container as AGENT_CARD_PORT or PORT. The agent card generation defaults to localhost and
port AGENT_CARD_PORT || PORT || 9000, so with no injected env it will advertise :9000 even when
the host mapping uses a different port.

docker/mosaico/smoke_test.sh[19-70]
pr_agent/mosaico/card.py[19-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`smoke_test.sh` allows overriding the host port via `PORT=...`, but it does not set `AGENT_CARD_HOST`/`AGENT_CARD_PORT` (unless the user happens to put them in `.env`) and does not validate `supportedInterfaces[0].url`. This can report a successful smoke test even though the card advertises an endpoint that doesn’t match the actual exposed host port.

## Issue Context
The agent card URL is computed from `AGENT_CARD_HOST` and `AGENT_CARD_PORT` (or `PORT`, or default 9000). If the container is started without those env vars, it will default to `localhost:9000` regardless of the host port mapping.

## Fix Focus Areas
- docker/mosaico/smoke_test.sh[19-70]
- pr_agent/mosaico/card.py[19-26]

### Suggested change
- When launching the container, set `AGENT_CARD_HOST=localhost` and `AGENT_CARD_PORT=$PORT` unless those keys are already provided in the `.env` file.
- Extend the Python card validation to assert `supportedInterfaces[0]['url'] == f'http://localhost:{PORT}/'` (or derived from `$BASE/`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

[qodo-free-for-open-source-projects]

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 67174a5

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 3bdb302

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 4203da6

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 1b9540c