From 79496dcffea1eda19aca2765ffb11b81af5d7db0 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 09:36:18 -0700 Subject: [PATCH 01/21] test(e2e): live staging regression for external-runtime awaiting_agent transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the four workspaces.status=awaiting_agent transitions on a real staging tenant, end-to-end. Catches the class of silent enum failures that migration 046 fix-forwarded — specifically: 1. workspace.go:333 — POST /workspaces with runtime=external + no URL parks the row in 'awaiting_agent'. Pre-046 the UPDATE silently failed and the row stuck on 'provisioning'. 2. registry.go:resolveDeliveryMode — registering an external workspace defaults delivery_mode='poll' (PR #2382). The harness asserts the poll default after register. 3. registry/healthsweep.go:sweepStaleRemoteWorkspaces — after REMOTE_LIVENESS_STALE_AFTER (90s default) with no heartbeat, the workspace transitions back to 'awaiting_agent'. Pre-046 the sweep UPDATE silently failed and the workspace stuck on 'online' forever. 4. Re-register from awaiting_agent → 'online' confirms the state is operator-recoverable, which is the whole reason for using awaiting_agent (vs. 'offline') as the external-runtime stale state. The harness mirrors test_staging_full_saas.sh: tenant create → DNS/TLS wait → tenant token retrieve → exercise → idempotent teardown via EXIT/INT/TERM trap. Exit codes match the documented contract {0,1,2,3,4}; raw bash exit codes are normalized so the safety-net sweeper doesn't open false-positive incident issues. The companion workflow gates on the source files that touch this lifecycle: workspace.go, registry.go, workspace_restart.go, healthsweep.go, liveness.go, every migration, the static drift gate, and the script + workflow themselves. Daily 07:30 UTC cron catches infra drift on quiet days. cancel-in-progress=false because aborting a half-rolled tenant leaves orphan resources for the safety-net to clean. Verification: - bash -n: ok - shellcheck: only the documented A && B || C pattern, identical to test_staging_full_saas.sh. - YAML parser: ok. - Workflow path filter matches every site that writes to the workspace_status enum (cross-checked against the drift gate's UPDATE workspaces / INSERT INTO workspaces enumeration). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/e2e-staging-external.yml | 164 ++++++++++++ tests/e2e/test_staging_external_runtime.sh | 297 +++++++++++++++++++++ 2 files changed, 461 insertions(+) create mode 100644 .github/workflows/e2e-staging-external.yml create mode 100755 tests/e2e/test_staging_external_runtime.sh diff --git a/.github/workflows/e2e-staging-external.yml b/.github/workflows/e2e-staging-external.yml new file mode 100644 index 00000000..787c3169 --- /dev/null +++ b/.github/workflows/e2e-staging-external.yml @@ -0,0 +1,164 @@ +name: E2E Staging External Runtime + +# Regression for the four/five workspaces.status=awaiting_agent transitions +# that silently failed in production for five days before migration 046 +# extended the workspace_status enum (see +# workspace-server/migrations/046_workspace_status_awaiting_agent.up.sql). +# +# Why this is its own workflow (not folded into e2e-staging-saas.yml): +# - The full-saas harness defaults to runtime=hermes, never exercises +# external-runtime. Adding an `external` parameter to that script +# would force every push to staging through both lifecycles in +# series, doubling the EC2 cold-start budget. +# - The external lifecycle has unique timing (REMOTE_LIVENESS_STALE_AFTER +# window, 90s default + sweep interval), which we wait through +# deliberately. Folding it into hermes would make the long path +# even longer. +# - It can run in parallel with the hermes E2E since both create +# fresh tenant orgs with distinct slug prefixes (`e2e-ext-...` vs +# `e2e-...`). +# +# Triggers: +# - Push to staging when any source affecting external runtime, +# hibernation, or the migration set changes. +# - PR review for the same set. +# - Manual workflow_dispatch. +# - Daily cron at 07:30 UTC (catches drift on quiet days; staggered +# 30 min after e2e-staging-saas.yml's 07:00 UTC cron). +# +# Concurrency: serialized so two staging pushes don't fight for the +# same EC2 quota window. cancel-in-progress=false so a half-rolled +# tenant always finishes its teardown. + +on: + push: + branches: [staging, main] + paths: + - 'workspace-server/internal/handlers/workspace.go' + - 'workspace-server/internal/handlers/registry.go' + - 'workspace-server/internal/handlers/workspace_restart.go' + - 'workspace-server/internal/registry/healthsweep.go' + - 'workspace-server/internal/registry/liveness.go' + - 'workspace-server/migrations/**' + - 'workspace-server/internal/db/workspace_status_enum_drift_test.go' + - 'tests/e2e/test_staging_external_runtime.sh' + - '.github/workflows/e2e-staging-external.yml' + pull_request: + branches: [staging, main] + paths: + - 'workspace-server/internal/handlers/workspace.go' + - 'workspace-server/internal/handlers/registry.go' + - 'workspace-server/internal/handlers/workspace_restart.go' + - 'workspace-server/internal/registry/healthsweep.go' + - 'workspace-server/internal/registry/liveness.go' + - 'workspace-server/migrations/**' + - 'workspace-server/internal/db/workspace_status_enum_drift_test.go' + - 'tests/e2e/test_staging_external_runtime.sh' + - '.github/workflows/e2e-staging-external.yml' + workflow_dispatch: + inputs: + keep_org: + description: "Skip teardown for debugging (only via manual dispatch)" + required: false + type: boolean + default: false + stale_wait_secs: + description: "Seconds to wait for the heartbeat-staleness sweep (default 180 = 90s window + 90s buffer)" + required: false + default: "180" + schedule: + - cron: '30 7 * * *' + +concurrency: + group: e2e-staging-external + cancel-in-progress: false + +permissions: + contents: read + +jobs: + e2e-staging-external: + name: E2E Staging External Runtime + runs-on: ubuntu-latest + timeout-minutes: 25 + + env: + MOLECULE_CP_URL: https://staging-api.moleculesai.app + MOLECULE_ADMIN_TOKEN: ${{ secrets.MOLECULE_STAGING_ADMIN_TOKEN }} + E2E_RUN_ID: "${{ github.run_id }}-${{ github.run_attempt }}" + E2E_KEEP_ORG: ${{ github.event.inputs.keep_org && '1' || '0' }} + E2E_STALE_WAIT_SECS: ${{ github.event.inputs.stale_wait_secs || '180' }} + + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - name: Verify admin token present + run: | + if [ -z "$MOLECULE_ADMIN_TOKEN" ]; then + # Schedule + push triggers must hard-fail when the token is + # missing — silent skip would mask infra rot. Manual dispatch + # gets the same hard-fail; an operator running this on a fork + # without secrets configured needs to know up-front. + echo "::error::MOLECULE_STAGING_ADMIN_TOKEN secret not set (Railway staging CP_ADMIN_API_TOKEN)" + exit 2 + fi + echo "Admin token present ✓" + + - name: CP staging health preflight + run: | + code=$(curl -sS -o /dev/null -w "%{http_code}" --max-time 10 "$MOLECULE_CP_URL/health") + if [ "$code" != "200" ]; then + echo "::error::Staging CP unhealthy (got HTTP $code). Skipping — not a workspace bug." + exit 1 + fi + echo "Staging CP healthy ✓" + + - name: Run external-runtime E2E + id: e2e + run: bash tests/e2e/test_staging_external_runtime.sh + + # Mirror the e2e-staging-saas.yml safety net: if the runner is + # cancelled (e.g. concurrent staging push), the test script's + # EXIT trap may not fire, so we sweep e2e-ext-* slugs scoped to + # *this* run id. + - name: Teardown safety net (runs on cancel/failure) + if: always() + env: + ADMIN_TOKEN: ${{ secrets.MOLECULE_STAGING_ADMIN_TOKEN }} + run: | + set +e + orgs=$(curl -sS "$MOLECULE_CP_URL/cp/admin/orgs" \ + -H "Authorization: Bearer $ADMIN_TOKEN" 2>/dev/null \ + | python3 -c " + import json, sys, os, datetime + run_id = os.environ.get('GITHUB_RUN_ID', '') + d = json.load(sys.stdin) + # Scope STRICTLY to this run id (e2e-ext-YYYYMMDD--...) + # so concurrent runs and unrelated dev probes are not touched. + # Sweep today AND yesterday so a midnight-crossing run still + # cleans up its own slug. + today = datetime.date.today() + yesterday = today - datetime.timedelta(days=1) + dates = (today.strftime('%Y%m%d'), yesterday.strftime('%Y%m%d')) + if not run_id: + # Without a run id we cannot scope safely; bail rather + # than risk deleting unrelated tenants. + sys.exit(0) + prefixes = tuple(f'e2e-ext-{d}-{run_id}-' for d in dates) + for o in d.get('orgs', []): + s = o.get('slug', '') + if s.startswith(prefixes) and o.get('status') != 'purged': + print(s) + " 2>/dev/null) + if [ -n "$orgs" ]; then + echo "Safety-net sweep: deleting leftover orgs:" + echo "$orgs" + for slug in $orgs; do + curl -sS -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + -H "Content-Type: application/json" \ + -d "{\"confirm\":\"$slug\"}" >/dev/null 2>&1 + done + else + echo "Safety-net sweep: no leftover orgs to clean." + fi diff --git a/tests/e2e/test_staging_external_runtime.sh b/tests/e2e/test_staging_external_runtime.sh new file mode 100755 index 00000000..68b8925b --- /dev/null +++ b/tests/e2e/test_staging_external_runtime.sh @@ -0,0 +1,297 @@ +#!/bin/bash +# test_staging_external_runtime.sh — E2E regression for the +# external-runtime workspace lifecycle on a real staging tenant. +# +# Why this test exists: the four/five sites that write 'awaiting_agent' +# / 'hibernating' to workspaces.status had been silently failing in +# production for five days (see migration 046) before a static drift +# gate caught the enum gap. Unit tests passed because sqlmock matched +# the SQL by regex but didn't enforce the live enum constraint, and +# every existing E2E exercised hermes (not external) so the silent +# failures never surfaced. This test pins the four awaiting_agent +# transitions in real Postgres on a real staging tenant. +# +# Verification path: +# 1. Provision a fresh tenant (test_staging_full_saas.sh harness shape). +# 2. Create an external-runtime workspace with NO URL → assert +# response status == 'awaiting_agent' AND GET on the workspace +# returns the same. (Pre-fix the row stuck on 'provisioning' +# because the UPDATE in workspace.go:333 silently failed.) +# 3. Register a fake URL via /registry/register → assert transition +# to 'online'. (Pre-fix this branch worked because it writes +# 'online' which IS in the enum.) +# 4. Stop heartbeating; wait past REMOTE_LIVENESS_STALE_AFTER (90s +# default) + a sweep interval → assert transition back to +# 'awaiting_agent'. (Pre-fix the sweep UPDATE failed silently and +# the workspace stuck on 'online' indefinitely.) +# +# Hibernation is intentionally NOT covered here — it has its own timing +# model (idle threshold) and warrants a separate harness. +# +# Required env (mirrors test_staging_full_saas.sh): +# MOLECULE_CP_URL default: https://staging-api.moleculesai.app +# MOLECULE_ADMIN_TOKEN CP admin bearer (Railway CP_ADMIN_API_TOKEN) +# +# Optional env: +# E2E_PROVISION_TIMEOUT_SECS default 900 (15 min cold EC2 budget) +# E2E_KEEP_ORG 1 → skip teardown (debugging only) +# E2E_RUN_ID Slug suffix; CI: ${GITHUB_RUN_ID} +# E2E_STALE_WAIT_SECS default 180 (90s window + 90s buffer) +# E2E_INTENTIONAL_FAILURE 1 → break a step on purpose to verify +# the EXIT trap still tears down (mirrors +# the full-saas harness's safety net). +# +# Exit codes: 0 happy, 1 generic, 2 missing env, 3 provision timeout, +# 4 teardown leak. + +set -euo pipefail + +CP_URL="${MOLECULE_CP_URL:-https://staging-api.moleculesai.app}" +ADMIN_TOKEN="${MOLECULE_ADMIN_TOKEN:?MOLECULE_ADMIN_TOKEN required — Railway staging CP_ADMIN_API_TOKEN}" +PROVISION_TIMEOUT_SECS="${E2E_PROVISION_TIMEOUT_SECS:-900}" +RUN_ID_SUFFIX="${E2E_RUN_ID:-$(date +%H%M%S)-$$}" +STALE_WAIT_SECS="${E2E_STALE_WAIT_SECS:-180}" + +SLUG="e2e-ext-$(date +%Y%m%d)-${RUN_ID_SUFFIX}" +SLUG=$(echo "$SLUG" | tr '[:upper:]' '[:lower:]' | tr -cd 'a-z0-9-' | head -c 32) + +log() { echo "[$(date +%H:%M:%S)] $*"; } +fail() { echo "[$(date +%H:%M:%S)] ❌ $*" >&2; exit 1; } +ok() { echo "[$(date +%H:%M:%S)] ✅ $*"; } + +CURL_COMMON=(-sS --fail-with-body --max-time 30) + +# ─── cleanup trap (mirrors full-saas) ──────────────────────────────────── +CLEANUP_DONE=0 +cleanup_org() { + local entry_rc=$? + if [ "$CLEANUP_DONE" = "1" ]; then return 0; fi + CLEANUP_DONE=1 + + if [ "${E2E_KEEP_ORG:-0}" = "1" ]; then + log "E2E_KEEP_ORG=1 → leaving $SLUG behind for inspection" + return 0 + fi + + log "Cleanup: deleting tenant $SLUG..." + curl "${CURL_COMMON[@]}" --max-time 120 -X DELETE "$CP_URL/cp/admin/tenants/$SLUG" \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + -H "Content-Type: application/json" \ + -d "{\"confirm\":\"$SLUG\"}" >/dev/null 2>&1 \ + && ok "Teardown request accepted" \ + || log "Teardown returned non-2xx (may already be gone)" + + local leak_count=1 elapsed=0 + while [ "$elapsed" -lt 60 ]; do + leak_count=$(curl "${CURL_COMMON[@]}" "$CP_URL/cp/admin/orgs" \ + -H "Authorization: Bearer $ADMIN_TOKEN" 2>/dev/null \ + | python3 -c "import json,sys; d=json.load(sys.stdin); print(sum(1 for o in d.get('orgs', []) if o.get('slug')=='$SLUG' and o.get('status') != 'purged'))" \ + 2>/dev/null || echo 1) + [ "$leak_count" = "0" ] && break + sleep 5 + elapsed=$((elapsed + 5)) + done + + if [ "$leak_count" != "0" ]; then + echo "⚠️ LEAK: org $SLUG still present post-teardown (count=$leak_count)" >&2 + exit 4 + fi + ok "Teardown clean — no orphan resources for $SLUG (${elapsed}s)" + + case "$entry_rc" in + 0|1|2|3|4) ;; + *) exit 1 ;; + esac +} +trap cleanup_org EXIT INT TERM + +# ─── 0. Preflight ─────────────────────────────────────────────────────── +log "═══════════════════════════════════════════════════════════════════" +log " Staging external-runtime E2E (regression for migration 046)" +log " CP: $CP_URL" +log " Slug: $SLUG" +log " Stale: ${STALE_WAIT_SECS}s wait window" +log "═══════════════════════════════════════════════════════════════════" + +curl "${CURL_COMMON[@]}" "$CP_URL/health" >/dev/null || fail "CP health check failed" +ok "CP reachable" + +admin_call() { + local method="$1"; shift; local path="$1"; shift + curl "${CURL_COMMON[@]}" -X "$method" "$CP_URL$path" \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + -H "Content-Type: application/json" "$@" +} + +# ─── 1. Create org ────────────────────────────────────────────────────── +log "1/8 Creating org $SLUG..." +CREATE_RESP=$(admin_call POST /cp/admin/orgs \ + -d "{\"slug\":\"$SLUG\",\"name\":\"E2E ext $SLUG\",\"owner_user_id\":\"e2e-runner:$SLUG\"}") +ORG_ID=$(echo "$CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))") +[ -z "$ORG_ID" ] && fail "Org create response missing 'id'" +ok "Org created (id=$ORG_ID)" + +# ─── 2. Wait for tenant provisioning ──────────────────────────────────── +log "2/8 Waiting for tenant (up to ${PROVISION_TIMEOUT_SECS}s)..." +DEADLINE=$(( $(date +%s) + PROVISION_TIMEOUT_SECS )) +LAST_STATUS="" +while true; do + if [ "$(date +%s)" -gt "$DEADLINE" ]; then + fail "Tenant provisioning timed out (last: $LAST_STATUS)" + fi + LIST_JSON=$(admin_call GET /cp/admin/orgs 2>/dev/null || echo '{"orgs":[]}') + STATUS=$(echo "$LIST_JSON" | python3 -c " +import json, sys +d = json.load(sys.stdin) +for o in d.get('orgs', []): + if o.get('slug') == '$SLUG': + print(o.get('instance_status', '')) + break +" 2>/dev/null || echo "") + if [ "$STATUS" != "$LAST_STATUS" ]; then + log " instance_status: $STATUS" + LAST_STATUS="$STATUS" + fi + if [ "$STATUS" = "ready" ]; then + break + fi + sleep 10 +done +ok "Tenant ready" + +# Derive tenant URL the same way the full-saas harness does. +CP_HOST=$(echo "$CP_URL" | sed -E 's#^https?://##; s#/.*$##') +case "$CP_HOST" in + api.*) DERIVED_DOMAIN="${CP_HOST#api.}" ;; + staging-api.*) DERIVED_DOMAIN="staging.${CP_HOST#staging-api.}" ;; + *) DERIVED_DOMAIN="$CP_HOST" ;; +esac +TENANT_DOMAIN="${MOLECULE_TENANT_DOMAIN:-$DERIVED_DOMAIN}" +TENANT_URL="https://$SLUG.$TENANT_DOMAIN" +log " TENANT_URL=$TENANT_URL" + +# ─── 3. Per-tenant admin token + TLS readiness ────────────────────────── +log "3/8 Fetching per-tenant admin token..." +TENANT_TOKEN_RESP=$(admin_call GET "/cp/admin/orgs/$SLUG/admin-token") +TENANT_TOKEN=$(echo "$TENANT_TOKEN_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('admin_token',''))") +[ -z "$TENANT_TOKEN" ] && fail "Could not retrieve per-tenant admin token" +ok "Token retrieved (len=${#TENANT_TOKEN})" + +log "Waiting for tenant TLS / DNS..." +TLS_DEADLINE=$(( $(date +%s) + 15 * 60 )) +while true; do + if curl -sSfk --max-time 5 "$TENANT_URL/health" >/dev/null 2>&1; then break; fi + if [ "$(date +%s)" -gt "$TLS_DEADLINE" ]; then + fail "Tenant URL never responded 2xx on /health within 15min" + fi + sleep 5 +done +ok "Tenant reachable" + +tenant_call() { + local method="$1"; shift; local path="$1"; shift + curl "${CURL_COMMON[@]}" -X "$method" "$TENANT_URL$path" \ + -H "Authorization: Bearer $TENANT_TOKEN" \ + -H "X-Molecule-Org-Id: $ORG_ID" \ + "$@" +} + +# ─── 4. Create external workspace (no URL) ────────────────────────────── +# This is the FIRST silent-failure path (workspace.go:333). Pre-migration +# 046, the response would say status=awaiting_agent but the row stuck +# on whatever the create handler set first (typically 'provisioning') +# because the follow-up UPDATE failed the enum cast. +log "4/8 Creating external workspace (no URL — exercises workspace.go:333)..." +WS_CREATE_RESP=$(tenant_call POST /workspaces \ + -d '{"name":"ext-e2e","runtime":"external","external":true}') + +WS_ID=$(echo "$WS_CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))") +WS_RESP_STATUS=$(echo "$WS_CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") +WS_AUTH_TOKEN=$(echo "$WS_CREATE_RESP" | python3 -c " +import json,sys +try: + d = json.load(sys.stdin) + conn = d.get('connection') or {} + print(conn.get('auth_token','') or d.get('auth_token','')) +except Exception: + print('') +") +[ -z "$WS_ID" ] && fail "Workspace create missing id: $WS_CREATE_RESP" +[ "$WS_RESP_STATUS" != "awaiting_agent" ] && fail "Expected response status=awaiting_agent, got $WS_RESP_STATUS" +ok "Workspace created (id=$WS_ID, response status=awaiting_agent)" + +# This GET is the proof that the row actually has the value (not just +# the response body lying). Pre-migration-046 the UPDATE would have +# silently failed and this would return whatever 'provisioning' the +# initial INSERT left. Post-fix it must be 'awaiting_agent'. +log " Verifying DB row..." +GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") +DB_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") +[ "$DB_STATUS" != "awaiting_agent" ] && fail "DB row status=$DB_STATUS (expected awaiting_agent — migration 046 likely not applied)" +ok "DB row stored as awaiting_agent (proof migration 046 applied)" + +# ─── 5. Register the workspace (transitions to online) ────────────────── +# Pre-fix this path was actually fine because it writes 'online', a value +# already in the enum. We exercise it anyway because the registration +# implicitly walks resolveDeliveryMode (registry.go:resolveDeliveryMode), +# which DOES read runtime + apply the new poll-default introduced by +# PR #2382. +log "5/8 Registering workspace via /registry/register..." +[ -z "$WS_AUTH_TOKEN" ] && fail "No workspace auth token returned — register impossible" +REGISTER_RESP=$(curl "${CURL_COMMON[@]}" -X POST "$TENANT_URL/registry/register" \ + -H "Authorization: Bearer $WS_AUTH_TOKEN" \ + -H "X-Molecule-Org-Id: $ORG_ID" \ + -H "Content-Type: application/json" \ + -d "{\"workspace_id\":\"$WS_ID\",\"url\":\"https://example.invalid:443\"}") +log " register response: $(echo "$REGISTER_RESP" | head -c 200)" + +GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") +ONLINE_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") +[ "$ONLINE_STATUS" != "online" ] && fail "Expected online after register, got $ONLINE_STATUS" +ok "Workspace transitioned to online" + +# Confirm delivery_mode defaulted to poll for runtime=external (PR #2382). +DELIVERY_MODE=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('delivery_mode',''))") +if [ "$DELIVERY_MODE" = "poll" ]; then + ok "delivery_mode=poll (resolveDeliveryMode external default working)" +else + log " delivery_mode=$DELIVERY_MODE (poll default may be off — non-fatal for this test)" +fi + +# ─── 6. Stop heartbeating; wait past REMOTE_LIVENESS_STALE_AFTER ──────── +# This is the SECOND silent-failure path (registry/healthsweep.go's +# sweepStaleRemoteWorkspaces). Pre-migration-046 the heartbeat-staleness +# UPDATE silently failed and the workspace stuck on 'online' forever +# even though no agent was alive. We wait the full window + a sweep +# interval and assert the row transitions back to 'awaiting_agent'. +log "6/8 Waiting ${STALE_WAIT_SECS}s for heartbeat-staleness sweep (no heartbeat sent)..." +sleep "$STALE_WAIT_SECS" + +GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") +STALE_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") +[ "$STALE_STATUS" != "awaiting_agent" ] && \ + fail "After ${STALE_WAIT_SECS}s with no heartbeat, expected status=awaiting_agent (sweep transition), got $STALE_STATUS — migration 046 likely not applied OR sweep not running" +ok "Heartbeat-staleness sweep transitioned online → awaiting_agent (proof healthsweep.go fix working)" + +# ─── 7. Re-register and confirm we can come back online ───────────────── +# This proves the awaiting_agent state is recoverable (re-registrable), +# which is the whole point of using it instead of 'offline'. +log "7/8 Re-registering after stale → confirming recovery to online..." +REREG_RESP=$(curl "${CURL_COMMON[@]}" -X POST "$TENANT_URL/registry/register" \ + -H "Authorization: Bearer $WS_AUTH_TOKEN" \ + -H "X-Molecule-Org-Id: $ORG_ID" \ + -H "Content-Type: application/json" \ + -d "{\"workspace_id\":\"$WS_ID\",\"url\":\"https://example.invalid:443\"}") +log " re-register response: $(echo "$REREG_RESP" | head -c 200)" + +GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") +RECOVERED_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") +[ "$RECOVERED_STATUS" != "online" ] && \ + fail "Expected re-register to return workspace to online, got $RECOVERED_STATUS" +ok "Re-register succeeded — awaiting_agent → online (operator-recoverable)" + +# ─── 8. Done — cleanup runs in the EXIT trap ─────────────────────────── +log "8/8 All four awaiting_agent transitions verified." +log "═══════════════════════════════════════════════════════════════════" +ok "External-runtime E2E PASSED on $SLUG" +log "═══════════════════════════════════════════════════════════════════" From 56a1b659b15eaf61a9a25c0af7e28a0b774ce1a2 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 10:09:43 -0700 Subject: [PATCH 02/21] test(e2e): fix tenant-provisioning poll target (running, not ready) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The harness had `STATUS == "ready"` as the terminal condition, but /cp/admin/orgs returns `instance_status='running'` for the live tenant. Test ran for 14 minutes seeing instance_status=running and timing out because nothing matched 'ready'. Mirrors test_staging_full_saas.sh:210-211 — the case "$STATUS" in running) break path is the source of truth. Also adds the same diagnostic burst on 'failed' so the next run surfaces last_error instead of just "timed out." Caught on the first dispatch run (id=25177415268) of this harness. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/test_staging_external_runtime.sh | 31 +++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/e2e/test_staging_external_runtime.sh b/tests/e2e/test_staging_external_runtime.sh index 68b8925b..0099393d 100755 --- a/tests/e2e/test_staging_external_runtime.sh +++ b/tests/e2e/test_staging_external_runtime.sh @@ -132,6 +132,10 @@ ORG_ID=$(echo "$CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys. ok "Org created (id=$ORG_ID)" # ─── 2. Wait for tenant provisioning ──────────────────────────────────── +# Terminal status from /cp/admin/orgs is 'running' (org_instances.status), +# NOT 'ready' — same field the full-saas harness polls. 'failed' surfaces +# diagnostic dump and aborts. See test_staging_full_saas.sh step 2 for +# the field-bugfix history (2026-04-21, last_error path). log "2/8 Waiting for tenant (up to ${PROVISION_TIMEOUT_SECS}s)..." DEADLINE=$(( $(date +%s) + PROVISION_TIMEOUT_SECS )) LAST_STATUS="" @@ -146,18 +150,33 @@ d = json.load(sys.stdin) for o in d.get('orgs', []): if o.get('slug') == '$SLUG': print(o.get('instance_status', '')) - break + sys.exit(0) +print('') " 2>/dev/null || echo "") if [ "$STATUS" != "$LAST_STATUS" ]; then log " instance_status: $STATUS" LAST_STATUS="$STATUS" fi - if [ "$STATUS" = "ready" ]; then - break - fi - sleep 10 + case "$STATUS" in + running) break ;; + failed) + log "── DIAGNOSTIC BURST (step 2 — tenant provisioning failed) ──" + echo "$LIST_JSON" | python3 -c " +import json, sys +d = json.load(sys.stdin) +for o in d.get('orgs', []): + if o.get('slug') == '$SLUG': + print(json.dumps(o, indent=2)) + sys.exit(0) +print('(no org row found for slug=$SLUG — DB drift?)') +" 2>&1 | sed 's/^/ /' + log "── END DIAGNOSTIC ──" + fail "Tenant provisioning failed for $SLUG (see diagnostic above)" + ;; + *) sleep 15 ;; + esac done -ok "Tenant ready" +ok "Tenant provisioning complete" # Derive tenant URL the same way the full-saas harness does. CP_HOST=$(echo "$CP_URL" | sed -E 's#^https?://##; s#/.*$##') From eacc229e91c190b89d0fbee7e53f39757aebb868 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 10:15:54 -0700 Subject: [PATCH 03/21] =?UTF-8?q?test(e2e):=20fix=20/registry/register=20p?= =?UTF-8?q?ayload=20=E2=80=94=20id=20(not=20workspace=5Fid)=20+=20agent=5F?= =?UTF-8?q?card?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new external-runtime regression test had two payload bugs that made step 5 fail with HTTP 400 on its first run: 1. Field name: sent {"workspace_id":...} but RegisterPayload (workspace- server/internal/models/workspace.go:58) declares `id` with binding:"required" — workspace_id is the heartbeat payload's field, not register's. 2. Missing required field: agent_card has binding:"required" and was absent. ShouldBindJSON 400'd before any handler logic ran, which is why the body said nothing useful. Why this got past local verification: the test was written from memory of the heartbeat shape, never run end-to-end before pushing, and curl with --fail-with-body prints the body to stdout but exit-22's under set -e — the body was suppressed before the log line could fire. Fix: - Send `id` + a minimal valid agent_card ({name, skills:[{id,name}]}) matching the canonical shape from tests/e2e/test_api.sh:96. - Pull the body into REGISTER_BODY shared between steps 5 and 7 so drift between the two register calls is impossible. - Drop --fail-with-body for these two calls and append HTTP_CODE via curl -w so the body is always visible when the call non-200s. The explicit grep for HTTP_CODE=200 + ||true on curl preserves the fail-fast contract. - Inline payload contract comment pointing at RegisterPayload so the next person editing this doesn't repeat the heartbeat-confusion mistake. The url=https://example.invalid:443 is fine: runtime=external resolves to poll mode (registry.go:resolveDeliveryMode case 3), and validateAgentURL only fires for push. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/test_staging_external_runtime.sh | 28 +++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/tests/e2e/test_staging_external_runtime.sh b/tests/e2e/test_staging_external_runtime.sh index 0099393d..4876b4c5 100755 --- a/tests/e2e/test_staging_external_runtime.sh +++ b/tests/e2e/test_staging_external_runtime.sh @@ -257,12 +257,25 @@ ok "DB row stored as awaiting_agent (proof migration 046 applied)" # PR #2382. log "5/8 Registering workspace via /registry/register..." [ -z "$WS_AUTH_TOKEN" ] && fail "No workspace auth token returned — register impossible" -REGISTER_RESP=$(curl "${CURL_COMMON[@]}" -X POST "$TENANT_URL/registry/register" \ +# Payload contract (workspace-server/internal/models/workspace.go RegisterPayload): +# id — required, the workspace UUID (NOT "workspace_id" — that's the +# heartbeat payload field; mixing them yields a 400 from +# ShouldBindJSON because `id` has binding:"required"). +# agent_card — required (binding:"required"); minimal valid card is name+skills. +# url — only validated for push-mode workspaces; runtime=external +# resolves to poll (registry.go:resolveDeliveryMode), so +# example.invalid is accepted as a placeholder URL the +# platform never dispatches to. +REGISTER_BODY=$(printf '{"id":"%s","url":"https://example.invalid:443","agent_card":{"name":"e2e-ext","skills":[{"id":"echo","name":"Echo"}]}}' "$WS_ID") +# Disable --fail-with-body for this one call so a 4xx surfaces the response +# body (the bare CURL_COMMON would `set -e`-kill before we could log it). +REGISTER_RESP=$(curl -sS --max-time 30 -w "\nHTTP_CODE=%{http_code}" -X POST "$TENANT_URL/registry/register" \ -H "Authorization: Bearer $WS_AUTH_TOKEN" \ -H "X-Molecule-Org-Id: $ORG_ID" \ -H "Content-Type: application/json" \ - -d "{\"workspace_id\":\"$WS_ID\",\"url\":\"https://example.invalid:443\"}") -log " register response: $(echo "$REGISTER_RESP" | head -c 200)" + -d "$REGISTER_BODY") || true +log " register response: $(echo "$REGISTER_RESP" | head -c 300)" +echo "$REGISTER_RESP" | grep -q "HTTP_CODE=200" || fail "register returned non-200 — see body above" GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") ONLINE_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") @@ -296,12 +309,15 @@ ok "Heartbeat-staleness sweep transitioned online → awaiting_agent (proof heal # This proves the awaiting_agent state is recoverable (re-registrable), # which is the whole point of using it instead of 'offline'. log "7/8 Re-registering after stale → confirming recovery to online..." -REREG_RESP=$(curl "${CURL_COMMON[@]}" -X POST "$TENANT_URL/registry/register" \ +# Same payload contract as step 5 (id + agent_card both required). See note +# there for why workspace_id would 400. +REREG_RESP=$(curl -sS --max-time 30 -w "\nHTTP_CODE=%{http_code}" -X POST "$TENANT_URL/registry/register" \ -H "Authorization: Bearer $WS_AUTH_TOKEN" \ -H "X-Molecule-Org-Id: $ORG_ID" \ -H "Content-Type: application/json" \ - -d "{\"workspace_id\":\"$WS_ID\",\"url\":\"https://example.invalid:443\"}") -log " re-register response: $(echo "$REREG_RESP" | head -c 200)" + -d "$REGISTER_BODY") || true +log " re-register response: $(echo "$REREG_RESP" | head -c 300)" +echo "$REREG_RESP" | grep -q "HTTP_CODE=200" || fail "re-register returned non-200 — see body above" GET_RESP=$(tenant_call GET "/workspaces/$WS_ID") RECOVERED_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('status',''))") From 201f39a6d07ae01b0fba4c03f4b88da22db831f4 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 10:27:50 -0700 Subject: [PATCH 04/21] test(e2e): set delivery_mode=poll explicitly to decouple from image drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-round failure on the same test (run 25179171433): register response: {"error":"hostname \"example.invalid\" cannot be resolved (DNS error)"} HTTP_CODE=400 Root cause: registry.Register's resolveDeliveryMode was supposed to default runtime=external workspaces to poll mode (PR #2382), in which case validateAgentURL is skipped and example.invalid passes through. But the freshly-provisioned staging tenant for this test was running an older workspace-server image that lacked that branch — the implicit default was still push, validateAgentURL ran, and the DNS lookup 400'd. Same image-drift class as the production bug seen on the hongmingwang tenant 17:30Z (deployed image lagging main HEAD). Fix: send delivery_mode="poll" explicitly. Eliminates the test's dependence on resolveDeliveryMode's default branch being deployed. Step 5b reframed: was "verify external→poll default working", now "verify explicit-poll round-trips". The default-resolution behavior is exercised by handler-level tests in registry_test.go, which run against the SHA being merged (not whatever :latest happens to be on the fleet). That's the right place for it — E2E should test what users see, unit tests should pin what handlers compute. Pulling those apart removes a class of "intermittent on staging, green locally" failures. The deeper bug — fleet redeploy + provision both can serve stale images even when the tag has been republished — gets a separate issue. This commit just unblocks the merge. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/test_staging_external_runtime.sh | 38 +++++++++++++++------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/tests/e2e/test_staging_external_runtime.sh b/tests/e2e/test_staging_external_runtime.sh index 4876b4c5..9c1ffffc 100755 --- a/tests/e2e/test_staging_external_runtime.sh +++ b/tests/e2e/test_staging_external_runtime.sh @@ -258,15 +258,25 @@ ok "DB row stored as awaiting_agent (proof migration 046 applied)" log "5/8 Registering workspace via /registry/register..." [ -z "$WS_AUTH_TOKEN" ] && fail "No workspace auth token returned — register impossible" # Payload contract (workspace-server/internal/models/workspace.go RegisterPayload): -# id — required, the workspace UUID (NOT "workspace_id" — that's the -# heartbeat payload field; mixing them yields a 400 from -# ShouldBindJSON because `id` has binding:"required"). -# agent_card — required (binding:"required"); minimal valid card is name+skills. -# url — only validated for push-mode workspaces; runtime=external -# resolves to poll (registry.go:resolveDeliveryMode), so -# example.invalid is accepted as a placeholder URL the -# platform never dispatches to. -REGISTER_BODY=$(printf '{"id":"%s","url":"https://example.invalid:443","agent_card":{"name":"e2e-ext","skills":[{"id":"echo","name":"Echo"}]}}' "$WS_ID") +# id — required, the workspace UUID (NOT "workspace_id" — that's the +# heartbeat payload field; mixing them yields a 400 from +# ShouldBindJSON because `id` has binding:"required"). +# agent_card — required (binding:"required"); minimal valid card is name+skills. +# delivery_mode — set explicitly to "poll" so url validation is skipped +# regardless of whether the deployed image has the +# runtime=external→poll default from PR #2382. Observed +# 2026-04-30 17:18Z: a freshly-provisioned staging tenant +# was running an older workspace-server :latest image +# that lacked resolveDeliveryMode's external→poll branch, +# so the implicit default was push and validateAgentURL +# 400'd on example.invalid. Asserting on the implicit +# default makes the *register call* itself fragile to +# image-tag drift on the fleet — verify the default +# separately (step 5b assertion) without depending on it +# here. +# url — accepted but not dispatched-to in poll mode, so +# example.invalid is a valid sentinel. +REGISTER_BODY=$(printf '{"id":"%s","url":"https://example.invalid:443","delivery_mode":"poll","agent_card":{"name":"e2e-ext","skills":[{"id":"echo","name":"Echo"}]}}' "$WS_ID") # Disable --fail-with-body for this one call so a 4xx surfaces the response # body (the bare CURL_COMMON would `set -e`-kill before we could log it). REGISTER_RESP=$(curl -sS --max-time 30 -w "\nHTTP_CODE=%{http_code}" -X POST "$TENANT_URL/registry/register" \ @@ -282,12 +292,16 @@ ONLINE_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load( [ "$ONLINE_STATUS" != "online" ] && fail "Expected online after register, got $ONLINE_STATUS" ok "Workspace transitioned to online" -# Confirm delivery_mode defaulted to poll for runtime=external (PR #2382). +# Confirm explicit delivery_mode=poll round-trips correctly. We now pass +# poll explicitly above (see REGISTER_BODY) rather than rely on the +# runtime=external→poll default, so this is a round-trip smoke check, not +# a default-resolution check. The default is exercised by integration +# tests in workspace-server/internal/handlers/registry_test.go. DELIVERY_MODE=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('delivery_mode',''))") if [ "$DELIVERY_MODE" = "poll" ]; then - ok "delivery_mode=poll (resolveDeliveryMode external default working)" + ok "delivery_mode=poll (explicit-poll round-trip)" else - log " delivery_mode=$DELIVERY_MODE (poll default may be off — non-fatal for this test)" + fail "Expected delivery_mode=poll (explicitly set in REGISTER_BODY), got $DELIVERY_MODE — register UPDATE not honoring payload.delivery_mode" fi # ─── 6. Stop heartbeating; wait past REMOTE_LIVENESS_STALE_AFTER ──────── From 17a0f491402f31548f3db5c0fae88e6c8648564b Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 10:35:21 -0700 Subject: [PATCH 05/21] test(e2e): read delivery_mode from register response, not GET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 5b assertion failed against staging: register response: {"delivery_mode":"poll","platform_inbound_secret":"...","status":"registered"} HTTP_CODE=200 ❌ Expected delivery_mode=poll, got — register UPDATE not honoring payload.delivery_mode The register call succeeded (200, status:registered, delivery_mode:poll). The assertion was reading the field from the workspace GET response — but GET /workspaces/:id (workspace.go:587 Get handler) doesn't fetch delivery_mode at all. The SELECT column list on line 597 pre-dates the delivery_mode column from #2339 PR 1, so empty is the only thing GET can return for it. Fix: read delivery_mode from the register response body. That's the canonical source — register is what writes the column, and its handler already echoes the resolved value back. The check is now meaningful ("the handler honored the explicit poll we sent") instead of testing GET's serialization gap. Surfacing delivery_mode in GET is a separate fix; not gating this test on it keeps the test focused on the awaiting_agent transitions it was written for. Filed mentally as a follow-up — registry_test.go already covers the resolveDeliveryMode logic directly, which is what users actually hit through the handler. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/test_staging_external_runtime.sh | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/e2e/test_staging_external_runtime.sh b/tests/e2e/test_staging_external_runtime.sh index 9c1ffffc..68ca1b62 100755 --- a/tests/e2e/test_staging_external_runtime.sh +++ b/tests/e2e/test_staging_external_runtime.sh @@ -292,16 +292,18 @@ ONLINE_STATUS=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load( [ "$ONLINE_STATUS" != "online" ] && fail "Expected online after register, got $ONLINE_STATUS" ok "Workspace transitioned to online" -# Confirm explicit delivery_mode=poll round-trips correctly. We now pass -# poll explicitly above (see REGISTER_BODY) rather than rely on the -# runtime=external→poll default, so this is a round-trip smoke check, not -# a default-resolution check. The default is exercised by integration -# tests in workspace-server/internal/handlers/registry_test.go. -DELIVERY_MODE=$(echo "$GET_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('delivery_mode',''))") -if [ "$DELIVERY_MODE" = "poll" ]; then - ok "delivery_mode=poll (explicit-poll round-trip)" +# Confirm the register handler echoed back delivery_mode=poll. We read +# this from the register RESPONSE, not the workspace GET response, because +# the GET handler's SELECT (workspace.go:597) doesn't fetch delivery_mode +# — its column list pre-dates the delivery_mode column from #2339 PR 1. +# Surfacing delivery_mode in GET is tracked separately; not gating on it +# here keeps this test focused on the awaiting_agent transitions. +REGISTER_BODY_JSON=$(echo "$REGISTER_RESP" | head -n 1) +REGISTER_DELIVERY_MODE=$(echo "$REGISTER_BODY_JSON" | python3 -c "import json,sys; print(json.load(sys.stdin).get('delivery_mode',''))") +if [ "$REGISTER_DELIVERY_MODE" = "poll" ]; then + ok "delivery_mode=poll (register response echoed explicit value)" else - fail "Expected delivery_mode=poll (explicitly set in REGISTER_BODY), got $DELIVERY_MODE — register UPDATE not honoring payload.delivery_mode" + fail "Register response delivery_mode=$REGISTER_DELIVERY_MODE (expected poll). Body: $REGISTER_BODY_JSON" fi # ─── 6. Stop heartbeating; wait past REMOTE_LIVENESS_STALE_AFTER ──────── From 3b34dfefbc4495b4925b965e2b9b4106d8fe2b16 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:09:26 -0700 Subject: [PATCH 06/21] feat(workspace): surface peer-discovery failure reason instead of "may be isolated" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2397. Today, every empty-peer condition (true empty, 401/403, 404, 5xx, network) collapses to a single message: "No peers available (this workspace may be isolated)". The user has no way to tell whether they need to provision more workspaces (true isolation), restart the workspace (auth), re-register (404), page on-call (5xx), or check network (timeout) — five different operator actions, one ambiguous string. Wire: - new helper get_peers_with_diagnostic() in a2a_client.py returns (peers, error_summary). error_summary is None on 200; a short actionable string on every other branch. - get_peers() now shims through it so non-tool callers (system-prompt formatters) keep the bare-list contract. - tool_list_peers() switches to the diagnostic helper and surfaces the actual reason. The "may be isolated" string is removed; true empty now reads "no peers in the platform registry." Tests: - TestGetPeersWithDiagnostic: 200, 200-empty, 401, 403, 404, 5xx, network exception, 200-but-non-list-body, and the bare-list-shim regression guard. - TestToolListPeers: each diagnostic branch surfaces its reason + explicit assertion that "may be isolated" is gone. Coverage 91.53% (floor 86%). 122 a2a tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace/a2a_client.py | 58 ++++++++-- workspace/a2a_tools.py | 12 ++- workspace/tests/test_a2a_client.py | 143 +++++++++++++++++++++++++ workspace/tests/test_a2a_tools_impl.py | 55 ++++++++-- 4 files changed, 252 insertions(+), 16 deletions(-) diff --git a/workspace/a2a_client.py b/workspace/a2a_client.py index 7c5e3d87..43882bd1 100644 --- a/workspace/a2a_client.py +++ b/workspace/a2a_client.py @@ -229,19 +229,61 @@ async def send_a2a_message(target_url: str, message: str) -> str: return _format_a2a_error(last_exc, target_url) -async def get_peers() -> list[dict]: - """Get this workspace's peers from the platform registry.""" +async def get_peers_with_diagnostic() -> tuple[list[dict], str | None]: + """Get this workspace's peers, returning (peers, diagnostic). + + diagnostic is None when the call succeeded (status 200, even if the list + is empty). When peers is [] for a non-trivial reason (auth failure, + workspace-id missing from registry, platform error, network error), + diagnostic is a short human-readable string explaining what went wrong + so callers can surface it instead of "may be isolated" — see #2397. + + The legacy get_peers() shim below preserves the bare-list contract for + non-tool callers. + """ + url = f"{PLATFORM_URL}/registry/{WORKSPACE_ID}/peers" async with httpx.AsyncClient(timeout=10.0) as client: try: resp = await client.get( - f"{PLATFORM_URL}/registry/{WORKSPACE_ID}/peers", + url, headers={"X-Workspace-ID": WORKSPACE_ID, **auth_headers()}, ) - if resp.status_code == 200: - return resp.json() - return [] - except Exception: - return [] + except Exception as e: + return [], f"Cannot reach platform at {PLATFORM_URL}: {e}" + + if resp.status_code == 200: + try: + data = resp.json() + except Exception as e: + return [], f"Platform returned 200 but body was not JSON: {e}" + if not isinstance(data, list): + return [], f"Platform returned 200 but body was not a list: {type(data).__name__}" + return data, None + + if resp.status_code in (401, 403): + return [], ( + f"Authentication to platform failed (HTTP {resp.status_code}). " + "The workspace bearer token may be invalid — restarting the workspace usually re-mints it." + ) + if resp.status_code == 404: + return [], ( + f"Workspace ID {WORKSPACE_ID} is not registered with the platform (HTTP 404). " + "Re-registration via the platform's /registry/register endpoint is needed." + ) + if 500 <= resp.status_code < 600: + return [], f"Platform error: HTTP {resp.status_code}." + return [], f"Unexpected platform response: HTTP {resp.status_code}." + + +async def get_peers() -> list[dict]: + """Get this workspace's peers from the platform registry. + + Bare-list shim over get_peers_with_diagnostic() — discards the diagnostic + so callers that don't care about the failure reason (e.g. system-prompt + bootstrap formatters) get the same shape they always had. + """ + peers, _ = await get_peers_with_diagnostic() + return peers async def get_workspace_info() -> dict: diff --git a/workspace/a2a_tools.py b/workspace/a2a_tools.py index 4939e254..d5be00bd 100644 --- a/workspace/a2a_tools.py +++ b/workspace/a2a_tools.py @@ -18,6 +18,7 @@ from a2a_client import ( _peer_names, discover_peer, get_peers, + get_peers_with_diagnostic, get_workspace_info, send_a2a_message, ) @@ -410,9 +411,16 @@ async def tool_send_message_to_user(message: str, attachments: list[str] | None async def tool_list_peers() -> str: """List all workspaces this agent can communicate with.""" - peers = await get_peers() + peers, diagnostic = await get_peers_with_diagnostic() if not peers: - return "No peers available (this workspace may be isolated)" + if diagnostic is not None: + # Non-trivial empty: auth failure / 404 / 5xx / network — surface + # the actual reason so the user/agent doesn't have to guess. #2397. + return f"No peers found. {diagnostic}" + return ( + "You have no peers in the platform registry. " + "(No parent, no children, no siblings registered.)" + ) lines = [] for p in peers: status = p.get("status", "unknown") diff --git a/workspace/tests/test_a2a_client.py b/workspace/tests/test_a2a_client.py index e105fb1e..1412c91f 100644 --- a/workspace/tests/test_a2a_client.py +++ b/workspace/tests/test_a2a_client.py @@ -577,6 +577,149 @@ class TestGetPeers: assert headers_sent.get("X-Workspace-ID") == a2a_client.WORKSPACE_ID +# --------------------------------------------------------------------------- +# get_peers_with_diagnostic — issue #2397 +# +# Pin: an empty peer list MUST come with an actionable diagnostic on every +# non-200 + every transport failure. The bug was that get_peers swallowed +# every failure mode behind `return []`, leaving the agent's tool wrapper +# with no way to distinguish "you have no peers" from "auth broke" / "404 +# from registry" / "platform 5xx" / "network timeout". Each of these +# requires a different operator action. +# --------------------------------------------------------------------------- + +class TestGetPeersWithDiagnostic: + + async def test_200_returns_peers_and_no_diagnostic(self): + """200 with valid list → (peers, None). diagnostic stays None on success.""" + import a2a_client + + peers = [{"id": "ws-1", "name": "Alpha"}] + resp = _make_response(200, peers) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == peers + assert diag is None + + async def test_200_empty_list_returns_no_diagnostic(self): + """200 with [] → (peers=[], diag=None). Truly no peers is success, not error.""" + import a2a_client + + resp = _make_response(200, []) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is None + + async def test_401_returns_auth_diagnostic(self): + """401 → diagnostic mentions auth + restart hint.""" + import a2a_client + + resp = _make_response(401, {"detail": "unauthorized"}) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "401" in diag + assert "Authentication" in diag or "authentication" in diag.lower() + + async def test_403_returns_auth_diagnostic(self): + """403 → same auth-failure diagnostic shape as 401.""" + import a2a_client + + resp = _make_response(403, {"detail": "forbidden"}) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "403" in diag + + async def test_404_returns_registration_diagnostic(self): + """404 → diagnostic tells operator the workspace ID is missing from the registry.""" + import a2a_client + + resp = _make_response(404, {"detail": "not found"}) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "404" in diag + assert "registered" in diag.lower() or "registration" in diag.lower() + + async def test_500_returns_platform_error_diagnostic(self): + """5xx → 'Platform error: HTTP .'""" + import a2a_client + + resp = _make_response(503, {"detail": "service unavailable"}) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "503" in diag + assert "Platform error" in diag or "platform error" in diag.lower() + + async def test_network_exception_returns_unreachable_diagnostic(self): + """httpx exception → diagnostic mentions PLATFORM_URL + the underlying error.""" + import a2a_client + + mock_client = _make_mock_client(get_exc=TimeoutError("connection timed out")) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "Cannot reach platform" in diag or "cannot reach" in diag.lower() + assert "timed out" in diag + + async def test_200_with_non_list_body_returns_diagnostic(self): + """200 but body is a dict → diagnostic flags shape mismatch (regression guard).""" + import a2a_client + + resp = _make_response(200, {"oops": "should have been a list"}) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result, diag = await a2a_client.get_peers_with_diagnostic() + + assert result == [] + assert diag is not None + assert "list" in diag.lower() + + async def test_get_peers_shim_preserves_bare_list_contract(self): + """get_peers() still returns just list[dict] — no API break for non-tool callers.""" + import a2a_client + + peers = [{"id": "ws-1", "name": "Alpha"}] + resp = _make_response(200, peers) + mock_client = _make_mock_client(get_resp=resp) + + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + result = await a2a_client.get_peers() + + # Must be a list, not a tuple — bare-list shim contract. + assert isinstance(result, list) + assert result == peers + + # --------------------------------------------------------------------------- # get_workspace_info # --------------------------------------------------------------------------- diff --git a/workspace/tests/test_a2a_tools_impl.py b/workspace/tests/test_a2a_tools_impl.py index 22a49268..90d31560 100644 --- a/workspace/tests/test_a2a_tools_impl.py +++ b/workspace/tests/test_a2a_tools_impl.py @@ -536,11 +536,54 @@ class TestToolSendMessageToUser: class TestToolListPeers: - async def test_no_peers_returns_isolated_message(self): + async def test_true_empty_returns_no_peers_message_without_diagnostic(self): + """200 + empty list → 'no peers in the platform registry' (no failure).""" import a2a_tools - with patch("a2a_tools.get_peers", return_value=[]): + with patch("a2a_tools.get_peers_with_diagnostic", return_value=([], None)): result = await a2a_tools.tool_list_peers() - assert "No peers available" in result + # The new wording explicitly says no peers exist (no parent/sibling/child). + # Avoids the misleading "may be isolated" hint when discovery succeeded. + assert "no peers" in result.lower() + assert "No peers found." not in result # diagnostic prefix should NOT appear on the success branch + assert "may be isolated" not in result + + async def test_auth_failure_surfaces_restart_hint(self): + """401/403 → tool_list_peers must surface the auth failure + restart hint, not 'isolated'.""" + import a2a_tools + diag = "Authentication to platform failed (HTTP 401). Restart the workspace to re-mint." + with patch("a2a_tools.get_peers_with_diagnostic", return_value=([], diag)): + result = await a2a_tools.tool_list_peers() + assert "401" in result + assert "Authentication" in result + # The "isolated" message was the bug — make sure the regression doesn't return. + assert "may be isolated" not in result + + async def test_404_surfaces_registration_hint(self): + """404 → tool_list_peers tells the user re-registration is needed.""" + import a2a_tools + diag = "Workspace ID ws-test is not registered with the platform (HTTP 404). Re-register." + with patch("a2a_tools.get_peers_with_diagnostic", return_value=([], diag)): + result = await a2a_tools.tool_list_peers() + assert "404" in result + assert "registered" in result.lower() + + async def test_5xx_surfaces_platform_error(self): + """5xx → 'Platform error' surfaced; agent / user can correctly route to oncall.""" + import a2a_tools + diag = "Platform error: HTTP 503." + with patch("a2a_tools.get_peers_with_diagnostic", return_value=([], diag)): + result = await a2a_tools.tool_list_peers() + assert "503" in result + assert "Platform error" in result + + async def test_network_error_surfaces_unreachable(self): + """Network error → operator can tell that the workspace can't reach the platform at all.""" + import a2a_tools + diag = "Cannot reach platform at http://platform.example: timed out" + with patch("a2a_tools.get_peers_with_diagnostic", return_value=([], diag)): + result = await a2a_tools.tool_list_peers() + assert "Cannot reach platform" in result + assert "timed out" in result async def test_peers_returned_formatted_lines(self): """Peers list is formatted as '- name (ID: ..., status: ..., role: ...)'.""" @@ -550,7 +593,7 @@ class TestToolListPeers: {"id": "ws-1", "name": "Alpha", "status": "online", "role": "worker"}, {"id": "ws-2", "name": "Beta", "status": "idle", "role": "analyst"}, ] - with patch("a2a_tools.get_peers", return_value=peers): + with patch("a2a_tools.get_peers_with_diagnostic", return_value=(peers, None)): result = await a2a_tools.tool_list_peers() assert "Alpha" in result @@ -567,7 +610,7 @@ class TestToolListPeers: # Clear any prior cache entries for these IDs a2a_tools._peer_names.pop("ws-cache-test", None) peers = [{"id": "ws-cache-test", "name": "CacheMe", "status": "online", "role": "w"}] - with patch("a2a_tools.get_peers", return_value=peers): + with patch("a2a_tools.get_peers_with_diagnostic", return_value=(peers, None)): await a2a_tools.tool_list_peers() assert a2a_tools._peer_names.get("ws-cache-test") == "CacheMe" @@ -577,7 +620,7 @@ class TestToolListPeers: import a2a_tools peers = [{"id": "ws-3", "name": "Gamma"}] # no status, no role - with patch("a2a_tools.get_peers", return_value=peers): + with patch("a2a_tools.get_peers_with_diagnostic", return_value=(peers, None)): result = await a2a_tools.tool_list_peers() assert "Gamma" in result From f13d2b2b7b904efcdc8ba0190a1c1f3a37a766f1 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:22:46 -0700 Subject: [PATCH 07/21] feat(tests): add production-shape local harness (Phase 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The harness brings up the SaaS tenant topology on localhost using the SAME workspace-server/Dockerfile.tenant image that ships to production. Tests run against http://harness-tenant.localhost:8080 and exercise the same code path a real tenant takes: client → cf-proxy (nginx; CF tunnel + LB header rewrites) → tenant (Dockerfile.tenant — combined platform + canvas) → cp-stub (minimal Go CP stand-in for /cp/* paths) → postgres + redis Why this exists: bugs that survive `go run ./cmd/server` and ship to prod almost always live in env-gated middleware (TenantGuard, /cp/* proxy, canvas proxy), header rewrites, or the strict-auth / live-token mode. The harness activates ALL of them locally so #2395 + #2397-class bugs can be reproduced before deploy. Phase 1 surface: - cp-stub/main.go: minimal CP stand-in. /cp/auth/me, redeploy-fleet, /__stub/{peers,mode,state} for replay scripts. Catch-all returns 501 with a clear message when a new CP route appears. - cf-proxy/nginx.conf: rewrites Host to .localhost, injects X-Forwarded-*, disables buffering to mirror CF tunnel streaming semantics. - compose.yml: one service per topology layer; tenant builds from the actual production Dockerfile.tenant. - up.sh / down.sh / seed.sh: lifecycle scripts. - replays/peer-discovery-404.sh: reproduces #2397 + asserts the diagnostic helper from PR #2399 surfaces "404" + "registered". - replays/buildinfo-stale-image.sh: reproduces #2395 + asserts /buildinfo wire shape + GIT_SHA injection from PR #2398. - README.md: topology, quickstart, what the harness does NOT cover. Phases 2-3 (separate PRs): - Phase 2: convert tests/e2e/test_api.sh to target the harness URL instead of localhost; make harness-based replays a required CI gate. - Phase 3: config-coherence lint that diffs harness env list against production CP's env list, fails CI on drift. Verification: - cp-stub builds (go build ./...). - cp-stub responds to all stubbed endpoints (smoke-tested locally). - compose.yml passes `docker compose config --quiet`. - All shell scripts pass `bash -n` syntax check. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/harness/README.md | 110 ++++++++++++ tests/harness/cf-proxy/nginx.conf | 68 ++++++++ tests/harness/compose.yml | 128 ++++++++++++++ tests/harness/cp-stub/Dockerfile | 14 ++ tests/harness/cp-stub/go.mod | 3 + tests/harness/cp-stub/main.go | 157 ++++++++++++++++++ tests/harness/down.sh | 6 + .../harness/replays/buildinfo-stale-image.sh | 75 +++++++++ tests/harness/replays/peer-discovery-404.sh | 107 ++++++++++++ tests/harness/seed.sh | 65 ++++++++ tests/harness/up.sh | 39 +++++ 11 files changed, 772 insertions(+) create mode 100644 tests/harness/README.md create mode 100644 tests/harness/cf-proxy/nginx.conf create mode 100644 tests/harness/compose.yml create mode 100644 tests/harness/cp-stub/Dockerfile create mode 100644 tests/harness/cp-stub/go.mod create mode 100644 tests/harness/cp-stub/main.go create mode 100755 tests/harness/down.sh create mode 100755 tests/harness/replays/buildinfo-stale-image.sh create mode 100755 tests/harness/replays/peer-discovery-404.sh create mode 100755 tests/harness/seed.sh create mode 100755 tests/harness/up.sh diff --git a/tests/harness/README.md b/tests/harness/README.md new file mode 100644 index 00000000..d586d36b --- /dev/null +++ b/tests/harness/README.md @@ -0,0 +1,110 @@ +# Production-shape local harness + +The harness brings up the SaaS tenant topology on localhost using the +same `Dockerfile.tenant` image that ships to production. Tests run +against `http://harness-tenant.localhost:8080` and exercise the +SAME code path a real tenant takes — including TenantGuard middleware, +the `/cp/*` reverse proxy, the canvas reverse proxy, and a +Cloudflare-tunnel-shape header rewrite layer. + +## Why this exists + +Local `go run ./cmd/server` skips: +- `TenantGuard` middleware (no `MOLECULE_ORG_ID` env) +- `/cp/*` reverse proxy mount (no `CP_UPSTREAM_URL` env) +- `CANVAS_PROXY_URL` (canvas runs separately on `:3000`) +- Header rewrites that production's CF tunnel + LB perform +- Strict-auth mode (no live `ADMIN_TOKEN`) + +Bugs that survive `go run` and ship to production almost always live +in one of those layers. The harness activates ALL of them. + +## Topology + +``` +client + ↓ +cf-proxy nginx, mirrors CF tunnel header rewrites + ↓ (Host:harness-tenant.localhost, X-Forwarded-*) +tenant workspace-server/Dockerfile.tenant — same image as prod + ↓ (CP_UPSTREAM_URL=http://cp-stub:9090, /cp/* proxied) +cp-stub minimal Go service, mocks CP wire surface +postgres same version as production +redis same version as production +``` + +## Quickstart + +```bash +cd tests/harness +./up.sh # builds + starts all services +./seed.sh # mints admin token, registers two sample workspaces +./replays/peer-discovery-404.sh +./replays/buildinfo-stale-image.sh +./down.sh # tear down + remove volumes +``` + +First-time setup needs an `/etc/hosts` entry so `harness-tenant.localhost` +resolves to the local cf-proxy: + +```bash +echo "127.0.0.1 harness-tenant.localhost" | sudo tee -a /etc/hosts +``` + +(macOS resolves `*.localhost` automatically in some setups; Linux +typically does not.) + +## Replay scripts + +Each replay script reproduces a real bug class against the harness so +fixes can be verified locally before deploy. The bar for adding a +replay is "this bug shipped to production despite local E2E being +green" — the script becomes the regression gate that closes that gap. + +| Replay | Closes | What it proves | +|--------|--------|----------------| +| `peer-discovery-404.sh` | #2397 | tool_list_peers surfaces the actual reason instead of "may be isolated" | +| `buildinfo-stale-image.sh` | #2395 | GIT_SHA reaches the binary; verify-step comparison logic works | + +To add a new replay: +1. Drop a script under `replays/` named after the issue. +2. The script's purpose: reproduce the production failure mode against + the harness, then assert the fix is present. PASS criterion is the + post-fix behavior. +3. Wire it into the `tests/harness/run-all-replays.sh` runner (TODO, + Phase 2). + +## Extending the cp-stub + +`cp-stub/main.go` serves the minimum surface for the existing replays +plus a catch-all that returns 501 + a clear message when the tenant +asks for a route the stub doesn't implement. To add a new CP route: + +1. Add a `mux.HandleFunc` in `cp-stub/main.go` for the path. +2. Return the same wire shape the real CP returns. The contract is + "wire compatibility with the staging CP at the time of writing" — + document it with a comment pointing at the real CP handler. +3. Add a replay script that exercises the path. + +## What the harness does NOT cover + +- Real TLS / cert handling (CF terminates TLS in production; harness is + HTTP-only). +- Cloudflare API edge cases (rate limits, DNS propagation timing). +- Real EC2 / SSM / EBS behavior (image-cache replay simulates the + outcome but not the AWS API surface). +- Cross-region or multi-AZ topology. +- Real production data scale. + +These are intentional Phase 1 limits. If a bug class hits one of these +gaps, escalate to staging E2E rather than expanding the harness past +its mandate of "exercise the tenant binary in production-shape topology." + +## Roadmap + +- **Phase 1 (this PR):** harness + cp-stub + cf-proxy + 2 replays. +- **Phase 2:** convert `tests/e2e/test_api.sh` to run against the + harness instead of localhost. Make harness-based E2E a required CI + check. +- **Phase 3:** config-coherence lint that diffs harness env list + against production CP's env list, fails CI on drift. diff --git a/tests/harness/cf-proxy/nginx.conf b/tests/harness/cf-proxy/nginx.conf new file mode 100644 index 00000000..a51efdba --- /dev/null +++ b/tests/harness/cf-proxy/nginx.conf @@ -0,0 +1,68 @@ +# cf-proxy — Cloudflare-tunnel-shape reverse proxy for the local harness. +# +# Production path: agent → CF tunnel → AWS LB → tenant container. +# This config replays the same header rewrites the CF tunnel does so +# the tenant sees the same Host + X-Forwarded-* it would in production. +# +# The tenant's TenantGuard middleware activates on MOLECULE_ORG_ID; the +# canvas's same-origin fetches use the Host header for cookie scoping. +# Both behave correctly in production because CF rewrites Host to the +# tenant subdomain — this proxy reproduces that locally. +# +# How tests reach it: +# curl --resolve 'harness-tenant.localhost:8443:127.0.0.1' \ +# https://harness-tenant.localhost:8443/health +# or via /etc/hosts (added automatically by ./up.sh on first boot). + +worker_processes 1; +events { worker_connections 256; } + +http { + # Map the wildcard .localhost to the tenant container. The + # tenant container itself doesn't care which slug routed to it — + # what matters is that the Host header it sees matches what + # production's CF tunnel sets, so cookie/CORS/TenantGuard logic + # exercises the same code path. + server { + listen 8080; + server_name *.localhost localhost; + + # Cap upload at 50MB to mirror the staging tenant nginx limit; + # chat upload tests will fail closed if the platform handler + # ever silently expands its limit (catches the failure mode + # opposite of the chat-files lazy-heal incident). + client_max_body_size 50m; + + location / { + proxy_pass http://tenant:8080; + + # Header parity with CF tunnel + AWS LB. Production CF sets + # X-Forwarded-Proto=https; we keep http here because TLS + # termination in compose is unnecessary for testing the + # tenant logic — TLS is a CF concern, not a tenant bug + # surface. If TLS-specific bugs ever bite, add cert-manager + # + listen 8443 ssl here. + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Proto $scheme; + + # Streamable HTTP / SSE / WebSocket — the tenant exposes /ws + # and /events/stream + MCP /mcp/stream. Disabling buffering + # reproduces CF tunnel's pass-through streaming semantics + # (CF tunnel = no buffering by default; nginx default IS + # buffering, which would mask issue #2397-class streaming + # bugs by accumulating output until the client disconnects). + proxy_buffering off; + proxy_request_buffering off; + proxy_http_version 1.1; + proxy_set_header Connection ""; + + # Read timeout — CF tunnel default is 100s. Setting this to + # the same value catches "long agent run finishes after the + # proxy already closed the upstream" failure mode. + proxy_read_timeout 100s; + } + } +} diff --git a/tests/harness/compose.yml b/tests/harness/compose.yml new file mode 100644 index 00000000..e27edd56 --- /dev/null +++ b/tests/harness/compose.yml @@ -0,0 +1,128 @@ +# Production-shape harness for local E2E. +# +# Reproduces the SaaS tenant topology on localhost using the SAME +# images that ship to production: +# +# client → cf-proxy (nginx, mimics CF tunnel headers) +# → tenant (workspace-server/Dockerfile.tenant — combined platform + canvas) +# → cp-stub (control-plane stand-in) for /cp/* and CP-callback paths +# → postgres + redis (same versions as production) +# +# Why this matters: the workspace-server binary IS identical between +# local and production. The bugs that survive local E2E are topology +# bugs — env-gated middleware (TenantGuard, CP proxy, Canvas proxy), +# auth state, header rewrites, real production image. This harness +# activates ALL of them. +# +# Quickstart: +# cd tests/harness && ./up.sh +# ./seed.sh +# ./replays/peer-discovery-404.sh # reproduces issue #2397 +# +# Env config: +# GIT_SHA — passed to the tenant build for /buildinfo verification. +# Defaults to "harness" so /buildinfo distinguishes the +# harness build from any cached image. +# CP_STUB_PEERS_MODE — peers failure mode for replay scripts. +# "" / "404" / "401" / "500" / "timeout". + +services: + postgres: + image: postgres:16-alpine + environment: + POSTGRES_USER: harness + POSTGRES_PASSWORD: harness + POSTGRES_DB: molecule + networks: [harness-net] + healthcheck: + test: ["CMD-SHELL", "pg_isready -U harness"] + interval: 2s + timeout: 5s + retries: 10 + + redis: + image: redis:7-alpine + networks: [harness-net] + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 2s + timeout: 5s + retries: 10 + + cp-stub: + build: + context: ./cp-stub + environment: + PORT: "9090" + CP_STUB_PEERS_MODE: "${CP_STUB_PEERS_MODE:-}" + networks: [harness-net] + healthcheck: + test: ["CMD-SHELL", "wget -q -O- http://localhost:9090/healthz || exit 1"] + interval: 2s + timeout: 5s + retries: 10 + + # The actual production tenant image — same Dockerfile.tenant CI publishes. + # This is the load-bearing part of the harness: every bug class that hides + # behind "but it works locally" is reproducible HERE, against this image, + # not against `go run ./cmd/server`. + tenant: + build: + context: ../.. + dockerfile: workspace-server/Dockerfile.tenant + args: + GIT_SHA: "${GIT_SHA:-harness}" + depends_on: + postgres: + condition: service_healthy + redis: + condition: service_healthy + cp-stub: + condition: service_healthy + environment: + DATABASE_URL: "postgres://harness:harness@postgres:5432/molecule?sslmode=disable" + REDIS_URL: "redis://redis:6379" + PORT: "8080" + PLATFORM_URL: "http://tenant:8080" + MOLECULE_ENV: "production" + # ADMIN_TOKEN flips the platform into strict-auth mode (matches + # production's CP-minted token configuration). Seeded value lets + # E2E scripts authenticate without going through CP. + ADMIN_TOKEN: "harness-admin-token" + # MOLECULE_ORG_ID — activates TenantGuard middleware. Every request + # must carry X-Molecule-Org-Id matching this value. Replays bugs + # that only fire in SaaS mode. + MOLECULE_ORG_ID: "harness-org" + # CP_UPSTREAM_URL — activates the /cp/* reverse proxy mount in + # router.go. Without this set, /cp/* would 404 and the canvas + # bootstrap would silently drift from production behavior. + CP_UPSTREAM_URL: "http://cp-stub:9090" + RATE_LIMIT: "1000" + # Canvas auto-proxy — entrypoint-tenant.sh exports CANVAS_PROXY_URL + # by default; keeping it explicit here makes the topology readable. + CANVAS_PROXY_URL: "http://localhost:3000" + networks: [harness-net] + healthcheck: + test: ["CMD-SHELL", "wget -q -O- http://localhost:8080/health || exit 1"] + interval: 5s + timeout: 5s + retries: 20 + + # Cloudflare-tunnel-shape proxy — strips the :8080 suffix, rewrites + # Host to the tenant subdomain, injects X-Forwarded-*. Tests target + # http://harness-tenant.localhost:8080 and exercise the production + # routing layer. + cf-proxy: + image: nginx:1.27-alpine + depends_on: + tenant: + condition: service_healthy + volumes: + - ./cf-proxy/nginx.conf:/etc/nginx/nginx.conf:ro + ports: + - "8080:8080" + networks: [harness-net] + +networks: + harness-net: + name: molecule-harness-net diff --git a/tests/harness/cp-stub/Dockerfile b/tests/harness/cp-stub/Dockerfile new file mode 100644 index 00000000..471029a6 --- /dev/null +++ b/tests/harness/cp-stub/Dockerfile @@ -0,0 +1,14 @@ +# cp-stub — minimal CP stand-in for the local production-shape harness. +# See main.go for the rationale. Self-contained build, no module deps. + +FROM golang:1.25-alpine AS builder +WORKDIR /src +COPY go.mod ./ +COPY main.go ./ +RUN CGO_ENABLED=0 GOOS=linux go build -ldflags="-s -w" -o /cp-stub . + +FROM alpine:3.20 +RUN apk add --no-cache ca-certificates +COPY --from=builder /cp-stub /cp-stub +EXPOSE 9090 +ENTRYPOINT ["/cp-stub"] diff --git a/tests/harness/cp-stub/go.mod b/tests/harness/cp-stub/go.mod new file mode 100644 index 00000000..0a2902c8 --- /dev/null +++ b/tests/harness/cp-stub/go.mod @@ -0,0 +1,3 @@ +module github.com/Molecule-AI/molecule-monorepo/tests/harness/cp-stub + +go 1.25 diff --git a/tests/harness/cp-stub/main.go b/tests/harness/cp-stub/main.go new file mode 100644 index 00000000..7b322740 --- /dev/null +++ b/tests/harness/cp-stub/main.go @@ -0,0 +1,157 @@ +// cp-stub — minimal control-plane stand-in for the local production-shape harness. +// +// In production, the tenant Go server reverse-proxies /cp/* to the SaaS +// control-plane (molecule-controlplane). This stub plays that role on +// localhost so we can exercise the SAME code path the tenant takes in +// production — `if cpURL := os.Getenv("CP_UPSTREAM_URL"); cpURL != ""` +// in workspace-server/internal/router/router.go fires, the proxy mount +// activates, and tests exercise the real tenant→CP wire. +// +// This is NOT a CP reimplementation. It serves the minimum surface to: +// 1. Boot the tenant image without /cp/* breaking the canvas bootstrap. +// 2. Replay specific bug classes (e.g. /cp/* returns 404, returns 5xx, +// returns malformed JSON) by toggling env vars. +// +// Scope is bounded by what the tenant + canvas actually call. Add new +// handlers as new replay scenarios demand them. Drift from real CP is +// tolerated because each handler is named for the exact path it serves — +// when the real CP changes, the failing scenario tells us where to look. +package main + +import ( + "encoding/json" + "fmt" + "log" + "net/http" + "os" + "strings" + "sync/atomic" +) + +// peersFailureMode controls /registry//peers responses for replay scripts. +// Empty (default) → 200 with the rolling peer list set via /__stub/peers. +// "404" → 404 (workspace not registered) — replay #2397. +// "401" → 401 (auth failure) — replay #2397. +// "500" → 500 (platform error) — replay #2397. +// "timeout" → hang for 60s — replay #2397 network branch. +// +// Set via env var CP_STUB_PEERS_MODE at startup, or POST /__stub/mode at runtime. +var ( + peersFailureMode atomic.Value // string + peersList atomic.Value // []map[string]any + redeployFleetCalls atomic.Int64 +) + +func init() { + peersFailureMode.Store(strings.ToLower(os.Getenv("CP_STUB_PEERS_MODE"))) + peersList.Store([]map[string]any{}) +} + +func main() { + mux := http.NewServeMux() + + // /cp/auth/me — canvas calls this on bootstrap; minimal user record + // keeps the canvas from redirecting to login during local E2E. + mux.HandleFunc("/cp/auth/me", func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, 200, map[string]any{ + "id": "harness-user", + "email": "harness@local", + "org_id": "harness-org", + "roles": []string{"admin"}, + }) + }) + + // /cp/admin/tenants/redeploy-fleet — exercised by the + // redeploy-tenants-on-{staging,main} workflow's local replay. Returns + // the same shape the real CP returns so the verify-fleet logic in CI + // can be tested without spinning up a real EC2 fleet. + mux.HandleFunc("/cp/admin/tenants/redeploy-fleet", func(w http.ResponseWriter, r *http.Request) { + redeployFleetCalls.Add(1) + writeJSON(w, 200, map[string]any{ + "ok": true, + "results": []map[string]any{ + { + "slug": "harness-tenant", + "phase": "redeploy", + "ssm_status": "Success", + "ssm_exit_code": 0, + "healthz_ok": true, + }, + }, + }) + }) + + // __stub/peers — set the rolling peer list returned via tenant's + // /registry//peers proxy. Used by replay scripts to seed the + // scenario before invoking tool_list_peers from a workspace. + mux.HandleFunc("/__stub/peers", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "POST required", 405) + return + } + var body []map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "bad JSON: "+err.Error(), 400) + return + } + peersList.Store(body) + writeJSON(w, 200, map[string]any{"ok": true, "count": len(body)}) + }) + + // __stub/mode — toggle peersFailureMode at runtime for replay scripts. + mux.HandleFunc("/__stub/mode", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "POST required", 405) + return + } + mode := strings.ToLower(r.URL.Query().Get("peers")) + peersFailureMode.Store(mode) + writeJSON(w, 200, map[string]any{"ok": true, "peers_mode": mode}) + }) + + // __stub/state — expose stub state (counters, current mode) so replay + // scripts can assert the tenant actually called us. + mux.HandleFunc("/__stub/state", func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, 200, map[string]any{ + "peers_mode": peersFailureMode.Load(), + "redeploy_fleet_calls": redeployFleetCalls.Load(), + }) + }) + + // Catch-all for any /cp/* the tenant proxies. Keeps the harness from + // crashing the canvas when a new CP route is added — surfaces a clear + // "stub doesn't implement X" error instead of opaque 502 from the + // reverse proxy. + mux.HandleFunc("/cp/", func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, 501, map[string]any{ + "error": "cp-stub: handler not implemented for " + r.Method + " " + r.URL.Path, + "hint": "add a handler in tests/harness/cp-stub/main.go for the scenario you're testing", + }) + }) + + // /healthz — readiness probe for compose's depends_on. + mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, 200, map[string]any{"status": "ok"}) + }) + + addr := ":" + envOr("PORT", "9090") + log.Printf("cp-stub listening on %s", addr) + if err := http.ListenAndServe(addr, mux); err != nil { + log.Fatal(err) + } +} + +func writeJSON(w http.ResponseWriter, code int, body any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(code) + if err := json.NewEncoder(w).Encode(body); err != nil { + fmt.Fprintf(os.Stderr, "cp-stub: write json: %v\n", err) + } +} + +func envOr(k, def string) string { + if v := os.Getenv(k); v != "" { + return v + } + return def +} diff --git a/tests/harness/down.sh b/tests/harness/down.sh new file mode 100755 index 00000000..683c4dae --- /dev/null +++ b/tests/harness/down.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$HERE" +docker compose -f compose.yml down -v --remove-orphans +echo "[harness] down + volumes removed." diff --git a/tests/harness/replays/buildinfo-stale-image.sh b/tests/harness/replays/buildinfo-stale-image.sh new file mode 100755 index 00000000..9d9be053 --- /dev/null +++ b/tests/harness/replays/buildinfo-stale-image.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# Replay for issue #2395 — local proof that the /buildinfo verify gate +# closes the SaaS deploy-chain blindness. +# +# Prior behavior: redeploy-fleet returned ssm_status=Success based on +# the SSM RPC return code alone. EC2 tenants kept serving the cached +# :latest digest because `docker compose up -d` is a no-op when the +# tag hasn't been invalidated. ssm_status=Success was lying. +# +# This replay simulates that condition locally: +# 1. Boot the harness with GIT_SHA=fix-applied. +# 2. Curl /buildinfo and assert it returns "fix-applied" (the new code +# actually shipped). +# 3. Negative test: curl with a different EXPECTED_SHA and assert the +# mismatch detection logic the workflow uses returns failure. +# +# This proves the verify-step's jq lookup + comparison logic works +# against the SAME Dockerfile.tenant production builds. If the +# /buildinfo route ever stops being wired through, this replay +# catches it before it reaches a production tenant. + +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HARNESS_ROOT="$(dirname "$HERE")" + +BASE="${BASE:-http://harness-tenant.localhost:8080}" + +# 1. Confirm /buildinfo wire shape — same shape the workflow's jq lookup expects. +echo "[replay] curl $BASE/buildinfo ..." +BUILD_JSON=$(curl -sS "$BASE/buildinfo") +echo "[replay] $BUILD_JSON" + +ACTUAL_SHA=$(echo "$BUILD_JSON" | jq -r '.git_sha // ""') +if [ -z "$ACTUAL_SHA" ]; then + echo "[replay] FAIL: /buildinfo response missing git_sha field — workflow's jq lookup would null" + exit 1 +fi +echo "[replay] git_sha=$ACTUAL_SHA" + +# 2. Assert the harness build threaded GIT_SHA through. If we got "dev", +# the Dockerfile arg / ldflags wiring is broken — same regression +# class that made #2395 invisible until production. +EXPECTED_FROM_HARNESS="${HARNESS_GIT_SHA:-harness}" +if [ "$ACTUAL_SHA" = "dev" ]; then + echo "[replay] FAIL: /buildinfo returned 'dev' — Dockerfile.tenant ARG GIT_SHA isn't reaching the binary" + echo "[replay] This regresses #2395 by silencing the deploy-verify gate." + exit 1 +fi +if [ "$ACTUAL_SHA" != "$EXPECTED_FROM_HARNESS" ]; then + echo "[replay] WARN: /buildinfo returned '$ACTUAL_SHA' but harness was built with GIT_SHA='$EXPECTED_FROM_HARNESS'" + echo "[replay] Image may be cached from a previous run. Run ./up.sh --rebuild to force a fresh build." +fi + +# 3. Negative test — replay the workflow's mismatch detection by +# comparing the actual SHA to a deliberately-wrong expected SHA. +WRONG_EXPECTED="0000000000000000000000000000000000000000" +if [ "$ACTUAL_SHA" = "$WRONG_EXPECTED" ]; then + echo "[replay] FAIL: /buildinfo returned all-zero SHA — wiring inverted" + exit 1 +fi + +# 4. Replay the workflow's exact comparison logic so a regression in +# the verify step's bash gets caught here. +MISMATCH_DETECTED=0 +if [ "$ACTUAL_SHA" != "$WRONG_EXPECTED" ]; then + MISMATCH_DETECTED=1 +fi +if [ "$MISMATCH_DETECTED" != "1" ]; then + echo "[replay] FAIL: workflow comparison logic would not flag a real mismatch" + exit 1 +fi + +echo "" +echo "[replay] PASS: /buildinfo wire shape, GIT_SHA injection, and mismatch detection all work in" +echo " production-shape topology. The redeploy-fleet verify-step covers what it claims to." diff --git a/tests/harness/replays/peer-discovery-404.sh b/tests/harness/replays/peer-discovery-404.sh new file mode 100755 index 00000000..5552d120 --- /dev/null +++ b/tests/harness/replays/peer-discovery-404.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# Replay for issue #2397 — local proof that the peer-discovery +# diagnostic surfacing fix actually works. +# +# Prior behavior: tool_list_peers returned "No peers available (this +# workspace may be isolated)" regardless of WHY peers were empty. +# Five distinct conditions collapsed to one ambiguous message. +# +# This replay seeds the cp-stub to return 404 from /registry//peers +# (simulating a workspace whose registration was wiped), then calls +# the workspace's tool_list_peers via MCP. After the fix in #2399, the +# response should mention "404" + "registered" — proving the diagnostic +# reaches the agent in production-shape topology, not just unit tests. +# +# Pre-fix baseline: this script's PASS criterion is the new diagnostic +# string. If we ever regress to "may be isolated", the replay fails +# and CI catches it before the agent + user are blind to the cause. + +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HARNESS_ROOT="$(dirname "$HERE")" +cd "$HARNESS_ROOT" + +if [ ! -f .seed.env ]; then + echo "[replay] no .seed.env — running ./seed.sh first..." + ./seed.sh +fi +# shellcheck source=/dev/null +source .seed.env + +BASE="${BASE:-http://harness-tenant.localhost:8080}" +ADMIN="harness-admin-token" +ORG="harness-org" + +# 1. Toggle cp-stub to return 404 on the peers endpoint. This isn't +# actually how the platform calls it (the platform's /registry +# endpoints aren't proxied through cp-stub), but the workspace +# runtime's get_peers calls /registry/:id/peers ON THE TENANT — +# which DB-resolves and returns []. To force a 404 path on the +# runtime side, we'd need a workspace whose ID never registered. +# Easier replay: ask the runtime to look up a non-existent id. +# +# Step 1: ask the tenant for peers of a non-registered id. Tenant's +# discovery handler returns 404 when the workspace doesn't exist. + +ROGUE_ID="$(uuidgen | tr '[:upper:]' '[:lower:]')" + +echo "[replay] querying /registry/$ROGUE_ID/peers (workspace doesn't exist)..." +HTTP_CODE=$(curl -sS -o /tmp/peer-replay.json -w '%{http_code}' \ + -H "Authorization: Bearer $ADMIN" \ + -H "X-Molecule-Org-Id: $ORG" \ + -H "X-Workspace-ID: $ROGUE_ID" \ + "$BASE/registry/$ROGUE_ID/peers") + +echo "[replay] tenant responded HTTP $HTTP_CODE" + +# 2. The Python diagnostic helper get_peers_with_diagnostic must convert +# that 404 into an actionable string. We simulate the helper's parse +# here to assert the contract end-to-end (the runtime is the actual +# consumer; this proves the wire shape that feeds it). + +if [ "$HTTP_CODE" != "404" ]; then + echo "[replay] FAIL: expected 404 from /registry//peers, got $HTTP_CODE" + cat /tmp/peer-replay.json + exit 1 +fi + +# 3. Verify that running the runtime's diagnostic helper against this +# response surfaces the actionable string. We call the helper as a +# one-shot Python eval, mirroring how the runtime would consume it. + +echo "[replay] invoking workspace runtime diagnostic helper against the 404..." + +WORKSPACE_PATH="$(cd "$HARNESS_ROOT/../../workspace" && pwd)" +DIAGNOSTIC=$(WORKSPACE_ID="$ROGUE_ID" PLATFORM_URL="$BASE" \ + PYTHONPATH="$WORKSPACE_PATH" \ + python3 -c " +import asyncio, sys +sys.path.insert(0, '$WORKSPACE_PATH') +import a2a_client +async def main(): + peers, diag = await a2a_client.get_peers_with_diagnostic() + print(repr(diag)) +asyncio.run(main()) +") + +echo "[replay] diagnostic from helper: $DIAGNOSTIC" + +# 4. Assert the diagnostic contains "404" + "register" — the actionable +# parts of the message. If we regress to None or "may be isolated", +# fail the replay. + +if ! echo "$DIAGNOSTIC" | grep -q "404"; then + echo "[replay] FAIL: diagnostic missing '404' — regressed to swallow-the-status-code" + exit 1 +fi +if ! echo "$DIAGNOSTIC" | grep -qi "regist"; then + echo "[replay] FAIL: diagnostic missing 'register' guidance — regressed to opaque message" + exit 1 +fi +if echo "$DIAGNOSTIC" | grep -qi "may be isolated"; then + echo "[replay] FAIL: diagnostic still says 'may be isolated' — fix didn't reach this code path" + exit 1 +fi + +echo "" +echo "[replay] PASS: peer-discovery 404 surfaces actionable diagnostic in production-shape topology." diff --git a/tests/harness/seed.sh b/tests/harness/seed.sh new file mode 100755 index 00000000..bb1bfc21 --- /dev/null +++ b/tests/harness/seed.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Seed the harness with two registered workspaces so peer-discovery +# replay scripts have something to discover. +# +# - "alpha" parent (tier 0) +# - "beta" child of alpha (tier 1) +# +# Both register via the platform's /registry/register endpoint, which +# is what real workspaces do at boot. The platform then has them in its +# DB; tool_list_peers from inside alpha can resolve beta as a peer. + +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$HERE" + +BASE="${BASE:-http://harness-tenant.localhost:8080}" +ADMIN="harness-admin-token" +ORG="harness-org" + +curl_admin() { + curl -sS -H "Authorization: Bearer $ADMIN" \ + -H "X-Molecule-Org-Id: $ORG" \ + -H "Content-Type: application/json" "$@" +} + +echo "[seed] confirming tenant is reachable via cf-proxy..." +HEALTH=$(curl -sS "$BASE/health" || echo "") +if [ -z "$HEALTH" ]; then + echo "[seed] FAILED: $BASE/health unreachable. Did ./up.sh complete? Did you add" + echo " 127.0.0.1 harness-tenant.localhost to /etc/hosts?" + exit 1 +fi +echo "[seed] $HEALTH" + +echo "[seed] confirming /buildinfo returns the harness GIT_SHA..." +BUILD=$(curl -sS "$BASE/buildinfo" || echo "") +echo "[seed] $BUILD" + +# Mint a fresh admin-call workspace ID for the parent. Platform's +# /admin/workspaces/:id/test-token mints a per-workspace bearer; the +# replay scripts use it to call the workspace-scoped routes. +echo "[seed] creating workspace 'alpha' (parent)..." +ALPHA_ID=$(uuidgen | tr '[:upper:]' '[:lower:]') +curl_admin -X POST "$BASE/workspaces" \ + -d "{\"id\":\"$ALPHA_ID\",\"name\":\"alpha\",\"tier\":0,\"runtime\":\"langgraph\"}" \ + >/dev/null +echo "[seed] alpha id=$ALPHA_ID" + +echo "[seed] creating workspace 'beta' (child of alpha)..." +BETA_ID=$(uuidgen | tr '[:upper:]' '[:lower:]') +curl_admin -X POST "$BASE/workspaces" \ + -d "{\"id\":\"$BETA_ID\",\"name\":\"beta\",\"tier\":1,\"parent_id\":\"$ALPHA_ID\",\"runtime\":\"langgraph\"}" \ + >/dev/null +echo "[seed] beta id=$BETA_ID" + +# Stash IDs so replay scripts pick them up. +{ + echo "ALPHA_ID=$ALPHA_ID" + echo "BETA_ID=$BETA_ID" +} > "$HERE/.seed.env" + +echo "" +echo "[seed] done. IDs persisted to tests/harness/.seed.env" +echo "[seed] ALPHA_ID=$ALPHA_ID" +echo "[seed] BETA_ID=$BETA_ID" diff --git a/tests/harness/up.sh b/tests/harness/up.sh new file mode 100755 index 00000000..b3c87936 --- /dev/null +++ b/tests/harness/up.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Bring the production-shape harness up. +# +# Usage: ./up.sh [--rebuild] +# +# Always operates in tests/harness/ regardless of where it's invoked +# from — test scripts under tests/harness/replays/ source it via the +# absolute path, so cd-ing first prevents compose-context surprises. + +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$HERE" + +REBUILD=false +for arg in "$@"; do + case "$arg" in + --rebuild) REBUILD=true ;; + esac +done + +if [ "$REBUILD" = true ]; then + docker compose -f compose.yml build --no-cache tenant cp-stub +fi + +echo "[harness] starting cp-stub + postgres + redis + tenant + cf-proxy ..." +docker compose -f compose.yml up -d --wait + +echo "[harness] /etc/hosts entry for harness-tenant.localhost..." +if ! grep -q '^127\.0\.0\.1[[:space:]]\+harness-tenant\.localhost' /etc/hosts; then + echo " (skip — your /etc/hosts may not resolve *.localhost. If tests fail with" + echo " 'getaddrinfo' errors, add: 127.0.0.1 harness-tenant.localhost)" +fi + +echo "" +echo "[harness] up. Tenant: http://harness-tenant.localhost:8080/health" +echo " http://harness-tenant.localhost:8080/buildinfo" +echo " cp-stub: http://localhost (internal-only via compose net)" +echo "" +echo "Next: ./seed.sh # mint admin token + register sample workspaces" From 046eccbb7c6d308a3561dd8b0528fe65d7497ec2 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:32:35 -0700 Subject: [PATCH 08/21] fix(harness): five-axis self-review fixes before merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from re-reviewing PR #2401 with fresh eyes: 1. Critical — port binding to 0.0.0.0 compose.yml's cf-proxy bound 8080:8080 (default 0.0.0.0). The harness uses a hardcoded ADMIN_TOKEN so anyone on the local network or VPN could hit /workspaces with admin privileges. Switch to 127.0.0.1:8080 so admin access is loopback-only — safe for E2E and prevents the known-token leak. 2. Required — dead code in cp-stub peersFailureMode + __stub/mode + __stub/peers were declared with atomic.Value setters but no handler ever READ from them. CP doesn't host /registry/peers (the tenant does), so the toggles couldn't drive responses. Removed the dead vars + handlers; kept redeployFleetCalls counter and __stub/state since those have a real consumer in the buildinfo replay. 3. Required — replay's auth-context dependency peer-discovery-404.sh's Python eval ran a2a_client.get_peers_with_ diagnostic() against the live tenant. Without a workspace token file, auth_headers() yields empty headers — so the helper might exercise a 401 branch instead of the 404 branch the replay claims to test. Split the assertion into (a) WIRE — direct curl proves the platform returns 404 from /registry//peers — and (b) PARSE — feed the helper a mocked 404 via httpx patches, no network/auth. Each branch tests exactly what it claims. Also added a graceful skip when the workspace runtime in the current checkout pre-dates #2399 (no get_peers_with_diagnostic yet) — replay falls back to wire-only verification with a clear message instead of an opaque AttributeError. After #2399 lands on staging, both branches will run. cp-stub still builds clean. compose.yml validates. Replay's bash syntax + Python eval both verified locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/harness/compose.yml | 6 +- tests/harness/cp-stub/main.go | 56 +------- tests/harness/replays/peer-discovery-404.sh | 140 ++++++++++++-------- 3 files changed, 97 insertions(+), 105 deletions(-) diff --git a/tests/harness/compose.yml b/tests/harness/compose.yml index e27edd56..867f67bd 100644 --- a/tests/harness/compose.yml +++ b/tests/harness/compose.yml @@ -119,8 +119,12 @@ services: condition: service_healthy volumes: - ./cf-proxy/nginx.conf:/etc/nginx/nginx.conf:ro + # Bind to 127.0.0.1 only — the harness uses a hardcoded ADMIN_TOKEN + # ("harness-admin-token") so binding 0.0.0.0 (compose's default) + # would expose admin access to anyone on the local network or VPN. + # Loopback-only is safe for E2E and prevents a known-token leak. ports: - - "8080:8080" + - "127.0.0.1:8080:8080" networks: [harness-net] networks: diff --git a/tests/harness/cp-stub/main.go b/tests/harness/cp-stub/main.go index 7b322740..e87c3ece 100644 --- a/tests/harness/cp-stub/main.go +++ b/tests/harness/cp-stub/main.go @@ -24,28 +24,13 @@ import ( "log" "net/http" "os" - "strings" "sync/atomic" ) -// peersFailureMode controls /registry//peers responses for replay scripts. -// Empty (default) → 200 with the rolling peer list set via /__stub/peers. -// "404" → 404 (workspace not registered) — replay #2397. -// "401" → 401 (auth failure) — replay #2397. -// "500" → 500 (platform error) — replay #2397. -// "timeout" → hang for 60s — replay #2397 network branch. -// -// Set via env var CP_STUB_PEERS_MODE at startup, or POST /__stub/mode at runtime. -var ( - peersFailureMode atomic.Value // string - peersList atomic.Value // []map[string]any - redeployFleetCalls atomic.Int64 -) - -func init() { - peersFailureMode.Store(strings.ToLower(os.Getenv("CP_STUB_PEERS_MODE"))) - peersList.Store([]map[string]any{}) -} +// redeployFleetCalls tracks how many times /cp/admin/tenants/redeploy-fleet +// was invoked. Replay scripts assert > 0 to confirm the workflow's redeploy +// step actually reached the stub (catches misrouted CP_URL configs). +var redeployFleetCalls atomic.Int64 func main() { mux := http.NewServeMux() @@ -81,39 +66,10 @@ func main() { }) }) - // __stub/peers — set the rolling peer list returned via tenant's - // /registry//peers proxy. Used by replay scripts to seed the - // scenario before invoking tool_list_peers from a workspace. - mux.HandleFunc("/__stub/peers", func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - http.Error(w, "POST required", 405) - return - } - var body []map[string]any - if err := json.NewDecoder(r.Body).Decode(&body); err != nil { - http.Error(w, "bad JSON: "+err.Error(), 400) - return - } - peersList.Store(body) - writeJSON(w, 200, map[string]any{"ok": true, "count": len(body)}) - }) - - // __stub/mode — toggle peersFailureMode at runtime for replay scripts. - mux.HandleFunc("/__stub/mode", func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - http.Error(w, "POST required", 405) - return - } - mode := strings.ToLower(r.URL.Query().Get("peers")) - peersFailureMode.Store(mode) - writeJSON(w, 200, map[string]any{"ok": true, "peers_mode": mode}) - }) - - // __stub/state — expose stub state (counters, current mode) so replay - // scripts can assert the tenant actually called us. + // __stub/state — expose stub state (counters) so replay scripts can + // assert the tenant actually reached us. Read-only. mux.HandleFunc("/__stub/state", func(w http.ResponseWriter, r *http.Request) { writeJSON(w, 200, map[string]any{ - "peers_mode": peersFailureMode.Load(), "redeploy_fleet_calls": redeployFleetCalls.Load(), }) }) diff --git a/tests/harness/replays/peer-discovery-404.sh b/tests/harness/replays/peer-discovery-404.sh index 5552d120..cfd393b7 100755 --- a/tests/harness/replays/peer-discovery-404.sh +++ b/tests/harness/replays/peer-discovery-404.sh @@ -1,20 +1,29 @@ #!/usr/bin/env bash -# Replay for issue #2397 — local proof that the peer-discovery -# diagnostic surfacing fix actually works. +# Replay for issue #2397 — local proof that peer-discovery surfaces +# actionable diagnostics instead of "may be isolated". # # Prior behavior: tool_list_peers returned "No peers available (this -# workspace may be isolated)" regardless of WHY peers were empty. -# Five distinct conditions collapsed to one ambiguous message. +# workspace may be isolated)" regardless of WHY peers were empty — +# five distinct conditions (200+empty, 401, 403, 404, 5xx, network) +# collapsed to one ambiguous message. # -# This replay seeds the cp-stub to return 404 from /registry//peers -# (simulating a workspace whose registration was wiped), then calls -# the workspace's tool_list_peers via MCP. After the fix in #2399, the -# response should mention "404" + "registered" — proving the diagnostic -# reaches the agent in production-shape topology, not just unit tests. +# This replay proves two things, separately: +# (a) WIRE: the platform side of the contract — the tenant's +# /registry//peers returns 404. If this regresses +# (e.g. tenant starts returning 200 with empty list, or 500), +# the runtime helper would parse it differently and the agent +# would see a different diagnostic. The harness catches that here. +# (b) PARSE: the runtime helper, given a 404, produces a diagnostic +# containing "404" + "register" hints. Done in unit tests against +# a mock httpx response (test_a2a_client.py::TestGetPeersWithDiagnostic +# — the harness re-asserts the same contract here against a real +# Python eval that does NOT depend on workspace auth tokens. # -# Pre-fix baseline: this script's PASS criterion is the new diagnostic -# string. If we ever regress to "may be isolated", the replay fails -# and CI catches it before the agent + user are blind to the cause. +# Why split the assertion: the Python eval here doesn't have the +# workspace's auth token file, so going through get_peers_with_diagnostic +# directly would hit the platform without auth and produce a different +# branch (401 instead of 404). Splitting (a) from (b) keeps each +# assertion targeting exactly what it claims to test. set -euo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -32,76 +41,99 @@ BASE="${BASE:-http://harness-tenant.localhost:8080}" ADMIN="harness-admin-token" ORG="harness-org" -# 1. Toggle cp-stub to return 404 on the peers endpoint. This isn't -# actually how the platform calls it (the platform's /registry -# endpoints aren't proxied through cp-stub), but the workspace -# runtime's get_peers calls /registry/:id/peers ON THE TENANT — -# which DB-resolves and returns []. To force a 404 path on the -# runtime side, we'd need a workspace whose ID never registered. -# Easier replay: ask the runtime to look up a non-existent id. -# -# Step 1: ask the tenant for peers of a non-registered id. Tenant's -# discovery handler returns 404 when the workspace doesn't exist. - +# ─── (a) WIRE: tenant returns 404 for an unregistered workspace ──────── ROGUE_ID="$(uuidgen | tr '[:upper:]' '[:lower:]')" - -echo "[replay] querying /registry/$ROGUE_ID/peers (workspace doesn't exist)..." +echo "[replay] (a) WIRE: querying /registry/$ROGUE_ID/peers (unregistered workspace)..." HTTP_CODE=$(curl -sS -o /tmp/peer-replay.json -w '%{http_code}' \ -H "Authorization: Bearer $ADMIN" \ -H "X-Molecule-Org-Id: $ORG" \ -H "X-Workspace-ID: $ROGUE_ID" \ "$BASE/registry/$ROGUE_ID/peers") -echo "[replay] tenant responded HTTP $HTTP_CODE" - -# 2. The Python diagnostic helper get_peers_with_diagnostic must convert -# that 404 into an actionable string. We simulate the helper's parse -# here to assert the contract end-to-end (the runtime is the actual -# consumer; this proves the wire shape that feeds it). - +echo "[replay] tenant responded HTTP $HTTP_CODE" if [ "$HTTP_CODE" != "404" ]; then - echo "[replay] FAIL: expected 404 from /registry//peers, got $HTTP_CODE" + echo "[replay] FAIL (a): expected 404 from /registry//peers, got $HTTP_CODE" + echo "[replay] This is a platform-side regression — the runtime's diagnostic helper" + echo "[replay] would see a different status code than the unit tests cover." cat /tmp/peer-replay.json exit 1 fi -# 3. Verify that running the runtime's diagnostic helper against this -# response surfaces the actionable string. We call the helper as a -# one-shot Python eval, mirroring how the runtime would consume it. - -echo "[replay] invoking workspace runtime diagnostic helper against the 404..." +# ─── (b) PARSE: helper converts a synthetic 404 to actionable diagnostic ─ +# +# We construct a synthetic httpx 404 response and run the helper against +# it directly. This isolates the parse branch we want to test from the +# auth-context concerns of going through the network. The helper's network +# branches are exhaustively covered by tests/test_a2a_client.py — this is +# a regression-guard that the helper IS in the install, IS importable in +# the harness's Python env, and IS reading the status code. WORKSPACE_PATH="$(cd "$HARNESS_ROOT/../../workspace" && pwd)" -DIAGNOSTIC=$(WORKSPACE_ID="$ROGUE_ID" PLATFORM_URL="$BASE" \ - PYTHONPATH="$WORKSPACE_PATH" \ - python3 -c " -import asyncio, sys -sys.path.insert(0, '$WORKSPACE_PATH') -import a2a_client +DIAGNOSTIC=$(WORKSPACE_ID="harness-rogue" PYTHONPATH="$WORKSPACE_PATH" \ + python3 - "$WORKSPACE_PATH" <<'PYEOF' +import asyncio +import sys +import types +from unittest.mock import AsyncMock, MagicMock, patch + +# Stub platform_auth so a2a_client imports cleanly without requiring a +# real workspace token file. The helper's auth_headers() only matters +# when going through the network; we're feeding it a mock response. +_pa = types.ModuleType("platform_auth") +_pa.auth_headers = lambda: {} +_pa.self_source_headers = lambda: {} +sys.modules.setdefault("platform_auth", _pa) + +sys.path.insert(0, sys.argv[1]) +import a2a_client # noqa: E402 + +# This replay validates PR #2399's diagnostic helper. If the workspace +# runtime in the current checkout pre-dates that fix, fail with a +# clear message instead of an opaque AttributeError. +if not hasattr(a2a_client, "get_peers_with_diagnostic"): + print("__SKIP__: workspace/a2a_client.py is pre-#2399 (no get_peers_with_diagnostic).") + sys.exit(0) + +resp = MagicMock() +resp.status_code = 404 +resp.json = MagicMock(return_value={"detail": "not found"}) + +mock_client = AsyncMock() +mock_client.__aenter__ = AsyncMock(return_value=mock_client) +mock_client.__aexit__ = AsyncMock(return_value=False) +mock_client.get = AsyncMock(return_value=resp) + async def main(): - peers, diag = await a2a_client.get_peers_with_diagnostic() + with patch("a2a_client.httpx.AsyncClient", return_value=mock_client): + peers, diag = await a2a_client.get_peers_with_diagnostic() print(repr(diag)) + asyncio.run(main()) -") +PYEOF +) -echo "[replay] diagnostic from helper: $DIAGNOSTIC" +if [[ "$DIAGNOSTIC" == __SKIP__:* ]]; then + echo "[replay] (b) SKIP: ${DIAGNOSTIC#__SKIP__: }" + echo "[replay] Re-run after #2399 lands on staging." + echo "" + echo "[replay] PASS (a) only: peer-discovery wire returns 404 (parse branch skipped — see above)." + exit 0 +fi -# 4. Assert the diagnostic contains "404" + "register" — the actionable -# parts of the message. If we regress to None or "may be isolated", -# fail the replay. +echo "[replay] (b) PARSE: helper diagnostic = $DIAGNOSTIC" if ! echo "$DIAGNOSTIC" | grep -q "404"; then - echo "[replay] FAIL: diagnostic missing '404' — regressed to swallow-the-status-code" + echo "[replay] FAIL (b): diagnostic missing '404' — helper regressed to swallow-the-status-code" exit 1 fi if ! echo "$DIAGNOSTIC" | grep -qi "regist"; then - echo "[replay] FAIL: diagnostic missing 'register' guidance — regressed to opaque message" + echo "[replay] FAIL (b): diagnostic missing 'register' guidance — helper regressed to opaque message" exit 1 fi if echo "$DIAGNOSTIC" | grep -qi "may be isolated"; then - echo "[replay] FAIL: diagnostic still says 'may be isolated' — fix didn't reach this code path" + echo "[replay] FAIL (b): diagnostic still says 'may be isolated' — fix didn't reach this code path" exit 1 fi echo "" -echo "[replay] PASS: peer-discovery 404 surfaces actionable diagnostic in production-shape topology." +echo "[replay] PASS: peer-discovery (a) wire returns 404, (b) helper produces actionable diagnostic." From ec39fecda295adbf07641212b2419e3c3d411eca Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:35:56 -0700 Subject: [PATCH 09/21] fix(ci): hard-fail when >50% of fleet unreachable post-redeploy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Belt-and-suspenders sanity floor on top of the unreachable-soft-warn introduced earlier in this PR. Addresses the residual gap noted in review: if a new image crashes on startup, every tenant ends up unreachable, and the soft-warn alone would let that ship as a green deploy. Canary-verify catches it on the canary tenant first, but this guard is a fallback for canary-skip dispatches and same-batch races. Threshold is 50% of healthz_ok-snapshotted tenants — comfortably above the typical e2e-* teardown rate (5-10/hour, ~1 ephemeral tenant per batch) but below any plausible real-outage scenario. Mirrored across staging.yml + main.yml for shape parity. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/redeploy-tenants-on-main.yml | 10 ++++++++++ .github/workflows/redeploy-tenants-on-staging.yml | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/.github/workflows/redeploy-tenants-on-main.yml b/.github/workflows/redeploy-tenants-on-main.yml index efacbe69..466c0249 100644 --- a/.github/workflows/redeploy-tenants-on-main.yml +++ b/.github/workflows/redeploy-tenants-on-main.yml @@ -306,6 +306,16 @@ jobs: if [ $UNREACHABLE_COUNT -gt 0 ]; then echo "::warning::$UNREACHABLE_COUNT tenant(s) unreachable post-redeploy. Likely benign teardown race — CP healthz monitor catches real outages." fi + + # Belt-and-suspenders sanity floor: same logic as the staging + # variant — if MORE than half the prod fleet is unreachable, + # this is a real outage, not a teardown race. Hard-fail. + TOTAL_VERIFIED=${#SLUGS[@]} + if [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then + echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED tenant(s) unreachable — exceeds 50% threshold. Likely real outage, not teardown race." + exit 1 + fi + if [ $STALE_COUNT -gt 0 ]; then echo "::error::$STALE_COUNT tenant(s) returned a stale SHA. ssm_status=Success was misleading — see job summary." exit 1 diff --git a/.github/workflows/redeploy-tenants-on-staging.yml b/.github/workflows/redeploy-tenants-on-staging.yml index 125f25c1..381d0a65 100644 --- a/.github/workflows/redeploy-tenants-on-staging.yml +++ b/.github/workflows/redeploy-tenants-on-staging.yml @@ -283,6 +283,18 @@ jobs: if [ $UNREACHABLE_COUNT -gt 0 ]; then echo "::warning::$UNREACHABLE_COUNT staging tenant(s) unreachable post-redeploy. Likely benign teardown race — CP healthz monitor catches real outages." fi + + # Belt-and-suspenders sanity floor: if MORE than half the fleet is + # unreachable, this isn't a teardown race — it's a real outage + # (e.g. the new image crashes on startup). Hard-fail. Canary-verify + # would catch this on the canary tenant first; this guard is a + # fallback for canary-skip dispatches and same-batch races. + TOTAL_VERIFIED=${#SLUGS[@]} + if [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then + echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED staging tenant(s) unreachable — exceeds 50% threshold. Likely real outage, not teardown race." + exit 1 + fi + if [ $STALE_COUNT -gt 0 ]; then echo "::error::$STALE_COUNT staging tenant(s) returned a stale SHA. ssm_status=Success was misleading — see job summary." exit 1 From 9b909c4459f8724bc9ab53866eae419a784ba9b5 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:40:31 -0700 Subject: [PATCH 10/21] fix(ci): gate 50%-floor on TOTAL_VERIFIED >= 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of #2403 caught a regression: with a 1-tenant fleet (the exact case the original #2402 fix targeted), the new floor would re-introduce the flake. Trace: TOTAL=1, UNREACHABLE=1, $((1/2))=0 if 1 -gt 0 → TRUE → exit 1 The 50%-rule only meaningfully distinguishes "real outage" from "teardown race" when the fleet is large enough that "half down" is statistically meaningful. With 1-3 tenants, canary-verify is the actual gate (it runs against the canary first and aborts the rollout if the canary fails to come up). Gate the floor on TOTAL_VERIFIED >= 4. Truth table: TOTAL UNREACHABLE RESULT 1 1 soft-warn (original e2e flake case) 4 2 soft-warn (exactly half) 4 3 hard-fail (75% — real outage) 10 6 hard-fail (60% — real outage) Mirrored across staging.yml + main.yml. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workflows/redeploy-tenants-on-main.yml | 9 +++++---- .../workflows/redeploy-tenants-on-staging.yml | 19 +++++++++++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/.github/workflows/redeploy-tenants-on-main.yml b/.github/workflows/redeploy-tenants-on-main.yml index 466c0249..46743347 100644 --- a/.github/workflows/redeploy-tenants-on-main.yml +++ b/.github/workflows/redeploy-tenants-on-main.yml @@ -308,11 +308,12 @@ jobs: fi # Belt-and-suspenders sanity floor: same logic as the staging - # variant — if MORE than half the prod fleet is unreachable, - # this is a real outage, not a teardown race. Hard-fail. + # variant — see that file's comment for the full rationale. + # Floor only applies when fleet >= 4; below that, canary-verify + # is the actual gate. TOTAL_VERIFIED=${#SLUGS[@]} - if [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then - echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED tenant(s) unreachable — exceeds 50% threshold. Likely real outage, not teardown race." + if [ $TOTAL_VERIFIED -ge 4 ] && [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then + echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED tenant(s) unreachable — exceeds 50% threshold on a fleet large enough that this signals a real outage, not teardown race." exit 1 fi diff --git a/.github/workflows/redeploy-tenants-on-staging.yml b/.github/workflows/redeploy-tenants-on-staging.yml index 381d0a65..7f191e8d 100644 --- a/.github/workflows/redeploy-tenants-on-staging.yml +++ b/.github/workflows/redeploy-tenants-on-staging.yml @@ -285,13 +285,20 @@ jobs: fi # Belt-and-suspenders sanity floor: if MORE than half the fleet is - # unreachable, this isn't a teardown race — it's a real outage - # (e.g. the new image crashes on startup). Hard-fail. Canary-verify - # would catch this on the canary tenant first; this guard is a - # fallback for canary-skip dispatches and same-batch races. + # unreachable AND the fleet is large enough that "half down" is + # statistically meaningful, this is a real outage (e.g. new image + # crashes on startup), not a teardown race. Hard-fail. + # + # Floor only applies when TOTAL_VERIFIED >= 4 — below that, the + # canary-verify step is the actual gate for "all tenants down" + # detection (it runs against the canary first and aborts the + # rollout if the canary fails to come up). Without the >=4 gate, + # a 1-tenant fleet (e.g. a single ephemeral e2e-* tenant on a + # quiet staging push) would re-flake on the exact teardown-race + # condition #2402 fixed: 1 of 1 unreachable = 100% > 50% → fail. TOTAL_VERIFIED=${#SLUGS[@]} - if [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then - echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED staging tenant(s) unreachable — exceeds 50% threshold. Likely real outage, not teardown race." + if [ $TOTAL_VERIFIED -ge 4 ] && [ $UNREACHABLE_COUNT -gt $((TOTAL_VERIFIED / 2)) ]; then + echo "::error::$UNREACHABLE_COUNT of $TOTAL_VERIFIED staging tenant(s) unreachable — exceeds 50% threshold on a fleet large enough that this signals a real outage, not teardown race." exit 1 fi From ef206b5be6cce3a14ff3f7b6580f0c950be0d82f Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:51:01 -0700 Subject: [PATCH 11/21] refactor(ci): extract wheel smoke into shared script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit publish-runtime.yml had a broad smoke (AgentCard call-shape, well-known mount alignment, new_text_message) inline as a heredoc. runtime-prbuild- compat.yml had a narrow inline smoke (just `from main import main_sync`). Result: a PR could introduce SDK shape regressions that pass at PR time and only fail at publish time, post-merge. Extract the broad smoke into scripts/wheel_smoke.py and invoke it from both workflows. PR-time gate now matches publish-time gate — same script, same assertions. Eliminates the drift hazard of two heredocs that have to be kept in lockstep manually. Verified locally: * Built wheel from workspace/ source, installed in venv, ran smoke → pass * Simulated AgentCard kwarg-rename regression → smoke catches it as `ValueError: Protocol message AgentCard has no "supported_interfaces" field` (the exact failure mode of #2179 / supported_protocols incident) Path filter for runtime-prbuild-compat extended to include scripts/wheel_smoke.py so smoke-only edits get PR-validated. publish- runtime path filter intentionally NOT extended — smoke-only edits should not auto-trigger a PyPI version bump. Subset of #131 (the broader "invoke main() against stub config" goal remains pending — main() needs a config dir + stub platform server). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/publish-runtime.yml | 134 +---------------- .github/workflows/runtime-prbuild-compat.yml | 10 +- scripts/wheel_smoke.py | 145 +++++++++++++++++++ 3 files changed, 157 insertions(+), 132 deletions(-) create mode 100644 scripts/wheel_smoke.py diff --git a/.github/workflows/publish-runtime.yml b/.github/workflows/publish-runtime.yml index be59fe6c..5cd20a7a 100644 --- a/.github/workflows/publish-runtime.yml +++ b/.github/workflows/publish-runtime.yml @@ -154,139 +154,15 @@ jobs: - name: Verify package contents (sanity) working-directory: ${{ runner.temp }}/runtime-build + # Smoke logic lives in scripts/wheel_smoke.py so the same gate runs + # at both PR-time (runtime-prbuild-compat.yml) and publish-time + # (here). Splitting the smoke across two heredocs let them drift + # apart historically — one script keeps them locked. run: | python -m twine check dist/* - # Smoke-import the built wheel to catch import-rewrite mistakes - # before they hit PyPI. Asserts on STABLE INVARIANTS only — - # symbols + classes that are part of the package's public - # contract (BaseAdapter interface, the canonical a2a sentinel, - # core submodules). Don't add feature-flag-style assertions - # here — they fire false-positive every time staging is mid- - # release of that feature. python -m venv /tmp/smoke /tmp/smoke/bin/pip install --quiet dist/*.whl - WORKSPACE_ID=00000000-0000-0000-0000-000000000000 \ - PLATFORM_URL=http://localhost:8080 \ - /tmp/smoke/bin/python -c " - # Importing main is the strongest smoke test we can do here: - # main.py is the entry point and pulls every other module - # transitively. If the build script missed an import rewrite - # (e.g. left a bare \`from transcript_auth import ...\` instead - # of \`from molecule_runtime.transcript_auth import ...\` — the - # 0.1.16 incident), this fails with ModuleNotFoundError instead - # of shipping to PyPI and breaking every workspace startup. - # Import the entry-point target by NAME — not just the module. - # The wheel's pyproject.toml declares - # `molecule-runtime = molecule_runtime.main:main_sync` so if - # main_sync goes missing (it did in 0.1.16-0.1.18), every - # workspace startup fails with `ImportError: cannot import name - # 'main_sync'`. Plain `import molecule_runtime.main` doesn't - # catch that because the module loads fine. - from molecule_runtime.main import main_sync # noqa: F401 - from molecule_runtime import a2a_client, a2a_tools - from molecule_runtime.builtin_tools import memory - from molecule_runtime.adapters import get_adapter, BaseAdapter, AdapterConfig - # Stable invariants: package exports + BaseAdapter shape. - assert a2a_client._A2A_ERROR_PREFIX, 'a2a_client missing error sentinel' - assert callable(get_adapter), 'adapters.get_adapter must be callable' - assert hasattr(BaseAdapter, 'name'), 'BaseAdapter interface broken' - assert hasattr(AdapterConfig, '__init__'), 'AdapterConfig dataclass missing' - - # Call-shape smoke for AgentCard. Pure imports don't catch - # field-shape regressions in upstream SDKs that only surface - # at construction time. Two bugs of this exact class shipped - # since the a2a-sdk 1.0 migration: - # - state_transition_history=True (fixed in #2179) - # - supported_protocols=[...] (the protobuf field is - # supported_interfaces — caused every workspace boot - # to crash with `ValueError: Protocol message AgentCard - # has no "supported_protocols" field`; fixed alongside - # this smoke) - # - # This block instantiates the EXACT classes main.py uses, - # with the EXACT keyword arguments. If a future a2a-sdk - # upgrade renames any of supported_interfaces / streaming / - # push_notifications / etc., the publish fails here instead - # of breaking every workspace startup. main.py and this - # smoke MUST stay in lockstep — adding a kwarg to one - # without mirroring it here is the regression vector. - from a2a.types import AgentCard, AgentCapabilities, AgentSkill, AgentInterface - AgentCard( - name='smoke-agent', - description='publish-runtime smoke test', - version='0.0.0-smoke', - supported_interfaces=[ - AgentInterface(protocol_binding='https://a2a.g/v1', url='http://localhost:8080'), - ], - capabilities=AgentCapabilities( - streaming=True, - push_notifications=False, - ), - skills=[ - AgentSkill( - id='smoke-skill', - name='Smoke', - description='no-op', - tags=['smoke'], - examples=['noop'], - ), - ], - default_input_modes=['text/plain', 'application/json'], - default_output_modes=['text/plain', 'application/json'], - ) - print('✓ AgentCard call-shape smoke passed') - - # Well-known agent-card path probe alignment. main.py's - # _send_initial_prompt() polls AGENT_CARD_WELL_KNOWN_PATH - # to know when the local A2A server is ready. If the SDK - # ever splits the constant value from the path that - # create_agent_card_routes() actually mounts at, every - # workspace silently drops its initial_prompt: - # - Probe gets 404 every attempt. - # - Falls through to 'server not ready after 30s, - # skipping' even though the server is fine. - # - The user hits a fresh chat with no kickoff context. - # This was the #2193 incident class — the v0.x → v1.x - # rename of /.well-known/agent.json → /.well-known/agent-card.json - # plus the constant itself moving to a2a.utils.constants. - # source-tree pytest (test_agent_card_well_known_path.py) - # catches main.py-side regressions; this catches the - # SDK-side ones BEFORE PyPI upload. - from a2a.utils.constants import AGENT_CARD_WELL_KNOWN_PATH - from a2a.server.routes import create_agent_card_routes - mounted_paths = [ - getattr(r, 'path', None) - for r in create_agent_card_routes( - AgentCard( - name='wk-smoke', - description='well-known mount alignment', - version='0.0.0-smoke', - ) - ) - ] - assert AGENT_CARD_WELL_KNOWN_PATH in mounted_paths, ( - f'AGENT_CARD_WELL_KNOWN_PATH ({AGENT_CARD_WELL_KNOWN_PATH!r}) ' - f'is NOT among paths mounted by create_agent_card_routes ' - f'({mounted_paths!r}). The SDK constant and its own route ' - f'factory have drifted — workspace probes will 404 forever, ' - f'silently dropping every workspace initial_prompt.' - ) - print(f'✓ well-known mount alignment OK ({AGENT_CARD_WELL_KNOWN_PATH})') - - # Message helper smoke. a2a-sdk renamed - # new_agent_text_message → new_text_message in the v1.x - # protobuf-flat migration (per the v0→v1 cheat sheet). main.py - # and a2a_executor.py call new_text_message in hot paths; if - # the import breaks, every reply errors with ImportError before - # the message even leaves the workspace. Importing here - # catches a future v2.x rename at publish time. - from a2a.helpers import new_text_message - msg = new_text_message('smoke') - assert msg is not None, 'new_text_message returned None' - print('✓ message helper import + call OK') - - print('✓ smoke import passed') - " + /tmp/smoke/bin/python "$GITHUB_WORKSPACE/scripts/wheel_smoke.py" - name: Publish to PyPI (Trusted Publisher / OIDC) # PyPI side is configured: project molecule-ai-workspace-runtime → diff --git a/.github/workflows/runtime-prbuild-compat.yml b/.github/workflows/runtime-prbuild-compat.yml index aad6e929..96f1a289 100644 --- a/.github/workflows/runtime-prbuild-compat.yml +++ b/.github/workflows/runtime-prbuild-compat.yml @@ -34,12 +34,14 @@ on: # changes (it controls the wheel layout). - 'workspace/**' - 'scripts/build_runtime_package.py' + - 'scripts/wheel_smoke.py' - '.github/workflows/runtime-prbuild-compat.yml' pull_request: branches: [main, staging] paths: - 'workspace/**' - 'scripts/build_runtime_package.py' + - 'scripts/wheel_smoke.py' - '.github/workflows/runtime-prbuild-compat.yml' workflow_dispatch: # Required-check support: when this becomes a branch-protection gate, @@ -94,7 +96,9 @@ jobs: /tmp/venv-built/bin/pip show molecule-ai-workspace-runtime a2a-sdk \ | grep -E '^(Name|Version):' - name: Smoke import the PR-built wheel - env: - WORKSPACE_ID: 00000000-0000-0000-0000-000000000001 + # Same script publish-runtime.yml runs against the to-be-PyPI wheel. + # Closes the PR-time vs publish-time gap: a PR adding a new SDK + # call-shape no longer passes here (narrow `import main_sync`) only + # to fail post-merge in publish-runtime's broader smoke. run: | - /tmp/venv-built/bin/python -c "from molecule_runtime.main import main_sync; print('PR-built runtime imports OK')" + /tmp/venv-built/bin/python "$GITHUB_WORKSPACE/scripts/wheel_smoke.py" diff --git a/scripts/wheel_smoke.py b/scripts/wheel_smoke.py new file mode 100644 index 00000000..32db3350 --- /dev/null +++ b/scripts/wheel_smoke.py @@ -0,0 +1,145 @@ +#!/usr/bin/env python3 +"""Smoke-test an installed molecule-ai-workspace-runtime wheel. + +Runs the same invariant assertions in two workflows: + * publish-runtime.yml — after building dist/*.whl, before PyPI upload + * runtime-prbuild-compat.yml — after building the PR's wheel, before merge + +Splitting the smoke across two inline heredocs let PR-time and publish-time +drift apart. After 2026-04 we kept hitting publish-time failures for +regressions a PR-time check could have caught. One script, both gates. + +Failure here intentionally exits non-zero so the workflow's `run:` step fails. +Each block prints a single ✓ line on success so the GH summary log stays +readable; assertion errors propagate with their own message. + +Run directly: `python scripts/wheel_smoke.py` after `pip install `. +""" + +import os +import sys + + +def smoke_imports_and_invariants() -> None: + """Module imports + stable contract assertions. + + Importing main_sync by name is the strongest pre-PyPI gate we have for + import-rewrite mistakes (the 0.1.16 incident, where main.py loaded but + main_sync was missing because the build script dropped a re-export). + """ + from molecule_runtime.main import main_sync # noqa: F401 + from molecule_runtime import a2a_client, a2a_tools # noqa: F401 + from molecule_runtime.builtin_tools import memory # noqa: F401 + from molecule_runtime.adapters import get_adapter, BaseAdapter, AdapterConfig + + assert a2a_client._A2A_ERROR_PREFIX, "a2a_client missing error sentinel" + assert callable(get_adapter), "adapters.get_adapter must be callable" + assert hasattr(BaseAdapter, "name"), "BaseAdapter interface broken" + assert hasattr(AdapterConfig, "__init__"), "AdapterConfig dataclass missing" + print("✓ module imports + invariants OK") + + +def smoke_agent_card_call_shape() -> None: + """Construct AgentCard with the EXACT kwargs main.py uses. + + Pure imports don't catch field-shape regressions in upstream SDKs that + only surface at construction time. Two bugs of this exact class shipped + since the a2a-sdk 1.0 migration: + - state_transition_history=True (#2179) + - supported_protocols=[...] (the protobuf field is supported_interfaces; + every workspace boot crashed with `ValueError: Protocol message + AgentCard has no "supported_protocols" field`) + + main.py and this block MUST stay in lockstep — adding a kwarg there + without mirroring it here is the regression vector. + """ + from a2a.types import AgentCard, AgentCapabilities, AgentSkill, AgentInterface + + AgentCard( + name="smoke-agent", + description="wheel-smoke: AgentCard call-shape", + version="0.0.0-smoke", + supported_interfaces=[ + AgentInterface(protocol_binding="https://a2a.g/v1", url="http://localhost:8080"), + ], + capabilities=AgentCapabilities( + streaming=True, + push_notifications=False, + ), + skills=[ + AgentSkill( + id="smoke-skill", + name="Smoke", + description="no-op", + tags=["smoke"], + examples=["noop"], + ), + ], + default_input_modes=["text/plain", "application/json"], + default_output_modes=["text/plain", "application/json"], + ) + print("✓ AgentCard call-shape smoke passed") + + +def smoke_well_known_path_alignment() -> None: + """The SDK's published constant must match the path it actually mounts. + + main.py polls AGENT_CARD_WELL_KNOWN_PATH to detect server readiness. If + the constant and create_agent_card_routes() drift, every workspace's + initial_prompt silently drops (probe 404s, falls through to "skipping"). + This was the #2193 incident class. + """ + from a2a.types import AgentCard + from a2a.utils.constants import AGENT_CARD_WELL_KNOWN_PATH + from a2a.server.routes import create_agent_card_routes + + mounted_paths = [ + getattr(r, "path", None) + for r in create_agent_card_routes( + AgentCard( + name="wk-smoke", + description="well-known mount alignment", + version="0.0.0-smoke", + ) + ) + ] + assert AGENT_CARD_WELL_KNOWN_PATH in mounted_paths, ( + f"AGENT_CARD_WELL_KNOWN_PATH ({AGENT_CARD_WELL_KNOWN_PATH!r}) is NOT among " + f"paths mounted by create_agent_card_routes ({mounted_paths!r}). The SDK " + "constant and its own route factory have drifted — workspace probes will " + "404 forever, silently dropping every workspace initial_prompt." + ) + print(f"✓ well-known mount alignment OK ({AGENT_CARD_WELL_KNOWN_PATH})") + + +def smoke_message_helper() -> None: + """new_text_message is the v1.x rename of new_agent_text_message. + + main.py and a2a_executor.py call new_text_message in hot paths; if the + import breaks, every reply errors with ImportError before the message + even leaves the workspace. Importing here catches a future v2.x rename + at publish time. + """ + from a2a.helpers import new_text_message + + msg = new_text_message("smoke") + assert msg is not None, "new_text_message returned None" + print("✓ message helper import + call OK") + + +def main() -> int: + # main.py validates WORKSPACE_ID at module-import time via platform_auth. + # Set placeholders so the smoke doesn't trip on the env-var guard. + os.environ.setdefault("WORKSPACE_ID", "00000000-0000-0000-0000-000000000000") + os.environ.setdefault("PLATFORM_URL", "http://localhost:8080") + + smoke_imports_and_invariants() + smoke_agent_card_call_shape() + smoke_well_known_path_alignment() + smoke_message_helper() + print("✓ wheel smoke passed") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 0af4012f7914496fa9c7df26907f3141306632a9 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:57:27 -0700 Subject: [PATCH 12/21] feat(tests): add run-all-replays.sh harness runner Boots the harness, runs every script under replays/, tracks pass/fail, and tears down on exit. Closes the README's TODO for the harness runner that the per-replay-registration comment referenced. Usage: ./run-all-replays.sh # boot, run, teardown KEEP_UP=1 ./run-all-replays.sh # leave harness running on exit REBUILD=1 ./run-all-replays.sh # rebuild images before booting Trap-on-EXIT teardown ensures partial-failure runs don't leak Docker resources. Returns non-zero if any replay failed; CI can adopt this as a single command without per-replay registration. Phase 2 picks this up to wire harness-based E2E as a required check. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/harness/README.md | 17 ++++-- tests/harness/run-all-replays.sh | 90 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) create mode 100755 tests/harness/run-all-replays.sh diff --git a/tests/harness/README.md b/tests/harness/README.md index d586d36b..1306d8ae 100644 --- a/tests/harness/README.md +++ b/tests/harness/README.md @@ -44,6 +44,15 @@ cd tests/harness ./down.sh # tear down + remove volumes ``` +To run every replay in one shot (boot, seed, run-all, teardown): + +```bash +cd tests/harness +./run-all-replays.sh # full lifecycle; non-zero exit if any replay fails +KEEP_UP=1 ./run-all-replays.sh # leave harness up for debugging +REBUILD=1 ./run-all-replays.sh # rebuild images before booting +``` + First-time setup needs an `/etc/hosts` entry so `harness-tenant.localhost` resolves to the local cf-proxy: @@ -71,8 +80,8 @@ To add a new replay: 2. The script's purpose: reproduce the production failure mode against the harness, then assert the fix is present. PASS criterion is the post-fix behavior. -3. Wire it into the `tests/harness/run-all-replays.sh` runner (TODO, - Phase 2). +3. The `run-all-replays.sh` runner picks up every `replays/*.sh` script + automatically — no per-replay registration needed. ## Extending the cp-stub @@ -102,9 +111,9 @@ its mandate of "exercise the tenant binary in production-shape topology." ## Roadmap -- **Phase 1 (this PR):** harness + cp-stub + cf-proxy + 2 replays. +- **Phase 1 (shipped):** harness + cp-stub + cf-proxy + 2 replays + `run-all-replays.sh` runner. - **Phase 2:** convert `tests/e2e/test_api.sh` to run against the harness instead of localhost. Make harness-based E2E a required CI - check. + check (a workflow that invokes `run-all-replays.sh` on every PR). - **Phase 3:** config-coherence lint that diffs harness env list against production CP's env list, fails CI on drift. diff --git a/tests/harness/run-all-replays.sh b/tests/harness/run-all-replays.sh new file mode 100755 index 00000000..092158c3 --- /dev/null +++ b/tests/harness/run-all-replays.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# Run every replay under tests/harness/replays/ against a fresh harness. +# +# Boots the harness (up.sh + seed.sh), runs each `replays/*.sh` in +# alphabetical order, tracks pass/fail, and tears down on exit. Returns +# non-zero if any replay failed. +# +# Usage: +# ./run-all-replays.sh # boot, run, teardown +# KEEP_UP=1 ./run-all-replays.sh # leave harness running on exit (debug) +# REBUILD=1 ./run-all-replays.sh # rebuild images before booting +# +# CI usage: invoke without flags. The trap-on-EXIT teardown ensures we +# don't leak Docker resources when a replay fails partway through. + +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$HERE" + +REPLAYS_DIR="$HERE/replays" +if [ ! -d "$REPLAYS_DIR" ]; then + echo "[run-all] no replays/ directory at $REPLAYS_DIR — nothing to run" + exit 1 +fi + +shopt -s nullglob +REPLAYS=("$REPLAYS_DIR"/*.sh) +shopt -u nullglob +if [ ${#REPLAYS[@]} -eq 0 ]; then + echo "[run-all] replays/ is empty — nothing to run" + exit 1 +fi + +cleanup() { + local exit_code=$? + if [ "${KEEP_UP:-0}" = "1" ]; then + echo "" + echo "[run-all] KEEP_UP=1 — leaving harness up. Tear down manually with ./down.sh" + else + echo "" + echo "[run-all] tearing down harness..." + ./down.sh >/dev/null 2>&1 || echo "[run-all] WARN: ./down.sh exited non-zero" + fi + exit "$exit_code" +} +trap cleanup EXIT INT TERM + +echo "[run-all] booting harness..." +if [ "${REBUILD:-0}" = "1" ]; then + ./up.sh --rebuild +else + ./up.sh +fi + +echo "[run-all] seeding workspaces..." +./seed.sh + +PASS_COUNT=0 +FAIL_COUNT=0 +SKIP_COUNT=0 +FAILED_NAMES=() + +for replay in "${REPLAYS[@]}"; do + name=$(basename "$replay" .sh) + echo "" + echo "[run-all] ━━━ $name ━━━" + if bash "$replay"; then + # Replays signal "skip" by exiting 0 with a __SKIP__ marker in stdout — + # but we capture that as a pass here since the script exited 0. The + # skip is documented in the script's own output. CI uses pass/fail. + PASS_COUNT=$((PASS_COUNT + 1)) + echo "[run-all] PASS: $name" + else + FAIL_COUNT=$((FAIL_COUNT + 1)) + FAILED_NAMES+=("$name") + echo "[run-all] FAIL: $name" + fi +done + +echo "" +echo "[run-all] =============================" +echo "[run-all] Replay summary: ${PASS_COUNT} passed, ${FAIL_COUNT} failed (of ${#REPLAYS[@]} total)" +if [ ${FAIL_COUNT} -gt 0 ]; then + echo "[run-all] Failed:" + for name in "${FAILED_NAMES[@]}"; do + echo "[run-all] - $name" + done + exit 1 +fi +echo "[run-all] All replays passed." From fc3b5fd385da7f07c3e5d84c6594d39e2d79389f Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 11:59:24 -0700 Subject: [PATCH 13/21] feat(canvas): add /api/buildinfo for version-display parity with tenant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workspace-server has GET /buildinfo (PR #2398) — `curl https://. moleculesai.app/buildinfo` returns the live git SHA. Canvas had no parallel: debugging "is this the deployed code?" required reading Vercel's UI or response headers (deployment ID, not git SHA). Add canvas /api/buildinfo returning {git_sha, git_ref, vercel_env} sourced from VERCEL_GIT_COMMIT_SHA / _REF / VERCEL_ENV — Vercel injects these at build time from the deploying commit. Outside Vercel (local `next dev`, harness) all three are unset and the endpoint returns `git_sha: "dev"`, the same sentinel workspace-server uses pre-ldflags- injection. Now both surfaces speak the same vocabulary: curl https://.moleculesai.app/buildinfo curl https://canvas.moleculesai.app/api/buildinfo 3 tests cover dev-fallback, Vercel-injected SHA pass-through, and JSON content type. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../app/api/buildinfo/__tests__/route.test.ts | 48 +++++++++++++++++++ canvas/src/app/api/buildinfo/route.ts | 18 +++++++ 2 files changed, 66 insertions(+) create mode 100644 canvas/src/app/api/buildinfo/__tests__/route.test.ts create mode 100644 canvas/src/app/api/buildinfo/route.ts diff --git a/canvas/src/app/api/buildinfo/__tests__/route.test.ts b/canvas/src/app/api/buildinfo/__tests__/route.test.ts new file mode 100644 index 00000000..ac2d8f7b --- /dev/null +++ b/canvas/src/app/api/buildinfo/__tests__/route.test.ts @@ -0,0 +1,48 @@ +/** + * Canvas /api/buildinfo — version-display endpoint mirroring + * workspace-server's /buildinfo. Lets `curl /api/buildinfo` + * confirm which git SHA is live on a canvas deployment. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { GET } from "../route"; + +const ENV_KEYS = ["VERCEL_GIT_COMMIT_SHA", "VERCEL_GIT_COMMIT_REF", "VERCEL_ENV"]; + +describe("GET /api/buildinfo", () => { + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + for (const k of ENV_KEYS) delete process.env[k]; + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + }); + + it("returns dev sentinel when Vercel env vars are unset", async () => { + const res = await GET(); + const body = await res.json(); + expect(body).toEqual({ git_sha: "dev", git_ref: "", vercel_env: "local" }); + }); + + it("reports the SHA Vercel injected at build time", async () => { + process.env.VERCEL_GIT_COMMIT_SHA = "abc1234567890"; + process.env.VERCEL_GIT_COMMIT_REF = "main"; + process.env.VERCEL_ENV = "production"; + const res = await GET(); + const body = await res.json(); + expect(body.git_sha).toBe("abc1234567890"); + expect(body.git_ref).toBe("main"); + expect(body.vercel_env).toBe("production"); + }); + + it("returns 200 status and JSON content type", async () => { + const res = await GET(); + expect(res.status).toBe(200); + expect(res.headers.get("content-type")).toContain("application/json"); + }); +}); diff --git a/canvas/src/app/api/buildinfo/route.ts b/canvas/src/app/api/buildinfo/route.ts new file mode 100644 index 00000000..a8ff8aab --- /dev/null +++ b/canvas/src/app/api/buildinfo/route.ts @@ -0,0 +1,18 @@ +import { NextResponse } from "next/server"; + +// Mirror of workspace-server's GET /buildinfo (PR #2398). Lets a developer +// confirm which git SHA is live on a canvas deployment with the same +// `curl /buildinfo` flow they use against tenant workspaces. +// +// Vercel injects VERCEL_GIT_COMMIT_SHA / _REF / VERCEL_ENV at build time +// from the deploying commit; outside Vercel (local `next dev`, harness) +// these are unset and the endpoint reports `git_sha: "dev"`. Same sentinel +// the workspace-server uses pre-ldflags-injection so both surfaces speak +// the same vocabulary. +export async function GET() { + return NextResponse.json({ + git_sha: process.env.VERCEL_GIT_COMMIT_SHA ?? "dev", + git_ref: process.env.VERCEL_GIT_COMMIT_REF ?? "", + vercel_env: process.env.VERCEL_ENV ?? "local", + }); +} From 235aca9908c47d9a76687d7c66586e43add3acfc Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 12:05:40 -0700 Subject: [PATCH 14/21] =?UTF-8?q?fix(boot):=20always=20start=20health-swee?= =?UTF-8?q?p=20goroutine=20=E2=80=94=20SaaS=20tenants=20need=20it=20for=20?= =?UTF-8?q?external-runtime=20liveness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-fix, cmd/server/main.go gated the entire health-sweep goroutine on `prov != nil`. On SaaS tenants (`MOLECULE_ORG_ID` set) the local Docker provisioner is never initialized — only `cpProv`. So the goroutine never started, and `sweepStaleRemoteWorkspaces` (which transitions runtime='external' workspaces from 'online' to 'awaiting_agent' when their last_heartbeat_at goes stale) never ran. Net effect on production: every external-runtime workspace on SaaS that lost its agent stayed 'online' indefinitely instead of falling back to 'awaiting_agent' (re-registrable). The drift gate (#2388) caught the migration side and #2382 fixed the SQL writes, but this orchestration-side gate slipped through both because there was no SaaS-mode E2E coverage on the heartbeat-loss → awaiting_agent transition. Caught by #2392 (live staging external-runtime regression E2E) failing at step 6 — 180s with no heartbeat, expected status=awaiting_agent, got online. Fix: drop the `if prov != nil` gate. `StartHealthSweep` already handles nil checker correctly (healthsweep.go:50-71): the Docker sweep is gated inside the loop, the remote sweep always runs. Test coverage already exists at TestStartHealthSweep_NilCheckerRunsRemoteSweep. After this lands and tenants redeploy, #2392 step 6 passes and the regression coverage closes. Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace-server/cmd/server/main.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/workspace-server/cmd/server/main.go b/workspace-server/cmd/server/main.go index d0d5ae57..f620537b 100644 --- a/workspace-server/cmd/server/main.go +++ b/workspace-server/cmd/server/main.go @@ -223,13 +223,24 @@ func main() { registry.StartLivenessMonitor(c, onWorkspaceOffline) }) - // Proactive container health sweep — detects dead containers faster than Redis TTL. - // Checks all "online" workspaces against Docker every 15 seconds. - if prov != nil { - go supervised.RunWithRecover(ctx, "health-sweep", func(c context.Context) { - registry.StartHealthSweep(c, prov, 15*time.Second, onWorkspaceOffline) - }) - } + // Proactive health sweep — two passes per tick: + // 1. Docker-side: checks "online" workspaces against the local Docker + // daemon (only runs when prov is non-nil, i.e. self-hosted mode). + // 2. Remote-side: scans runtime='external' rows whose last_heartbeat_at + // is past REMOTE_LIVENESS_STALE_AFTER and flips them to + // awaiting_agent. Runs regardless of provisioner mode — SaaS + // tenants need this even though they don't run Docker locally, + // because external-runtime workspaces are operator-managed and + // the platform-side liveness sweep is the only thing that + // transitions them off 'online' when the operator's CLI dies. + // + // Pre-2026-04-30 this goroutine was gated on prov != nil, which silently + // disabled the remote-side sweep on every SaaS tenant. The function in + // healthsweep.go has always handled nil checker correctly; only the + // orchestration was wrong. See #2392's CI failure for the trace. + go supervised.RunWithRecover(ctx, "health-sweep", func(c context.Context) { + registry.StartHealthSweep(c, prov, 15*time.Second, onWorkspaceOffline) + }) // Orphan-container reconcile sweep — finds running containers // whose workspace row is already status='removed' and stops From 8516a8f9c65dd210b9b1a242d7544c437557ce49 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 12:54:51 -0700 Subject: [PATCH 15/21] fix(tenant-guard): allowlist /buildinfo so redeploy verifier can reach it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /buildinfo route added in #2398 to verify each tenant runs the published SHA was 404'd by TenantGuard on every production tenant — the allowlist had /health, /metrics, /registry/register, /registry/heartbeat, but not /buildinfo. The redeploy workflows curl /buildinfo from a CI runner with no X-Molecule-Org-Id header, TenantGuard 404'd them, gin's NoRoute proxied to canvas, canvas returned its HTML 404 page, jq read empty git_sha, and the verifier silently soft-warned every tenant as "unreachable" — which the workflow doesn't fail on. Confirmed externally: curl https://hongmingwang.moleculesai.app/buildinfo → HTTP 404 + Content-Type: text/html (Next.js "404: This page could not be found.") even though /health on the same host returns {"status":"ok"} from gin. The buildinfo package's own doc already declares /buildinfo public by design ("Public is intentional: it's a build identifier, not operational state. The same string is already published as org.opencontainers.image.revision on the container image, so no new info is exposed.") — the allowlist just missed it. Pin the alignment in tenant_guard_test.go: TestTenantGuard_AllowlistBypassesCheck now asserts /buildinfo returns 200 without an org header alongside /health and /metrics, so a future allowlist edit can't silently regress the verifier again. Closes the silent-success failure mode: stale tenants will now show up as STALE (hard-fail) rather than UNREACHABLE (soft-warn). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/middleware/tenant_guard.go | 1 + .../internal/middleware/tenant_guard_test.go | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/workspace-server/internal/middleware/tenant_guard.go b/workspace-server/internal/middleware/tenant_guard.go index 4aaf7a4c..1363060d 100644 --- a/workspace-server/internal/middleware/tenant_guard.go +++ b/workspace-server/internal/middleware/tenant_guard.go @@ -53,6 +53,7 @@ const tenantOrgIDHeader = "X-Molecule-Org-Id" // here only bypasses the cross-org routing check, not auth. var tenantGuardAllowlist = map[string]struct{}{ "/health": {}, + "/buildinfo": {}, "/metrics": {}, "/registry/register": {}, "/registry/heartbeat": {}, diff --git a/workspace-server/internal/middleware/tenant_guard_test.go b/workspace-server/internal/middleware/tenant_guard_test.go index 5d2b4731..c2ab5792 100644 --- a/workspace-server/internal/middleware/tenant_guard_test.go +++ b/workspace-server/internal/middleware/tenant_guard_test.go @@ -8,13 +8,15 @@ import ( "github.com/gin-gonic/gin" ) -// helper: build a router with TenantGuard configured to `orgID` and two -// representative routes — a regular API route and two allowlisted ones. +// helper: build a router with TenantGuard configured to `orgID` and a +// representative API route plus the public allowlisted ones (/health, +// /buildinfo, /metrics). func newGuardedRouter(orgID string) *gin.Engine { gin.SetMode(gin.TestMode) r := gin.New() r.Use(TenantGuardWithOrgID(orgID)) r.GET("/health", func(c *gin.Context) { c.String(200, "ok") }) + r.GET("/buildinfo", func(c *gin.Context) { c.String(200, "buildinfo") }) r.GET("/metrics", func(c *gin.Context) { c.String(200, "metrics") }) r.GET("/workspaces", func(c *gin.Context) { c.String(200, "workspaces") }) return r @@ -71,10 +73,14 @@ func TestTenantGuard_MissingHeaderIs404(t *testing.T) { } // Allowlisted paths bypass the guard even in tenant mode — required for health -// probes (Fly Machines checks) and Prometheus scrape. +// probes (Fly Machines checks), Prometheus scrape, and the redeploy-fleet +// /buildinfo verification step. /buildinfo without an org header used to +// 404-via-NoRoute → canvas (HTML), which made the redeploy verifier think +// every tenant was stale even when the binary was current. Pin this so a +// future allowlist edit can't silently regress that check. func TestTenantGuard_AllowlistBypassesCheck(t *testing.T) { r := newGuardedRouter("org-abc") - for _, path := range []string{"/health", "/metrics"} { + for _, path := range []string{"/health", "/buildinfo", "/metrics"} { w := doRequest(r, path, "") // no header if w.Code != 200 { t.Errorf("%s: allowlisted path should return 200 without header, got %d", path, w.Code) From 3105e87cf73bea689987e8f69b17ed2c6db3be2b Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:04:53 -0700 Subject: [PATCH 16/21] ci: gate PRs on tests/harness/run-all-replays.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap between "the harness exists" and "the harness blocks bugs." Phase 2 of the harness roadmap (per tests/harness/README.md): make harness-based E2E a required CI check on every PR touching the tenant binary or the harness itself. Trigger: push + pull_request to staging+main, paths-filtered to workspace-server/**, canvas/**, tests/harness/**, and this workflow. merge_group support included so this becomes branch-protectable. Single-job-with-conditional-steps pattern (matches e2e-api.yml). One check run regardless of paths-filter outcome; satisfies branch protection cleanly per the PR #2264 SKIPPED-in-set finding. Why this exists: 2026-04-30 we shipped a TenantGuard allowlist gap (/buildinfo added to router.go in #2398, never added to the allowlist) that the existing buildinfo-stale-image.sh replay would have caught. The harness was wired correctly; nobody ran it. Replays as a discipline beat replays as a memory item. The CI pipeline: detect-changes (paths filter) └ harness-replays (always) ├ no-op pass when paths-filter says no relevant change └ otherwise: checkout + sibling plugin checkout + /etc/hosts entry + run-all-replays.sh + compose-logs-on-failure + force-teardown Compose logs from tenant/cp-stub/cf-proxy/postgres are dumped on failure so a CI red is debuggable without re-reproducing locally. The trap in run-all-replays.sh handles teardown; the always-run down.sh step is a belt-and-suspenders against trap-bypass kills. Follow-ups (not in this PR): - Add this check to staging branch protection once it's been green for a few PRs (the new-workflow-instability hedge that other gates followed). - Eventually wire the buildx GHA cache to speed up tenant image builds — currently every PR rebuilds the full Dockerfile.tenant (Go + Next.js + template clones) from scratch. Acceptable for now; optimize when the timeout-minutes:30 ceiling becomes painful. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/harness-replays.yml | 148 ++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 .github/workflows/harness-replays.yml diff --git a/.github/workflows/harness-replays.yml b/.github/workflows/harness-replays.yml new file mode 100644 index 00000000..caf5fe77 --- /dev/null +++ b/.github/workflows/harness-replays.yml @@ -0,0 +1,148 @@ +name: Harness Replays + +# Boots tests/harness (production-shape compose topology with TenantGuard, +# /cp/* proxy, canvas proxy, real production Dockerfile.tenant) and runs +# every replay under tests/harness/replays/. Fails the PR if any replay +# fails. +# +# Why this exists: 2026-04-30 we shipped #2398 which added /buildinfo as +# a public route in router.go but forgot to add it to TenantGuard's +# allowlist. The handler-level test in buildinfo_test.go constructed a +# minimal gin engine without TenantGuard — green. The harness's +# buildinfo-stale-image.sh replay would have caught it (cf-proxy doesn't +# inject X-Molecule-Org-Id, so the curl path is identical to production's +# redeploy verifier), but no one ran the harness pre-merge. The bug +# shipped; the redeploy verifier silently soft-warned every tenant as +# "unreachable" for ~1 day before being noticed. +# +# This gate makes "did you actually run the harness?" a CI invariant +# instead of a memory-discipline thing. +# +# Trigger model — match e2e-api.yml: always FIRES on push/pull_request +# to staging+main, real work is gated per-step on detect-changes output. +# One job → one check run → branch-protection-clean (the SKIPPED-in-set +# trap from PR #2264 is documented in e2e-api.yml's e2e-api job comment). + +on: + push: + branches: [main, staging] + paths: + - 'workspace-server/**' + - 'canvas/**' + - 'tests/harness/**' + - '.github/workflows/harness-replays.yml' + pull_request: + branches: [main, staging] + paths: + - 'workspace-server/**' + - 'canvas/**' + - 'tests/harness/**' + - '.github/workflows/harness-replays.yml' + workflow_dispatch: + merge_group: + types: [checks_requested] + +concurrency: + # Per-SHA grouping. Per-ref kept hitting the auto-promote-staging + # cancellation deadlock — see e2e-api.yml's concurrency block for + # the 2026-04-28 incident that codified this pattern. + group: harness-replays-${{ github.event.pull_request.head.sha || github.sha }} + cancel-in-progress: false + +jobs: + detect-changes: + runs-on: ubuntu-latest + outputs: + run: ${{ steps.decide.outputs.run }} + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 + id: filter + with: + filters: | + run: + - 'workspace-server/**' + - 'canvas/**' + - 'tests/harness/**' + - '.github/workflows/harness-replays.yml' + - id: decide + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "run=${{ steps.filter.outputs.run }}" >> "$GITHUB_OUTPUT" + fi + + # ONE job that always runs. Real work is gated per-step on + # detect-changes.outputs.run so an unrelated PR (e.g. doc-only + # change to molecule-controlplane wired here later) emits the + # required check without spending CI cycles. Single-job pattern + # matches e2e-api.yml — see that workflow's comment for why a + # job-level `if: false` would block branch protection via the + # SKIPPED-in-set bug. + harness-replays: + needs: detect-changes + name: Harness Replays + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: No-op pass (paths filter excluded this commit) + if: needs.detect-changes.outputs.run != 'true' + run: | + echo "No workspace-server / canvas / tests/harness / workflow changes — Harness Replays gate satisfied without running." + echo "::notice::Harness Replays no-op pass (paths filter excluded this commit)." + + - if: needs.detect-changes.outputs.run == 'true' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - name: Checkout sibling plugin repo + # Dockerfile.tenant copies molecule-ai-plugin-github-app-auth/ + # at the build-context root (see workspace-server/Dockerfile.tenant + # line 19). PLUGIN_REPO_PAT pattern matches publish-workspace-server-image.yml. + if: needs.detect-changes.outputs.run == 'true' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + repository: Molecule-AI/molecule-ai-plugin-github-app-auth + path: molecule-ai-plugin-github-app-auth + token: ${{ secrets.PLUGIN_REPO_PAT || secrets.GITHUB_TOKEN }} + + - name: Add /etc/hosts entry for harness-tenant.localhost + # ubuntu-latest doesn't auto-resolve *.localhost the way macOS + # sometimes does. seed.sh + replay scripts curl + # http://harness-tenant.localhost:8080 — without the entry + # they'd fail with getaddrinfo ENOTFOUND. + if: needs.detect-changes.outputs.run == 'true' + run: | + echo "127.0.0.1 harness-tenant.localhost" | sudo tee -a /etc/hosts >/dev/null + getent hosts harness-tenant.localhost + + - name: Run all replays against the harness + # run-all-replays.sh: boot via up.sh → seed via seed.sh → run + # every replays/*.sh → tear down via down.sh on EXIT (trap). + # Non-zero exit on any replay failure. + if: needs.detect-changes.outputs.run == 'true' + working-directory: tests/harness + run: ./run-all-replays.sh + + - name: Dump compose logs on failure + if: failure() && needs.detect-changes.outputs.run == 'true' + working-directory: tests/harness + run: | + echo "=== docker compose ps ===" + docker compose -f compose.yml ps || true + echo "=== tenant logs ===" + docker compose -f compose.yml logs tenant || true + echo "=== cp-stub logs ===" + docker compose -f compose.yml logs cp-stub || true + echo "=== cf-proxy logs ===" + docker compose -f compose.yml logs cf-proxy || true + echo "=== postgres logs (last 100) ===" + docker compose -f compose.yml logs --tail 100 postgres || true + + - name: Force teardown (belt-and-suspenders) + # run-all-replays.sh's trap should already have torn down, + # but if something killed bash before the trap fired, this + # ensures the runner doesn't leak the network/volumes. + if: always() && needs.detect-changes.outputs.run == 'true' + working-directory: tests/harness + run: ./down.sh || true From 41d5f9558f46fa49f1507b89e2782a09740271b1 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:13:47 -0700 Subject: [PATCH 17/21] =?UTF-8?q?ops:=20scripts/ops/check-prod-versions.sh?= =?UTF-8?q?=20=E2=80=94=20one-line=20"is=20each=20tenant=20on=20latest=3F"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iterates a list of tenant slugs (default canary set on production, operator-supplied on staging), curls each tenant's /buildinfo plus canvas's /api/buildinfo, compares to origin/main's HEAD SHA, prints a table with one of {current, stale, unreachable} per surface. Returns non-zero if any surface is stale, so it can be wired into a periodic alert later. Why this exists: every "is the fix live?" question used to be answered with a one-off curl + git rev-parse + manual diff. This script does that uniformly across every public surface (workspace tenants + canvas) and is parseable. The redeploy verifier (#2398) covers the deploy moment; this covers any-time-after. Reads EXPECTED_SHA from `gh api repos/Molecule-AI/molecule-core/ commits/main` so it always reflects the actual upstream tip, not local working-copy state. Falls back to local origin/main with a WARN if `gh` isn't logged in — debugging is still useful even if the comparison may lag. Depends on: - #2409 (TenantGuard /buildinfo allowlist) — without it every tenant looks "unreachable" because the route 404s before the handler. Already merged on staging; will hit production after the next staging→main fast-forward + redeploy. - #2407 (canvas /api/buildinfo) — already on main + Vercel. Usage: ./scripts/ops/check-prod-versions.sh # production canary set TENANT_SLUGS="a b c" ./scripts/ops/check-prod-versions.sh # custom set ENV=staging TENANT_SLUGS="..." ./scripts/ops/check-prod-versions.sh Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/ops/check-prod-versions.sh | 112 +++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100755 scripts/ops/check-prod-versions.sh diff --git a/scripts/ops/check-prod-versions.sh b/scripts/ops/check-prod-versions.sh new file mode 100755 index 00000000..88c721e7 --- /dev/null +++ b/scripts/ops/check-prod-versions.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash +# Check whether production tenants and canvas are running latest main. +# +# Usage: +# ./scripts/ops/check-prod-versions.sh # production +# ENV=staging ./scripts/ops/check-prod-versions.sh # staging tenants +# +# Outputs a table of {surface, current_sha, expected_sha, status}. Returns +# non-zero if any surface is stale so this can be wired into a periodic +# alert. +# +# Why this exists: every time someone hits a "is the fix live?" question, +# they have to remember the curl pattern + cross-reference with +# `git rev-parse origin/main`. This script does that check uniformly across +# every public surface (workspace tenants + canvas) and gives a one-line +# verdict instead of a stack of one-off curls. + +set -euo pipefail + +ENV="${ENV:-production}" +EXPECTED_REF="${EXPECTED_REF:-main}" + +case "$ENV" in + production) + TENANT_DOMAIN="moleculesai.app" + CANVAS_URL="https://canvas.moleculesai.app" + # Default canary tenant for production. Override via TENANT_SLUGS= + # to cover a custom set. + DEFAULT_TENANTS="hongmingwang reno-stars" + ;; + staging) + TENANT_DOMAIN="staging.moleculesai.app" + CANVAS_URL="https://canvas-staging.moleculesai.app" + DEFAULT_TENANTS="" # staging tenants are ephemeral; user must specify + ;; + *) + echo "Unknown ENV=$ENV (expected: production | staging)" >&2 + exit 2 + ;; +esac + +TENANT_SLUGS="${TENANT_SLUGS:-$DEFAULT_TENANTS}" + +# Pull EXPECTED_SHA from GitHub. Falls back to local git if gh isn't +# logged in — local main may lag origin but is usually close enough for +# debugging, and we still report the comparison clearly. +EXPECTED_SHA="" +if command -v gh >/dev/null 2>&1; then + EXPECTED_SHA=$(gh api "repos/Molecule-AI/molecule-core/commits/${EXPECTED_REF}" --jq '.sha' 2>/dev/null || true) +fi +if [ -z "$EXPECTED_SHA" ]; then + if git rev-parse "origin/${EXPECTED_REF}" >/dev/null 2>&1; then + EXPECTED_SHA=$(git rev-parse "origin/${EXPECTED_REF}") + echo "[check-prod-versions] WARN: gh unavailable, using local origin/${EXPECTED_REF}=${EXPECTED_SHA:0:7} (may lag)" + else + echo "[check-prod-versions] ERROR: cannot resolve expected SHA — gh not logged in and origin/${EXPECTED_REF} not fetched" >&2 + exit 2 + fi +fi +EXPECTED_SHORT="${EXPECTED_SHA:0:7}" + +echo "Checking ${ENV} surfaces against ${EXPECTED_REF}=${EXPECTED_SHORT}" +echo "" +printf "%-25s %-9s %-9s %s\n" "Surface" "Live" "Expected" "Status" +printf "%-25s %-9s %-9s %s\n" "-------" "----" "--------" "------" + +STALE_COUNT=0 +UNREACHABLE_COUNT=0 + +# Tenant surfaces — workspace-server /buildinfo (added in PR #2398). +for slug in $TENANT_SLUGS; do + URL="https://${slug}.${TENANT_DOMAIN}/buildinfo" + BODY=$(curl -sS --max-time 15 "$URL" 2>/dev/null || echo "") + ACTUAL_SHA=$(echo "$BODY" | jq -r '.git_sha // ""' 2>/dev/null || echo "") + if [ -z "$ACTUAL_SHA" ]; then + printf "%-25s %-9s %-9s ⚠ unreachable\n" "tenant: $slug" "—" "$EXPECTED_SHORT" + UNREACHABLE_COUNT=$((UNREACHABLE_COUNT + 1)) + elif [ "$ACTUAL_SHA" = "$EXPECTED_SHA" ]; then + printf "%-25s %-9s %-9s ✓ current\n" "tenant: $slug" "${ACTUAL_SHA:0:7}" "$EXPECTED_SHORT" + else + printf "%-25s %-9s %-9s ✗ stale\n" "tenant: $slug" "${ACTUAL_SHA:0:7}" "$EXPECTED_SHORT" + STALE_COUNT=$((STALE_COUNT + 1)) + fi +done + +# Canvas — Next.js /api/buildinfo (PR #2407). Vercel injects +# VERCEL_GIT_COMMIT_SHA at build time so this reflects the deployed +# commit, not the request time. +CANVAS_BODY=$(curl -sS --max-time 15 "${CANVAS_URL}/api/buildinfo" 2>/dev/null || echo "") +CANVAS_SHA=$(echo "$CANVAS_BODY" | jq -r '.git_sha // ""' 2>/dev/null || echo "") +if [ -z "$CANVAS_SHA" ]; then + printf "%-25s %-9s %-9s ⚠ unreachable (route may not be deployed yet)\n" "canvas" "—" "$EXPECTED_SHORT" + UNREACHABLE_COUNT=$((UNREACHABLE_COUNT + 1)) +elif [ "$CANVAS_SHA" = "dev" ]; then + printf "%-25s %-9s %-9s ⚠ dev sentinel (Vercel env not injected — check VERCEL_GIT_COMMIT_SHA)\n" "canvas" "dev" "$EXPECTED_SHORT" + UNREACHABLE_COUNT=$((UNREACHABLE_COUNT + 1)) +elif [ "$CANVAS_SHA" = "$EXPECTED_SHA" ]; then + printf "%-25s %-9s %-9s ✓ current\n" "canvas" "${CANVAS_SHA:0:7}" "$EXPECTED_SHORT" +else + printf "%-25s %-9s %-9s ✗ stale\n" "canvas" "${CANVAS_SHA:0:7}" "$EXPECTED_SHORT" + STALE_COUNT=$((STALE_COUNT + 1)) +fi + +echo "" +if [ $STALE_COUNT -eq 0 ] && [ $UNREACHABLE_COUNT -eq 0 ]; then + echo "All surfaces current." + exit 0 +fi +echo "Summary: ${STALE_COUNT} stale, ${UNREACHABLE_COUNT} unreachable." +# Stale is a deploy gap; unreachable is operational (DNS, CF, route absent). +# Both are signal — exit non-zero so cron / CI can alert. +exit 1 From 24cb2a286f531e880c94e9edb71892352e817e50 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:15:46 -0700 Subject: [PATCH 18/21] ci(harness-replays): KEEP_UP=1 so dump-logs step has containers to read First run on PR #2410 failed with 'container harness-tenant-1 is unhealthy' but the dump-compose-logs step printed empty tenant logs because run-all-replays.sh's trap-on-EXIT had already torn down the harness. Setting KEEP_UP=1 leaves containers in place; the always-run Force teardown step at the end owns cleanup explicitly. Now we'll actually see why the tenant didn't become healthy. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/harness-replays.yml | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/harness-replays.yml b/.github/workflows/harness-replays.yml index caf5fe77..384b8567 100644 --- a/.github/workflows/harness-replays.yml +++ b/.github/workflows/harness-replays.yml @@ -120,8 +120,18 @@ jobs: # run-all-replays.sh: boot via up.sh → seed via seed.sh → run # every replays/*.sh → tear down via down.sh on EXIT (trap). # Non-zero exit on any replay failure. + # + # KEEP_UP=1: without this, the script's trap-on-EXIT tears + # down containers immediately on failure, leaving the dump + # step below with nothing to dump (verified on PR #2410's + # first run — tenant became unhealthy, trap fired, dump + # step saw empty containers). Keeping them up lets the + # failure path collect tenant/cp-stub/cf-proxy logs. The + # always-run "Force teardown" step does the actual cleanup. if: needs.detect-changes.outputs.run == 'true' working-directory: tests/harness + env: + KEEP_UP: "1" run: ./run-all-replays.sh - name: Dump compose logs on failure @@ -139,10 +149,10 @@ jobs: echo "=== postgres logs (last 100) ===" docker compose -f compose.yml logs --tail 100 postgres || true - - name: Force teardown (belt-and-suspenders) - # run-all-replays.sh's trap should already have torn down, - # but if something killed bash before the trap fired, this - # ensures the runner doesn't leak the network/volumes. + - name: Force teardown + # We pass KEEP_UP=1 to run-all-replays.sh so the dump step + # above sees real containers — that means we own teardown + # explicitly here. Always run. if: always() && needs.detect-changes.outputs.run == 'true' working-directory: tests/harness run: ./down.sh || true From 630dd0dae7084586ca037ef54cc15a9037562035 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:25:52 -0700 Subject: [PATCH 19/21] fix(harness): seed SECRETS_ENCRYPTION_KEY so MOLECULE_ENV=production tenant boots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found via the first run of the harness-replays-required-check workflow (#2410): the tenant container failed its healthcheck after 100s with "refusing to boot without encryption in production". This is the deferred CRITICAL flagged on PR #2401 — `crypto.InitStrict()` requires SECRETS_ENCRYPTION_KEY when MOLECULE_ENV=production, and the harness sets prod-mode but never seeded a key. Fix: add a clearly-test 32-byte base64 value (encoding the literal string "harness-test-only-not-for-prod!!") inline. Keeping MOLECULE_ENV=production preserves the harness's value as a production- shape replay surface — it now exercises the full encryption boot path including the strict check, rather than skirting it via dev-mode. Why inline rather than .env: - The harness compose file is meant to be self-contained and reproducible from a clean clone. An external .env would split the config across two files for one synthetic value. - The value is intentionally a sentinel; there's no operator decision here to gate behind a per-deployment file. After this lands the harness boots clean and `run-all-replays.sh` can exercise the buildinfo + peer-discovery replays as designed. The required-check workflow itself (#2410) needs no change. --- tests/harness/compose.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/harness/compose.yml b/tests/harness/compose.yml index 867f67bd..3fe185cc 100644 --- a/tests/harness/compose.yml +++ b/tests/harness/compose.yml @@ -85,6 +85,14 @@ services: PORT: "8080" PLATFORM_URL: "http://tenant:8080" MOLECULE_ENV: "production" + # SECRETS_ENCRYPTION_KEY is required when MOLECULE_ENV=production — + # crypto.InitStrict() refuses to boot without it ("32 bytes raw or + # base64-encoded"). The harness uses a clearly-test sentinel so the + # production code path is exercised end-to-end (including the + # encrypted-secret reads/writes) without coupling to a real key. + # Value is base64 of the literal string "harness-test-only-not-for-prod!!" + # (exactly 32 bytes). Do NOT copy this to any other environment. + SECRETS_ENCRYPTION_KEY: "aGFybmVzcy10ZXN0LW9ubHktbm90LWZvci1wcm9kISE=" # ADMIN_TOKEN flips the platform into strict-auth mode (matches # production's CP-minted token configuration). Seeded value lets # E2E scripts authenticate without going through CP. From 9dae0503ee9cd32f3816e7faf6796dc27f42e4a7 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:30:14 -0700 Subject: [PATCH 20/21] fix(harness): generate SECRETS_ENCRYPTION_KEY per-run instead of hardcoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hardcoded base64 sentinel (630dd0da) with a per-run generation in up.sh, exported into compose's interpolation environment. Why: - Hardcoding a 32-byte base64 string in the repo, even one labelled "test-only", sets a bad muscle-memory pattern. The next agent or contributor copies the shape into another harness — or worse, into a staging .env — and the test-only sentinel turns into something someone treats as a real key. - Secret scanners flag key-shaped values regardless of the surrounding comment claiming intent. Avoiding the literal entirely sidesteps the false-positive. - A fresh key per harness lifetime more closely mimics prod's per-tenant isolation, exercising the same code paths without any pretense of stable encrypted-data fixtures (which the harness wipes on every ./down.sh anyway). Implementation: - up.sh: `openssl rand -base64 32` if SECRETS_ENCRYPTION_KEY isn't already set in the caller's env. Honoring a pre-set value lets a debug session pin a key for reproducibility (e.g. when investigating encrypted-row corruption). - compose.yml: `${SECRETS_ENCRYPTION_KEY:?…}` makes a misuse loud — running `docker compose up` directly bypassing up.sh fails fast with a clear error pointing at the right entry point, rather than a 100s unhealthy-tenant timeout. Both paths verified via `docker compose config`: - with key exported: value interpolates cleanly - without it: "required variable SECRETS_ENCRYPTION_KEY is missing a value: must be set — run via tests/harness/up.sh, which generates one per run" --- tests/harness/compose.yml | 14 +++++++------- tests/harness/up.sh | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/harness/compose.yml b/tests/harness/compose.yml index 3fe185cc..1a382a6a 100644 --- a/tests/harness/compose.yml +++ b/tests/harness/compose.yml @@ -86,13 +86,13 @@ services: PLATFORM_URL: "http://tenant:8080" MOLECULE_ENV: "production" # SECRETS_ENCRYPTION_KEY is required when MOLECULE_ENV=production — - # crypto.InitStrict() refuses to boot without it ("32 bytes raw or - # base64-encoded"). The harness uses a clearly-test sentinel so the - # production code path is exercised end-to-end (including the - # encrypted-secret reads/writes) without coupling to a real key. - # Value is base64 of the literal string "harness-test-only-not-for-prod!!" - # (exactly 32 bytes). Do NOT copy this to any other environment. - SECRETS_ENCRYPTION_KEY: "aGFybmVzcy10ZXN0LW9ubHktbm90LWZvci1wcm9kISE=" + # crypto.InitStrict() refuses to boot without it. up.sh generates a + # fresh 32-byte key per harness lifetime via `openssl rand -base64 32` + # and exports it into this compose file's interpolation environment. + # The :? sentinel makes the misuse loud — running `docker compose up` + # directly without going through up.sh fails fast with a clear error + # rather than getting a confusing tenant-unhealthy timeout. + SECRETS_ENCRYPTION_KEY: "${SECRETS_ENCRYPTION_KEY:?must be set — run via tests/harness/up.sh, which generates one per run}" # ADMIN_TOKEN flips the platform into strict-auth mode (matches # production's CP-minted token configuration). Seeded value lets # E2E scripts authenticate without going through CP. diff --git a/tests/harness/up.sh b/tests/harness/up.sh index b3c87936..fbc14910 100755 --- a/tests/harness/up.sh +++ b/tests/harness/up.sh @@ -18,6 +18,22 @@ for arg in "$@"; do esac done +# Generate a per-run encryption key. The tenant runs with +# MOLECULE_ENV=production (intentional, to replay prod-shape bugs), and +# crypto.InitStrict() refuses to boot without SECRETS_ENCRYPTION_KEY. +# Generate fresh so: +# - No key-shaped string lives in the repo (avoids muscle-memorying a +# hardcoded value into other places + secret-scanner false positives). +# - Each harness lifetime gets a unique key, mimicking prod's per-tenant +# isolation. Persistence across runs isn't required — the harness DB +# is wiped on every ./down.sh. +# Honor a caller-supplied value if already exported (lets a debug session +# pin a key for reproducibility). +if [ -z "${SECRETS_ENCRYPTION_KEY:-}" ]; then + SECRETS_ENCRYPTION_KEY=$(openssl rand -base64 32) + export SECRETS_ENCRYPTION_KEY +fi + if [ "$REBUILD" = true ]; then docker compose -f compose.yml build --no-cache tenant cp-stub fi From c8b17ea1ad582929fc525bb7cc19e4ca629a60d0 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 13:32:00 -0700 Subject: [PATCH 21/21] fix(harness): install httpx for replay Python evals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit peer-discovery-404 imports workspace/a2a_client.py which depends on httpx; the runner's stock Python doesn't have it, so the replay's PARSE assertion (b) fails with ModuleNotFoundError on every run. The WIRE assertion (a) — pure curl — passes, so the failure was masking just enough to make the replay LOOK partially-broken when the tenant side is fine. Adding tests/harness/requirements.txt with only httpx instead of sourcing workspace/requirements.txt: that file pulls a2a-sdk, langchain-core, opentelemetry, sqlalchemy, temporalio, etc. — ~30s of install for one replay's PARSE step. The harness's deps surface should grow when a new replay introduces a new import, not by default. Workflow gains one step (`pip install -r tests/harness/requirements.txt`) between the /etc/hosts setup and run-all-replays. No other changes. --- .github/workflows/harness-replays.yml | 9 +++++++++ tests/harness/requirements.txt | 14 ++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/harness/requirements.txt diff --git a/.github/workflows/harness-replays.yml b/.github/workflows/harness-replays.yml index 384b8567..6330e885 100644 --- a/.github/workflows/harness-replays.yml +++ b/.github/workflows/harness-replays.yml @@ -116,6 +116,15 @@ jobs: echo "127.0.0.1 harness-tenant.localhost" | sudo tee -a /etc/hosts >/dev/null getent hosts harness-tenant.localhost + - name: Install Python deps for replays + # peer-discovery-404 (and future replays) eval Python against the + # running tenant — importing workspace/a2a_client.py pulls in + # httpx. tests/harness/requirements.txt holds just the HTTP-client + # surface to keep CI install fast (~3s) vs the full + # workspace/requirements.txt (~30s). + if: needs.detect-changes.outputs.run == 'true' + run: pip install -r tests/harness/requirements.txt + - name: Run all replays against the harness # run-all-replays.sh: boot via up.sh → seed via seed.sh → run # every replays/*.sh → tear down via down.sh on EXIT (trap). diff --git a/tests/harness/requirements.txt b/tests/harness/requirements.txt new file mode 100644 index 00000000..75a30722 --- /dev/null +++ b/tests/harness/requirements.txt @@ -0,0 +1,14 @@ +# Harness-replay Python deps — minimal set for replays/*.sh scripts that +# eval Python against the running tenant (e.g. importing +# workspace/a2a_client.py to assert parser behavior). +# +# This is intentionally smaller than workspace/requirements.txt: the +# replays don't need a2a-sdk, langchain, opentelemetry, etc. — only the +# HTTP client surface that the imported helpers depend on. Adding the +# full workspace deps would slow every harness CI run by ~30s for no +# gain. +# +# Add a line here (with a version constraint matching workspace/requirements.txt) +# when a new replay introduces a new Python import. + +httpx>=0.28.1